Closed
Bug 764378
Opened 12 years ago
Closed 12 years ago
RIL: sometimes RIL model was not detected
Categories
(Core :: DOM: Device Interfaces, defect)
Tracking
()
RESOLVED
FIXED
mozilla16
People
(Reporter: swu, Assigned: vliu)
Details
Attachments
(1 file, 3 obsolete files)
1.28 KB,
patch
|
philikon
:
review+
|
Details | Diff | Splinter Review |
Sometimes RIL fails to get correct RIL model. On SGS2 or Nexus-S, this may cause problem to receive phone calls. 1. NG case: I/Gecko ( 1847): RIL Worker: Handling parcel as REQUEST_VOICE_REGISTRATION_STATE I/Gecko ( 1847): RIL Worker: voice registration state: 0,0000,00000000 I/Gecko ( 1847): RIL Worker: Detected RIL implementation Samsung RIL(IPC) v2.0 I/Gecko ( 1847): RIL Worker: Detected RIL model I/Gecko ( 1847): RIL Worker: Next parcel size unknown, going to sleep. I/Gecko ( 1847): RIL Worker: Received 4 bytes. I/Gecko ( 1847): RIL Worker: Already read 0 When this happens, stop/start b2g can recover it. 2. OK case: I/Gecko ( 2253): RIL Worker: Handling parcel as REQUEST_VOICE_REGISTRATION_STATE I/Gecko ( 2253): RIL Worker: voice registration state: 1,2833,01230737 I/Gecko ( 2253): RIL Worker: Detected RIL implementation Samsung RIL(IPC) v2.0 I/Gecko ( 2253): RIL Worker: Detected RIL model I9100 I/Gecko ( 2253): RIL Worker: Detected I9100, enabling RILQUIRKS_CALLSTATE_EXTRA_UINT32, RILQUIRKS_DATACALLSTATE_DOWN_IS_UP, RILQUIRKS_REQUEST_USE_DIAL_EMERGENCY_CALL. I/Gecko ( 2253): RIL Worker: New outgoing parcel of type 100 I/Gecko ( 2253): RIL Worker: Outgoing parcel: 0,0,0,8,100,0,0,0,14,0,0,0 I/Gecko ( 2253): RIL Worker: Next parcel size unknown, going to sleep.
Reporter | ||
Comment 1•12 years ago
|
||
Not sure why it fails to get RIL model in ril_worker.js. We should not set "this.rilQuirksInitialized = true" in this condition.
Assignee | ||
Comment 2•12 years ago
|
||
(In reply to Shian-Yow Wu from comment #1) > Not sure why it fails to get RIL model in ril_worker.js. > We should not set "this.rilQuirksInitialized = true" in this condition. I modify code to solve this issue. Please see the attached file. Actually it fails to get model_id at the first time and then it can get the correct model_id in the following calling. This patch can skip the failed acquisition. This is the reason why I add this patch.
Attachment #634453 -
Flags: feedback?(philipp)
Comment 3•12 years ago
|
||
(In reply to Shian-Yow Wu from comment #1) > Not sure why it fails to get RIL model in ril_worker.js. In some cases the RIL needs to be "warmed up", e.g. having made a call or similar. > We should not set "this.rilQuirksInitialized = true" in this condition. Good point.
Comment 4•12 years ago
|
||
Comment on attachment 634453 [details] [diff] [review] Patch file to solve this issue. Review of attachment 634453 [details] [diff] [review]: ----------------------------------------------------------------- ::: dom/system/gonk/ril_worker.js @@ +700,5 @@ > } > RILQUIRKS_DATACALLSTATE_DOWN_IS_UP = true; > + } else { > + if (DEBUG) debug("Fail to get model_id."); > + return; It would be more correct to test whether actually don't get a value for `model_id` (null or "" both evaluate to false): let model_id = libcutils.property_get("ril.model_id"); if (!model_id) { // On some RIL models, the RIL has to be "warmed up" for us to read this property. // It apparently isn't warmed up yet, going to try again later. if (DEBUG) debug("Could not detect model_id. Going to try later."); return; } if (DEBUG) debug("Detected RIL model " + model_id);
Attachment #634453 -
Flags: feedback?(philipp) → feedback-
Assignee | ||
Comment 5•12 years ago
|
||
(In reply to Philipp von Weitershausen [:philikon] from comment #4) > It would be more correct to test whether actually don't get a value for > `model_id` (null or "" both evaluate to false): > > let model_id = libcutils.property_get("ril.model_id"); > if (!model_id) { > // On some RIL models, the RIL has to be "warmed up" for us to read this > property. > // It apparently isn't warmed up yet, going to try again later. > if (DEBUG) debug("Could not detect model_id. Going to try later."); > return; > } > if (DEBUG) debug("Detected RIL model " + model_id); Thanks for the suggestion and add the patch file.
Attachment #634723 -
Flags: review?(philipp)
Assignee | ||
Comment 6•12 years ago
|
||
(In reply to Philipp von Weitershausen [:philikon] from comment #4) > It would be more correct to test whether actually don't get a value for > `model_id` (null or "" both evaluate to false): > > let model_id = libcutils.property_get("ril.model_id"); > if (!model_id) { > // On some RIL models, the RIL has to be "warmed up" for us to read this > property. > // It apparently isn't warmed up yet, going to try again later. > if (DEBUG) debug("Could not detect model_id. Going to try later."); > return; > } > if (DEBUG) debug("Detected RIL model " + model_id); Thanks for the suggestion and add the patch file.
Attachment #634453 -
Attachment is obsolete: true
Attachment #634723 -
Attachment is obsolete: true
Attachment #634723 -
Flags: review?(philipp)
Attachment #634768 -
Flags: review?(philipp)
Comment 7•12 years ago
|
||
Comment on attachment 634768 [details] [diff] [review] Patch file to solve this issue. Review of attachment 634768 [details] [diff] [review]: ----------------------------------------------------------------- ::: dom/system/gonk/ril_worker.js @@ +691,1 @@ > if (DEBUG) debug("Detected RIL model " + model_id); Nit: if we put the debug statement before the `if` block, we will know which RIL model caused us to bail out. r=me with that.
Attachment #634768 -
Flags: review?(philipp) → review+
Assignee | ||
Comment 8•12 years ago
|
||
(In reply to Philipp von Weitershausen [:philikon] from comment #7) > Comment on attachment 634768 [details] [diff] [review] > Patch file to solve this issue. > > Review of attachment 634768 [details] [diff] [review]: > ----------------------------------------------------------------- > > ::: dom/system/gonk/ril_worker.js > @@ +691,1 @@ > > if (DEBUG) debug("Detected RIL model " + model_id); > > Nit: if we put the debug statement before the `if` block, we will know which > RIL model caused us to bail out. > > r=me with that. Added the patch.
Attachment #634768 -
Attachment is obsolete: true
Attachment #635196 -
Flags: review?(philipp)
Updated•12 years ago
|
Attachment #635196 -
Flags: review?(philipp) → review+
Updated•12 years ago
|
Keywords: checkin-needed
Comment 9•12 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/b792285b0815 Thanks for the patch, vliu!. In the future, please make sure your patch contains your committer info and a valid commit message. See http://blog.bonardo.net/2010/06/22/so-youre-about-to-use-checkin-needed
Comment 10•12 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/b792285b0815
Status: NEW → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
You need to log in
before you can comment on or make changes to this bug.
Description
•