831396 - On inserting wired headset call is getting disconnected

Status: VERIFIED FIXED

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

Description

[Anshul]

While in a call, inserting a wired headset is disconnecting the call. Steps to reproduce.

1. Make a call
2. When the call is connected, insert a wired headset

Observed behavior
I see in the status bar the icon for the wired headset and then I see that call is getting disconnected.

From the logs I see receiving RIL:Hangup request from the content process.

Comment 1

[Andrew Overholt [:overholt]]

Vicamo, who should own this?

Comment 2

[Chris Jones [:cjones] inactive; ni?/f?/r? if you need me]

I don't reproduce this on an otoro with moz RIL built from

$ (cd gecko/ && git rev-parse HEAD && cd ../gaia && git rev-parse HEAD)
2751cf873c70a03933c407d3bc06003e72c7fbc3
3c84fa2b74b3dbe591f0d4fbedbac9aac5ca4654

Tried TRS and TRRS jacks.

Comment 3

[Anshul]

Attachment: Android logs with Moz RIL

Comment 4

[Anshul]

Chris, I tried again on our tip of b2g-18 branch and I still see that the call is getting disconnected as soon as the wired headset is connected.

Gecko: 2751cf873c70a03933c407d3bc06003e72c7fbc3
Gaia : 3c84fa2b74b3dbe591f0d4fbedbac9aac5ca4654

Please find attached the android log with Moz RIL.

Comment 5

[Vicamo Yang [:vicamo][:vyang]]

https://github.com/mozilla-b2g/gaia/blob/master/apps/communications/dialer/js/oncall.js#L489

  function handleHSCommand(message) {
    if (telephony.active) {
      end();
    } ...
  }

It seems Gaia does this on purpose? Code landed in bug 809704.

Comment 6

[Andrew Overholt [:overholt]]

Francisco, see Vicamo's comment #5 regarding your patch from bug 809704.

Comment 7

[Alex Keybl [:akeybl]]

Unless this represents a significant amount of risk or a high level of effort, we should really fix this for v1.0.0. We're thinking about the use case where somebody is in the car, picks up the phone, wants to be safe and plug in their headset, and loses the call.

Comment 8

[Vicamo Yang [:vicamo][:vyang]]

(In reply to Andrew Overholt [:overholt] from comment #6)
> Francisco, see Vicamo's comment #5 regarding your patch from
> bug 809704.

Ken Chang found that could probably be a hardware/driver problem. The headset-button event is used to connect/disconnect calls in Gaia and that's fine. However, plugging the jack too fast or too slow may have a chance to issue a should-not-be headset-button event. The hardware/driver may need some de-bouncing to work around this problem.

Comment 9

[Andrew Overholt [:overholt]]

(In reply to Vicamo Yang [:vicamo][:vyang] from comment #8)
> (In reply to Andrew Overholt [:overholt] from comment #6)
> > Francisco, see Vicamo's comment #5 regarding your patch from
> > bug 809704.
> 
> Ken Chang found that could probably be a hardware/driver problem.

So is this something we have to get a partner to fix?

Comment 10

[Vicamo Yang [:vicamo][:vyang]]

      _   __  __  __  __
     / \_/  ||  ||  ||  |
     |  _   || F|| G|| H|
+> | \_/ \__||__||__||__|
|  |       ^   ^   ^
|  |       |   |   |
+  +       +   +   +
A  B       C   D   E

Above is a 4-conductor (TRRS) 3.5mm phone connector. When an audio jack being inserted into the connector slot, the internal metal conductor B touches A and forms a closed circuit and then open when it's fully inserted. Hardware/kernel detects such changes and then knows there is a jack connected and emits an uevent to the user space listener.

However, the detection might be inaccurate at the definition of "fully connected". For example, we expects C->F, D->G, E->H short circuits on receiving that uevent, but the real world might give you C->?, D->F, E->G instead. You can have a quick test on Unagi by plugging the jack into the socket, leave 2~3 mm out and see whether the headset icon pops up in the status bar. In this situation, "headset-button" event is triggered because the impedance of earphones(G) is quite low and acts almost the same as you press the button and create a short circuit.

Not every phone has such problem. My Sony Xperia S just always emits the event after the jack is fully inserted, leaving 0 mm outside. The internal hardware may looks like:

      _   __  __  __  __
     / \_/  ||  ||  ||  |
+> | |  _   || F|| G|| H|
|  | \_/ \__||__||__||__|
|  |         ^   ^   ^
|  |         |   |   |
+  +         +   +   +
A  B         C   D   E

The jack must go all the way to the end before it may triggers A->B, so we'll always have C->F, D->G, E->H.

Comment 11

[Milan Sreckovic [:milan] (needinfo for best results)]

Understood that there is weirdness in how the events may come in, but is there a way for us to be more fault tolerant on our side and still end up with the correct behaviour?

Comment 12

[Francisco Jordano [:arcturus] [:francisco]]

Hi,

I've been trying to reproduce this bug with the Unagi headphones and couldn't reproduce it.

Also tried with phones from apple, and other 3rd party and again couldn't reproduce it. Cannot use non unagi headphones to do the hang up but the call doesn't stop if introduced during the call.

As I cannot reproduce the bug, what I can try to do in gaia patch to be sure we just listen to the 'headset-button-press' right now ignore 'headset-button-release'.

Anyway if we still receive a 'headset-button-press' with a 3rd party phones while plugin the headset to the unagi, there is not much we can do in the gaia side.

Also asking Randy for feedback as he wrote the backend part for this.

Thanks!

Comment 13

[Francisco Jordano [:arcturus] [:francisco]]

Attachment: WIP - Trying to be more restrict on the events detected

Andrew, could you check if with this patch you still are able to reproduce?

Comment 14

[Andrew Overholt [:overholt]]

Comment on attachment 704927 [details]
WIP - Trying to be more restrict on the events detected

Passing feedback? to Anshul as I can't reproduce this.

Comment 15

[Anshul]

Andrew, I will test this patch tomorrow and get back to you.

Comment 16

[Randy Lin [:rlin] - Be Mozillian on 2015/01/26]

The same issue as Bug 821957,
The unagi device's headset jack HW is designed by china standard (ground on the
 sleeve). The headset detection may fail if user plug-in wrong type of headphone. Some partner would also use headphone's impedance to distinguish headphone type, so Anshul can reproduce this, but others can't.

Comment 17

[Anshul]

Andrew, I tested your patch and now I see that the call is not getting disconnected when plugging in a headset! Could you please work on getting this change merged?

Comment 18

[Chris Jones [:cjones] inactive; ni?/f?/r? if you need me]

Comment on attachment 704927 [details]
WIP - Trying to be more restrict on the events detected

Review race! :)

Comment 19

[Etienne Segonzac (:etienne)]

Comment on attachment 704927 [details]
WIP - Trying to be more restrict on the events detected

Clear, not breaking the feature, *and* apparently fixing a bug :)

Comment 20

[Chris Jones [:cjones] inactive; ni?/f?/r? if you need me]

(I assume this will find its way onto the right branches automatically ...)

https://github.com/mozilla-b2g/gaia/commit/b39f61ecf6e32400f3fa2e1b4c6011f1fa42ea51

Comment 21

[Michael Vines [:m1] [:evilmachines]]

No uplift to gaia/v1.0.0 yet FYI, still stuck in gaia/master

Comment 22

[Michael Vines [:m1] [:evilmachines]]

How do we get this in gaia v1.0.0?

Comment 23

[Chris Jones [:cjones] inactive; ni?/f?/r? if you need me]

Good question.

Are we doing anything smarter for gaia than devs triple-landing?

Comment 24

[Vivien Nicolas (:vingtetun) (:21) - (NOT reading bugmails, needinfo? please)]

(In reply to Chris Jones [:cjones] [:warhammer] from comment #23)
> Good question.
> 
> Are we doing anything smarter for gaia than devs triple-landing?

Smarter I'm not sure. Bug /me triple landing if the bugs live in Boot2Gecko/Gaia lands (this one was in General so it goes out of my bz requests) and if the status is not already set to status-b2g18:fixed.

I just merge it to v1-train and v1.0.0.

Comment 25

[Anshul]

Francisco, the change finally landed in our tree but unfortunately not working now, strange!. I added debug messages to the oncall.js method handleHSCommand and I see that when I insert the headset the message received is "headset-button-press" and therefore this fix isn't resolving the issue. 

I am not sure why when I tried the patch last time on the same hardware and with the same wired headset the patch did work. The wired headset I am using has two rings (TRS) on it.

Comment 26

[Jason Smith [:jsmith]]

(In reply to Anshul from comment #25)
> Francisco, the change finally landed in our tree but unfortunately not
> working now, strange!. I added debug messages to the oncall.js method
> handleHSCommand and I see that when I insert the headset the message
> received is "headset-button-press" and therefore this fix isn't resolving
> the issue. 
> 
> I am not sure why when I tried the patch last time on the same hardware and
> with the same wired headset the patch did work. The wired headset I am using
> has two rings (TRS) on it.

File a new bug please and link it to this one. Avoid reopening bugs - it screws up landings, blocker tracking, etc.

Comment 27

[Lukas Blakk [:lsblakk] use ?needinfo]

Landed on mozilla-b2g18/gaia master prior to the 1/25 branching to mozilla-b2g18_v1_0_0/v1.0.0, updating status-b2g-v1.0.0 to fixed.

Comment 28

[pallavi]

This issue is not reproducing on Unagi
Build ID: 20130215070202
Gecko  http://hg.mozilla.org/releases/mozilla-b2g18_v1_0_1/rev/a9e4f8912607
Gaia   21ba59d933c66024cb351c2379315301d5352e0c
Kernal: Dec5th

Note: Insted of reopening 831396, New bug was opened :836424. so, changing status of both the bug to verified

Comment 29

[Julien Wajsberg [:julienw]]

for some reason, this is not in v1-train nor v1.0.1. I guess this was marked as "fixed" where it should not have (presumably because it was in the wrong component). I hope we don't have other bug in the same situation.

Also I don't know how comment 28 found this as verified.

Comment 30

[Julien Wajsberg [:julienw]]

ah it seems to be in v1.0.0 though.

Comment 31

[John Ford [:jhford] CET/CEST Berlin Time]

I don't know why the flags were set the way they were, this commit wasn't on either good v1* branch.

I've uplifted b39f61ecf6e32400f3fa2e1b4c6011f1fa42ea51 as:

v1-train: 3dedddff1890156502cc6c41d2561935d6827a81
v1.0.1: ffec597b718c39bdd4f7f9cf161e1015a614ce06