Closed
Bug 1010284
Opened 10 years ago
Closed 10 years ago
Support LTE cell networks in network location provider
Categories
(Core :: DOM: Geolocation, defect)
Tracking
()
RESOLVED
FIXED
mozilla35
Tracking | Status | |
---|---|---|
b2g-v1.4 | --- | unaffected |
b2g-v2.0 | --- | disabled |
b2g-v2.0M | --- | wontfix |
b2g-v2.1 | --- | wontfix |
b2g-v2.2 | --- | fixed |
People
(Reporter: hschlichting, Assigned: gerard-majax)
References
Details
(Whiteboard: [systemsfe])
Attachments
(1 file, 3 obsolete files)
1.93 KB,
patch
|
garvan
:
review+
hschlichting
:
superreview+
|
Details | Diff | Splinter Review |
Similar to https://bugzilla.mozilla.org/show_bug.cgi?id=1010278. I'm unclear on the current general LTE support of the base system. When that support is available, the code in NetworkGeolocationProvider.js needs to be updated to use the correct API's and send the correct radio type. Currently nsIMobileConnectionInfo.type already lists a possible value of "lte" in a comment. In the outgoing API call, the radio field should be set to "lte" in this case. The cellId field should send the LTE cell identity and the locationAreaCode the LTE tracking area code. It's unclear whether those will be made available via the nsIMobileCellInfo.gsmCellId / gsmLocationAreaCode API's or if that interface will be extended with new attributes for LTE.
Assignee | ||
Comment 1•10 years ago
|
||
LTE does work on Gecko now. If it helps, here is an extract of the Gecko RIL output when connected:
> I/Gecko ( 258): MobileConnectionService: notifyNetworkInfoChanged for 0: {"rilMessageType":"networkinfochanged","rilMessageClientId":0,"signal":{"voice":{"signalStrength":-118,"relSignalStrength":100},"data":{"signalStrength":-118,"relSignalStrength":100},"rilMessageType":"signalstrengthchange"},"voiceRegistrationState":{"regState":1,"state":"registered","connected":true,"roaming":false,"emergencyCallsOnly":false,"cell":{"gsmLocationAreaCode":30142,"gsmCellId":128117764},"radioTech":14,"type":"lte","rilMessageType":"voiceregistrationstatechange"},"dataRegistrationState":{"regState":1,"state":"registered","connected":true,"roaming":false,"emergencyCallsOnly":false,"cell":{"gsmLocationAreaCode":30142,"gsmCellId":128117764},"radioTech":14,"type":"lte","rilMessageType":"dataregistrationstatechange"}}
Assignee | ||
Comment 2•10 years ago
|
||
Having this mapping is needed in order to properly identify the radio technology family being used by the SIM card.
Assignee | ||
Updated•10 years ago
|
Attachment #8491587 -
Flags: review?(gkeeley)
Assignee | ||
Comment 3•10 years ago
|
||
Having this mapping is needed in order to properly identify the radio technology family being used by the connected network.
Assignee | ||
Updated•10 years ago
|
Attachment #8491587 -
Attachment is obsolete: true
Attachment #8491587 -
Flags: review?(gkeeley)
Assignee | ||
Comment 4•10 years ago
|
||
Having this mapping is needed in order to properly identify the radio technology family being used by the connected network.
Assignee | ||
Updated•10 years ago
|
Attachment #8492152 -
Attachment is obsolete: true
Assignee | ||
Comment 5•10 years ago
|
||
Comment on attachment 8492154 [details] [diff] [review] Add radio tech mapping for MLS The set of radioTech we can send is limited to gsm, cdma and wcdma.
Attachment #8492154 -
Flags: review?(gkeeley)
Comment on attachment 8492154 [details] [diff] [review] Add radio tech mapping for MLS This is much clearer, but we need a server-side review of the CDMA section.
Attachment #8492154 -
Flags: superreview?(hschlichting)
Attachment #8492154 -
Flags: review?(gkeeley)
Attachment #8492154 -
Flags: review+
Reporter | ||
Comment 7•10 years ago
|
||
(In reply to Alexandre LISSY :gerard-majax from comment #5) > Comment on attachment 8492154 [details] [diff] [review] > Add radio tech mapping for MLS > > The set of radioTech we can send is limited to gsm, cdma and wcdma. That isn't entirely true. The official Google API has no support for LTE networks yet. But the MLS API has a slight extension over it and accepts a "lte" key for the radioType field. And in fact it needs to get a "lte" key to work properly. I'd restrict this patch to only deal with the LTE radioTech extension and leave the CDMA support for its own bug (https://bugzilla.mozilla.org/show_bug.cgi?id=1010282). For CDMA it's not just the radioTech field that needs to change, but also some of the other fields.
Reporter | ||
Comment 8•10 years ago
|
||
Comment on attachment 8492154 [details] [diff] [review] Add radio tech mapping for MLS The general idea of using a switch statement is good, but some of the details are still wrong here. See my other comment on the bug itself.
Attachment #8492154 -
Flags: superreview?(hschlichting) → superreview-
Reporter | ||
Comment 9•10 years ago
|
||
I've tried to look up the state of general LTE support for FxOS but cannot find anything easily. The values in http://dxr.mozilla.org/mozilla-central/source/dom/mobileconnection/MobileCellInfo.h only mention specific values for GSM (and likely WCDMA) and CDMA but don't mention LTE. The docs at https://developer.mozilla.org/en-US/docs/Web/API/MozMobileCellInfo are even further behind. Potentially the gsmCellId / gsmLocationAreaCode functions return the "cell id" and "tracking area code" for LTE networks as well, but this is something that would need to be verified.
Assignee | ||
Comment 10•10 years ago
|
||
(In reply to Hanno Schlichting [:hannosch] from comment #9) > I've tried to look up the state of general LTE support for FxOS but cannot > find anything easily. The values in > http://dxr.mozilla.org/mozilla-central/source/dom/mobileconnection/ > MobileCellInfo.h only mention specific values for GSM (and likely WCDMA) and > CDMA but don't mention LTE. The docs at > https://developer.mozilla.org/en-US/docs/Web/API/MozMobileCellInfo are even > further behind. > > Potentially the gsmCellId / gsmLocationAreaCode functions return the "cell > id" and "tracking area code" for LTE networks as well, but this is something > that would need to be verified. We already have those values filled, as documented on comment 1. And per Android'd documentation, there is only Cdma and Gsm specific CellLocation: http://developer.android.com/reference/android/telephony/CellLocation.html
Assignee | ||
Comment 11•10 years ago
|
||
Having this mapping is needed in order to properly identify the radio technology family being used by the connected network.
Assignee | ||
Comment 12•10 years ago
|
||
Comment on attachment 8497515 [details] [diff] [review] Add radio tech mapping for MLS Adding back LTE to address comment 7.
Attachment #8497515 -
Flags: superreview?(hschlichting)
Attachment #8497515 -
Flags: review?(gkeeley)
Assignee | ||
Updated•10 years ago
|
Attachment #8492154 -
Attachment is obsolete: true
Reporter | ||
Updated•10 years ago
|
Attachment #8497515 -
Flags: superreview?(hschlichting) → superreview+
Reporter | ||
Comment 13•10 years ago
|
||
Turns out the service had a bug as well, and didn't accept the radioTech key of "lte" yet. This was fixed in https://github.com/mozilla/ichnaea/commit/e824123ea72974db5b8efd716644262e789ef71e and will be deployed to the production service next Tuesday.
Attachment #8497515 -
Flags: review?(gkeeley) → review+
Comment 14•10 years ago
|
||
Comment on attachment 8497515 [details] [diff] [review] Add radio tech mapping for MLS Review of attachment 8497515 [details] [diff] [review]: ----------------------------------------------------------------- ::: dom/system/NetworkGeolocationProvider.js @@ +442,5 @@ > + radioTechFamily = "lte"; > + break; > + // CDMA cases to be handled in bug 1010282 > + }; > + result.push({ radio: radioTechFamily, should we be logging at least if radioTechFamily is null? or falling back to a value as the code did originally?
Reporter | ||
Comment 15•10 years ago
|
||
(In reply to Garvan Keeley [:garvank] from comment #14) > Comment on attachment 8497515 [details] [diff] [review] > Add radio tech mapping for MLS > > Review of attachment 8497515 [details] [diff] [review]: > ----------------------------------------------------------------- > > ::: dom/system/NetworkGeolocationProvider.js > @@ +442,5 @@ > > + radioTechFamily = "lte"; > > + break; > > + // CDMA cases to be handled in bug 1010282 > > + }; > > + result.push({ radio: radioTechFamily, > > should we be logging at least if radioTechFamily is null? > or falling back to a value as the code did originally? A fallback value doesn't help, as you only get invalid data. The service might get confused and return you a wrong location. Instead I'd add a check before the "result.push" line and only add entries that have a known/non-null radioTechFamily value.
Comment 16•10 years ago
|
||
Alexandre: do you know if B2G 2.1 supports LTE? Should we try to uplift your LTE fix to 2.1 (Gecko 34 currently in Aurora)?
B2G has supported LTE on compatible devices (e.g. the Nexus 5) since at least 2.0.
Assignee | ||
Comment 18•10 years ago
|
||
Chris, as Kyle replied, it may even affect 2.0.
Flags: needinfo?(lissyx+mozillians)
Assignee | ||
Updated•10 years ago
|
Keywords: checkin-needed
Comment 19•10 years ago
|
||
https://hg.mozilla.org/integration/b2g-inbound/rev/4b751d735154
Assignee: nobody → lissyx+mozillians
Keywords: checkin-needed
Comment 20•10 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/4b751d735154
Status: NEW → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla35
Comment 21•10 years ago
|
||
It would be nice to uplift geolocation support for LTE to B2G 2.1 (Gecko 34), but I doubt Release Management will accept new feature work in the last week of the Aurora 34. :)
Updated•10 years ago
|
Whiteboard: [systemsfe]
Comment 22•10 years ago
|
||
Woodduck for 2.0M does not support LET.
You need to log in
before you can comment on or make changes to this bug.
Description
•