891725 - [User Story] Cell Broadcast Settings Runtime Customization by SIM

Status: RESOLVED FIXED

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

Description

[Peter Dolanjski [:pdol]]

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.

Comment 1

[Peter Dolanjski [:pdol]]

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.

Comment 2

[José Antonio Olivera Ortega [:jaoo]]

There's already some support for this feature. See bug 802121 please.

Comment 3

[Peter Dolanjski [:pdol]]

(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.

Comment 4

[Candice Serran (:cserran)]

Kyle Machulis changed story state to started in Pivotal Tracker

Comment 5

[Candice Serran (:cserran)]

Kyle Machulis changed story state to unstarted in Pivotal Tracker

Comment 6

[Candice Serran (:cserran)]

Kyle Machulis changed story state to started in Pivotal Tracker

Comment 7

[Kyle Machulis [:qdot] [:kmachulis] (INACTIVE)]

Attachment: Patch 1 (v1) - Unit tests for cell broadcast channel and voicemail customization

Work is already done via bug 898409. Just needed to write unit tests to confirm.

Comment 8

[Fabien Cazenave [:kaze]]

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.

Comment 9

[José Antonio Olivera Ortega [:jaoo]]

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.

Comment 10

[Aus Lacroix [:aus]]

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?

Comment 11

[José Antonio Olivera Ortega [:jaoo]]

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.

Comment 12

[Kyle Machulis [:qdot] [:kmachulis] (INACTIVE)]

Ok, I see what's up. So if we kill our listener after applySettings() is called once, would that fix this?

Comment 13

[Kyle Machulis [:qdot] [:kmachulis] (INACTIVE)]

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?

Comment 14

[Aus Lacroix [:aus]]

Kyle, that'll work fine.

Comment 15

[Kyle Machulis [:qdot] [:kmachulis] (INACTIVE)]

https://github.com/mozilla-b2g/gaia/pull/12121

Comment 16

[Candice Serran (:cserran)]

Kyle Machulis changed story state to accepted in Pivotal Tracker

Comment 17

[Candice Serran (:cserran)]

Kyle Machulis changed story state to accepted in Pivotal Tracker

Comment 18

[Candice Serran (:cserran)]

Kyle Machulis changed story state to accepted in Pivotal Tracker

Comment 19

[Jason Smith [:jsmith]]

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?

Comment 20

[Kyle Machulis [:qdot] [:kmachulis] (INACTIVE)]

Test data was checked in with the unit tests as part of apn changes. See github commit.

Comment 21

[Jason Smith [:jsmith]]

https://moztrap.mozilla.org/manage/cases/?pagenumber=1&pagesize=20&sortfield=created_on&sortdirection=desc&filter-tag=2266