Check if mantarays are charging, and alert on all Android device errors.

build/android/device_status_check.py

Index: build/android/device_status_check.py
diff --git a/build/android/device_status_check.py b/build/android/device_status_check.py
index 5f78ddce3d87a2d3778c5748c355b84ffd52e745..678540bc2378275021343910051899f42d6ada09 100755
--- a/build/android/device_status_check.py
+++ b/build/android/device_status_check.py
@@ -33,14 +33,17 @@ def DeviceInfo(serial):
 
   device_type = AdbShellCmd('getprop ro.build.product')
   device_build = AdbShellCmd('getprop ro.build.id')
+  device_product_name = AdbShellCmd('getprop ro.product.name')
 
   setup_wizard_disabled = AdbShellCmd(
       'getprop ro.setupwizard.mode') == 'DISABLED'
   battery = AdbShellCmd('dumpsys battery')
   if 'Error' in battery:
+    ac_power = 'Unknown'
     battery_level = 'Unknown'
     battery_temp = 'Unknown'
   else:
+    ac_power = re.findall('AC powered: (\w+)', battery)[0]
     battery_level = int(re.findall('level: (\d+)', battery)[0])
     battery_temp = float(re.findall('temperature: (\d+)', battery)[0]) / 10
   report = ['Device %s (%s)' % (serial, device_type),
@@ -54,13 +57,15 @@ def DeviceInfo(serial):
             '  Wifi IP: %s' % AdbShellCmd('getprop dhcp.wlan0.ipaddress'),
             '']
 
-  warnings = []
+  errors = []
   if battery_level < 5:
-    warnings += ['critically low battery']
+    errors += ['Device critically low in battery.']
   if not setup_wizard_disabled:
-    warnings += ['Setup wizard not disabled. Was it provisioned correctly?']
+    errors += ['Setup wizard not disabled. Was it provisioned correctly?']
+  if device_product_name == 'mantaray' and ac_power != 'true':
+    errors += ['Mantaray device not connected to AC power.']
 
-  return device_type, device_build, '\n'.join(report), warnings
+  return device_type, device_build, '\n'.join(report), errors
 
 
 def CheckForMissingDevices(options, adb_online_devs):
@@ -100,24 +105,18 @@ def CheckForMissingDevices(options, adb_online_devs):
 
   last_devices_path = os.path.join(out_dir, '.last_devices')
   last_devices = ReadDeviceList('.last_devices')
-
   missing_devs = list(set(last_devices) - set(adb_online_devs))
+
+  WriteDeviceList('.last_devices', (adb_online_devs + last_devices))
+  WriteDeviceList('.last_missing', missing_devs)
+
   if missing_devs:
-    from_address = 'buildbot@chromium.org'
-    to_address = 'chromium-android-device-alerts@google.com'
-    bot_name = os.environ.get('BUILDBOT_BUILDERNAME')
-    slave_name = os.environ.get('BUILDBOT_SLAVENAME')
-    num_online_devs = len(adb_online_devs)
-    subject = 'Devices offline on %s, %s (%d remaining).' % (slave_name,
-                                                             bot_name,
-                                                             num_online_devs)
-    buildbot_report.PrintWarning()
     devices_missing_msg = '%d devices not detected.' % len(missing_devs)
     buildbot_report.PrintSummaryText(devices_missing_msg)
 
     # TODO(navabi): Debug by printing both output from GetCmdOutput and
     # GetAttachedDevices to compare results.
-    body = '\n'.join(
+    err = '\n'.join(

[Isaac (away), 2013/03/13 02:54:11]

Return this string directly, instead of making a One-Time-Used variable.

[Siva Chandra, 2013/03/13 20:58:15]

Done.

         ['Current online devices: %s' % adb_online_devs,
          '%s are no longer visible. Were they removed?\n' % missing_devs,
          'SHERIFF: See go/chrome_device_monitor',
@@ -126,24 +125,7 @@ def CheckForMissingDevices(options, adb_online_devs):
          'adb devices(GetAttachedDevices): %s' %
          android_commands.GetAttachedDevices()])
 
-    print body
-
-    # Only send email if the first time a particular device goes offline
-    last_missing = ReadDeviceList('.last_missing')
-    new_missing_devs = set(missing_devs) - set(last_missing)
-
-    if new_missing_devs and bot_name:
-      msg_body = '\r\n'.join(
-          ['From: %s' % from_address,
-           'To: %s' % to_address,
-           'Subject: %s' % subject,
-           '', body])
-      try:
-        server = smtplib.SMTP('localhost')
-        server.sendmail(from_address, [to_address], msg_body)
-        server.quit()
-      except Exception as e:
-        print 'Failed to send alert email. Error: %s' % e
+    return err
   else:
     new_devs = set(adb_online_devs) - set(last_devices)
     if new_devs and os.path.exists(last_devices_path):
@@ -152,8 +134,22 @@ def CheckForMissingDevices(options, adb_online_devs):
           '%d new devices detected' % len(new_devs))
       print ('New devices detected %s. And now back to your '
              'regularly scheduled program.' % list(new_devs))
-  WriteDeviceList('.last_devices', (adb_online_devs + last_devices))
-  WriteDeviceList('.last_missing', missing_devs)
+
+
+def SendDeviceStatusAlert(msg):
+  from_address = 'buildbot@chromium.org'
+  to_address = 'chromium-android-device-alerts@google.com'
+  bot_name = os.environ.get('BUILDBOT_BUILDERNAME')
+  slave_name = os.environ.get('BUILDBOT_SLAVENAME')
+  subject = 'Device status check errors on %s, %s.' % (slave_name, bot_name)
+  msg_body = '\r\n'.join(['From: %s' % from_address, 'To: %s' % to_address,
+                          'Subject: %s' % subject, '', msg])
+  try:
+    server = smtplib.SMTP('localhost')
+    server.sendmail(from_address, [to_address], msg_body)
+    server.quit()
+  except Exception as e:
+    print 'Failed to send alert email. Error: %s' % e
 
 
 def main():
@@ -171,6 +167,13 @@ def main():
   if devices:
     types, builds, reports, errors = zip(*[DeviceInfo(dev) for dev in devices])
 
+  missing_devices_msg = CheckForMissingDevices(options, devices)

[Isaac (away), 2013/03/13 02:54:11]

We don't need two variables (missing_devices_msg, deivce_info_err_msg).  Just
keep one.  error_msg.  You can then delete lines 171-176.

[Siva Chandra, 2013/03/13 20:58:15]

There are two variables because one of them should be preceded with a
STEP_FAILURE, and the other one with a warning. We are treating only missing
devices as a failure. All other errors will be STEP_WARNINGS, and will send out
an alert message, but will not fail the step/flunk the build.

Though I have not done what you suggested, I re-did a bit to make it look
better.

[Isaac (away), 2013/03/13 21:13:49]

Since we have turned on flunk_on_failure, the only information here you need is
a non-zero error code.  Let's save the error code to a separate variable and
combine the strings.

[Siva Chandra, 2013/03/13 22:22:57]

Done.

+  if missing_devices_msg:
+    buildbot_report.PrintError()

[Isaac (away), 2013/03/13 21:13:49]

remove this -- not needed w/ non-zero error code.

[Siva Chandra, 2013/03/13 22:22:57]

Done.

+    print missing_devices_msg
+  else:
+    missing_devices_msg = ''
+
   unique_types = list(set(types))
   unique_builds = list(set(builds))
 
@@ -179,13 +182,23 @@ def main():
   print '\n'.join(reports)
 
   full_errors = []
-  for serial, device_errors in zip(devices, errors):
-    full_errors.extend('%s: %s' % (serial, error) for error in device_errors)
+  device_info_err_msg = ''
+  for serial, dev_errors in zip(devices, errors):
+    if dev_errors:

[Isaac (away), 2013/03/13 02:54:11]

what is the purpose of this change?

[Siva Chandra, 2013/03/13 20:58:15]

We want to add to full_errors only if there were dev_errors. The immediate next
line below, which is adding something like a section header, requires that. The
line after that does not.

[Isaac (away), 2013/03/13 21:13:49]

This code doesn't make sense to me.  Can you explain more about what your'e
trying to do here?  I.e. what are you trying to print?

[Siva Chandra, 2013/03/13 22:22:57]

Since we are now using only err_msg, it looks slightly different, but the point
is this. I want to print errors this way:

<serial for device 1> errors:
        <error 1>
        <error 2>
<serial for device 2> errors:
        <error 1>
        <error 2>

So, the line which prints "<serial for device> errors:" should be executed only
if there were dev_errors for that device. That is why I have the if statement.

+      full_errors += ['%s errors:' % serial]
+      full_errors.extend('\t%s' % error for error in dev_errors)
+
   if full_errors:
     buildbot_report.PrintWarning()
-    print '\n'.join(full_errors)
+    device_info_err_msg = '\n'.join(full_errors)
+    print device_info_err_msg
+
+  if missing_devices_msg or device_info_err_msg:
+    SendDeviceStatusAlert(missing_devices_msg + device_info_err_msg)
+
+  if missing_devices_msg:
+    return 1
 
-  CheckForMissingDevices(options, devices)
 
 if __name__ == '__main__':
   sys.exit(main())