-
Notifications
You must be signed in to change notification settings - Fork 27.2k
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
Discover devices in parallel instead of serially waiting for each device type #51015
Conversation
yield device; | ||
} | ||
} | ||
Future<List<Device>> getAllConnectedDevices() async { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@zanderso in #49854 (comment):
This could return a
Stream
if we're actually getting results bit-by-bit [...] or if this is implementing an interface that requires it to be aStream
, but otherwise we should probably preferFuture<List>
, orFuture<Iterable>
.(
async*
Stream
s also can have some surprising behavior. IIRC they don't interact well withFakeAsync
for tests.)
This change looks good to me, but it will likely require a corresponding update in the google3 tool- I can point you at the sources tomorrow. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is great, LGTM
Kicking cocoon, landing on red. |
Description
For the next part of wireless iOS device support, make all the
DeviceDiscovery.devices
calls run at the same time in aFuture.wait<List<Device>>()
. This is necessary so the network device timeouts (to be introduced in the next PR) increase the time linearly instead of exponentially.Breaking out just this change to make the next PR smaller, and changing the Stream=>List return value in the signature created some churn.
Related Issues
Another piece of #15072.
Tests
Updated tests to use lists instead of streams.
Checklist
///
).flutter analyze --flutter-repo
) does not report any problems on my PR.Breaking Change