Use new IDevice.getBattery method

Don't block on getting battery level when listing devices or
scheduling commands.

Bug: 17004834

Change-Id: Iee4e2ab2e5fef854f930bc85046e776b46a374f2

8 files changed, 68 insertions, 63 deletions

diff --git a/src/com/android/tradefed/device/DeviceSelectionOptions.java b/src/com/android/tradefed/device/DeviceSelectionOptions.java
index 375c4bda8..64762956a 100644
--- a/src/com/android/tradefed/device/DeviceSelectionOptions.java
+++ b/src/com/android/tradefed/device/DeviceSelectionOptions.java
@@ -15,28 +15,25 @@
  */
 package com.android.tradefed.device;
 
-import com.android.ddmlib.AdbCommandRejectedException;
 import com.android.ddmlib.IDevice;
-import com.android.ddmlib.ShellCommandUnresponsiveException;
-import com.android.ddmlib.TimeoutException;
 import com.android.tradefed.config.Option;
 import com.android.tradefed.log.LogUtil.CLog;
 
-import java.io.IOException;
 import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.Collection;
 import java.util.HashMap;
 import java.util.HashSet;
 import java.util.Map;
+import java.util.concurrent.ExecutionException;
+import java.util.concurrent.Future;
+import java.util.concurrent.TimeUnit;
 
 /**
  * Container for for device selection criteria.
  */
 public class DeviceSelectionOptions implements IDeviceSelection {
 
-    private static final String LOG_TAG = "DeviceSelectionOptions";
-
     @Option(name = "serial", shortName = 's', description =
         "run this test on a specific device with given serial number(s).")
     private Collection<String> mSerials = new ArrayList<String>();
@@ -437,15 +434,14 @@ public class DeviceSelectionOptions implements IDeviceSelection {
     @Override
     public Integer getBatteryLevel(IDevice device) {
         try {
-            return device.getBatteryLevel();
-        } catch (TimeoutException e) {
-            handleBatteryException(device, e);
-        } catch (AdbCommandRejectedException e) {
-            handleBatteryException(device, e);
-        } catch (IOException e) {
-            handleBatteryException(device, e);
-        } catch (ShellCommandUnresponsiveException e) {
-            handleBatteryException(device, e);
+            // use default 5 minutes freshness
+            Future<Integer> batteryFuture = device.getBattery();
+            // don't block on battery level, get cached value
+            return batteryFuture.get(1, TimeUnit.MILLISECONDS);
+        } catch (InterruptedException | ExecutionException |
+                java.util.concurrent.TimeoutException e) {
+            CLog.w("Failed to query battery level for %s: %s", device.getSerialNumber(),
+                    e.toString());
         }
         return null;
     }
@@ -466,10 +462,6 @@ public class DeviceSelectionOptions implements IDeviceSelection {
         return apiLevel;
     }
 
-    private void handleBatteryException(IDevice device, Exception e) {
-        CLog.w("Failed to query battery level for %s: %s", device.getSerialNumber(), e.toString());
-    }
-
     /**
      * Helper factory method to create a {@link IDeviceSelection} that will only match device
      * with given serial
diff --git a/src/com/android/tradefed/device/StubDevice.java b/src/com/android/tradefed/device/StubDevice.java
index 3e692c5a9..6dc783bc3 100644
--- a/src/com/android/tradefed/device/StubDevice.java
+++ b/src/com/android/tradefed/device/StubDevice.java
@@ -433,6 +433,9 @@ public class StubDevice implements IDevice {
         return true;
     }
 
+    /**
+     * {@inheritDoc}
+     */
     @Override
     public Future<String> getSystemProperty(String name) {
         SettableFuture<String> f = SettableFuture.create();
@@ -440,6 +443,9 @@ public class StubDevice implements IDevice {
         return f;
     }
 
+    /**
+     * {@inheritDoc}
+     */
     @Override
     public Future<Integer> getBattery() {
         SettableFuture<Integer> f = SettableFuture.create();
@@ -447,10 +453,11 @@ public class StubDevice implements IDevice {
         return f;
     }
 
+    /**
+     * {@inheritDoc}
+     */
     @Override
-    public Future<Integer> getBattery(long arg0, TimeUnit arg1) {
-        SettableFuture<Integer> f = SettableFuture.create();
-        f.set(0);
-        return f;
+    public Future<Integer> getBattery(long freshnessTime, TimeUnit timeUnit) {
+        return getBattery();
     }
 }
diff --git a/src/com/android/tradefed/device/TestDevice.java b/src/com/android/tradefed/device/TestDevice.java
index 9df9e76b9..e075343ac 100644
--- a/src/com/android/tradefed/device/TestDevice.java
+++ b/src/com/android/tradefed/device/TestDevice.java
@@ -330,7 +330,22 @@ class TestDevice implements IManagedTestDevice {
                 try {
                     result[0] = getIDevice().getSystemProperty(name).get();
                 } catch (InterruptedException | ExecutionException e) {
-                    throw new IOException(e);
+                    // getProperty will stash the original exception inside
+                    // ExecutionException.getCause
+                    // throw the specific original exception if available in case TF ever does
+                    // specific handling for different exceptions
+                    if (e.getCause() instanceof IOException) {
+                        throw (IOException)e.getCause();
+                    } else if (e.getCause() instanceof TimeoutException) {
+                        throw (TimeoutException)e.getCause();
+                    } else if (e.getCause() instanceof AdbCommandRejectedException) {
+                        throw (AdbCommandRejectedException)e.getCause();
+                    } else if (e.getCause() instanceof ShellCommandUnresponsiveException) {
+                        throw (ShellCommandUnresponsiveException)e.getCause();
+                    }
+                    else {
+                        throw new IOException(e);
+                    }
                 }
                 return true;
             }
diff --git a/src/com/android/tradefed/device/WaitDeviceRecovery.java b/src/com/android/tradefed/device/WaitDeviceRecovery.java
index fc6a8f56b..823b2aa79 100644
--- a/src/com/android/tradefed/device/WaitDeviceRecovery.java
+++ b/src/com/android/tradefed/device/WaitDeviceRecovery.java
@@ -18,7 +18,6 @@ package com.android.tradefed.device;
 import com.android.ddmlib.AdbCommandRejectedException;
 import com.android.ddmlib.IDevice;
 import com.android.ddmlib.Log;
-import com.android.ddmlib.ShellCommandUnresponsiveException;
 import com.android.ddmlib.TimeoutException;
 import com.android.tradefed.config.Option;
 import com.android.tradefed.log.LogUtil.CLog;
@@ -26,6 +25,7 @@ import com.android.tradefed.util.IRunUtil;
 import com.android.tradefed.util.RunUtil;
 
 import java.io.IOException;
+import java.util.concurrent.ExecutionException;
 
 /**
  * A simple implementation of a {@link IDeviceRecovery} that waits for device to be online and
@@ -149,7 +149,7 @@ public class WaitDeviceRecovery implements IDeviceRecovery {
             return;
         }
         try {
-            Integer level = device.getBatteryLevel();
+            Integer level = device.getBattery().get();
             if (level == null) {
                 // can't read battery level but we are requiring a min, reject
                 // device
@@ -161,13 +161,7 @@ public class WaitDeviceRecovery implements IDeviceRecovery {
                         level, mRequiredMinBattery));
             }
             return;
-        } catch (TimeoutException e) {
-            throw new DeviceNotAvailableException("exception while reading battery level", e);
-        } catch (AdbCommandRejectedException e) {
-            throw new DeviceNotAvailableException("exception while reading battery level", e);
-        } catch (IOException e) {
-            throw new DeviceNotAvailableException("exception while reading battery level", e);
-        } catch (ShellCommandUnresponsiveException e) {
+        } catch (InterruptedException | ExecutionException e) {
             throw new DeviceNotAvailableException("exception while reading battery level", e);
         }
     }
diff --git a/src/com/android/tradefed/invoker/TestInvocation.java b/src/com/android/tradefed/invoker/TestInvocation.java
index 2fff91a8f..02d9740a2 100644
--- a/src/com/android/tradefed/invoker/TestInvocation.java
+++ b/src/com/android/tradefed/invoker/TestInvocation.java
@@ -15,11 +15,8 @@
  */
 package com.android.tradefed.invoker;
 
-import com.android.ddmlib.AdbCommandRejectedException;
 import com.android.ddmlib.IDevice;
 import com.android.ddmlib.Log.LogLevel;
-import com.android.ddmlib.ShellCommandUnresponsiveException;
-import com.android.ddmlib.TimeoutException;
 import com.android.tradefed.build.BuildRetrievalError;
 import com.android.tradefed.build.ExistingBuildProvider;
 import com.android.tradefed.build.IBuildInfo;
@@ -61,6 +58,8 @@ import java.util.Arrays;
 import java.util.Collection;
 import java.util.List;
 import java.util.ListIterator;
+import java.util.concurrent.ExecutionException;
+import java.util.concurrent.TimeUnit;
 
 /**
  * Default implementation of {@link ITestInvocation}.
@@ -703,17 +702,13 @@ public class TestInvocation implements ITestInvocation {
             return;
         }
         try {
-            CLog.v("%s - %s - %d%%", BATT_TAG, event, device.getBatteryLevel(0));
+            CLog.v("%s - %s - %d%%", BATT_TAG, event,
+                    device.getBattery(0, TimeUnit.MILLISECONDS).get());
             return;
-        } catch (TimeoutException e) {
-            // fall through
-        } catch (AdbCommandRejectedException e) {
-            // fall through
-        } catch (IOException e) {
-            // fall through
-        } catch (ShellCommandUnresponsiveException e) {
+        } catch (InterruptedException | ExecutionException e) {
             // fall through
         }
+
         CLog.v("Failed to get battery level");
     }
 }
diff --git a/src/com/android/tradefed/testtype/DeviceBatteryLevelChecker.java b/src/com/android/tradefed/testtype/DeviceBatteryLevelChecker.java
index c390edae7..15018baab 100644
--- a/src/com/android/tradefed/testtype/DeviceBatteryLevelChecker.java
+++ b/src/com/android/tradefed/testtype/DeviceBatteryLevelChecker.java
@@ -16,10 +16,7 @@
 
 package com.android.tradefed.testtype;
 
-import com.android.ddmlib.AdbCommandRejectedException;
 import com.android.ddmlib.IDevice;
-import com.android.ddmlib.ShellCommandUnresponsiveException;
-import com.android.ddmlib.TimeoutException;
 import com.android.tradefed.config.Option;
 import com.android.tradefed.config.OptionClass;
 import com.android.tradefed.device.DeviceNotAvailableException;
@@ -33,7 +30,8 @@ import com.android.tradefed.util.RunUtil;
 
 import junit.framework.Assert;
 
-import java.io.IOException;
+import java.util.concurrent.ExecutionException;
+import java.util.concurrent.TimeUnit;
 
 /**
  * An {@link ITargetPreparer} that checks for a minimum battery charge, and waits for the battery
@@ -77,14 +75,8 @@ public class DeviceBatteryLevelChecker implements IDeviceTest, IRemoteTest {
         try {
             IDevice idevice = device.getIDevice();
             // Force a synchronous check, which will also tell us if the device is still alive
-            return idevice.getBatteryLevel(0);
-        } catch (AdbCommandRejectedException e) {
-            return null;
-        } catch (IOException e) {
-            return null;
-        } catch (TimeoutException e) {
-            return null;
-        } catch (ShellCommandUnresponsiveException e) {
+            return idevice.getBattery(0, TimeUnit.MILLISECONDS).get();
+        } catch (InterruptedException | ExecutionException e) {
             return null;
         }
     }
diff --git a/tests/src/com/android/tradefed/device/DeviceSelectionOptionsTest.java b/tests/src/com/android/tradefed/device/DeviceSelectionOptionsTest.java
index b50b9e54f..43d44f76e 100644
--- a/tests/src/com/android/tradefed/device/DeviceSelectionOptionsTest.java
+++ b/tests/src/com/android/tradefed/device/DeviceSelectionOptionsTest.java
@@ -17,6 +17,7 @@ package com.android.tradefed.device;
 
 import com.android.ddmlib.IDevice;
 import com.android.tradefed.config.ArgsOptionParser;
+import com.google.common.util.concurrent.SettableFuture;
 
 import junit.framework.TestCase;
 
@@ -268,7 +269,7 @@ public class DeviceSelectionOptionsTest extends TestCase {
     public void testMatches_minBatteryPass() throws Exception {
         DeviceSelectionOptions options = new DeviceSelectionOptions();
         options.setMinBatteryLevel(25);
-        EasyMock.expect(mMockDevice.getBatteryLevel()).andStubReturn(50);
+        mockBatteryCheck(50);
         EasyMock.replay(mMockDevice, mMockEmulatorDevice);
         assertTrue(options.matches(mMockDevice));
     }
@@ -279,7 +280,7 @@ public class DeviceSelectionOptionsTest extends TestCase {
     public void testMatches_minBatteryFail() throws Exception {
         DeviceSelectionOptions options = new DeviceSelectionOptions();
         options.setMinBatteryLevel(75);
-        EasyMock.expect(mMockDevice.getBatteryLevel()).andStubReturn(50);
+        mockBatteryCheck(50);
         EasyMock.replay(mMockDevice, mMockEmulatorDevice);
         assertFalse(options.matches(mMockDevice));
     }
@@ -290,7 +291,7 @@ public class DeviceSelectionOptionsTest extends TestCase {
     public void testMatches_maxBatteryPass() throws Exception {
         DeviceSelectionOptions options = new DeviceSelectionOptions();
         options.setMaxBatteryLevel(75);
-        EasyMock.expect(mMockDevice.getBatteryLevel()).andStubReturn(50);
+        mockBatteryCheck(50);
         EasyMock.replay(mMockDevice, mMockEmulatorDevice);
         assertTrue(options.matches(mMockDevice));
     }
@@ -301,7 +302,7 @@ public class DeviceSelectionOptionsTest extends TestCase {
     public void testMatches_maxBatteryFail() throws Exception {
         DeviceSelectionOptions options = new DeviceSelectionOptions();
         options.setMaxBatteryLevel(25);
-        EasyMock.expect(mMockDevice.getBatteryLevel()).andStubReturn(50);
+        mockBatteryCheck(50);
         EasyMock.replay(mMockDevice, mMockEmulatorDevice);
         assertFalse(options.matches(mMockDevice));
     }
@@ -312,7 +313,7 @@ public class DeviceSelectionOptionsTest extends TestCase {
     public void testMatches_forceBatteryCheckTrue() throws Exception {
         DeviceSelectionOptions options = new DeviceSelectionOptions();
         options.setRequireBatteryCheck(true);
-        EasyMock.expect(mMockDevice.getBatteryLevel()).andStubReturn(null);
+        mockBatteryCheck(null);
         EasyMock.replay(mMockDevice, mMockEmulatorDevice);
         assertTrue(options.matches(mMockDevice));
         options.setMinBatteryLevel(25);
@@ -325,7 +326,7 @@ public class DeviceSelectionOptionsTest extends TestCase {
     public void testMatches_forceBatteryCheckFalse() throws Exception {
         DeviceSelectionOptions options = new DeviceSelectionOptions();
         options.setRequireBatteryCheck(false);
-        EasyMock.expect(mMockDevice.getBatteryLevel()).andStubReturn(null);
+        mockBatteryCheck(null);
         EasyMock.replay(mMockDevice, mMockEmulatorDevice);
         assertTrue(options.matches(mMockDevice));
         options.setMinBatteryLevel(25);
@@ -373,4 +374,10 @@ public class DeviceSelectionOptionsTest extends TestCase {
         EasyMock.replay(mMockDevice, mMockEmulatorDevice);
         assertFalse(options.matches(mMockDevice));
     }
+
+    private void mockBatteryCheck(Integer battery) {
+        SettableFuture<Integer> batteryFuture = SettableFuture.create();
+        batteryFuture.set(battery);
+        EasyMock.expect(mMockDevice.getBattery()).andStubReturn(batteryFuture);
+    }
 }
diff --git a/tests/src/com/android/tradefed/testtype/DeviceBatteryLevelCheckerTest.java b/tests/src/com/android/tradefed/testtype/DeviceBatteryLevelCheckerTest.java
index 89698efbb..6a1f88605 100644
--- a/tests/src/com/android/tradefed/testtype/DeviceBatteryLevelCheckerTest.java
+++ b/tests/src/com/android/tradefed/testtype/DeviceBatteryLevelCheckerTest.java
@@ -19,6 +19,7 @@ package com.android.tradefed.testtype;
 import com.android.ddmlib.IDevice;
 import com.android.tradefed.device.ITestDevice;
 import com.android.tradefed.util.IRunUtil;
+import com.google.common.util.concurrent.SettableFuture;
 
 import junit.framework.TestCase;
 
@@ -76,7 +77,9 @@ public class DeviceBatteryLevelCheckerTest extends TestCase {
     }
 
     private void expectBattLevel(Integer level) throws Exception {
-        EasyMock.expect(mFakeDevice.getBatteryLevel()).andReturn(level);
+        SettableFuture<Integer> f = SettableFuture.create();
+        f.set(level);
+        EasyMock.expect(mFakeDevice.getBattery()).andReturn(f);
     }
 
     private void replayDevices() {