728886 - B2G RIL: support RIL_VERSION = 6

Status: RESOLVED FIXED

(Core :: DOM: Device Interfaces, defect)

Description

[Fernando Jiménez Moreno [:ferjm]]

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]]

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]]

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]]

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]]

Attachment: 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]]

Attachment: 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 [:qdot] [:kmachulis] (INACTIVE)]

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]]

Attachment: 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 [:qdot] [:kmachulis] (INACTIVE)]

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]]

(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]]

Attachment: Part 1 (v2): Introduce ril.h v6 consts

A few more missing constants and a typo fix.

Comment 11

[Philipp von Weitershausen [:philikon]]

Attachment: 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.

Comment 12

[Philipp von Weitershausen [:philikon]]

https://hg.mozilla.org/mozilla-central/rev/53001c577ab9
https://hg.mozilla.org/mozilla-central/rev/87aa00676c4b
https://hg.mozilla.org/mozilla-central/rev/1db916a98bd4