917221 - [Bluetooth][Dialer] Handle 'CHLD=3' command to add a held call to the conversation (conference call)

Status: RESOLVED FIXED

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

Description

[Ben Tian (inactive)]

Gaia dialer needs to implement 'CHLD=3' command handler.

Comment 1

[Ben Tian (inactive)]

'CHLD=3' command from gecko bluetooth requires Dialer to merge a held call into current conversation.

Comment 2

[KM Lee [:rexboy] (inactivate after Sep. 22)]

Several cases should be considered. Suppose we have our calls set like:

 (A). Conference call
 (B). Individual call

Then, 
Case 1: (B) is active. This is a straightforward case which (B) should be merged into (A).

Case 2: (A) is active. Because we can have more than one individual calls outside conference
call, each individual calls have its own merge button and the call need to be merged is determined
by the button pushed.
But we can't determinate it with BT command - unless there are only 1 individual call like this
case. Should we consider just this special case and implement it?

Comment 3

[Jamin Liu [:jaliu][:Jamin]]

If we have more than one individual call are held, we have to choose a held call by some kind of rules we made.
The hand-free device can't indicate which call should be merged through BT command AT+CHLD=3.

I think we can choose a held call by First-In-First-Merge or Last-In-First-Merge method, as far as I know, it's not defined in Bluetooth HFP spec.
Should we ask UX team to make this decision ?

Comment 4

[Michael Schwartz [:m4]]

(In reply to KM Lee [:rexboy] from comment #2)
> Because we can have more than one individual calls
> outside conference

How would that happen?

Comment 5

[KM Lee [:rexboy] (inactivate after Sep. 22)]

Sorry my fault. Just made a discussion and confirmed with Shawn and confirmed
*  From 3GPP spec we can't have 1 active call with more than 2 hold calls
   (conference call counts as 1). 

Some reference phones have specific UI to handle call waiting when call
limit of 2 already reached (e.g. let user choose which phone should be hanged up),
but it's not the case in FFOS for now. If we have another incoming call in
the example case of comment 2, Dialer hangs up incoming call directly.

So it's straightforward that we just need to
* If conference call doesn't exist, merge hold call with active as a conference call.
* If conference call exist, merge another individual call into it.

If we want to handle the case of incoming call when call limit reached, we can open another
bug for it.

Comment 6

[KM Lee [:rexboy] (inactivate after Sep. 22)]

WIP:
https://github.com/rexboy7/gaia/commit/97d12697309a901984119cdcfb107c7933fe36af

The unit test is also written. Not sure if this behaves good..
We can try to review it first if we agree this behavior is OK.
is there any progress on the Gecko side?

Comment 7

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

(In reply to KM Lee [:rexboy] from comment #6)
> WIP:
> https://github.com/rexboy7/gaia/commit/
> 97d12697309a901984119cdcfb107c7933fe36af
> 
> The unit test is also written. Not sure if this behaves good..
> We can try to review it first if we agree this behavior is OK.
> is there any progress on the Gecko side?

Gecko patch is being reviewed(bug 881194).

Comment 8

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

Hi Rex,

Just let you know that patches for Bug 881194 are going to land.

Comment 9

[KM Lee [:rexboy] (inactivate after Sep. 22)]

Thanks to Jamin's testing on this patch, From his testing the patch passes PTS test items
with patch of Gecko side of bug 881194.

In debug mode MOZ_ASSERT will be triggered due to bluetooth call of CDMA call waiting. The
issue is addressed in bug 947086, but it should be an unrelated issue.

Let's put the patch into review process.

Comment 10

[KM Lee [:rexboy] (inactivate after Sep. 22)]

Attachment: patch

Hello Etienne, would you review this patch for BT device?

Since it needs specific device to test (Not all BT handsfree devices
supports CHLD=3 since it's an optional feature in spec), Jamin
will help testing this patch before landing.

Comment 11

[Etienne Segonzac (:etienne)]

Comment on attachment 8345107 [details]
patch

cool, r=me without the user facing debug message

Comment 12

[KM Lee [:rexboy] (inactivate after Sep. 22)]

Thanks for helping Etienne!
I would choose console.warn() version (See updated patch).
Jamin would confirm the updated patch before merging.

Comment 13

[Jamin Liu [:jaliu][:Jamin]]

Verified the patch Attachment #8345107 [details] by running PTS test case:TC_AG_TWC_BV_04_I in release build.
P.S. There are three related gecko patches in Bug 81194 are required for CHLD=3 supporting.

Comment 14

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

> P.S. There are three related gecko patches in Bug 81194 are required for
                                                ^^^^^^^^^
Should be bug 881194.

Comment 15

[KM Lee [:rexboy] (inactivate after Sep. 22)]

Hi Jamin:

Thanks for testing and confirming related issue!
Seems those issue doesn't need further change in this patch, I'll land it after Travis goes green.

Comment 16

[KM Lee [:rexboy] (inactivate after Sep. 22)]

merged on master:
https://github.com/mozilla-b2g/gaia/commit/c3ee8d3f54c468b5e5576416dabde0b4b55a7dbb

(The only UI-test failure seems unrelated)