Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Separate attached and wireless devices #122615

Merged
merged 2 commits into from Mar 15, 2023
Merged

Separate attached and wireless devices #122615

merged 2 commits into from Mar 15, 2023

Conversation

vashworth
Copy link
Contributor

@vashworth vashworth commented Mar 14, 2023 •
edited

This PR includes 4 changes:

  1. Separates attached and wireless devices in device selection flow when running commands like flutter run.
Multiple devices found:
target-device-1 (mobile) • xxx • android • Android 10

Wirelessly connected devices:
target-device-4 (mobile) • xxx • android • Android 10

[1]: target-device-1 (xxx)
[2]: target-device-4 (xxx)
  1. Separates attached and wireless devices in flutter devices command.
2 connected devices:

ephemeral (mobile) • ephemeral • android-arm    • Test SDK (1.2.3) (emulator)
webby (mobile)     • webby     • web-javascript • Web SDK (1.2.4) (emulator)

1 wirelessly connected device:

wireless android (mobile) • wireless-android • android-arm • Test SDK (1.2.3) (emulator)
  1. Fixes Device list missing when Flutter finds multiple devices with matching name/id #119271. When more than 1 match for the -d flag is found, either allow the user to select from them (if stdin is available) or display the list of them (if no stdin).
Found 2 devices with name or id matching target-device:
target-device-1 (mobile) • xxx • android • Android 10

Wirelessly connected devices:
target-device-4 (mobile) • xxx • android • Android 10

[1]: target-device-1 (xxx)
[3]: target-device-4 (xxx)
  1. Replaces IOSDeviceConnectionInterface with DeviceConnectionInterface since it's no longer needed.
  • IOSDeviceConnectionInterface.network -> DeviceConnectionInterface.wireless
  • IOSDeviceConnectionInterface.usb -> DeviceConnectionInterface.attached
  • IOSDeviceConnectionInterface.none -> DeviceConnectionInterface.attached

Note: This PR does not include updates for extended search for wireless iOS devices.

Part 3 in breakdown of #121262.

Fixes #119271.

Pre-launch Checklist

  • I read the Contributor Guide and followed the process outlined there for submitting PRs.
  • I read the Tree Hygiene wiki page, which explains my responsibilities.
  • I read and followed the Flutter Style Guide, including Features we expect every widget to implement.
  • I signed the CLA.
  • I listed at least one issue that this PR fixes in the description above.
  • I updated/added relevant documentation (doc comments with ///).
  • I added new tests to check the change I am making, or this PR is test-exempt.
  • All existing and new tests are passing.

If you need help, consider asking for advice on the #hackers-new channel on Discord.

@flutter-dashboard flutter-dashboard bot added a: text input Entering text in a text field or keyboard related problems platform-ios iOS applications specifically tool Affects the "flutter" command-line tool. See also t: labels. labels Mar 14, 2023
jmagman
jmagman reviewed Mar 14, 2023
View reviewed changes
packages/flutter_tools/test/commands.shard/hermetic/attach_test.dart Outdated
@@ -1302,6 +1306,13 @@ class FakeIOSDevice extends Fake implements IOSDevice {
@override
final IOSDeviceConnectionInterface interfaceType;

@override
DeviceConnectionInterface get connectionInterface {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I know you added this enum in a previous refactor PR #121359, but is there any reason to maintain both DeviceConnectionInterface and IOSDeviceConnectionInterface? Can we swap the iOS device to use DeviceConnectionInterface instead, with the none either removed, or added to DeviceConnectionInterface?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good point, on further inspection, I think we can safely get rid of IOSDeviceConnectionInterface. The none case is for simulators, which we consider attached and aren't even included in IOSDevice. The "unknown future interfaces" gets categorized as usb currently, which will get changed to attached.

flutter/packages/flutter_tools/lib/src/macos/xcdevice.dart

Lines 360 to 371 in fcf1134

// Interface can be "usb", "network", or "none" for simulators
// and unknown future interfaces.
final Object? interface = deviceProperties['interface'];
if (interface is String) {
if (interface.toLowerCase() == 'network') {
return IOSDeviceConnectionInterface.network;
} else {
return IOSDeviceConnectionInterface.usb;
}
}
return IOSDeviceConnectionInterface.none;

jmagman
jmagman approved these changes Mar 14, 2023
View reviewed changes
Copy link
Member

@jmagman jmagman left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@vashworth vashworth added the autosubmit Merge PR when tree becomes green via auto submit App label Mar 15, 2023
@auto-submit auto-submit bot merged commit 27248d4 into flutter:master Mar 15, 2023
81 checks passed
@engine-flutter-autoroll engine-flutter-autoroll mentioned this pull request Mar 16, 2023
Closed
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Mar 16, 2023
@engine-flutter-autoroll
27248d4b6 Separate attached and wireless devices (flutter/flutter#122615
9afa60f 
)
@engine-flutter-autoroll engine-flutter-autoroll mentioned this pull request Mar 16, 2023
Closed
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Mar 16, 2023
@engine-flutter-autoroll
27248d4b6 Separate attached and wireless devices (flutter/flutter#122615
04fe876 
)
@engine-flutter-autoroll engine-flutter-autoroll mentioned this pull request Mar 16, 2023
Closed
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Mar 16, 2023
@engine-flutter-autoroll
27248d4b6 Separate attached and wireless devices (flutter/flutter#122615
6a9e7ad 
)
@engine-flutter-autoroll engine-flutter-autoroll mentioned this pull request Mar 16, 2023
Closed
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Mar 16, 2023
@engine-flutter-autoroll
27248d4b6 Separate attached and wireless devices (flutter/flutter#122615
84e8e29 
)
@engine-flutter-autoroll engine-flutter-autoroll mentioned this pull request Mar 16, 2023
Merged
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Mar 16, 2023
@engine-flutter-autoroll
27248d4b6 Separate attached and wireless devices (flutter/flutter#122615
9ebdb2a 
)
@jmagman jmagman mentioned this pull request Apr 5, 2023
Closed
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request May 10, 2023
@engine-flutter-autoroll
27248d4b6 Separate attached and wireless devices (flutter/flutter#122615
4618231 
)
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request May 10, 2023
@engine-flutter-autoroll
27248d4b6 Separate attached and wireless devices (flutter/flutter#122615
1c6f13b 
)
@vashworth vashworth deleted the wireless_device_discovery_part_3 branch September 27, 2023 15:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jmagman jmagman jmagman approved these changes

@christopherfujino christopherfujino Awaiting requested review from christopherfujino

Assignees
No one assigned
Labels
a: text input Entering text in a text field or keyboard related problems autosubmit Merge PR when tree becomes green via auto submit App platform-ios iOS applications specifically tool Affects the "flutter" command-line tool. See also t: labels.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Device list missing when Flutter finds multiple devices with matching name/id
2 participants
@vashworth @jmagman