[flutter_tools] Parse wireless ADB serials that contain a space (mDNS conflict suffix)#187845
[flutter_tools] Parse wireless ADB serials that contain a space (mDNS conflict suffix)#187845ishaquehassan wants to merge 2 commits into
Conversation
… conflict suffix)
There was a problem hiding this comment.
Code Review
This pull request updates the regular expression used to parse adb devices output to correctly handle wireless serials containing spaces (such as mDNS name conflicts) and adds a unit test to verify this behavior. Feedback points out a potential edge case where a device serial containing a state keyword (e.g., "host") could cause incorrect regex matching, and suggests using a lookahead assertion to prevent this.
…arse correctly The lazily matched serial could hand the state group a keyword that appears as a standalone word inside the serial itself. The state now must be followed by a key:value property list or the end of the line. no permissions keeps its own branch because adb appends an explanation to that state, such as (user in plugdev group); see [url], which would fail the property lookahead. Adds a test for a serial containing a state keyword and a regression test for the no permissions line format.
|
Just FYI: #187943 seeks to address the same issue. |
Thanks for the heads up. Looks like it takes the same approach, and since it's already with the team I'm happy to close this one in its favor. Feel free to grab any of the test cases here if they're useful, otherwise I'll close it out. Thanks! |
When
adbresolves an mDNS name conflict for a wirelessly paired device it appends(2)to the instance name, producing a serial likeadb-ZY22MGW35T-Z3uXXq (2)._adb-tls-connect._tcp. The device list parser inandroid_device_discovery.dartusedRegExp(r'^(\S+)\s+(\S+)(.*)'), and\S+stops at the first whitespace, so the serial was truncated toadb-ZY22MGW35T-Z3uXXqand the rest of the line was misread as the device state. The tool then ranadb -swith the truncated id, adb answered withdevice not found, and the device surfaced influtter devicesasAndroid null (API null) (unsupported).This PR makes the serial capture lazy and anchors the state capture to the closed set of states
adb devices -lcan emit (device,offline,unauthorized,no permissions,bootloader,recovery,sideload,connecting,host), so everything before the state keyword is kept as the serial. The downstreamswitchondeviceStateonly special-casesunauthorizedandoffline, every other state still falls throughdefaultand is added as a device, so behavior for all previously parsed lines is unchanged. Header and daemon lines are already skipped before the regex runs, so the broader serial capture cannot pick them up.Adds
AndroidDevices preserves wireless serial containing space from mDNS name conflicttoandroid_device_discovery_test.dart. It feeds the exact line from the issue and verifies the full serial is preserved, the model name parses, and the device is detected as wireless. The new test fails on master with the old regex and passes with this change. The full test file passes locally, 11/11.Fixes #187092
Pre-launch Checklist
///).If you need help, consider asking for advice on the #hackers-new channel on Discord.