833277 - B2G CDMA Tell if we are on CDMA network

Status: RESOLVED FIXED

(Core :: DOM: Device Interfaces, defect)

Description

[Patrick Wang (Chih-Kai Wang) (:kk1fff)]

We need to tell if we are on CDMA network. We can get current radio technology from UNSOLICITED_RESPONSE_RADIO_STATE_CHANGED handler, or use REQUEST_VOICE_RADIO_TECH that is introduced in RIL v7.

Comment 1

[Patrick Wang (Chih-Kai Wang) (:kk1fff)]

Attachment: Patch: Tell if we are on CDMA network

Comment 2

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

Comment on attachment 705245 [details] [diff] [review]
Patch: Tell if we are on CDMA network

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

r=me with some nits fixed :)

::: dom/system/gonk/ril_consts.js
@@ +2309,5 @@
>    "evdob",
>    "ehrpd",
>    "lte",
>    "hspa+",
> +  "gsm"

You'll also need a NETWORK_CREG_TECH_GSM.

::: dom/system/gonk/ril_worker.js
@@ +3268,5 @@
> +        radioTechString == 'hsupa' ||
> +        radioTechString == 'hspa' ||
> +        radioTechString == 'lte' ||
> +        radioTechString == 'hspa+' ||
> +        radioTechString == 'gsm') {

Could you compare radioTech with NETWORK_CREG_TECH_* instead?

@@ +4951,5 @@
>    this.acknowledgeIncomingGsmSmsWithPDU(success, responsePduLen, options);
>  };
> +RIL[REQUEST_VOICE_RADIO_TECH] = function REQUEST_VOICE_RADIO_TECH(length, options) {
> +  let radioTech = Buf.readUint32List();
> +  if (radioTech && radioTech.length > 0) {

Please check rilRequestError first.

@@ +4954,5 @@
> +  let radioTech = Buf.readUint32List();
> +  if (radioTech && radioTech.length > 0) {
> +    this._processRadioTech(radioTech[0]);
> +  } else {
> +    debug("Error: REQUEST_VOICE_RADIO_TECH has no tech!"); // Should not happen.

It seems it's unnecessary. Just remove it.

Comment 3

[Patrick Wang (Chih-Kai Wang) (:kk1fff)]

Attachment: Patch: Tell if we are on CDMA network v2

Fix according to previous review and use switch/case to identify GSM family.

Comment 4

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

Comment on attachment 705805 [details] [diff] [review]
Patch: Tell if we are on CDMA network v2

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

::: dom/system/gonk/ril_worker.js
@@ +5012,5 @@
>    this.acknowledgeIncomingGsmSmsWithPDU(success, responsePduLen, options);
>  };
> +RIL[REQUEST_VOICE_RADIO_TECH] = function REQUEST_VOICE_RADIO_TECH(length, options) {
> +  if (options.rilRequestError) {
> +    return;

Sorry, I should have commented in previous review, but could you add a debug message here? If somehow we have to fetch voice radio tech and it fails, then the following steps to fetch necessary info will never proceed. Having an debug message here gives a handy hint for development phase.

Comment 5

[Patrick Wang (Chih-Kai Wang) (:kk1fff)]

Attachment: Patch for checkin: Tell if we are on CDMA network

Adding comment according to comment 4.

Comment 6

[Patrick Wang (Chih-Kai Wang) (:kk1fff)]

(In reply to Patrick Wang [:kk1fff] from comment #5)
> Adding comment according to comment 4.
         ^ sorry... adding debug message for failure to get voice radio tech.

Comment 7

[Patrick Wang (Chih-Kai Wang) (:kk1fff)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/418c2f404d74

Comment 8

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/418c2f404d74