Skip to content

Commit

Permalink
AdbHelper: only restart adb when no device is attached
Browse files Browse the repository at this point in the history
Summary:
Currently the adb server is restarted both when there is no device
attached, or when there is more than one device attached, but the user
hasn't enabled multi install mode.  Restarting the adb server helps when
adb is flaky and shows devices as offline or not attached, but it
doesn't help when there are too many devices are attached and the user
forgets to enable multi install mode.  This is usually a user error.

Disable the server restart and save a little time for the user.

Test plan:
Test buck install with different device configurations and show it does
the right thing about restarting the adb server.
Closes facebook#379
  • Loading branch information
tgummerer authored and sdwilsh committed Jul 17, 2015
1 parent ded812b commit 2ddb600
Show file tree
Hide file tree
Showing 2 changed files with 16 additions and 14 deletions.
26 changes: 13 additions & 13 deletions src/com/facebook/buck/android/AdbHelper.java
Original file line number Diff line number Diff line change
Expand Up @@ -183,19 +183,6 @@ List<IDevice> filterDevices(IDevice[] allDevices) {
return null;
}

// Found multiple devices but multi-install mode is not enabled.
if (!options.isMultiInstallModeEnabled() && devices.size() > 1) {
console.printBuildFailure(
String.format("%d device(s) matches specified device filter (1 expected).\n" +
"Either disconnect other devices or enable multi-install mode (%s).",
devices.size(), AdbOptions.MULTI_INSTALL_MODE_SHORT_ARG));
return null;
}

// Report if multiple devices are matching the filter.
if (devices.size() > 1) {
console.getStdOut().printf("Found " + devices.size() + " matching devices.\n");
}
return devices;
}

Expand Down Expand Up @@ -245,6 +232,19 @@ public List<IDevice> getDevices() throws InterruptedException {

// Build list of matching devices.
List<IDevice> devices = filterDevices(adb.getDevices());
if (devices != null && devices.size() > 1) {
// Found multiple devices but multi-install mode is not enabled.
if (!options.isMultiInstallModeEnabled()) {
console.printBuildFailure(
String.format("%d device(s) matches specified device filter (1 expected).\n" +
"Either disconnect other devices or enable multi-install mode (%s).",
devices.size(), AdbOptions.MULTI_INSTALL_MODE_SHORT_ARG));
return null;
}
// Report if multiple devices are matching the filter.
console.getStdOut().printf("Found " + devices.size() + " matching devices.\n");
}

if (devices == null && restartAdbOnFailure) {
console.printErrorText("No devices found with adb, restarting adb-server.");
adb.restart();
Expand Down
4 changes: 3 additions & 1 deletion test/com/facebook/buck/android/AdbHelperTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -154,7 +154,9 @@ public void testDeviceFilterMultipleDevices() throws CmdLineException {
createRealDevice("5", IDevice.DeviceState.ONLINE)
};

assertNull(basicAdbHelper.filterDevices(devices));
List<IDevice> filteredDevicesNoMultiInstall = basicAdbHelper.filterDevices(devices);
assertNotNull(filteredDevicesNoMultiInstall);
assertEquals(devices.length, filteredDevicesNoMultiInstall.size());

AdbHelper myAdbHelper = createAdbHelper(
new AdbOptions(0, true),
Expand Down

0 comments on commit 2ddb600

Please sign in to comment.