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/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/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 22b0562..3045f2c 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.tempoDisplay { + TempoControl( + tempo: t, + linkDriving: vm.tempoIsLinkDriven, + 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..1c10c4e --- /dev/null +++ b/Sources/KitchenSyncApp/Views/TempoControl.swift @@ -0,0 +1,174 @@ +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 { + /// 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 driving. Link owns the tempo; the set controls stand + /// down and just report Link's value. + 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] = [] + /// 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. + @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 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. + 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 } + commit((60.0 / avg).rounded()) + } + + 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) { + 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) + .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(perform: beginEdit) + } + } + .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") + } + .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 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 + if let o = optimistic, abs(new - o) < 0.75 { optimistic = nil } + } + } + + 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/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() 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)) + } } 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) + } } 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. 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