Closed
Bug 751550
Opened 13 years ago
Closed 12 years ago
B2G Emergency Calls: Notify the DOM about wrong emergency numbers.
Categories
(Core :: DOM: Device Interfaces, defect)
Tracking
()
RESOLVED
DUPLICATE
of bug 765231
blocking-basecamp | - |
People
(Reporter: jaoo, Unassigned)
References
Details
Attachments
(1 file, 3 obsolete files)
1.36 KB,
patch
|
philikon
:
feedback+
|
Details | Diff | Splinter Review |
No description provided.
Reporter | ||
Updated•13 years ago
|
Summary: B2G A → B2G Emergency Calls: Notify the DOM about wrong emergency numbers.
Reporter | ||
Comment 1•13 years ago
|
||
Once bug 712944 has landed we have to send an error event to the DOM when using wrong emergency numbers when making an emergency call.
Reporter | ||
Comment 2•13 years ago
|
||
WIP patch. Just noticed emergengy calls do not work on ICS. It seems the DIAL_EMERGENCY_CALL request type is no longer available. I have to touch a bit our loved initRILQuirks() function. I'll file a bug for fixing the problem. Current WIP has also another problem I don't know the cause yet. Any feedback is also welcome ;). Find attached a log with some information.
Attachment #625661 -
Flags: feedback?(philipp)
Reporter | ||
Comment 3•13 years ago
|
||
Error log contaning a |adb logcat| output during a test.
Comment 4•13 years ago
|
||
Comment on attachment 625661 [details] [diff] [review]
WIP v1
Review of attachment 625661 [details] [diff] [review]:
-----------------------------------------------------------------
::: dom/system/gonk/ril_worker.js
@@ +1145,5 @@
> + options.callIndex = -1;
> + options.type = "callError";
> + options.error = GECKO_CALL_ERROR_BAD_NUMBER;
> + debug("Dial: GECKO_CALL_ERROR_BAD_NUMBER");
> + this.sendDOMMessage(options);
I would have a slight preference for morphing `options` rather than creating a new object. That way all the info that the main thread added to the message (e.g. the number) will be preserved.
Looks good otherwise!
Attachment #625661 -
Flags: feedback?(philipp) → feedback+
Comment 5•13 years ago
|
||
Not absolutely required for shipping.
No longer blocks: b2g-telephony
blocking-basecamp: --- → -
Reporter | ||
Comment 6•13 years ago
|
||
I go back to work in Emergency calls. I hope we can solve this remaining piece asap. This patch addresses philikon's comments. This is a WIP patch because it doesn't work at all. I really want some feedback on it. In fact, emergency call works in otoro and galaxys2 (the latter needs to have [1] merged) but the functionality we want to implement here doesn't. Here I paste some traces from my tests.
otoro
I/Gecko ( 115): -*- RadioInterfaceLayer: Received 'RIL:Dial' message from content process
I/Gecko ( 115): -*- RadioInterfaceLayer: Dialing 11
I/Gecko ( 115): RIL Worker: Received DOM message {"type":"dial","number":"11"}
I/Gecko ( 115): RIL Worker: Dial: GECKO_CALL_ERROR_BAD_NUMBER
I/Gecko ( 115): -*- RadioInterfaceLayer: Received message from worker: {"type":"callError","number":"11","callIndex":-1,"error":"BadNumberError"}
F/libc ( 115): Fatal signal 11 (SIGSEGV) at 0x00000080 (code=1)
I/DEBUG ( 100): debuggerd committing suicide to free the zombie!
I/DEBUG ( 414): debuggerd: Jun 12 2012 11:23:02
galaxys2
I/Gecko ( 1844): RIL Worker: Received DOM message {"type":"dial","number":"123"}
I/Gecko ( 1844): RIL Worker: Dial: GECKO_CALL_ERROR_BAD_NUMBER
I/Gecko ( 1844): -*- RadioInterfaceLayer: Received message from worker: {"type":"callError","number":"123","callIndex":-1,"error":"BadNumberError"}
F/libc ( 1844): Fatal signal 11 (SIGSEGV) at 0x00dc9fb8 (code=2)
Is it something wrong here?
+ // The connection is not established yet.
+ options.callIndex = -1;
+ options.type = "callError";
+ options.error = GECKO_CALL_ERROR_BAD_NUMBER;
+ debug("Dial: GECKO_CALL_ERROR_BAD_NUMBER");
+ this.sendDOMMessage(options);
[1] https://github.com/mozilla-b2g/android-device-galaxys2/pull/11
Attachment #625661 -
Attachment is obsolete: true
Attachment #625664 -
Attachment is obsolete: true
Attachment #632237 -
Flags: feedback?(philipp)
Comment 7•13 years ago
|
||
(In reply to José Antonio Olivera Ortega [:jaoo] from comment #6)
> Created attachment 632237 [details] [diff] [review]
> WIP v2
>
> I go back to work in Emergency calls. I hope we can solve this remaining
> piece asap. This patch addresses philikon's comments. This is a WIP patch
> because it doesn't work at all. I really want some feedback on it. In fact,
> emergency call works in otoro and galaxys2 (the latter needs to have [1]
> merged) but the functionality we want to implement here doesn't. Here I
> paste some traces from my tests.
>
> otoro
>
> I/Gecko ( 115): -*- RadioInterfaceLayer: Received 'RIL:Dial' message from
> content process
> I/Gecko ( 115): -*- RadioInterfaceLayer: Dialing 11
> I/Gecko ( 115): RIL Worker: Received DOM message
> {"type":"dial","number":"11"}
> I/Gecko ( 115): RIL Worker: Dial: GECKO_CALL_ERROR_BAD_NUMBER
> I/Gecko ( 115): -*- RadioInterfaceLayer: Received message from worker:
> {"type":"callError","number":"11","callIndex":-1,"error":"BadNumberError"}
> F/libc ( 115): Fatal signal 11 (SIGSEGV) at 0x00000080 (code=1)
> I/DEBUG ( 100): debuggerd committing suicide to free the zombie!
> I/DEBUG ( 414): debuggerd: Jun 12 2012 11:23:02
>
> galaxys2
>
> I/Gecko ( 1844): RIL Worker: Received DOM message
> {"type":"dial","number":"123"}
> I/Gecko ( 1844): RIL Worker: Dial: GECKO_CALL_ERROR_BAD_NUMBER
> I/Gecko ( 1844): -*- RadioInterfaceLayer: Received message from worker:
> {"type":"callError","number":"123","callIndex":-1,"error":"BadNumberError"}
> F/libc ( 1844): Fatal signal 11 (SIGSEGV) at 0x00dc9fb8 (code=2)
>
> Is it something wrong here?
>
> + // The connection is not established yet.
> + options.callIndex = -1;
> + options.type = "callError";
> + options.error = GECKO_CALL_ERROR_BAD_NUMBER;
> + debug("Dial: GECKO_CALL_ERROR_BAD_NUMBER");
> + this.sendDOMMessage(options);
>
> [1] https://github.com/mozilla-b2g/android-device-galaxys2/pull/11
I wonder this is the same issue in Bug 764690.
Comment 8•13 years ago
|
||
Comment on attachment 632237 [details] [diff] [review]
WIP v2
Review of attachment 632237 [details] [diff] [review]:
-----------------------------------------------------------------
Looks great! Please address the nits *and write tests for this*. Thanks!
::: dom/system/gonk/RadioInterfaceLayer.js
@@ +11,5 @@
>
> var RIL = {};
> Cu.import("resource://gre/modules/ril_consts.js", RIL);
>
> +const DEBUG = true; // set to true to see debug messages
ahem.
::: dom/system/gonk/ril_worker.js
@@ +31,5 @@
>
> // We leave this as 'undefined' instead of setting it to 'false'. That
> // way an outer scope can define it to 'true' (e.g. for testing purposes)
> // without us overriding that here.
> +let DEBUG = true;
cough.
@@ +1447,5 @@
> + // The connection is not established yet.
> + options.callIndex = -1;
> + options.type = "callError";
> + options.error = GECKO_CALL_ERROR_BAD_NUMBER;
> + debug("Dial: GECKO_CALL_ERROR_BAD_NUMBER");
All debug statements should be gated by an `if (DEBUG)`, but there's really no need for that here since in debug mode we print all messages between the threads.
Attachment #632237 -
Flags: feedback?(philipp) → feedback+
Reporter | ||
Comment 9•13 years ago
|
||
Nits addressed. This functionality depends on bug 764690, thanks hsinyi!. I made a test for checking if everything is ok and what I see was:
I/Gecko ( 1199): -*- RILContentHelper: Dialing 111
I/Gecko ( 1199): -*- RadioInterfaceLayer: Received 'RIL:Dial' message from content process
I/Gecko ( 1199): -*- RadioInterfaceLayer: Dialing 111
I/Gecko ( 1199): RIL Worker: Received DOM message {"type":"dial","number":"111"}
I/Gecko ( 1199): RIL Worker: Dial: GECKO_CALL_ERROR_BAD_NUMBER
I/Gecko ( 1199): -*- RadioInterfaceLayer: Received message from worker: {"type":"callError","number":"111","callIndex":-1,"error":"BadNumberError"}
I/Gecko ( 1199): -*- RILContentHelper: Received message 'RIL:CallError': {"type":"callError","number":"111","callIndex":-1,"error":"BadNumberError"}
I/Gecko ( 1199): -*- RILContentHelper: Requesting enumeration of calls for callback: [xpconnect wrapped nsIRILTelephonyCallback]
I/Gecko ( 1199): -*- RILContentHelper: Registered telephony callback: [xpconnect wrapped nsIRILTelephonyCallback]
Should I see anything in the dailer app when the error occurs?
Attachment #632237 -
Attachment is obsolete: true
Attachment #634118 -
Flags: review?(philipp)
Reporter | ||
Comment 10•13 years ago
|
||
(In reply to Philipp von Weitershausen [:philikon] from comment #8)
> Comment on attachment 632237 [details] [diff] [review]
> WIP v2
>
> Review of attachment 632237 [details] [diff] [review]:
> -----------------------------------------------------------------
>
> Looks great! Please address the nits *and write tests for this*. Thanks!
Philipp, regarding your request about tests what about if I file a bug for adding tests for the whole emergency calls functionality?
Comment 11•13 years ago
|
||
(In reply to José Antonio Olivera Ortega [:jaoo] from comment #10)
> Philipp, regarding your request about tests what about if I file a bug for
> adding tests for the whole emergency calls functionality?
Or just do it as part of this bug.
Comment 12•13 years ago
|
||
Comment on attachment 634118 [details] [diff] [review]
WIP v3
Review of attachment 634118 [details] [diff] [review]:
-----------------------------------------------------------------
Great. Still needs tests.
Attachment #634118 -
Flags: review?(philipp) → feedback+
Reporter | ||
Updated•12 years ago
|
Assignee: josea.olivera → nobody
Updated•12 years ago
|
Status: NEW → RESOLVED
Closed: 12 years ago
Resolution: --- → DUPLICATE
You need to log in
before you can comment on or make changes to this bug.
Description
•