906775 - Create a CallGroupHandler

Status: RESOLVED FIXED

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

Description

[Etienne Segonzac (:etienne)]

The way the conference call is implemented in gecko we have 1 and only 1 telephony.callGroup.

So we should have a CallGroupHandler singleton to support:

* displaying/hiding the conference call "line" on the main screen
* changing the style of the "line" on callGroup.statechange (hold/resume)
* maintaining a conference call detail view (with handledCall's nodes) on callGroup.callschanged

Comment 1

[Joe Cheng [:jcheng] (please needinfo)]

koi+ as it links to koi-comms bug

Comment 2

[Etienne Segonzac (:etienne)]

Attachment: Pointer to gaia PR

Here we go!
This patch is definitely ready to start the review process.

Hsin-Yi, I couldn't do any real world testing (on my unagi doing a |telephony.conferenceGroup.add(telephony.calls[0], telephony.calls[1])| with 2 calls does nothing right now). 

Could you test this patch?
If you programatically create the conference call this patch should make the call screen display 1 "Call group (2)" line instead of the 2 calls lines.

Thanks!

Comment 3

[Hsin-Yi Tsai (she/her) [:hsinyi]]

(In reply to Etienne Segonzac (:etienne) from comment #2)
> Created attachment 796794 [details]
> Pointer to gaia PR
> 
> Here we go!
> This patch is definitely ready to start the review process.
> 
> Hsin-Yi, I couldn't do any real world testing (on my unagi doing a
> |telephony.conferenceGroup.add(telephony.calls[0], telephony.calls[1])| with
> 2 calls does nothing right now). 

Oh... is that because your sim card without conference call service enabled?
What are the call states of the two calls?

> 
> Could you test this patch?
> If you programatically create the conference call this patch should make the
> call screen display 1 "Call group (2)" line instead of the 2 calls lines.
> 
> Thanks!

I'd love to help but sorry that I might not be able to test until I get back to Taiwan next week. I will get back to you next week. :)

And, can I create a conference call through this patch? Or I would need to hack the code to create a concall first?

Comment 4

[Etienne Segonzac (:etienne)]

Attachment: Quick and dirty patch to auto-merge 2 calls into a conferenceGroup

How to use the patch (once applied)
- make a call
- receive a second call
- choose hold and answer

- 1.5sec after answering the second call we will try to merge the 2
- 1.5sec after that we log the state of the calls

I get:
E/GeckoConsole(  500): Content JS LOG at app://communications.gaiamobile.org/dialer/js/calls_handler.js:487 in holdAndAnswer/<: +++ auto merging the 2 calls
E/GeckoConsole(  500): Content JS LOG at app://communications.gaiamobile.org/dialer/js/calls_handler.js:489 in holdAndAnswer/<: +++ auto merged the 2 calls
E/GeckoConsole(  500): Content JS LOG at app://communications.gaiamobile.org/dialer/js/calls_handler.js:491 in holdAndAnswer/</<: +++ a few moment after the merge, the calls are 
E/GeckoConsole(  500): Content JS LOG at app://communications.gaiamobile.org/dialer/js/calls_handler.js:493 in holdAndAnswer/</</<: +++ 0 - call.state held
E/GeckoConsole(  500): Content JS LOG at app://communications.gaiamobile.org/dialer/js/calls_handler.js:493 in holdAndAnswer/</</<: +++ 1 - call.state connected


And I don't actually now if my SIM supports conference calls...

Comment 5

[Etienne Segonzac (:etienne)]

Attachment: \o/

Just tried with my personal simcard and it worked :)
Thanks Hsin-Yi for the pointer!

Comment 6

[Hsin-Yi Tsai (she/her) [:hsinyi]]

(In reply to Etienne Segonzac (:etienne) from comment #5)
> Created attachment 797250 [details]
> \o/
> 
> Just tried with my personal simcard and it worked :)
> Thanks Hsin-Yi for the pointer!

Awesome!!!  (dance)

Comment 7

[Anthony Ricaud (:rik)]

Hsin-Yi: We have a question about the API. Do calls included in mozTelephony.group.calls also exist in mozTelephony.calls?

Comment 8

[Anthony Ricaud (:rik)]

Comment on attachment 796794 [details]
Pointer to gaia PR

I have one change that I'd like to see. Other changes depend on the question I asked Hsin-Yi and if you agree about the logic change I'm proposing.

Comment 9

[Etienne Segonzac (:etienne)]

(In reply to Anthony Ricaud (:rik) from comment #7)
> Hsin-Yi: We have a question about the API. Do calls included in
> mozTelephony.group.calls also exist in mozTelephony.calls?

The webidl says:
  // A call is contained either in Telephony or in TelephonyCallGroup.

So my working assumption is no.

Comment 10

[Etienne Segonzac (:etienne)]

Comment on attachment 796794 [details]
Pointer to gaia PR

Ready for the second round.

Comment 11

[Hsin-Yi Tsai (she/her) [:hsinyi]]

(In reply to Etienne Segonzac (:etienne) from comment #9)
> (In reply to Anthony Ricaud (:rik) from comment #7)
> > Hsin-Yi: We have a question about the API. Do calls included in
> > mozTelephony.group.calls also exist in mozTelephony.calls?
> 
> The webidl says:
>   // A call is contained either in Telephony or in TelephonyCallGroup.
> 
> So my working assumption is no.

As Etienne said, no. They are exclusive.

Comment 12

[Anthony Ricaud (:rik)]

Comment on attachment 796794 [details]
Pointer to gaia PR

I like it a lot. The work in the previous bugs really paid off!

Two easy comments left in the PR.

Comment 13

[Etienne Segonzac (:etienne)]

Comment on attachment 796794 [details]
Pointer to gaia PR

Asking for a last r? since I had to add a small change in handled_call.js.
The current code was racy since the hc.call could be nullified before we remove the hc from the handledCalls array (discovered when a participant in a conf call hanged up).

Thanks again!

Comment 14

[Hsin-Yi Tsai (she/her) [:hsinyi]]

Comment on attachment 796794 [details]
Pointer to gaia PR

I am cancelling f? for now as Etienne is fine to test concall with the real network. Feel free to pin me again if anything I can help :)

Comment 15

[Anthony Ricaud (:rik)]

Comment on attachment 796794 [details]
Pointer to gaia PR

Good for me.
A last minute change that I forgot: we need to update fake-oncall-desktop.js |CallScreen.callsCount = 1| -> |CallScreen.bigDuration = true|

Comment 16

[Etienne Segonzac (:etienne)]

https://github.com/mozilla-b2g/gaia/commit/3ba0d2e516ed109a1937c032ec75827322ee56b0

Comment 17

[Etienne Segonzac (:etienne)]

Following up on a call screen exit issue here: bug 912060