Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[ofono-binder-plugin] Treat no call as a remote hangup. #29

Merged
merged 1 commit into from Feb 26, 2024

Conversation

dcaliste
Copy link
Contributor

@dcaliste dcaliste commented Jan 26, 2024

On an incoming call, when the caller hangup
before pickup the call on device, the reason
is set as OFONO_DISCONNECT_REASON_ERROR, which
makes the device emitting three tones at full
blow.

This is due to the following exchange in the
modem:

  • slot1 > 2 callStateChanged
  • slot1 < [00000157] 10 getCurrentCalls
  • slot1 > [00000157] 140 getCurrentCallsResponse_1_2
  • slot1 < [0000015b] 19 getLastCallFailCause
  • slot1 > [0000015b] 18 getLastCallFailCauseResponse

The current calls response contains no call,
so self->calls in binder_voicecall.c is emptied.
Then, the getLastCallFailCauseResponse, gives
RADIO_LAST_CALL_FAIL_NORMAL_UNSPECIFIED as a reason, so the code look for the status of the call. Which has been deleted from the list.

@monich I don't know if you would have fixed that issue like that. Don't hesitate to comment !

I'm getting this full blow tones on every remote hangup on an Xperia 10ii using 4.5.0.21, with French "Free" operator. If you want a full log, no problem to send you one, or put it here for completeness.

@mlehtima
Copy link
Contributor

Based on code https://github.com/mer-hybris/ofono-binder-plugin/blob/master/src/binder_voicecall.c#L379 it should be possible to check for value -1 which indicates call was NULL.

@dcaliste
Copy link
Contributor Author

Yes, but -1 is not a value of the enum... Can we do:

enum foo
{
  a = 0,
  b
};
enum foo val = -1;

If so, indeed, I can just check -1 in the if.

Besides, I'm wondering if the two RIL commands getCurrentCalls and getLastCallFailCause were inverted, if it would give the desired result ? But I don't know where they are defined... They seem to come from src/alien-radio-service.c but I don't know where this file belongs to...

@dcaliste
Copy link
Contributor Author

In case you need a JB or something related to a bug report, I've found this from the forum : https://forum.sailfishos.org/t/audible-tone-after-ignoring-call/2718/9

@pvuorela
Copy link

Yes, but -1 is not a value of the enum... Can we do:

Then again it's already having -1 returned as enum value.

Oh well, if we want to honor enum typing, perhaps the binder_voicecall_status_with_id(), being static function and having only two calls for it, could be equipped with some "bool *ok" parameter and check with that whether something was found.

On an incoming call, when the caller hangup
before pickup the call on device, the reason
is set as OFONO_DISCONNECT_REASON_ERROR, which
makes the device emitting three tones at full
blow.

This is due to the following exchange in the
modem:
- slot1 > 2 callStateChanged
- slot1 < [00000157] 10 getCurrentCalls
- slot1 > [00000157] 140 getCurrentCallsResponse_1_2
- slot1 < [0000015b] 19 getLastCallFailCause
- slot1 > [0000015b] 18 getLastCallFailCauseResponse
The current calls response contains no call,
so self->calls in binder_voicecall.c is emptied.
Then, the getLastCallFailCauseResponse, gives
RADIO_LAST_CALL_FAIL_NORMAL_UNSPECIFIED as a reason,
so the code look for the status of the call. Which
has been deleted from the list.
@dcaliste
Copy link
Contributor Author

I've updated the PR with simply checking the value -1. I've added a comment inline to explain the case.

@mlehtima mlehtima merged commit 4e703c4 into mer-hybris:master Feb 26, 2024
@dcaliste
Copy link
Contributor Author

Thanks for the reviews and accepting the patch !

@mlehtima
Copy link
Contributor

I need to check if the other ofono plugin have need for similar change.

@dcaliste
Copy link
Contributor Author

Indeed, I may have done it myself. Older devices not using the binder code, like in ofono-ril-plugin have the exact same code. I can submit a PR there also.

@mlehtima
Copy link
Contributor

Feel free to make a PR also to ofono-ril-plugin so we can keep these in sync.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants