Small improvements to api.platforms.android.

appengine/swarming/swarming_bot/api/platforms/android.py

 # Copyright 2015 The Swarming Authors. All rights reserved.
 # Use of this source code is governed by the Apache v2.0 license that can be
 # found in the LICENSE file.
 
 """Android specific utility functions.
 
 This file serves as an API to bot_config.py. bot_config.py can be replaced on
 the server to allow additional server-specific functionality.
 """
 
 import logging
 import os
 import re
 import subprocess
 import sys
 
-# This file must be imported from swarming_bot.zip (or with '..' in sys.path).
-# libusb1 must have been put in the path already.
+# This file must be imported from swarming_bot.zip (or with '../..' in
+# sys.path). libusb1 must have been put in the path already.
 
 import adb
 import adb.adb_commands
 
 
 # Find abs path to third_party/ dir in the bot root dir.
 import third_party
 THIRD_PARTY_DIR = os.path.dirname(os.path.abspath(third_party.__file__))
 sys.path.insert(0, os.path.join(THIRD_PARTY_DIR, 'rsa'))
 sys.path.insert(0, os.path.join(THIRD_PARTY_DIR, 'pyasn1'))
@@ skipping 15 matching lines @@
   def __init__(self):
     self._buf = ''
   def update(self, msg):
     self._buf += msg
   def digest(self):
     return self._buf
 pkcs1.HASH_METHODS['SHA-1-PREHASHED'] = _Accum
 pkcs1.HASH_ASN1['SHA-1-PREHASHED'] = pkcs1.HASH_ASN1['SHA-1']
 
 
-# Set when ADB is initialized. It contains one or multiple key used to
-# authenticate to Android debug protocol (adb).
+# Both following are set when ADB is initialized.
+# _ADB_KEYS is set to a list of PythonRSASigner instances. It contains one or
+# multiple key used to authenticate to Android debug protocol (adb).
 _ADB_KEYS = None
+# _ADB_KEYS_RAW is set to a dict(pub: priv) when initialized, with the raw
+# content of each key.
+_ADB_KEYS_RAW = None
 
 
 # Cache of /system/build.prop on Android devices connected to this host.
 _BUILD_PROP_ANDROID = {}
 
 
 def _dumpsys(device, arg):
   """dumpsys is a native android tool that returns semi structured data.
 
   It acts as a directory service but each service return their data without any
@@ skipping 18 matching lines @@
       group = match.group(i)
       char = group[4:8]
       if char != '    ':
         out.append(char)
       char = group[0:4]
       if char != '    ':
         out.append(char)
   return out
 
 
-class _Device(object):
+class Device(object):
   """Wraps an AdbCommands to make it exception safe.
 
   The fact that exceptions can be thrown any time makes the client code really
   hard to write safely. Convert USBError* to None return value.
   """
   def __init__(self, adb_cmd, port_path):
     if adb_cmd:
       assert isinstance(adb_cmd, adb.adb_commands.AdbCommands), adb_cmd
     self.adb_cmd = adb_cmd
     # Try to get the real serial number if possible.
@@ skipping 73 matching lines @@
     assert out[-1] == '\n', out
     # Strip the last line to extract the exit code.
     parts = out[:-1].rsplit('\n', 1)
     if len(parts) == 2:
       return parts[0] + '\n', int(parts[1])
     # The command itself generated no output.
     return '', int(parts[0])
 
 
 def _load_device(handle):
-  """Given a adb.USBDevice, return a _Device wrapping an
+  """Given a adb.USBDevice, return a Device wrapping an
   adb_commands.AdbCommands.
   """
   port_path = '/'.join(map(str, handle.port_path))
   try:
     handle.Open()
   except adb.common.usb1.USBErrorNoDevice as e:
     logging.warning('Got USBErrorNoDevice for %s: %s', port_path, e)
     # Do not kill adb, it just means the USB host is likely resetting and the
     # device is temporarily unavailable.We can't use handle.serial_number since
     # this communicates with the device.
@@ skipping 32 matching lines @@
       logging.warning('AUTH FAILURE: %s: %s', port_path, e)
     except adb.usb_exceptions.ReadFailedError as e:
       logging.warning('READ FAILURE: %s: %s', port_path, e)
     except adb.usb_exceptions.WriteFailedError as e:
       logging.warning('WRITE FAILURE: %s: %s', port_path, e)
     except ValueError as e:
       # This is generated when there's a protocol level failure, e.g. the data
       # is garbled.
       logging.warning(
           'Trying unpluging and pluging it back: %s: %s', port_path, e)
-  # Always create a _Device, even if it points to nothing. It makes using the
+  # Always create a Device, even if it points to nothing. It makes using the
   # client code much easier.
-  return _Device(cmd, port_path)
+  return Device(cmd, port_path)
 
 
 ### Public API.
 
 
 def initialize(pub_key, priv_key):
   """Initialize Android support through adb.
 
   You can steal pub_key, priv_key pair from ~/.android/adbkey and
   ~/.android/adbkey.pub.
   """
   global _ADB_KEYS
+  global _ADB_KEYS_RAW
+  if _ADB_KEYS is not None:
+    assert False, 'initialize() was called repeatedly: ignoring keys'
   assert bool(pub_key) == bool(priv_key)
-  if _ADB_KEYS is None:
-    _ADB_KEYS = []
-    if pub_key:
-      try:
-        _ADB_KEYS.append(PythonRSASigner(pub_key, priv_key))
-      except ValueError as exc:
-        logging.warning('Ignoring adb private key: %s', exc)
+  pub_key = pub_key.strip() if pub_key else pub_key
+  priv_key = priv_key.strip() if priv_key else priv_key
 
-    # Try to add local adb keys if available.
-    path = os.path.expanduser('~/.android/adbkey')
-    if os.path.isfile(path) and os.path.isfile(path+'.pub'):
-      with open(path + '.pub', 'rb') as f:
-        pub = f.read()
-      with open(path, 'rb') as f:
-        priv = f.read()
-      try:
-        _ADB_KEYS.append(PythonRSASigner(pub, priv))
-      except ValueError as exc:
-        logging.warning('Ignoring adb private key %s: %s', path, exc)
+  _ADB_KEYS = []
+  _ADB_KEYS_RAW = {}
+  if pub_key:
+    _ADB_KEYS.append(PythonRSASigner(pub_key, priv_key))
+    _ADB_KEYS_RAW[pub_key] = priv_key
 
-    if not _ADB_KEYS:
-      return False
-  else:
-    if pub_key:
-      logging.warning('initialize() was called repeatedly: ignoring keys')
+  # Try to add local adb keys if available.
+  path = os.path.expanduser('~/.android/adbkey')
+  if os.path.isfile(path) and os.path.isfile(path + '.pub'):
+    with open(path + '.pub', 'rb') as f:
+      pub = f.read().strip()
+    with open(path, 'rb') as f:
+      priv = f.read().strip()
+    _ADB_KEYS.append(PythonRSASigner(pub, priv))
+    _ADB_KEYS_RAW[pub] = priv
+
   return bool(_ADB_KEYS)
 
 
 class PythonRSASigner(object):
   """Implements adb_protocol.AuthSigner using http://stuvel.eu/rsa."""
   def __init__(self, pub, priv):
     self.priv_key = load_rsa_private_key(priv)
     self.pub_key = pub
 
   def Sign(self, data):
@@ skipping 14 matching lines @@
     keyinfo, _ = decoder.decode(der)
     if keyinfo[1][0] != univ.ObjectIdentifier('1.2.840.113549.1.1.1'):
         raise ValueError('Not a DER-encoded OpenSSL private RSA key')
     private_key_der = keyinfo[2].asOctets()
   except IndexError:
     raise ValueError('Not a DER-encoded OpenSSL private RSA key')
   return rsa.PrivateKey.load_pkcs1(private_key_der, format='DER')
 
 
 def kill_adb():
-  """adb sucks. Kill it with fire."""
+  """Stops the adb daemon.
+
+  The Swarming bot doesn't use the Android's SDK adb. It uses python-adb to
+  directly communicate with the devices. This works much better when a lot of
+  devices are connected to the host.
+
+  adb's stability is less than stellar. Kill it with fire.
+  """
   if not adb:
     return
-  try:
-    subprocess.call(['adb', 'kill-server'])
-  except OSError:
-    pass
-  subprocess.call(['killall', '--exact', 'adb'])
+  while True:
+    try:
+      subprocess.check_output(['pgrep', 'adb'])
+    except subprocess.CalledProcessError:
+      return
+    try:
+      subprocess.call(
+          ['adb', 'kill-server'],
+          stdout=subprocess.PIPE, stderr=subprocess.STDOUT)
+    except OSError:
+      pass
+    subprocess.call(
+        ['killall', '--exact', 'adb'],
+        stdout=subprocess.PIPE, stderr=subprocess.STDOUT)
 
 
-def get_devices():
+def get_devices(bot=None):  # pylint: disable=unused-argument

[ghost stip (do not use), 2015/10/08 20:13:28]

?

   """Returns the list of devices available.
 
   Caller MUST call close_devices(devices) on the return value.
 
   Returns one of:
     - dict of {serial_number: }. The value may be
       None if there was an Auth failure.
     - None if adb is unavailable.
   """
   # TODO(maruel): Return a list instead of a dict. It's a bit tricky as we need
@@ skipping 26 matching lines @@
   return devices
 
 
 def close_devices(devices):
   """Closes all devices opened by get_devices()."""
   for _, device in sorted((devices or {}).iteritems()):
     device.close()
 
 
 class CpuScalingGovernor(object):
-  """List of valid CPU scaling governor values."""
-  ON_DEMAND = 'ondemand'
+  """List of known CPU scaling governor values.
+  """
+  CONSERVATIVE = 'conservative'  # Not available on Nexus 10
+  INTERACTIVE = 'interactive'  # Default on Nexus 10
+  ON_DEMAND = 'ondemand'  # Not available on Nexus 10. Default on Nexus 4
   PERFORMANCE = 'performance'
-  CONSERVATIVE = 'conservative'
-  POWER_SAVE = 'powersave'
+  POWER_SAVE = 'powersave'  # Not available on Nexus 10
   USER_DEFINED = 'userspace'
 
   @classmethod
   def is_valid(cls, name):
-    return name in [getattr(cls, m) for m in dir(cls) if m[0].isupper()]
+    return name in cls.all_values()
+
+  @classmethod
+  def all_values(cls):

[ghost stip (do not use), 2015/10/08 20:13:28]

I don't know why you're doing all this. why not just use the string values
directly?

+    return sorted(getattr(cls, m) for m in dir(cls) if m[0].isupper())
 
 
 def set_cpu_scaling_governor(device, governor):
   """Sets the CPU scaling governor to the one specified.
 
   Returns:
     True on success.
   """
-  assert isinstance(device, _Device), device
+  assert isinstance(device, Device), device
   assert CpuScalingGovernor.is_valid(governor), governor
   _, exit_code = device.shell(
       'echo "%s">/sys/devices/system/cpu/cpu0/cpufreq/scaling_governor' %
       governor)
   return exit_code == 0
 
 
 def set_cpu_scaling(device, speed):
   """Enforces strict CPU speed and disable the CPU scaling governor.
 
   Returns:
     True on success.
   """
-  assert isinstance(device, _Device), device
+  assert isinstance(device, Device), device
   assert isinstance(speed, int), speed
   assert 10000 <= speed <= 10000000, speed
   success = set_cpu_scaling_governor(device, CpuScalingGovernor.USER_DEFINED)
   _, exit_code = device.shell(
       'echo "%d">/sys/devices/system/cpu/cpu0/cpufreq/scaling_setspeed' %
       speed)
   return success and exit_code == 0
 
 
 def get_build_prop(device):
@@ skipping 12 matching lines @@
         if line.startswith(u'#') or not line:
           continue
         key, value = line.split(u'=', 1)
         properties[key] = value
     _BUILD_PROP_ANDROID[device.serial] = properties
   return _BUILD_PROP_ANDROID[device.serial]
 
 
 def get_temp(device):
   """Returns the device's 2 temperatures if available."""
+  assert isinstance(device, Device), device
   # Not all devices export this file. On other devices, the only real way to
   # read it is via Java
   # developer.android.com/guide/topics/sensors/sensors_environment.html
   temps = []
   for i in xrange(2):
     out, _ = device.shell('cat /sys/class/thermal/thermal_zone%d/temp' % i)
     if out:
-      temps.append(int(out))
+      try:
+        temps.append(int(out))
+      except ValueError:
+        pass
   return temps
 
 
 def get_battery(device):
   """Returns details about the battery's state."""
+  assert isinstance(device, Device), device
   props = {}
   out = _dumpsys(device, 'battery')
   if not out:
     return props
   for line in out.splitlines():
     if line.endswith(u':'):
       continue
     # On Android 4.1.2, it uses "voltage:123" instead of "voltage: 123".
-    key, value = line.split(u':', 2)
-    props[key.lstrip()] = value.strip()
+    parts = line.split(u':', 2)
+    if len(parts) == 2:
+      key, value = parts
+      props[key.lstrip()] = value.strip()
   out = {u'power': []}
-  if props[u'AC powered'] == u'true':
+  if props.get(u'AC powered') == u'true':
     out[u'power'].append(u'AC')
-  if props[u'USB powered'] == u'true':
+  if props.get(u'USB powered') == u'true':
     out[u'power'].append(u'USB')
   if props.get(u'Wireless powered') == u'true':
     out[u'power'].append(u'Wireless')
   for key in (u'health', u'level', u'status', u'temperature', u'voltage'):
     out[key] = int(props[key])
   return out
 
 
 def get_cpu_scale(device):
   """Returns the CPU scaling factor."""
+  assert isinstance(device, Device), device
   mapping = {
     'cpuinfo_max_freq': u'max',
     'cpuinfo_min_freq': u'min',
     'scaling_cur_freq': u'cur',
     'scaling_governor': u'governor',
   }
   return {
     v: device.shell('cat /sys/devices/system/cpu/cpu0/cpufreq/' + k)[0]
     for k, v in mapping.iteritems()
   }
 
 
 def get_uptime(device):
   """Returns the device's uptime in second."""
+  assert isinstance(device, Device), device
   out, _ = device.shell('cat /proc/uptime')
   if out:
     return float(out.split()[1])
   return None
 
 
 def get_disk(device):
   """Returns details about the battery's state."""
+  assert isinstance(device, Device), device
   props = {}
   out = _dumpsys(device, 'diskstats')
   if not out:
     return props
   for line in out.splitlines():
     if line.endswith(u':'):
       continue
-    key, value = line.split(u': ', 2)
-    match = re.match(u'^(\d+)K / (\d+)K.*', value)
-    if match:
-      props[key.lstrip()] = {
-        'free_mb': round(float(match.group(1)) / 1024., 1),
-        'size_mb': round(float(match.group(2)) / 1024., 1),
-      }
+    parts = line.split(u': ', 2)

[ghost stip (do not use), 2015/10/08 20:13:28]

I would have preferred this to be a separate CL to make it easier to review

+    if len(parts) == 2:
+      key, value = parts
+      match = re.match(u'^(\d+)K / (\d+)K.*', value)
+      if match:
+        props[key.lstrip()] = {
+          'free_mb': round(float(match.group(1)) / 1024., 1),
+          'size_mb': round(float(match.group(2)) / 1024., 1),
+        }
   return {
     u'cache': props[u'Cache-Free'],
     u'data': props[u'Data-Free'],
     u'system': props[u'System-Free'],
   }
 
 
 def get_imei(device):
   """Returns the phone's IMEI."""
+  assert isinstance(device, Device), device
   # Android <5.0.
   out = _dumpsys(device, 'iphonesubinfo')
   if out:
     match = re.search('  Device ID = (.+)$', out)
     if match:
       return match.group(1)
 
   # Android >= 5.0.
   out, _ = device.shell('service call iphonesubinfo 1')
   if out:
     lines = out.splitlines()
     if len(lines) >= 4 and lines[0] == 'Result: Parcel(':
       # Process the UTF-16 string.
       chars = _parcel_to_list(lines[1:])[4:-1]
       return u''.join(map(unichr, (int(i, 16) for i in chars)))
   return None
 
 
 def get_ip(device):
   """Returns the IP address."""
+  assert isinstance(device, Device), device
   # If ever needed, read wifi.interface from /system/build.prop if a device
   # doesn't use wlan0.
   ip, _ = device.shell('getprop dhcp.wlan0.ipaddress')
   if not ip:
     return None
   return ip.strip()