[Android] Convert to BatteryUtils

build/android/pylib/device/battery_utils.py

Index: build/android/pylib/device/battery_utils.py
diff --git a/build/android/pylib/device/battery_utils.py b/build/android/pylib/device/battery_utils.py
index 96e3bf660c5841642ba3fc0fb88e560b11314df5..9d872e0e45ae0ae699e8af75527b0da4bcb97db6 100644
--- a/build/android/pylib/device/battery_utils.py
+++ b/build/android/pylib/device/battery_utils.py
@@ -246,3 +246,23 @@ class BatteryUtils(object):
       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)