B2G Emergency Calls: Notify the DOM about wrong emergency numbers.

RESOLVED DUPLICATE of bug 765231

Status

()

Core
DOM: Device Interfaces
RESOLVED DUPLICATE of bug 765231
6 years ago
3 years ago

People

(Reporter: jaoo, Unassigned)

Tracking

Trunk
ARM
Gonk (Firefox OS)
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(blocking-basecamp:-)

Details

Attachments

(1 attachment, 3 obsolete attachments)

Comment hidden (empty)
(Reporter)

Updated

6 years ago
Summary: B2G A → B2G Emergency Calls: Notify the DOM about wrong emergency numbers.
(Reporter)

Comment 1

6 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.
Assignee: nobody → josea.olivera
Blocks: 699235
Depends on: 712944
(Reporter)

Comment 2

6 years ago
Created attachment 625661 [details] [diff] [review]
WIP v1

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

6 years ago
Created attachment 625664 [details]
Error log

Error log contaning a |adb logcat| output during a test.
(Reporter)

Updated

6 years ago
Depends on: 757852
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+
Not absolutely required for shipping.
No longer blocks: 699235
blocking-basecamp: --- → -
(Reporter)

Comment 6

6 years ago
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
Attachment #625661 - Attachment is obsolete: true
Attachment #625664 - Attachment is obsolete: true
Attachment #632237 - Flags: feedback?(philipp)
(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 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

6 years ago
Created attachment 634118 [details] [diff] [review]
WIP v3

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)

Updated

6 years ago
Depends on: 764690
(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?
(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 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

6 years ago
Assignee: josea.olivera → nobody

Updated

6 years ago
Status: NEW → RESOLVED
Last Resolved: 6 years ago
Resolution: --- → DUPLICATE
Duplicate of bug: 765231
You need to log in before you can comment on or make changes to this bug.