[Telemetry] Use new battery disabling method for telemetry.

[Telemetry] Use new battery disabling method for telemetry.

A new way to enable collection of power data that is not
dependant on the divice was implemented in
https://codereview.chromium.org/993733002/
This CL is to change telemetry to use this when getting dumpsys data.
This method of disabling charging does not work with monsoon since
it does not actualy disable charging, but just starts the dumpsys collection.

BUG=

Committed: https://crrev.com/f83d74abc7eb83445e074cec0925f57fb6f53dd2
Cr-Commit-Position: refs/heads/master@{#321945}

[rnephew (Wrong account), 2015-03-18 19:30:58]

rnephew@google.com changed reviewers:
+ dtu@chromium.org, rnephew@google.com, sullivan@chromium.org

[rnephew (Wrong account), 2015-03-18 19:33:33]

rnephew@google.com changed reviewers:
+ jayker@chromium.org, klundberg@chromium.org

[rnephew (Wrong account), 2015-03-18 21:26:56]

On 2015/03/18 19:33:33, rnephew wrote:

The trybot run is here: https://codereview.chromium.org/1019613002

It failed because one of the commands takes over 3 minutes on K. Is there any
precedent in telemetry for having different code paths for L v. K? It shouldn't
even run dumpsys power collection on K since it is not supported

[sullivan, 2015-03-19 13:52:51]

On 2015/03/18 21:26:56, rnephew wrote:
> On 2015/03/18 19:33:33, rnephew wrote:
> 
> The trybot run is here: https://codereview.chromium.org/1019613002
> 
> It failed because one of the commands takes over 3 minutes on K. Is there any
> precedent in telemetry for having different code paths for L v. K? It
shouldn't
> even run dumpsys power collection on K since it is not supported

Dave, do you know the answer to this, or who to ask? I think a lot of the power
methods we use don't work on L, but don't know if there's any graceful fallback
implemented.

[rnephew (Wrong account), 2015-03-19 14:24:00]

On 2015/03/19 13:52:51, sullivan wrote:
> On 2015/03/18 21:26:56, rnephew wrote:
> > On 2015/03/18 19:33:33, rnephew wrote:
> > 
> > The trybot run is here: https://codereview.chromium.org/1019613002
> > 
> > It failed because one of the commands takes over 3 minutes on K. Is there
any
> > precedent in telemetry for having different code paths for L v. K? It
> shouldn't
> > even run dumpsys power collection on K since it is not supported
> 
> Dave, do you know the answer to this, or who to ask? I think a lot of the
power
> methods we use don't work on L, but don't know if there's any graceful
fallback
> implemented.

For power in particular, in
telemetry/telemetry/core/platform/power_monitor/android_dumpsys_power_monitor.py
it checks the dumpsys version, and if its not 8 or 9 it doesn't return power
data; but I was wondering if there is in general a way telemetry handles
differences in OS versions.

[sullivan, 2015-03-19 14:30:03]

sullivan@chromium.org changed reviewers:
+ nednguyen@google.com

[sullivan, 2015-03-19 14:30:04]

+Ned as well, to see if he has opinions on handling the difference in OS
versions here.

[rnephew (Wrong account), 2015-03-23 16:58:33]

I re-ran the trybot after https://codereview.chromium.org/1012383006/ landed.

https://codereview.chromium.org/1030683002

Works on KK with that fix.

[sullivan, 2015-03-24 01:08:37]

lgtm

[nednguyen, 2015-03-24 01:17:05]

On 2015/03/24 01:08:37, sullivan wrote:
> lgtm

Do those 2 methods still work on both L & K?

[sullivan, 2015-03-24 01:20:57]

On 2015/03/24 01:17:05, nednguyen wrote:
> On 2015/03/24 01:08:37, sullivan wrote:
> > lgtm
> 
> Do those 2 methods still work on both L & K?

The trybot result shows a passing test for K. (It "works" in that the test
doesn't fail, but there is no way to actually disable battery on K. Randy, I
assume you tested L locally since there are no trybots yet for L?

[nednguyen, 2015-03-24 01:49:31]

https://codereview.chromium.org/1010213008/diff/1/tools/telemetry/telemetry/c...
File
tools/telemetry/telemetry/core/platform/power_monitor/android_dumpsys_power_monitor.py
(right):

https://codereview.chromium.org/1010213008/diff/1/tools/telemetry/telemetry/c...
tools/telemetry/telemetry/core/platform/power_monitor/android_dumpsys_power_monitor.py:38:
self._device.DisableBatteryUpdates()
If this doesn't really work on K, I think you need to throw exception. I think
we should either do the performance measurement correctly, or we should make it
fail hard.

[rnephew (Wrong account), 2015-03-24 02:04:53]

On 2015/03/24 01:49:31, nednguyen wrote:
>
https://codereview.chromium.org/1010213008/diff/1/tools/telemetry/telemetry/c...
> File
>
tools/telemetry/telemetry/core/platform/power_monitor/android_dumpsys_power_monitor.py
> (right):
> 
>
https://codereview.chromium.org/1010213008/diff/1/tools/telemetry/telemetry/c...
>
tools/telemetry/telemetry/core/platform/power_monitor/android_dumpsys_power_monitor.py:38:
> self._device.DisableBatteryUpdates()
> If this doesn't really work on K, I think you need to throw exception. I think
> we should either do the performance measurement correctly, or we should make
it
> fail hard.

This has been tested and works on L.

These methods works on K; it is the collection of power data via dumpsys in the
power_monitor that does not work on K; disabling charging in this way works.
Power Monitor here:
https://code.google.com/p/chromium/codesearch#chromium/src/tools/telemetry/te...

Under the current implementation it will read 'missing' for the power readings.
I would be hesitant to throw an exception instead of issuing 'missing' because
it does still take any other measurement that it can, while still displaying
(and outputting a warning message) that it is missing power data. I think that's
why it was implemented in this way.

Waiting to submit until this message is acknowledged.

[nednguyen, 2015-03-24 02:31:12]

On 2015/03/24 02:04:53, rnephew wrote:
> On 2015/03/24 01:49:31, nednguyen wrote:
> >
>
https://codereview.chromium.org/1010213008/diff/1/tools/telemetry/telemetry/c...
> > File
> >
>
tools/telemetry/telemetry/core/platform/power_monitor/android_dumpsys_power_monitor.py
> > (right):
> > 
> >
>
https://codereview.chromium.org/1010213008/diff/1/tools/telemetry/telemetry/c...
> >
>
tools/telemetry/telemetry/core/platform/power_monitor/android_dumpsys_power_monitor.py:38:
> > self._device.DisableBatteryUpdates()
> > If this doesn't really work on K, I think you need to throw exception. I
think
> > we should either do the performance measurement correctly, or we should make
> it
> > fail hard.
> 
> This has been tested and works on L.
> 
> These methods works on K; it is the collection of power data via dumpsys in
the
> power_monitor that does not work on K; disabling charging in this way works.
> Power Monitor here:
>
https://code.google.com/p/chromium/codesearch#chromium/src/tools/telemetry/te...
> 
> Under the current implementation it will read 'missing' for the power
readings.
> I would be hesitant to throw an exception instead of issuing 'missing' because
> it does still take any other measurement that it can, while still displaying
> (and outputting a warning message) that it is missing power data. I think
that's
> why it was implemented in this way.
> 
> Waiting to submit until this message is acknowledged.

If it correctly disable charging, this is LGTM. Agree that missing data doesn't
need to be fatal, wrong data should be fatal though :)

[rnephew (Wrong account), 2015-03-24 05:58:05]

The CQ bit was checked by rnephew@google.com

[commit-bot: I haz the power, 2015-03-24 05:58:31]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1010213008/1

[commit-bot: I haz the power, 2015-03-24 07:02:06]

Committed patchset #1 (id:1)

[commit-bot: I haz the power, 2015-03-24 07:02:50]

Patchset 1 (id:??) landed as
https://crrev.com/f83d74abc7eb83445e074cec0925f57fb6f53dd2
Cr-Commit-Position: refs/heads/master@{#321945}