880369 - navigator.mozMobileConnection.lastKnownHomeNetwork reports an invalid MCC/MNC pair for Polish T-Mobile SIM

Status: RESOLVED FIXED

(Core :: DOM: Device Interfaces, defect)

Description

[Christopher Van Wiemeersch [:cvan]]

1) Insert a Polish T-Mobile SIM card.

2) Look at navigator.mozMobileConnection.lastKnownHomeNetwork or navigator.mozMobileConnection.lastKnownNetwork, both of which are reported as

   260-02137341765

An easy way to get this information is to install this packaged app from the Marketplace:

    https://marketplace.firefox.com/app/carrier-info/

Or logcat the Marketplace app:

    adb logcat | grep -Ei 'MCC|MNC|lastknown|mobilenetwork'

3) The MCC (260) for Poland is correct (per http://mcclist.com/mobile-network-codes-country-codes.asp and http://en.wikipedia.org/wiki/Mobile_Network_Code#Poland_-_PL). But the MNC (02137341765) is not.

The maximum length of a MNC is 3 digits, and I'm not even sure how to parse that MNC. It looks like 260-02 would be a valid MCC/MNC pair, so that's probably what the API needs to return.

Comment 1

[Christopher Van Wiemeersch [:cvan]]

Affected:
Boot2Gecko 1.0.1.0-prerelease

Comment 2

[Jason Smith [:jsmith]]

Operator shelf for marketplace will be incorrect if we don't fix this in Poland.

Comment 3

[Edgar Chen [:edgar]]

Hi :cvan,

I can not reproduce this issue with Chunghwa SIM card. In your case, it seems device can not parse MNC correctly, this may a SIM card dependent issue. Could you help to capture log with RIL debug enabled?

---------------------------------------
Enable RIL debug log:
adb remount
adb pull /system/b2g/defaults/pref/user.js user.js
[update user.js with adding/modifying 'pref("ril.debugging.enabled", true);']
adb push user.js /system/b2g/defaults/pref/user.js
adb shell reboot

---------------------------------------
Please capture log by below command and start at the beginning of device boot-up:
adb logcat -b main -b radio -v threadtime > logcat.txt

Thanks

Comment 4

[Andrew Overholt [:overholt]]

ni?(cvan) to provide radio logs for comment #3.

Comment 5

[Christopher Van Wiemeersch [:cvan]]

Attachment: adb logcat -b main -b radio -v threadtime

Comment 6

[Christopher Van Wiemeersch [:cvan]]

Attachment: adb logcat -b main -b radio -v threadtime | grep -Ei 'mcc|mnc|lastknown'

Comment 7

[Edgar Chen [:edgar]]

From the log, the EF_AD doesn't contain the length of MNC in Polish T-Mobile SIM card. So that MOZ RIL could not parse MNC correctly.

06-10 14:05:15.652   106   269 I Gecko   : RIL Worker: AD: 0, 255, 255, 
06-10 14:05:15.652   106   269 I Gecko   : RIL Worker: MCC: 260 MNC: 021373417656

From the recent spec. [1], the length of MNC is a mandatory field in EF_AD. But from the old spec. [2], it is an optional field. Although most SIM card contains the length of MNC in EF_AD, but I think we still need to have some way to get MNC correctly for the case that EF_AD doesn't contain the length of MNC.

Thanks.

[1] Please see TS 31.102 section 4.2.18
[2] Please see TS 51.011 section 10.3.18

Comment 8

[Edgar Chen [:edgar]]

Attachment: Part 1: Check mcc table if EF_AD dose not contain the length of mnc, v1

If EF_AD dose not contain the length of MNC, check the MCC table to decide the length.

Comment 9

[Edgar Chen [:edgar]]

Attachment: Part 2: Xpcshell tests for mcc/mnc parsing, v1

pcshell tests

Comment 10

[Yoshi Cheng-Hao Huang [:allstars.chh][:allstarschh][:yoshi]]

Comment on attachment 760813 [details] [diff] [review]
Part 1: Check mcc table if EF_AD dose not contain the length of mnc, v1

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

::: dom/system/gonk/ril_consts.js
@@ +2554,5 @@
>  this.PDU_CDMA_MSG_CODING_IS_91_TYPE_CLI = 0x84;
>  this.PDU_CDMA_MSG_CODING_IS_91_TYPE_SMS = 0x85;
>  
>  // Allow this file to be imported via Components.utils.import().
>  this.EXPORTED_SYMBOLS = Object.keys(this);

We'd better leave this line in the last, so I suggest you move your code before this.

::: dom/system/gonk/ril_worker.js
@@ +10202,5 @@
>        if (imsi) {
>          // MCC is the first 3 digits of IMSI.
>          RIL.iccInfo.mcc = imsi.substr(0,3);
>          // The 4th byte of the response is the length of MNC.
> +        let mncLength = ad[3];

You should add another check for ad,
if ad is undefined, (Some SIM from China seems doesn't have EF_AD at all)
ad[3] will throw TypeError.

@@ +10207,5 @@
> +        if (!mncLength) {
> +          // If response dose not contain the length of MNC, check the MCC table
> +          // to decide the length of MNC.
> +          let index = MCC_TABLE_FOR_MNC_LENGTH_IS_3.indexOf(RIL.iccInfo.mcc);
> +          mncLength = (index != -1) ? 3 : 2;

nit, !==

Comment 11

[Edgar Chen [:edgar]]

Attachment: Part 1: Check mcc table if EF_AD dose not contain the length of mnc, v2, r=allstars.chh

Address review comment #10

Comment 12

[Edgar Chen [:edgar]]

try server result: https://tbpl.mozilla.org/?tree=Try&rev=23adefd69b1c

Comment 13

[Yoshi Cheng-Hao Huang [:allstars.chh][:allstarschh][:yoshi]]

Comment on attachment 761471 [details] [diff] [review]
Part 1: Check mcc table if EF_AD dose not contain the length of mnc, v2, r=allstars.chh

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

::: dom/system/gonk/ril_worker.js
@@ +10202,5 @@
>        if (imsi) {
>          // MCC is the first 3 digits of IMSI.
>          RIL.iccInfo.mcc = imsi.substr(0,3);
>          // The 4th byte of the response is the length of MNC.
> +        let mncLength = (ad) ? ad[3] : 0;

ad && ad[3];
seems shorter and better.

Comment 14

[Edgar Chen [:edgar]]

(In reply to Yoshi Huang[:allstars.chh][:yoshi] from comment #13)
> Comment on attachment 761471 [details] [diff] [review]
> Part 1: Check mcc table if EF_AD dose not contain the length of mnc, v2,
> r=allstars.chh
> 
> Review of attachment 761471 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> ::: dom/system/gonk/ril_worker.js
> @@ +10202,5 @@
> >        if (imsi) {
> >          // MCC is the first 3 digits of IMSI.
> >          RIL.iccInfo.mcc = imsi.substr(0,3);
> >          // The 4th byte of the response is the length of MNC.
> > +        let mncLength = (ad) ? ad[3] : 0;
> 
> ad && ad[3];
> seems shorter and better.

okay, I use this to check ad, thanks.

Comment 15

[Edgar Chen [:edgar]]

Attachment: Part 1: Check mcc table if EF_AD dose not contain the length of mnc, v3, r=allstars.chh

Address comment #13

Comment 16

[Edgar Chen [:edgar]]

try server: https://tbpl.mozilla.org/?tree=Try&rev=0de49ab5cc0d

Comment 17

[Yoshi Cheng-Hao Huang [:allstars.chh][:allstarschh][:yoshi]]

http://hg.mozilla.org/projects/birch/rev/7eb2fe2b8551
http://hg.mozilla.org/projects/birch/rev/eeceac81b3e3

Comment 18

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/7eb2fe2b8551
https://hg.mozilla.org/mozilla-central/rev/eeceac81b3e3

Comment 19

[Yoshi Cheng-Hao Huang [:allstars.chh][:allstarschh][:yoshi]]

Comment on attachment 760816 [details] [diff] [review]
Part 2: Xpcshell tests for mcc/mnc parsing, v1

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

::: dom/system/gonk/tests/test_ril_worker_icc.js
@@ +2606,5 @@
>  
> +/**
> + * Verify MCC and MNC parsing
> + */
> +add_test(function test_mcc_mnc_parsing()) {

/home/allstars/src/mozilla/mozilla-central/obj-x86_64-unknown-linux-gnu/_tests/xpcshell/dom/system/gonk/tests/test_ril_worker_icc.js:2624: SyntaxError: syntax error:
/home/allstars/src/mozilla/mozilla-central/obj-x86_64-unknown-linux-gnu/_tests/xpcshell/dom/system/gonk/tests/test_ril_worker_icc.js:2624: add_test(function test_mcc_mnc_parsing()) {

/home/allstars/src/mozilla/mozilla-central/obj-x86_64-unknown-linux-gnu/_tests/xpcshell/dom/system/gonk/tests/test_ril_worker_icc.js:2624: ........................................^

Comment 20

[Alex Keybl [:akeybl]]

All of our partners are TA, so this will have to land for the first time in 1.1. leo+

Comment 21

[Ryan VanderMeulen [:RyanVM]]

Needs a b2g18 patch for uplift. I assume you'll pick up the fix for bug 887674 while you're at it.

Comment 22

[Edgar Chen [:edgar]]

Yes, I will pick up the fix for bug 887674 when I provide the b2g18 patch here.

Comment 23

[Edgar Chen [:edgar]]

Attachment: [b2g18] Part 1: Check mcc table if EF_AD dose not contain the length of mnc, v3, r=allstars.chh, a=leo+

patch for b2g18

Comment 24

[Edgar Chen [:edgar]]

Attachment: [b2g18] Part 2: Xpcshell tests for mcc/mnc parsing, v1, r=allstars.chh, a=leo+

Patch for b2g18 (include the fix for bug 887674)

Comment 25

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-b2g18/rev/2f3e948185d6
https://hg.mozilla.org/releases/mozilla-b2g18/rev/01eacfdb4793

Comment 26

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-b2g18_v1_1_0_hd/rev/2f3e948185d6
https://hg.mozilla.org/releases/mozilla-b2g18_v1_1_0_hd/rev/01eacfdb4793