951764 - [LTE] Add bearer data to APN settings

Status: RESOLVED FIXED

(Firefox OS Graveyard :: Gaia::Settings, defect)

Description

[pgravel]

To support LTE, gaia will need to update the APN database to also include LTE bearer data and expose these values in the APN settings.

Comment 1

[Bruce Huang [:bhuang] (inactive)]

Can we get the exact fields or examples for bearer data?  Are these commonly user editable or just provisioned at build time?

Comment 2

[José Antonio Olivera Ortega [:jaoo]]

The APN database we use in FxOS is the same used in AOSP (see [1]). Doesn't it include LTE bearer data? If it includes them we just need to sync up with the latest version of the AOSP database.

[1]https://android.googlesource.com/device/sample/+/master/etc/apns-full-conf.xml

Comment 3

[pgravel]

The AOSP database does contains the LTE bearer data, so a sync-up is probably all that is required on the database side, but there will likely be some work involved to ensure the data properly propagates to ril.data.apnSettings and also allow users to specify bearer with custom apn entry.

Comment 4

[José Antonio Olivera Ortega [:jaoo]]

(In reply to pgravel from comment #3)
> The AOSP database does contains the LTE bearer data, so a sync-up is
> probably all that is required on the database side, but there will likely be
> some work involved to ensure the data properly propagates to
> ril.data.apnSettings and also allow users to specify bearer with custom apn
> entry.

We sync up the AOSP dabatase we use to build our apn.json database a few days ago (bug 947946). Not sure if we need to hack a bit to ensure the data properly propagates to ril.data.apnSettings and also allow users to specify bearer with custom apn entry. Could you provide an example of a carrier (its APNs) that already contains LTE bearer data please? Thanks.

Comment 5

[pgravel]

The best example is Verizon, but it's in a different xml file (same directory as the apn-full-conf.xml)
https://android.googlesource.com/device/sample/+/master/etc/apns-conf_verizon.xml (line 92)

From the full-conf.xml file, you can probably refer to the Sprint APNs. Although they are not LTE, they all specify the data bearer.

Comment 6

[José Antonio Olivera Ortega [:jaoo]]

(In reply to pgravel from comment #5)
> The best example is Verizon, but it's in a different xml file (same
> directory as the apn-full-conf.xml)
> https://android.googlesource.com/device/sample/+/master/etc/apns-
> conf_verizon.xml (line 92)

Would be ok to add the apns-conf_verizon.xml database to our apn.json database?

Comment 7

[Wesley Huang [:wesley_huang] (EPM) (NI me)]

1.4 user story supposedly needs to be complete before Sprint3.
Will communicate with team to reconfirm target milestone.

Comment 8

[Ken Chang[:kenkai]]

Arthur, would you like to take this bug?

Comment 9

[Ken Chang[:kenkai]]

This is a 1.4+ bug.

Comment 10

[José Antonio Olivera Ortega [:jaoo]]

(In reply to José Antonio Olivera Ortega [:jaoo] from comment #6)
> (In reply to pgravel from comment #5)
> > The best example is Verizon, but it's in a different xml file (same
> > directory as the apn-full-conf.xml)
> > https://android.googlesource.com/device/sample/+/master/etc/apns-
> > conf_verizon.xml (line 92)
> 
> Would be ok to add the apns-conf_verizon.xml database to our apn.json
> database?

Comment 11

[Ken Chang[:kenkai]]

We should also add 2 items, protocol and roaming protocol, in APN setting menu.

Comment 12

[Ken Chang[:kenkai]]

Anshul, can you elaborate more details about bearer data? If we don't handle this, what will be happened.

Comment 13

[Arthur Chen [:arthurcc][inactive after 6/30]]

Per an offline discussion, Jose will help on this. Thank you so much, Jose!

Comment 14

[pgravel]

(In reply to José Antonio Olivera Ortega [:jaoo] from comment #10)
> (In reply to José Antonio Olivera Ortega [:jaoo] from comment #6)
> > (In reply to pgravel from comment #5)
> > > The best example is Verizon, but it's in a different xml file (same
> > > directory as the apn-full-conf.xml)
> > > https://android.googlesource.com/device/sample/+/master/etc/apns-
> > > conf_verizon.xml (line 92)
> > 
> > Would be ok to add the apns-conf_verizon.xml database to our apn.json
> > database?

As far as i know, yes it should be ok. Those APNs are necessary data to work on a Verizon network, so they must be included somewhere.

Comment 15

[Ken Chang[:kenkai]]

pgravel, could you please also check the following comment.
https://bugzilla.mozilla.org/show_bug.cgi?id=969305#c3
It seems unnecessary to have bearer type in APN setting.

Comment 16

[Wesley Huang [:wesley_huang] (EPM) (NI me)]

Feature work and user story bug should not have 1.4+ flag.

Comment 17

[pgravel]

(In reply to Ken Chang[:ken] from comment #15)
> pgravel, could you please also check the following comment.
> https://bugzilla.mozilla.org/show_bug.cgi?id=969305#c3
> It seems unnecessary to have bearer type in APN setting.

If the APN selection is done purely in gaia and properly handles switching from LTE to CDMA and back, then it is not needed in the apnSettings.

Comment 18

[Ken Chang[:kenkai]]

It is not 1.4+.

Comment 19

[Ken Chang[:kenkai]]

To have protocol and roaming protocol don't block this bug. Removing 969305 from this bug.

Comment 20

[José Antonio Olivera Ortega [:jaoo]]

Well, let me guys define the scope of this bug and what we are going to do here. We are going to add some APNs to the apn.json database. We won't touch the APN panels in the setting app at all. We will take care of the UI changes in the APN panels in bug 975259 in which we will add protocol and roaming protocol fields in the APN form.

Comment 21

[Ken Chang[:kenkai]]

I think the scope of this bug is supposed to get the right APN setting from database(apn.json) for current radio technology(bearer). That means we also need to add bearer information in apn.json. 

Anshul, if it isn't what you want, please correct me.

Comment 22

[Eric Chang [:ericcc] [:echang]]

http://mzl.la/1hrAUB8

Comment 23

[pgravel]

(In reply to Ken Chang[:ken] from comment #21)
> I think the scope of this bug is supposed to get the right APN setting from
> database(apn.json) for current radio technology(bearer). That means we also
> need to add bearer information in apn.json. 
> 
> Anshul, if it isn't what you want, please correct me.

Correct.

Comment 24

[Kai-Chih Hu [:khu]]

Are you working on this bug? Thanks.

Comment 25

[Wesley Huang [:wesley_huang] (EPM) (NI me)]

Is this under your radar?

Comment 26

[José Antonio Olivera Ortega [:jaoo]]

I wont be able to finish this bug for 2/28. Leaving the bug so anyone else could take and work on it.

Comment 27

[Anthony Ricaud (:rik)]

Wesley: David is at MWC this week so answering for him. Nope, it's not part of David's team. I also don't know who could work on that.

Comment 28

[Gabriele Svelto [:gsvelto] (PTO)]

I'm done with my bugs so I could have a look at this but I might need more information; I'll report here as soon as I'm done figuring out what's needed here.

Comment 29

[Kai-Chih Hu [:khu]]

(In reply to Gabriele Svelto [:gsvelto] from comment #28)
> I'm done with my bugs so I could have a look at this but I might need more
> information; I'll report here as soon as I'm done figuring out what's needed
> here.

Thank you, Gabriele.

Comment 30

[Gabriele Svelto [:gsvelto] (PTO)]

Attachment: [PATCH] Filter APNs by bearer/network type

OK, let's see if we can pull this off. This changes the filtering logic for compatible APNs to match the bearer to the network type.

We translate the network type string into an integer which is equivalent to the bearer numbers in the equivalent Android logic (the bearer numbers in our APN database are identical to those for Android). Then if the bearer value is present and != 0 we compare it to the translated network type value. We assume that a bearer value of 0 or no bearer value at all implies that the APN is compatible with all networks; this also behaves in the same way as the corresponding logic in Android.

Comment 31

[Gabriele Svelto [:gsvelto] (PTO)]

Attachment: [PATCH WIP] Filter APNs by bearer/network type

Wrong patch, here's the correct one.

Comment 32

[José Antonio Olivera Ortega [:jaoo]]

Comment on attachment 8383077 [details] [diff] [review]
[PATCH WIP] Filter APNs by bearer/network type

LGTM. Thanks for the quick response on having a WIP patch really soon.

Comment 33

[Gabriele Svelto [:gsvelto] (PTO)]

Attachment: [PATCH 1/2 WIP] Filter APNs by bearer/network type

Same patch as before but with APN filtering moved to a shared helper.

Comment 34

[Gabriele Svelto [:gsvelto] (PTO)]

Attachment: [PATCH 2/2 WIP] Add network type filtering to the APN selection

This patch makes the settings app use the network type / bearer filtering.

Comment 35

[Arthur Chen [:arthurcc][inactive after 6/30]]

If I understand this correctly, we should also respond to the network technology changes and switch to the corresponding APN setting. But if users specify a custom APN setting manually, we don't change the setting.

Comment 36

[José Antonio Olivera Ortega [:jaoo]]

(In reply to Arthur Chen [:arthurcc] from comment #35)
> If I understand this correctly, we should also respond to the network
> technology changes and switch to the corresponding APN setting. But if users
> specify a custom APN setting manually, we don't change the setting.
To listen for network technology changes and do whatever is simple but I am not sure whether we should chante the APN on the fly. To be honest I do not know how to do the right thing. It would be great some feedback from Jessica or pgravel.

Comment 37

[Anthony Ricaud (:rik)]

Needinfo for comment 36.

Comment 38

[Jessica Jong [:jessica] (inactive)]

(In reply to Anthony Ricaud (:rik) from comment #37)
> Needinfo for comment 36.

Sorry, I am not sure about this either. Let's wait for pgravel's feedback.

Comment 39

[Gabriele Svelto [:gsvelto] (PTO)]

Comment on attachment 8383147 [details] [diff] [review]
[PATCH 1/2 WIP] Filter APNs by bearer/network type

OK, let's have these patches reviewed, I'll prepare another patch with unit-tests in the meantime.

Comment 40

[José Antonio Olivera Ortega [:jaoo]]

Comment on attachment 8383147 [details] [diff] [review]
[PATCH 1/2 WIP] Filter APNs by bearer/network type

LGTM, unfortunately I won't be able to review this properly. I much prefer to forward this request to Fabien. Sorry for the inconveniences but I have a really important matter to attend and would not like to review this in depth.

Comment 41

[José Antonio Olivera Ortega [:jaoo]]

Comment on attachment 8383148 [details] [diff] [review]
[PATCH 2/2 WIP] Add network type filtering to the APN selection

If it helps this patch looks good to me too. Just a comment. As the client provisioning APNs do not have yet the bearer property it would not be neccesary to filter them. I will file a follow-up to figure out if the provisining document provides the bearer property and if we should parse it. If so we would need to filter the provisioning APN too. Either way as the filter function doesn't brake anything is up to you guys to keep it.

Comment 42

[Gabriele Svelto [:gsvelto] (PTO)]

Attachment: [PATCH 1/2 v2] Filter APNs by bearer/network type

While writing the tests I've immediately stumbled upon an issue in my patch so here's the fix. Fabien, I'm not sure who'd be more indicated to review this patch as all of the code in this file has been written by José; I'm leaving the review to you since most of the code will be shared with the Settings app.

Comment 43

[Gabriele Svelto [:gsvelto] (PTO)]

Attachment: [PATCH 1/2 v3] Filter APNs by bearer/network type

New patch, now complete with unit-tests.

Comment 44

[Gabriele Svelto [:gsvelto] (PTO)]

Attachment: [PATCH 2/2 v2] Add network type filtering to the APN selection

Same as before but properly rebased; no unit-tests in this one as there are no existing ones from what I could gather and besides the change here is almost mechanical so it shouldn't be a problem IMHO.

Comment 45

[Gabriele Svelto [:gsvelto] (PTO)]

Attachment: [PULL REQUEST] Add bearer data to APN settings

Comment 46

[Fabien Cazenave [:kaze]]

Comment on attachment 8383718 [details] [diff] [review]
[PATCH 2/2 v2] Add network type filtering to the APN selection

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

LGTM

::: apps/settings/js/carrier.js
@@ +823,5 @@
>     * messages.
>     *
>     * @param {Function} callback Function to be called once the work is done.
>     * @param {String} usage The usage for the APNs in the panel.
> +   * @param {String} type The network type which the APN must be compatible with

Nit: trailing point.

Comment 47

[Fabien Cazenave [:kaze]]

Comment on attachment 8383715 [details] [diff] [review]
[PATCH 1/2 v3] Filter APNs by bearer/network type

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

LGTM

::: apps/system/test/unit/operator_variant_test.js
@@ +232,5 @@
> +    MockNavigatorMozMobileConnections[0].data.type = 'gsm';
> +    ovh.retrieveOperatorVariantSettings(function(list) {
> +      assert.equal(list.length, 2);
> +      assert.isTrue(list.some(function(element) {
> +        return (element.carrier ==='NoBearer');

nit, missing space after ===

@@ +235,5 @@
> +      assert.isTrue(list.some(function(element) {
> +        return (element.carrier ==='NoBearer');
> +      }));
> +      assert.isTrue(list.some(function(element) {
> +        return (element.carrier ==='ZeroBearer');

same here

@@ +242,5 @@
> +      MockNavigatorMozMobileConnections[0].data.type = 'evdo0';
> +        ovh.retrieveOperatorVariantSettings(function(list) {
> +        assert.equal(list.length, 3);
> +        assert.isTrue(list.some(function(element) {
> +          return (element.carrier ==='NoBearer');

same here

@@ +245,5 @@
> +        assert.isTrue(list.some(function(element) {
> +          return (element.carrier ==='NoBearer');
> +        }));
> +        assert.isTrue(list.some(function(element) {
> +          return (element.carrier ==='ZeroBearer');

same here

@@ +248,5 @@
> +        assert.isTrue(list.some(function(element) {
> +          return (element.carrier ==='ZeroBearer');
> +        }));
> +        assert.isTrue(list.some(function(element) {
> +          return (element.carrier ==='Evdo0Bearer');

same here

Comment 48

[Gabriele Svelto [:gsvelto] (PTO)]

Thanks for the review! I've fixed all the nits and updated the PR, Travis was green:

https://travis-ci.org/mozilla-b2g/gaia/builds/19813215

... so merged to gaia/master b151d0fb07067a3b43ab253d000852863d70c597

Comment 49

[pgravel]

(In reply to Jessica Jong [:jjong] [:jessica] from comment #38)
> (In reply to Anthony Ricaud (:rik) from comment #37)
> > Needinfo for comment 36.
> 
> Sorry, I am not sure about this either. Let's wait for pgravel's feedback.

Changing APNs on the fly will be necessary in some cases.
In general when switching from CDMA to LTE the data call will not be automatic transferred. The data call will get disconnected and will need to be reconnected. At this time the mcc/mnc values will change and we'll also need the new APN as it will be different.
I said "in general" because for some CDMA radio technology (specifically EHRPD) the hand off will occur automatically and the data call will remain up. In this case we do not need new APNs, as the mcc/mnc will remain constant and the APN for LTE and EHRPD happen to be the same.