[Android][Telemetry] Seperate fuel gauge from AndroidDumpsysPowerMonitor

[Android][Telemetry] Seperate fuel gauge from AndroidDumpsysPowerMonitor

BUG=513004
Committed: https://crrev.com/5e8eeb9b79d3887e6d9332e0f83d01cc5909b237
Cr-Commit-Position: refs/heads/master@{#340461}

[rnephew (Reviews Here), 2015-07-21 21:45:37]

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

[nednguyen, 2015-07-23 17:30:26]

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

[nednguyen, 2015-07-23 17:30:26]

lgtm

Please also wait for Jared's comments.

[jdduke (slow), 2015-07-23 18:21:53]

https://codereview.chromium.org/1251673002/diff/60001/tools/telemetry/telemet...
File
tools/telemetry/telemetry/internal/platform/power_monitor/android_fuelgauge_power_monitor.py
(right):

https://codereview.chromium.org/1251673002/diff/60001/tools/telemetry/telemet...
tools/telemetry/telemetry/internal/platform/power_monitor/android_fuelgauge_power_monitor.py:53:
power_results = {'identifier': 'fuelgauge', 'power_samples_mw': []}
Let's leave off power_samples_mw for now.

https://codereview.chromium.org/1251673002/diff/60001/tools/telemetry/telemet...
tools/telemetry/telemetry/internal/platform/power_monitor/android_fuelgauge_power_monitor.py:56:
return {'fuel_gauge': power_results}
Hmm, is this the return type format we want/need? What patches need to land
before this? Any reason one identifier is fuel_gauge and the other fuelgauge?

[rnephew (Reviews Here), 2015-07-23 18:36:32]

Its also important to point out that we will temporarily lose fuel gauge
reporting until the CL for supporting multiple power monitors lands, which I'll
put up as soon as this one lands.

https://codereview.chromium.org/1251673002/diff/60001/tools/telemetry/telemet...
File
tools/telemetry/telemetry/internal/platform/power_monitor/android_fuelgauge_power_monitor.py
(right):

https://codereview.chromium.org/1251673002/diff/60001/tools/telemetry/telemet...
tools/telemetry/telemetry/internal/platform/power_monitor/android_fuelgauge_power_monitor.py:53:
power_results = {'identifier': 'fuelgauge', 'power_samples_mw': []}
On 2015/07/23 18:21:53, jdduke wrote:
> Let's leave off power_samples_mw for now.

Done.

https://codereview.chromium.org/1251673002/diff/60001/tools/telemetry/telemet...
tools/telemetry/telemetry/internal/platform/power_monitor/android_fuelgauge_power_monitor.py:56:
return {'fuel_gauge': power_results}
On 2015/07/23 18:21:53, jdduke wrote:
> Hmm, is this the return type format we want/need? What patches need to land
> before this? Any reason one identifier is fuel_gauge and the other fuelgauge?

I should probably have this do the old way that clobbers the identifiers, I have
a seperate CL that changes metrics/power.py and all the power monitors to
support the power_monitor dictionary having multiple unique identifiers and
doesn't step on each others toes. 

The CL is stand alone so it doesn't need anything else to work. It is needed for
the support for multiple power monitors to land, but that will go up as soon as
this lands.

As for the difference in fuelgauge and fuel_gauge.. I'll switch them all to
fuel_gauge for consistency.

[jdduke (slow), 2015-07-23 18:43:25]

On 2015/07/23 18:36:32, rnephew1 wrote:
> The CL is stand alone so it doesn't need anything else to work. It is needed
for
> the support for multiple power monitors to land, but that will go up as soon
as
> this lands.

Hmm, why can't we land this after adding support for multiple monitors?

[rnephew (Reviews Here), 2015-07-23 19:14:11]

On 2015/07/23 18:43:25, jdduke wrote:
> On 2015/07/23 18:36:32, rnephew1 wrote:
> > The CL is stand alone so it doesn't need anything else to work. It is needed
> for
> > the support for multiple power monitors to land, but that will go up as soon
> as
> > this lands.
> 
> Hmm, why can't we land this after adding support for multiple monitors?

We can, CL up. https://codereview.chromium.org/1255673002

[rnephew (Wrong account), 2015-07-24 03:51:57]

On 2015/07/23 19:14:11, rnephew1 wrote:
> On 2015/07/23 18:43:25, jdduke wrote:
> > On 2015/07/23 18:36:32, rnephew1 wrote:
> > > The CL is stand alone so it doesn't need anything else to work. It is
needed
> > for
> > > the support for multiple power monitors to land, but that will go up as
soon
> > as
> > > this lands.
> > 
> > Hmm, why can't we land this after adding support for multiple monitors?
> 
> We can, CL up. https://codereview.chromium.org/1255673002

Rebased with multiple monitors CL. PTAL

[jdduke (slow), 2015-07-24 23:35:33]

lgtm assuming this keeps our graphs nice and happy. Thanks.

https://codereview.chromium.org/1251673002/diff/90001/tools/telemetry/telemet...
File
tools/telemetry/telemetry/internal/platform/power_monitor/android_fuelgauge_power_monitor.py
(right):

https://codereview.chromium.org/1251673002/diff/90001/tools/telemetry/telemet...
tools/telemetry/telemetry/internal/platform/power_monitor/android_fuelgauge_power_monitor.py:35:
fuel_gauge_delta = (
I wonder if we should reverse the order of querying the counter and enabling
charging?

https://codereview.chromium.org/1251673002/diff/90001/tools/telemetry/telemet...
tools/telemetry/telemetry/internal/platform/power_monitor/android_fuelgauge_power_monitor.py:47:
logging.info('Device voltage at %s', voltage)
Is the voltage logged elsewhere? Is the logging useful?

[rnephew (Reviews Here), 2015-07-25 00:22:28]

https://codereview.chromium.org/1251673002/diff/90001/tools/telemetry/telemet...
File
tools/telemetry/telemetry/internal/platform/power_monitor/android_fuelgauge_power_monitor.py
(right):

https://codereview.chromium.org/1251673002/diff/90001/tools/telemetry/telemet...
tools/telemetry/telemetry/internal/platform/power_monitor/android_fuelgauge_power_monitor.py:35:
fuel_gauge_delta = (
On 2015/07/24 23:35:33, jdduke wrote:
> I wonder if we should reverse the order of querying the counter and enabling
> charging?

Done.

https://codereview.chromium.org/1251673002/diff/90001/tools/telemetry/telemet...
tools/telemetry/telemetry/internal/platform/power_monitor/android_fuelgauge_power_monitor.py:47:
logging.info('Device voltage at %s', voltage)
On 2015/07/24 23:35:33, jdduke wrote:
> Is the voltage logged elsewhere? Is the logging useful?

This probably isn't all that useful, removed.

[rnephew (Reviews Here), 2015-07-27 13:32:11]

The CQ bit was checked by rnephew@chromium.org

[rnephew (Reviews Here), 2015-07-27 13:32:12]

The patchset sent to the CQ was uploaded after l-g-t-m from
nednguyen@google.com, jdduke@chromium.org
Link to the patchset: https://codereview.chromium.org/1251673002/#ps110001
(title: " ")

[commit-bot: I haz the power, 2015-07-27 13:32:33]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1251673002/110001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1251673002/110001

[commit-bot: I haz the power, 2015-07-27 14:46:12]

Committed patchset #7 (id:110001)

[commit-bot: I haz the power, 2015-07-27 14:46:59]

Patchset 7 (id:??) landed as
https://crrev.com/5e8eeb9b79d3887e6d9332e0f83d01cc5909b237
Cr-Commit-Position: refs/heads/master@{#340461}