Closed Bug 784733 Opened 12 years ago Closed 12 years ago

Otoro device drains overnight when left untethered with wifi enabled

Categories

(Firefox OS Graveyard :: General, defect)

Product:
Firefox OS Graveyard
Component:
General
Platform:
ARM
Gonk (Firefox OS)
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

(Not tracked)

Status:
RESOLVED FIXED

People

(Reporter: marcia, Assigned: vchang)

References

Details

Attachments

(2 files, 3 obsolete files)

Part1, idl v1.1
2.07 KB, patch
Details | Diff | Splinter Review
Part2, implementation v1.2
7.48 KB, patch
Details | Diff | Splinter Review 
I have seen this now over the last several days with the device I am testing, using different builds.

I charge my phone up during the day, and then leave it running overnight untethered to a power source. When I come in the morning, the phone has run out of power and takes a while to start back up (see Bug 782708)

Display setting is set to Automatic Brightness.

Maybe an OEM issue, but filing just so we are aware that it is happening.
We've had a bug where the screen doesn't automatically turn off.  Are you manually turning the screen off?

If so, then this is really bad, yeah.
cjones: I will test this again tonight and report back, as I don't recall specifically if I did turn it off manually.
With the screen off, the battery was not completely drained this morning, but it was almost out when I turned the phone on.

Wifi was active, and I have an AT&T card in the phone.
I just pulled out a Nexus S to test something for b2g.  I found that I'd left it on (!!) accidentally for about 60 hours.  It still had about 3/4 battery remaining (!!!).

However, it didn't have a SIM card inserted, and wifi was disabled.

So we need to narrow down what the problem is.  The possibilities are
 - problem in otoro kernel compared to Nexus S kernel
 - wifi is not sleeping intelligently.  Perhaps because of kernel problem.
 - same for radio

If QA has enough spare phones, we can test these hypotheses in parallel.  The experiments we want to run are
          wifi   | radio   
        +--------+-------
otoro   | off    | off
nexus s | off    | off
otoro   | on     | off
nexus s | on     | off
otoro   | off    | on
nexus s | off    | on
otoro   | on     | on
nexus s | on     | on

In each case, we want to leave a fully-charged phone disconnected and with the screen off overnight.  Then measure charge the next day.
For fairness, "radio off" should mean "no SIM card".
1. For the report on comment 1, that may be related to 
   "Phone device should go to suspend mode automatically if there is no input even"
   I found in the recent build, the device will not go into suspend mode automatically
   and in setting there is no item for configuring the timeout value.

2. For the comment 3, If B2G actually asked the system got into suspend mode not just "screen off" state, then it should be the issue on power management from OEM side.
It seems there is a lot of digging QA can do for us here. Marcia, can someone from your team own this item and get some concrete issues extracted? This is a really important area to get right.
I will take point on this. I don't have enough devices to do the testing on Comment 4, but I'll work on getting them or try to source them from people within the team that already have them.

(In reply to Andreas Gal :gal from comment #7)
> It seems there is a lot of digging QA can do for us here. Marcia, can
> someone from your team own this item and get some concrete issues extracted?
> This is a really important area to get right.
QA Contact: mozillamarcia.knous
It would be nice to have a tool of some sort for rating the battery consumption rate per application as well to help us measure and have some sort of readings in our study.
(In reply to Naoki Hirata :nhirata from comment #9)
> It would be nice to have a tool of some sort for rating the battery
> consumption rate per application as well to help us measure and have some
> sort of readings in our study.

Yeah, this has come up in the status meeting as well. I think there was some hope re: overall consumption metering, but per-app was decidedly non-trivial.
In the spec now, does B2G device need go to suspend mode automatically?
Because I didn't see this behavior in recent build.
(In reply to Marco Chen [:mchen] from comment #11)
> In the spec now, does B2G device need go to suspend mode automatically?
> Because I didn't see this behavior in recent build.

This is bug 781076, which will be fixed by the patch in bug 781620
Keywords: qawanted
I have a setup ready to go with 4 Otoro devices to test tonight, but I don't have any Nexus phones yet set up with B2G. 

As a first test I will just leave the 4 Otoro devices in the state in Comment 4, and report those results back first.

In the meantime I will see if I can scare up some Nexus phones and get them flashed.
          wifi   | radio   
        +--------+-------
otoro   | off    | off   Result: 3/4 battery remaining
otoro   | on     | off   Result: 3/4 battery remaining
otoro   | off    | on    Result: Battery almost full
otoro   | on     | on    Result: Battery completely dead

Testing conditions:

Elapsed time of test: 15 hours, 30 minutes

*All phones started fully charged
*All phones with SIM cards were using AT&T
*All phones running the same build ID
*All phones set to default display settings
*Phones that were connected to wifi were connected to Mozilla guest network
I repeated the test last night just for comparison, but I did not get the same results:

          wifi   | radio   
        +--------+-------
otoro   | off    | off   Result: almost full
otoro   | on     | off   Result: almost full
otoro   | off    | on    Result: almost full
otoro   | on     | on    Result: almost full

All conditions were the same as in Comment 14.

During the day I left the display on, and several of the phones ran out of juice about mid day - I guess the bug with the phone not auto suspending is still happening.

Geo is also bringing in something to help test the battery charge tomorrow.
Marcia, to clarify, that's still same build as the test in Comment 14, right? If so, means the test isn't giving us stable results. If not, then might just mean we've improved functionality somehow.

Re: what I'm bringing in, going to bring a Kill-a-Watt meter in to check the power draw on the charger. 

It won't really intersect with this bug as much as some of the anecdotal reports regarding the device either not fully charging or draining while on the tether. Really specifically, I wanted to figure out if it was going into trickle mode at 100% reliably. Not sure what I'll see yet, but it's a more data is better thing.
Yes, I was testing with the same builds in Comment 14 and 15.
After updating all four Otoro phones to the 20120911 build, I performed the test again last evening and here are the results:

          wifi   | radio   
        +--------+-------
otoro   | off    | off   Result: 3/4
otoro   | on     | off   Result: 1/3
otoro   | off    | on    Result: almost full
otoro   | on     | on    Result: 1/3
Can we put one more Otoro phone with original Android-ICS image, and turn on wifi/radio for comparison?
Good idea.
Summary: Otoro device drains overnight when left untethered → Otoro device drains overnight when left untethered with wifi enabled
Maybe we could get a power meter and record the actual power usage?
As I know, wifi will generally use less power if you are continuously sending/receiving a lot of data, however, 3G will likely use less power for occasionally checking email, light web browsing.

Does otoro device support separate measurement points for CPU/wifi/3G/display ? In that case we can get more detail information about power usage between different hardware components. I used NI USB-6009 DAQ to measure power usage on Beagle board. It supports measurement points for CPU/USB/Wifi. 

BTW, we don't unload wifi driver on Otoro device when we turn off wifi due to driver or kernel bug ? It might consume less energy if we unload it.
(In reply to Vincent Chang from comment #22)
> BTW, we don't unload wifi driver on Otoro device when we turn off wifi due
> to driver or kernel bug ? It might consume less energy if we unload it.

Having the wifi driver loaded has nothing to do with whether the wifi is on or off on properly written drivers. Bringing the interface up turns on the wifi hardware, and bringing the interface down turns off the wifi hardware. Unloading the module is really more for buggy drivers.

Note that we don't unload wifi modules on otoro due to wifi breaking on unload, though newer kernel updates may have fixed this issue.
One thing that I've often seen on Android is the phone turning off wifi when the device isn't being used. Not sure what it does exactly but it probably helps with wifi power consumption. It's not a great solution though - some wifi APs require reauthentication every time one reconnects.

What should be done at minimum is to take advantage of the power save features that 802.11 specifies. These features allow wifi interfaces to increase latency in exchange for reduced power usage.
I was told this would not be easy since there is no way to roll back the phones we have here in MV to Android-ICS.

(In reply to Shian-Yow Wu from comment #19)
> Can we put one more Otoro phone with original Android-ICS image, and turn on
> wifi/radio for comparison?
(In reply to Marcia Knous [:marcia] from comment #25)
> I was told this would not be easy since there is no way to roll back the
> phones we have here in MV to Android-ICS.
> 

You should already have the tool to flash otoros back to Android.
(In reply to Michael Wu [:mwu] from comment #24)
> One thing that I've often seen on Android is the phone turning off wifi when
> the device isn't being used. Not sure what it does exactly but it probably
> helps with wifi power consumption. It's not a great solution though - some
> wifi APs require reauthentication every time one reconnects.

We are doing that in gaia/apps/system/js/wifi.js right now. I am checking right now if that actually works.
Attached patch Part1, idl (obsolete) — — Splinter Review 
Define wifi power saving mode api interface
Attachment #662108 - Flags: review?(mrbkap)
Attached patch Part2, implementation (obsolete) — — Splinter Review 
Implementation wifi power saving mode api interface 

The wifi driver supports two commands to help to reduce power consumption.
One is "DRIVER POWERMODE", the other one is "DRIVER SETSUSPENDOPT". I found that only "DRIVER POWERMODE" command is valid on Otoro device. But it's still worth to try how much improvement using this command. 

BTW, These two commands work well on SGS2.    

We can use javascript console to enabe/disable wifi power saving mode. 
adb forward tcp:9999 tcp:9999
netcat localhost 9999

navigator.mozWifiManager.setPowerSavingMode(true) -> enable 
navigator.mozWifiManager.setPowerSavingMode(false) -> disable
navigator.battery.level -> battery level
Attachment #662111 - Flags: review?(mrbkap)
(In reply to Vincent Chang from comment #29)
> The wifi driver supports two commands to help to reduce power consumption.
> One is "DRIVER POWERMODE", the other one is "DRIVER SETSUSPENDOPT". I found
> that only "DRIVER POWERMODE" command is valid on Otoro device. But it's
> still worth to try how much improvement using this command. 
> 

Where is DRIVER SETSUSPENDOPT implemented in wpa supplicant?
(In reply to Michael Wu [:mwu] from comment #30)
> (In reply to Vincent Chang from comment #29)
> > The wifi driver supports two commands to help to reduce power consumption.
> > One is "DRIVER POWERMODE", the other one is "DRIVER SETSUSPENDOPT". I found
> > that only "DRIVER POWERMODE" command is valid on Otoro device. But it's
> > still worth to try how much improvement using this command. 
> > 
> 
> Where is DRIVER SETSUSPENDOPT implemented in wpa supplicant?

Should be in external/hostap/src/drivers/driver_nl80211.c function wpa_driver_nl80211_driver_cmd ?
Attachment #662108 - Flags: review?(mrbkap) → review+
Comment on attachment 662111 [details] [diff] [review]
Part2, implementation

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

One small comment.

::: dom/wifi/WifiWorker.js
@@ +2273,5 @@
> +    // Power saving mode.
> +    const POWER_MODE_AUTO = 0;
> +    let self = this;
> +    let enabled = msg.data;
> +    let mode = enabled ? POWER_MODE_AUTO : POWER_MODE_ACTIVE;

Rather than defining these constants here, I'd prefer to make the WifiManager's API take a string "ACTIVE" or "AUTO" and do the translation there.
Attachment #662111 - Flags: review?(mrbkap)
Attached patch Part1, idl v1.1 — — Splinter Review 
added the hg tip header
Attachment #662108 - Attachment is obsolete: true
Attached patch Part2, implementation v1.1 (obsolete) — — Splinter Review 
1. Addressed command 32 
2. Added hg tip header
Attachment #662111 - Attachment is obsolete: true
Attachment #662825 - Flags: review?(mrbkap)
Comment on attachment 662825 [details] [diff] [review]
Part2, implementation v1.1

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

::: dom/wifi/WifiWorker.js
@@ +403,5 @@
>      });
>    }
>  
>    function setPowerModeCommand(mode, callback) {
> +    doBooleanCommand("DRIVER POWERMODE " + (mode == "AUTO" ? 0 : 1), "OK", callback);

Uber-nit: === please.

@@ +2276,5 @@
> +    // even if suspend optimization command failed.
> +    WifiManager.setSuspendOptimizations(enabled, function(ok) {
> +      WifiManager.setPowerMode(mode, function(ok) {
> +        if (ok) {
> +          self._sendMessage(message, true, true, msg);

Can this use a single _sendMessage call? If it's too much of a hassle, don't worry about it.
Attachment #662825 - Flags: review?(mrbkap) → review+
Updated the patch to address comment 35. 
Using '===' instead of '=='.
Attachment #662825 - Attachment is obsolete: true
Assignee: nobody → vchang
Keywords: checkin-needed
https://hg.mozilla.org/integration/mozilla-inbound/rev/8381cb18f0f6
https://hg.mozilla.org/integration/mozilla-inbound/rev/bebaecb60f44

In the future, please use different commit messages for your patches, preferably ones that describe what each individual patch is doing. Thanks!
Keywords: checkin-needed
(In reply to Ryan VanderMeulen from comment #37)
> https://hg.mozilla.org/integration/mozilla-inbound/rev/8381cb18f0f6
> https://hg.mozilla.org/integration/mozilla-inbound/rev/bebaecb60f44
> 
> In the future, please use different commit messages for your patches,
> preferably ones that describe what each individual patch is doing. Thanks!

Got it, will make it clearly next time. Thanks for your help.
(In reply to Vincent Chang from comment #31)
> (In reply to Michael Wu [:mwu] from comment #30)
> > (In reply to Vincent Chang from comment #29)
> > > The wifi driver supports two commands to help to reduce power consumption.
> > > One is "DRIVER POWERMODE", the other one is "DRIVER SETSUSPENDOPT". I found
> > > that only "DRIVER POWERMODE" command is valid on Otoro device. But it's
> > > still worth to try how much improvement using this command. 
> > > 
> > 
> > Where is DRIVER SETSUSPENDOPT implemented in wpa supplicant?
> 
> Should be in external/hostap/src/drivers/driver_nl80211.c function
> wpa_driver_nl80211_driver_cmd ?

I don't see it - https://www.codeaurora.org/gitweb/quic/la/?p=platform/external/hostap.git;a=blob;f=src/drivers/driver_nl80211.c;h=8694699eca22dc0dcd3525170ce6c26f285c16fb;hb=ics_chocolate_rb4.2#l8454
Keywords: qawanted
Blocks: 888269
Blocks: 888552
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: