Closed Bug 1010284 Opened 6 years ago Closed 5 years ago

Support LTE cell networks in network location provider

Categories

(Core :: DOM: Geolocation, defect)

ARM
Gonk (Firefox OS)
defect
Not set

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)

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.
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"}}
Having this mapping is needed in order to properly identify the radio
technology family being used by the SIM card.
Attachment #8491587 - Flags: review?(gkeeley)
Attached patch Add radio tech mapping for MLS (obsolete) — Splinter Review
Having this mapping is needed in order to properly identify the radio
technology family being used by the connected network.
Attachment #8491587 - Attachment is obsolete: true
Attachment #8491587 - Flags: review?(gkeeley)
Attached patch Add radio tech mapping for MLS (obsolete) — Splinter Review
Having this mapping is needed in order to properly identify the radio
technology family being used by the connected network.
Attachment #8492152 - Attachment is obsolete: true
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+
(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.
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-
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.
(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
Having this mapping is needed in order to properly identify the radio
technology family being used by the connected network.
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)
Attachment #8492154 - Attachment is obsolete: true
Attachment #8497515 - Flags: superreview?(hschlichting) → superreview+
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 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?
(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.
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)?
status-b2g-v2.1: --- → ?
Flags: needinfo?(lissyx+mozillians)
B2G has supported LTE on compatible devices (e.g. the Nexus 5) since at least 2.0.
Chris, as Kyle replied, it may even affect 2.0.
Flags: needinfo?(lissyx+mozillians)
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/4b751d735154
Status: NEW → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla35
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. :)
Whiteboard: [systemsfe]
Woodduck for 2.0M does not support LET.
You need to log in before you can comment on or make changes to this bug.