Closed Bug 962447 Opened 6 years ago Closed 6 years ago

[fugu][b2g]dial a number out the fdn list,no error message be showed

Categories

(Firefox OS Graveyard :: RIL, defect, blocker)

Other
Gonk (Firefox OS)
defect
Not set
blocker

Tracking

(blocking-b2g:1.3+, firefox28 wontfix, firefox29 wontfix, firefox30 fixed, b2g-v1.3 fixed, b2g-v1.3T fixed, b2g-v1.4 fixed)

RESOLVED FIXED
1.3 C3/1.4 S3(31jan)
blocking-b2g 1.3+
Tracking Status
firefox28 --- wontfix
firefox29 --- wontfix
firefox30 --- fixed
b2g-v1.3 --- fixed
b2g-v1.3T --- fixed
b2g-v1.4 --- fixed

People

(Reporter: ming.li, Assigned: hsinyi)

References

()

Details

Attachments

(2 files)

steps to reproduce:
1.enable fdn
2.dial a number not in the fdn list

expect: error message tells failed by fdn.

but: i got nothing


logs maybe help: 
I/Gecko   (  123): TelephonyProvider: Dialing 10010
I/Gecko   (  123): RIL Worker[0]: Received chrome message {"number":"10010","isDialEmergency":false,"rilMessageToken":48,"rilMessageType":"dial"}

I/Gecko   (  123): -*- RadioInterface[0]: Received message from worker: {"rilMessageType":"callError","callIndex":-1,"errorMsg":"GenericFailure"}
I/Gecko   (  123): TelephonyProvider: listener for notifyError threw an exception: [Exception... "Component returned failure code: 0x8000ffff (NS_ERROR_UNEXPECTED) [nsITelephonyListener.notifyError]"  nsresult: "0x8000ffff (NS_ERROR_UNEXPECTED)"  location: "JS frame :: jar:file:///system/b2g/omni.ja!/components/TelephonyProvider.js :: _notifyAllListeners :: line 158"  data: no]
dear vyang  
pls confirm this
Flags: needinfo?(vyang)
Summary: [b2g]dial a number out the fdn list,no error message be showed → [fugu][b2g]dial a number out the fdn list,no error message be showed
I will take a look.
Flags: needinfo?(vyang)
(In reply to Ming from comment #0)
> steps to reproduce:
> 1.enable fdn
> 2.dial a number not in the fdn list
> 
> expect: error message tells failed by fdn.
> 
> but: i got nothing
> 
> 
> logs maybe help: 
> I/Gecko   (  123): TelephonyProvider: Dialing 10010
> I/Gecko   (  123): RIL Worker[0]: Received chrome message
> {"number":"10010","isDialEmergency":false,"rilMessageToken":48,
> "rilMessageType":"dial"}
> 
> I/Gecko   (  123): -*- RadioInterface[0]: Received message from worker:
> {"rilMessageType":"callError","callIndex":-1,"errorMsg":"GenericFailure"}
> I/Gecko   (  123): TelephonyProvider: listener for notifyError threw an
> exception: [Exception... "Component returned failure code: 0x8000ffff
> (NS_ERROR_UNEXPECTED) [nsITelephonyListener.notifyError]"  nsresult:
> "0x8000ffff (NS_ERROR_UNEXPECTED)"  location: "JS frame ::
> jar:file:///system/b2g/omni.ja!/components/TelephonyProvider.js ::
> _notifyAllListeners :: line 158"  data: no]

These exception messages are not really errors. In Telephony.cpp, we check if a Telephony instance is the one who requested dial() while we receive "notifyError" callback. If not, we don't dispatch error event on that instance. That's why we see NS_ERROR_UNEXPECTED from the log.

Then why is there no error messages popping out on dialer screen?
My observation:
1) From the log you pasted, the fugu rild/modem returns "GenericFailure." Nevertheless, [1] doesn't check "GenericFailure." 

Fugu rild/modem should make sure a specific error message is returned so that gaia could show the definite information to user.
 
2) [1] checks "FDNBlockedError." However, there's another error message "FdnCheckFailure" that would be sent by rild/modem. I tried some devices and "FdnCheckFailure" has been the only error type I got so far, when I was reproducing this issue. I am not very sure when "FDNBlockedError" is gonna happen though this is indeed defined in android ril.h.

So I'd suggest [1] adds one more supported error type -- FdnCheckFailure, in order to not miss this error message.

[1] https://github.com/mozilla-b2g/gaia/blob/master/apps/communications/dialer/js/telephony_helper.js#L101
(In reply to Hsin-Yi Tsai  [:hsinyi] from comment #3)

> Then why is there no error messages popping out on dialer screen?
> My observation:
> 1) From the log you pasted, the fugu rild/modem returns "GenericFailure."
> Nevertheless, [1] doesn't check "GenericFailure." 
> 
> Fugu rild/modem should make sure a specific error message is returned so
> that gaia could show the definite information to user.

hi ,the specific cause for the failer of the last call can be get by ril request of  REQUEST_GET_LAST_CALL_FAIL. So why don't ril_worker do the request of REQUEST_GET_LAST_CALL_FAIL.
I remember that v1.2 does well on this case.
In fact ril/modem did told the specific cause :
D/AT      (  116): [w] Channel1: AT> ATD10010;
D/AT      (  116): [w] Channel0: AT< +ECEER: 241
D/AT      (  116): [w] Channel0: AT< ^CEND: 255,,101,241
D/RIL     (  116): [w] The last call fail cause: 241  
D/AT      (  116): [w] Channel1: AT< +CME ERROR: 21

this.CALL_FAIL_FDN_BLOCKED = 241;
(In reply to Ming from comment #4)
> (In reply to Hsin-Yi Tsai  [:hsinyi] from comment #3)
> 
> > Then why is there no error messages popping out on dialer screen?
> > My observation:
> > 1) From the log you pasted, the fugu rild/modem returns "GenericFailure."
> > Nevertheless, [1] doesn't check "GenericFailure." 
> > 
> > Fugu rild/modem should make sure a specific error message is returned so
> > that gaia could show the definite information to user.
> 
> hi ,the specific cause for the failer of the last call can be get by ril
> request of  REQUEST_GET_LAST_CALL_FAIL. So why don't ril_worker do the
> request of REQUEST_GET_LAST_CALL_FAIL.
> I remember that v1.2 does well on this case.
> In fact ril/modem did told the specific cause :
> D/AT      (  116): [w] Channel1: AT> ATD10010;
> D/AT      (  116): [w] Channel0: AT< +ECEER: 241
> D/AT      (  116): [w] Channel0: AT< ^CEND: 255,,101,241
> D/RIL     (  116): [w] The last call fail cause: 241  
> D/AT      (  116): [w] Channel1: AT< +CME ERROR: 21
> 
> this.CALL_FAIL_FDN_BLOCKED = 241;

We do check that even in the current code.
However, the error happens on REQUEST_DIAL. That means, we don't even successfully make a call out. In this case it makes no sense to have additional call to REQUEST_GET_LAST_CALL_FAIL_CAUSE.

RIL_Errno in ril.h defines a specific error number "RIL_E_FDN_CHECK_FAILURE = 14" and says very clearly that the number is returned due to command failed because recipient is not on FDN list.
As you see , ril/modem get the error CME ERROR:21 ,which means "+CME ERROR: invalid index" .
So which RIL_Errno should it be ? It doesn't match the cause  "RIL_E_FDN_CHECK_FAILURE = 14"

RIL_E_FDN_CHECK_FAILURE = 14"

Code of v1.2  works well :

RIL[REQUEST_DIAL] = function REQUEST_DIAL(length, options) {
  if (options.rilRequestError) {
    // The connection is not established yet.
    options.callIndex = -1;
    this.getFailCauseCode(options);
    return;
  }
};
Attached patch 962447.patchSplinter Review
Per our partner's request, we should use 'getFailCauseCode()' to get the error message even the call connection hasn't been established yet.

It works on other vendor's solution as well.
Attachment #8364184 - Flags: review?(vyang)
tks,also I will verify this with the modem team.
(In reply to Ming from comment #8)
> tks,also I will verify this with the modem team.

No problem! Glad to see the issue being fixed as well :)
How does the solution here affect bug 944589?
(In reply to Vicamo Yang [:vicamo][:vyang] from comment #10)
> How does the solution here affect bug 944589?

Hey Vicamo, 

Bug 944589 reported an issue: if a call is released unexpectedly, in TelephonyProvider.js the code path would be 'notifyCallError' instead of normal 'handleCallDisconnected.' Sadly, TelephonyProvider.js updated audio state only when handleCallDisconnected() been called. This fix of bug 944589 is to make sure we handle a disconnected call well no matter what the fail cause of a call is.

This patch changes the fail cause of a call that wouldn't and shouldn't affect bug 944589.

Also, here is how AOSP addresses this:

In AOSP, the response of REQUEST_DIAL is null. AOSP doesn't address the error of the request. Right after calling REQUEST_DIAL, a new call connection is created and tracked. Then, handlePollCalls() is called. If the new connection doesn't show up in CLCC list in polling, the connection is removed from the tracked list and getLastCallFailCause() is called. Though AOSP doesn't handle error of REQUEST_DIAL, different from us, this patch shares the same logica as AOSP regarding acquiring a fail cause of a call connection which isn't established or isn't in CLCC. Thanks!
Attachment #8364184 - Flags: review?(vyang) → review+
(In reply to Hsin-Yi Tsai  [:hsinyi] from comment #3)

> Fugu rild/modem should make sure a specific error message is returned so
> that gaia could show the definite information to user.

Hi hsin , which specific should i refer to?
(In reply to Ming from comment #12)
> (In reply to Hsin-Yi Tsai  [:hsinyi] from comment #3)
> 
> > Fugu rild/modem should make sure a specific error message is returned so
> > that gaia could show the definite information to user.
> 
> Hi hsin , which specific should i refer to?

Hey Ming,

Our RIL is going to use getFailCauseCode(). The error expected for fdn case is "this.CALL_FAIL_FDN_BLOCKED = 241;" and I believe the fugu already supports per your comment 4.
(In reply to Hsin-Yi Tsai  [:hsinyi] from comment #13)
Ok , tks
Yes , supports .
b2g-inboud TREE is closed. I'll push the patch once it is open again.
(In reply to Hsin-Yi Tsai  [:hsinyi] from comment #15)
> b2g-inboud TREE is closed. I'll push the patch once it is open again.

Still closed... waiting...
https://hg.mozilla.org/mozilla-central/rev/2a4544efc702
Assignee: nobody → htsai
Status: UNCONFIRMED → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → 1.3 C3/1.4 S3(31jan)
pls merge this patch into v1.3 ,needed by TARA
blocking-b2g: --- → 1.3?
Flags: needinfo?(ryanvm)
All 1.3 uplifts have to be approved by Fabrice. Please nominate the patch for approval-mozilla-b2g28.
Flags: needinfo?(ryanvm)
Duplicate of this bug: 967159
1.3+ for the same. Bad FDN error
blocking-b2g: 1.3? → 1.3+
This can't be uplifted to v1.3 until the patch has approval-mozilla-b2g28.
(In reply to Ryan VanderMeulen [:RyanVM UTC-5] from comment #23)
> This can't be uplifted to v1.3 until the patch has approval-mozilla-b2g28.
Flags: needinfo?(htsai)
patch for b2g28_v1_3
Flags: needinfo?(htsai)
Comment on attachment 8378800 [details] [diff] [review]
962447-v1_3.patch -- patch for b2g28_v1_3

NOTE: This flag is now for security issues only. Please see https://wiki.mozilla.org/Release_Management/B2G_Landing to better understand the B2G approval process and landings.

[Approval Request Comment]
Bug caused by (feature/regressing bug #): bug 962447
User impact if declined: user has no idea a phone number can't be dialed out because the number is blocked by FDN
Testing completed: yes
Risk to taking this patch (and alternatives if risky): very low
String or UUID changes made by this patch: no
Attachment #8378800 - Flags: approval-mozilla-b2g28?
Attachment #8378800 - Flags: approval-mozilla-b2g28? → approval-mozilla-b2g28+
Flags: in-moztrap?
Flags: in-moztrap? → in-moztrap+
You need to log in before you can comment on or make changes to this bug.