diff --git a/Sources/KitchenSyncApp/Models/ClockFault.swift b/Sources/KitchenSyncApp/Models/ClockFault.swift new file mode 100644 index 0000000..f251a52 --- /dev/null +++ b/Sources/KitchenSyncApp/Models/ClockFault.swift @@ -0,0 +1,36 @@ +import Foundation + +/// Whether the device's clock task is faulting **right now**. +/// +/// The tick-health counters are LIFETIME totals, not windowed — the firmware is explicit about it +/// (`ks_tick_health.h`: *"Lifetime counters, not windowed — a worst-case that scrolled past is not +/// a measurement."*). So a non-zero `droppedTicks` says only that the device dropped a tick at +/// SOME point, which for a real unit is usually boot. Warning on `> 0` lights an amber dot forever +/// on a perfectly healthy box, and a warning that is always on is one nobody reads. +/// +/// What matters is the RATE. The app polls continuously, so it can simply look at the delta. +enum ClockFault: Equatable { + case none + /// The counters MOVED between polls. The clock task is dropping ticks now — this is the class + /// of fault ESP-027 was about, and it is worth an alarm. + case droppingNow + + /// `previous == nil` — the first sample of a session. There is no rate yet, so there is nothing + /// to say. A device that booted with 47 drops must not alarm the instant you open its screen; + /// that is the exact bug this replaces. + /// + /// A counter going DOWN means the device REBOOTED (lifetime counters zero on restart) — and + /// this app reboots devices deliberately, via WRITE & REBOOT and OTA. A reset is not a fault, + /// and must not be read as one or as a huge negative rate. + static func between(previous: TickHealth?, current: TickHealth) -> ClockFault { + guard let previous else { return .none } + + if current.droppedTicks < previous.droppedTicks || current.overruns < previous.overruns { + return .none // rebooted + } + + let dropped = current.droppedTicks > previous.droppedTicks + let overran = current.overruns > previous.overruns + return dropped || overran ? .droppingNow : .none + } +} diff --git a/Sources/KitchenSyncApp/ViewModels/DeviceDetailViewModel.swift b/Sources/KitchenSyncApp/ViewModels/DeviceDetailViewModel.swift index 698ae4a..eefc786 100644 --- a/Sources/KitchenSyncApp/ViewModels/DeviceDetailViewModel.swift +++ b/Sources/KitchenSyncApp/ViewModels/DeviceDetailViewModel.swift @@ -61,9 +61,23 @@ final class DeviceDetailViewModel: ObservableObject { @Published private(set) var statusAvailability: StatusAvailability = .unknown + /// Whether the clock task is faulting RIGHT NOW — a rate, not a lifetime total. + /// + /// The tick counters are lifetime, so `droppedTicks > 0` lights forever on a device that + /// dropped a tick at boot and has been perfect since. Seen on real hardware: `drop:47`, static + /// for the whole session. Only a rising counter is news. See `ClockFault`. + @Published private(set) var clockFault: ClockFault = .none + + /// The previous poll's tick health — the other half of a rate. + private var lastTick: TickHealth? + func refreshStatus() async { do { let fresh = try await client.fetchStatus() + if let tick = fresh.tickHealth { + clockFault = .between(previous: lastTick, current: tick) + lastTick = tick + } status = fresh statusAvailability = .available isRebooting = false // the device answering IS the signal diff --git a/Sources/KitchenSyncApp/Views/DeviceDetailView.swift b/Sources/KitchenSyncApp/Views/DeviceDetailView.swift index bf443ca..6d29f19 100644 --- a/Sources/KitchenSyncApp/Views/DeviceDetailView.swift +++ b/Sources/KitchenSyncApp/Views/DeviceDetailView.swift @@ -412,11 +412,26 @@ struct DeviceDetailView: View { DisclosureGroup { VStack(spacing: 6) { if let t = vm.status?.tickHealth { - DiagRow("dropped ticks", "\(t.droppedTicks)", warn: t.droppedTicks > 0) + // These are LIFETIME totals — the firmware says so. A non-zero value means + // "this happened at some point", usually at boot, and colouring it amber + // forever is how a real, healthy device ends up permanently flagged. The + // number is still worth SHOWING; it just isn't an alarm. Only a counter + // that is MOVING right now warns (see `hasDiagWarning`). + DiagRow("dropped ticks", "\(t.droppedTicks) since boot", + warn: vm.clockFault == .droppingNow) DiagRow("bursts", "\(t.bursts)") DiagRow("max gap µs", "\(t.maxGapMicros)") DiagRow("max work µs", "\(t.maxWorkMicros)") - DiagRow("overruns", "\(t.overruns)", warn: t.overruns > 0) + DiagRow("overruns", "\(t.overruns) since boot", + warn: vm.clockFault == .droppingNow) + + if vm.clockFault == .droppingNow { + Text("The clock task is dropping ticks RIGHT NOW.") + .font(.ksMono(11)) + .foregroundStyle(KS.amberText) + .frame(maxWidth: .infinity, alignment: .leading) + .padding(.top, 2) + } } if let p = vm.status?.phaseHealth { // maxStepMicros is the number that, unread, cost 138 seconds of @@ -448,11 +463,14 @@ struct DeviceDetailView: View { } } - /// Only unambiguous signals. A dropped tick or an overrun is a fault by - /// definition — no threshold needed, and none invented. + /// A RATE, not a lifetime total. + /// + /// This used to be `droppedTicks > 0`. But those are LIFETIME counters, so a real device that + /// dropped 47 ticks at boot and has been flawless since lit an amber warning **forever** — and + /// a warning that is always on is one nobody reads, which makes it worthless on the day it + /// matters. Only a counter that MOVES between polls is news. See `ClockFault`. private var hasDiagWarning: Bool { - guard let t = vm.status?.tickHealth else { return false } - return t.droppedTicks > 0 || t.overruns > 0 + vm.clockFault == .droppingNow } } diff --git a/Tests/KitchenSyncAppTests/ClockFaultTests.swift b/Tests/KitchenSyncAppTests/ClockFaultTests.swift new file mode 100644 index 0000000..16d644b --- /dev/null +++ b/Tests/KitchenSyncAppTests/ClockFaultTests.swift @@ -0,0 +1,64 @@ +import XCTest +@testable import KitchenSyncApp + +/// TDD: written before `ClockFault` exists. +/// +/// **Found on real hardware, 2026-07-14.** The diagnostics dot was `droppedTicks > 0` — but those +/// are LIFETIME counters (`ks_tick_health.h`: *"Lifetime counters, not windowed — a worst-case +/// that scrolled past is not a measurement."*). +/// +/// So a real Touch reporting `drop:47`, accumulated once at boot, lit an amber warning **forever** +/// on a perfectly healthy device. Sampled every 4s for 32s, `drop` never moved off 47 and `over` +/// stayed 0. A warning that is always on is a warning nobody reads — and it will be ignored on the +/// one day it means something. +/// +/// The distinction that matters is the RATE, and the app polls continuously, so it can see it: +/// - non-zero but STATIC -> dropped ticks at boot, fine now. Say nothing. +/// - INCREASING -> the clock task is dropping ticks RIGHT NOW. This is ESP-027. +final class ClockFaultTests: XCTestCase { + private func tick(dropped: UInt32, overruns: UInt32 = 0) -> TickHealth { + TickHealth(droppedTicks: dropped, bursts: 0, maxGapMicros: 1487, maxWorkMicros: 229, + overruns: overruns, core: 1, beatsWritten: 0, clockPulsesWritten: 0, reprimes: 0) + } + + /// THE BUG. The real bench device: 47 dropped ticks at boot, static ever since. + func test_a_static_lifetime_counter_is_not_a_fault() { + let fault = ClockFault.between(previous: tick(dropped: 47), current: tick(dropped: 47)) + XCTAssertEqual(fault, .none, + "47 dropped at boot with none since is a healthy device — do not cry wolf") + } + + /// The one we actually want to catch: the counter MOVED. + func test_a_rising_dropped_count_is_a_fault() { + let fault = ClockFault.between(previous: tick(dropped: 47), current: tick(dropped: 48)) + XCTAssertEqual(fault, .droppingNow) + } + + func test_a_rising_overrun_count_is_a_fault() { + let fault = ClockFault.between(previous: tick(dropped: 47, overruns: 0), + current: tick(dropped: 47, overruns: 1)) + XCTAssertEqual(fault, .droppingNow) + } + + /// The first sample has nothing to compare against. A device that booted with 47 drops must + /// not alarm the instant you open its screen — that is the very bug being fixed. + func test_the_first_sample_cannot_know_a_rate_and_must_not_alarm() { + let fault = ClockFault.between(previous: nil, current: tick(dropped: 47)) + XCTAssertEqual(fault, .none) + } + + /// **The reboot trap.** Lifetime counters reset to 0 when the device restarts — and this app + /// reboots devices deliberately (WRITE & REBOOT, OTA). A counter going DOWN is a reboot, not a + /// fault, and must not be read as a fault or as a huge negative rate. + func test_counters_resetting_after_a_reboot_is_not_a_fault() { + let fault = ClockFault.between(previous: tick(dropped: 47, overruns: 3), + current: tick(dropped: 0, overruns: 0)) + XCTAssertEqual(fault, .none, "a reboot zeroes lifetime counters — that is not a clock fault") + } + + /// And immediately after that reboot, a genuine new drop still faults. + func test_a_drop_after_a_reboot_still_faults() { + let fault = ClockFault.between(previous: tick(dropped: 0), current: tick(dropped: 1)) + XCTAssertEqual(fault, .droppingNow) + } +} diff --git a/project.yml b/project.yml index 1439214..e500fec 100644 --- a/project.yml +++ b/project.yml @@ -32,6 +32,12 @@ targets: settings: base: PRODUCT_BUNDLE_IDENTIFIER: dev.ericdahl.kitchensync + # Signing for on-device runs. Xcode manages the profile; this just names the team. + # NOTE: on a real phone iOS will PROMPT for Local Network access on first launch. + # Deny it and the app sees nothing at all — Bonjour browsing and `.local` + # resolution both die silently, with no error to go on. + DEVELOPMENT_TEAM: 5HR8E5CWR7 + CODE_SIGN_STYLE: Automatic SWIFT_VERSION: "5.0" TARGETED_DEVICE_FAMILY: "1,2" MARKETING_VERSION: "0.1.0" diff --git a/tasks/T-018.md b/tasks/T-018.md new file mode 100644 index 0000000..ebc67fc --- /dev/null +++ b/tasks/T-018.md @@ -0,0 +1,77 @@ +--- +id: T-018 +title: Diagnostics must warn on a RATE, not a lifetime counter (alarm fatigue) +status: done +assignee: null +priority: high +tags: + - hardware + - diagnostics +depends_on: [] +created_at: 2026-07-14 +updated_at: 2026-07-14 +--- + +## Goal + +**The diagnostics warning dot is permanently lit on a healthy device.** Found on real hardware, +2026-07-14. + +`DeviceDetailView.hasDiagWarning` is: + +```swift +return t.droppedTicks > 0 || t.overruns > 0 +``` + +But **these are LIFETIME counters** — the firmware says so explicitly (`ks_tick_health.h`: +*"Lifetime counters, not windowed — a worst-case that scrolled past is not a measurement."*). + +So the real `kstouch-dfd0` reports `drop:47`, accumulated **once at boot**, and the dot is amber +**forever**. Sampled every 4s for 32s: `drop` never moved off 47, `over` stayed 0. The clock is +perfectly healthy and the app says otherwise, permanently. + +**That is alarm fatigue by construction.** A warning that is always on is a warning nobody reads, +and it will be ignored on the one day it means something. + +## The actual distinction + +| | means | what a user should do | +|---|---|---| +| `drop` non-zero but **static** | the device dropped ticks at some point (boot, WiFi association) and is fine now | nothing | +| `drop` **increasing** between polls | the clock task is dropping ticks RIGHT NOW | this is the fault ESP-027 was about — investigate | + +The app polls `/status` continuously, so it can see the delta. It just isn't looking. + +## Acceptance Criteria + +- [ ] The warning fires on an INCREASE between polls, not on a lifetime total being non-zero +- [ ] A device with `drop:47` static shows **no** warning +- [ ] A device whose `drop` climbs between polls **does** warn +- [ ] The counters are still DISPLAYED (they're real data — just not alarming) +- [ ] Distinguish visually: "47 dropped since boot" reads differently from "dropping now" +- [ ] Survives a device reboot, where lifetime counters reset to 0 — a decrease must not be + mistaken for a fault, or read as a huge negative rate +- [ ] Written test-first per the /tdd rule, with fixtures for static / rising / reset-after-reboot + +## Notes + +**This is the same error, a fourth time.** Today already: a `/config.json` 404 is not a dead network +(T-009); a decode failure is not an unreachable device (T-016); one dropped poll is not a +disconnection (T-010). Now: a lifetime counter is not a live fault. Every one of them is the same +shape — **two different situations collapsed into one signal, and the user sent to fix the wrong +thing.** + +Worth generalising the rule rather than fixing it a fifth time: *before showing a warning, ask what +the user would DO about it. If two causes need different actions, they need different signals.* + +`maxStepMicros` is deliberately still un-warned — nobody has told us a threshold, and inventing one +would be worse than showing the raw number. That decision stands and is unaffected by this task. + +Bench evidence (kstouch-dfd0, current firmware, 2026-07-14): + +``` +t+ 4s drop=47 over=0 gap=1487 work=229 pulses=8550 +t+32s drop=47 over=0 gap=1487 work=229 pulses=8550 ← identical, 8 samples +``` + +## Progress diff --git a/tasks/T-019.md b/tasks/T-019.md new file mode 100644 index 0000000..c982340 --- /dev/null +++ b/tasks/T-019.md @@ -0,0 +1,70 @@ +--- +id: T-019 +title: Fall back to the resolved IP when .local resolution flakes +status: todo +assignee: null +priority: medium +tags: + - discovery + - hardware +depends_on: [] +created_at: 2026-07-14 +updated_at: 2026-07-14 +--- + +## Goal + +**A live device can look unreachable because `.local` resolution flaked, not because the device +went anywhere.** Observed on real hardware, 2026-07-14, mid-session: + +``` +curl http://192.168.0.101/status -> HTTP 200 (the device, answering fine) +curl http://kstouch-dfd0.local/status -> timeout +dns-sd -B _http._tcp local -> kstouch-dfd0 (STILL ADVERTISING) +``` + +Bonjour **browse** worked. Bonjour **resolution** didn't. The device never moved. + +The app builds every request URL from `.local` (`KitchenSyncDevice.host`), so when that +happens the app reports the device UNREACHABLE (T-010's three-strike counter) even though it is +sitting there answering on its IP. + +## The trade this re-opens + +`KitchenSyncDiscovery`'s doc comment makes a deliberate choice, and it's a good one: + +> *"Deliberately does NOT resolve the matched service to a raw IP via NWConnection… Instead this +> hands the hostname straight back as `.local`, and URLSession resolves that through the +> system's mDNS responder exactly the way Safari resolves it… one resolution path, not two."* + +The reasoning holds — one resolution path is easier to debug than two. **But we have now seen that +path fail on a device that was fine**, so the cost of the trade is no longer hypothetical. + +## Acceptance Criteria + +- [ ] Discovery captures the resolved IP alongside the hostname (an `NWBrowser` result can be + resolved via `NWConnection`, or via the `.service` endpoint's resolution) +- [ ] `.local` stays the PRIMARY path — it is the one Safari uses and the one users can type +- [ ] On repeated `.local` failures, retry against the last known IP before declaring UNREACHABLE +- [ ] A device that has genuinely gone away is still reported UNREACHABLE (T-010 preserved — do not + trade one false signal for another) +- [ ] The IP is treated as a CACHE, not as truth: DHCP moves, and a stale IP that now belongs to + someone else's box must not be talked to. Re-resolve on success; discard on mismatch. +- [ ] Written test-first per the /tdd rule + +## Notes + +**Be careful what this fixes.** T-016 already established that a device answering in a dialect we +can't read is `.unsupported`, not `.unreachable`. This adds a third case: a device that is *there* +but whose NAME won't resolve. Three distinct failures, three distinct signals — do not collapse +them, which is the mistake this codebase keeps making (see T-009, T-010, T-016, T-018). + +**A stale IP is a real hazard, not a theoretical one.** DHCP hands addresses around; if we cache +192.168.0.101 and the Touch later moves, that address may belong to a laptop. Talking to it is at +best useless and at worst confusing. Trust the name; use the IP only as a fallback, and only until +the name works again. + +Consider whether this is worth doing at all before ESP-031 lands (TXT records) — the same +`NWConnection` resolution that yields the IP is where TXT gets read, so the two touch the same code. + +## Progress