Closed Bug 891725 Opened 7 years ago Closed 7 years ago

[User Story] Cell Broadcast Settings Runtime Customization by SIM

Categories

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

ARM
Gonk (Firefox OS)
defect
Not set

Tracking

(blocking-b2g:koi+)

RESOLVED FIXED
blocking-b2g koi+

People

(Reporter: pdol, Assigned: qdot)

References

Details

(Keywords: feature, verifyme, Whiteboard: [ucid:System27, FT:systems-fe, KOI:P1][systemsfe])

Attachments

(1 file)

User Story:

As an OEM, I want to be able to specify which cell broadcast channels should be used on the device as well as whether or not cell broadcast should be on or off by default based on the MNC/MCC setting from the SIM card inserted during the First Run Experience in order to target customizations to locales without needing to use separate builds.


Acceptance Criteria:

1. If certain cell broadcast settings (including channels to be used and whether cell broadcast is on or off by default) are specified to be included for an MNC/MCC combination, and a SIM card with that MNC/MCC combination is in the device during the First Run Experience, the cell broadcast settings used by the device are those specified by the cell broadcast settings.
2. If no SIM card is inserted during the First Run Experience, the OEM specified default cell broadcast settings are used by the device.
3. If no SIM card is inserted during the First Run Experience, and no default cell broadcast settings are specified, no cell broadcast channels will be used by the device and the on/off setting will be set to off.
Note that the ability for the user to turn CB on/off landed in leo: bug 874796.  Also note that it applies to ALL channels at this point.
blocking-b2g: --- → koi?
There's already some support for this feature. See bug 802121 please.
(In reply to José Antonio Olivera Ortega [:jaoo] from comment #2)
> There's already some support for this feature. See bug 802121 please.

Hi José, you're right that much of this already exists.  This user story is ensure inclusion of unit tests and the full implementation including defaulting to on/off based on what is specified by MNC/MCC pairs.

The original story has been revised after further discussion.   
It is now:

As an OEM, I want to be able to specify which cell broadcast channels should be used on the device as well as whether or not cell broadcast should be on or off by default based on the MNC/MCC setting from the SIM card inserted in order to target customizations to locales without needing to use separate builds.

Acceptance Criteria:

1. If a certain cell broadcast settings (including channels to be used and whether cell broadcast is on or off by default) are specified to be included for an MNC/MCC combination, and a SIM card with that MNC/MCC combination is inserted into the device, the cell broadcast settings used by the device are those specified by the cell broadcast settings.
2. If no SIM card is inserted, the specified default cell broadcast settings are used by the device.
Whiteboard: [ucid:System27] → [ucid:System27, FT:systems-fe, KOI:P1]
Flags: in-moztrap?
Assignee: nobody → kyle
Kyle Machulis changed story state to started in Pivotal Tracker
Kyle Machulis changed story state to unstarted in Pivotal Tracker
Flags: in-moztrap? → in-moztrap?(jhammink)
Kyle Machulis changed story state to started in Pivotal Tracker
Work is already done via bug 898409. Just needed to write unit tests to confirm.
Attachment #803007 - Flags: review?(kaze)
Whiteboard: [ucid:System27, FT:systems-fe, KOI:P1] → [ucid:System27, FT:systems-fe, KOI:P1][systemsfe]
Comment on attachment 803007 [details] [diff] [review]
Patch 1 (v1) - Unit tests for cell broadcast channel and voicemail customization

LGTM but forwarding to Jose, who knows the code much better.
Attachment #803007 - Flags: review?(kaze) → review?(josea.olivera)
Comment on attachment 803007 [details] [diff] [review]
Patch 1 (v1) - Unit tests for cell broadcast channel and voicemail customization

Since the pointer of the PR I've taking a look at is an attachment from this bug I leave my comments here.

New code and changes look good to me and also test were needed (thanks for adding them). Awesome!

Travis failed but the failure is not related to the work done here. I ran the test locally and they pass. I've checked everything is fine, the device work as expected.

Sadly I cannot give r=me because cannot support hot swapping of SIM card due an IOT blocker. See bug 860411 please.
Attachment #803007 - Flags: review?(josea.olivera)
José, it isn't expected that hot swapping will work because of the changes in this patch. I think it's possible that some of the comments in the code itself are causing confusion. 

The OperatorVariantHelper in theory will support hot swapping of SIM cards and updating the settings because of how it listens for icc info changes. However, it's not expected to work now as the underlying infrastructure doesn't support hot swapping of SIM cards.

Since the patch does work as it should, could we go ahead and r+ this and land it?
Attachment #803007 - Flags: review?(josea.olivera)
Comment on attachment 803007 [details] [diff] [review]
Patch 1 (v1) - Unit tests for cell broadcast channel and voicemail customization

(In reply to Ghislain 'Aus' Lacroix from comment #10)
> José, it isn't expected that hot swapping will work because of the changes
> in this patch. I think it's possible that some of the comments in the code
> itself are causing confusion. 

It's not because of the comments. I left a comment on the PR. If I'm not wrong you call `listen()` and that will cause the `applySettings()` function to be added as event listener of `iccinfochange` events. Since you don't persist that the customizations have been applied the `applySettings()` function will be called again.
Attachment #803007 - Flags: review?(josea.olivera)
Ok, I see what's up. So if we kill our listener after applySettings() is called once, would that fix this?
Update pull request. Added a this.listen(false) call in the same place your removal of the event listener was. I think this should clear it up? Does this also need a test to make sure if we try to customize twice with two different values, only the first is taken?
Kyle, that'll work fine.
Attachment #803007 - Flags: review?(josea.olivera)
Attachment #803007 - Flags: review?(josea.olivera) → review+
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Kyle Machulis changed story state to accepted in Pivotal Tracker
Kyle Machulis changed story state to accepted in Pivotal Tracker
Kyle Machulis changed story state to accepted in Pivotal Tracker
Keywords: verifyme
blocking-b2g: koi? → koi+
Flags: in-moztrap?(jhammink) → in-moztrap?
Kyle - I'm trying to work updating the reference customization for testing. Could provide the relevant json file (name of it) demonstrating a sample customization for cell broadcast according to this implementation?
Flags: needinfo?(kyle)
Test data was checked in with the unit tests as part of apn changes. See github commit.
Flags: needinfo?(kyle)
Duplicate of this bug: 845366
You need to log in before you can comment on or make changes to this bug.