Closed Bug 872890 Opened 7 years ago Closed 7 years ago

[bluetooth] callstate is not updated when call end because unknown reason.

Categories

(Firefox OS Graveyard :: Bluetooth, defect, P1)

x86
Gonk (Firefox OS)
defect

Tracking

(blocking-b2g:leo+, firefox22 wontfix, firefox23 wontfix, firefox24 fixed, b2g18 verified, b2g18-v1.0.0 wontfix, b2g18-v1.0.1 wontfix, b2g-v1.1hd fixed)

VERIFIED FIXED
1.1 QE2 (6jun)
blocking-b2g leo+
Tracking Status
firefox22 --- wontfix
firefox23 --- wontfix
firefox24 --- fixed
b2g18 --- verified
b2g18-v1.0.0 --- wontfix
b2g18-v1.0.1 --- wontfix
b2g-v1.1hd --- fixed

People

(Reporter: leo.bugzilla.gecko, Assigned: shawnjohnjr)

Details

(Whiteboard: [fixed-in-birch])

Attachments

(3 files, 9 obsolete files)

5.08 KB, text/plain
Details
1.32 KB, patch
Details | Diff | Splinter Review
1.34 KB, patch
Details | Diff | Splinter Review
Dear Mozilla Team,

In active call, call ended because unknown reason.
So, callstate is not updated and at this time if incoming call is received, no action anymore by headset ag key.
Because call state keep going wrong.

01-13 14:42:13.240 V       150      BluetoothHfpManager    HandleCallStateChanged:1236  aCallIndex = 1, aCallState = 5, aSend = 1
01-13 14:42:13.240 V       150      BluetoothHfpManager    SendLine:1100  sending message = +CIEV: 2,1
01-13 14:42:13.240 V       150      BluetoothHfpManager    SendLine:1100  sending message = +CIEV: 4,0
01-13 14:42:37.560 V       150      BluetoothHfpManager    SendLine:1100  sending message = +CIEV: 6,5
01-13 14:43:17.240 V       150      BluetoothHfpManager    HandleCallStateChanged:1236  aCallIndex = 1, aCallState = 11, aSend = 1

In my opinion, BluetoothHfpManager need exception handling for callstate in HandleCallStateChanged.

Please check it.
Thanks.
Assignee: nobody → shuang
blocking-b2g: --- → leo?
Priority: -- → P1
Severity: normal → critical
Thanks for reporting. I will investigate this bug as the first priority.
Target Milestone: --- → 1.1 QE2
It shall be ok to add exceptional check. But is it normal that call state change from connected to incoming then disconnected?
It's weired for me to see callindex=1, and call state change from connected to incoming.
But I agree we can add extra check. And I want to know how to reproduce this bug? And with what kind of RIL interface?


01-13 14:42:13.240 V       150      BluetoothHfpManager    HandleCallStateChanged:1236  aCallIndex = 1, aCallState = 5, aSend = 1

01-13 14:43:17.240 V       150      BluetoothHfpManager    HandleCallStateChanged:1236  aCallIndex = 1, aCallState = 11, aSend = 1

01-13 14:43:40.840 V       150      BluetoothHfpManager    HandleCallStateChanged:1236  aCallIndex = 1, aCallState = 10, aSend = 1
Flags: needinfo?(leo.bugzilla.gecko)
Dear Mozilla Team,

After the 3G network is disconnected, I was able to find this bug.
In order to ease, this seems to be reproduced as out of range or put in sheild box.
To explain more details about the log,

aCallIndex = 1, aCallState = 5, aSend = 1 => Active call state
.....
<Actually, network is disconnected at this time , so call is ended but call state is not updated.>
.....
aCallIndex = 1, aCallState = 11, aSend = 1 => Incomming call occurs but call state is not cleared, so headset status seem to go wrong !

Please check this.
Thanks.
Flags: needinfo?(leo.bugzilla.gecko)
Thanks. It looks like a very common use case if user enters elevator. And when next incoming call, user cannot answer call anymore via Bluetooth headset anymore.
blocking-b2g: leo? → leo+
Shawn, can you elaborate what testing you want done?   once you specify details, please add qawanted to the keyword field.   that's the correct process to get QA attention on dev requested bugs.

thanks!
Tony, sorry for the confusion. I'm thinking we probably need to add some out of range test cases for bluetooth as Comment 5 mentioned (probably manual test cases). Anyway, I can reproduce this bug by myself, so qawanted is not required.
re-schedule priority, i will finish it before May27.
Hi Shawn,

I wonder that this bug is fixed.
Please let me know.

Thanks.
> <Actually, network is disconnected at this time , so call is ended but call
> state is not updated.>
> .....
> aCallIndex = 1, aCallState = 11, aSend = 1 => Incomming call occurs but call
> state is not cleared, so headset status seem to go wrong !
We will reset call array when we got call state became CALL_STATE_DISCONNECTED.
When GSM/3G network signal is lost, we suppose and expect CALL_STATE_DISCONNECTED shall be sent from RIL network? Add RIL team expert htsai for discussion here.
Flags: needinfo?(htsai)
(In reply to Shawn Huang from comment #12)
> > <Actually, network is disconnected at this time , so call is ended but call
> > state is not updated.>
> > .....
> > aCallIndex = 1, aCallState = 11, aSend = 1 => Incomming call occurs but call
> > state is not cleared, so headset status seem to go wrong !
> We will reset call array when we got call state became
> CALL_STATE_DISCONNECTED.
> When GSM/3G network signal is lost, we suppose and expect
> CALL_STATE_DISCONNECTED shall be sent from RIL network? Add RIL team expert
> htsai for discussion here.

Hi, 
nsITelephonyListener defines two callbacks: callStateChanged() and notifyError().
Every listener would need to handle them both in order to not miss any related state transition. If a call is released abnormally, then notifyError() will be called, instead of callStateChanged(). That's why you don't see an internal transition to call_state_disconnected. Let me know if anything I can help :)
Flags: needinfo?(htsai)
Comment on attachment 754674 [details] [diff] [review]
Reset call array if call ended abnormally (WIP)

Review of attachment 754674 [details] [diff] [review]:
-----------------------------------------------------------------

::: dom/bluetooth/BluetoothRilListener.cpp
@@ +65,5 @@
> +  // via setting CALL_STATE_DISCONNECTED
> +  hfp->HandleCallStateChanged(aCallIndex,
> +                              nsIRadioInterfaceLayer::CALL_STATE_DISCONNECTED,
> +                              EmptyString(), false, false);
> +

We might want to show the error message in BT_WARNING.
Attachment #754674 - Flags: feedback?(gyeh) → feedback+
Hi leo,
Since we're based on different RIL. Can you help to double confirm with your own RIL?
Flags: needinfo?(leo.bugzilla.gecko)
cancel ni?. Due to i still found some problems
Flags: needinfo?(leo.bugzilla.gecko)
Comment on attachment 754685 [details] [diff] [review]
Reset call array if call ended abnormally (WIP)

+  hfp->HandleCallStateChanged(aCallIndex,
+                              nsIRadioInterfaceLayer::CALL_STATE_DISCONNECTED,
+                              EmptyString(), false, false)
The last argument should be true. I need to update call status correctly, reset call array is not enough. I will update newer version.
Attachment #754685 - Attachment is obsolete: true
How to verify:
1. Make an incoming call with BT headset connected
2. Turn on airplane mode to switch off radio
3. Call will be dropped, no CALL_STATE_DISCONECTED and this can simulate the network problem
4. We will get NotifyError() callback
5. Verify call array will be reset and double confirm CIEV: 2,0 will be sent
Attachment #754722 - Flags: review?(gyeh)
Attachment #754722 - Flags: review?(echou)
Attachment #754722 - Flags: review?(gyeh)
Attachment #754722 - Flags: review?(echou)
Attached patch Patch1:V2 for central (obsolete) — Splinter Review
Attachment #754727 - Flags: review?(gyeh)
Attachment #754727 - Flags: review?(echou)
Will leo help to check your RIL will call NotifyError() callback if you encounter mobile network error?
Flags: needinfo?(leo.bugzilla.gecko)
Hi Shawn,

I applied this patch and it is working well in our RIL.
I have tested with lab's shield room and call state is update normally after network is disconnected. So state of headset has no problem.
Thanks for your help.

Thanks.
Flags: needinfo?(leo.bugzilla.gecko)
Comment on attachment 754727 [details] [diff] [review]
Patch1:V2 for central

Review of attachment 754727 [details] [diff] [review]:
-----------------------------------------------------------------

Seems to work :)
Attachment #754727 - Flags: review?(gyeh) → review+
Comment on attachment 754727 [details] [diff] [review]
Patch1:V2 for central

Review of attachment 754727 [details] [diff] [review]:
-----------------------------------------------------------------

Looks good. Thanks!

::: dom/bluetooth/BluetoothTelephonyListener.cpp
@@ +75,5 @@
> +  // via setting CALL_STATE_DISCONNECTED
> +  hfp->HandleCallStateChanged(aCallIndex,
> +                              nsIRadioInterfaceLayer::CALL_STATE_DISCONNECTED,
> +                              EmptyString(), false, true);
> +  NS_WARNING("Reset the call state due to call transition ends abnormally");

nit: let's print out the error message as well.
Attachment #754727 - Flags: review?(echou) → review+
Attached patch Final:Bug872890-central.patch (obsolete) — Splinter Review
Add NS_ERROR
Attachment #755169 - Flags: review?(gyeh)
Attachment #755169 - Attachment is obsolete: true
Attachment #755169 - Flags: review?(gyeh)
Attachment #755171 - Attachment description: Final:Bug872890-central.patch → Final:Bug872890-b2g18.patch
Attachment #755171 - Attachment filename: Bug872890.patch → Bug872890-b2g18.patch
Attachment #755263 - Attachment description: Bug 872890:Reset HFP call array when RIL reports NotifyError(), r=echou → b2g18-Bug 872890:Reset HFP call array when RIL reports NotifyError(), r=echou
Whiteboard: [fix-in-birch]
Comment on attachment 755258 [details] [diff] [review]
Final: Bug 872890:Reset HFP call array when RIL reports NotifyError(), r=echou

Review of attachment 755258 [details] [diff] [review]:
-----------------------------------------------------------------

::: dom/bluetooth/BluetoothTelephonyListener.cpp
@@ +73,5 @@
> +  // If a call is released abnormally, NotifyError() will be called,
> +  // instead of CallStateChanged(). We need to reset the call array state
> +  // via setting CALL_STATE_DISCONNECTED
> +  hfp->HandleCallStateChanged(aCallIndex,
> +                              nsIRadioInterfaceLayer::CALL_STATE_DISCONNECTED,

We should use |nsITelephonyProvider| rather than |nsIRadioInterfaceLayer| for m-c.

Please update your patch. Thanks.
https://hg.mozilla.org/mozilla-central/rev/662f6427c909
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Whiteboard: [fix-in-birch] → [fixed-in-birch]
And backed out for breaking B2G ARM debug builds (which we don't run on birch...sigh...).
https://hg.mozilla.org/mozilla-central/rev/4c7a395fadcc

https://tbpl.mozilla.org/php/getParsedLog.php?id=23633156&tree=Mozilla-Central
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
(In reply to Ryan VanderMeulen [:RyanVM] from comment #38)
> And backed out for breaking B2G ARM debug builds (which we don't run on
> birch...sigh...).
> https://hg.mozilla.org/mozilla-central/rev/4c7a395fadcc
> 
> https://tbpl.mozilla.org/php/getParsedLog.php?id=23633156&tree=Mozilla-
> Central

Thanks, Ryan. We should be more careful.
https://hg.mozilla.org/mozilla-central/rev/bfae538a757d
Status: REOPENED → RESOLVED
Closed: 7 years ago7 years ago
Resolution: --- → FIXED
Flags: in-moztrap-
Verified fixed on:
Device: Leo phone 
Build Identifier: 20130611074722
Update channel: leo/1.1.0/nightly
Gecko: http://hg.mozilla.org/releases/mozilla-b2g18/rev/8d0562d20324
Gaia: 8c64e19b82d4e0490a7780232d3d6bd07d0ba9ec1370937291
Git commit info: 2013-06-11 07:54:51 
OS version: 1.1.0.0-prerelease

Prerequisites: 
2 phones and 1 bluetooth headset are needed. The B2G phone should have bluetooth turned on and paired with the bluetooth headset.

Steps to Reproduce:
1. Make a phone call from the second phone to the B2G phone.
2. When the B2G phone gets the incoming call, accept the call through the bluetooth headset.
3. While the call is active, turn on airplane mode in the B2G phone to simulate disconnection from cellular network. (Other ways of simulating this are walking into an elevator, physically moving out of range like going into a tunnel, or placing the phone in a shield box.)
3. Note that the call drops.
4. Turn on airplane mode in the B2G phone to simulate re-connection with cellular network. (Other ways of simulating this are walking out of an elevator, physically moving into cellular coverage like coming out of a tunnel, or removing the phone from the shield box.) 
5. Make another phone call from the second phone to the B2G phone.
6. When the B2G phone gets the incoming call, accept the call through the bluetooth headset.

Expected Result:
The bluetooth headset should be able to handle the incoming call.

Actual Result:
Matches the expected result.
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.