Note: There are a few cases of duplicates in user autocompletion which are being worked on.

B2G RIL: reponse id collision in Akami & RILv6

RESOLVED FIXED in mozilla14

Status

()

Core
DOM: Device Interfaces
RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: vicamo, Assigned: allstars)

Tracking

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

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(2 attachments, 3 obsolete attachments)

(Reporter)

Description

5 years ago
Excerpted from bug 740238 comment 8:

> Full log is available in http://pastebin.com/bTziLNhr . The log is captured
> with gecko built WITHOUT attachment 611760 [details] [diff] [review].
> 
> Line 124: rild crash at boot.
> Line 663: Parcel (size 8): 1,0,0,0,10,4,0,0
> Line 666: got UNSOLICITED_RIL_CONNECTED(1034), but it is actually
>           UNSOLICITED_VOICE_RADIO_TECH_CHANGED(also 1034) for Akami. Since
>           all payload was already consumed, parcel guard throws an
>           exception in next read attempt.
> Line 1035: got 'sendSMS' dom message from RadioInterfaceLayer.
> Line 1036: sending REQUEST_GET_SMSC_ADDRESS(100)
> Line 1043: got response of REQUEST_GET_SMSC_ADDRESS(100) with error
>            ERROR_GENERIC_FAILURE(2)
Hey Yoshi, since you have an Akami right now, could you take a look at this? Thanks!
Assignee: nobody → yhuang
(Assignee)

Comment 2

5 years ago
(In reply to Philipp von Weitershausen [:philikon] from comment #1)
> Hey Yoshi, since you have an Akami right now, could you take a look at this?
> Thanks!

Sure !
(Assignee)

Comment 3

5 years ago
Created attachment 613904 [details] [diff] [review]
Patch to prevent response id collision

I found the headers for ril can be found in [1]
Also I found the collision has been fixed in their ics branch. [2]
So instead to write code like  
if (model_id === "??" && version === "??") .... something like that, 

I just add a simple check for the length.

The long term solution may be we generate the ril_consts.js according to the target we did in 'make config-XXX' to get the corresponding ril.h in the build time, like what Android did in RILConstants.java 

thanks


[1] :https://www.codeaurora.org/gitweb/quic/la/?p=platform/hardware/ril.git;a=summary
[2]: http://bit.ly/Ixx0J7
Attachment #613904 - Flags: review?(philipp)
Comment on attachment 613904 [details] [diff] [review]
Patch to prevent response id collision

Review of attachment 613904 [details] [diff] [review]:
-----------------------------------------------------------------

::: dom/system/gonk/ril_worker.js
@@ +2430,5 @@
>  RIL[UNSOLICITED_OEM_HOOK_RAW] = null;
>  RIL[UNSOLICITED_RINGBACK_TONE] = null;
>  RIL[UNSOLICITED_RESEND_INCALL_MUTE] = null;
>  RIL[UNSOLICITED_RIL_CONNECTED] = function UNSOLICITED_RIL_CONNECTED(length) {
> +  // To prevent responsed id collision between UNSOLICITED_RIL_CONNECTED and

s/To p/P/
s/responsed/response/

@@ +2432,5 @@
>  RIL[UNSOLICITED_RESEND_INCALL_MUTE] = null;
>  RIL[UNSOLICITED_RIL_CONNECTED] = function UNSOLICITED_RIL_CONNECTED(length) {
> +  // To prevent responsed id collision between UNSOLICITED_RIL_CONNECTED and
> +  // UNSOLICITED_VOICE_RADIO_TECH_CHANGED for Akami on gingerbread branch.
> +  if (length === 0) {

Could just be `if (!length)`
Attachment #613904 - Flags: review?(philipp) → review+
(Assignee)

Comment 5

5 years ago
Created attachment 613915 [details] [diff] [review]
Patch to prevent response id collision

Philikon's comment addressed.
Thanks, philikon :)
Attachment #613904 - Attachment is obsolete: true
(Assignee)

Comment 6

5 years ago
Created attachment 613938 [details] [diff] [review]
Part 2: Set RILQUIRKS_V5_LEGACY to false on akami

I take the WIP from Bug 714973, thanks to jaoo.
and call this initRILQuirks in RIL_CONNECTED handler(which should be UNSOLICITED_VOICE_RADIO_TECH_CHANGED handler on akami).
I found this response (UNSOLICITED_VOICE_RADIO_TECH_CHANGED) comes just after SIGNAL_STRENGTH and RADIO_STATE_CHANGED, so call initRILQuirks here so RILQUIRKS_V5_LEGACY should already be set before any code needs it.

thanks
Attachment #613938 - Flags: review?(philipp)
Comment on attachment 613938 [details] [diff] [review]
Part 2: Set RILQUIRKS_V5_LEGACY to false on akami

Review of attachment 613938 [details] [diff] [review]:
-----------------------------------------------------------------

::: dom/system/gonk/ril_worker.js
@@ +2439,5 @@
>    if (!length) {
> +    this.initRILQuirks();
> +    if (DEBUG) {
> +      debug("RILQUIRKS_V5_LEGACY is " + RILQUIRKS_V5_LEGACY);
> +    }

Please move this debug statement into initRILQuirks(), akin to the other ones ("Detected ..., enabling ..."). r=me with that!
Attachment #613938 - Flags: review?(philipp) → review+
(Assignee)

Comment 8

5 years ago
Created attachment 614320 [details] [diff] [review]
Part 1: Prevent response id collision v2

Address to philikon's comments. 
Thanks, philikon.
Attachment #613915 - Attachment is obsolete: true
Attachment #614320 - Flags: review?(philipp)
(Assignee)

Comment 9

5 years ago
Created attachment 614321 [details] [diff] [review]
Part 2: Set RILQUIRKS_V5_LEGACY to false on akami v2

Address to philikon's coments.
Since in the patch I set RILQUIRKS_V5_LEGACY to false, 
so I change to debug message to "disabling", instead of "enabling".

thanks
Attachment #613938 - Attachment is obsolete: true
Attachment #614321 - Flags: review?(philipp)
Attachment #614320 - Flags: review?(philipp) → review+
Attachment #614321 - Flags: review?(philipp) → review+
(Assignee)

Comment 10

5 years ago
(In reply to Yoshi Huang[:yoshi] from comment #9)
> Created attachment 614321 [details] [diff] [review]
> Part 2: Set RILQUIRKS_V5_LEGACY to false on akami v2
Part 2 uses code from jaoo's patch in Bug714973 to detect ril implementation.

Thanks jaoo :)
https://hg.mozilla.org/integration/mozilla-inbound/rev/bfa9350d9ed9
https://hg.mozilla.org/integration/mozilla-inbound/rev/a19f6f1a4615
https://hg.mozilla.org/mozilla-central/rev/bfa9350d9ed9
https://hg.mozilla.org/mozilla-central/rev/a19f6f1a4615
Status: NEW → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla14
You need to log in before you can comment on or make changes to this bug.