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)
(deleted),
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)
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•12 years ago
|
||
Not absolutely required for shipping.
No longer blocks: b2g-telephony
blocking-basecamp: --- → -
Reporter | ||
Comment 6•12 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•12 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•12 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•12 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•12 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•12 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•12 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
•