B2G RIL: Data call not working on Nexus S

RESOLVED FIXED in mozilla26

Status

()

defect
RESOLVED FIXED
6 years ago
6 years ago

People

(Reporter: gerard-majax, Assigned: kchang)

Tracking

({dogfood})

unspecified
mozilla26
ARM
Gonk (Firefox OS)
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 7 obsolete attachments)

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.
Assignee: nobody → lissyx+mozillians
This is from bug 833271.
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.
Attachment #785396 - Flags: review?(vyang)
Attachment #785396 - Flags: review?(allstars.chh)
Blocks: b2g-nexuss
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
Attachment #785396 - Flags: review?(vyang)
Attachment #785396 - Flags: review?(allstars.chh)
Attachment #785396 - Flags: review-
(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.
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.
Attachment #785396 - Attachment is obsolete: true
(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 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.
Attachment #785620 - Flags: review?(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.
Attachment #785620 - Flags: review?(vyang)
Moving to connect().
Attachment #785620 - Attachment is obsolete: true
Attachment #785637 - Flags: review?(vyang)
Fixing radioTechnology redefinition. I'm testing this one.
Attachment #785637 - Attachment is obsolete: true
Attachment #785637 - Flags: review?(vyang)
Component: General → DOM: Device Interfaces
Product: Boot2Gecko → Core
Summary: Data call not working on Nexus S → B2G RIL: Data call not working on Nexus S
Okay, I can't explain why, but I can't reproduce this issue now. Doing +2 for me forces CDMA when I'm UMTS.
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → WORKSFORME
The code is obviously wrong, maybe we can do something and find out what actually happens.
Status: RESOLVED → REOPENED
Resolution: WORKSFORME → ---
Assignee: lissyx+mozillians → nobody
Assignee: nobody → kchang
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.
Attachment #785642 - Attachment is obsolete: true
Attachment #786176 - Flags: review?(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 {
  ...
}
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.
Have tested this patch on unagi, n970, and nexus-s.
Attachment #786176 - Attachment is obsolete: true
Attachment #786176 - Flags: review?(vyang)
Attachment #790124 - Flags: review?(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;
Attachment #790124 - Flags: review?(vyang) → review+
Attachment #790124 - Attachment is obsolete: true
Attachment #792037 - Flags: review+
Attachment #792037 - Flags: checkin?
Attachment #792037 - Flags: review+
Attachment #792037 - Flags: checkin?
Rebase.
Attachment #792037 - Attachment is obsolete: true
Attachment #794407 - Flags: review+
Attachment #794407 - Flags: checkin?
Status: REOPENED → ASSIGNED
https://hg.mozilla.org/mozilla-central/rev/bea35fbe37d0
Status: ASSIGNED → RESOLVED
Closed: 6 years ago6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla26
You need to log in before you can comment on or make changes to this bug.