879695 - B2G RIL: Notify modem that the screen is off

Status: RESOLVED FIXED

(Core :: DOM: Device Interfaces, defect)

Description

[:gerard-majax]

On Android devices, one can notice (adb logcat -b radio) that when you power off the screen, the event gets propagated to the modem so that it can enter in low power mode. One side effect of this is that Android apps can't get radio signal updates when the screen is off.

When checking this behavior on B2G, it's not the case. The RIL Worker has some setScreenState() method to notify the modem, defined in https://mxr.mozilla.org/mozilla-b2g18/source/dom/system/gonk/ril_worker.js#2460 but looking at the source shows it is never called.

I'd suspect we should notify our RIL code from https://mxr.mozilla.org/mozilla-b2g18/source/dom/power/PowerManager.cpp#147 that we are entering or leaving a screen state change, and thus notify the modem via setScreenState() from the RIL Worker.

Comment 1

[Yoshi Cheng-Hao Huang [:allstars.chh][:allstarschh][:yoshi]]

Siddarth has raised this issue in Bug 843382, but seems not active for 2 months.

Comment 2

[Vicamo Yang [:vicamo][:vyang]]

Oops, so let's go there for further discuss.

Comment 3

[Vicamo Yang [:vicamo][:vyang]]

Bug 843382 is a confidential bug for some reason it shouldn't be.  Sorry for the message spam, but I'm going to reverse the case here.

Comment 4

[Vicamo Yang [:vicamo][:vyang]]

I think we can have a new observer event "screen-state-changed" in PowerManager and make RIL listen to it.  The code to broadcast such event can be referenced in:

  http://mxr.mozilla.org/mozilla-central/source/dom/time/TimeChangeObserver.cpp#60
  http://mxr.mozilla.org/mozilla-central/source/dom/system/gonk/RadioInterfaceLayer.js#349
  http://mxr.mozilla.org/mozilla-central/source/dom/system/gonk/RadioInterfaceLayer.js#1924

Comment 6

[:gerard-majax]

Attachment: Patch v1

This patch, on b2g18, with Nexus S:

I/power   (11850): *** set_screen_state 0
I/Gecko   (11850): -*- RadioInterfaceLayer: Received a screen-state-changed event: "off"
I/Gecko   (11850): -*- RadioInterfaceLayer: setScreenState: "off"
I/Gecko   (11850): RIL Worker: Received DOM message {"rilMessageType":"setScreenState","on":false}
I/Gecko   (11850): RIL Worker: Will set RIL_REQUEST_SCREEN_STATE: {"rilMessageType":"setScreenState","on":false}
I/power   (11850): *** set_screen_state 1
I/Gecko   (11850): -*- RadioInterfaceLayer: Received a screen-state-changed event: "on"
I/Gecko   (11850): -*- RadioInterfaceLayer: setScreenState: "on"
I/Gecko   (11850): RIL Worker: Received DOM message {"rilMessageType":"setScreenState","on":true}
I/Gecko   (11850): RIL Worker: Will set RIL_REQUEST_SCREEN_STATE: {"rilMessageType":"setScreenState","on":true}
I/power   (11850): *** set_screen_state 0
I/Gecko   (11850): -*- RadioInterfaceLayer: Received a screen-state-changed event: "off"
I/Gecko   (11850): -*- RadioInterfaceLayer: setScreenState: "off"
I/Gecko   (11850): RIL Worker: Received DOM message {"rilMessageType":"setScreenState","on":false}
I/Gecko   (11850): RIL Worker: Will set RIL_REQUEST_SCREEN_STATE: {"rilMessageType":"setScreenState","on":false}

Comment 7

[Vicamo Yang [:vicamo][:vyang]]

Comment on attachment 758492 [details] [diff] [review]
Patch v1

Review of attachment 758492 [details] [diff] [review]:
-----------------------------------------------------------------

Thank you for this quick patch :)

::: dom/system/gonk/RadioInterfaceLayer.js
@@ +2044,5 @@
>  
> +  setScreenState: function setScreenState(state) {
> +    debug("setScreenState: " + JSON.stringify(state));
> +    this.worker.postMessage({rilMessageType: "setScreenState",
> +                            on: (state === "on")});

nit: please have:

  this.worker.postMessage({
    rilMessageType: "setScreenState",
    on: (state === "on")
  });

Comment 8

[:gerard-majax]

Attachment: Patch v2

Thanks for your comments, here is a new patch that addresses the remarks.

Comment 9

[Kan-Ru Chen [:kanru] (UTC+9)]

Comment on attachment 758678 [details] [diff] [review]
Patch v2

Review of attachment 758678 [details] [diff] [review]:
-----------------------------------------------------------------

LGTM

Comment 10

[Vicamo Yang [:vicamo][:vyang]]

https://hg.mozilla.org/projects/birch/rev/0dbc58367613

Comment 11

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/0dbc58367613