mirror of
https://github.com/gesellix/Bose-SoundTouch.git
synced 2026-09-07 15:07:17 +00:00
Tobias GesellchenandClaude Sonnet 5
df62073ab0
Five findings from reviewing gesellix/Bose-SoundTouch#644 (Retry configured player devices during discovery): 1. Source-labeling used exact string equality (device.DiscoveryMethod == "Configuration"), which breaks once a configured host is also found via mDNS/UPnP in the same sweep: mergeDeviceData concatenates methods into e.g. "Configuration+mDNS/Bonjour", so the check silently failed and the device got labeled "discovered" instead of "manual". Extracted the decision into classifySource() and switched to a substring match. Added TestClassifySource, which fails against the old exact-equality logic on exactly the composite-string cases (verified) and would have caught this before merge -- the PR's own test disables mDNS/UPnP, so it never exercised this path. 2. Manually configured devices no longer registered immediately at startup -- they now wait for the full mDNS/UPnP sweep (up to the 10s discovery timeout) to complete, since the PR removed the synchronous registration loop and relies entirely on PreferredDevices. Restored the immediate loop alongside (not instead of) folding manualHosts into PreferredDevices, so a currently-online configured device registers immediately as before, while an offline one still gets retried on every subsequent discovery pass -- the actual value this PR adds. 3. The new PreferredDevices-seeding loop didn't dedupe against hosts already loaded from PREFERRED_DEVICES, so setting both for the same host produced duplicate entries. Currently harmless (absorbed by AddDeviceByHost's fast path) but fragile. Added dedup by host. 4. NewDiscoveryService's doc comment didn't mention the new configuredHosts parameter or its retry-on-every-sweep behavior. Documented. 5. The new test's second DiscoverDevices call spawns a one-shot status-update goroutine and a 30s-ticker poll loop with no guaranteed drain before the deferred server.Close(), risking benign but real -race/CI flakiness. Added a bounded settle delay after RemoveDevice. Verified: full build/vet/race test suite/lint clean; the new TestClassifySource fails against the pre-fix logic and passes with it; TestDiscoverDevicesRetriesConfiguredHosts re-run 20x under -race with no flakiness. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>