Closed Bug 1059649 Opened 7 years ago Closed 7 years ago

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

Categories

(Firefox OS Graveyard :: RIL, defect)

ARM
Gonk (Firefox OS)
defect
Not set
major

Tracking

(blocking-b2g:2.0M+, b2g-v2.0 unaffected, b2g-v2.0M verified, b2g-v2.1 affected, b2g-v2.2 fixed)

RESOLVED FIXED
2.1 S7 (24Oct)
blocking-b2g 2.0M+
Tracking Status
b2g-v2.0 --- unaffected
b2g-v2.0M --- verified
b2g-v2.1 --- affected
b2g-v2.2 --- fixed

People

(Reporter: arvin.zhang, Assigned: aknow)

References

Details

Attachments

(7 files, 5 obsolete files)

197.48 KB, image/png
Details
1.51 MB, text/plain
Details
20.24 KB, patch
aknow
: review+
Details | Diff | Splinter Review
2.54 KB, patch
aknow
: review+
Details | Diff | Splinter Review
20.82 KB, patch
aknow
: review+
Details | Diff | Splinter Review
2.54 KB, patch
aknow
: review+
Details | Diff | Splinter Review
5.78 MB, video/mp4
Details
Attached image 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.
Hi Arvin:
 Would you mind attach device/radio log for us to check this issue?
Thanks!!
Shawn
Flags: needinfo?(arvin.zhang)
Attached file 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}
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.
Flags: needinfo?(arvin.zhang)
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.
Flags: needinfo?(sku)
Hi Aknow:
 Could you please help check this issue (comment 2 and comment 3)?
Thanks!!
Shawn
Flags: needinfo?(sku) → needinfo?(szchen)
(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
Flags: needinfo?(szchen)
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.
Flags: needinfo?(sam.hua)
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
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.
Flags: needinfo?(sam.hua) → needinfo?(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".
Flags: needinfo?(sku) → needinfo?(sam.hua)
Duplicate of this bug: 1067780
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
Flags: needinfo?(szchen)
Flags: needinfo?(sam.hua)
Flags: needinfo?(htsai)
(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.
Flags: needinfo?(htsai)
Assignee: nobody → szchen
Flags: needinfo?(szchen)
Hi Aknow:
 Can we know the status of this bug?
Thanks!!
Shawn
Flags: needinfo?(szchen)
(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.
Flags: needinfo?(szchen)
Blocks: Woodduck
blocking-b2g: --- → 2.0M+
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.
Attachment #8504569 - Flags: review?(htsai)
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!
Attachment #8504568 - Flags: review?(htsai) → review+
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.
Attachment #8504569 - Flags: review?(htsai) → review+
Hi Norry, 
qawanted for Flame 2.0 Thanks!
status-b2g-v2.0: --- → ?
Flags: needinfo?(fan.luo)
Keywords: qawanted
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
QA Whiteboard: [QAnalyst-Triage?]
Flags: needinfo?(jmitchell)
Keywords: qawantedregression
Keywords: regression
QA Whiteboard: [QAnalyst-Triage?] → [QAnalyst-Triage+]
Flags: needinfo?(jmitchell)
Keywords: qawanted → regression
So we skip.
Flags: needinfo?(fan.luo)
rebase
Attachment #8504569 - Attachment is obsolete: true
Attachment #8508548 - Flags: review+
Attachment #8508547 - Attachment description: Part 1#2: Redesign the logic of conference detection. r=hsinyi → [final] Part 1: Redesign the logic of conference detection. r=hsinyi
Attachment #8508548 - Attachment description: Part 2#2: Remove pending conference check. r=hsinyi → [final] Part 2: Remove pending conference check. r=hsinyi
https://hg.mozilla.org/mozilla-central/rev/ac509c4c0027
https://hg.mozilla.org/mozilla-central/rev/095295d04172
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → 2.1 S7 (24Oct)
Uplift to 2.0m got conflicts.
Flags: needinfo?(szchen)
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!
Flags: needinfo?(szchen)
Attachment #8510228 - Attachment is obsolete: true
Flags: needinfo?(szchen)
Hi seinlin,

The previous commit of my patch is 

Author: Shawn Ku <sku@mozilla.com>
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.
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!
Flags: needinfo?(szchen)
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.
Flags: needinfo?(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.
Flags: needinfo?(kli)
Aknow, bug 1077190 is landed on 2.0m. Can you rebase a new patch for 2.0m? Thanks!
Keywords: verifyme
Attachment #8510768 - Attachment is obsolete: true
Flags: needinfo?(szchen)
Attachment #8514222 - Flags: review+
Attachment #8514223 - Attachment description: (2.0m) [final] Part 2: Remove pending conference check. r=hsinyi → (2.0m) [final#3] Part 2: Remove pending conference check. r=hsinyi
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
Blocks: 1088690
You need to log in before you can comment on or make changes to this bug.