Closed
Bug 951764
Opened 11 years ago
Closed 11 years ago
[LTE] Add bearer data to APN settings
Categories
(Firefox OS Graveyard :: Gaia::Settings, defect)
Tracking
(Not tracked)
RESOLVED
FIXED
1.4 S2 (28feb)
People
(Reporter: pgravel, Assigned: gsvelto)
References
Details
(Whiteboard: [ucid:LTE1, 1.4, FT:RIL],[dependency:Marketplace])
Attachments
(3 files, 5 obsolete files)
(deleted),
patch
|
kaze
:
review+
|
Details | Diff | Splinter Review |
(deleted),
patch
|
kaze
:
review+
|
Details | Diff | Splinter Review |
(deleted),
text/x-github-pull-request
|
Details |
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•11 years ago
|
||
Can we get the exact fields or examples for bearer data? Are these commonly user editable or just provisioned at build time?
Flags: needinfo?(pgravel)
Comment 2•11 years ago
|
||
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
Updated•11 years ago
|
Blocks: 1.4-RIL/Net/Conn
Updated•11 years ago
|
Blocks: b2g-LTE-1.4
Blocks: b2g--telephony-1.4
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.
Flags: needinfo?(pgravel)
Comment 4•11 years ago
|
||
(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.
Flags: needinfo?(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.
Flags: needinfo?(pgravel)
Comment 6•11 years ago
|
||
(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•11 years ago
|
||
1.4 user story supposedly needs to be complete before Sprint3. Will communicate with team to reconfirm target milestone.
blocking-b2g: 1.4? → backlog
Whiteboard: [ucid:LTE1, 1.4, FT:RIL]
Target Milestone: --- → 1.4 S4 (28mar)
Updated•11 years ago
|
Target Milestone: 1.4 S4 (28mar) → 1.4 S3 (14mar)
Updated•11 years ago
|
No longer blocks: 1.4-RIL/Net/Conn
Comment 10•11 years ago
|
||
(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?
Flags: needinfo?(pgravel)
Updated•11 years ago
|
Flags: in-testsuite?
Flags: in-moztrap?(echu)
Comment 11•11 years ago
|
||
We should also add 2 items, protocol and roaming protocol, in APN setting menu.
Summary: [LTE] Add bearer data to APN settings → [LTE] Add bearer data, protocol, and roaming protocol to APN settings
Comment 12•11 years ago
|
||
Anshul, can you elaborate more details about bearer data? If we don't handle this, what will be happened.
Flags: needinfo?(anshulj)
Comment 13•11 years ago
|
||
Per an offline discussion, Jose will help on this. Thank you so much, Jose!
Assignee: nobody → josea.olivera
Flags: needinfo?(arthur.chen)
Reporter | ||
Comment 14•11 years ago
|
||
(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.
Flags: needinfo?(pgravel)
Comment 15•11 years ago
|
||
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•11 years ago
|
||
Feature work and user story bug should not have 1.4+ flag.
blocking-b2g: 1.4+ → backlog
Reporter | ||
Comment 17•11 years ago
|
||
(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.
Flags: needinfo?(pgravel)
Updated•11 years ago
|
blocking-b2g: backlog → 1.4+
Updated•11 years ago
|
Target Milestone: 1.4 S3 (14mar) → 1.4 S2 (28feb)
Comment 19•11 years ago
|
||
To have protocol and roaming protocol don't block this bug. Removing 969305 from this bug.
No longer depends on: 969305
Summary: [LTE] Add bearer data, protocol, and roaming protocol to APN settings → [LTE] Add bearer data to APN settings
Comment 20•11 years ago
|
||
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•11 years ago
|
||
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.
Flags: needinfo?(anshulj)
Comment 22•11 years ago
|
||
http://mzl.la/1hrAUB8
Flags: in-moztrap?(echang) → in-moztrap+
QA Contact: echang
No longer blocks: b2g--telephony-1.4
Reporter | ||
Comment 23•11 years ago
|
||
(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.
Flags: needinfo?(pgravel)
Comment 26•11 years ago
|
||
I wont be able to finish this bug for 2/28. Leaving the bug so anyone else could take and work on it.
Assignee: josea.olivera → nobody
Flags: needinfo?(josea.olivera)
Comment 27•11 years ago
|
||
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.
Flags: needinfo?(dscravaglieri)
Assignee | ||
Comment 28•11 years ago
|
||
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•11 years ago
|
||
(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.
Assignee | ||
Comment 30•11 years ago
|
||
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.
Assignee: nobody → gsvelto
Status: NEW → ASSIGNED
Attachment #8383070 -
Flags: feedback?(josea.olivera)
Assignee | ||
Comment 31•11 years ago
|
||
Wrong patch, here's the correct one.
Attachment #8383070 -
Attachment is obsolete: true
Attachment #8383070 -
Flags: feedback?(josea.olivera)
Attachment #8383077 -
Flags: feedback?(josea.olivera)
Comment 32•11 years ago
|
||
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.
Attachment #8383077 -
Flags: feedback?(josea.olivera) → feedback+
Assignee | ||
Comment 33•11 years ago
|
||
Same patch as before but with APN filtering moved to a shared helper.
Attachment #8383077 -
Attachment is obsolete: true
Assignee | ||
Comment 34•11 years ago
|
||
This patch makes the settings app use the network type / bearer filtering.
Comment 35•11 years ago
|
||
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•11 years ago
|
||
(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 38•11 years ago
|
||
(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.
Flags: needinfo?(jjong)
Assignee | ||
Comment 39•11 years ago
|
||
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.
Attachment #8383147 -
Flags: review?(josea.olivera)
Assignee | ||
Updated•11 years ago
|
Attachment #8383148 -
Flags: review?(kaze)
Comment 40•11 years ago
|
||
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.
Attachment #8383147 -
Flags: review?(josea.olivera) → review?(kaze)
Comment 41•11 years ago
|
||
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.
Assignee | ||
Comment 42•11 years ago
|
||
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.
Attachment #8383147 -
Attachment is obsolete: true
Attachment #8383147 -
Flags: review?(kaze)
Attachment #8383552 -
Flags: review?(kaze)
Assignee | ||
Comment 43•11 years ago
|
||
New patch, now complete with unit-tests.
Attachment #8383552 -
Attachment is obsolete: true
Attachment #8383552 -
Flags: review?(kaze)
Attachment #8383715 -
Flags: review?(kaze)
Assignee | ||
Comment 44•11 years ago
|
||
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.
Attachment #8383148 -
Attachment is obsolete: true
Attachment #8383148 -
Flags: review?(kaze)
Attachment #8383718 -
Flags: review?(kaze)
Assignee | ||
Comment 45•11 years ago
|
||
Comment 46•11 years ago
|
||
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.
Attachment #8383718 -
Flags: review?(kaze) → review+
Comment 47•11 years ago
|
||
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
Attachment #8383715 -
Flags: review?(kaze) → review+
Assignee | ||
Comment 48•11 years ago
|
||
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
Status: ASSIGNED → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Reporter | ||
Comment 49•11 years ago
|
||
(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.
Flags: needinfo?(pgravel)
Updated•11 years ago
|
Whiteboard: [ucid:LTE1, 1.4, FT:RIL] → [ucid:LTE1, 1.4, FT:RIL],[dependency:Marketplace]
You need to log in
before you can comment on or make changes to this bug.
Description
•