Closed Bug 1030912 Opened 6 years ago Closed 5 years ago

[B2G][Dialer][Bluetooth] While in a call the Speaker icon is shown instead of the Bluetooth Headset icon

Categories

(Firefox OS Graveyard :: Gaia::Dialer, defect)

ARM
Gonk (Firefox OS)
defect
Not set

Tracking

(b2g-v1.3T wontfix, b2g-v1.4 fixed, b2g-v2.0 verified, b2g-v2.1 verified)

RESOLVED FIXED
2.0 S6 (18july)
Tracking Status
b2g-v1.3T --- wontfix
b2g-v1.4 --- fixed
b2g-v2.0 --- verified
b2g-v2.1 --- verified

People

(Reporter: jschmitt, Assigned: drs)

References

Details

(Keywords: regression, Whiteboard: [2.0-daily-testing])

Attachments

(5 files, 1 obsolete file)

Attached image 2014-06-26-15-02-27.png
adding qawanted to check other branches/devices.
QA Whiteboard: [QAnalyst-Triage?]
Flags: needinfo?(jmitchell)
Keywords: qawanted
QA Whiteboard: [QAnalyst-Triage?]
Flags: needinfo?(jmitchell)
QA Contact: ckreinbring
The bug repros on Flame 2.1, Flame 2.0 and Buri 2.0

Flame 2.1
Build ID: 20140702040207
Gaia: 85e97290431ce6aa0a965421e84d6070cd899129
Gecko: 7075808c3306
Platform Version: 33.0a1
Firmware Version: v122
User Agent: Mozilla/5.0 (Mobile; rv:33.0) Gecko/33.0 Firefox/33.0

Flame 2.0
Build ID: 20140702000201
Gaia: 3bfe47c58c959c42f5ffe0309b5380ea514ccd69
Gecko: f40e767ea283
Platform Version: 32.0a2
Firmware Version: v122
User Agent: Mozilla/5.0 (Mobile; rv:32.0) Gecko/32.0 Firefox/32.0

Buri 2.0
Build ID: 20140703064557
Gaia: 26f82bf95d237272e66955bbca3323ccf8fcb74a
Gecko: 747a8c44ff44
Platform Version: 32.0a2 (2.0)
Firmware Version: v1.2device.cfg
User Agent: Mozilla/5.0 (Mobile; rv:32.0) Gecko/32.0 Firefox/32.0

Actual result: After connecting to a Bluetooth audio device, the icon on the far right of the in-call screen will show a speaker for the first call and a headphone icon on subsequent calls.

--------------------------------------------------------------------------------------------------------

The bug does not repro on Flame 1.4

Build ID: 20140630000201
Gaia: aa896d5db1b4929f3bf31a0f4bb7de50530222a8
Gecko: 8cba60bc12ef
Platform Version: 30.0
Firmware Version: v122
User Agent: Mozilla/5.0 (Mobile; rv:30.0) Gecko/30.0 Firefox/30.0

Actual result: After connecting to a Bluetooth audio device, the icon on the far right of the in-call screen will show a headphone icon on all calls.
QA Whiteboard: [QAnalyst-Triage?]
Flags: needinfo?(jmitchell)
Keywords: qawanted
issue is a regression but not nomming - issue seems cosmetic
QA Whiteboard: [QAnalyst-Triage?] → [QAnalyst-Triage+]
Flags: needinfo?(jmitchell)
Is the fact that a speaker icon, rather than a bluetooth icon, showing in the dialer app during a call with a bluetooth headset a cert issue or not?
Flags: needinfo?(beatriz.rodriguezgomez)
The behaviour must be consistent. The difference of the behaviour will block the certification. It seems that the headphone icon is the suitable option.
Flags: needinfo?(beatriz.rodriguezgomez)
Nominating accordingly.
blocking-b2g: --- → 2.0?
blocking-b2g: 2.0? → 2.0+
QA Whiteboard: [QAnalyst-Triage+] → [QAnalyst-Triage+][lead-review+]
QA Whiteboard: [QAnalyst-Triage+][lead-review+]
Attached image headset-paired.png
When a headset is paired, the status bar shows a headset around the bluetooth icon. In attachment 8446704 [details], that headset is not present in the status bar. So I don't think this is a Dialer bug but rather a pairing issue.
Assignee: nobody → drs+bugzilla
Target Milestone: --- → 2.0 S6 (18july)
I was able to repro this with the original STR on master.
Status: NEW → ASSIGNED
This only repros with a new profile.

New Repro Steps:
1) Update a Flame to v2.0 or v2.1/master
2) Clear the profile using |NOFTU=1 make reset-gaia|. If you used B2G-flash-tool, it deletes your profile for you.
3) Open Settings > Bluetooth
4) Connect to a Bluetooth Headset
5) With a 2nd device call the DUT

Actual:
The speaker icon is displayed and the Bluetooth Headset icon is not

Expected:
The Bluetooth Headset icon is displayed while connected to a Bluetooth Headset
Tracked down to commit 7afce8a6e965a45f38988227828ccdfb315394f3
QA Whiteboard: [QAnalyst-Triage+]
This is a regression. It affects every current branch because bug 990003 was uplifted to all of them.
blocking-b2g: 2.0+ → 1.3T?
Keywords: regression
Does this reproduce without NOFTU=1 ? If it doesn't, then it's probably not a release blocker.
(In reply to Anthony Ricaud (:rik) from comment #13)
> Does this reproduce without NOFTU=1 ? If it doesn't, then it's probably not
> a release blocker.

Yes, I only wrote that because it makes bisecting faster since the FTU setting doesn't matter for reproing.
The problem was that BluetoothHelper was being loaded when _bluetooth (pointer to window.navigator.mozBluetooth) was null. This is only loaded once on initialization, so we have to re-check it later, when it's not null. Patch coming.
Comment on attachment 8453840 [details] [diff] [review]
Make BluetoothHelper check later if window.navigator.mozBluetooth isn't loaded.

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

::: apps/callscreen/js/index.js
@@ +9,5 @@
>  });
>  
>  // Don't keep an audio channel open when the callscreen is not displayed
>  document.addEventListener('visibilitychange', function visibilitychanged() {
> +  CallsHandler.setup();

debug ?
(if not I have further suggestions)

::: shared/js/bluetooth_helper.js
@@ +25,5 @@
> +    _isLoading = true;
> +
> +    var req = _bluetooth.getDefaultAdapter();
> +    req.onsuccess = function() {
> +      _isReady = true;

shouldn't we |_isLoading = false| here too?
Attachment #8453840 - Flags: review?(etienne)
(In reply to Etienne Segonzac (:etienne) from comment #17)
> debug ?
> (if not I have further suggestions)

No, this was intentionally. I wasn't happy with this since it should get queued up by the getConnectedDevices() call anyways. I didn't think it was worth investigating more but if you have another suggestion or you think I should get it working that way, I'd be fine with that.

> ::: shared/js/bluetooth_helper.js
> @@ +25,5 @@
> > +    _isLoading = true;
> > +
> > +    var req = _bluetooth.getDefaultAdapter();
> > +    req.onsuccess = function() {
> > +      _isReady = true;
> 
> shouldn't we |_isLoading = false| here too?

Yeah, but once _isReady is set, it's not used anywhere. I'll add this in on the next iteration anyways.
(In reply to Doug Sherk (:drs) from comment #18)
> (In reply to Etienne Segonzac (:etienne) from comment #17)
> > debug ?
> > (if not I have further suggestions)
> 
> No, this was intentionally. I wasn't happy with this since it should get
> queued up by the getConnectedDevices() call anyways. I didn't think it was
> worth investigating more but if you have another suggestion or you think I
> should get it working that way, I'd be fine with that.

* I'd like to extract the BT setup in it's own method if it's the only setup that needs to run on visibility change
* And can we do it only when !document.hidden?
PR: https://github.com/mozilla-b2g/gaia/pull/21636

Okay, sorry, that patch was crap. This is much cleaner. Not sure why I didn't just check if mozBluetooth fires any events before.

Etienne, do you think I should ask Ian for review again? I'm not sure how comfortable you are with this code.
Attachment #8453840 - Attachment is obsolete: true
Attachment #8453840 - Flags: review?(iliu)
Attachment #8454589 - Flags: review?(etienne)
Comment on attachment 8454589 [details] [diff] [review]
Make BluetoothHelper listen for mozBluetooth events.

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

We're so used to workarounds that sometime we forget about clean solutions :)

But yes, I think Ian should have a look.
Attachment #8454589 - Flags: review?(iliu)
Attachment #8454589 - Flags: review?(etienne)
Attachment #8454589 - Flags: feedback+
QA Whiteboard: [QAnalyst-Triage+] → [QAnalyst-Triage+][lead-review+]
(In reply to Doug Sherk (:drs) from comment #18)
> (In reply to Etienne Segonzac (:etienne) from comment #17)
> > debug ?
> > (if not I have further suggestions)
> 
> No, this was intentionally. I wasn't happy with this since it should get
> queued up by the getConnectedDevices() call anyways. I didn't think it was
> worth investigating more but if you have another suggestion or you think I
> should get it working that way, I'd be fine with that.
> 
> > ::: shared/js/bluetooth_helper.js
> > @@ +25,5 @@
> > > +    _isLoading = true;
> > > +
> > > +    var req = _bluetooth.getDefaultAdapter();
> > > +    req.onsuccess = function() {
> > > +      _isReady = true;
> > 
> > shouldn't we |_isLoading = false| here too?
> 
> Yeah, but once _isReady is set, it's not used anywhere. I'll add this in on
> the next iteration anyways.

The flag '_isLoading' of first patch is looking like a semaphore to avoid other request doing getDefaultAdapter() again. This is not a good idea to have a flag in API wrapper. And the naming of flag '_isLoading' is not accurate enough to figure out its intent.
Comment on attachment 8454589 [details] [diff] [review]
Make BluetoothHelper listen for mozBluetooth events.

The patch is okay for me. And please also add a test for 'adapteradded' event to call _getAdapter() while receiving the event. That would be perfect.:)
Attachment #8454589 - Flags: review?(iliu) → review+
For the paired headset reconnection, I remember the reconnection is triggered while the bluetooth.js of settings app is running. Is the patch really fixing the bug here? Please make sure the problem is happened while the paired headset is connected, and the status bar icon is indicating connected state. The speaker/bluetooth-headset icon of call screen should be consistent with paired headset status.
(In reply to Ian Liu [:ianliu] from comment #24)
> For the paired headset reconnection, I remember the reconnection is
> triggered while the bluetooth.js of settings app is running. Is the patch
> really fixing the bug here? Please make sure the problem is happened while
> the paired headset is connected, and the status bar icon is indicating
> connected state. The speaker/bluetooth-headset icon of call screen should be
> consistent with paired headset status.

The problem is within the callscreen app, which is started on boot. Its instance of BluetoothHelper is also initialized during the app's startup. We get an "error getting bluetooth adapter" message in logcat during this time.
Added test and merged:

https://github.com/mozilla-b2g/gaia/commit/343272f78ce6fdf0c4ccf2a7d2f7568b8f01df12
Status: ASSIGNED → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Comment on attachment 8454589 [details] [diff] [review]
Make BluetoothHelper listen for mozBluetooth events.

[Approval Request Comment]
[Bug caused by] (feature/regressing bug #): bug 990003
[User impact] if declined: The first time a user makes a call with a Bluetooth headset, the callscreen will show a speaker icon instead of a Bluetooth icon.
[Testing completed]: I tested it.
[Risk to taking this patch] (and alternatives if risky): Low.
[String changes made]: None.
Attachment #8454589 - Flags: approval-gaia-v2.0?(release-mgmt)
Attachment #8454589 - Flags: approval-gaia-v1.4?(release-mgmt)
Attachment #8454589 - Flags: approval-gaia-v1.3?(release-mgmt)
Comment on attachment 8454589 [details] [diff] [review]
Make BluetoothHelper listen for mozBluetooth events.

I spoke with drs about his testing. 1.4+ and 2.0+

ni on Vance to comment on whether it's worth taking this fix on 1.3.
Attachment #8454589 - Flags: approval-gaia-v2.0?(release-mgmt)
Attachment #8454589 - Flags: approval-gaia-v2.0+
Attachment #8454589 - Flags: approval-gaia-v1.4?(release-mgmt)
Attachment #8454589 - Flags: approval-gaia-v1.4+
Flags: needinfo?(vchen)
(In reply to Lawrence Mandel [:lmandel] from comment #28)
> Comment on attachment 8454589 [details] [diff] [review]
> Make BluetoothHelper listen for mozBluetooth events.
> 
> I spoke with drs about his testing. 1.4+ and 2.0+
> 
> ni on Vance to comment on whether it's worth taking this fix on 1.3.

I will inform partner about this bug and let them decide whether to cherry-pick this one or not

Thanks

Vance
Flags: needinfo?(vchen)
Needs a branch patch for v1.4 uplift.
Flags: needinfo?(drs+bugzilla)
blocking-b2g: 1.3T? → ---
Duplicate of this bug: 1020149
Duplicate of this bug: 1020147
Attachment #8454589 - Flags: approval-gaia-v1.3?(release-mgmt)
This issue has been verified successfully on Flame2.0&2.1
Verify video:"verify_1030912.mp4".

Flame2.0 build
Gaia-Rev        824a61cccec4c69be9a86ad5cb629a1f61fa142f
Gecko-Rev       https://hg.mozilla.org/mozilla-central/rev/acde07cb4e4d
Build-ID        20141125040209
Version         36.0a1
Device-Name     flame
FW-Release      4.4.2
FW-Incremental  eng.cltbld.20141125.113029
FW-Date         Tue Nov 25 11:30:43 EST 2014
Bootloader      L1TC00011880

Flame2.1 build:
Gaia-Rev        db2e84860f5a7cc334464618c6ea9e92ff82e9dd
Gecko-Rev       https://hg.mozilla.org/releases/mozilla-b2g34_v2_1/rev/211eae88f119
Build-ID        20141126001202
Version         34.0
Device-Name     flame
FW-Release      4.4.2
FW-Incremental  eng.cltbld.20141126.033519
FW-Date         Wed Nov 26 03:35:30 EST 2014
Bootloader      L1TC00011880
You need to log in before you can comment on or make changes to this bug.