Closed
Bug 999458
Opened 10 years ago
Closed 10 years ago
B2G RIL: illegal gsmLocationAreaCode and gsmCellId reported
Categories
(Firefox OS Graveyard :: RIL, defect)
Tracking
(Not tracked)
RESOLVED
FIXED
2.0 S1 (9may)
People
(Reporter: vicamo, Assigned: vicamo)
Details
(Whiteboard: [p=1])
Attachments
(7 files, 3 obsolete files)
62 bytes,
text/x-github-pull-request
|
edgar
:
review+
hsinyi
:
review+
|
Details | Review |
61 bytes,
text/x-github-pull-request
|
edgar
:
review+
|
Details | Review |
1.39 KB,
patch
|
edgar
:
review+
|
Details | Diff | Splinter Review |
6.92 KB,
patch
|
edgar
:
review+
|
Details | Diff | Splinter Review |
1.37 KB,
patch
|
vicamo
:
review+
|
Details | Diff | Splinter Review |
9.64 KB,
patch
|
vicamo
:
review+
|
Details | Diff | Splinter Review |
6.48 KB,
patch
|
vicamo
:
review+
|
Details | Diff | Splinter Review |
From ril_worker.js line 3580, gsmLocationAreaCode off the range is still accepted and reported to content. So we may have: "cell":{"gsmLocationAreaCode":4294967295,"gsmCellId":4294967295} Actually, a bug in hardware/ril makes this always happen inside emulator. From ril.h line 1239 [2], "((const char **)response)[1] is LAC if registered on a GSM/WCDMA system or NULL if not. Valid LAC are 0x0000 - 0xffff". However, in reference-ril.c line L1602 [3], rild always prints out invalid hex octets even it's previously set to -1 in line 1470 [4]. [1]: http://dxr.mozilla.org/mozilla-central/source/dom/system/gonk/ril_worker.js#3580 [2]: https://github.com/mozilla-b2g/platform_hardware_ril/blob/master/include/telephony/ril.h#L1239 [3]: https://github.com/mozilla-b2g/platform_hardware_ril/blob/master/reference-ril/reference-ril.c#L1602 [4]: https://github.com/mozilla-b2g/platform_hardware_ril/blob/master/reference-ril/reference-ril.c#L1470
Assignee | ||
Comment 1•10 years ago
|
||
Should not initialize lac & cid to -1 in qemu, either. See https://github.com/mozilla-b2g/platform_external_qemu/blob/master/telephony/android_modem.c#L656 .
Assignee | ||
Comment 2•10 years ago
|
||
WIP: https://github.com/vicamo/b2g_platform_external_qemu/tree/bugzilla/999458/null-lac-cid-when-unavailable https://github.com/vicamo/b2g_platform_hardware_ril/tree/bugzilla/999458/null-lac-cid-when-unavailable https://github.com/vicamo/b2g_mozilla-central/tree/bugzilla/999458/null-lac-cid-when-unavailable
Assignee | ||
Comment 3•10 years ago
|
||
Several things in concern: 1) Per comment 0, rild may return illegal lac/cid values. This is supposed to be fixed in https://github.com/mozilla-b2g/platform_hardware_ril/pull/35 . 2) Per comment 1, we should initialize lac/cid to UNKNOWN, which is numeric 0, instead of -1 in qemu. This is supposed to be fixed in https://github.com/mozilla-b2g/platform_external_qemu/pull/76 part 1. 3) I also have part 2 in https://github.com/mozilla-b2g/platform_external_qemu/pull/76 to rewrite OPL/PNN records for complex tests. However, such rewrite breaks current test cases and there seems to be no easy way to merge them concurrently. I have to land a patch to disable related tests in Gecko first, and then merge PRs in qemu and rild, and then land patches to adopt new OPL/PNN contents and rewrite test cases. 4) In Gecko, I have a patch part 1/2 to temporarily disable GSM location related test cases, and part 2.a/2 to revert it. Part 2.b/2 replaces some duplicated code and creates two shared utility function setEmulatorGsmLocationAndWait and setEmulatorOperatorNamesAndWait. Part 2.c/2 is the corresponding patch for rild pull request. Part 2.d/2 rewrites test_mobile_operator_names_plmnlist to include more OPL/PNN scenarios. 5) During the process, one thing is worth a special note here. In RIL worker function _processOperator [1], we only re-evaluate network name when either given longName/shortName or mcc/mnc changes. However, determining network name also involves location area code. So we might have a corner case that: 1) OPL contains a record that specifies mcc/mnc and a limited, effective LAC range, 2) the user is currently camped on to a network with that mcc/mnc but he's out of that range, 3) he moves and is registered on a network with the same provider but he's now inside that LAC range, 4) network name is not updated as PNN/OPL suggest. When this problem is mixed with screen state, it becomes even more complex because SET_SCREEN_STATE request may turn off modem lac/cid position reporting and we'll get negative gsmLocationAreaCode from RIL worker. Such transition should not be taken into consideration and should not result in network names change. This problem is not addressed in this bug. [1]: http://mxr.mozilla.org/mozilla-central/source/dom/system/gonk/ril_worker.js#3658
Assignee | ||
Updated•10 years ago
|
Assignee: nobody → vyang
Whiteboard: [p=3]
Target Milestone: --- → 1.4 S6 (25apr)
Assignee | ||
Comment 4•10 years ago
|
||
This, if Bluetooth on JB or above (bug 944299) is not taken into account, is the last known unconditional failure on emulator-kk.
Assignee | ||
Comment 5•10 years ago
|
||
Please see comment 3 2) and 3).
Attachment #8411579 -
Flags: review?(htsai)
Attachment #8411579 -
Flags: review?(echen)
Assignee | ||
Comment 6•10 years ago
|
||
Please see comment 3 1).
Attachment #8411580 -
Flags: review?(echen)
Assignee | ||
Comment 7•10 years ago
|
||
Attachment #8411581 -
Flags: review?(echen)
Assignee | ||
Comment 8•10 years ago
|
||
Attachment #8411584 -
Flags: review?(echen)
Assignee | ||
Comment 9•10 years ago
|
||
Attachment #8411585 -
Flags: review?(echen)
Assignee | ||
Updated•10 years ago
|
Attachment #8411584 -
Attachment description: [Gecko] 2.a/2: revert part 1/2 → [Gecko] part 2.a/2: revert part 1/2
Assignee | ||
Comment 10•10 years ago
|
||
Attachment #8411586 -
Flags: review?(echen)
Assignee | ||
Comment 11•10 years ago
|
||
Attachment #8411587 -
Flags: review?(echen)
Assignee | ||
Comment 12•10 years ago
|
||
Landing sequence: 1) Gecko part 1, 2) qemu, rild pull requests, 3) Gecko part 2.x parts.
Assignee | ||
Comment 13•10 years ago
|
||
(In reply to Vicamo Yang [:vicamo][:vyang] from comment #3) > However, such rewrite breaks current > test cases and there seems to be no easy way to merge them concurrently. I > have to land a patch to disable related tests in Gecko first, and then merge > PRs in qemu and rild, and then land patches to adopt new OPL/PNN contents > and rewrite test cases. Ummm .... it would be really nice to have the ability to adjust SIM EF files at runtime :X
Assignee | ||
Updated•10 years ago
|
Attachment #8411579 -
Attachment mime type: text/plain → text/x-github-pull-request
Updated•10 years ago
|
Attachment #8411581 -
Flags: review?(echen) → review+
Comment 14•10 years ago
|
||
Comment on attachment 8411580 [details] [review] Github PR for hardware/ril, master branch r=me with comments on github addressed. Thank you.
Attachment #8411580 -
Flags: review?(echen) → review+
Comment 15•10 years ago
|
||
Comment on attachment 8411585 [details] [diff] [review] [Gecko] part 2.b/2: add two utility functions to head.js Review of attachment 8411585 [details] [diff] [review]: ----------------------------------------------------------------- ::: dom/mobileconnection/tests/marionette/head.js @@ +543,5 @@ > + * A boolean value. Default false. > + * > + * @return A deferred promise. > + */ > +function setEmulatorGsmLocationAndWait(aLac, aCid, aWaitVoice, aWaitData) { nit: setEmulatorGsmLocationAndWait( ... , aWaitVoice = true, ...) @@ +657,5 @@ > + * > + * @return A deferred promise. > + */ > +function setEmulatorOperatorNamesAndWait(aOperator, aLongName, aShortName, > + aMcc, aMnc, aWaitVoice, aWaitData) { nit: aWaitVoice = true
Attachment #8411585 -
Flags: review?(echen) → review+
Updated•10 years ago
|
Attachment #8411586 -
Flags: review?(echen) → review+
Updated•10 years ago
|
Attachment #8411579 -
Flags: review?(echen) → review+
Comment 16•10 years ago
|
||
Comment on attachment 8411587 [details] [diff] [review] [Gecko] part 2.d/2: test cases Review of attachment 8411587 [details] [diff] [review]: ----------------------------------------------------------------- Thank you. :) ::: dom/mobileconnection/tests/marionette/test_mobile_operator_names_plmnlist.js @@ +92,5 @@ > + > + // Test if we match MNC "01" and "001" correctly. > + // > + // Use |diffCid| to ensure voicechange event will be triggered in the > + // next run. I guess this comment is for the last test (the reset one). If it is the case, could you please help to adjust it? Thank you.
Attachment #8411587 -
Flags: review?(echen) → review+
Updated•10 years ago
|
Attachment #8411584 -
Flags: review?(echen) → review+
Comment 17•10 years ago
|
||
Comment on attachment 8411579 [details] [review] Github PR for external/qemu, master branch Looks good.
Attachment #8411579 -
Flags: review?(htsai) → review+
Assignee | ||
Comment 18•10 years ago
|
||
update commit summary only.
Attachment #8411584 -
Attachment is obsolete: true
Attachment #8415019 -
Flags: review+
Assignee | ||
Comment 19•10 years ago
|
||
address comment 15.
Attachment #8411585 -
Attachment is obsolete: true
Attachment #8415020 -
Flags: review+
Assignee | ||
Comment 20•10 years ago
|
||
1) Remove redundant comments pointed out in comment 15. I had two similar comments before. The first one had been removed before that patch being ever uploaded, but I missed the second one. 2) Add a few more test cases to ensure we don't miss anything and don't go wrong on some corner cases.
Attachment #8411587 -
Attachment is obsolete: true
Attachment #8415023 -
Flags: review+
Assignee | ||
Comment 21•10 years ago
|
||
https://hg.mozilla.org/integration/b2g-inbound/rev/876baa06feb9
Whiteboard: [p=3] → [p=1][leave open]
Target Milestone: 1.4 S6 (25apr) → 2.0 S1 (9may)
Assignee | ||
Comment 23•10 years ago
|
||
https://github.com/mozilla-b2g/platform_external_qemu/commit/d5800c36b2d5822fc3fe1899b9280401de466e1e https://github.com/mozilla-b2g/platform_hardware_ril/commit/ca283b9db2b151d465cfd2e19346cf58fe89e413 https://hg.mozilla.org/integration/b2g-inbound/rev/6603e7193742
Assignee | ||
Comment 24•10 years ago
|
||
http://hg.mozilla.org/mozilla-central/rev/35f9431188ca
Assignee | ||
Comment 25•10 years ago
|
||
https://hg.mozilla.org/integration/b2g-inbound/rev/6d0323d19035 https://hg.mozilla.org/integration/b2g-inbound/rev/f8468bd898d6 https://hg.mozilla.org/integration/b2g-inbound/rev/3e8b96116e13 https://hg.mozilla.org/integration/b2g-inbound/rev/faa7c2b09118
Whiteboard: [p=1][leave open] → [p=1]
https://hg.mozilla.org/mozilla-central/rev/6d0323d19035 https://hg.mozilla.org/mozilla-central/rev/f8468bd898d6 https://hg.mozilla.org/mozilla-central/rev/3e8b96116e13 https://hg.mozilla.org/mozilla-central/rev/faa7c2b09118
Status: NEW → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Assignee | ||
Comment 27•10 years ago
|
||
JellyBean: https://github.com/mozilla-b2g/platform_external_qemu/commit/ccccdcecc0ed9cff6b2445748903809ece2e04f3 https://github.com/mozilla-b2g/platform_hardware_ril/commit/dd94b2e17a146cb782d71933d25dcaa9c060e6ce KitKat: https://github.com/mozilla-b2g/platform_external_qemu/commit/6a6f02db3080566372b00108f38be5238a617ebc https://github.com/mozilla-b2g/platform_hardware_ril/commit/461d356b81d66d50f87c14bd72dc78f4fecafa47 https://hg.mozilla.org/integration/b2g-inbound/rev/0868d9030599
Assignee | ||
Comment 28•10 years ago
|
||
http://hg.mozilla.org/mozilla-central/rev/827f4dc53387
You need to log in
before you can comment on or make changes to this bug.
Description
•