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

Status: VERIFIED FIXED

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

Description

[leo.bugzilla.gecko]

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.

Comment 1

[leo.bugzilla.gecko]

Attachment: call_state_is_not_updated

Comment 2

[Shawn Huang [:shawnjohnjr][:shuang] (as a happy gecko contributor)]

Thanks for reporting. I will investigate this bug as the first priority.

Comment 3

[Shawn Huang [:shawnjohnjr][:shuang] (as a happy gecko contributor)]

It shall be ok to add exceptional check. But is it normal that call state change from connected to incoming then disconnected?

Comment 4

[Shawn Huang [:shawnjohnjr][:shuang] (as a happy gecko contributor)]

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

Comment 5

[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.

Comment 6

[Shawn Huang [:shawnjohnjr][:shuang] (as a happy gecko contributor)]

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.

Comment 7

[Shawn Huang [:shawnjohnjr][:shuang] (as a happy gecko contributor)]

Add tchung for testing.

Comment 8

[Tony Chung [:tchung]]

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!

Comment 9

[Shawn Huang [:shawnjohnjr][:shuang] (as a happy gecko contributor)]

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.

Comment 10

[Shawn Huang [:shawnjohnjr][:shuang] (as a happy gecko contributor)]

re-schedule priority, i will finish it before May27.

Comment 11

[leo.bugzilla.gecko]

Hi Shawn,

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

Thanks.

Comment 12

[Shawn Huang [:shawnjohnjr][:shuang] (as a happy gecko contributor)]

> <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.

Comment 13

[Hsin-Yi Tsai (she/her) [:hsinyi]]

(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 :)

Comment 14

[Shawn Huang [:shawnjohnjr][:shuang] (as a happy gecko contributor)]

Attachment: Reset call array if call ended abnormally (WIP)

Comment 15

[Gina Yeh [:gyeh] [:ginayeh]]

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.

Comment 16

[Shawn Huang [:shawnjohnjr][:shuang] (as a happy gecko contributor)]

Attachment: Reset call array if call ended abnormally (WIP)

Update with NS_Warning

Comment 17

[Shawn Huang [:shawnjohnjr][:shuang] (as a happy gecko contributor)]

Hi leo,
Since we're based on different RIL. Can you help to double confirm with your own RIL?

Comment 18

[Shawn Huang [:shawnjohnjr][:shuang] (as a happy gecko contributor)]

cancel ni?. Due to i still found some problems

Comment 19

[Shawn Huang [:shawnjohnjr][:shuang] (as a happy gecko contributor)]

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.

Comment 20

[Shawn Huang [:shawnjohnjr][:shuang] (as a happy gecko contributor)]

Attachment: Patch1-V2-Bug 872890-b2g18 version

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

Comment 21

[Shawn Huang [:shawnjohnjr][:shuang] (as a happy gecko contributor)]

Attachment: Patch1:V2 for central

Comment 22

[Shawn Huang [:shawnjohnjr][:shuang] (as a happy gecko contributor)]

Will leo help to check your RIL will call NotifyError() callback if you encounter mobile network error?

Comment 23

[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.

Comment 24

[Gina Yeh [:gyeh] [:ginayeh]]

Comment on attachment 754727 [details] [diff] [review]
Patch1:V2 for central

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

Seems to work :)

Comment 25

[Eric Chou [:ericchou] [:echou]]

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.

Comment 26

[Shawn Huang [:shawnjohnjr][:shuang] (as a happy gecko contributor)]

Attachment: Final:Bug872890-central.patch

Add NS_ERROR

Comment 27

[Shawn Huang [:shawnjohnjr][:shuang] (as a happy gecko contributor)]

Attachment: Bug872890 for b2g18

Comment 28

[Shawn Huang [:shawnjohnjr][:shuang] (as a happy gecko contributor)]

Attachment: Final:Bug872890-b2g18.patch

Comment 29

[Shawn Huang [:shawnjohnjr][:shuang] (as a happy gecko contributor)]

Attachment: Final:Bug872890-central.patch

Comment 30

[Shawn Huang [:shawnjohnjr][:shuang] (as a happy gecko contributor)]

Attachment: Final: Bug 872890:Reset HFP call array when RIL reports NotifyError(), r=echou

Comment 31

[Shawn Huang [:shawnjohnjr][:shuang] (as a happy gecko contributor)]

Attachment: b2g18-Bug 872890:Reset HFP call array when RIL reports NotifyError(), r=echou

Comment 32

[Gina Yeh [:gyeh] [:ginayeh]]

https://hg.mozilla.org/projects/birch/rev/8cdcc0221c0f

Comment 33

[Gina Yeh [:gyeh] [:ginayeh]]

Backout for build bustage.

https://hg.mozilla.org/projects/birch/rev/b06c62cc684e

Comment 34

[Gina Yeh [:gyeh] [:ginayeh]]

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.

Comment 35

[Shawn Huang [:shawnjohnjr][:shuang] (as a happy gecko contributor)]

Attachment: Final: Bug 872890:Reset HFP call array when RIL reports NotifyError(), r=echou

Updated

Comment 36

[Gina Yeh [:gyeh] [:ginayeh]]

http://hg.mozilla.org/projects/birch/rev/662f6427c909

Comment 37

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/662f6427c909

Comment 38

[Ryan VanderMeulen [:RyanVM]]

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

Comment 39

[Eric Chou [:ericchou] [:echou]]

(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.

Comment 40

[Eric Chou [:ericchou] [:echou]]

https://hg.mozilla.org/projects/birch/rev/bfae538a757d

Comment 41

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/bfae538a757d

Comment 42

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-b2g18/rev/bdbc09b30984

Comment 43

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-b2g18_v1_1_0_hd/rev/bdbc09b30984

Comment 44

[Parul Mathur]

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.