Last Comment Bug 728886 - B2G RIL: support RIL_VERSION = 6
: B2G RIL: support RIL_VERSION = 6
Status: RESOLVED FIXED
:
Product: Core
Classification: Components
Component: DOM: Device Interfaces (show other bugs)
: Trunk
: ARM Gonk (Firefox OS)
: -- normal (vote)
: mozilla14
Assigned To: Philipp von Weitershausen [:philikon]
:
Mentors:
Depends on:
Blocks: 726233 gonk-ics 905007
  Show dependency treegraph
 
Reported: 2012-02-20 07:51 PST by Fernando Jiménez Moreno [:ferjm] (PTO until August)
Modified: 2013-08-13 23:02 PDT (History)
8 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
Part 0 (v1): Use constants when initializing RadioInterfaceLayer (1.54 KB, patch)
2012-03-20 17:43 PDT, Philipp von Weitershausen [:philikon]
kyle: review+
Details | Diff | Splinter Review
Part 1 (v1): Introduce ril.h v6 consts (15.12 KB, patch)
2012-03-20 17:47 PDT, Philipp von Weitershausen [:philikon]
kyle: review+
Details | Diff | Splinter Review
Part 2 (v1): Support RIL v6 parcels (off by default) (17.48 KB, patch)
2012-03-20 18:00 PDT, Philipp von Weitershausen [:philikon]
kyle: review+
Details | Diff | Splinter Review
Part 1 (v2): Introduce ril.h v6 consts (17.28 KB, patch)
2012-03-27 14:47 PDT, Philipp von Weitershausen [:philikon]
kyle: review+
Details | Diff | Splinter Review
Part 2 (v2): Support RIL v6 parcels (26.46 KB, patch)
2012-03-27 14:49 PDT, Philipp von Weitershausen [:philikon]
kyle: review+
Details | Diff | Splinter Review

Description Fernando Jiménez Moreno [:ferjm] (PTO until August) 2012-02-20 07:51:43 PST
The SMSC can´t be retrieved using Akami devices. It just shows 'Cannot send the SMS. Need to get the SMSC address first.' message. I´ll give further info (RIL data) as soon as I get an akami again...
Comment 1 Fernando Jiménez Moreno [:ferjm] (PTO until August) 2012-02-20 07:57:31 PST
This may sound like a firmware issue. Anyway, we would probably want to deal with this TODO -> https://hg.mozilla.org/mozilla-central/file/0a7410527788/dom/system/b2g/ril_worker.js#l2039
Comment 2 Philipp von Weitershausen [:philikon] 2012-02-20 16:19:30 PST
Digging a bit deeper, it seems that the radio state isn't notified beyond RADIO_STATE_SIM_NOT_READY. 

First I'm getting a notification for RADIO_STATE_UNAVAILABLE = 1 (the initial value being RADIO_STATE_UNAVAILABLE = 1):

I/Gecko   (   83): RIL Worker: Handling parcel as UNSOLICITED_RESPONSE_RADIO_STATE_CHANGED
I/Gecko   (   83): RIL Worker: Radio state changed from 1 to 1

Then, surprisingly, I get a notification for UNSOLICITED_RESPONSE_SIM_STATUS_CHANGED. AFAIUI, this is an invitation to query REQUEST_GET_SIM_STATUS = 1 which is what the RIL worker does immediately, but it gets an error:

I/Gecko   (   83): RIL Worker: Parcel (size 12): 0,0,0,0,1,0,0,0,1,0,0,0
I/Gecko   (   83): RIL Worker: We have at least one complete parcel.
I/Gecko   (   83): RIL Worker: Received error 1 for solicited parcel type 1

Later, we get another UNSOLICITED_RESPONSE_RADIO_STATE_CHANGED notification for RADIO_STATE_OFF = 0:

I/Gecko   (   83): RIL Worker: Handling parcel as UNSOLICITED_RESPONSE_RADIO_STATE_CHANGED
I/Gecko   (   83): RIL Worker: Radio state changed from 1 to 0

The last radio state notification I get is for RADIO_STATE_SIM_NOT_READY = 2;

I/Gecko   (   83): RIL Worker: Handling parcel as UNSOLICITED_RESPONSE_RADIO_STATE_CHANGED
I/Gecko   (   83): RIL Worker: Radio state changed from 0 to 2

After this I don't get any UNSOLICITED_RESPONSE_RADIO_STATE_CHANGED or UNSOLICITED_RESPONSE_SIM_STATUS_CHANGED anymore, even though clearly later the SIM card comes online (e.g. signal strength is notified, calls can be made, etc.). Could this be a rild bug on the akami?
Comment 3 Philipp von Weitershausen [:philikon] 2012-02-22 01:27:33 PST
Yeaaaah, so it turns out our Gonk is just stuck in the past and the akami's RIL is newer. Header file changed pretty dramatically.

Working on a fix.
Comment 4 Philipp von Weitershausen [:philikon] 2012-03-20 17:43:30 PDT
Created attachment 607793 [details] [diff] [review]
Part 0 (v1): Use constants when initializing RadioInterfaceLayer

Going to attach these patches without much testing to get the review off the ground while my build monster is churning through several builds in parallel.

Here's some trivial stuff to warm up!
Comment 5 Philipp von Weitershausen [:philikon] 2012-03-20 17:47:04 PDT
Created attachment 607795 [details] [diff] [review]
Part 1 (v1): Introduce ril.h v6 consts

Pretty mechanical stuff: add/change some consts according to ril.h v6. And, in some cases, the ril.h on Maguro/Akami. Which is slightly different. Kill me now.
Comment 6 Kyle Machulis [:kmachulis] [:qdot] 2012-03-20 17:54:40 PDT
Comment on attachment 607795 [details] [diff] [review]
Part 1 (v1): Introduce ril.h v6 consts

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

::: dom/system/gonk/RadioInterfaceLayer.js
@@ +201,3 @@
>          this.updateDataConnection(state);
>  
>          //TODO for simplicity's sake, for now we only look at

Nit: space between // and TODO
Comment 7 Philipp von Weitershausen [:philikon] 2012-03-20 18:00:52 PDT
Created attachment 607802 [details] [diff] [review]
Part 2 (v1): Support RIL v6 parcels (off by default)

This adds support for RIL v6 parcels, disabled for now. We need to figure out a runtime way of detecting the RIL version. I'd like to punt on that for now, at least in this patch, but possibly even this bug.

To summarize this patch:

* Several parcels have changed structure, so in some cases we now have readFoobar_v5 and readFoobar_v6. In other other cases, when the delta is small, it's an inline check.

* Radio state and card state are handled differently now. The existing code is one of the oldest pieces in RIL that I wrote when I really didn't know much about what the state machine would look like. We have a better idea now. Also, RIL v6 handles radio state and card state separately, whereas v5 conflated the two ideas. So we have to work around that a little, basically deferring all things that depend on the card being ready to the actual REQUEST_GET_SIM_STATUS handler.
Comment 8 Kyle Machulis [:kmachulis] [:qdot] 2012-03-20 18:06:04 PDT
Comment on attachment 607802 [details] [diff] [review]
Part 2 (v1): Support RIL v6 parcels (off by default)

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

::: dom/system/gonk/ril_consts.js
@@ +1215,5 @@
> +const DATACALL_FAIL_SIGNAL_LOST = -3;
> +const DATACALL_FAIL_PREF_RADIO_TECH_CHANGED = -4;
> +const DATACALL_FAIL_RADIO_POWER_OFF = -5;
> +const DATACALL_FAIL_TETHERED_CALL_ACTIVE = -6;
> +const DATACALL_FAIL_ERROR_UNSPECIFIED = 0xffff;

Are these our consts? 'cause the sign bit on the unspecified error not being flipped seems weird. But if we're just copying, ignore me.
Comment 9 Philipp von Weitershausen [:philikon] 2012-03-20 18:10:43 PDT
(In reply to Kyle Machulis [:kmachulis] [:qdot] from comment #8)
> Comment on attachment 607802 [details] [diff] [review]
> Part 2 (v1): Support RIL v6 parcels (off by default)
> 
> Review of attachment 607802 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> ::: dom/system/gonk/ril_consts.js
> @@ +1215,5 @@
> > +const DATACALL_FAIL_SIGNAL_LOST = -3;
> > +const DATACALL_FAIL_PREF_RADIO_TECH_CHANGED = -4;
> > +const DATACALL_FAIL_RADIO_POWER_OFF = -5;
> > +const DATACALL_FAIL_TETHERED_CALL_ACTIVE = -6;
> > +const DATACALL_FAIL_ERROR_UNSPECIFIED = 0xffff;
> 
> Are these our consts? 'cause the sign bit on the unspecified error not being
> flipped seems weird. But if we're just copying, ignore me.

Just copying from the RIL_DataCallFailCause struct in ril.h.
Comment 10 Philipp von Weitershausen [:philikon] 2012-03-27 14:47:57 PDT
Created attachment 609895 [details] [diff] [review]
Part 1 (v2): Introduce ril.h v6 consts

A few more missing constants and a typo fix.
Comment 11 Philipp von Weitershausen [:philikon] 2012-03-27 14:49:54 PDT
Created attachment 609896 [details] [diff] [review]
Part 2 (v2): Support RIL v6 parcels

There were some typos and incomplete implementations of the v6 parcels in the v1 patch. These are all fixed now and tested on the Nexus S + ICS gonk. We also auto-detect RIL v6 now.

Note You need to log in before you can comment on or make changes to this bug.