feat(mac): add ADB device support and manual IP:port connection - #129
feat(mac): add ADB device support and manual IP:port connection#129ayufan wants to merge 2 commits into
Conversation
Add a `manual` connection target dialed over the existing TCP transport, with an endpoint field in the device list accepting `host`, `host:port`, and `[v6]:port` (port defaults to the receiver's 9000). Useful on networks where Bonjour/mDNS discovery is filtered. Connected endpoints are remembered in the device list until explicitly removed and are never auto-dialed, since a dead address would keep dialing forever.
macOS cannot reach an Android receiver over USB tethering: RNDIS, the protocol most Android devices support, is not supported by macOS, and only newer devices offer NCM. ADB works with any developer-enabled Android device instead: discover devices via `adb devices -l`, run a per-device `adb forward tcp:0 tcp:9000`, and dial the allocated loopback port with the existing TCP transport. Locate `adb` via ANDROID_SDK_ROOT/ANDROID_HOME, ~/Library/Android/sdk, PATH, and Homebrew locations, since GUI apps do not inherit the shell PATH. Keep unauthorized/offline devices listed with a hint to authorize USB debugging. Auto-connect wired Android devices like USB iOS devices, matching receiver install ids across transports to avoid duplicate sessions. When adb is not installed, ADB discovery stays off.
peetzweg
left a comment
There was a problem hiding this comment.
Thanks for this — ADB support in particular removes a real hole (RNDIS not working on macOS) and the design here is careful: a private serial queue owns all the adb Process work so it never blocks the main thread, and the poll timer is a proper cancellable DispatchSourceTimer torn down in stop() rather than a self-rescheduling chain. Overall this is close, but there's a real correctness bug in the manual-endpoint persistence that needs fixing before merge.
Verdict: request changes — one bug that breaks a feature this PR advertises (IPv6 manual endpoints), the rest is polish.
Blocking
- IPv6 manual endpoints corrupt themselves on the very next redraw.
connect()persists a manual target as"\(host):\(portNum)"(Mac/OpenSidecarMacApp.swift:538), anddeviceEntriesreparses that same string withparseEndpoint(line 716) every time it recomputes — which is constantly, not just across restarts.parseEndpoint(line 453) only recognizeshost:portwhen there's exactly one colon, and requires[host]:portbrackets to disambiguate anything with more. So for[fe80::1]:9000, the persisted key becomes the unbracketedfe80::1:9000; reparsing it finds three colons, none of the special-cased branches match, and it falls through to "bare address" — host becomes the literal stringfe80::1:9000and port silently resets to 9000. The row's own reconnect target is now wrong. Worse,forget()(line 489) rebuilds the removal key from the entry's (already-corrupted) host/port, so it never matches what's actually inmanualRemembered— the broken row can't even be removed from the UI once it exists. Fix: bracket IPv6 hosts when serializing (host.contains(":") ? "[\(host)]:\(port)" : "\(host):\(port)"), matching whatparseEndpointexpects on the way back in.
Worth fixing
- No timeout on the adb subprocess.
run()(Mac/Adb.swift:174-187) callsprocess.waitUntilExit()with no deadline. Everything inAdbDeviceWatcherfunnels through one serial queue, so ifadbever hangs (not unheard of — the adb server wedges occasionally), that queue never drains: the timer's next tick just queues up behind it forever, and ADB discovery silently freezes with no error surfaced. It also meansstop()'squeue.sync(Mac/Adb.swift:228) — called fromapplicationWillTerminateon the main thread — would hang app quit indefinitely in that scenario. Worth adding a hard timeout around the process call (kill + treat as failure) so a wedged adb degrades instead of freezing things. - Heads up, not a request: open PR #131 touches the same
connect/disconnect/ConnectionTarget/DeviceEntrymachinery in OpenSidecarMacApp.swift (plus MacSender.swift) for transport switching. Whichever lands second is going to need a real rebase, not just a fast-forward — flagging so it doesn't surprise anyone.
Minor
AdbDeviceWatcher.ownedPorts(Mac/Adb.swift ~217) never prunes serials that disappear fromadb devices -l(permanent unplug/re-pair with a new serial) — unbounded but trivial growth over an app session, not urgent.adbAvailable's initializer (Mac/OpenSidecarMacApp.swift:229,Adb.executableURL() != nil) runs the PATH/Homebrew filesystem probe synchronously on the main thread at launch, the one place this scan isn't kept off-main the way the rest of the ADB code is careful to do. Likely fine in practice (small number of stat calls) but inconsistent with the pattern elsewhere.forget()on a still-connected manual session only removes it frommanualRemembered; the live session itself isn't torn down, so it reappears as a bare dangling row (no Forget button) until separately disconnected. Reads as intentional given the comment distinguishing forget from disconnect, but a short doc comment onforget()would save the next reader from re-deriving that.
Nice write-up in the PR description, and the ADB discovery/forward lifecycle logic itself (preferred-port reuse, tcp:0 atomicity, keeping unauthorized/offline devices visible with a hint) is solid work — just want the IPv6 persistence bug fixed before this goes in.
|
@peetzweg lets wait for your PR be merged first |
What this does
Two additions to the Mac sender app, one commit each:
ADB device support
macOS cannot reach an Android receiver over USB tethering: RNDIS, the protocol most Android devices support, is not supported by macOS, and only newer devices offer NCM. ADB sidesteps this and works with any developer-enabled Android device over a plain USB cable. The app discovers devices via
adb devices -l, runs a per-deviceadb forward tcp:0 tcp:9000, and dials the allocated loopback port with the existing TCP transport.adbis located viaANDROID_SDK_ROOT/ANDROID_HOME,~/Library/Android/sdk,PATH, and Homebrew locations (GUI apps don't inherit the shell PATH). Unauthorized or offline devices stay listed with a hint to authorize USB debugging. Ifadbis not installed, ADB discovery simply stays off.To install
adbon macOS via Homebrew:Manual IP:port connection
a field in the device list to connect to a receiver by address (
host,host:port, or[v6]:port; port defaults to 9000), for networks where Bonjour/mDNS discovery is filtered. Connected endpoints are remembered in the device list until explicitly removed and are never auto-dialed.Screenshot
Related