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

RESOLVED FIXED in Firefox 24, Firefox OS v1.1hd

Status

()

Core
DOM: Device Interfaces
RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: cvan, Assigned: edgar)

Tracking

Trunk
mozilla24
ARM
Gonk (Firefox OS)
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(blocking-b2g:leo+, firefox22 wontfix, firefox23 wontfix, firefox24 fixed, b2g18 fixed, b2g18-v1.0.0 wontfix, b2g18-v1.0.1 wontfix, b2g-v1.1hd fixed)

Details

(Whiteboard: [fixed-in-birch])

Attachments

(6 attachments, 2 obsolete attachments)

(Reporter)

Description

5 years ago
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.
(Reporter)

Comment 1

5 years ago
Affected:
Boot2Gecko 1.0.1.0-prerelease
Operator shelf for marketplace will be incorrect if we don't fix this in Poland.
blocking-b2g: --- → tef?
(Assignee)

Comment 3

5 years ago
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
ni?(cvan) to provide radio logs for comment #3.
Flags: needinfo?(cvan)
(Reporter)

Comment 5

5 years ago
Created attachment 760579 [details]
adb logcat -b main -b radio -v threadtime
Flags: needinfo?(cvan)
(Reporter)

Comment 6

5 years ago
Created attachment 760580 [details]
adb logcat -b main -b radio -v threadtime | grep -Ei 'mcc|mnc|lastknown'
(Assignee)

Comment 7

5 years ago
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
Assignee: nobody → echen
(Assignee)

Comment 8

5 years ago
Created attachment 760813 [details] [diff] [review]
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.
(Assignee)

Comment 9

5 years ago
Created attachment 760816 [details] [diff] [review]
Part 2: Xpcshell tests for mcc/mnc parsing, v1

Xpcshell tests
(Assignee)

Updated

5 years ago
Attachment #760813 - Flags: review?(allstars.chh)
(Assignee)

Updated

5 years ago
Attachment #760816 - Flags: review?(allstars.chh)
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, !==
Attachment #760813 - Flags: review?(allstars.chh) → review+
Attachment #760816 - Flags: review?(allstars.chh) → review+
(Assignee)

Comment 11

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

Address review comment #10
Attachment #760813 - Attachment is obsolete: true
Attachment #761471 - Flags: review+
(Assignee)

Comment 12

5 years ago
try server result: https://tbpl.mozilla.org/?tree=Try&rev=23adefd69b1c
(Assignee)

Updated

5 years ago
Keywords: checkin-needed
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.
(Assignee)

Comment 14

5 years ago
(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.
Keywords: checkin-needed
(Assignee)

Comment 15

5 years ago
Created attachment 761856 [details] [diff] [review]
Part 1: Check mcc table if EF_AD dose not contain the length of mnc, v3, r=allstars.chh

Address comment #13
Attachment #761471 - Attachment is obsolete: true
Attachment #761856 - Flags: review+
(Assignee)

Comment 16

5 years ago
try server: https://tbpl.mozilla.org/?tree=Try&rev=0de49ab5cc0d
(Assignee)

Updated

5 years ago
Blocks: 866645
(Assignee)

Updated

5 years ago
Keywords: checkin-needed
http://hg.mozilla.org/projects/birch/rev/7eb2fe2b8551
http://hg.mozilla.org/projects/birch/rev/eeceac81b3e3
Keywords: checkin-needed
Whiteboard: [fixed-in-birch]
https://hg.mozilla.org/mozilla-central/rev/7eb2fe2b8551
https://hg.mozilla.org/mozilla-central/rev/eeceac81b3e3
Status: NEW → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla24
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: ........................................^
Depends on: 887674
All of our partners are TA, so this will have to land for the first time in 1.1. leo+
blocking-b2g: tef? → leo+
Needs a b2g18 patch for uplift. I assume you'll pick up the fix for bug 887674 while you're at it.
status-b2g18: --- → affected
status-b2g18-v1.0.0: --- → wontfix
status-b2g18-v1.0.1: --- → wontfix
status-b2g-v1.1hd: --- → affected
status-firefox22: --- → wontfix
status-firefox23: --- → wontfix
status-firefox24: --- → fixed
Flags: needinfo?(echen)
Keywords: branch-patch-needed
(Assignee)

Comment 22

5 years ago
Yes, I will pick up the fix for bug 887674 when I provide the b2g18 patch here.
Flags: needinfo?(echen)
(Assignee)

Comment 23

5 years ago
Created attachment 770090 [details] [diff] [review]
[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
(Assignee)

Comment 24

5 years ago
Created attachment 770091 [details] [diff] [review]
[b2g18] Part 2: Xpcshell tests for mcc/mnc parsing, v1, r=allstars.chh, a=leo+

Patch for b2g18 (include the fix for bug 887674)
https://hg.mozilla.org/releases/mozilla-b2g18/rev/2f3e948185d6
https://hg.mozilla.org/releases/mozilla-b2g18/rev/01eacfdb4793
status-b2g18: affected → fixed
Keywords: branch-patch-needed
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
status-b2g-v1.1hd: affected → fixed
You need to log in before you can comment on or make changes to this bug.