Closed Bug 917344 Opened 11 years ago Closed 11 years ago

[Dialer][V1.2] FxOS still highlights the "Speaker icon" while the voice is from "BT earphone"

Categories

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

ARM
Gonk (Firefox OS)
defect
Not set
normal

Tracking

(blocking-b2g:koi+, b2g-v1.2 fixed)

VERIFIED FIXED
1.2 C4(Nov8)
blocking-b2g koi+
Tracking Status
b2g-v1.2 --- fixed

People

(Reporter: whsu, Assigned: rexboy)

References

Details

Attachments

(2 files)

Attached image Speaker_Icon.png
* Description:
  This problem happens on v1.2 and v1.1.0 branch.
  If user reconnects the BT earphone, the speaker icon of dialer page cannot follow the voice output status.
  The speaker icon is still highlighted while the voice is from "BT earphone"
  Attaching the screenshot.

* Precondition:
  Prepare two FxOS phones.

* Reproduction steps:
  1. A connects the "BT earphone"
  2. B calls A
  3. A answers the call from B
  4. A changes to handfree mode
  5. Disconnect and connect the "BT earphone"
  6. Observing the status of phone A

* Expected result:
  The voice is from "BT earphone" and the speaker icon isn't highlighted.

* Actual result:
  FxOS highlights the speaker icon

* Test build:(Mozilla-Central)
 - Gaia:     c6b4cc05b2de6884a652c1c5ab8401216ffa46c1
 - Gecko:    78b3dbc50a8cddea792b6c2870c0bfbe3726335c
 - BuildID   20130917060716
 - Version   26.0a1


Many Thanks!
blocking-b2g: --- → koi?
triage: koi+ as it confuses the users
blocking-b2g: koi? → koi+
Hey Shawn, any idea of what we can do here?
Flags: needinfo?(shuang)
Hi Etienne,
Sorry for late reply. I just talked with William, he said audio path can be correctly routed to BT headset but dialer UI showing "Speaker" icon is in "on" state. From STR, test does not press "Speaker on" icon, but after BT headset was been reconnected, "Speaker" icon is on.
Flags: needinfo?(shuang)
Any chance, speaker icon will be toggled?
When should we turn off the speaker icon? Do we get a system message?
Flags: needinfo?(shuang)
V1.2: You can reference adapter.onhfpstatuschanged to get Handsfree connection state.
 https://github.com/mozilla-b2g/gaia/blob/master/apps/system/js/bluetooth.js#L178
Flags: needinfo?(shuang)
It is suppose to turn off the speaker icon whenever SCO connection established. Please use |onscostatuschanged| instead of |onhfpstatuschanged|.
Thanks Shawn!
Not available to take the bug right now, but I trust the koi+ will get it some attention :)

The fix should be fairly similar to what we we've done for the wired headset [1].

[1] https://github.com/mozilla-b2g/gaia/blob/master/apps/communications/dialer/js/calls_handler.js#L67-74
Assignee: nobody → rexboy
Attached file patch
Per comment 7 and comment 8, the patch is not difficult but bluetooth permission is required on comms app.

Etienn, may you help review this patch?
Attachment #817647 - Flags: review?(etienne)
Comment on attachment 817647 [details]
patch

Hey Rex, this change should probably be done on top of bug 912005.

Can you work with Ben to finish the unit tests for bug 912005 (your mock_mozbluetooth.js will be helpful) and rebase your patch on top of it?

Thanks!
Attachment #817647 - Flags: review?(etienne)
Note: we can also introduce the BluetoothHelper as part of this patch in order to prevent this koi+ bug from depending on another non-koi bug.

But it's not that big of a deal if we need to uplift both.
I just talked to Ben, I can help on unit test of 912005. But I'm not sure whether we need to uplift both of them. Maybe I can ask Joe's opinion later.
Note that this issue is also reproducible on b2g18.

For b2g18, we need another patch for fixing this issue since we don't have event handler |onscostatuschanged|. But, you can register system message of |bluetooth-sco-status-changed| for the same purpose.
Comment on attachment 817647 [details]
patch

After talking with Ben, we decided to let bluetooth_helper.js introduced in bug 912005 go here first. I preserved the three methods of bluetooth_helper used there in the patch.

Hello Etienne, may you help review the patch again?
Attachment #817647 - Flags: review?(etienne)
Comment on attachment 817647 [details]
patch

Looking good.
Still some comments on the tests but we're getting close.
Attachment #817647 - Flags: review?(etienne)
Comment on attachment 817647 [details]
patch

Thanks for the suggestions!
Just a few changes per comments above.
Attachment #817647 - Flags: review?(etienne)
Target Milestone: --- → 1.3 Sprint 4 - 11/8
Target Milestone: 1.3 Sprint 4 - 11/8 → 1.2 C4(Nov8)
Comment on attachment 817647 [details]
patch

Thanks for the hard work!
This is good to go.
Attachment #817647 - Flags: review?(etienne) → review+
Thanks for your reviewing Etienne!
And bug 912005 can based on bluetooth_helper.js of this bug when we want to land it.

master:
https://github.com/mozilla-b2g/gaia/commit/7d7864ce51f3faac0f8f55e487a10ba5b58d226c
Status: NEW → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
See Also: → 912005
Uplifted 7d7864ce51f3faac0f8f55e487a10ba5b58d226c to:
v1.2: 9663995cbeca17f3ecf0100f56b751ae4bd41d7d
Hi, all,

The patch doesn't work as expected.

Now, reconnecting the BT headset (Step 5), the sound comes from speaker and FxOS highlights the "Speaker icon".
As I know, the expected result is FxOS will turn off hands-free mode and the sound will come from BT headset.

Did I misunderstand anything?
Thanks!
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
William - Can you file a followup bug for the issue you hit?
Status: REOPENED → RESOLVED
Closed: 11 years ago11 years ago
Flags: needinfo?(whsu)
Resolution: --- → FIXED
Hi, Jason,

The patch doesn't work as expected.
I'm concerned that the patch had side effect.

So, I would like to reopen this bug, because
 1) The status (REOPEN) can help developer to continue to trace this issue.
 2) This bug has been classified as koi+ (triage).
    If I submit a new bug, stakeholders need to classified it again.

Thanks!
Status: RESOLVED → REOPENED
Flags: needinfo?(whsu)
Resolution: FIXED → ---
(In reply to William Hsu [:whsu] from comment #24)
> Hi, Jason,
> 
> The patch doesn't work as expected.
> I'm concerned that the patch had side effect.
> 
> So, I would like to reopen this bug, because
>  1) The status (REOPEN) can help developer to continue to trace this issue.

Do not reopen bugs for this purpose. You should always file followup bugs in this case. This will confuse tree management if you reopen for that purpose. Please file a followup bug.

>  2) This bug has been classified as koi+ (triage).
>     If I submit a new bug, stakeholders need to classified it again.

That's an important consideration for why it does need a followup bug. Triage needs to decide the path forward in this case.
Status: REOPENED → RESOLVED
Closed: 11 years ago11 years ago
Resolution: --- → FIXED
Hi, Jason,

Thanks for your reply.
Could you please tell me which situation I can use "REOPEN"?
Flags: needinfo?(jsmith)
(In reply to William Hsu [:whsu] from comment #26)
> Hi, Jason,
> 
> Thanks for your reply.
> Could you please tell me which situation I can use "REOPEN"?

Only in the case when the landed patch is backed out. If we really think that's the right path forward here, then we should ask the developer here to back out the patch.
Flags: needinfo?(jsmith)
As comment 22 mentioned, marked it as "Verified"
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: