From f8fecfdcefd909392cb4a7fd3c13ca6303cc6db5 Mon Sep 17 00:00:00 2001 From: Eric Dahl Date: Tue, 14 Jul 2026 16:03:57 -0400 Subject: [PATCH 1/6] feat(discovery): recognise the headless X32Link (ESP-035) "I see kstouch. not the headless." The X32Link was powered, on the network, and following the Link session, and the app showed nothing. Adding `x32link` to the hostname prefix list is the small half of this. The real cause was in the firmware: that unit had NO mDNS WHATSOEVER -- it never advertised, so no matcher on this side could have found it, and `dns-sd` could not see it either. Fixed in link-devices (ESP-035), where it now announces `x32link-` like the rest of the fleet and serves the shared /status and /config.json. The prefix list stays a documented HEURISTIC, not a guarantee -- a TXT record is still the fix that would make identity a fact. Tests pin both halves: the headless is one of ours, and a stranger's box at a venue still is not. Verified against the real unit: both devices now appear in the list, live BPM. Claude-Session: https://claude.ai/code/session_01F7YsQgso3KNpeXbP5vNVj6 --- .../KitchenSyncApp/Discovery/DeviceMatch.swift | 11 ++++++++--- .../KitchenSyncAppTests/DeviceMatchTests.swift | 17 +++++++++++++++++ 2 files changed, 25 insertions(+), 3 deletions(-) diff --git a/Sources/KitchenSyncApp/Discovery/DeviceMatch.swift b/Sources/KitchenSyncApp/Discovery/DeviceMatch.swift index 459832b..bd973f4 100644 --- a/Sources/KitchenSyncApp/Discovery/DeviceMatch.swift +++ b/Sources/KitchenSyncApp/Discovery/DeviceMatch.swift @@ -30,11 +30,16 @@ enum DeviceMatch { /// units working, not because it is correct. /// /// The firmware advertises generic `_http._tcp` with no TXT record, so the hostname is all a - /// client has. `ks_hostname()` builds `-`: `kitchensync-a4f2` on - /// the P4, `kstouch-dfd0` on the Touch. Missing `kstouch` is what made a real device invisible. + /// client has. The hostname is `-`: `kitchensync-a4f2` on the + /// P4, `kstouch-dfd0` on the Touch, `x32link-7a1c` on the headless (ESP-035). Missing + /// `kstouch` is what made a real device invisible once already. private static func matchesKnownHostname(_ name: String) -> Bool { knownHostPrefixes.contains { name.hasPrefix($0) } } - private static let knownHostPrefixes = ["kitchensync", "kstouch"] + /// `x32link` joined this list in ESP-035 — but adding it here was the SMALL half. That + /// unit had no mDNS whatsoever: it never advertised, so no matcher could have found it. + /// The user saw it exactly that way — "I see kstouch, not the headless" — while the box + /// sat there powered, on the network, and following the session. + private static let knownHostPrefixes = ["kitchensync", "kstouch", "x32link"] } diff --git a/Tests/KitchenSyncAppTests/DeviceMatchTests.swift b/Tests/KitchenSyncAppTests/DeviceMatchTests.swift index 914ea1b..e7bf692 100644 --- a/Tests/KitchenSyncAppTests/DeviceMatchTests.swift +++ b/Tests/KitchenSyncAppTests/DeviceMatchTests.swift @@ -47,4 +47,21 @@ final class DeviceMatchTests: XCTestCase { txt: ["dev": "some-other-product"]), "a TXT record that says it ISN'T ours must beat a hostname that says it is") } + + /// ESP-035: the headless X32Link. It was the last unit in the fleet with NO mDNS at + /// all -- it never advertised, so it could not be found no matter what this matcher + /// did. Now it announces `x32link-`, and the prefix list has to know the name. + /// + /// The user hit this exactly: "I see kstouch. not the headless." The device was + /// powered, on the network, and following the Link session. + func test_the_headless_x32link_is_one_of_ours() { + XCTAssertTrue(DeviceMatch.isKitchenSync(serviceName: "x32link-7a1c", txt: nil)) + } + + /// The prefix is still a HEURISTIC, and it must not swallow the whole network. A + /// stranger's box at a venue is not ours. + func test_a_stranger_is_still_not_one_of_ours() { + XCTAssertFalse(DeviceMatch.isKitchenSync(serviceName: "brother-printer", txt: nil)) + XCTAssertFalse(DeviceMatch.isKitchenSync(serviceName: "x32-mixer", txt: nil)) + } } From 913e74781b266b07c718157f4748f0686d36c923 Mon Sep 17 00:00:00 2001 From: Eric Dahl Date: Wed, 15 Jul 2026 00:12:03 -0400 Subject: [PATCH 2/6] chore(tasks): file T-020..T-024 from architecture review (deepening opportunities) --- tasks/T-020.md | 79 ++++++++++++++++++++++++++++++++++++++++++++++++++ tasks/T-021.md | 68 +++++++++++++++++++++++++++++++++++++++++++ tasks/T-022.md | 60 ++++++++++++++++++++++++++++++++++++++ tasks/T-023.md | 66 +++++++++++++++++++++++++++++++++++++++++ tasks/T-024.md | 62 +++++++++++++++++++++++++++++++++++++++ 5 files changed, 335 insertions(+) create mode 100644 tasks/T-020.md create mode 100644 tasks/T-021.md create mode 100644 tasks/T-022.md create mode 100644 tasks/T-023.md create mode 100644 tasks/T-024.md diff --git a/tasks/T-020.md b/tasks/T-020.md new file mode 100644 index 0000000..4c57c57 --- /dev/null +++ b/tasks/T-020.md @@ -0,0 +1,79 @@ +--- +id: T-020 +title: Give the form-field wire grammar one home +status: todo +assignee: null +priority: high +tags: + - architecture + - refactor + - protocol +depends_on: [] +created_at: 2026-07-15 +updated_at: 2026-07-15 +--- + +## Goal + +**One firmware fact — the form-field wire grammar — is copied into four places, and nothing but a +test keeps the copies aligned.** The `KsLiveEdit`-case ↔ wire-key ↔ config-field relationship is a +single thing (it comes from `ks_config.c`'s `ks_config_set()`), but it has no single owner: + +- `Models/KsConfig.swift:90-97` — `CodingKeys` (decode side) +- `Models/KsConfig.swift:210-262` — `saveFormFields` (save-encode) + `boolField`/`colorField` +- `Models/KsLiveEdit.swift:32-54` — `formField` (live-encode) + `boolValue`/`colorValue` +- `Models/KsConfig.swift:276-298` — `apply(_:)` (fold an accepted edit back in) + +The value encoders are byte-for-byte duplicated: + +```swift +// KsLiveEdit.swift:54 +private func colorValue(_ v: UInt32) -> String { String(format: "#%06X", v & 0xFFFFFF) } +// KsConfig.swift:260-262 +fileprivate static func colorField(_ value: UInt32) -> String { String(format: "#%06X", value & 0xFFFFFF) } +``` + +and `boolValue` / `boolField` both return `v ? "1" : "0"`. The keys themselves (`led_beat`, +`clk\(i)_phase`, …) appear in both `formField` and `saveFormFields`. + +This is a **shallow seam with no locality**: the grammar is a deep fact spread thin across four +modules. Deepen it into one module that owns the key strings and the value encoders, keyed by +`KsLiveEdit` case, so decode / save / live / apply all read from one place. + +## Acceptance Criteria + +- [ ] A single module owns each field's wire key and its value encoding (bool, color, int) +- [ ] `saveFormFields`, `KsLiveEdit.formField` and `apply(_:)` all reference that one source — no + duplicated key literals, no second copy of the `#%06X` / `"1"/"0"` encoders +- [ ] The live-vs-reboot partition is **derived from the types**, not reconstructed by a test that + reaches across three interfaces (see `LiveRebootPartitionTests`, and the Notes below) +- [ ] `LiveRebootPartitionTests` still passes — but ideally it now asserts a property the types + already guarantee, rather than being the only thing that guarantees it +- [ ] Every existing decode/encode test still passes against fixtures built from real firmware + `snprintf` / `ks_config_set()` output +- [ ] Written test-first per the /tdd rule + +## Notes + +**This touches the single most important rule in the codebase — read it before starting.** The +split between live-editable and reboot-required IS the `KsLiveEdit` case list (CLAUDE.md, and the +codebase-memory ADR). The three defenses must survive untouched: + +1. Every control on `DeviceDetailView` routes through a `KsLiveEdit` case — that screen *cannot* + reboot the device. +2. Reboot-required settings live only in `DeviceSettingsSheet`, behind the consequence band. +3. `LiveRebootPartitionTests` asserts every `saveFormFields` key is either live-editable or in + `KsConfig.rebootRequiredFormKeys` — never both, never neither. + +This deepening **strengthens** that invariant (moves the guarantee from a test into the types); it +does **not** reopen the ADR. If at any point the refactor would let a reboot-only field become +routable through `/live`, stop — that is the exact failure the current design prevents by hand. + +The trap to preserve: `metro_accent`, `metro_vol`, `metro_voice` are live, but metronome *enable* +is not; `led` enable is live but `follow_beat` enable is not. The grammar module must not flatten +that distinction — it is not guessable from the field names. + +Source of this task: architecture review 2026-07-14 (candidate 01, the top recommendation). It is +also the root of T-021's duplication — the same "spread a decision across call sites" shape. + +## Progress diff --git a/tasks/T-021.md b/tasks/T-021.md new file mode 100644 index 0000000..fce24a5 --- /dev/null +++ b/tasks/T-021.md @@ -0,0 +1,68 @@ +--- +id: T-021 +title: One classifier for a failed device request, not four catch-ladders +status: todo +assignee: null +priority: high +tags: + - architecture + - refactor +depends_on: [] +created_at: 2026-07-15 +updated_at: 2026-07-15 +--- + +## Goal + +**"What kind of failure was that?" is a decision this app has been burned by repeatedly — and it is +hand-rolled at four call sites instead of living in one deep module.** The shape "map a thrown +`KitchenSyncClientError.httpStatus(404)` / `DecodingError` / everything-else onto a domain outcome" +appears four times, each producing a slightly different enum: + +- `ViewModels/DeviceDetailViewModel.swift:74-93` — `refreshStatus`: decode→`.unsupported`, else→`.unreachable` +- `ViewModels/DeviceDetailViewModel.swift:115-127` — `loadConfig`: 404→`.unsupportedByFirmware`, else→`.failed` +- `ViewModels/DeviceDetailViewModel.swift:159-171` — `apply`: 404→`.unsupportedByFirmware`, status→`.rejectedByDevice`, else→`.unreachable` +- `ViewModels/DeviceListViewModel.swift:123-166` — `refreshAll`: decode→`.undecodable`, else→`.noAnswer` + +`DeviceDetailViewModel.StatusAvailability` and `DeviceListViewModel.Reachability` are the **same +three-way concept** — answered-and-read / answered-but-undecodable / didn't-answer — classified +independently in two view models, both deriving it from an identical `catch is DecodingError`. + +Two problems: the transport/decode layer's error types (`KitchenSyncClientError`, `DecodingError`) +**leak across the seam** into every view model's `catch`, and the classification itself has **no +locality** — the "undecodable is not gone" rule is written as prose twice. + +Deepen: one pure `DeviceFault.classify(Error) -> DeviceFault` module (cases along the lines of +`undecodable` / `notFound` / `rejected(status)` / `unreachable`). Each view model maps that one +fault onto its own presentation enum. + +## Acceptance Criteria + +- [ ] A pure function classifies a thrown request error into a single `DeviceFault` value +- [ ] All four call sites above translate a `DeviceFault` into their local enum — none re-inspect + `KitchenSyncClientError` / `DecodingError` directly +- [ ] The classifier is tested **directly at its interface** (feed it errors, assert the fault) — + no need to drive a whole view model through `StubURLProtocol` to exercise the decision +- [ ] The distinct-signals invariant is preserved: undecodable, unreachable and old-firmware stay + three separate outcomes (T-009, T-010, T-016, T-018 — do not re-collapse them) +- [ ] `DeviceListViewModel`'s consecutive-miss counting stays where it is — that is list policy, not + classification, and only `noAnswer` feeds it +- [ ] Existing `DeviceDetailViewModelTests`, `LiveEditFeedbackTests`, `DeviceListViewModelTests`, + `UnsupportedDeviceTests` all still pass +- [ ] Written test-first per the /tdd rule + +## Notes + +The presentation enums genuinely differ (the list cares reachable/unsupported; the detail-config +path cares unsupportedByFirmware/failed), so the goal is **not** to unify the surface enums — it is +to stop re-deriving the underlying error classification four times. One classifier, four thin +translations. + +This is the recurring bug of the codebase in structural form: the same decision, made in more than +one place, drifts. Every one of T-009 / T-010 / T-016 / T-018 was "two situations collapsed into one +signal." Centralising the classification makes the fifth instance harder to write by accident. + +Source: architecture review 2026-07-14 (candidate 02). Pairs naturally with T-020 — both are +"a decision spread across call sites, give it one home." Do T-020 first (higher leverage). + +## Progress diff --git a/tasks/T-022.md b/tasks/T-022.md new file mode 100644 index 0000000..1936e1c --- /dev/null +++ b/tasks/T-022.md @@ -0,0 +1,60 @@ +--- +id: T-022 +title: Route the master transport through TransportAppearance too +status: todo +assignee: null +priority: medium +tags: + - architecture + - refactor + - transport +depends_on: [] +created_at: 2026-07-15 +updated_at: 2026-07-15 +--- + +## Goal + +**There are two state→face mappings for the transport, and only one of them is tested.** The +per-output button correctly routes through the pure, tested `TransportAppearance.appearance(for: +linkOwned:)` (`Models/TransportAppearance.swift:29-44`, consumed at `Views/TransportButton.swift:25-27`). +The **master** transport re-derives the same kind of decision inline in the View: + +- `Views/DeviceDetailView.swift:243-249` — `playFace` maps `TransportSummary → TransportFace` by hand +- `Views/DeviceDetailView.swift:230` — `blinks: vm.status?.transportSummary == .arming` inline +- `Views/DeviceDetailView.swift:213-220` — `summaryText` maps `TransportSummary → String` + +This is a decision (state → appearance) living in a `View`, which the codebase's own rule says +belongs on a model as a pure function — and it is a decision **on stage**, with no test behind it, +running parallel to a tested one it can silently diverge from. + +Deepen: extend `TransportAppearance` (or add a sibling constructor) to cover the master-summary +case, so both the per-output button and the master button cross one interface. + +## Acceptance Criteria + +- [ ] The master transport's face + blink decision is produced by `TransportAppearance` (or a + sibling pure function), not by `playFace` / inline expressions in the View +- [ ] That mapping is tested directly, the way `TransportAppearanceTests` tests the per-output case +- [ ] The two-buttons-not-one-toggle design is preserved (`DeviceDetailView.swift:222-241` — a single + derived toggle shows the wrong thing during a mixed arming/playing moment; keep PLAY and STOP + separate) +- [ ] Armed still blinks at beat rate; no soft `repeatForever` throb (armed is a cocked hammer) +- [ ] `summaryText` stringification either moves onto the model alongside the face, or is left as + pure presentation — pick one home, don't leave the decision half in the View +- [ ] Written test-first per the /tdd rule + +## Notes + +Scope check: `TransportButton`'s `TransportFace → Color` switches (`:112-149`) are pure presentation +and should **stay** in the View — `TransportAppearance`'s own doc says "turning a face into a +gradient is not [worth testing]." This task is only about the *state → face/blink* decision, which +is the part that belongs on the model. + +Watch the transport-is-quantized rule: the master face must be derived from the device-reported +`transportSummary`, never predicted client-side. No local timers for the arming→running flip. + +Source: architecture review 2026-07-14 (candidate 03). Lower leverage than T-020/T-021 but a clean, +contained win — one tested mapping instead of two, one of them blind. + +## Progress diff --git a/tasks/T-023.md b/tasks/T-023.md new file mode 100644 index 0000000..8e1e910 --- /dev/null +++ b/tasks/T-023.md @@ -0,0 +1,66 @@ +--- +id: T-023 +title: Give discovery a seam so the list view model can be tested with a fake +status: todo +assignee: null +priority: medium +tags: + - architecture + - refactor + - discovery +depends_on: [] +created_at: 2026-07-15 +updated_at: 2026-07-15 +--- + +## Goal + +**Discovery has no seam, so the list view model can't be tested with a fake — and a test has been +left reaching past the interface to compensate.** `DeviceListViewModel` news up its discovery +concretely, and `KitchenSyncDiscovery` news up `NWBrowser` concretely: + +- `ViewModels/DeviceListViewModel.swift:35` — `private let discovery = KitchenSyncDiscovery()` +- `Discovery/KitchenSyncDiscovery.swift:38` — `NWBrowser(for: .bonjour(...))` + +The tell is at `ViewModels/DeviceListViewModel.swift:98-108`: `merge(discovered:)` is left `internal` +"so a test can put a DISCOVERED device into the list without standing up Bonjour." The test reaches +*past* the interface because there is no discovery adapter to substitute. And the restart policy in +the glue — `.failed` → restart, a denial → deliberately do **not** restart +(`Discovery/KitchenSyncDiscovery.swift:44-52, 78-85`) — has **no test at all**, because `NWBrowser` +is hard-wired. + +Introduce a small interface (a `DeviceSource`: emits hostname sets + `DiscoveryStatus`) with two +adapters — the real `NWBrowser` one and an in-memory one for tests. Two adapters justify the seam. + +## Acceptance Criteria + +- [ ] A `DeviceSource` interface exists: it publishes discovered hostnames and status changes +- [ ] `DeviceListViewModel` takes a `DeviceSource` (injected, defaulting to the `NWBrowser` adapter) + instead of constructing `KitchenSyncDiscovery()` directly +- [ ] `merge(discovered:)` can return to `private` — tests drive discovery through the in-memory + adapter, not through an internal method +- [ ] The restart policy is now testable and tested: a `.failed` browser state triggers a restart; a + permission denial does **not** (it would just be refused again — see the doc comment) +- [ ] `DeviceMatch` stays exactly where it is — it is already a pure, tested decision; this task does + not touch the matcher, only the glue's dependency shape +- [ ] Written test-first per the /tdd rule + +## Notes + +**Don't over-fire on this one.** The pure decision (`DeviceMatch.isKitchenSync`) and the status +mapping (`DiscoveryStatus(browserState:)`) are already correctly extracted and tested — the latter +is a rare case where the test doesn't have to reach past the interface. This task is strictly about +(a) the view model's hard-wired discovery dependency and (b) the untested restart *policy* in the +glue. Everything behind `DeviceMatch` is fine. + +Real seam, not hypothetical: the second adapter (in-memory) is what makes tests honest, and the +`NWBrowser` adapter is the production one — two adapters, so the seam earns its place rather than +being speculative indirection. + +This intersects T-019 (fall back to resolved IP): the same `NWBrowser`/`NWConnection` resolution +path is where the IP and the future TXT record get read. If T-019 lands first, fold its resolution +into the same adapter rather than adding a second discovery path. + +Source: architecture review 2026-07-14 (candidate 04). + +## Progress diff --git a/tasks/T-024.md b/tasks/T-024.md new file mode 100644 index 0000000..43927de --- /dev/null +++ b/tasks/T-024.md @@ -0,0 +1,62 @@ +--- +id: T-024 +title: Collapse TransportTapIntent — a shallow single-caller module +status: todo +assignee: null +priority: low +tags: + - architecture + - refactor +depends_on: [] +created_at: 2026-07-15 +updated_at: 2026-07-15 +--- + +## Goal + +**`TransportTapIntent` is a shallow module — its interface is wider than its implementation, and it +has exactly one caller.** The whole implementation is one expression: + +```swift +// Models/TransportTapIntent.swift:10-12 +static func play(for state: TransportLaunchState) -> Bool { state == .stopped } +``` + +wrapped in an enum, a static func and an 8-line doc comment. The sole production caller is +`Views/TransportButton.swift:180` — `onTap(TransportTapIntent.play(for: state))`. + +Apply the **deletion test**: inline `state == .stopped` at the call site and complexity does **not** +reappear across N callers — there is exactly one, and it already holds the state. The module was +extracted purely to give the one-liner a unit test (`Tests/TransportTapIntentTests.swift`, 3 cases). +That is a hypothetical seam, not a real one. + +Consider inlining the rule and keeping the intent as a comment at the call site. + +## Acceptance Criteria + +- [ ] Decision made and recorded: inline `TransportTapIntent`, or consciously keep it (see the + counter-argument below) — either is acceptable, but the shallowness should be a choice, not an + accident +- [ ] If inlined: `TransportTapIntent.swift` and `TransportTapIntentTests.swift` are removed, the + rule lives at `TransportButton.swift:180` with a comment naming the intent, and the build + + remaining suite stay green +- [ ] If kept: a one-line note on why the module stays (e.g. the test is load-bearing + documentation) so a future review doesn't re-raise it + +## Notes + +**This is the lowest-confidence item in the review (Speculative), and deliberately so.** The +counter-argument is real: the test documents "a tap while stopped plays; otherwise it stops," a +transport rule this app cares about getting right, and un-extracting trades a test for a comment. +It's worth doing only if `play: state == .stopped` reads as obviously correct at the call site. + +This is the *inverse* of T-020/T-021/T-022 — not all extraction is deepening. A pure function pulled +out for a unit test, with one caller and an interface larger than its body, is shallow by +definition; naming that is the point of the review, even when the verdict is "leave it." + +No firmware/protocol risk here — `state == .stopped` is app-side transport intent, and the quantized +arm/fire behaviour is unaffected (this only decides which *request* a tap sends). + +Source: architecture review 2026-07-14 (candidate 05). + +## Progress From 7fe885998edbab4127feaabb57400060db0dc683 Mon Sep 17 00:00:00 2001 From: Eric Dahl Date: Wed, 15 Jul 2026 00:27:09 -0400 Subject: [PATCH 3/6] =?UTF-8?q?feat(app):=20set-tempo=20control=20?= =?UTF-8?q?=E2=80=94=20tap,=20numeric,=20=C2=B1=20(ESP-037,=20part=203)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The app can now set a clock box's free-run tempo, the last piece of ESP-037. - KsLiveEdit.setTempo(Double) -> the `bpm` /live field. Formatted en_US_POSIX so a European phone's decimal comma can't turn 128.5 into a "128" the firmware's atof stops at. Tap, numeric and ± all resolve to this one number. - KsConfig.tempo (Double?), decoded from `bpm`. nil = listener-only (X32Link omits it), the same nil-means-not-fitted rule as metronome/led. This is the SET tempo, distinct from KsStatus.bpm (the EFFECTIVE tempo Link drives) -- so the control doesn't jump when a session takes over. apply(.setTempo) only reflects it when tempo != nil. - TempoControl: the set surface, shown only when config.tempo != nil. tap -- computed HERE from local tap intervals, never sent as tap events over WiFi (jitter would wreck the timing); resolves to a BPM and sends it. numeric -- tap the readout, type it. ± -- nudge by 1. Clamped to 20..300 (the firmware's band). When a Link session is driving it says "LINK IS DRIVING" -- a set tempo the device is currently ignoring is exactly the silent lie this app keeps stamping out. Verified against the bench Touch protocol side: /live bpm applies on the wire and /config.json reads the stored tempo back; the app model decodes it and posts the same field. Full app suite green; installed on the phone. Claude-Session: https://claude.ai/code/session_01F7YsQgso3KNpeXbP5vNVj6 --- Sources/KitchenSyncApp/Models/KsConfig.swift | 19 ++- .../KitchenSyncApp/Models/KsLiveEdit.swift | 12 ++ .../Views/DeviceDetailView.swift | 10 ++ .../KitchenSyncApp/Views/TempoControl.swift | 132 ++++++++++++++++++ Tests/KitchenSyncAppTests/KsConfigTests.swift | 16 +++ .../KitchenSyncAppTests/KsLiveEditTests.swift | 9 ++ 6 files changed, 197 insertions(+), 1 deletion(-) create mode 100644 Sources/KitchenSyncApp/Views/TempoControl.swift diff --git a/Sources/KitchenSyncApp/Models/KsConfig.swift b/Sources/KitchenSyncApp/Models/KsConfig.swift index 844cd39..47ea7b9 100644 --- a/Sources/KitchenSyncApp/Models/KsConfig.swift +++ b/Sources/KitchenSyncApp/Models/KsConfig.swift @@ -87,13 +87,23 @@ struct KsConfig: Decodable, Equatable { var wifi: [WifiSlot] var clock: [ClockOutputConfig] + /// The device's STORED free-run tempo, BPM. **`nil` means this build can't set a + /// tempo** — it's listener-only (X32Link), so `/config.json` omits `bpm` + /// (`link-devices` ESP-037, gated by `KsCaps.settable_tempo`), same capability rule + /// as `metronome`/`led`. + /// + /// This is the tempo you SET — distinct from `KsStatus.bpm`, the tempo the device is + /// currently CLOCKING (Link's, when a session is driving). The tempo control shows + /// this one; it doesn't jump around when Link takes over. + var tempo: Double? + private enum CodingKeys: String, CodingKey { case clockOutEnabled = "clock_out", metronomeEnabled = "metronome" case metronomeAccent = "metro_accent", metronomeVolume = "metro_vol" case metronomeVoice = "metro_voice", ledEnabled = "led" case ledBrightness = "led_bright", ledMode = "led_mode", ledFade = "led_fade" case ledBeatColor = "led_beat", ledAccentColor = "led_accent" - case followBeatEnabled = "follow_beat", wifi, clock + case followBeatEnabled = "follow_beat", tempo = "bpm", wifi, clock } init(from decoder: Decoder) throws { @@ -130,6 +140,7 @@ struct KsConfig: Decodable, Equatable { } followBeatEnabled = try c.decodeIfPresent(Bool.self, forKey: .followBeatEnabled) + tempo = try c.decodeIfPresent(Double.self, forKey: .tempo) // ESP-037: nil = listener-only wifi = try c.decode([WifiSlot].self, forKey: .wifi) clock = try c.decode([ClockOutputConfig].self, forKey: .clock) } @@ -294,6 +305,12 @@ extension KsConfig { case .outputPhase(let i, let v): withOutput(i) { $0.phaseMilliBeats = v } case .outputSwing(let i, let v): withOutput(i) { $0.swingMilliBeats = v } case .outputFollowsLink(let i, let v): withOutput(i) { $0.followsLinkTransport = v } + + // ESP-037: only reflect the set tempo if this device HAS a settable one. On a + // listener-only config (tempo == nil) a set is meaningless and must not conjure + // a value the device doesn't keep — the same nil-means-not-fitted rule as the + // capability sections above. + case .setTempo(let v): if tempo != nil { tempo = v } } } diff --git a/Sources/KitchenSyncApp/Models/KsLiveEdit.swift b/Sources/KitchenSyncApp/Models/KsLiveEdit.swift index b46f8d4..4ae79c2 100644 --- a/Sources/KitchenSyncApp/Models/KsLiveEdit.swift +++ b/Sources/KitchenSyncApp/Models/KsLiveEdit.swift @@ -25,6 +25,12 @@ enum KsLiveEdit { case outputSwing(index: Int, Int) case outputFollowsLink(index: Int, Bool) + /// ESP-037: the device-global free-run tempo, BPM. Drives the clock when the device + /// is solo; a Link session still wins. Tap, numeric entry, and the ± steppers ALL + /// resolve to this one number — tap is computed in the app (local timing, no WiFi + /// jitter), never sent as tap events over the network. + case setTempo(Double) + /// The one `(key, value)` pair this edit POSTs. `/live` is a PARTIAL patch /// (`ks_form_apply` on the firmware) — fields not present keep their /// current value, so sending exactly one pair per edit is correct, not a @@ -47,9 +53,15 @@ enum KsLiveEdit { case .outputPhase(let i, let v): return ("clk\(i)_phase", String(v)) case .outputSwing(let i, let v): return ("clk\(i)_swing", String(v)) case .outputFollowsLink(let i, let v): return ("clk\(i)_follow", boolValue(v)) + case .setTempo(let v): return ("bpm", bpmValue(v)) } } private func boolValue(_ v: Bool) -> String { v ? "1" : "0" } private func colorValue(_ v: UInt32) -> String { String(format: "#%06X", v & 0xFFFFFF) } + /// Milli-BPM precision, locale-independent (the "." must not become a "," on a + /// European phone — the firmware parses with atof, which is C-locale). + private func bpmValue(_ v: Double) -> String { + String(format: "%.3f", locale: Locale(identifier: "en_US_POSIX"), v) + } } diff --git a/Sources/KitchenSyncApp/Views/DeviceDetailView.swift b/Sources/KitchenSyncApp/Views/DeviceDetailView.swift index 22b0562..0bf9409 100644 --- a/Sources/KitchenSyncApp/Views/DeviceDetailView.swift +++ b/Sources/KitchenSyncApp/Views/DeviceDetailView.swift @@ -35,6 +35,16 @@ struct DeviceDetailView: View { meterBridge masterTransport + // ESP-037: the set-tempo control, only when the device has a settable + // tempo (config.tempo != nil — a listener-only box omits it). + if let t = vm.config?.tempo { + TempoControl( + tempo: t, + linkDriving: (vm.status?.peers ?? 0) > 0, + onSet: { bpm in Task { await vm.apply(.setTempo(bpm)) } } + ) + } + if let failure = vm.liveEditFailure { liveEditBanner(failure) } diff --git a/Sources/KitchenSyncApp/Views/TempoControl.swift b/Sources/KitchenSyncApp/Views/TempoControl.swift new file mode 100644 index 0000000..767ea20 --- /dev/null +++ b/Sources/KitchenSyncApp/Views/TempoControl.swift @@ -0,0 +1,132 @@ +import SwiftUI + +/// The set-tempo surface (ESP-037). Three ways in, one number out: +/// +/// - **Tap** — computed HERE, from local tap intervals, never sent as tap events over +/// WiFi (network jitter would wreck the timing). The app resolves taps to a BPM and +/// sends that. +/// - **Numeric** — type it. +/// - **± steppers** — nudge by 1. +/// +/// All three call `onSet(bpm)`, which routes through `KsLiveEdit.setTempo` → `/live`. +/// Only shown when the device actually has a settable tempo (`config.tempo != nil`); +/// a listener-only box has no business drawing this. +/// +/// The number shown is the SET tempo (`config.tempo`), not the effective one the clock +/// is running (`status.bpm`) — so it doesn't jump when a Link session takes over. When +/// Link IS driving, the control says so, because a tempo you set that the device is +/// currently ignoring is exactly the kind of silent lie this app keeps stamping out. +struct TempoControl: View { + let tempo: Double + /// True when a Link session is currently driving the clock (peers > 0). The set + /// tempo still persists and applies the moment Link leaves — but say who's in charge. + let linkDriving: Bool + let onSet: (Double) -> Void + + static let minBPM = 20.0 + static let maxBPM = 300.0 + + @State private var editing = false + @State private var draft = "" + @State private var tapTimes: [Date] = [] + + private func clamp(_ v: Double) -> Double { min(Self.maxBPM, max(Self.minBPM, v)) } + + private func step(_ delta: Double) { onSet(clamp((tempo + delta).rounded())) } + + /// Resolve taps to a BPM locally. Keep only taps within 2 s of each other (a gap + /// starts a fresh count), average the intervals, and set once we have two taps. + private func tap() { + let now = Date() + if let last = tapTimes.last, now.timeIntervalSince(last) > 2.0 { tapTimes = [] } + tapTimes.append(now) + if tapTimes.count > 6 { tapTimes.removeFirst(tapTimes.count - 6) } + guard tapTimes.count >= 2 else { return } + var intervals: [TimeInterval] = [] + for i in 1.. 0 else { return } + onSet(clamp((60.0 / avg).rounded())) + } + + private func commitDraft() { + editing = false + if let v = Double(draft.replacingOccurrences(of: ",", with: ".")) { onSet(clamp(v)) } + } + + var body: some View { + VStack(alignment: .leading, spacing: 8) { + HStack(spacing: 8) { + Text("TEMPO") + .font(.ksMono(10)).tracking(1.8) + .foregroundStyle(Color(hex: 0x6F8A4D)) + Spacer() + if linkDriving { + Text("LINK IS DRIVING") + .font(.ksMono(10)).tracking(1.8) + .foregroundStyle(KS.amberText) + } + } + + HStack(spacing: 10) { + stepButton("−") { step(-1) } + + Group { + if editing { + TextField("", text: $draft) + .keyboardType(.decimalPad) + .multilineTextAlignment(.center) + .font(.ksDisplay(30)) + .foregroundStyle(KS.ink) + .onSubmit(commitDraft) + .submitLabel(.done) + } else { + Text(tempo.formatted(.number.precision(.fractionLength(0...1)))) + .font(.ksDisplay(30)) + .foregroundStyle(KS.ink) + .onTapGesture { + draft = tempo.formatted(.number.precision(.fractionLength(0...1))) + editing = true + } + } + } + .frame(maxWidth: .infinity) + .overlay(alignment: .bottom) { + Text("BPM").font(.ksMono(9)).tracking(1.6) + .foregroundStyle(KS.mut).offset(y: 12) + } + + stepButton("+") { step(1) } + + Button(action: tap) { + Text("TAP") + .font(.ksDisplay(15)).tracking(1.2) + .foregroundStyle(KS.onInk) + .frame(width: 66, height: 44) + .background(KS.ledFill, in: RoundedRectangle(cornerRadius: 9)) + } + .buttonStyle(.plain) + .accessibilityLabel("Tap tempo") + } + } + .padding(14) + .background(KS.panel, in: RoundedRectangle(cornerRadius: 12)) + .overlay(RoundedRectangle(cornerRadius: 12).strokeBorder(KS.line)) + } + + private func stepButton(_ label: String, _ action: @escaping () -> Void) -> some View { + Button(action: action) { + Text(label) + .font(.ksDisplay(24)) + .foregroundStyle(KS.ink) + .frame(width: 44, height: 44) + .background( + LinearGradient(colors: [Color(hex: 0x1A1F25), Color(hex: 0x12161B)], + startPoint: .top, endPoint: .bottom), + in: RoundedRectangle(cornerRadius: 9)) + .overlay(RoundedRectangle(cornerRadius: 9).strokeBorder(KS.line)) + } + .buttonStyle(.plain) + .accessibilityLabel(label == "−" ? "Tempo down" : "Tempo up") + } +} diff --git a/Tests/KitchenSyncAppTests/KsConfigTests.swift b/Tests/KitchenSyncAppTests/KsConfigTests.swift index 5688d77..e806344 100644 --- a/Tests/KitchenSyncAppTests/KsConfigTests.swift +++ b/Tests/KitchenSyncAppTests/KsConfigTests.swift @@ -30,6 +30,22 @@ final class KsConfigDecodingTests: XCTestCase { XCTAssertEqual(config.followBeatEnabled, false) } + // ESP-037: the stored free-run tempo. A clock box emits `bpm`; the app shows THIS + // number (distinct from the Link-driven KsStatus.bpm). + func test_decodes_the_stored_tempo_from_bpm() throws { + let json = #"{"clock_out":true,"bpm":128.500,"wifi":[],"clock":[]}"# + let config = try JSONDecoder().decode(KsConfig.self, from: Data(json.utf8)) + XCTAssertEqual(config.tempo ?? 0, 128.5, accuracy: 0.001) + } + + // A listener-only board (X32Link) omits `bpm` entirely — tempo is nil, not 0, so the + // app knows to hide the tempo control rather than show a fake 0 BPM. + func test_a_config_without_bpm_has_nil_tempo() throws { + let json = #"{"clock_out":false,"wifi":[],"clock":[]}"# + let config = try JSONDecoder().decode(KsConfig.self, from: Data(json.utf8)) + XCTAssertNil(config.tempo) + } + func test_decodes_hash_prefixed_hex_colors() throws { let config = try JSONDecoder().decode(KsConfig.self, from: Data(sampleConfigJSON.utf8)) XCTAssertEqual(config.led?.beatColor, 0x00B400) diff --git a/Tests/KitchenSyncAppTests/KsLiveEditTests.swift b/Tests/KitchenSyncAppTests/KsLiveEditTests.swift index 21dd44d..c498fc3 100644 --- a/Tests/KitchenSyncAppTests/KsLiveEditTests.swift +++ b/Tests/KitchenSyncAppTests/KsLiveEditTests.swift @@ -37,4 +37,13 @@ final class KsLiveEditTests: XCTestCase { XCTAssertTrue(KsLiveEdit.outputSwing(index: 0, 125).formField == ("clk0_swing", "125")) XCTAssertTrue(KsLiveEdit.outputFollowsLink(index: 1, false).formField == ("clk1_follow", "0")) } + + // ESP-037: the device-global free-run tempo. `bpm`, decimal, the one number tap / + // numeric / +- all resolve to. `ks_config_set` parses it with atof, so fractional + // BPM (from tap) survives. + func test_set_tempo_posts_the_bpm_field() { + let (key, value) = KsLiveEdit.setTempo(128.5).formField + XCTAssertEqual(key, "bpm") + XCTAssertEqual(Double(value) ?? 0, 128.5, accuracy: 0.001) + } } From ba5fcf0bea85d77a2153fe78debc49a57adb1748 Mon Sep 17 00:00:00 2001 From: Eric Dahl Date: Wed, 15 Jul 2026 00:51:12 -0400 Subject: [PATCH 4/6] fix(app): tempo control follows the effective tempo, no jumps (ESP-037) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Reported: set 150 manually, turn on Link (Ableton 120), and the control kept showing 150 while the box clocked 120. It should track what's actually playing. - The control now shows the EFFECTIVE tempo (status.bpm, polled) via DeviceDetailViewModel.tempoDisplay, not the stored set value. So it follows Link while a session drives, and when Link leaves there's no jump: the box keeps Link's tempo (the firmware arbiter's free-run seed) and the control was already showing it. - While Link drives, the ± / tap / numeric stand down (dimmed, disabled) -- Link owns the tempo; the control just reports it. Editable again when solo. - Edits are OPTIMISTIC: a ± tap shows immediately instead of waiting ~500 ms for the next /status to catch up, then reconciles when the device's reported tempo converges. Pairs with a firmware change (the box records Link's tempo as its own and persists it, so a power-cycle comes back at the last tempo it played). Full app suite green; installed on the phone. Claude-Session: https://claude.ai/code/session_01F7YsQgso3KNpeXbP5vNVj6 --- .../ViewModels/DeviceDetailViewModel.swift | 15 +++++++ .../Views/DeviceDetailView.swift | 4 +- .../KitchenSyncApp/Views/TempoControl.swift | 39 +++++++++++++++---- .../DeviceDetailViewModelTests.swift | 33 ++++++++++++++++ 4 files changed, 81 insertions(+), 10 deletions(-) diff --git a/Sources/KitchenSyncApp/ViewModels/DeviceDetailViewModel.swift b/Sources/KitchenSyncApp/ViewModels/DeviceDetailViewModel.swift index 33681c1..d1d6f37 100644 --- a/Sources/KitchenSyncApp/ViewModels/DeviceDetailViewModel.swift +++ b/Sources/KitchenSyncApp/ViewModels/DeviceDetailViewModel.swift @@ -71,6 +71,21 @@ final class DeviceDetailViewModel: ObservableObject { /// The previous poll's tick health — the other half of a rate. private var lastTick: TickHealth? + /// True when a Link session is currently driving the clock. Then Link owns the tempo + /// (the box follows it), so the set controls stand down. + var tempoIsLinkDriven: Bool { (status?.peers ?? 0) > 0 } + + /// The tempo the control should SHOW. Not the stored set value — the tempo the box is + /// actually CLOCKING (`status.bpm`) when it's running, falling back to the stored set + /// value before the first poll. So it follows Link while a session drives, and when + /// Link leaves there's no jump: the box keeps Link's tempo, and the control was + /// already showing it. `nil` on a listener-only device (no settable tempo at all). + var tempoDisplay: Double? { + guard let stored = config?.tempo else { return nil } + if let b = status?.bpm, b > 0 { return b } + return stored + } + func refreshStatus() async { do { let fresh = try await client.fetchStatus() diff --git a/Sources/KitchenSyncApp/Views/DeviceDetailView.swift b/Sources/KitchenSyncApp/Views/DeviceDetailView.swift index 0bf9409..3045f2c 100644 --- a/Sources/KitchenSyncApp/Views/DeviceDetailView.swift +++ b/Sources/KitchenSyncApp/Views/DeviceDetailView.swift @@ -37,10 +37,10 @@ struct DeviceDetailView: View { // ESP-037: the set-tempo control, only when the device has a settable // tempo (config.tempo != nil — a listener-only box omits it). - if let t = vm.config?.tempo { + if let t = vm.tempoDisplay { TempoControl( tempo: t, - linkDriving: (vm.status?.peers ?? 0) > 0, + linkDriving: vm.tempoIsLinkDriven, onSet: { bpm in Task { await vm.apply(.setTempo(bpm)) } } ) } diff --git a/Sources/KitchenSyncApp/Views/TempoControl.swift b/Sources/KitchenSyncApp/Views/TempoControl.swift index 767ea20..1a68700 100644 --- a/Sources/KitchenSyncApp/Views/TempoControl.swift +++ b/Sources/KitchenSyncApp/Views/TempoControl.swift @@ -17,9 +17,11 @@ import SwiftUI /// Link IS driving, the control says so, because a tempo you set that the device is /// currently ignoring is exactly the kind of silent lie this app keeps stamping out. struct TempoControl: View { + /// The tempo to SHOW — the effective one (what the box is clocking), so it follows + /// Link and never jumps. See `DeviceDetailViewModel.tempoDisplay`. let tempo: Double - /// True when a Link session is currently driving the clock (peers > 0). The set - /// tempo still persists and applies the moment Link leaves — but say who's in charge. + /// True when a Link session is driving. Link owns the tempo; the set controls stand + /// down and just report Link's value. let linkDriving: Bool let onSet: (Double) -> Void @@ -29,10 +31,23 @@ struct TempoControl: View { @State private var editing = false @State private var draft = "" @State private var tapTimes: [Date] = [] + /// Show an edit immediately instead of waiting a poll for `status.bpm` to catch up — + /// otherwise a ± tap looks dead for half a second, then snaps. Cleared once the + /// device's reported tempo converges on it. + @State private var optimistic: Double? + + private var shown: Double { optimistic ?? tempo } + private var editable: Bool { !linkDriving } private func clamp(_ v: Double) -> Double { min(Self.maxBPM, max(Self.minBPM, v)) } - private func step(_ delta: Double) { onSet(clamp((tempo + delta).rounded())) } + private func commit(_ v: Double) { + let c = clamp(v) + optimistic = c + onSet(c) + } + + private func step(_ delta: Double) { commit((shown + delta).rounded()) } /// Resolve taps to a BPM locally. Keep only taps within 2 s of each other (a gap /// starts a fresh count), average the intervals, and set once we have two taps. @@ -46,12 +61,12 @@ struct TempoControl: View { for i in 1.. 0 else { return } - onSet(clamp((60.0 / avg).rounded())) + commit((60.0 / avg).rounded()) } private func commitDraft() { editing = false - if let v = Double(draft.replacingOccurrences(of: ",", with: ".")) { onSet(clamp(v)) } + if let v = Double(draft.replacingOccurrences(of: ",", with: ".")) { commit(v) } } var body: some View { @@ -81,11 +96,12 @@ struct TempoControl: View { .onSubmit(commitDraft) .submitLabel(.done) } else { - Text(tempo.formatted(.number.precision(.fractionLength(0...1)))) + Text(shown.formatted(.number.precision(.fractionLength(0...1)))) .font(.ksDisplay(30)) - .foregroundStyle(KS.ink) + .foregroundStyle(editable ? KS.ink : KS.mut) .onTapGesture { - draft = tempo.formatted(.number.precision(.fractionLength(0...1))) + guard editable else { return } + draft = shown.formatted(.number.precision(.fractionLength(0...1))) editing = true } } @@ -108,10 +124,17 @@ struct TempoControl: View { .buttonStyle(.plain) .accessibilityLabel("Tap tempo") } + .disabled(!editable) // Link owns the tempo while it's driving + .opacity(editable ? 1 : 0.5) } .padding(14) .background(KS.panel, in: RoundedRectangle(cornerRadius: 12)) .overlay(RoundedRectangle(cornerRadius: 12).strokeBorder(KS.line)) + // The device caught up (its reported tempo reached our optimistic value, within + // rounding): drop the optimistic override and follow the device again. + .onChange(of: tempo) { _, new in + if let o = optimistic, abs(new - o) < 0.75 { optimistic = nil } + } } private func stepButton(_ label: String, _ action: @escaping () -> Void) -> some View { diff --git a/Tests/KitchenSyncAppTests/DeviceDetailViewModelTests.swift b/Tests/KitchenSyncAppTests/DeviceDetailViewModelTests.swift index 217e186..12b33f0 100644 --- a/Tests/KitchenSyncAppTests/DeviceDetailViewModelTests.swift +++ b/Tests/KitchenSyncAppTests/DeviceDetailViewModelTests.swift @@ -28,6 +28,39 @@ final class DeviceDetailViewModelTests: XCTestCase { "launch":[0,1,2,0],"playing":true,"link_owns":false} """.data(using: .utf8)! + // ESP-037: the control shows the EFFECTIVE tempo, not the stored set value -- so it + // follows Link while a session drives, and there's no jump when Link leaves. + private static let configTempo150 = """ + {"clock_out":true,"bpm":150.000,"wifi":[],"clock":[]} + """.data(using: .utf8)! + private static let statusLinkAt120 = """ + {"bpm":120.0,"min":0.0,"peers":1,"usb":false,"tx":0,"fw":"2.2.0", + "follow_enabled":false,"follow_bpm":0.0,"follow_confidence":0.0,"follow_valid":false, + "launch":[0],"playing":false,"link_owns":true} + """.data(using: .utf8)! + + func test_tempo_display_follows_the_effective_bpm_while_link_drives() async { + StubURLProtocol.routes["GET /config.json"] = .init(body: Self.configTempo150) + StubURLProtocol.routes["GET /status"] = .init(body: Self.statusLinkAt120) + let vm = makeVM() + + await vm.loadConfig() + await vm.refreshStatus() + + XCTAssertEqual(vm.tempoDisplay ?? 0, 120, accuracy: 0.1) // Link's, not the stored 150 + XCTAssertTrue(vm.tempoIsLinkDriven) + } + + func test_tempo_display_uses_the_stored_value_before_the_first_poll() async { + StubURLProtocol.routes["GET /config.json"] = .init(body: Self.configTempo150) + let vm = makeVM() + + await vm.loadConfig() // no status yet + + XCTAssertEqual(vm.tempoDisplay ?? 0, 150, accuracy: 0.1) + XCTAssertFalse(vm.tempoIsLinkDriven) + } + func test_refresh_publishes_the_devices_status() async { StubURLProtocol.routes["GET /status"] = .init(body: Self.statusJSON) let vm = makeVM() From 1057341832c79d0f8d26b446099fe93a71b8edbe Mon Sep 17 00:00:00 2001 From: Eric Dahl Date: Wed, 15 Jul 2026 00:59:02 -0400 Subject: [PATCH 5/6] =?UTF-8?q?fix(app):=20the=20typed=20tempo=20had=20no?= =?UTF-8?q?=20way=20to=20commit=20=E2=80=94=20decimalPad=20has=20no=20retu?= =?UTF-8?q?rn=20key?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The numeric entry was a dead end: .onSubmit/.submitLabel(.done) do nothing on a decimalPad keyboard, which has no return key at all. You could type a BPM and never set it. Focus drives it now: a "Set" button in the keyboard toolbar, and commit-on-dismiss when you tap away from the field. commitDraft guards against the double (Set button also drops focus). ± and TAP were never affected. Claude-Session: https://claude.ai/code/session_01F7YsQgso3KNpeXbP5vNVj6 --- .../KitchenSyncApp/Views/TempoControl.swift | 33 +++++++++++++++---- 1 file changed, 26 insertions(+), 7 deletions(-) diff --git a/Sources/KitchenSyncApp/Views/TempoControl.swift b/Sources/KitchenSyncApp/Views/TempoControl.swift index 1a68700..1c10c4e 100644 --- a/Sources/KitchenSyncApp/Views/TempoControl.swift +++ b/Sources/KitchenSyncApp/Views/TempoControl.swift @@ -31,6 +31,10 @@ struct TempoControl: View { @State private var editing = false @State private var draft = "" @State private var tapTimes: [Date] = [] + /// The decimalPad keyboard has NO return key, so `.onSubmit` never fires — a typed + /// tempo could never be committed. Focus drives it instead: a "Set" button in the + /// keyboard toolbar, and commit-on-dismiss when you tap away. + @FocusState private var fieldFocused: Bool /// Show an edit immediately instead of waiting a poll for `status.bpm` to catch up — /// otherwise a ± tap looks dead for half a second, then snaps. Cleared once the /// device's reported tempo converges on it. @@ -65,10 +69,19 @@ struct TempoControl: View { } private func commitDraft() { + guard editing else { return } // avoid a double-commit (Set button + focus-loss) editing = false + fieldFocused = false if let v = Double(draft.replacingOccurrences(of: ",", with: ".")) { commit(v) } } + private func beginEdit() { + guard editable else { return } + draft = shown.formatted(.number.precision(.fractionLength(0...1))) + editing = true + fieldFocused = true + } + var body: some View { VStack(alignment: .leading, spacing: 8) { HStack(spacing: 8) { @@ -93,17 +106,15 @@ struct TempoControl: View { .multilineTextAlignment(.center) .font(.ksDisplay(30)) .foregroundStyle(KS.ink) - .onSubmit(commitDraft) - .submitLabel(.done) + .focused($fieldFocused) + .onChange(of: fieldFocused) { _, focused in + if !focused { commitDraft() } // tapped away → set it + } } else { Text(shown.formatted(.number.precision(.fractionLength(0...1)))) .font(.ksDisplay(30)) .foregroundStyle(editable ? KS.ink : KS.mut) - .onTapGesture { - guard editable else { return } - draft = shown.formatted(.number.precision(.fractionLength(0...1))) - editing = true - } + .onTapGesture(perform: beginEdit) } } .frame(maxWidth: .infinity) @@ -130,6 +141,14 @@ struct TempoControl: View { .padding(14) .background(KS.panel, in: RoundedRectangle(cornerRadius: 12)) .overlay(RoundedRectangle(cornerRadius: 12).strokeBorder(KS.line)) + // The decimalPad has no return key — give the number a way to commit. + .toolbar { + ToolbarItemGroup(placement: .keyboard) { + Spacer() + Button("Set", action: commitDraft) + .disabled(!editing) + } + } // The device caught up (its reported tempo reached our optimistic value, within // rounding): drop the optimistic override and follow the device again. .onChange(of: tempo) { _, new in From 8c6170eea3bff5ee46b8ea76008f513e76edf938 Mon Sep 17 00:00:00 2001 From: Eric Dahl Date: Wed, 15 Jul 2026 00:59:10 -0400 Subject: [PATCH 6/6] docs(plan): mixer tempo bridge app design (app-as-firmware + hardware funnel) --- .../2026-07-15-mixer-tempo-bridge-design.md | 188 ++++++++++++++++++ 1 file changed, 188 insertions(+) create mode 100644 docs/plans/2026-07-15-mixer-tempo-bridge-design.md diff --git a/docs/plans/2026-07-15-mixer-tempo-bridge-design.md b/docs/plans/2026-07-15-mixer-tempo-bridge-design.md new file mode 100644 index 0000000..60a6edd --- /dev/null +++ b/docs/plans/2026-07-15-mixer-tempo-bridge-design.md @@ -0,0 +1,188 @@ +# Mixer Tempo Bridge — design + +**Date:** 2026-07-15 +**Status:** design, not yet scheduled +**Working title:** "mixer tempo bridge" (deliberately neutral — see [Naming](#naming)) + +> A standalone iOS app that turns the phone itself into the tempo bridge the X32Link +> *hardware* does: take a tempo (Ableton Link, MIDI clock in, or an in-app tap), and drive a +> mixing console's tempo over its own protocol (OSC for the Behringer X32/M32 to start). Sell it +> cheap, and let the app's own limits sell the hardware. + +## How this idea arrived + +It started as "X32Link could be its own app" — splitting the headless X32Link device out of the +unified KitchenSync companion app. That much was already well-motivated: X32Link serves a different +user (a front-of-house / console engineer, not a KitchenSync clock musician), speaks its **own** +firmware web surface (`X32Link/web_config.cpp`, `X32Link/ks_status.c` in `link-devices` — *not* +`KitchenSync/main/ks_web.cpp`), and its lineage is separate — it began life as a **MIDI-in → OSC** +box that syncs an X32's tempo, growing Link and MIDI-out only later. It *converged* on the +KitchenSync web protocol; it wasn't born from it. + +Then the idea grew a second, larger half: **the app can replace the firmware.** iOS runs Ableton +Link natively (Link is a network protocol — no hardware required), computes the beat, and sends OSC +straight to the console over WiFi. That is exactly what the box does, minus the box. + +So the product is two things at once: + +1. **A $1 app** that bridges tempo → console for anyone with a phone on the same network. +2. **A storefront for the hardware**, upsold at the exact moments the app can't win. + +## Why the funnel is honest + +The phone *cannot* be the always-on bridge: iOS suspends apps, the phone leaves the venue in +someone's pocket, and it has no DIN/USB MIDI out. So the $1 app is the "soundcheck / small gig / try +it" tier, and the box is the "patch once, forget, survives the show, real MIDI I/O" tier. **The +limitation of the app literally is the sales pitch for the hardware.** That makes the upsell truthful +rather than nagging. + +## Architecture + +One deep module in the middle; everything else is an adapter at a seam. + +``` + TempoSource ──▶ BridgeEngine ──▶ MixerTempoSink ──▶ console + Link (the core) ├─ X32/M32 (OSC/UDP :10023) ◀ build now + manual tap ├─ Allen & Heath ◀ later + MIDI-in └─ Yamaha / … ◀ later +``` + +This is the same `source → master clock → output` spine the firmware runs +(`X32Link/master_clock.h`, `beat_source.h`), which is why it ports cleanly — and keeps *the firmware +is the source of truth* alive even in a rewrite. + +### `BridgeEngine` — the deep module + +Owns the current `{ bpm, isPlaying, beatPhase }`, decides when the console needs an update, and +**never predicts transport client-side** — the source is the truth, the engine mirrors it (the same +rule the KitchenSync app lives by: never render a lock that isn't real). It mentions neither Link nor +OSC. Pure and testable: feed it a source event, assert the sink intent it produces. + +### `TempoSource` — the input seam + +Interface emitting `{ bpm, beatPhase, isPlaying }`. Adapters: + +- `LinkTempoSource` — LinkKit; the phone joins the network Link session. *Gated on Ableton + licensing.* +- `ManualTempoSource` — in-app tap / BPM entry; works standalone with nothing else on the network. +- `MidiClockTempoSource` — Core MIDI (USB camera-kit or network/RTP MIDI); mirrors the device's + original MIDI-in heritage. **Additive** — same interface, so it can land after v1. + +### `MixerTempoSink` — the output seam + +Interface: *"a console you can hand a tempo to."* Given engine state, deliver it however **this** +console wants — tap-tempo messages, a tempo parameter, whatever — and own **finding and connecting +to** that console (an X32 answers its own OSC `/info` handshake; another mixer won't). Protocol- +agnostic on purpose: not every console speaks OSC. + +- `X32MixerSink` — OSC/UDP :10023, the one concrete adapter for v1 (covers X32 and M32). +- Other consoles drop in against the same interface later, without the engine knowing. + +**Deletion test, both seams:** delete `MixerTempoSink` and the X32's OSC quirks leak into the +engine; delete `TempoSource` and three tempo inputs each grow their own copy of the engine. Both earn +their keep the moment the second adapter exists — which the roadmap guarantees. + +## Reliability, background execution, and failure + +This is where a live-audio tool is won or lost. + +- **Background — best-effort, and say so.** Stay alive when locked via the standard Link pattern: + `UIBackgroundModes: audio` + a live `AVAudioSession` keep-alive; disable auto-lock + (`isIdleTimerDisabled`) on the bridge screen. v1 can be **foreground-first**, background a bonus. + The ceiling isn't hidden — it's surfaced at the moment it bites, as the hardware pitch. +- **Timing is coarse, which is why this works over WiFi.** We *set a tempo* (a BPM value / tap), we + do **not** stream a sample-accurate clock. OSC-over-WiFi jitter is tolerable for a BPM set. The + engine sends on meaningful change (debounced) plus a slow keepalive — not a packet per beat. +- **Failure gets distinct signals** — the recurring lesson of this codebase (T-009/010/016/018/021): + three problems, three fixes, never collapsed. + - **No tempo in** — Link peers gone / MIDI-in silent / manual unset. + - **Console unreachable** — network or mixer gone. + - **Console reachable but not accepting** — connected, wrong dialect / rejected. + A pure, testable `BridgeStatus` classifier owns that three-way decision (same shape as + `TransportAppearance` / the `DeviceFault` idea in T-021). The "lock is holding" indicator is the + primary UI and shows **truth, never optimism.** + +## Product surface (v1) + +- **Bridge only.** Do *not* also build "configure my hardware box" for launch — the box already has + its own web UI (`web_config.cpp`), so a box owner isn't stranded. Box-config is a later feature. +- **Upsell is limitation-triggered**, not a banner: phone-about-to-sleep, wants-MIDI-out, + leave-it-at-the-venue — each nudge is the app honestly naming what it can't do, plus one store + link. +- **Apple constraint:** physical hardware must **not** use in-app purchase; physical goods sell via + an external link out. So "$1" is the app (paid up front, or a $1 unlock), and the hardware is a + link, never an IAP. That's the compliant path anyway. + +The app surface stays small: a bridge screen (source picker, mixer connection, a big "lock is +holding" indicator) and a "get the hardware" route. + +## The shared core with KitchenSync — deferred on purpose + +The bridge pivot **dissolves** the original "shared package, two apps" premise. When X32Link was a +*companion* app it shared KitchenSync's whole substrate (`KitchenSyncClient`, `KsConfig`/`KsStatus` +decode, discovery, the form machinery). A *bridge* app shares almost none of that — its spine +(`TempoSource`, `BridgeEngine`, `MixerTempoSink`) is brand new and, in v1, never talks to a +KitchenSync device's web API at all. The overlap is a Bonjour helper and some UI tokens: a +coincidence, not a seam. + +So **v1 is a clean standalone app; no shared package.** Forcing a share now is exactly the +premature-abstraction the architecture review warns against — one weak overlap is a *hypothetical* +seam. + +The package (`LinkDeviceKit`) earns its place **later**, at one trigger: when the bridge app grows +the deferred box-config feature, which reuses KitchenSync's companion substrate (discovery + +`/status`/`/config.json` decode against the box's `web_config.cpp`). Two real consumers — then +extract. Not before. + +## Testing + +The seams exist so the whole bridge is testable against fakes — no hardware, no phone-on-network, no +console, no Link session. Same discipline as the KitchenSync app's `StubURLProtocol` (test the *real* +thing against a stub, never a mock of ourselves). + +- **`BridgeEngine` in isolation** — scripted `TempoSource` events in, assert `MixerTempoSink` intents + out: change-detection, debounce, keepalive, transport-truth. Pure, no I/O; gets the most tests. +- **In-memory adapters at both seams** — a fake source (drive sequences) and a recording sink + (capture what would be sent) exercise the bridge end-to-end with zero real dependencies. +- **`BridgeStatus` classifier** — pure function, tested directly: (source-state × sink-state) → + three-way signal. +- **`X32MixerSink` against golden OSC fixtures** — the one place encoding a real external protocol, + so build fixtures from *actual* X32 OSC bytes (real wire output, not plausible-looking messages). +- **Named known gap** (house style): LinkKit joining a real session, iOS background survival, WiFi + jitter, a real console accepting the OSC — none unit-testable. Needs a bench (real X32 + a Link + source). Until then, verified against a *reading* of the X32 OSC spec, not against a console. + +TDD carries over: tests first. + +## Open questions + +These are gates and unknowns to resolve before (or early in) implementation — parked here so they +don't get lost: + +1. **Naming.** *Both* halves of "X32Link" are trademarks: "X32" is Behringer's, "Link" is Ableton's + (the Link license restricts putting "Link" in an app name). The app needs a neutral, likely + made-up brand. This is a real gate on shipping, not cosmetic. → run a dedicated naming pass. +2. **Ableton Link licensing.** LinkKit is free but requires registering the app with Ableton and + accepting the Link license. This gates the `LinkTempoSource` adapter and the whole "sync to a + session" story. Confirm terms (and any branding/attribution requirements) before building on it. +3. **X32 OSC dialect — exactly which messages.** Tap-tempo vs. setting an FX tempo parameter vs. + both; how the console is discovered (`/info` handshake); ports and rate limits. Needs the X32 OSC + spec pinned down and a bench unit to confirm. +4. **Background-execution reality.** How reliable is `audio`-mode keep-alive across current iOS + versions and lock/background/network-change transitions? Decide whether v1 claims background at + all, or ships foreground-first. +5. **Pricing mechanics.** $1 paid-up-front vs. free + $1 unlock IAP. And the hardware store-link flow + (must stay outside IAP for physical goods). +6. **Box-config timing.** When (if) the bridge app grows "configure my hardware box," that's the + trigger to extract the shared `LinkDeviceKit` package with KitchenSync. Not a v1 concern; flagged + so the future extraction has a defined cause. +7. **MIDI-in scope.** `MidiClockTempoSource` is additive but non-trivial on iOS (Core MIDI, camera- + kit vs. network MIDI). Confirm it's post-v1 unless a specific user needs it at launch. + +## Relationship to existing work + +Standalone app; **not** part of the current KitchenSync iOS codebase in v1. It mirrors the firmware's +`source → master clock → output` architecture (`X32Link/` in `link-devices`), and reuses this +project's house disciplines wholesale: distinct-signals error classification, decisions-as-pure- +functions, truth-never-optimism UI, firmware-derived test fixtures, and naming a known gap rather +than pretending it's covered.