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

Status: RESOLVED FIXED

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

Description

[Ming]

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.

Comment 1

[Ming]

Attachment: patch for this bug

Comment 2

[Ming]

I hope someone can verify my patch.

Comment 3

[Ming]

Attachment: update patch

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;

Comment 4

[Ming]

Hi edgar ,pls help to review my patch.

Comment 5

[Ming]

Comment on attachment 826378 [details] [diff] [review]
patch for this bug

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

abandon this patch

Comment 6

[Edgar Chen [:edgar]]

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". :)

Comment 7

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

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().

Comment 8

[Ken Chang[:kenkai]]

Hsinyi, Should we take this issue?

Comment 9

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

(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?

Comment 10

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

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.

Comment 11

[Jason Smith [:jsmith]]

QA Wanted - Can we check to see if this reproduces on the latest 1.2 Buri build?

Comment 12

[Preeti Raghunath(:Preeti)]

Please provide:

1) Build that was used to test on Buri?

Comment 13

[Sarah Parsons]

This issue reproduces on the Buri 1.2 Build ID:20131119004006

Gaia   9439907a255e04de4c33493fe03d6670c8256e2f
SourceStamp fe2a7f0db27b
BuildID 20131119004006
Version 26.0
Base: 20131115

Comment 14

[Jason Smith [:jsmith]]

Marking blocking then - this is an obvious blocker if it reproduces.

Comment 15

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

Rex, can you take this bug? since this reproduces on Buri

Comment 16

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

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?

Comment 17

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

(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.

Comment 18

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

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?

Comment 19

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

(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

Comment 20

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

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:(

Comment 21

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

Attachment: RIL log

this is the log after applying the patch above on Buri. (latest gecko 28.0)

Comment 22

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

(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.

Comment 23

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

(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.

Comment 24

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

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.

Comment 26

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

(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.

Comment 27

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

Thanks for Hsin-Yi's help.

I'll send a patch later to confirm Fugu works in this bug.

Comment 28

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

Attachment: 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?

Comment 29

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

Comment on attachment 8339155 [details]
Patch

Seems it's still reproducible on latest Gecko, let me find the cause.
Sorry for bothering.

Comment 30

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

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 31

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

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?

Comment 32

[Etienne Segonzac (:etienne)]

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.

Comment 33

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

Comment on attachment 8339155 [details]
Patch

OK, I Revised by comments. (With a few test case added)
Etienne may you help review again?

Comment 34

[Etienne Segonzac (:etienne)]

Comment on attachment 8339155 [details]
Patch

Perfect, this is ready to be squashed and landed on a green travis.

Comment 35

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

Thanks for reviewing Etienne!

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

Comment 36

[John Ford [:jhford] CET/CEST Berlin Time]

Uplifted 286be9eb5ffc0e879d05703f0d291b5efbd818e6 to:
v1.2: 1d9cc8c6ac39f71d277f1aa785234f767b87f156