[Android] Convert to BatteryUtils

build/android/pylib/device/battery_utils.py

 # Copyright 2015 The Chromium Authors. All rights reserved.
 # Use of this source code is governed by a BSD-style license that can be
 # found in the LICENSE file.
 
 """Provides a variety of device interactions with power.
 """
 # pylint: disable=unused-argument
 
 import collections
 import contextlib
@@ skipping 228 matching lines @@
       device_errors.CommandFailedError: If device is not L or higher.
     """
     if (self._device.build_version_sdk <
         constants.ANDROID_SDK_VERSION_CODES.LOLLIPOP):
       raise device_errors.DeviceVersionError('Device must be L or higher.')
     try:
       self.DisableBatteryUpdates(timeout=timeout, retries=retries)
       yield
     finally:
       self.EnableBatteryUpdates(timeout=timeout, retries=retries)
+
+  def ChargeDeviceToLevel(self, level):

[nednguyen, 2015/04/01 22:17:39]

Maybe this needn't be addressed in this patch but I find this API awkward
because it's unsure what would happen to the charging state after the device has
reached the specified level.

I think this probably should be changed to WaitForDeviceToCharge(self, level)
method which throw exception if the charging bit is False.

[jbudorick, 2015/04/01 22:32:21]

Why does this matter?
Strongly disagree with this. Easier to make a mistake for no benefit.

[jbudorick, 2015/04/01 22:34:15]

Actually, this comment should just make it explicit that this function:
 - enables charging
 - waits for the device to charge to the level

[nednguyen, 2015/04/01 22:49:06]

Just looking at the method name, it's unclear what this will do in case the
charge level you are asking for is lower the the actual charged level of the
device.

Another question comes into to mind is whether this will stop charging the
device when the device has reached specified level. Based on the function name,
I expect it to stop charging, but it doesn't based on the implementation.

[jbudorick, 2015/04/01 22:58:51]

We generally want our devices in a charging state unless explicitly disabled.
This is doing _exactly_ what I think we want it to do, and I think we'd rather
make any possible ambiguity clear in the comment than have a function name like
"EnableChargingAndChargeDeviceToLevel"

If a caller has a strong preference about the charging state coming out of this,
fine. They can set the charging state. Otherwise, let's leave the device in the
default charging state -- i.e., with charging enabled.

[rnephew (Wrong account), 2015/04/01 23:01:04]

Added comment saying it enabled charging.

+    """ Waits for device to be charged to given level.
+
+    Args:
+      level: level of charge to wait for.
+    """
+    self.SetCharging(True)

[nednguyen, 2015/04/01 22:17:39]

Assume that the device's charging state is off, and it is at level 90%. Making
the call ChargeDeviceToLevel(70) just turns on the charging state is a very
surprising.

[jbudorick, 2015/04/01 22:32:21]

I don't see why a user asking for a device to be charged to some level would be
surprised that a device is charging.

+
+    def device_charged():
+      battery_level = self.GetBatteryInfo().get('level')
+      if battery_level is None:
+        logging.warning('Unable to find current battery level.')
+        battery_level = 100
+      else:
+        logging.info('current battery level: %s', battery_level)
+        battery_level = int(battery_level)
+      return battery_level >= level
+
+    timeout_retry.WaitFor(device_charged, wait_period=60)