1026475 - Device switch from Handset/Headset to BTSCO fails during call

Status: RESOLVED FIXED

(Firefox OS Graveyard :: Bluetooth, defect)

Description

[vasanth]

Steps to reproduce
-------------------
1. Connect BT headset device
2. Make a voice call
3. BT device will be enabled and speech is transmitted fine
4. Insert normal headset in to DUT
5. Switch from BT to normal headset and the device switch will be successful
6. Again switch back from headset to BT, observe that the device switch fails and you can still hear voice on headset only
7. Now switch from headset to speaker and the device switch will be successful.
8. Now try switching from speaker to BTSCO, observe that the device switch fails, instead of switching to BT device switch happens from speaker to headset.
9. Repeat the above steps with handset in place of headset and observe the same issue.

Expected:
--------
Always switch to BT headset should be successful when it is connected.

Analysis
--------
This is a regression caused by Bug 989233 fix.
That fix introduces doNotConnect arg to calls_handler.js switchToDefaultOut()

call_screen.js switchToDefaultOut() is binded to switchToDeviceButton onClick event [1]
When user clicks that button, switchToDefaultOut() is called with "evt" arg but it is assumed as doNotConnect arg here which is wrong [2]

Correct thing to do is like below patch. [4]
But with that call_screen::switchToDeafultOut is called twice, 2nd time from here [3]
Not sure if its ok?

Issue seen in 1.4 as well.

[1] https://github.com/mozilla-b2g/gaia/blob/master/apps/callscreen/js/call_screen.js#L127
[2] https://github.com/mozilla-b2g/gaia/blob/master/apps/callscreen/js/call_screen.js#L402
[3] https://github.com/mozilla-b2g/gaia/blob/master/apps/callscreen/js/calls_handler.js#L62

[4] Suggested patch
--- a/apps/communications/dialer/js/call_screen.js
+++ b/apps/communications/dialer/js/call_screen.js
@@ -120,7 +120,7 @@ var CallScreen = {
                                     this.hideGroupDetails.bind(this));
 
     this.switchToDeviceButton.addEventListener('click',
-                                    this.switchToDefaultOut.bind(this));
+                                    this.switchToBTHeadset.bind(this));
@@ -369,11 +369,17 @@ var CallScreen = {
     this.toggleBluetoothMenu(false);
   },
 
+  switchToBTHeadset: function cs_switchToBTHeadset() {
+    this.switchToDefaultOut(false /* Connect to BT device if available*/);
+  },

Comment 1

[Preeti Raghunath(:Preeti)]

Vasanth,

Was this found in dogfood testing or stability testing? 

Thanks for pointing out that its a regression issue.

Comment 2

[Preeti Raghunath(:Preeti)]

Vasanth,

Was this found in dogfood testing or stability testing? 

Thanks for pointing out that its a regression issue.

Comment 3

[Inder]

Preeti -- this was from the regular testing. In our initial triage we were first attributing it to some internal changes but Vasanth helped identify it to be this issue.

Comment 4

[Ben Tian (inactive)]

Attachment: (m-c) link to PR

The patch passes false as 1st parameter for mouse event case, and connects BT SCO even if headphone is plugged in.

Comment 5

[Etienne Segonzac (:etienne)]

Comment on attachment 8441830 [details]
(m-c) link to PR

r=me with the small comment addressed

Comment 7

[Ben Tian (inactive)]

(In reply to Etienne Segonzac (:etienne) from comment #5)
> Comment on attachment 8441830 [details]
> (m-c) link to PR
> 
> r=me with the small comment addressed

Thanks! Updated PR and will merge it once all tests are passed.

Comment 8

[Preeti Raghunath(:Preeti)]

(In reply to Ben Tian [:btian] from comment #7)
> (In reply to Etienne Segonzac (:etienne) from comment #5)
> > Comment on attachment 8441830 [details]
> > (m-c) link to PR
> > 
> > r=me with the small comment addressed
> 
> Thanks! Updated PR and will merge it once all tests are passed.

Is this impacting 2.0 as well?

Comment 9

[Ben Tian (inactive)]

(In reply to Preeti Raghunath(:Preeti) from comment #8)
> Is this impacting 2.0 as well?

Yes. m-c, 2.0, and 1.4 are all affected.

Comment 10

[Ben Tian (inactive)]

Merged to gaia-master: https://github.com/mozilla-b2g/gaia/commit/2d50292b3f4f8adce0680a9cd2df76f339f55ff3

Comment 11

[Ben Tian (inactive)]

Attachment: (1.4) link to PR, r=etienne

Create PR for 1.4 as these files have been relocated.

Comment 12

[Inder]

(In reply to Ben Tian [:btian] from comment #9)
> (In reply to Preeti Raghunath(:Preeti) from comment #8)
> > Is this impacting 2.0 as well?
> 
> Yes. m-c, 2.0, and 1.4 are all affected.

Please upload the fix to 2.0 as well. Thanks.

Comment 13

[Ben Tian (inactive)]

gaia v1.4: https://github.com/mozilla-b2g/gaia/commit/dff52e807187955300bdfcc880692d2875e37b40

Note travis build fails due to irrelevant mozAlarm errors, so I still land my patch to 1.4.

Comment 14

[Ben Tian (inactive)]

Set 'checkin-needed' to uplift this fix to gaia 2.0.

Comment 15

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

This bug also blocks Bluetooth certification.

Comment 16

[Ryan VanderMeulen [:RyanVM]]

v2.0:

Comment 17

[Ryan VanderMeulen [:RyanVM]]

v2.0: https://github.com/mozilla-b2g/gaia/commit/509ee4025b8e8bd72ae5de6593239aa81ef4274a

Comment 20

[[:SalvadorR]]

This will need a new test case to be covered

Comment 21

[KTucker [:KTucker][Inactive 3/4/2016]]

Test case added in moztrap:

https://moztrap.mozilla.org/manage/case/14372/