From 3dba3f2a0fcc386b6a80f2e1424a8f9cc51fe181 Mon Sep 17 00:00:00 2001 From: Lau Ching Jun Date: Wed, 13 Mar 2024 15:24:25 -0700 Subject: [PATCH] Update proxied devices to handle connection interface and diagnostics. (#145061) Also improve the performance of daemon device discovery by parallelizing the calls. --- .../lib/src/commands/daemon.dart | 37 ++++++++++++++++-- packages/flutter_tools/lib/src/device.dart | 21 ++++++++++ .../lib/src/proxied_devices/devices.dart | 27 +++++++++++++ .../commands.shard/hermetic/daemon_test.dart | 32 ++++++++++++++++ .../hermetic/proxied_devices_test.dart | 3 ++ .../proxied_devices/proxied_devices_test.dart | 38 +++++++++++++++++++ .../flutter_tools/test/src/fake_devices.dart | 5 +++ 7 files changed, 159 insertions(+), 4 deletions(-) diff --git a/packages/flutter_tools/lib/src/commands/daemon.dart b/packages/flutter_tools/lib/src/commands/daemon.dart index 009f485023d..d74f0ae73cd 100644 --- a/packages/flutter_tools/lib/src/commands/daemon.dart +++ b/packages/flutter_tools/lib/src/commands/daemon.dart @@ -1008,6 +1008,7 @@ class DeviceDomain extends Domain { registerHandler('startDartDevelopmentService', startDartDevelopmentService); registerHandler('shutdownDartDevelopmentService', shutdownDartDevelopmentService); registerHandler('setExternalDevToolsUriForDartDevelopmentService', setExternalDevToolsUriForDartDevelopmentService); + registerHandler('getDiagnostics', getDiagnostics); // Use the device manager discovery so that client provided device types // are usable via the daemon protocol. @@ -1059,12 +1060,23 @@ class DeviceDomain extends Domain { } /// Return a list of the current devices, discarding existing cache of devices. - Future>> discoverDevices([ Map? args ]) async { - return >[ + Future>> discoverDevices(Map args) async { + final int? timeoutInMilliseconds = _getIntArg(args, 'timeoutInMilliseconds'); + final Duration? timeout = timeoutInMilliseconds != null ? Duration(milliseconds: timeoutInMilliseconds) : null; + + // Calling `discoverDevices()` and `_deviceToMap()` in parallel for better performance. + final List> devicesListList = await Future.wait(>>[ for (final PollingDeviceDiscovery discoverer in _discoverers) - for (final Device device in await discoverer.discoverDevices()) - await _deviceToMap(device), + discoverer.discoverDevices(timeout: timeout), + ]); + + final List devices = [ + for (final List devicesList in devicesListList) + ...devicesList, ]; + return Future.wait(>>[ + for (final Device device in devices) _deviceToMap(device), + ]); } /// Enable device events. @@ -1298,6 +1310,21 @@ class DeviceDomain extends Domain { } return null; } + + /// Gets a list of diagnostic messages pertaining to issues with any connected + /// devices. + Future> getDiagnostics(Map args) async { + // Call `getDiagnostics()` in parallel to improve performance. + final List> diagnosticsLists = await Future.wait(>>[ + for (final PollingDeviceDiscovery discoverer in _discoverers) + discoverer.getDiagnostics(), + ]); + + return [ + for (final List diagnostics in diagnosticsLists) + ...diagnostics, + ]; + } } class DevToolsDomain extends Domain { @@ -1333,6 +1360,8 @@ Future> _deviceToMap(Device device) async { 'ephemeral': device.ephemeral, 'emulatorId': await device.emulatorId, 'sdk': await device.sdkNameAndVersion, + 'isConnected': device.isConnected, + 'connectionInterface': getNameForDeviceConnectionInterface(device.connectionInterface), 'capabilities': { 'hotReload': device.supportsHotReload, 'hotRestart': device.supportsHotRestart, diff --git a/packages/flutter_tools/lib/src/device.dart b/packages/flutter_tools/lib/src/device.dart index 1f0cc0b2414..b681df851b4 100644 --- a/packages/flutter_tools/lib/src/device.dart +++ b/packages/flutter_tools/lib/src/device.dart @@ -586,6 +586,27 @@ enum DeviceConnectionInterface { wireless, } +/// Returns the `DeviceConnectionInterface` enum based on its string name. +DeviceConnectionInterface getDeviceConnectionInterfaceForName(String name) { + switch (name) { + case 'attached': + return DeviceConnectionInterface.attached; + case 'wireless': + return DeviceConnectionInterface.wireless; + } + throw Exception('Unsupported DeviceConnectionInterface name "$name"'); +} + +/// Returns a `DeviceConnectionInterface`'s string name. +String getNameForDeviceConnectionInterface(DeviceConnectionInterface connectionInterface) { + switch (connectionInterface) { + case DeviceConnectionInterface.attached: + return 'attached'; + case DeviceConnectionInterface.wireless: + return 'wireless'; + } +} + /// A device is a physical hardware that can run a Flutter application. /// /// This may correspond to a connected iOS or Android device, or represent diff --git a/packages/flutter_tools/lib/src/proxied_devices/devices.dart b/packages/flutter_tools/lib/src/proxied_devices/devices.dart index 10aecd3b1a8..12142a471db 100644 --- a/packages/flutter_tools/lib/src/proxied_devices/devices.dart +++ b/packages/flutter_tools/lib/src/proxied_devices/devices.dart @@ -102,6 +102,8 @@ class ProxiedDevices extends PollingDeviceDiscovery { @visibleForTesting ProxiedDevice deviceFromDaemonResult(Map device) { final Map capabilities = _cast>(device['capabilities']); + final String? connectionInterfaceName = _cast(device['connectionInterface']); + final DeviceConnectionInterface? connectionInterface = connectionInterfaceName != null ? getDeviceConnectionInterfaceForName(connectionInterfaceName) : null; return ProxiedDevice( connection, _cast(device['id']), deltaFileTransfer: _deltaFileTransfer, @@ -110,6 +112,8 @@ class ProxiedDevices extends PollingDeviceDiscovery { platformType: PlatformType.fromString(_cast(device['platformType'])), targetPlatform: getTargetPlatformForName(_cast(device['platform'])), ephemeral: _cast(device['ephemeral']), + isConnected: _cast(device['isConnected']) ?? true, + connectionInterface: connectionInterface ?? DeviceConnectionInterface.attached, name: 'Proxied ${device['name']}', isLocalEmulator: _cast(device['emulator']), emulatorId: _cast(device['emulatorId']), @@ -124,6 +128,21 @@ class ProxiedDevices extends PollingDeviceDiscovery { fileTransfer: _fileTransfer, ); } + + @override + Future> getDiagnostics() async { + try { + final List diagnostics = _cast>(await connection.sendRequest('device.getDiagnostics')).cast(); + return diagnostics; + } on String catch (e) { // Daemon actually does throw string types. + if (e.contains('command not understood')) { + _logger.printTrace('The daemon is on an older version that does not support `device.getDiagnostics`.'); + // Silently ignore. + return []; + } + rethrow; + } + } } /// A [Device] that acts as a proxy to remotely connected device. @@ -143,6 +162,8 @@ class ProxiedDevice extends Device { required PlatformType? platformType, required TargetPlatform targetPlatform, required bool ephemeral, + required this.isConnected, + required this.connectionInterface, required this.name, required bool isLocalEmulator, required String? emulatorId, @@ -180,6 +201,12 @@ class ProxiedDevice extends Device { final FileTransfer _fileTransfer; + @override + final bool isConnected; + + @override + final DeviceConnectionInterface connectionInterface; + @override final String name; diff --git a/packages/flutter_tools/test/commands.shard/hermetic/daemon_test.dart b/packages/flutter_tools/test/commands.shard/hermetic/daemon_test.dart index d0db26737fb..d467da24828 100644 --- a/packages/flutter_tools/test/commands.shard/hermetic/daemon_test.dart +++ b/packages/flutter_tools/test/commands.shard/hermetic/daemon_test.dart @@ -459,6 +459,8 @@ void main() { 'ephemeral': false, 'emulatorId': 'device', 'sdk': 'Android 12', + 'isConnected': true, + 'connectionInterface': 'attached', 'capabilities': { 'hotReload': true, 'hotRestart': true, @@ -479,6 +481,8 @@ void main() { 'ephemeral': false, 'emulatorId': null, 'sdk': 'preview', + 'isConnected': true, + 'connectionInterface': 'attached', 'capabilities': { 'hotReload': true, 'hotRestart': true, @@ -725,6 +729,31 @@ void main() { expect(device.dds.shutdownCalled, true); }); + testUsingContext('device.getDiagnostics returns correct value', () async { + daemon = Daemon( + daemonConnection, + notifyingLogger: notifyingLogger, + ); + final FakePollingDeviceDiscovery discoverer1 = FakePollingDeviceDiscovery(); + discoverer1.diagnostics = ['fake diagnostic 1', 'fake diagnostic 2']; + final FakePollingDeviceDiscovery discoverer2 = FakePollingDeviceDiscovery(); + discoverer2.diagnostics = ['fake diagnostic 3', 'fake diagnostic 4']; + daemon.deviceDomain.addDeviceDiscoverer(discoverer1); + daemon.deviceDomain.addDeviceDiscoverer(discoverer2); + daemonStreams.inputs.add(DaemonMessage({ + 'id': 0, + 'method': 'device.getDiagnostics', + })); + final DaemonMessage response = await daemonStreams.outputs.stream.firstWhere(_notEvent); + expect(response.data['id'], 0); + expect(response.data['result'], [ + 'fake diagnostic 1', + 'fake diagnostic 2', + 'fake diagnostic 3', + 'fake diagnostic 4', + ]); + }); + testUsingContext('emulator.launch without an emulatorId should report an error', () async { daemon = Daemon( daemonConnection, @@ -1128,6 +1157,9 @@ class FakeAndroidDevice extends Fake implements AndroidDevice { @override final bool isConnected = true; + @override + final DeviceConnectionInterface connectionInterface = DeviceConnectionInterface.attached; + @override Future get sdkNameAndVersion async => 'Android 12'; diff --git a/packages/flutter_tools/test/commands.shard/hermetic/proxied_devices_test.dart b/packages/flutter_tools/test/commands.shard/hermetic/proxied_devices_test.dart index 29d2bafdf3c..35468443cbb 100644 --- a/packages/flutter_tools/test/commands.shard/hermetic/proxied_devices_test.dart +++ b/packages/flutter_tools/test/commands.shard/hermetic/proxied_devices_test.dart @@ -274,6 +274,9 @@ class FakeAndroidDevice extends Fake implements AndroidDevice { @override bool get isConnected => true; + @override + final DeviceConnectionInterface connectionInterface = DeviceConnectionInterface.attached; + @override Future get sdkNameAndVersion async => 'Android 12'; diff --git a/packages/flutter_tools/test/general.shard/proxied_devices/proxied_devices_test.dart b/packages/flutter_tools/test/general.shard/proxied_devices/proxied_devices_test.dart index 26c1a7ece3d..7cbcdf5fc55 100644 --- a/packages/flutter_tools/test/general.shard/proxied_devices/proxied_devices_test.dart +++ b/packages/flutter_tools/test/general.shard/proxied_devices/proxied_devices_test.dart @@ -562,6 +562,44 @@ void main() { expect(devicesAdded[0].id, fakeDevice['id']); expect(devicesAdded[1].id, fakeDevice2['id']); }); + + testWithoutContext('handles getDiagnostics', () async { + bufferLogger = BufferLogger.test(); + final ProxiedDevices proxiedDevices = ProxiedDevices( + clientDaemonConnection, + logger: bufferLogger, + ); + + final Future> resultFuture = proxiedDevices.getDiagnostics(); + + final DaemonMessage message = await serverDaemonConnection.incomingCommands.first; + expect(message.data['id'], isNotNull); + expect(message.data['method'], 'device.getDiagnostics'); + + serverDaemonConnection.sendResponse(message.data['id']!, ['1', '2']); + + final List result = await resultFuture; + expect(result, ['1', '2']); + }); + + testWithoutContext('returns empty result when daemon does not understand getDiagnostics', () async { + bufferLogger = BufferLogger.test(); + final ProxiedDevices proxiedDevices = ProxiedDevices( + clientDaemonConnection, + logger: bufferLogger, + ); + + final Future> resultFuture = proxiedDevices.getDiagnostics(); + + final DaemonMessage message = await serverDaemonConnection.incomingCommands.first; + expect(message.data['id'], isNotNull); + expect(message.data['method'], 'device.getDiagnostics'); + + serverDaemonConnection.sendErrorResponse(message.data['id']!, 'command not understood: device.getDiagnostics', StackTrace.current); + + final List result = await resultFuture; + expect(result, isEmpty); + }); }); group('ProxiedDartDevelopmentService', () { diff --git a/packages/flutter_tools/test/src/fake_devices.dart b/packages/flutter_tools/test/src/fake_devices.dart index cb72de2a34e..4f6ab9e329e 100644 --- a/packages/flutter_tools/test/src/fake_devices.dart +++ b/packages/flutter_tools/test/src/fake_devices.dart @@ -269,6 +269,11 @@ class FakePollingDeviceDiscovery extends PollingDeviceDiscovery { @override List wellKnownIds = []; + + List diagnostics = []; + + @override + Future> getDiagnostics() => Future>.value(diagnostics); } /// A fake implementation of the [DeviceLogReader].