feat(macos): find the AWTRIX clock from the app and configure the server (#57) - #60
feat(macos): find the AWTRIX clock from the app and configure the server (#57)#60tarakanof wants to merge 11 commits into
Conversation
The IPv6 authority rules (bracketing, RFC 6874 zone encoding) and the NWEndpoint.Host rendering were private to ServerDiscovery. A second browser (client-side clock discovery) needs exactly the same rules, and getting them wrong twice is the failure mode worth designing out.
The server's own browse only works when its container can see multicast (host or macvlan networking); on a default Docker bridge it finds nothing and the clock has to be typed in by hand. The app has full-stack mDNS, so it can find the clock regardless of how the server is deployed. ClockDiscovery mirrors internal/discovery's matching rules exactly — browse _http._tcp, keep only hosts whose /api/stats answers 200 with a non-empty uid, and shape the base URL with an explicit port so a candidate found here is byte-identical to the same clock reported by GET /v1/device/discover. What differs is the environment it runs in, not the logic. Refs #57.
Discovery no longer depends on the server's environment. "Find clock" runs the server's own browse first — that path keeps working exactly as before — and falls back to this Mac's browse when the server returns nothing, which is what a bridge-networked container always does. The caption names which scan produced the list, so one button never hides which half of the system answered. The same rows appear in Connection (during first-run, before the server has ever seen the clock) and in Device, where an unreachable clock now auto-expands the Clock section and prompts the app-side scan as the fix instead of telling the user to "check discovery". Picking a clock still PUTs /v1/device/config: the server remains the only writer to the device. Refs #57.
The architecture doc's discovery section said mDNS "requires host/macvlan networking" in both directions. That is still true of the server, but the clock is now findable from the app regardless, and the reason the duplicate browse exists (environment, not logic) belongs next to the server's own implementation. Refs #57.
load()'s generic catch folded APIError.notConfigured and .transport into the clock-unreachable branch, so a fresh install with no server URL was told to run clock discovery — which finds the clock, then fails to save it because the PUT has nowhere to go. Same dead end with the server down or the VPN off. The server already draws the line: its /v1/device/* proxies answer 502 when the clock, not the server, is the thing that didn't respond. DeviceFailure.classify keys off that, so discovery is only offered when it can actually help; server-side failures point back at Connection, as the pre-branch message did. Refs #57.
NWBrowser replays the entire result set on every change and mDNS answers trickle in over the first seconds, so the missing de-dupe meant ~N TCP connections per callback on a LAN with N _http._tcp instances — and _http._tcp is the busiest type on a home network. The base-URL guard in fingerprint() can't prevent it: that key only exists after the connection has already succeeded, which is the cost being paid. Claim the Bonjour instance before creating the NWConnection, mirroring ServerDiscovery's guard, and drop the claims on stop() so a re-scan still retries a service that failed to resolve. Refs #57.
A probe outlives the browse by up to its 3s resource timeout. Uncancelled it lands after stop() has cleared `clocks` and repopulates a list the user already dismissed. Hold the tasks and cancel them with the scan. Refs #57.
Picking wiped the whole list, so choosing the other of two discovered clocks meant a full re-scan; the tick moving as currentBaseURL updates already shows which one is live. The success path also left a previous "Couldn't switch clock" error on screen. Refs #57.
Code review (independent pass)Reviewed against Verdict: ship with follow-ups. Two findings were regressions and are fixed in this PR ( Verification actually run
A SourceKit diagnostic claiming "Main actor-isolated static property Fixed in this PR
Deferred — see the PR descriptionBrowse lifecycle (nothing stops the browse when the 4 s window ends; teardown rides on a Confirmed sound
|
The 502-vs-transport split was unobservable in the case it was written for. The server gives its request to the clock 8s before answering 502; the app's shared session aborts every request at 5s. So a blackholed clock IP — issue #56, the reason app-side discovery exists — surfaced as APIError.transport, classified as "server unreachable", and the Device tab hid the discovery prompt it should have been showing. The previous message was wrong for a different reason; this one was wrong for the right one. Route /v1/device/* through a session with a 12s budget so the server's 502 wins the race, leaving the 5s fail-fast on every other route. A URLSession timeout now maps to APIError.timeout rather than .transport, and DeviceFailure.timedOut blames neither end: it out-waits the server's own budget, so at that point either could be at fault. Refs #57.
Removing the message that steered fresh installs into the scan left the trap itself: with no server URL the browse still ran, listed the clock, and failed on pick with "Couldn't switch clock: Server not configured". Gate the button and the rows on DeviceService.isConfigured and say why, so the dead end is closed at the entrance rather than at the exit. Refs #57.
A claim taken before the NWConnection was never given back, so a host that answered EHOSTUNREACH or refused once — ESP32 mid-reboot, no ARP entry yet, Wi-Fi power save — was written off for the whole scan and the user got "No clock found" for a clock that was there. Release the claim when a connection ends without reaching .ready; in-flight and successful resolves keep theirs, which is where the per-callback fan-out lives. finish() now removes from `pending` before acting, so the .cancelled that follows our own cancel of a ready connection can't be mistaken for a failed resolve and hand the claim back. The claim/connect ordering is what the fan-out fix rests on, so the seam is now claimNew() — the batch selection resolve() actually loops over — rather than a Set-semantics helper that would stay green if the claim moved after NWConnection(to:). The probe transport is injectable so the cancel-before-write ordering is testable too; both new tests were checked against mutants. Refs #57.
Verification pass on the four fix commits — and a third roundAn adversarial pass over P1 — the 502 split was unobservable in the case it was written for
So for a blackholed clock IP or a hanging ESP32 — issue #56's case, the whole reason app-side discovery exists — URLSession aborted first, producing
12s was chosen, not measured: it clears the 8s budget by 4s. Cost is that P2 — the fresh-install trap was only half-closed
P3 — a transient resolve failure hid a clock for the whole scanThe claim in
Test quality — the gap was real and is closedThe two original
Verified sound, left alone
Cross-language contract
Commands (actual output)Still unverified at runtimeNo LAN, clock, or running server was available. The real |
1 participant
Summary
The macOS app gains a client-side mDNS browse for the AWTRIX clock, so discovery no longer depends on the server's Docker networking. Complements the server-side self-heal shipped in #56 (Fix A of #55) — this is Fix B.
The app browses
_http._tcpand fingerprints each candidate with the same/api/statsuidcheck the server uses, then configures the server viaPUT /v1/device/config.How it surfaces
FindClockView./v1/device/settings), the Clock section auto-expands with "The server can't reach the clock. Find it from this Mac."Automatic fallback, not two buttons: the server's
GET /v1/device/discoverruns first and is unchanged, so the existing path can't regress; the local browse only runs if the server's scan comes up empty. The caption always discloses which one answered — "Found by the Ember server" vs "Found by this Mac — the server's own scan came up empty".Changes
EmberKit (logic, testable, no UI coupling)
ClockDiscovery.swift(new) —@MainActor @ObservableNWBrowserover_http._tcp, resolving each service (IPv4-forced, mirroring the server'sbaseURLForv4 preference) and probing<base>/api/stats. Purenonisolated statichelpers mirrorinternal/discovery:baseURL(host:port:)(port always explicit, 0→80, so candidates are byte-identical to the server's),candidate(host:baseURL:status:body:)(rejects non-200 / undecodable / empty-uid),probe(...)(session injectable),merged(_:adding:)(de-dup byuid, sorted by host). Structure followsServerDiscovery.swift.EndpointFormat.swift(new) —NWEndpoint.Hostrendering + RFC 6874 IPv6 authority formatting, extracted fromServerDiscoveryso both browsers share it. Moved verbatim; the two pre-existing IPv6 tests still pass.DeviceService.swift— newDeviceFailureclassification (unauthorized/serverUnreachable/clockUnreachable/other), so callers can tell "the clock is unreachable" from "the server is unreachable".DeviceModels.swift—DiscoveredClockgains a public init. No new endpoint:DeviceService.setConfigalready shapesPUT /v1/device/config.App (view layer, thin)
FindClockView.swift(new) — the shared Find-clock rows, fallback logic, Local Network grant path.DeviceTab.swift— usesFindClockView; failure handling switches onDeviceFailure.ConnectionTab.swift— new Clock section; its privateopenLocalNetworkSettingsmoved toSupport/SystemSettings.swift(two callers now).Info.plist—_http._tcpadded toNSBonjourServices(required for the browse).Docs —
docs/ARCHITECTURE.mddiscovery section,macos/README.md.Review fixes folded in
Four findings from review were fixed before this PR opened:
4dd8723—load()'s genericcatchreported "clock unreachable" for server failures too. Fresh install with no server URL →.notConfigured→ user was told to run discovery, the local browse genuinely found the clock, and the pick then failed with "Server not configured". Now only HTTP 502 drives that path.3ee0ecb—resolve()had droppedServerDiscovery's dedup guard.NWBrowserreturns the full result set on every change, so this opened ~N connections per callback;_http._tcpis crowded on a home LAN (printers, NAS, Sonos, ESPHome, HomeKit bridges). Now claims eachservice(name,type,domain)before constructing anyNWConnection.c0591e7— in-flight probe tasks survivedstop()and could repopulate a dismissed list up to 3 s later. Now held and cancelled.2df88b8—pick()wiped the candidate list (picking the wrong one of two clocks forced a full re-scan) and never cleared a stale error on success.Test plan
swift test --package-path macos→ 172 tests pass (151 onmain; +21). New coverage: fingerprint accept/reject, base-URL shaping incl. port-0 and IPv6 bracketing, uid de-dup + ordering, probe request shaping over a stubbed transport,PUT /v1/device/configbody/auth, theDeviceFailuresplit, and the resolve-claim/clear-on-stop behaviour. No test touches real mDNS.xcodegen generate→ OK;xcodebuild -scheme Ember -destination 'platform=macOS' clean build→ BUILD SUCCEEDED, 0 errors.git diff --name-only origin/main..HEAD -- '*.go' go.mod→ empty).Not verified
No LAN, clock, or running server was available, so these are unverified at runtime: the real
_http._tcpbrowse, the probe against an actual AWTRIX device, the Local Network permission prompt (theNSBonjourServicesentry only takes effect in an installed build), and the live 502 path. The 502 split is verified at the classification level only.Note:
xcodebuildneedsCODE_SIGNING_ALLOWED=NO CODE_SIGN_IDENTITY=-on a machine without the Developer ID identity —project.ymlpins it and the post-compile producer signing script needs it. Pre-existing, unrelated to this change.Known follow-ups (deliberately deferred)
List-row.onDisappear. Switching tabs mid-scan can leak a browse; row recycling could in principle kill one early. Wants a structured.task-scoped rework.hostfield parity — the server reports the mDNS hostname (awtrix_116ae8.local.), the app reports the resolved IP, so a locally-found row renders the IP twice andmergedsorts by IP string. The browse result does carry the instance name.resolve()pins v4, soEndpointFormat's bracketing path is unreachable fromClockDiscoveryand a v6-only segment would find nothing where the server would. Safe direction (app rejects what the server accepts), butbaseURLhas no link-local/zone rejection, unlike Go'sbaseURLFor..needsAccessconflates "Local Network denied" with "Wi-Fi off" (inherited fromServerDiscovery); the grant button shows unconditionally; and the selected-clock checkmark won't match a portless configured URL likehttp://192.168.0.14(pre-existing — the old picker compared the same way).Closes #57.