Closed
Bug 1074092
Opened 10 years ago
Closed 10 years ago
[CBS] Move CBS Settings implementation from RadioInterfaceLayer to CellBroadcastService.
Categories
(Firefox OS Graveyard :: RIL, defect)
Tracking
(Not tracked)
RESOLVED
FIXED
2.1 S6 (10oct)
People
(Reporter: bevis, Assigned: bevis)
References
Details
Attachments
(1 file, 1 obsolete file)
No description provided.
Assignee | ||
Comment 1•10 years ago
|
||
This patch is to move the CBS Settings related implementation from RadioInterfaceLayer to CellBroadcastService.js. Hi Hsinyi, May I have your review for this change? Thanks!
Attachment #8500898 -
Flags: review?(htsai)
Comment 2•10 years ago
|
||
Comment on attachment 8500898 [details] [diff] [review] Patch v1: Move CBS Settings implementation from RadioInterfaceLayer to CellBroadcastService. Review of attachment 8500898 [details] [diff] [review]: ----------------------------------------------------------------- Looks good to me, thank you. ::: dom/cellbroadcast/gonk/CellBroadcastService.js @@ +134,5 @@ > + return Array.isArray(aSettings) ? aSettings[aClientId] : aSettings; > + }, > + > + /** > + * Helper function to set CellBroadcastDisabled to Each RadioInterface. nit: s/Each/each/ @@ +148,5 @@ > + } > + }, > + > + /** > + * Helper function to set CellBroadcastSearchList to Each RadioInterface. ditto. @@ +154,5 @@ > + setCellBroadcastSearchList: function(aSettings) { > + let numOfRilClients = gRadioInterfaceLayer.numRadioInterfaces; > + let responses = []; > + for (let clientId = 0; clientId < numOfRilClients; clientId++) { > + let newSearchList = this._getSettingValueByClientId(clientId, aSettings); nit: I'd suggest the name "_retrieveSettingValueByClient" because "_get" confuses me a little. I can't stop thinking that we already know the settings, why do we need "_get" :P @@ +160,5 @@ > + this._cellBroadcastSearchList); > + > + if ((newSearchList == oldSearchList) || > + (newSearchList && oldSearchList && > + newSearchList.gsm == oldSearchList.gsm && nit: align line #163, 164 and 165 @@ +167,5 @@ > + } > + > + gRadioInterfaceLayer > + .getRadioInterface(clientId).sendWorkerMessage("setCellBroadcastSearchList", > + { searchList: newSearchList }, nit: indention @@ +298,5 @@ > + switch (aName) { > + case kSettingsCellBroadcastSearchList: > + if (DEBUG) { > + debug("'" + kSettingsCellBroadcastSearchList + > + "' is now " + JSON.stringify(aResult)); nit: indention. @@ +306,5 @@ > + break; > + case kSettingsCellBroadcastDisabled: > + if (DEBUG) { > + debug("'" + kSettingsCellBroadcastDisabled + > + "' is now " + JSON.stringify(aResult)); ditto.
Attachment #8500898 -
Flags: review?(htsai) → review+
Assignee | ||
Comment 3•10 years ago
|
||
(In reply to Hsin-Yi Tsai [:hsinyi] from comment #2) > Comment on attachment 8500898 [details] [diff] [review] > Patch v1: Move CBS Settings implementation from RadioInterfaceLayer to > CellBroadcastService. Thanks, I'll address these nits in next update.
Assignee | ||
Comment 4•10 years ago
|
||
Address nits in comment 2.
Attachment #8500898 -
Attachment is obsolete: true
Attachment #8502276 -
Flags: review+
Assignee | ||
Comment 5•10 years ago
|
||
update try server result: https://tbpl.mozilla.org/?tree=Try&rev=0afd5167a573
Keywords: checkin-needed
Comment 6•10 years ago
|
||
https://hg.mozilla.org/integration/b2g-inbound/rev/1a18c0aba319
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/1a18c0aba319
Status: NEW → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Target Milestone: --- → 2.1 S6 (10oct)
You need to log in
before you can comment on or make changes to this bug.
Description
•