Closed Bug 933153 Opened 8 years ago Closed 8 years ago

[Fugu] [B2G] Put the first call on hold before dialing out the 2nd call, otherwise we cannot end those two calls.

Categories

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

ARM
Linux
defect
Not set
critical

Tracking

(blocking-b2g:koi+, b2g-v1.2 fixed)

RESOLVED FIXED
1.2 C6(Dec6)
blocking-b2g koi+
Tracking Status
b2g-v1.2 --- fixed

People

(Reporter: ming.li, Assigned: rexboy)

References

Details

Attachments

(3 files, 1 obsolete file)

Follow these steps :
1.make a new call. 
2.on the calling screen , add a new call
3.try to stop these calls

result : i can't stop them.


In fact , refered to android dev,before it places a new call ,it will hold the other calls by sending the AT command "AT+CHLD=2".
if the sim card is enabled the call-waiting, this will return successfully, otherwise ,it will fail.

But,ffos did't do that.
Attached patch patch for this bug (obsolete) — Splinter Review
I hope someone can verify my patch.
Attached patch update patchSplinter Review
Finally ,i found that there already is a member function of class of TelephonyCall which can hold the specified call.
And there are two same definitions for the "hold" request.

this.REQUEST_SWITCH_WAITING_OR_HOLDING_AND_ACTIVE = 15;
this.REQUEST_SWITCH_HOLDING_AND_ACTIVE = 15;
Hi edgar ,pls help to review my patch.
Flags: needinfo?(echen)
Comment on attachment 826378 [details] [diff] [review]
patch for this bug

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

abandon this patch
Attachment #826378 - Flags: review-
Comment on attachment 826378 [details] [diff] [review]
patch for this bug

If you want to abandon a patch, you can simply mark it as "obsolete". :)
Attachment #826378 - Attachment is obsolete: true
Attachment #826378 - Flags: review-
Flags: needinfo?(echen)
Flags: needinfo?(echen)
Attachment #826624 - Flags: review?(mchen)
Attachment #826624 - Flags: review?(echen)
Attachment #826624 - Flags: review?(mchen) → review?(vyang)
Attachment #826624 - Flags: review?(vyang)
Attachment #826624 - Flags: review?(htsai)
Attachment #826624 - Flags: review?(echen)
Comment on attachment 826624 [details] [diff] [review]
update patch

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

I agree that strict flow should be holding active calls before making another outgoing call since not every modem well supports that.

However, r- because 
1) There could be an 'active' conferenceCall which needs to be put onhold as well. This patch doesn't well handle that case.
2) Even we put the active call onhold, we would need to wait for response from modem before the next step. Otherwise, we may still make a second call with the wrong state.

To resolve this issue, I'd suggest:
1) In *Gaia*, check there's no active call before dial(). Hold the activeCall if there's any.
2) In *Gecko/Telephony*, have a strict policy in DialInternal().
Attachment #826624 - Flags: review?(htsai) → review-
Hsinyi, Should we take this issue?
Flags: needinfo?(htsai)
(In reply to Ken Chang from comment #8)
> Hsinyi, Should we take this issue?

IMO (comment 7), both gaia and gecko work are required to fully resolve this issue. I could take care of gecko part. We still need some input from gaia. Rex, any comments?
Flags: needinfo?(htsai) → needinfo?(rexboy)
Just looked around the codes, looks fine to me so far to call hold() before making another new call.
I can help on Gaia part if you want to solve this issue.
Flags: needinfo?(rexboy)
Flags: needinfo?(echen)
blocking-b2g: --- → koi?
QA Wanted - Can we check to see if this reproduces on the latest 1.2 Buri build?
Keywords: qawanted
Please provide:

1) Build that was used to test on Buri?
Flags: needinfo?(pcheng)
Keywords: qawanted
Keywords: qawanted
QA Contact: sparsons
This issue reproduces on the Buri 1.2 Build ID:20131119004006

Gaia   9439907a255e04de4c33493fe03d6670c8256e2f
SourceStamp fe2a7f0db27b
BuildID 20131119004006
Version 26.0
Base: 20131115
Keywords: qawanted
Flags: needinfo?(pcheng)
Marking blocking then - this is an obvious blocker if it reproduces.
blocking-b2g: koi? → koi+
Rex, can you take this bug? since this reproduces on Buri
Flags: needinfo?(rexboy)
Assignee: nobody → rexboy
Flags: needinfo?(rexboy)
Summary: [Fugu][B2G]On calling screen, after adding a new call, it can't end these calls → [Fugu] [Buri] [B2G]On calling screen, after adding a new call, it can't end these calls
Status: UNCONFIRMED → NEW
Ever confirmed: true
Target Milestone: --- → 1.2 C6(Dec6)
I'm still reproducing this after calling held() of active call on Buri
(No matter waiting for active.onheld or not)
I guess there are some Gecko changes required here (mentioned in Comment 7?)
The error I got was
11-22 06:48:26.110: E/GeckoConsole(140): [JavaScript Error: "Error: Unknown rilSuppSvcNotification: null" {file: "jar:file:///system/b2g/omni.ja!/components/TelephonyProvider.js" line: 285}]
11-22 06:48:26.110: E/GeckoConsole(140): [JavaScript Error: "NS_ERROR_XPC_JS_THREW_JS_OBJECT: 'Error: Unknown rilSuppSvcNotification: null' when calling method: [nsIGonkTelephonyProvider::notifySupplementaryService]" {file: "jar:file:///system/b2g/omni.ja!/components/RadioInterfaceLayer.js" line: 1170}]

Hsinyi may you confirm on this issue?
Flags: needinfo?(htsai)
(In reply to KM Lee [:rexboy] from comment #16)
> I'm still reproducing this after calling held() of active call on Buri
> (No matter waiting for active.onheld or not)
> I guess there are some Gecko changes required here (mentioned in Comment 7?)
> The error I got was
> 11-22 06:48:26.110: E/GeckoConsole(140): [JavaScript Error: "Error: Unknown
> rilSuppSvcNotification: null" {file:
> "jar:file:///system/b2g/omni.ja!/components/TelephonyProvider.js" line: 285}]
> 11-22 06:48:26.110: E/GeckoConsole(140): [JavaScript Error:
> "NS_ERROR_XPC_JS_THREW_JS_OBJECT: 'Error: Unknown rilSuppSvcNotification:
> null' when calling method:
> [nsIGonkTelephonyProvider::notifySupplementaryService]" {file:
> "jar:file:///system/b2g/omni.ja!/components/RadioInterfaceLayer.js" line:
> 1170}]
> 
> Hsinyi may you confirm on this issue?

Hi Rex,
The error message looks scaring but it's simply because some type of notification hasn't been supported yet. This isn't a real error. Please ignore it.

Regarding comment 7, we need to have gecko's improvement for solid condition check; however, I believe once gaia work is done, this issue could be fixed.
Flags: needinfo?(htsai)
1. Reproducible on Fugu. And confirmed that hold before calling works.
2. I can't reproduce it on another Hamachi/Buri device of 26.0a1
   (Build number 20131114111651)

3. But reproducible on 28.0a1
   (Build number 20131122061537)
   And performing hold before calling doesn't works either. I wonder if
   it's a regression in case of Buri?
(In reply to KM Lee [:rexboy] from comment #18)
> 1. Reproducible on Fugu. And confirmed that hold before calling works.
> 2. I can't reproduce it on another Hamachi/Buri device of 26.0a1
>    (Build number 20131114111651)
> 
> 3. But reproducible on 28.0a1
>    (Build number 20131122061537)
>    And performing hold before calling doesn't works either. I wonder if
>    it's a regression in case of Buri?

Hi Rex,

Would you please help provide ril debug messages, thanks? You could refer to the below document to know how to enable ril debugger: 

https://wiki.mozilla.org/B2G/QA/Tips_And_Tricks#Changing_preferences
WIP,
https://github.com/rexboy7/gaia/commit/43ad21ed7e252cdb7dabe07631b6e5e75d5333e4

works for Fugu but as I said the behavior of Buri need to be confirmed.
I'll paste the RIL debug message later... after I found a working Buri since I just
bricked two Buri devices during flashing:(
Attached file RIL log
this is the log after applying the patch above on Buri. (latest gecko 28.0)
(In reply to KM Lee [:rexboy] from comment #21)
> Created attachment 8337656 [details]
> RIL log
> 
> this is the log after applying the patch above on Buri. (latest gecko 28.0)

Hi Rex, thanks for the log; however I didn't see what I was looking for. :(
With ril debug enabled, we are expecting to see messages with label 'RIL Worker'.
Could you please enable that and attach log again? Thanks.
(In reply to Hsin-Yi Tsai  [:hsinyi] from comment #22)
> (In reply to KM Lee [:rexboy] from comment #21)
> > Created attachment 8337656 [details]
> > RIL log
> > 
> > this is the log after applying the patch above on Buri. (latest gecko 28.0)
> 
> Hi Rex, thanks for the log; however I didn't see what I was looking for. :(
> With ril debug enabled, we are expecting to see messages with label 'RIL
> Worker'.
> Could you please enable that and attach log again? Thanks.

Oh, there are two lines of 'RIL Worker' but we should see more, tens of lines. Maybe the attachment is filtered.
Regarding buri issue, please see what I found:
https://bugzilla.mozilla.org/show_bug.cgi?id=943117#c7

I think the evidence shows our vendor should make the image updated.
Duplicate of this bug: 943117
(In reply to Hsin-Yi Tsai  [:hsinyi] from comment #24)
> Regarding buri issue, please see what I found:
> https://bugzilla.mozilla.org/show_bug.cgi?id=943117#c7
> 
> I think the evidence shows our vendor should make the image updated.

Root causes for fugu and buri are different. They are not duplicates.
For fugu, dialer should be more careful when making a 2nd call.
For buri, base image needs to have the property 'ro.moz.ril.extra_int_2nd_call=true'

I suggest keep the bug scope clear, i.e. using this bug for fugu issue as it originally was, and bug 943117 for tracking buri issue.
Summary: [Fugu] [Buri] [B2G]On calling screen, after adding a new call, it can't end these calls → [Fugu] [B2G] Put the first call on hold before dialing out the 2nd call, otherwise we cannot end those two calls.
Thanks for Hsin-Yi's help.

I'll send a patch later to confirm Fugu works in this bug.
Attached file Patch
It's a simple patch but it changes both dialer and system.

I've tested dialing on Fugu but not the case of Voicemail notification.
(since our carrier uses SMS for voicemail notification)
it should work I think since the change is very straightforward. 

Etienne may you help on reviewing this bug?
Attachment #8339155 - Flags: review?(etienne)
Comment on attachment 8339155 [details]
Patch

Seems it's still reproducible on latest Gecko, let me find the cause.
Sorry for bothering.
Attachment #8339155 - Flags: review?(etienne)
Seems it depends on sim card :-O
There are some problems when holding active phone using certain sim card, so it just hanged up.

Still investigating..
Comment on attachment 8339155 [details]
Patch

By face to face discussion with Etienne and Hsin-Yi, the status is now:
1. Let's Open another bug for handling case of sim card doesn't
   support call holding.
2. The patch now make call after waiting **until active call changes to held**
   rather than right after calling hold() to prevent possible timing issue.

And I think I may need to leave the voicemail case in system app later, because
I think we should discuss, in some way, make use of dialer app to call out here
to have telephony_helper handling all kinds of situations, rather than use
mozTelephony.call directly in system app.

Hello Etienne, may you take a look again?
Attachment #8339155 - Flags: review?(etienne)
See Also: → 944302
Comment on attachment 8339155 [details]
Patch

comments on github, I think we need to take the conferenceGroup into account when checking if we can make the call.
Attachment #8339155 - Flags: review?(etienne)
Comment on attachment 8339155 [details]
Patch

OK, I Revised by comments. (With a few test case added)
Etienne may you help review again?
Attachment #8339155 - Flags: review?(etienne)
Comment on attachment 8339155 [details]
Patch

Perfect, this is ready to be squashed and landed on a green travis.
Attachment #8339155 - Flags: review?(etienne) → review+
Thanks for reviewing Etienne!

merged on master:
https://github.com/mozilla-b2g/gaia/commit/286be9eb5ffc0e879d05703f0d291b5efbd818e6
Status: NEW → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Uplifted 286be9eb5ffc0e879d05703f0d291b5efbd818e6 to:
v1.2: 1d9cc8c6ac39f71d277f1aa785234f767b87f156
You need to log in before you can comment on or make changes to this bug.