Closed
Bug 928851
Opened 11 years ago
Closed 11 years ago
[Gaia][DSDS][Gaia::Settings] stop using the settings key 'ril.radio.disabled' to turn off RIL radio
Categories
(Firefox OS Graveyard :: Gaia::Settings, defect)
Tracking
(blocking-b2g:1.3+)
People
(Reporter: hsinyi, Assigned: arthurcc)
References
Details
(Whiteboard: [FT:RIL])
Attachments
(1 file)
We've been through several problems with using settings key to control hardware. In Bug 856553, we are going to have a new API for radio control and deprecate the settings key 'ril.radio.disabled.'
This bug is for handling backward compatibility and new API detection in gaia so that the landing of bug 856553 won't break gaia.
Assignee | ||
Updated•11 years ago
|
Assignee: nobody → arthur.chen
Comment 1•11 years ago
|
||
Hi Hsin-Yi,
Does Gecko stabilize API and have a quick use case like https://bugzilla.mozilla.org/show_bug.cgi?id=927724 ?
If yes, can you add some use cases here ? If no, you can just comment back later when finished.
Thanks for your great help :D
Flags: needinfo?(htsai)
Reporter | ||
Comment 2•11 years ago
|
||
The basic picture looks promising though we are still refining the API in bug 856553. I'll keep you updated once the new API is review granted. Thank you.
However, the main concept is:
afterwards, gaia would need to call
var req = navigator.mozMobileConnection.setRadioPower(true);
to turn on radio.
One more readonly attribute 'radioState' is exposed to MobileConnection. 'radiostatechange' will be fired when radio state changes.
Flags: needinfo?(htsai)
Comment 3•11 years ago
|
||
(In reply to Hsin-Yi Tsai [:hsinyi] from comment #2)
> The basic picture looks promising though we are still refining the API in
> bug 856553. I'll keep you updated once the new API is review granted. Thank
> you.
>
> However, the main concept is:
> afterwards, gaia would need to call
> var req = navigator.mozMobileConnection.setRadioPower(true);
> to turn on radio.
>
> One more readonly attribute 'radioState' is exposed to MobileConnection.
> 'radiostatechange' will be fired when radio state changes.
OK !! Thanks Hsin-Yi !
Assignee | ||
Comment 4•11 years ago
|
||
Fix the dependency.
Assignee | ||
Comment 5•11 years ago
|
||
Fix the dependency.
No longer blocks: 926356
Summary: [Gaia::Settings] stop using the settings key 'ril.radio.disabled' to turn off RIL radio → [Gaia][DSDS][Gaia::Settings] stop using the settings key 'ril.radio.disabled' to turn off RIL radio
Comment 6•11 years ago
|
||
Hi Arthur,
Interface is confirmed.
https://bugzilla.mozilla.org/attachment.cgi?id=823820&action=diff
1. add MozMobileConnection.radioState
possible values: null (unknown), 'enabling', 'enabled', 'disabling'
2. use MozMobileConnection.setRadio(in boolean enabled) to enable or disable the radio.
a) If you call the api when radioState not in a steady state, you will receive 'InvalidStateError' error.
non steady => null, 'enabling', 'disabling'
b) Fire onsuccess/onerror to indicate the result.
It is suggest to update the setting value only when the request success, so that it could sync with current radio state.
3. add 'radiostatechange' event whenever radioState changes.
For 2a, you could use the event to get a notice if radioState becomes steady and available to call setRadio()
When setRadio(true), radioState will change from X => 'enabling' => 'enabled'
setRadio(false), X => 'disabling' => 'disabled
Comment 7•11 years ago
|
||
> 1. add MozMobileConnection.radioState
> possible values: null (unknown), 'enabling', 'enabled', 'disabling'
and 'disabled'
Comment 8•11 years ago
|
||
Hi Arthur,
We just modify the function name to:
MozMobileConnection.setRadioEnabled(in boolean enabled)
^^^^^^^
Sorry for the change.
Assignee | ||
Updated•11 years ago
|
Updated•11 years ago
|
Assignee | ||
Comment 9•11 years ago
|
||
Alive, Evelyn, could you help review the patch? Thanks.
Attachment #825783 -
Flags: review?(ehung)
Attachment #825783 -
Flags: review?(alive)
Comment 10•11 years ago
|
||
Comment on attachment 825783 [details]
link to https://github.com/mozilla-b2g/gaia/pull/13284
Please group |enable/disable AirplaneMode| codes all together in AirplaneMode module. Thanks.
Attachment #825783 -
Flags: review?(alive)
Assignee | ||
Comment 11•11 years ago
|
||
Comment on attachment 825783 [details]
link to https://github.com/mozilla-b2g/gaia/pull/13284
Comments addressed, please review it again, thanks!
Attachment #825783 -
Flags: review?(alive)
Comment 12•11 years ago
|
||
Comment on attachment 825783 [details]
link to https://github.com/mozilla-b2g/gaia/pull/13284
Thanks
Attachment #825783 -
Flags: review?(alive) → review+
Updated•11 years ago
|
blocking-b2g: 1.3? → 1.3+
Comment 13•11 years ago
|
||
Comment on attachment 825783 [details]
link to https://github.com/mozilla-b2g/gaia/pull/13284
clear review flag because the 'radiostatechange' listener is missing, and I feel we need more discussion regarding this API change. So here are my questions:
1. Does `mozMobileConnection.setRadioEnabled` turn the rest interfaces(BT, wifi, geolocation, ril.data) off? or it only turns ril.data off and we still rely on the control in Gaia to turn off others?
2. either the answer of (1) is yes or no, I think we need to listen 'radiostatechange' for UI update, e.g., we have to disable the toggle when it's in a transition state(enabling, disabling). Or even prevent users to touch BT toggle in this situation.
3. are we going to get rid off `ril.radio.disabled` key? I don't want to reuse it to represent "user preference" because it's confusing.
Attachment #825783 -
Flags: review?(ehung)
Comment 14•11 years ago
|
||
ni aknow for answering my question 1 in comment 13.
Flags: needinfo?(szchen)
Assignee | ||
Comment 15•11 years ago
|
||
This patch is for landing the new gecko API without break any tests, so that I just did minimal changes. We need to file followup bugs if we would like to do other refactor things.
Comment 16•11 years ago
|
||
(In reply to Evelyn Hung [:evelyn] from comment #14)
> ni aknow for answering my question 1 in comment 13.
Hi Evelyn,
1. It only turns off the radio of modem part. Thus all the services of call, sms, data service are stopped. It won't affect BT, wifi, and geolocation (gps). You still have to turn the others off in airplane mode.
For 3, I agree with Arthur's comment. We should file another bug if we want to get rid of the key "ril.radio.disabled". Currently, FMRadio is using that key for airplane mode checking and then toggle it self on and off. From my opinion, it is confusing and we should use another key that could better represent the airplane mode.
Flags: needinfo?(szchen)
Comment 17•11 years ago
|
||
(In reply to Arthur Chen [:arthurcc] from comment #15)
> This patch is for landing the new gecko API without break any tests, so that
> I just did minimal changes. We need to file followup bugs if we would like
> to do other refactor things.
okay, then I'm fine to merge this patch first just for continuing Gecko work. But I still don't like this patch, and I can point out a few misses there. Please do file a follow-up bug and make sure it will be fixed in v1.3. Thank you so much. :)
Comment 18•11 years ago
|
||
Comment on attachment 825783 [details]
link to https://github.com/mozilla-b2g/gaia/pull/13284
r+ with follow-up bug filed. Thank you.
Attachment #825783 -
Flags: review+
Comment 19•11 years ago
|
||
(In reply to Arthur Chen [:arthurcc] from comment #15)
> This patch is for landing the new gecko API without break any tests, so that
> I just did minimal changes. We need to file followup bugs if we would like
> to do other refactor things.
Hi Arthur,
Based on your comment, I suggest to modify the bug title to a proper one. The purpose of this patch is to support the new API and also maintain the backward compatibility. We are not really "stop using the settings key 'ril.radio.disabled'" in this bug.
Assignee | ||
Comment 20•11 years ago
|
||
Hi Aknow,
I think the title is fine as it says that stop using 'ril.radio.disabled' to "turn off" RIL radio instead of removing 'ril.radio.disabled'. I'll file followup bugs for changing the key name. Thanks for the suggestion.
(In reply to Szu-Yu Chen [:aknow] from comment #19)
> (In reply to Arthur Chen [:arthurcc] from comment #15)
> > This patch is for landing the new gecko API without break any tests, so that
> > I just did minimal changes. We need to file followup bugs if we would like
> > to do other refactor things.
>
> Hi Arthur,
> Based on your comment, I suggest to modify the bug title to a proper one.
> The purpose of this patch is to support the new API and also maintain the
> backward compatibility. We are not really "stop using the settings key
> 'ril.radio.disabled'" in this bug.
Updated•11 years ago
|
Whiteboard: [FT:RIL]
Assignee | ||
Comment 21•11 years ago
|
||
Thanks for reviewing!
master: 9005f113136219c644d00fc3d165eb49228caf61
Status: NEW → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Assignee | ||
Comment 22•11 years ago
|
||
Backed out the code as it fails to enable airplane mode. In system/js/airplane_mode.js we should use the user specified value instead of the negation of the current state to set radio state.
Assignee | ||
Comment 23•11 years ago
|
||
master: fbfcc8ab43e701956d8a73cb8a1ad472bb573830
Assignee | ||
Comment 24•11 years ago
|
||
Evelyn, could you point out the misses so that I won't miss anything when creating the follow up bugs? Thanks!
(In reply to Evelyn Hung [:evelyn] from comment #17)
> (In reply to Arthur Chen [:arthurcc] from comment #15)
> > This patch is for landing the new gecko API without break any tests, so that
> > I just did minimal changes. We need to file followup bugs if we would like
> > to do other refactor things.
>
> okay, then I'm fine to merge this patch first just for continuing Gecko
> work. But I still don't like this patch, and I can point out a few misses
> there. Please do file a follow-up bug and make sure it will be fixed in
> v1.3. Thank you so much. :)
Flags: needinfo?(ehung)
Comment 25•11 years ago
|
||
The most important thing are (as my comment 13 said): 1). considering all transitioning status; 2) making preference setting clear. We can open one follow up bug for both.
Flags: needinfo?(ehung)
Comment 26•11 years ago
|
||
We added airplane mode monitoring (bug 876597) in FM radio app[1], should it be removed too?
[1] https://github.com/mozilla-b2g/gaia/blob/master/apps/fm/js/fm.js#L773
Flags: needinfo?(ehung)
Assignee | ||
Comment 27•11 years ago
|
||
We still write to ril.radio.enabled so the patch does not break FM radio app. But suggest to use the radio state change event after bug 856553 lands.
Flags: needinfo?(ehung)
You need to log in
before you can comment on or make changes to this bug.
Description
•