845629 - [Gaia] change mcc/mnc type from integer to string.

Status: RESOLVED FIXED

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

Description

[Edgar Chen [:edgar]]

In bug 828307, we plan to change mcc/mnc type from integer to string. This change might affect Gaia, so create this bug for tracking Gaia side. Thanks.

Comment 1

[Tim Guan-tin Chien [:timdream] (please needinfo)]

We need at least verify which part of Gaia will be impacted by the change.

Comment 2

[[e-mail], [e-mail]]

Attachment: Pointer to Github pull request: https://github.com/mozilla-b2g/gaia/pull/8934

Pointer to Github pull-request

Comment 3

[[e-mail], [e-mail]]

Comment on attachment 732232 [details]
Pointer to Github pull request: https://github.com/mozilla-b2g/gaia/pull/8934

Change mcc/mnc type from integer to string.

Kaze,
Could you please help to review my pr?
I think that you are familiar with mcc/mnc.
Thank you.

Comment 4

[Fabien Cazenave [:kaze]]

I had a few nitpicks on the pull request, but my main worry is that the shared `apn.json` database still relies on integer comparison: mnc values don’t have any leading zero.

Unfortunately I don’t have any SIM card with an mnc code lesser than 10, so I can’t test this patch right now. Let me see if I can find somebody who has an Orange SIM card…

Comment 5

[Fabien Cazenave [:kaze]]

Étienne, would you test this patch with your Orange SIM card please?

 • first test: make sure the Settings > Cellular & Data > Data settings still show the usual APN list;

 • second test: reboot the phone with another SIM card (or reset the phone to clear the mcc/mnc mozSettings), insert your Orange SIM card, and check the APN list is OK.

Thanks!

Comment 6

[Etienne Segonzac (:etienne)]

(In reply to Fabien Cazenave [:kaze] from comment #5)
> Étienne, would you test this patch with your Orange SIM card please?
> 
>  • first test: make sure the Settings > Cellular & Data > Data settings
> still show the usual APN list;
> 
>  • second test: reboot the phone with another SIM card (or reset the phone
> to clear the mcc/mnc mozSettings), insert your Orange SIM card, and check
> the APN list is OK.
> 
> Thanks!

all good !

Comment 7

[Fabien Cazenave [:kaze]]

Thanks Étienne!

Ian, please address my nits so I can r+ and merge your patch.

Comment 8

[[e-mail], [e-mail]]

Kaze and Étienne,
Thanks for your review effort. I have revised the nits according your suggestion.

Comment 9

[Fabien Cazenave [:kaze]]

Comment on attachment 732232 [details]
Pointer to Github pull request: https://github.com/mozilla-b2g/gaia/pull/8934

Looks good to me.

Comment 10

[Fabien Cazenave [:kaze]]

https://github.com/mozilla-b2g/gaia/commit/96a236d8a4c47447a4a16be60c1b457d7254acf9

Comment 11

[José Antonio Olivera Ortega [:jaoo]]

Comment on attachment 732232 [details]
Pointer to Github pull request: https://github.com/mozilla-b2g/gaia/pull/8934

NOTE: Please see https://wiki.mozilla.org/Release_Management/B2G_Landing to better understand the B2G approval process and landings.

[Approval Request Comment]
Bug caused by (feature/regressing bug #): This bug needs to be uplifted because bug 860411 depends on it.
User impact if declined: See [1] (this is the gaia counterpart). Bugs 860411, 863125 and 863126. Read about those bug to see the user impact if declined.
Testing completed: Works fine in Gaia master branch.
Risk to taking this patch (and alternatives if risky): (No risky) Low in Gaia side.
String or UUID changes made by this patch: No changes.

[1]

Comment 12

[José Antonio Olivera Ortega [:jaoo]]

(In reply to José Antonio Olivera Ortega [:jaoo] from comment #11)
> Comment on attachment 732232 [details]
> Pointer to Github pull request: https://github.com/mozilla-b2g/gaia/pull/8934
> [1]

[1] https://bugzilla.mozilla.org/show_bug.cgi?id=828307#c0

Comment 13

[Andrew Overholt [:overholt]]

This blocks a blocker so should itself be a blocker.

Comment 14

[James Lal [:lightsofapollo]]

This bug was partially uplifted.

Uplifted 96a236d8a4c47447a4a16be60c1b457d7254acf9 to:
v1-train: 92e9257dcf133241e6f9542548916b4eb21a5b92

Commit 96a236d8a4c47447a4a16be60c1b457d7254acf9 didn't uplift to branch v1.0.1

Comment 15

[José Antonio Olivera Ortega [:jaoo]]

(In reply to James Lal [:lightsofapollo] from comment #14)
> Commit 96a236d8a4c47447a4a16be60c1b457d7254acf9 didn't uplift to branch
> v1.0.1

The conflict is in build/settings.py file, see:

<<<<<<< HEAD
 "operatorvariant.mcc": 0,
 "operatorvariant.mnc": 0,
=======
 "mail.sent-sound.enabled": True,
 "message.sent-sound.enabled": True,
 "operatorvariant.mcc": "0",
 "operatorvariant.mnc": "0",
>>>>>>> 96a236d... Merge pull request #8934 ...

Just keep:

 "mail.sent-sound.enabled": True,
 "message.sent-sound.enabled": True,
 "operatorvariant.mcc": "0",
 "operatorvariant.mnc": "0",

After that both bug 863125 and bug 860411 should uplift without conflicts (apply cleanly).

Comment 16

[José Antonio Olivera Ortega [:jaoo]]

James, I'll do the uplift to v1.0.1 branch once we get a response from https://bugzilla.mozilla.org/show_bug.cgi?id=828307#c50. Thanks.

Comment 17

[José Antonio Olivera Ortega [:jaoo]]

v1.0.1: f050910456cf5b801e92a770aa1c9146b53ba80e

Comment 18

[Cody Roesch [:croesch]]

Can you please provide steps to verify this fix - as we will blackbox test from the UI?

Comment 19

[Deepa Subramanian]

Can you please provide steps to verify this fix - as we can perform blackbox testing from the UI?