Merge pull request #1418 from devoncarew/refactor_list

refactor the list command

packages/flutter_tools/lib/src/android/device_android.dart

@@ -17,8 +17,22 @@ import '../flx.dart' as flx;
 import '../toolchain.dart';
 import 'android.dart';
 
+const String _defaultAdbPath = 'adb';
+
+class AndroidDeviceDiscovery extends DeviceDiscovery {
+  List<Device> _devices = <Device>[];
+
+  bool get supportsPlatform => true;
+
+  Future init() {
+    _devices = getAdbDevices();
+    return new Future.value();
+  }
+
+  List<Device> get devices => _devices;
+}
+
 class AndroidDevice extends Device {
-  static const String _defaultAdbPath = 'adb';
   static const int _observatoryPort = 8181;
 
   static final String defaultDeviceID = 'default_android_device';
@@ -64,79 +78,6 @@ class AndroidDevice extends Device {
     }
   }
 
-  /// mockAndroid argument is only to facilitate testing with mocks, so that
-  /// we don't have to rely on the test setup having adb available to it.
-  static List<AndroidDevice> getAttachedDevices([AndroidDevice mockAndroid]) {
-    List<AndroidDevice> devices = [];
-    String adbPath = (mockAndroid != null) ? mockAndroid.adbPath : getAdbPath();
-
-    try {
-      runCheckedSync([adbPath, 'version']);
-    } catch (e) {
-      logging.severe('Unable to find adb. Is "adb" in your path?');
-      return devices;
-    }
-
-    List<String> output = runSync([adbPath, 'devices', '-l']).trim().split('\n');
-
-    // 015d172c98400a03       device usb:340787200X product:nakasi model:Nexus_7 device:grouper
-    RegExp deviceRegex1 = new RegExp(
-        r'^(\S+)\s+device\s+.*product:(\S+)\s+model:(\S+)\s+device:(\S+)$');
-
-    // 0149947A0D01500C       device usb:340787200X
-    RegExp deviceRegex2 = new RegExp(r'^(\S+)\s+device\s+\S+$');
-    RegExp unauthorizedRegex = new RegExp(r'^(\S+)\s+unauthorized\s+\S+$');
-    RegExp offlineRegex = new RegExp(r'^(\S+)\s+offline\s+\S+$');
-
-    // Skip first line, which is always 'List of devices attached'.
-    for (String line in output.skip(1)) {
-      // Skip lines like:
-      // * daemon not running. starting it now on port 5037 *
-      // * daemon started successfully *
-      if (line.startsWith('* daemon '))
-        continue;
-
-      if (line.startsWith('List of devices'))
-        continue;
-
-      if (deviceRegex1.hasMatch(line)) {
-        Match match = deviceRegex1.firstMatch(line);
-        String deviceID = match[1];
-        String productID = match[2];
-        String modelID = match[3];
-        String deviceCodeName = match[4];
-
-        devices.add(new AndroidDevice(
-            id: deviceID,
-            productID: productID,
-            modelID: modelID,
-            deviceCodeName: deviceCodeName
-        ));
-      } else if (deviceRegex2.hasMatch(line)) {
-        Match match = deviceRegex2.firstMatch(line);
-        String deviceID = match[1];
-        devices.add(new AndroidDevice(id: deviceID));
-      } else if (unauthorizedRegex.hasMatch(line)) {
-        Match match = unauthorizedRegex.firstMatch(line);
-        String deviceID = match[1];
-        logging.warning(
-          'Device $deviceID is not authorized.\n'
-          'You might need to check your device for an authorization dialog.'
-        );
-      } else if (offlineRegex.hasMatch(line)) {
-        Match match = offlineRegex.firstMatch(line);
-        String deviceID = match[1];
-        logging.warning('Device $deviceID is offline.');
-      } else {
-        logging.warning(
-          'Unexpected failure parsing device information from adb output:\n'
-          '$line\n'
-          'Please report a bug at https://github.com/flutter/flutter/issues/new');
-      }
-    }
-    return devices;
-  }
-
   static String getAndroidSdkPath() {
     if (Platform.environment.containsKey('ANDROID_HOME')) {
       String androidHomeDir = Platform.environment['ANDROID_HOME'];
@@ -156,25 +97,6 @@ class AndroidDevice extends Device {
     }
   }
 
-  static String getAdbPath() {
-    if (Platform.environment.containsKey('ANDROID_HOME')) {
-      String androidHomeDir = Platform.environment['ANDROID_HOME'];
-      String adbPath1 = path.join(androidHomeDir, 'sdk', 'platform-tools', 'adb');
-      String adbPath2 = path.join(androidHomeDir, 'platform-tools', 'adb');
-      if (FileSystemEntity.isFileSync(adbPath1)) {
-        return adbPath1;
-      } else if (FileSystemEntity.isFileSync(adbPath2)) {
-        return adbPath2;
-      } else {
-        logging.info('"adb" not found at\n  "$adbPath1" or\n  "$adbPath2"\n' +
-            'using default path "$_defaultAdbPath"');
-        return _defaultAdbPath;
-      }
-    } else {
-      return _defaultAdbPath;
-    }
-  }
-
   List<String> adbCommandForDevice(List<String> args) {
     List<String> result = <String>[adbPath];
     if (id != defaultDeviceID) {
@@ -515,3 +437,99 @@ class AndroidDevice extends Device {
     _connected = value;
   }
 }
+
+/// The [mockAndroid] argument is only to facilitate testing with mocks, so that
+/// we don't have to rely on the test setup having adb available to it.
+List<AndroidDevice> getAdbDevices([AndroidDevice mockAndroid]) {
+  List<AndroidDevice> devices = [];
+  String adbPath = (mockAndroid != null) ? mockAndroid.adbPath : getAdbPath();
+
+  try {
+    runCheckedSync([adbPath, 'version']);
+  } catch (e) {
+    logging.severe('Unable to find adb. Is "adb" in your path?');
+    return devices;
+  }
+
+  List<String> output = runSync([adbPath, 'devices', '-l']).trim().split('\n');
+
+  // 015d172c98400a03       device usb:340787200X product:nakasi model:Nexus_7 device:grouper
+  RegExp deviceRegex1 = new RegExp(
+      r'^(\S+)\s+device\s+.*product:(\S+)\s+model:(\S+)\s+device:(\S+)$');
+
+  // 0149947A0D01500C       device usb:340787200X
+  RegExp deviceRegex2 = new RegExp(r'^(\S+)\s+device\s+\S+$');
+  RegExp unauthorizedRegex = new RegExp(r'^(\S+)\s+unauthorized\s+\S+$');
+  RegExp offlineRegex = new RegExp(r'^(\S+)\s+offline\s+\S+$');
+
+  // Skip first line, which is always 'List of devices attached'.
+  for (String line in output.skip(1)) {
+    // Skip lines like:
+    // * daemon not running. starting it now on port 5037 *
+    // * daemon started successfully *
+    if (line.startsWith('* daemon '))
+      continue;
+
+    if (line.startsWith('List of devices'))
+      continue;
+
+    if (deviceRegex1.hasMatch(line)) {
+      Match match = deviceRegex1.firstMatch(line);
+      String deviceID = match[1];
+      String productID = match[2];
+      String modelID = match[3];
+      String deviceCodeName = match[4];
+
+      // Convert `Nexus_7` / `Nexus_5X` style names to `Nexus 7` ones.
+      if (modelID != null)
+        modelID = modelID.replaceAll('_', ' ');
+
+      devices.add(new AndroidDevice(
+          id: deviceID,
+          productID: productID,
+          modelID: modelID,
+          deviceCodeName: deviceCodeName
+      ));
+    } else if (deviceRegex2.hasMatch(line)) {
+      Match match = deviceRegex2.firstMatch(line);
+      String deviceID = match[1];
+      devices.add(new AndroidDevice(id: deviceID));
+    } else if (unauthorizedRegex.hasMatch(line)) {
+      Match match = unauthorizedRegex.firstMatch(line);
+      String deviceID = match[1];
+      logging.warning(
+        'Device $deviceID is not authorized.\n'
+        'You might need to check your device for an authorization dialog.'
+      );
+    } else if (offlineRegex.hasMatch(line)) {
+      Match match = offlineRegex.firstMatch(line);
+      String deviceID = match[1];
+      logging.warning('Device $deviceID is offline.');
+    } else {
+      logging.warning(
+        'Unexpected failure parsing device information from adb output:\n'
+        '$line\n'
+        'Please report a bug at https://github.com/flutter/flutter/issues/new');
+    }
+  }
+  return devices;
+}
+
+String getAdbPath() {
+  if (Platform.environment.containsKey('ANDROID_HOME')) {
+    String androidHomeDir = Platform.environment['ANDROID_HOME'];
+    String adbPath1 = path.join(androidHomeDir, 'sdk', 'platform-tools', 'adb');
+    String adbPath2 = path.join(androidHomeDir, 'platform-tools', 'adb');
+    if (FileSystemEntity.isFileSync(adbPath1)) {
+      return adbPath1;
+    } else if (FileSystemEntity.isFileSync(adbPath2)) {
+      return adbPath2;
+    } else {
+      logging.info('"adb" not found at\n  "$adbPath1" or\n  "$adbPath2"\n' +
+          'using default path "$_defaultAdbPath"');
+      return _defaultAdbPath;
+    }
+  } else {
+    return _defaultAdbPath;
+  }
+}

packages/flutter_tools/lib/src/commands/daemon.dart

@@ -325,7 +325,7 @@ class AndroidDeviceDiscovery {
 
   void _initAdb() {
     if (_adb == null) {
-      _adb = new Adb(AndroidDevice.getAdbPath());
+      _adb = new Adb(getAdbPath());
       if (!_adb.exists())
         _adb = null;
     }

packages/flutter_tools/lib/src/commands/list.dart

@@ -3,71 +3,33 @@
 // found in the LICENSE file.
 
 import 'dart:async';
-import 'dart:io';
 
-import '../android/device_android.dart';
-import '../ios/device_ios.dart';
+import '../device.dart';
 import '../runner/flutter_command.dart';
 
 class ListCommand extends FlutterCommand {
   final String name = 'list';
   final String description = 'List all connected devices.';
 
-  ListCommand() {
-    argParser.addFlag('details',
-        abbr: 'd',
-        negatable: false,
-        help: 'Log additional details about attached devices.');
-  }
-
   bool get requiresProjectRoot => false;
 
-  @override
   Future<int> runInProject() async {
-    connectToDevices();
+    DeviceManager deviceManager = new DeviceManager();
 
-    bool details = argResults['details'];
+    List<Device> devices = await deviceManager.getDevices();
 
-    if (details)
-      print('Android Devices:');
-
-    // TODO(devoncarew): We should have a more generic mechanism for device discovery.
-    // DeviceDiscoveryService? DeviceDiscoveryParticipant?
-    for (AndroidDevice device in AndroidDevice.getAttachedDevices(devices.android)) {
-      if (details) {
-        print('${device.id}\t'
-            '${device.modelID}\t'
-            '${device.productID}\t'
-            '${device.deviceCodeName}');
-      } else {
-        print(device.id);
-      }
-    }
-
-    if (Platform.isMacOS) {
-      if (details)
-        print('iOS Devices:');
-
-      for (IOSDevice device in IOSDevice.getAttachedDevices(devices.iOS)) {
-        if (details) {
-          print('${device.id}\t${device.name}');
-        } else {
-          print(device.id);
-        }
-      }
-
-      if (details)
-        print('iOS Simulators:');
-
-      for (IOSSimulator device in IOSSimulator.getAttachedDevices(devices.iOSSimulator)) {
-        if (details) {
-          print('${device.id}\t${device.name}');
-        } else {
-          print(device.id);
-        }
+    if (devices.isEmpty) {
+      print('No connected devices.');
+    } else {
+      print('${devices.length} connected ${pluralize('device', devices.length)}:');
+      print('');
+      for (Device device in devices) {
+        print('${device.name} (${device.id})');
       }
     }
 
     return 0;
   }
 }
+
+String pluralize(String word, int count) => count == 1 ? word : word + 's';

packages/flutter_tools/lib/src/device.dart

@@ -11,6 +11,46 @@ import 'build_configuration.dart';
 import 'ios/device_ios.dart';
 import 'toolchain.dart';
 
+/// A class to get all available devices.
+class DeviceManager {
+  DeviceManager() {
+    // Init the known discoverers.
+    _deviceDiscoverers.add(new AndroidDeviceDiscovery());
+    _deviceDiscoverers.add(new IOSDeviceDiscovery());
+    _deviceDiscoverers.add(new IOSSimulatorDiscovery());
+
+    Future.forEach(_deviceDiscoverers, (DeviceDiscovery discoverer) {
+      if (!discoverer.supportsPlatform)
+        return null;
+      return discoverer.init();
+    }).then((_) {
+      _initedCompleter.complete();
+    }).catchError((error, stackTrace) {
+      _initedCompleter.completeError(error, stackTrace);
+    });
+  }
+
+  List<DeviceDiscovery> _deviceDiscoverers = <DeviceDiscovery>[];
+
+  Completer _initedCompleter = new Completer();
+
+  Future<List<Device>> getDevices() async {
+    await _initedCompleter.future;
+
+    return _deviceDiscoverers
+      .where((DeviceDiscovery discoverer) => discoverer.supportsPlatform)
+      .expand((DeviceDiscovery discoverer) => discoverer.devices)
+      .toList();
+  }
+}
+
+/// An abstract class to discover and enumerate a specific type of devices.
+abstract class DeviceDiscovery {
+  bool get supportsPlatform;
+  Future init();
+  List<Device> get devices;
+}
+
 abstract class Device {
   final String id;
   static Map<String, Device> _deviceCache = {};
@@ -59,6 +99,7 @@ abstract class Device {
   String toString() => '$runtimeType $id';
 }
 
+// TODO(devoncarew): Unify this with [DeviceManager].
 class DeviceStore {
   final AndroidDevice android;
   final IOSDevice iOS;
@@ -115,7 +156,7 @@ class DeviceStore {
       switch (config.targetPlatform) {
         case TargetPlatform.android:
           assert(android == null);
-          android = _deviceForConfig(config, AndroidDevice.getAttachedDevices());
+          android = _deviceForConfig(config, getAdbDevices());
           break;
         case TargetPlatform.iOS:
           assert(iOS == null);

packages/flutter_tools/lib/src/ios/device_ios.dart

@@ -14,6 +14,32 @@ import '../build_configuration.dart';
 import '../device.dart';
 import '../toolchain.dart';
 
+class IOSDeviceDiscovery extends DeviceDiscovery {
+  List<Device> _devices = <Device>[];
+
+  bool get supportsPlatform => Platform.isMacOS;
+
+  Future init() {
+    _devices = IOSDevice.getAttachedDevices();
+    return new Future.value();
+  }
+
+  List<Device> get devices => _devices;
+}
+
+class IOSSimulatorDiscovery extends DeviceDiscovery {
+  List<Device> _devices = <Device>[];
+
+  bool get supportsPlatform => Platform.isMacOS;
+
+  Future init() {
+    _devices = IOSSimulator.getAttachedDevices();
+    return new Future.value();
+  }
+
+  List<Device> get devices => _devices;
+}
+
 class IOSDevice extends Device {
   static final String defaultDeviceID = 'default_ios_id';
 
@@ -90,7 +116,7 @@ class IOSDevice extends Device {
     String informerPath = (mockIOS != null)
         ? mockIOS.informerPath
         : _checkForCommand('ideviceinfo');
-    return runSync([informerPath, '-k', 'DeviceName', '-u', deviceID]);
+    return runSync([informerPath, '-k', 'DeviceName', '-u', deviceID]).trim();
   }
 
   static final Map<String, String> _commandMap = {};

packages/flutter_tools/test/device_test.dart

@@ -0,0 +1,19 @@
+// Copyright 2015 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+import 'package:flutter_tools/src/device.dart';
+import 'package:test/test.dart';
+
+main() => defineTests();
+
+defineTests() {
+  group('DeviceManager', () {
+    test('getDevices', () async {
+      // Test that DeviceManager.getDevices() doesn't throw.
+      DeviceManager deviceManager = new DeviceManager();
+      List<Device> devices = await deviceManager.getDevices();
+      expect(devices, isList);
+    });
+  });
+}