1059649 - [tarako/dolphin]After we hangup any one of the two calls in a group call, we'll never success to merge conference call again

Status: RESOLVED FIXED

(Firefox OS Graveyard :: RIL, defect)

Description

[helloarvin]

Attachment: merge-failed.png

Steps to reproduce:
1> Insert a sim card which supports conference call feature;
2> Boot the device and Access a MTC;
3> Make a MOC and then merge the two calls;
4> Hangup one call and then make a MOC;
5> Fail to merge the two calls into a conference call

Please check the attachment to get the screen shot of the issue.

Comment 1

[shawn ku [:sku]]

Hi Arvin:
 Would you mind attach device/radio log for us to check this issue?
Thanks!!
Shawn

Comment 2

[helloarvin]

Attachment: conferencecall.txt

Log analysis result:

// two MOC exists
// 1st 10086
// 2nd 10010

// to merge two calls
08-24 22:23:22.140 I/Gecko   (  108): -*- RadioInterface[1]: Received message from worker: {"rilMessageClientId":1,"rilMessageToken":30,"rilMessageType":"conferenceCall","rilRequestType":16,"rilRequestError":0,"success":true}

// call state changed of 10086
08-24 22:23:22.170 I/Gecko   (  108): TelephonyProvider: handleCallStateChange: {"state":0,"callIndex":1,"toa":129,"isMpty":true,"isMT":false,"als":0,"isVoice":true,"isVoicePrivacy":false,"number":"10086","numberPresentation":0,"name":null,"namePresentation":0,"uusInfo":null,"isEmergency":false,"isOutgoing":true,"isConference":true,"started":1408918986253}

// call state changed of 10010
08-24 22:23:22.280 I/Gecko   (  108): TelephonyProvider: handleCallStateChange: {"state":0,"callIndex":2,"toa":129,"isMpty":true,"isMT":false,"als":0,"isVoice":true,"isVoicePrivacy":false,"number":"10010","numberPresentation":0,"name":null,"namePresentation":0,"uusInfo":null,"isEmergency":false,"isOutgoing":true,"isConference":true,"started":1408918999490}

// hangUp 10010
08-24 22:23:24.780 I/Gecko   (  108): RIL Worker: [1] Received chrome message {"callIndex":2,"rilMessageClientId":1,"rilMessageToken":31,"rilMessageType":"hangUp"}

// Pay attention to the left call 10086
// "isConference":false (former is true)
// "isMpty":true (former is true)
08-24 22:23:25.250 I/Gecko   (  108): TelephonyProvider: handleCallStateChange: {"state":0,"callIndex":1,"toa":129,"isMpty":true,"isMT":false,"als":0,"isVoice":true,"isVoicePrivacy":false,"number":"10086","numberPresentation":0,"name":null,"namePresentation":0,"uusInfo":null,"isEmergency":false,"isOutgoing":true,"isConference":false,"started":1408918986253}

// dial 10010 again

// click to merge two calls again
08-24 22:23:40.440 I/Gecko   (  108): -*- RadioInterface[1]: Received message from worker: {"rilMessageClientId":1,"rilMessageToken":35,"rilMessageType":"conferenceCall","rilRequestType":16,"rilRequestError":0,"success":true}

// fail to merge
08-24 22:23:40.440 I/Gecko   (  108): TelephonyProvider: handleCallStateChange: {"state":0,"callIndex":1,"toa":129,"isMpty":true,"isMT":false,"als":0,"isVoice":true,"isVoicePrivacy":false,"number":"10086","numberPresentation":0,"name":null,"namePresentation":0,"uusInfo":null,"isEmergency":false,"isOutgoing":true,"isConference":false,"started":1408918986253}

// the call 10010 changed to "isConference":true
08-24 22:23:40.480 I/Gecko   (  108): TelephonyProvider: handleCallStateChange: {"state":0,"callIndex":2,"toa":129,"isMpty":true,"isMT":false,"als":0,"isVoice":true,"isVoicePrivacy":false,"number":"10010","numberPresentation":0,"name":null,"namePresentation":0,"uusInfo":null,"isEmergency":false,"isOutgoing":true,"isConference":true,"started":1408919017456}

// Later, the call 10010 changed to "isConference":false
08-24 22:23:40.540 I/Gecko   (  108): TelephonyProvider: handleCallStateChange: {"state":0,"callIndex":2,"toa":129,"isMpty":true,"isMT":false,"als":0,"isVoice":true,"isVoicePrivacy":false,"number":"10010","numberPresentation":0,"name":null,"namePresentation":0,"uusInfo":null,"isEmergency":false,"isOutgoing":true,"isConference":false,"started":1408919017456}

Comment 3

[helloarvin]

Dear shawn,

I think this might be a gecko issue. We need set the flag isMpty as false when only one call remained in a conference call.

ril_worker.js:
 _ensureConference: function() {

    if (remaining.length == 1) {
      // Remove that if only does one remain in a conference call.
      let call = this.currentCalls[remaining[0]];
      call.isConference = false;
    + call.isMpty = false;

Please sku help to check the issue, thank you.

Comment 4

[helloarvin]

Dear Shawn,

Any update on the issue?
It's an urgent block issue for us as it has 100% reproduce rate. Try to be understanding and thank you very much.

Comment 5

[shawn ku [:sku]]

Hi Aknow:
 Could you please help check this issue (comment 2 and comment 3)?
Thanks!!
Shawn

Comment 6

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

(In reply to shawn ku [:sku] from comment #5)
> Hi Aknow:
>  Could you please help check this issue (comment 2 and comment 3)?
> Thanks!!
> Shawn

(In reply to helloarvin from comment #3)
> Dear shawn,
> 
> I think this might be a gecko issue. We need set the flag isMpty as false
> when only one call remained in a conference call.
> 
> ril_worker.js:
>  _ensureConference: function() {
> 
>     if (remaining.length == 1) {
>       // Remove that if only does one remain in a conference call.
>       let call = this.currentCalls[remaining[0]];
>       call.isConference = false;
>     + call.isMpty = false;
> 
> Please sku help to check the issue, thank you.

Hi, Arvin,

Gecko reads |isMpty| directly read from rild [1], and doesn't modify the value. Gecko counts on that value to maintain conference state correctly. I am wondering if it's correct/expected behaviour that rild reports |isMpty == true| even there's no multiparty call, because ril.h defines |isMpty; /* nonzero if is mpty call */|. Could you help clarify it? Thanks!

[1] http://dxr.mozilla.org/mozilla-central/source/dom/system/gonk/ril_worker.js?from=ril_worker.js#5601

Comment 7

[shawn ku [:sku]]

Hi Arvin:
 I agree with HsinYi.

Hi Sam:
 Please help check with modem team to see if rild got correct information regarding CLCC.

// 3GPP 27.007 7.18 List current calls +CLCC
[+CLCC: <ccid1>,<dir>,<stat>,<mode>,<mpty>[,<number>
,<type>[,<alpha>[,<priority>[,<CLI validity>]]]]
[<CR><LF>+CLCC: <ccid2>,<dir>,<stat>,<mode>,<mpty>[,
<number>,<type>[,<alpha>[,<priority>[,<CLI validity>
]]]]
[...]]]

...
<mpty>: integer type
0 call is not one of multiparty (conference) call parties
1 call is one of multiparty (conference) call parties


08-24 22:23:34.600 D/use-Rlog/RLOG-AT(  122): [w] Channel4: AT> ATD10010;
...
08-24 22:23:40.420 D/use-Rlog/RLOG-AT(  122): [w] Channel4: AT> AT+CLCC
08-24 22:23:40.420 D/use-Rlog/RLOG-AT(  122): [w] Channel4: AT< +CLCC: 1,0,0,0,1,"10086",129
08-24 22:23:40.420 D/use-Rlog/RLOG-AT(  122): [w] Channel4: AT< +CLCC: 2,0,0,0,1,"10010",129
08-24 22:23:40.430 D/use-Rlog/RLOG-AT(  122): [w] Channel4: AT> AT+CLCC
08-24 22:23:40.430 D/use-Rlog/RLOG-AT(  122): [w] Channel4: AT< +CLCC: 1,0,0,0,1,"10086",129
08-24 22:23:40.430 D/use-Rlog/RLOG-AT(  122): [w] Channel4: AT< +CLCC: 2,0,0,0,1,"10010",129


CLCC told us both 10086 and 10010 are in conference call state. This should be wrong.

Comment 8

[shawn ku [:sku]]

correct log snippet and time stamp.

Looks like 10086 with wrong mpty state beginning from "08-24 22:23:25.060"

// only 1 call alive, but mpty flag seems wrong.
08-24 22:23:25.060 D/use-Rlog/RLOG-AT(  122): [w] Channel4: AT< +CLCC: 1,0,0,0,1,"10086",129
...
08-24 22:23:34.390 D/use-Rlog/RLOG-AT(  122): [w] Channel4: AT> AT+CLCC
08-24 22:23:34.390 D/use-Rlog/RLOG-AT(  122): [w] Channel4: AT< +CLCC: 1,0,1,0,1,"10086",129
08-24 22:23:34.400 D/use-Rlog/RLOG-AT(  122): [w] Channel4: AT> AT+CLCC
08-24 22:23:34.400 D/use-Rlog/RLOG-AT(  122): [w] Channel4: AT< +CLCC: 1,0,1,0,1,"10086",129
...
08-24 22:23:34.600 D/use-Rlog/RLOG-AT(  122): [w] Channel4: AT> ATD10010;
...
08-24 22:23:34.930 D/use-Rlog/RLOG-AT(  122): [w] Channel4: AT> AT+CLCC
08-24 22:23:34.930 D/use-Rlog/RLOG-AT(  122): [w] Channel4: AT< +CLCC: 1,0,1,0,1,"10086",129
08-24 22:23:34.930 D/use-Rlog/RLOG-AT(  122): [w] Channel4: AT< +CLCC: 2,0,2,0,0,"10010",129
08-24 22:23:37.320 D/use-Rlog/RLOG-AT(  122): [w] Channel4: AT> AT+CLCC
08-24 22:23:37.320 D/use-Rlog/RLOG-AT(  122): [w] Channel4: AT< +CLCC: 1,0,1,0,1,"10086",129
08-24 22:23:37.320 D/use-Rlog/RLOG-AT(  122): [w] Channel4: AT< +CLCC: 2,0,3,0,0,"10010",129
08-24 22:23:37.430 D/use-Rlog/RLOG-AT(  122): [w] Channel4: AT> AT+CLCC
08-24 22:23:37.440 D/use-Rlog/RLOG-AT(  122): [w] Channel4: AT< +CLCC: 1,0,1,0,1,"10086",129
08-24 22:23:37.440 D/use-Rlog/RLOG-AT(  122): [w] Channel4: AT< +CLCC: 2,0,0,0,0,"10010",129
...
08-24 22:23:39.560 D/use-Rlog/RLOG-RILC(  122): [w] [0101]> CONFERENCE 
08-24 22:23:40.350 D/use-Rlog/RLOG-RILC(  122): [w] [0101]< CONFERENCE
...
08-24 22:23:40.420 D/use-Rlog/RLOG-AT(  122): [w] Channel4: AT> AT+CLCC
08-24 22:23:40.420 D/use-Rlog/RLOG-AT(  122): [w] Channel4: AT< +CLCC: 1,0,0,0,1,"10086",129
08-24 22:23:40.420 D/use-Rlog/RLOG-AT(  122): [w] Channel4: AT< +CLCC: 2,0,0,0,1,"10010",129
08-24 22:23:40.430 D/use-Rlog/RLOG-AT(  122): [w] Channel4: AT> AT+CLCC
08-24 22:23:40.430 D/use-Rlog/RLOG-AT(  122): [w] Channel4: AT< +CLCC: 1,0,0,0,1,"10086",129
08-24 22:23:40.430 D/use-Rlog/RLOG-AT(  122): [w] Channel4: AT< +CLCC: 2,0,0,0,1,"10010",129

Comment 9

[sam.hua]

Hi Shawn/Hsin,

Towards to GSM 02.84, clause 1.3.8, once a MPTY is established, it is terminated in the case when on remote parties remain.

So our modem team think in this case, the mpty call state should be 1 instead of 0.

Comment 10

[shawn ku [:sku]]

Hi Sam:
 I think your modem do the wrong interpretation on spec.

GSM 02.84, clause 1.3.8.2 Managing an active multiParty call
...
(v) Disconnect a remote party:
Explicitly release the remote parties on a one at a time basis (see NOTE 1). In the case when no remote parties remain, the multiParty call is terminated.


This talks about "No remote parties" (not "No remote party remains") remain, the multiParty call is terminated.

Let's move back to check Arvin's steps,
...
3> Make a MOC and then merge the two calls;
4> Hangup one call and then make a MOC;
...

After step4, the multiParty call should be terminated due to "no remote parties" because we only have one active party (ie: 10086).
In other words, the 5th variable should be 0 (not 1) for "10086".

Comment 12

[shawn ku [:sku]]

According to 3GPP TS 51.010-1 clause 31.4.2.2.1 and 31.4.2.1.4, MPTY flag remains true if remote end call.

3GPP TS 51.010-1 version 10.3.0 Release 10
31.4.2.2.1 Release from the MultiParty call
Step 8: Transaction identifier of Call A-C, state U10, with auxiliary state "Call in MPTY"

31.4.2.1.4 Explicitly disconnect a remote party
Step 7: Transaction identifier of Call A-C, state U10, with auxilliary state "Call in MPTY"


Hi Aknow/Hsinyi:
 Could you please help check how to fix this issue w/o checking mpty flag?

Thanks!!
Shawn

Comment 13

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

(In reply to shawn ku [:sku] from comment #12)
> According to 3GPP TS 51.010-1 clause 31.4.2.2.1 and 31.4.2.1.4, MPTY flag
> remains true if remote end call.
> 
> 3GPP TS 51.010-1 version 10.3.0 Release 10
> 31.4.2.2.1 Release from the MultiParty call
> Step 8: Transaction identifier of Call A-C, state U10, with auxiliary state
> "Call in MPTY"
> 
> 31.4.2.1.4 Explicitly disconnect a remote party
> Step 7: Transaction identifier of Call A-C, state U10, with auxilliary state
> "Call in MPTY"
> 

Thanks for the info!


> 
> Hi Aknow/Hsinyi:
>  Could you please help check how to fix this issue w/o checking mpty flag?
> 
> Thanks!!
> Shawn

Seems we need to check state of every call and count the number of calls sharing the same state to maintain .isConference manually.

Comment 14

[shawn ku [:sku]]

Hi Aknow:
 Can we know the status of this bug?
Thanks!!
Shawn

Comment 15

[Szu-Yu Chen [:aknow] (inactive)]

(In reply to shawn ku [:sku] from comment #14)
> Hi Aknow:
>  Can we know the status of this bug?
> Thanks!!
> Shawn

It's my first priority bug now and I'm working on it. However I haven't figured out a better way to determine the conference info without mpty... I guess I can have some progress in this week.

Comment 16

[Szu-Yu Chen [:aknow] (inactive)]

Attachment: Part 1: Redesign the logic of conference detection

Comment 17

[Szu-Yu Chen [:aknow] (inactive)]

Attachment: Part 2: Remove pending conference check

I think we can remove the hasConferenceRequest check that introduced in Bug 1035130. The new logic should work well even if gaia calls 'conference' multiple times.

Comment 18

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

Comment on attachment 8504568 [details] [diff] [review]
Part 1: Redesign the logic of conference detection

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

Can't think of anything better :)

::: dom/system/gonk/ril_worker.js
@@ +374,5 @@
>     */
>    currentCalls: null,
>  
>    /**
>     * Existing conference call and its participants.

Please modify the comment accordingly.

@@ +3977,5 @@
> +    // the order of callDisconnected and conferenceCallStateChanged
> +    // unpredictable.
> +    if (removed) {
> +      this.getFailCauseCode((function(removed, remained, added, failCause) {
> +        this._processClassifiedCalls(removed, remained, added, failCause);

Thanks for correcting the usage of getFailCauseCode!

Comment 19

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

Comment on attachment 8504569 [details] [diff] [review]
Part 2: Remove pending conference check

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

Agree that with part 1 patch, we don't need _hasConferenceRequest flag anymore. Please be sure to clean the flag up thoroughly,e.g. http://dxr.mozilla.org/mozilla-central/source/dom/system/gonk/ril_worker.js?from=ril_worker.js&case=true#1923 and http://dxr.mozilla.org/mozilla-central/source/dom/system/gonk/ril_worker.js?from=ril_worker.js&case=true#5485

r=me with above.

Comment 20

[Josh Cheng [:josh]]

Hi Norry, 
qawanted for Flame 2.0 Thanks!

Comment 21

[Cody Roesch [:croesch]]

Able to successfully merge the conference call the second time using Flame 2.0 KK.

STR:
1. Receive a call to device A from device B.
2. While on a call with Device B, Device A calls Device C.
3. Device A merges the calls with B and C to form a conference call.
4. Device A Hangs up with Device B.
5. Device A calls Device B again.
6. Device A merges the calls with B and C again.

Result: The second Merge is successful and the conference call is established again.

***NOTE: Please let me know if these STR is incorrect. This is how we interpreted the original STR. I tried to put the STR in terms our QA could follow easier :)

Repro Rate: 1/1

Shallow Flash using Flame device set to 319mb

Device: Flame 2.0 KK
BuildID: 20141016184643
Gaia: 9c7dec14e058efef81f2267b724dad0850fc07e4
Gecko: c17df9fe087d
Version: 32.0 (2.0)
Firmware: V180
User Agent: Mozilla/5.0 (Mobile; rv:32.0) Gecko/32.0 Firefox/32.0

Comment 22

[Norry.L.F(Leave from Mozilla)]

Keywords: qawanted → regression
So we skip.

Comment 23

[Szu-Yu Chen [:aknow] (inactive)]

Attachment: [final] Part 1: Redesign the logic of conference detection. r=hsinyi

rebase

Comment 24

[Szu-Yu Chen [:aknow] (inactive)]

Attachment: [final] Part 2: Remove pending conference check. r=hsinyi

rebase

Comment 25

[Szu-Yu Chen [:aknow] (inactive)]

https://treeherder.mozilla.org/ui/#/jobs?repo=try&revision=a3e751eb1bf7

Comment 26

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/integration/b2g-inbound/rev/ac509c4c0027
https://hg.mozilla.org/integration/b2g-inbound/rev/095295d04172

Comment 27

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/ac509c4c0027
https://hg.mozilla.org/mozilla-central/rev/095295d04172

Comment 28

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/ac509c4c0027
https://hg.mozilla.org/mozilla-central/rev/095295d04172

Comment 29

[Kai-Zhen Li [:seinlin][:kli]]

Uplift to 2.0m got conflicts.

Comment 30

[Szu-Yu Chen [:aknow] (inactive)]

Attachment: (2.0m) [final] Part 1: Redesign the logic of conference detection. r=hsinyi

Comment 31

[Szu-Yu Chen [:aknow] (inactive)]

Attachment: (2.0m) [final] Part 2: Remove pending conference check. r=hsinyi

Comment 32

[Kai-Zhen Li [:seinlin][:kli]]

There is still some conflict on part-1. 
--
patching file dom/system/gonk/ril_worker.js
Hunk #3 FAILED at 3953
1 out of 3 hunks FAILED -- saving rejects to file dom/system/gonk/ril_worker.js.rej
patch failed, unable to continue (try -v)
patch failed, rejects left in working dir
--

Could you pull the latest 2.0m source rebbase part-1? 
http://hg.mozilla.org/releases/mozilla-b2g32_v2_0m

Thanks!

Comment 33

[Szu-Yu Chen [:aknow] (inactive)]

Attachment: (2.0m) [final#2] Part 1: Redesign the logic of conference detection. r=hsinyi

Comment 34

[Szu-Yu Chen [:aknow] (inactive)]

Hi seinlin,

The previous commit of my patch is 

Author: Shawn Ku <[e-mail]>
Date:   Thu Oct 23 13:46:55 2014 +0800

    Bug 1046649 - B2G RIL: need to handle wild char for EF_OPL. r=Edgar, a=2.0m+

However, it looks like the same with my previous attachment.

Comment 35

[Kai-Zhen Li [:seinlin][:kli]]

http://hg.mozilla.org/releases/mozilla-b2g32_v2_0m/rev/38c256bb74a1
http://hg.mozilla.org/releases/mozilla-b2g32_v2_0m/rev/88774e7c1b9f

Comment 36

[Kai-Zhen Li [:seinlin][:kli]]

Patch on 2.0m was backout for causing test failed.
https://tbpl.mozilla.org/php/getParsedLog.php?id=50958567&tree=Try

Aknow, Could you have a look? Thanks!

Comment 37

[Szu-Yu Chen [:aknow] (inactive)]

Hi seinlin,

Are you planning to land this one on 2.0m? It's requested to land on 2.0 and 2.1.
https://bugzilla.mozilla.org/show_bug.cgi?id=1077190

Some of the code there is used by the patch of this bug. However, bug 1077190 is not landed on 2.0m so it caused the test failure.

I can also provide the fixed patch which is not relied on bug 1077190. Just want to know your idea.

Comment 38

[Kai-Zhen Li [:seinlin][:kli]]

If bug 1077190 is going to be landed on 2.0, it will get merged into 2.0m. Let's wait if bug 1077190 can get the approval to land on 2.0 till next week. 

If bug 1077190 didn't get the approval to land on 2.0, I'll land it on 2.0m and then land this bug to 2.0m.

Comment 39

[Kai-Zhen Li [:seinlin][:kli]]

Aknow, bug 1077190 is landed on 2.0m. Can you rebase a new patch for 2.0m? Thanks!

Comment 40

[Szu-Yu Chen [:aknow] (inactive)]

Attachment: (2.0m) [final#3] Part 1: Redesign the logic of conference detection. r=hsinyi

Comment 41

[Szu-Yu Chen [:aknow] (inactive)]

Attachment: (2.0m) [final#3] Part 2: Remove pending conference check. r=hsinyi

Comment 42

[Szu-Yu Chen [:aknow] (inactive)]

checkin-needed for 2.0m
https://treeherder.mozilla.org/ui/#/jobs?repo=try&revision=f91d6c147e9c

Comment 43

[Kai-Zhen Li [:seinlin][:kli]]

http://hg.mozilla.org/releases/mozilla-b2g32_v2_0m/rev/9740fa769d8d
http://hg.mozilla.org/releases/mozilla-b2g32_v2_0m/rev/ef4bd5e71188

Comment 44

[Coler]

This issue has been verified successfully on Woodduck 2.0.
Reproducing rate: 0/5
See attachment: Verify_Woodduck_Mergecall.mp4

Woodduck build version:
Gaia-Rev        d742e375aca6dc1bf3a36638000ad7f5338ef457
Gecko-Rev       d049d4ef127844121c9cf14d2e8ca91fd9045fcb
Build-ID        20141126050313
Version         32.0

Comment 45

[Coler]

Attachment: Verify_Woodduck_Mergecal.MP4