901232 - B2G RIL: Data call not working on Nexus S

Status: RESOLVED FIXED

(Core :: DOM: Device Interfaces, defect)

Description

[:gerard-majax]

While testing Nexus S with Gecko master, I ran into an issue of data call not working at all.

This happens on two SIM cards, Free Mobile and Bouygues Telecom. This is not related to APN configuration error, but rather to the setupDataCall() method of the RIL which issues the wrong data to the RIL.

I'm currently cross checking the behavior on an Inari, but as far as I can say on Nexus S, setupDataCall() gets called with the following parameters:

I/Gecko   (   77): RIL Worker[0]: setupDataCall: with: {"rilMessageType":"setupDataCall","radioTech":9,"apn":"mmsbouygtel.com","chappap":3,"pdptype":"IP","rilRequestType":27,"rilRequestError":null}

And the method directly uses the radioTech and chappap fields. Comments of the method indicates:
 - radioTech should be an integer indicating the radio technology, and can its value can either be DATACALL_RADIOTECHNOLOGY_CDMA or DATACALL_RADIOTECHNOLOGY_GSM
 - chappap indicates the type of authentication to use, and it can be either: DATACALL_AUTH_NONE, DATACALL_AUTH_PAP, DATACALL_AUTH_CHAP or DATACALL_AUTH_PAP_OR_CHAP

Those constants are defined in ril_consts.js as this:

  this.DATACALL_RADIOTECHNOLOGY_CDMA = 0; 
  this.DATACALL_RADIOTECHNOLOGY_GSM = 1; 
  
  this.DATACALL_AUTH_NONE = 0; 
  this.DATACALL_AUTH_PAP = 1; 
  this.DATACALL_AUTH_CHAP = 2; 
  this.DATACALL_AUTH_PAP_OR_CHAP = 3; 

So, the values passed are falses, at least for the radio technology. The obvious outcome is that trying to perform a CDMA data call when registered on a GSM network will fail.

Forcing the value by rewriting the options.radioTech field with DATACALL_RADIOTECHNOLOGY_GSM makes it working again.

Comment 1

[:gerard-majax]

This is from bug 833271.

Comment 2

[:gerard-majax]

Attachment: Establish data call with the correct radio technology set

Please find a patch that add a mapping from the radio technologies names used in Gecko to the radio technology family (CDMA or GSM) that the modem is supposed to receive. This fixes the issue on Nexus S.

As far as I could test, this does not break anything on Inari.

Comment 3

[Vicamo Yang [:vicamo][:vyang]]

Comment on attachment 785396 [details] [diff] [review]
Establish data call with the correct radio technology set

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

Hi, Lissy,

This patch is also not quite correct.  According to hardware/ril[1] and frameworks/base[2], |radioTech| should be |RIL.DATACALL_RADIOTECHNOLOGY_GSM| if |RILQUIRKS_V5_LEGACY| is true, and |RIL.GECKO_RADIO_TECH.indexOf(radioTechType) + 2| otherwise.

[1]: https://github.com/mozilla-b2g/platform_hardware_ril/blob/master/include/telephony/ril.h#L1544
[2]: http://androidxref.com/4.0.4/xref/frameworks/base/telephony/java/com/android/internal/telephony/DataConnection.java#358

Comment 5

[:gerard-majax]

(In reply to Vicamo Yang [:vicamo][:vyang] from comment #3)
> Comment on attachment 785396 [details] [diff] [review]
> Establish data call with the correct radio technology set
> 
> Review of attachment 785396 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> Hi, Lissy,
> 
> This patch is also not quite correct.  According to hardware/ril[1] and
> frameworks/base[2], |radioTech| should be |RIL.DATACALL_RADIOTECHNOLOGY_GSM|
> if |RILQUIRKS_V5_LEGACY| is true, and
> |RIL.GECKO_RADIO_TECH.indexOf(radioTechType) + 2| otherwise.
> 
> [1]:
> https://github.com/mozilla-b2g/platform_hardware_ril/blob/master/include/
> telephony/ril.h#L1544
> [2]:
> http://androidxref.com/4.0.4/xref/frameworks/base/telephony/java/com/android/
> internal/telephony/DataConnection.java#358

I just got an eye on the ril.h, and I'm not convinced by the comments:

  1516  * ((const char **)data)[0] Radio technology to use: 0-CDMA, 1-GSM/UMTS, 2...
  1517  *                          for values above 2 this is RIL_RadioTechnology + 2.

It is unclear to me, since they mixup GSM and UMTS.

After playing with strings on the libsec-ril.so file, I'm starting to suspect that Samsung's RIL is v5, so I'll update my patch to meet the correct behavior for RIL v5 (and this would clearly be consistent).

Thanks for those pointers.

Comment 6

[:gerard-majax]

Attachment: Establish data call with the correct radio technology set v2

Updated patch, Samsung's RIL seems to be v5 in fact, so just correct the behavior for v5 RIL.
This has not been tested yet, so not setting r? for now.

Comment 7

[:gerard-majax]

(In reply to Alexandre LISSY :gerard-majax from comment #6)
> Created attachment 785620 [details] [diff] [review]
> Establish data call with the correct radio technology set v2
> 
> Updated patch, Samsung's RIL seems to be v5 in fact, so just correct the
> behavior for v5 RIL.
> This has not been tested yet, so not setting r? for now.

I checked, |adb logcat radio -b | grep version| clearly shows "RIL version 6".

Comment 8

[:gerard-majax]

Comment on attachment 785620 [details] [diff] [review]
Establish data call with the correct radio technology set v2

Putting r? since after testing this seems to fix it.

Comment 9

[Vicamo Yang [:vicamo][:vyang]]

Comment on attachment 785620 [details] [diff] [review]
Establish data call with the correct radio technology set v2

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

Please move this fix into RILNetworkInterface::connect.

Comment 10

[:gerard-majax]

Attachment: Establish data call with the correct radio technology set v3

Moving to connect().

Comment 11

[:gerard-majax]

Attachment: Establish data call with the correct radio technology set v4

Fixing radioTechnology redefinition. I'm testing this one.

Comment 12

[:gerard-majax]

Okay, I can't explain why, but I can't reproduce this issue now. Doing +2 for me forces CDMA when I'm UMTS.

Comment 13

[Vicamo Yang [:vicamo][:vyang]]

The code is obviously wrong, maybe we can do something and find out what actually happens.

Comment 14

[Ken Chang[:kenkai]]

After offline discussion with Vicamo, we are going to fix this bug in ril_work in order to modify the this code for vendor dependent RILD in the future.
Will base on Lissy's patch 785620 to provide a new one.

Comment 15

[Ken Chang[:kenkai]]

Attachment: stablish data call with the correct radio technology set v5

Comment 16

[Vicamo Yang [:vicamo][:vyang]]

Comment on attachment 786176 [details] [diff] [review]
stablish data call with the correct radio technology set v5

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

We'll still have to know which devices work with this change and which ones don't before committing it.

::: dom/system/gonk/ril_worker.js
@@ +2168,5 @@
> +    // otherwise, it must be + 2
> +    //
> +    // See also bug 901232 and 867873
> +    let radioTech = options.radioTech + 2;
> +    if (RILQUIRKS_V5_LEGACY) {

let radioTech;
if (RILQUIRKS_V5_LEGACY) {
  ...
} else {
  ...
}

Comment 17

[:gerard-majax]

I'm getting the same behavior than Nexus S on Desire Z (RIL reports version 2). I'm unable to make data call unless I force the radioTech to be 0.

Comment 18

[Ken Chang[:kenkai]]

Attachment: Stablish data call with the correct radio technology set v6

Have tested this patch on unagi, n970, and nexus-s.

Comment 19

[Vicamo Yang [:vicamo][:vyang]]

Comment on attachment 790124 [details] [diff] [review]
Stablish data call with the correct radio technology set v6

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

::: dom/system/gonk/ril_worker.js
@@ +2247,5 @@
> +    if (RILQUIRKS_V5_LEGACY) {
> +      radioTech = DATACALL_RADIOTECHNOLOGY_GSM;
> +      if (this._isCdma) {
> +        radioTech = DATACALL_RADIOTECHNOLOGY_CDMA;
> +      }

radioTech = this._isCdma ? DATACALL_RADIOTECHNOLOGY_GSM
                         : DATACALL_RADIOTECHNOLOGY_CDMA;

Comment 20

[Ken Chang[:kenkai]]

Attachment: Stablish data call with the correct radio technology set, r=vicamo

Comment 21

[Ken Chang[:kenkai]]

try: https://tbpl.mozilla.org/?tree=Try&rev=240772bb8ad7

Comment 22

[Ken Chang[:kenkai]]

Attachment: Stablish data call with the correct radio technology set, r=vyang

Rebase.

Comment 23

[Vicamo Yang [:vicamo][:vyang]]

https://hg.mozilla.org/integration/b2g-inbound/rev/bea35fbe37d0

Comment 24

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/bea35fbe37d0