diff --git a/CLAUDE.md b/CLAUDE.md index f9780af..6c0a718 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -44,7 +44,7 @@ Three SwiftPM targets (`Package.swift`): ## Things that are easy to get wrong - **Firmware lies**: it answers `{"ok":1}` to any payload, malformed or not. The LEDs are the only ground truth. `PadEmulator` reproduces this on purpose (and the stock keymap, and silent unbound keys) — keep it faithful when changing it. -- **Only one HID client at a time**: Work Louder's Input app and the Codex desktop app fight over the same lighting. `contendingClient` has two signals, combined only in `updateContention()`: a reply with a response id we never issued sets `trafficSeen` (`noteResponse(id:)` and `noteIssued(id:)`, also the test seams), and an IORegistry scan (`WLDevice.otherClients()`, behind the `scanClients` seam) lists the other processes holding the pad. An `.authoritative` scan (USB vendor interface) with others raises the flag by itself, unless Recheck acknowledged them (`acknowledgedPIDs`: the pids a Recheck's scan listed, so an app that opens every HID device and never sends cannot pin the warning; pruned by every scan that no longer lists them, so a relaunch raises again; kept by `.unavailable`; reset by `start()`/`stop()`; their replies still raise through `trafficSeen`); an `.advisory` one (Bluetooth) only names them; an empty list from either clears `trafficSeen`; `.unavailable` (emulator, failed walk) leaves the replies to decide. Scans run on panel open (`scanContention()`), on Recheck (`recheckContention()`: clear traffic, scan, acknowledge, repaint if running), after `openDevice()` connects, and after a foreign reply (at most once per `replyScanInterval`); never while off. `contenders` names them in the warning. A recheck must never clear `issuedIDs`: a reply to our own call still in flight would look foreign and re-raise the warning at once. The Inspector is a separate process and counts as another client. +- **Only one HID client at a time**: Work Louder's Input app and the Codex desktop app fight over the same lighting. Only a reply with a response id we never issued raises `contendingClient` (`noteResponse(id:result:error:)` and `noteIssued(id:)`, also the test seams), and not a null one: Input sends `host.focused_app` on every app switch, which the firmware answers with a bare null (lighting calls answer `{"ok":1}`); the filter runs after the own-id check, so our own ids are still consumed. **Holding the pad never raises it**: Input keeps the vendor interface open in the background without sending, and a presence rule warned on every launch. An IORegistry scan (`WLDevice.otherClients()`, behind the `scanClients` seam; `.authoritative` and `.advisory` are treated alike) says who may have sent and when they have gone: `traffic` is `.from(pids)` — laid at the newcomers, i.e. others not in `quietPIDs` (seen holding the pad while no reply was outstanding), else at every other — and clears once a scan lists none of them; `.unattributed` (no scan could say) clears only on an empty list. Newcomers blamed go into `suspectedPIDs` and are never counted quiet while present, so a Recheck with the Inspector still open does not spread its next reply to Input. `reconcile()` applies a scan; `.unavailable` (emulator, failed walk) changes nothing; `start()`/`stop()` forget it all. Scans run on panel open (`scanContention()`), on Recheck (`recheckContention()`: clear traffic, scan, repaint if running), after `openDevice()` connects, and after a foreign reply (at most once per `replyScanInterval`); never while off. `contenders` names the suspects still present, else every other. A recheck must never clear `issuedIDs`: a reply to our own call still in flight would look foreign and re-raise the warning at once. The Inspector is a separate process and counts as another client. - **Signing matters**: Input Monitoring is granted per code signature, so ad-hoc builds need re-granting every rebuild. `bundle.sh` prefers a real Apple Development / Developer ID identity (`WL_SIGN_IDENTITY` overrides). `swift run` works only because it inherits the terminal's grant. - **A remote Herdr is an owned ssh**: `SSHTunnel` runs its own `/usr/bin/ssh` (`ControlMaster=no`, so the process lifetime *is* the tunnel's) with a remote `cat >/dev/null` on a stdin pipe we hold, so even a crash ends it by EOF. `HERDR_SOCKET_PATH` beats any tunnel, so the panel disables the target picker when it is set — and Herdr itself sets it for processes it spawns, so a `swift run` from inside a Herdr pane gets the disabled picker (and the `LiveBridgeTargetTests` skip). - **The socket path is global, so a switch must not leak.** `setSocketPath` bumps a generation; a reply that lands after a switch becomes `HerdrError.targetChanged`, and the bridge guards every async callback (tunnel state, lifecycle restart, status-stream `onClosed`, poll, in-flight `agent.list`) with its own generation counter plus a `herdrActive` flag. Event streams resolve the path in `start()`, not `init`. Pane ids are only unique per server — never carry agent state across targets. diff --git a/README.md b/README.md index dae9679..ae47b70 100644 --- a/README.md +++ b/README.md @@ -290,25 +290,25 @@ never issued is a reliable tell. The **Inspector** counts too: it is a separate process with its own connection to the pad. The manager also asks macOS which other processes hold the pad open, each time -the panel opens. Over USB that answer is exact: the warning names the other app -("Also driving this pad: Input — colours may fight."), and it clears by itself -the next time you open the panel after that app has quit. **Recheck** next to -the warning checks again on the spot and repaints the pad with the manager's -colours. Switching the manager off clears the warning too. - -Some apps (input remappers, macro or streaming tools) open every HID device -without ever touching the lighting, and would keep the warning up for as long -as they run. **Recheck** accepts the apps holding the pad at that moment: they -no longer raise the warning on their own, though they are still named if it -comes up for another reason. It comes back when one of them sends to the pad, -when a different app opens it, or when an accepted app quits and is started -again. Switching the manager off and on forgets what was accepted. +the panel opens and whenever such a reply comes in. The warning names the app +that most likely sent it ("Also driving this pad: Micro Manager Inspector — +colours may fight."), and it clears by itself the next time you open the panel after that +app has quit. **Recheck** next to the warning clears it on the spot and +repaints the pad with the manager's colours. Switching the manager off clears +the warning too. + +Holding the pad open is not driving it, so that alone never raises the +warning. Work Louder's Input app keeps the pad open while it runs in the +background, and tells the pad which app is in front on every app switch; the +manager ignores those messages, since they leave the lighting alone. Some apps (input remappers, macro or streaming tools) open +every HID device without ever touching the lighting. The manager remembers +the apps that were holding the pad quietly, and blames a reply on an app that +arrived since. If one of the quiet apps starts sending, the warning names all +of them and stays until they have quit or you press **Recheck**. Over Bluetooth the pad is a single device that also carries the keyboard, so -apps that only listen for keys (chat apps, for one) may hold it open too. There the -check can name the apps but not raise the warning by itself: it still takes a -reply we never asked for, and it stays until the other app has quit and you -press **Recheck**, or the check finds nobody else at all. See +apps that only listen for keys (chat apps, for one) hold it open too. They +are quiet holders like any other. See [design/pad-contention.md](design/pad-contention.md) for the details. ## Development diff --git a/Sources/WLKit/BridgeController.swift b/Sources/WLKit/BridgeController.swift index 261d89c..a4272ab 100644 --- a/Sources/WLKit/BridgeController.swift +++ b/Sources/WLKit/BridgeController.swift @@ -33,13 +33,17 @@ public final class BridgeController: ObservableObject { @Published public private(set) var keyEffects: [Int: OAI.Effect] = [:] @Published public private(set) var aggregateState: String? @Published public private(set) var lastError: String? - /// Another process is driving the same pad. Two signals feed it: a reply - /// id we never issued (a shared HID open means we receive other clients' - /// replies too), and a scan of the IORegistry for other processes holding - /// the pad open. `updateContention()` has the rule that combines them. + /// Another process is driving the same pad: a reply id we never issued + /// has been seen (a shared HID open means we receive other clients' + /// replies too). A scan of the IORegistry for the other processes holding + /// the pad says who may have sent it, and clears the warning once they + /// have all gone. Holding the pad without sending never raises it: an + /// app that opens every HID device, or Work Louder's Input idling in the + /// background, cannot make the colours fight. @Published public private(set) var contendingClient = false /// The other processes the last registry scan found, to name them in the - /// warning. Empty when the scan was unavailable. + /// warning: those that may have sent the replies, else all of them. + /// Empty when the scan was unavailable. @Published public private(set) var contenders: [HIDClient] = [] /// Whether the GitButler stack is on screen. Only the key light cares — /// the panel itself lives in the app layer. @@ -106,17 +110,31 @@ public final class BridgeController: ObservableObject { private var reopenTask: Task? private var lastFingerprint: String? private var issuedIDs = Set() - /// A reply id we never issued has been seen since the last clear. - private var trafficSeen = false + /// Foreign replies seen since the last clear, and who may have sent them. + private var traffic: Traffic = .none + private enum Traffic: Equatable { + case none + /// From one of these other clients, by the scans around the replies. + /// The warning clears once a scan lists none of them. + case from(Set) + /// Seen while no scan could say who was there; only a scan that + /// finds nobody else clears it. + case unattributed + } /// The last registry scan's verdict; `.unavailable` until one has run /// on an open device. private var lastScan: HIDClientScan = .unavailable - /// Other clients the user let stay with Recheck. They still hold the - /// pad, but no longer raise the warning on their own: an app that opens - /// every HID device and never sends would otherwise keep it up for good. - /// Their replies still do, through `trafficSeen`. A pid drops out once a - /// scan no longer lists it, so a relaunched app raises the warning again. - private var acknowledgedPIDs = Set() + /// Other clients seen holding the pad while no foreign reply was + /// outstanding, and never suspected of sending one: they hold it without + /// talking (an app that opens every HID device, Input in the background). + /// A foreign reply is laid at a newcomer's door before theirs, so the + /// warning still clears once the newcomer has gone. A pid drops out once + /// a scan no longer lists it. + private var quietPIDs = Set() + /// Newcomers a foreign reply was laid at, kept while they hold the pad. + /// A scan never counts them as quiet: a Recheck while the Inspector is + /// still open would otherwise blame its next reply on everyone there. + private var suspectedPIDs = Set() /// When the last reply-triggered scan ran, on the `uptime` clock. private var lastReplyScan: TimeInterval? /// Who else holds the pad open. A seam for tests; reads the device at @@ -192,8 +210,8 @@ public final class BridgeController: ObservableObject { device.onTX = { [weak self] _, _, id in self?.noteIssued(id: id) } - device.onResponse = { [weak self] id, _, _ in - self?.noteResponse(id: id) + device.onResponse = { [weak self] id, result, error in + self?.noteResponse(id: id, result: result, error: error) } device.onNotification = { [weak self] method, params in guard let self, method == OAI.notifyHID else { return } @@ -210,98 +228,132 @@ public final class BridgeController: ObservableObject { /// Every reply the pad sends, ours or not. Internal so tests can land a /// reply id the bridge never issued, as another client's would. - func noteResponse(id: Int) { + func noteResponse(id: Int, result: Any?, error: String?) { // A reply to an id we never sent came from another client. guard issuedIDs.remove(id) == nil else { return } - trafficSeen = true - // Scan to put a name on it, at most once a second. A reply within - // the window still raises the warning; it just keeps the last names. + // Not every call is about the lighting. Input announces the frontmost + // app (`host.focused_app`) on every app switch, and the firmware + // answers that with a bare null; lighting calls are answered + // {"ok":1}. A null reply cannot be a fight over the colours. + guard error != nil || !(result == nil || result is NSNull) else { return } + // Scan to see who may have sent it, at most once a second. A reply + // within the window still raises the warning; it is laid at the + // clients of the last scan. let now = uptime() if lastReplyScan.map({ now - $0 >= Self.replyScanInterval }) ?? true { lastReplyScan = now - applyScan() - } else { - updateContention() + scan() + } + switch (traffic, likelySenders()) { + case (.none, let senders?): traffic = .from(senders) + case (.from(let earlier), let senders?): traffic = .from(earlier.union(senders)) + default: traffic = .unattributed } + reconcile() } /// Re-reads who else holds the pad open, without repainting. The panel /// calls this every time it opens, so the warning clears by itself once /// the other app has quit. public func scanContention() { - applyScan() + scan() + reconcile() } - /// The panel's Recheck: forgets the replies seen so far, rescans, - /// acknowledges the other clients the scan still lists, and repaints so - /// our colours replace whatever the other app left on the pad. This - /// proves nothing by itself — the warning comes back as soon as the - /// other app, if it is still there, sends to the pad again, or a client - /// that was not acknowledged opens it. + /// The panel's Recheck: forgets the replies seen so far, rescans, and + /// repaints so our colours replace whatever the other app left on the + /// pad. This proves nothing by itself — the warning comes back as soon + /// as the other app, if it is still there, sends to the pad again. public func recheckContention() async { // `issuedIDs` is left alone on purpose: a reply to one of our own // calls still in flight would otherwise look foreign and raise the // warning again at once. - trafficSeen = false - applyScan() - if case .authoritative(let others) = lastScan { - acknowledgedPIDs = Set(others.map(\.pid)) - updateContention() - } + traffic = .none + scanContention() guard isRunning else { return } await forceRepaint() } /// Scans the registry, if there is an open pad to scan. An off bridge /// drives nothing, so its scan is `.unavailable` without asking. - private func applyScan() { + private func scan() { lastScan = isRunning && deviceConnected ? scanClients() : .unavailable + } + + /// The other clients in the last scan; nil when it was unavailable. Over + /// USB (`.authoritative`) they hold the vendor interface; over Bluetooth + /// (`.advisory`) they may only listen for keys. Either way only replies + /// raise the warning, so the bridge treats the two alike. + private var lastOthers: [HIDClient]? { switch lastScan { - case .authoritative(let others), .advisory(let others): - // Nobody else holds the pad now, so whoever sent those replies - // has gone. Only a fresh scan clears: replies seen after it - // raise the warning again until the next one. - if others.isEmpty { trafficSeen = false } - // An acknowledged app that has quit is forgotten, so the same app - // relaunched (a new pid) raises the warning again. - acknowledgedPIDs.formIntersection(others.map(\.pid)) - case .unavailable: - break + case .unavailable: return nil + case .authoritative(let others), .advisory(let others): return others + } + } + + /// Who may have sent a foreign reply, by the last scan: the other + /// clients not known to hold the pad quietly, or all of them when every + /// one is. Nil when the scan cannot say. + private func likelySenders() -> Set? { + guard let others = lastOthers else { return nil } + let present = Set(others.map(\.pid)) + let newcomers = present.subtracting(quietPIDs) + // Someone quiet has started sending, and any of them may have. They + // stay quiet, so a later newcomer's reply is still laid at it alone. + guard !newcomers.isEmpty else { return present } + suspectedPIDs.formUnion(newcomers) + return newcomers + } + + /// Brings the replies seen and the quiet clients up to date with the + /// last scan, then decides the warning. An unavailable scan says nothing + /// about who left, so it changes neither. + private func reconcile() { + if let others = lastOthers { + let present = Set(others.map(\.pid)) + switch traffic { + case .none: + break + case .from(let senders): + // Only a fresh scan clears: replies seen after it raise the + // warning again until the next one. + let stillThere = senders.intersection(present) + traffic = stillThere.isEmpty ? .none : .from(stillThere) + case .unattributed: + // Nobody else holds the pad now, so whoever sent has gone. + if present.isEmpty { traffic = .none } + } + quietPIDs.formIntersection(present) + suspectedPIDs.formIntersection(present) + // Whoever holds the pad now has sent nothing since the last + // clear, or the reply would be outstanding. + if traffic == .none { + quietPIDs.formUnion(present.subtracting(suspectedPIDs)) + } } updateContention() } - /// The one place `contendingClient` is decided, from the replies seen - /// and the last scan: - /// - /// - unavailable: the replies alone (passive detection); - /// - authoritative (a dedicated vendor interface, USB): anyone else - /// there can drive the lighting, so a client Recheck did not - /// acknowledge raises it; - /// - advisory (the pad's only interface, Bluetooth): others may only be - /// listening for keys, so the list names but cannot raise. - /// - /// An empty list has already cleared the replies, in `applyScan()`. + /// Publishes the warning and the names it shows. Only replies raise it; + /// the scan names who may have sent them, else everyone else there. private func updateContention() { - switch lastScan { - case .unavailable: - contenders = [] - contendingClient = trafficSeen - case .authoritative(let others): + contendingClient = traffic != .none + let others = lastOthers ?? [] + guard case .from(let senders) = traffic else { contenders = others - contendingClient = trafficSeen || others.contains { !acknowledgedPIDs.contains($0.pid) } - case .advisory(let others): - contenders = others - contendingClient = trafficSeen + return } + let named = others.filter { senders.contains($0.pid) } + contenders = named.isEmpty ? others : named } - /// Clears both signals and the acknowledgement; for `start()` and - /// `stop()`. + /// Clears the replies seen, the last scan and what it taught; for + /// `start()` and `stop()`. private func resetContention() { - trafficSeen = false + traffic = .none lastScan = .unavailable - acknowledgedPIDs = [] + quietPIDs = [] + suspectedPIDs = [] lastReplyScan = nil updateContention() } @@ -578,9 +630,9 @@ public final class BridgeController: ObservableObject { warnedPermission = false deviceName = device.info?.product ?? "Work Louder device" lastError = nil - // A client that was already holding the pad is seen now, not - // only once it next sends something. - applyScan() + // Whoever already holds the pad has sent nothing we saw, so it + // counts as quiet: a later reply is laid at a newcomer first. + scanContention() } catch { deviceConnected = false let message = error.localizedDescription diff --git a/Sources/WLKit/PadEmulator.swift b/Sources/WLKit/PadEmulator.swift index 3413c95..a305818 100644 --- a/Sources/WLKit/PadEmulator.swift +++ b/Sources/WLKit/PadEmulator.swift @@ -102,7 +102,8 @@ public final class PadEmulator: ObservableObject { return (["ok": 1], nil) case "host.focused_app": - return (["ok": 1], nil) + // A bare null on the device, unlike the lighting calls' {"ok":1}. + return (NSNull(), nil) default: // Notifications are not callable, and neither is anything else the diff --git a/Sources/WLMicroManager/MenuPanelView.swift b/Sources/WLMicroManager/MenuPanelView.swift index 9fb08a1..dd172d1 100644 --- a/Sources/WLMicroManager/MenuPanelView.swift +++ b/Sources/WLMicroManager/MenuPanelView.swift @@ -47,7 +47,7 @@ struct MenuPanelView: View { Spacer(minLength: 0) Button("Recheck") { Task { await bridge.recheckContention() } } .controlSize(.small) - .help("Check again for other apps, accept the ones still holding the pad, and repaint it. The warning comes back if one of them sends to the pad, or another app opens it.") + .help("Clear the warning, check again for other apps, and repaint the pad. The warning comes back if another app sends to the pad again.") } .padding(.horizontal, 14).padding(.vertical, 8) } diff --git a/Tests/WLKitTests/BridgeContentionTests.swift b/Tests/WLKitTests/BridgeContentionTests.swift index 01a4298..cc1c503 100644 --- a/Tests/WLKitTests/BridgeContentionTests.swift +++ b/Tests/WLKitTests/BridgeContentionTests.swift @@ -1,6 +1,11 @@ import XCTest @testable import WLKit +private extension BridgeController { + /// A reply to a lighting call, which the firmware answers {"ok":1}. + func noteResponse(id: Int) { noteResponse(id: id, result: ["ok": 1], error: nil) } +} + /// The "another app is also driving this pad" warning, on the built-in /// virtual pad with `agent.list` stubbed out, so these run everywhere. /// @@ -100,6 +105,8 @@ final class BridgeContentionTests: XCTestCase { // MARK: - Registry scan private let inspector = HIDClient(pid: 4242, name: "Inspector") + private let input = HIDClient(pid: 5151, name: "Input") + private let relaunched = HIDClient(pid: 4343, name: "Inspector") /// What the stubbed scan returns next, and how often it was asked. private var scanResult: HIDClientScan = .unavailable private var scans = 0 @@ -125,6 +132,14 @@ final class BridgeContentionTests: XCTestCase { clock += BridgeController.replyScanInterval } + /// A foreign reply while the registry lists `clients`; the reply scans, + /// and the clock leaves the window for whatever the test does next. + private func reply(while clients: [HIDClient]) { + scanResult = .authoritative(clients) + bridge.noteResponse(id: foreignID) + clock += BridgeController.replyScanInterval + } + /// The emulator has no registry entry, so the real scan is unavailable /// and nothing is named: Phase 1 behaviour, which the tests above check. func testEmulatorScanIsUnavailable() async { @@ -135,8 +150,8 @@ final class BridgeContentionTests: XCTestCase { XCTAssertEqual(bridge.contenders, []) } - /// Row 1: an unavailable scan leaves the replies to decide, and does not - /// clear what they raised. + /// An unavailable scan leaves the replies to decide, and does not clear + /// what they raised. func testUnavailableScanFollowsTraffic() async { await startScanned() bridge.scanContention() @@ -148,8 +163,8 @@ final class BridgeContentionTests: XCTestCase { XCTAssertEqual(bridge.contenders, []) } - /// Row 2: nobody else on a dedicated vendor interface clears the - /// warning, and forgets the replies behind it. + /// Nobody else on the pad clears the warning, and forgets the replies + /// behind it. func testAuthoritativeEmptyClearsTraffic() async { await startScanned() seeTraffic() @@ -164,52 +179,280 @@ final class BridgeContentionTests: XCTestCase { XCTAssertFalse(bridge.contendingClient, "the replies were forgotten, not just outvoted") } - /// Row 3: anyone else on the vendor interface raises it with no traffic - /// at all, and is named. - func testAuthoritativeOthersRaise() async { + /// The same over Bluetooth. + func testAdvisoryEmptyClearsTraffic() async { await startScanned() + seeTraffic() + + scanResult = .advisory([]) + bridge.scanContention() XCTAssertFalse(bridge.contendingClient) - scanResult = .authoritative([inspector]) + scanResult = .unavailable + bridge.scanContention() + XCTAssertFalse(bridge.contendingClient, "the replies were forgotten, not just outvoted") + } + + /// The reported bug: Input running in the background holds the vendor + /// interface but sends nothing, and raised the warning on every launch + /// until Recheck. Holding the pad alone never raises it, over USB or + /// Bluetooth, at open, on a later scan, or after an off/on toggle; the + /// holder is still named should the warning come up. + func testHoldingThePadAloneNeverRaises() async { + await startScanned(.authoritative([input])) + XCTAssertFalse(bridge.contendingClient, "a holder seen as the pad opens") + XCTAssertEqual(bridge.contenders, [input]) + + scanResult = .authoritative([input, inspector]) bridge.scanContention() + XCTAssertFalse(bridge.contendingClient, "a holder that arrives later") + + scanResult = .advisory([input]) + bridge.scanContention() + XCTAssertFalse(bridge.contendingClient, "over Bluetooth") + + scanResult = .authoritative([input]) + await bridge.stop() + await bridge.start() + XCTAssertFalse(bridge.contendingClient, "after an off/on toggle") + } + + /// The user check: with Input quietly holding the pad, the Inspector + /// opens and sends. The warning names the Inspector alone and clears by + /// itself once a scan no longer lists it, though Input is still there. + func testAReplyIsLaidAtTheNewcomer() async { + await startScanned(.authoritative([input])) + + reply(while: [input, inspector]) XCTAssertTrue(bridge.contendingClient) XCTAssertEqual(bridge.contenders, [inspector]) - // And it goes once the other app has. + scanResult = .authoritative([input, inspector]) + bridge.scanContention() + XCTAssertTrue(bridge.contendingClient, "the Inspector is still there") + + scanResult = .authoritative([input]) + bridge.scanContention() + XCTAssertFalse(bridge.contendingClient, "the Inspector has quit") + XCTAssertEqual(bridge.contenders, [input]) + } + + /// The second report: Input announces the frontmost app + /// (`host.focused_app`) on every app switch, including the one when the + /// Inspector quits, and the firmware answers that with a bare null. Such + /// a reply neither raises the warning nor scans, so it cannot blame Input + /// once the Inspector has gone. + func testANullReplyIsNotAboutTheLighting() async { + await startScanned(.authoritative([input])) + reply(while: [input, inspector]) + XCTAssertEqual(bridge.contenders, [inspector]) + + scanResult = .authoritative([input]) + let before = scans + bridge.noteResponse(id: foreignID, result: NSNull(), error: nil) + bridge.noteResponse(id: foreignID, result: nil, error: nil) + XCTAssertEqual(scans, before, "a null reply does not scan") + XCTAssertTrue(bridge.contendingClient, "nor does it clear what the Inspector raised") + + bridge.scanContention() + XCTAssertFalse(bridge.contendingClient) + bridge.noteResponse(id: foreignID, result: NSNull(), error: nil) + XCTAssertFalse(bridge.contendingClient, "Input's focus announcement") + } + + /// The device's reply callback hands the result on, so the filter sees + /// what the pad answered. + func testTheDeviceReplyCarriesItsResult() async { + await bridge.start() + bridge.device.onResponse?(foreignID, NSNull(), nil) + XCTAssertFalse(bridge.contendingClient) + bridge.device.onResponse?(foreignID, ["ok": 1], nil) + XCTAssertTrue(bridge.contendingClient) + } + + /// The emulator answers `host.focused_app` the way the device does. + func testEmulatorAnswersFocusedAppWithNull() throws { + let pad = PadEmulator() + let (result, error) = pad.handle("host.focused_app", params: ["name": "Finder", "bundle_id": "com.apple.finder"]) + XCTAssertNil(error) + XCTAssertTrue(result is NSNull) + } + + /// An error reply still counts: whoever got it is talking to the pad, + /// and nothing says it was not about the lighting. + func testAnErrorReplyStillRaises() async { + await startScanned(.authoritative([input])) + scanResult = .authoritative([input, inspector]) + bridge.noteResponse(id: foreignID, result: nil, error: "Method not found") + XCTAssertTrue(bridge.contendingClient) + XCTAssertEqual(bridge.contenders, [inspector]) + } + + /// A null reply to one of our own calls still ends that call: had the + /// id stayed in flight, another client's reply under the same id would + /// later be taken for ours. + func testOurOwnNullReplyIsStillConsumed() async { + await bridge.start() + bridge.noteIssued(id: 500) + bridge.noteResponse(id: 500, result: NSNull(), error: nil) + bridge.noteResponse(id: 500) + XCTAssertTrue(bridge.contendingClient) + } + + /// A quiet holder that starts sending is all there is to blame, and the + /// warning stays until it has gone. + func testAQuietHolderThatSendsIsBlamed() async { + await startScanned(.authoritative([input])) + + reply(while: [input]) + XCTAssertTrue(bridge.contendingClient) + XCTAssertEqual(bridge.contenders, [input]) + + bridge.scanContention() + XCTAssertTrue(bridge.contendingClient) + scanResult = .authoritative([]) bridge.scanContention() XCTAssertFalse(bridge.contendingClient) - XCTAssertEqual(bridge.contenders, []) } - /// Row 4: an advisory empty list clears the warning and the replies. - func testAdvisoryEmptyClearsTraffic() async { - await startScanned() + /// A later reply adds to whom the earlier ones were laid at; it does + /// not replace them. + func testSuspectsAddUp() async { + await startScanned(.authoritative([input])) + reply(while: [input]) + reply(while: [input, inspector]) + XCTAssertEqual(bridge.contenders, [input, inspector]) + + scanResult = .authoritative([input]) + bridge.scanContention() + XCTAssertTrue(bridge.contendingClient, "Input sent the first reply") + } + + /// When every holder is quiet, a reply is laid at all of them, so the + /// warning outlives the sender's quitting until Recheck. They are not + /// marked for good, though: a later newcomer's reply is still laid at + /// the newcomer alone. + func testAReplyFromTheQuietIsLaidAtAllOfThem() async { + await startScanned(.authoritative([input, inspector])) + + reply(while: [input, inspector]) + XCTAssertEqual(bridge.contenders, [input, inspector]) + + scanResult = .authoritative([input]) + bridge.scanContention() + XCTAssertTrue(bridge.contendingClient, "Input may have sent it") + + await bridge.recheckContention() + XCTAssertFalse(bridge.contendingClient) + + reply(while: [input, relaunched]) + XCTAssertEqual(bridge.contenders, [relaunched]) + scanResult = .authoritative([input]) + bridge.scanContention() + XCTAssertFalse(bridge.contendingClient) + } + + /// Recheck with the Inspector still open clears the warning, and a scan + /// does not then take the Inspector for a quiet holder: its next reply + /// is laid at it alone, and the warning clears once it has quit. + func testRecheckKeepsANewcomerSuspected() async { + await startScanned(.authoritative([input])) + reply(while: [input, inspector]) + + await bridge.recheckContention() + XCTAssertFalse(bridge.contendingClient) + bridge.scanContention() + + reply(while: [input, inspector]) + XCTAssertEqual(bridge.contenders, [inspector]) + scanResult = .authoritative([input]) + bridge.scanContention() + XCTAssertFalse(bridge.contendingClient) + } + + /// A reply no scan could attribute stays until a scan finds nobody else: + /// any holder may have sent it. + func testAnUnattributedReplyNeedsAnEmptyScan() async { + await startScanned(.authoritative([input])) + scanResult = .unavailable seeTraffic() - scanResult = .advisory([]) + scanResult = .authoritative([input]) + bridge.scanContention() + XCTAssertTrue(bridge.contendingClient, "even the quiet Input may have sent it") + XCTAssertEqual(bridge.contenders, [input]) + + reply(while: [input, inspector]) + scanResult = .authoritative([input]) + bridge.scanContention() + XCTAssertTrue(bridge.contendingClient, "an attributed reply does not undo an unattributed one") + + scanResult = .authoritative([]) bridge.scanContention() XCTAssertFalse(bridge.contendingClient) + } + /// An unavailable scan says nothing about who left: it keeps the + /// suspects and the quiet holders alike. + func testUnavailableScanKeepsWhatItKnew() async { + await startScanned(.authoritative([input])) scanResult = .unavailable bridge.scanContention() - XCTAssertFalse(bridge.contendingClient, "the replies were forgotten, not just outvoted") + + reply(while: [input, inspector]) + XCTAssertEqual(bridge.contenders, [inspector], "Input is still quiet") + + scanResult = .unavailable + bridge.scanContention() + XCTAssertTrue(bridge.contendingClient, "the Inspector is still suspected") + + scanResult = .authoritative([input]) + bridge.scanContention() + XCTAssertFalse(bridge.contendingClient) } - /// Row 5: on a pad that also carries the keyboard, others may only be - /// listening for keys: the list names them but cannot raise the warning. - func testAdvisoryOthersOnlyLabel() async { - await startScanned(.advisory([inspector])) - XCTAssertFalse(bridge.contendingClient, "an advisory list alone does not raise it") - XCTAssertEqual(bridge.contenders, [inspector]) + /// A holder that leaves is no longer quiet: back again, it is a newcomer + /// to blame. + func testAHolderThatLeftIsNoLongerQuiet() async { + await startScanned(.authoritative([input])) + scanResult = .authoritative([]) + bridge.scanContention() - bridge.noteResponse(id: foreignID) - XCTAssertTrue(bridge.contendingClient) - XCTAssertEqual(bridge.contenders, [inspector]) + reply(while: [input, inspector]) + XCTAssertEqual(bridge.contenders, [input, inspector]) + } + + /// A suspect that leaves is forgotten: a process back under its pid + /// (pids are reused) and holding the pad quietly is quiet like any other. + func testASuspectThatLeftIsForgotten() async { + await startScanned(.authoritative([input])) + reply(while: [input, inspector]) + scanResult = .authoritative([input]) + bridge.scanContention() + XCTAssertFalse(bridge.contendingClient) + + scanResult = .authoritative([input, inspector]) + bridge.scanContention() + reply(while: [input, inspector]) + XCTAssertEqual(bridge.contenders, [input, inspector]) + } + + /// Switching off and on forgets who was suspected: the reopen takes + /// every holder for quiet. + func testStopForgetsTheSuspects() async { + await startScanned(.authoritative([input])) + reply(while: [input, inspector]) + await bridge.recheckContention() + + await bridge.stop() + await bridge.start() + reply(while: [input, inspector]) + XCTAssertEqual(bridge.contenders, [input, inspector]) } - /// A foreign reply scans to name its sender, but at most once a second. - /// A reply inside the window still raises the warning. + /// A foreign reply scans to see who may have sent it, but at most once + /// a second. A reply inside the window still raises the warning. func testReplyScansAreLimitedToOnePerSecond() async { await startScanned() let afterStart = scans @@ -237,37 +480,37 @@ final class BridgeContentionTests: XCTestCase { } /// The reply-triggered scan is what puts a name on a client first seen - /// by its traffic. + /// by its traffic, over USB and Bluetooth alike. func testForeignReplyNamesTheSender() async { await startScanned() scanResult = .authoritative([inspector]) bridge.noteResponse(id: foreignID) XCTAssertTrue(bridge.contendingClient) XCTAssertEqual(bridge.contenders, [inspector]) - } - /// A client already holding the pad is seen as the bridge opens it. - func testOpenScans() async { - await startScanned(.authoritative([inspector])) + await bridge.stop() + await startScanned() + scanResult = .advisory([inspector]) + bridge.noteResponse(id: foreignID) XCTAssertTrue(bridge.contendingClient) XCTAssertEqual(bridge.contenders, [inspector]) } /// Who held a pad that has gone says nothing about the pad that comes - /// back: an unplug drops the scan, and the reopen scans afresh. + /// back: an unplug drops the names, and the reopen scans afresh. The + /// replies seen stay, as they do without a scan. func testUnplugDropsTheScan() async { - await startScanned(.authoritative([inspector])) - XCTAssertTrue(bridge.contendingClient) + await startScanned(.authoritative([input])) + reply(while: [input, inspector]) bridge.device.disconnect(reason: "unplugged") XCTAssertFalse(bridge.deviceConnected) - XCTAssertFalse(bridge.contendingClient) + XCTAssertTrue(bridge.contendingClient) XCTAssertEqual(bridge.contenders, []) } - /// Recheck forgets the replies and rescans: an empty authoritative list - /// clears the warning, and so does one that still has the other app, - /// which Recheck acknowledges. It repaints either way. + /// Recheck forgets the replies and rescans, whoever is still there, and + /// repaints (`testRecheckClearsAndRepaints`). func testRecheckScans() async throws { await startScanned() seeTraffic() @@ -276,114 +519,19 @@ final class BridgeContentionTests: XCTestCase { scanResult = .authoritative([inspector]) await bridge.recheckContention() XCTAssertEqual(scans, before + 1) - XCTAssertFalse(bridge.contendingClient, "the app still there is acknowledged") - XCTAssertEqual(bridge.contenders, [inspector], "and still listed") - - scanResult = .authoritative([]) - await bridge.recheckContention() XCTAssertFalse(bridge.contendingClient) + XCTAssertEqual(bridge.contenders, [inspector], "and still listed") scanResult = .unavailable bridge.scanContention() XCTAssertFalse(bridge.contendingClient, "the replies seen earlier stay forgotten") } - // MARK: - Acknowledged clients - - private let relaunched = HIDClient(pid: 4343, name: "Inspector") - private let input = HIDClient(pid: 5151, name: "Input") - - /// An app that holds the vendor interface but never sends (one that opens - /// every HID device, say) raises the warning on every scan. Recheck lets - /// it stay: later scans that still list it keep the warning down, while - /// it is still named should the warning come up for another reason. - func testRecheckAcknowledgesAPassiveHolder() async { - await startScanned(.authoritative([inspector])) - XCTAssertTrue(bridge.contendingClient) - - await bridge.recheckContention() - XCTAssertFalse(bridge.contendingClient) - - bridge.scanContention() - XCTAssertFalse(bridge.contendingClient, "a later scan still listing it") - XCTAssertEqual(bridge.contenders, [inspector]) - } - - /// Acknowledging one app does not excuse another that opens the pad - /// later. - func testAnUnacknowledgedClientStillRaises() async { - await startScanned(.authoritative([inspector])) - await bridge.recheckContention() - XCTAssertFalse(bridge.contendingClient) - - scanResult = .authoritative([inspector, input]) - bridge.scanContention() - XCTAssertTrue(bridge.contendingClient) - XCTAssertEqual(bridge.contenders, [inspector, input]) - } - - /// An acknowledged app that sends to the pad raises the warning through - /// its replies, and is named. - func testAnAcknowledgedClientsReplyStillRaises() async { - await startScanned(.authoritative([inspector])) - await bridge.recheckContention() - XCTAssertFalse(bridge.contendingClient) - - bridge.noteResponse(id: foreignID) - XCTAssertTrue(bridge.contendingClient) - XCTAssertEqual(bridge.contenders, [inspector]) - } - - /// The acknowledgement is by pid. Once a scan no longer lists the app it - /// is forgotten, so the same app relaunched raises the warning again — - /// even if it comes back under the old pid. - func testAQuitAppIsForgotten() async { - await startScanned(.authoritative([inspector])) - await bridge.recheckContention() - - scanResult = .authoritative([]) - bridge.scanContention() - XCTAssertFalse(bridge.contendingClient) - - scanResult = .authoritative([relaunched]) - bridge.scanContention() - XCTAssertTrue(bridge.contendingClient, "a relaunch under a new pid") - - scanResult = .authoritative([]) - bridge.scanContention() - scanResult = .authoritative([inspector]) - bridge.scanContention() - XCTAssertTrue(bridge.contendingClient, "the old pid, after a scan without it") - } - - /// An unavailable scan says nothing about who left, so it keeps the - /// acknowledgement. - func testUnavailableScanKeepsTheAcknowledgement() async { - await startScanned(.authoritative([inspector])) - await bridge.recheckContention() - - scanResult = .unavailable - bridge.scanContention() - scanResult = .authoritative([inspector]) - bridge.scanContention() - XCTAssertFalse(bridge.contendingClient) - } - - /// Switching off and on forgets what Recheck acknowledged. - func testStopForgetsTheAcknowledgement() async { - await startScanned(.authoritative([inspector])) - await bridge.recheckContention() - XCTAssertFalse(bridge.contendingClient) - - await bridge.stop() - await bridge.start() - XCTAssertTrue(bridge.contendingClient, "the open scan lists it again") - } - /// Switched off, nothing is scanned and nothing is named, whatever the /// registry would say. func testOffBridgeDoesNotScan() async { - await startScanned(.authoritative([inspector])) + await startScanned(.authoritative([input])) + reply(while: [input, inspector]) XCTAssertTrue(bridge.contendingClient) await bridge.stop() diff --git a/design/pad-contention.md b/design/pad-contention.md index 1b86990..2d482e0 100644 --- a/design/pad-contention.md +++ b/design/pad-contention.md @@ -166,40 +166,36 @@ not see this process's own client is `.unavailable` too (Phase 2 Notes). #### Bridge and panel (Phase 3) -State, in `BridgeController`: - -- `trafficSeen: Bool` (private): today's flag, renamed. Set by - `noteResponse(id:)` for a foreign id, cleared by `start()`, `stop()` and a - recheck. +State, in `BridgeController` (as fixed after Phase 3; see "Passive holders" +below for the rule this replaced): + +- `traffic` (private): `.none`; `.from(Set)`, the other clients that + may have sent the foreign replies seen since the last clear; or + `.unattributed`, replies seen while no scan could say who was there. +- `quietPIDs` (private): other clients seen holding the pad while `traffic` + was `.none`, i.e. holding it without sending. +- `suspectedPIDs` (private): newcomers a reply was laid at. A scan never + counts them as quiet while they are present. - `@Published public private(set) var contenders: [HIDClient]`: the last - scan's list, for the label. + scan's others that are in `.from`, else all of them, for the label. - `@Published public private(set) var contendingClient: Bool` stays the one - flag the panel shows. It is recomputed in one private `updateContention()` - from `trafficSeen` and the last scan: - -| last scan | others | `contendingClient` | effect on `trafficSeen` | -|---|---|---|---| -| `.unavailable` | — | `trafficSeen` | none (Phase 1 behaviour) | -| `.authoritative` | empty | false | cleared: whoever sent it is gone | -| `.authoritative` | some | true, unless all acknowledged | none | -| `.advisory` | empty | false | cleared | -| `.advisory` | some | `trafficSeen` | none: the list labels, it cannot raise | - -Acknowledged clients (added after PR #20's review): `recheckContention()` -records the pids of the `.authoritative` others it finds in a private -`acknowledgedPIDs: Set`, and only others outside that set raise the -warning. Without it, an app that opens every HID device (an input remapper, -a macro or streaming tool) but never sends would raise the warning on every -scan, and nothing short of quitting it would clear it: Recheck rescans and -finds it again, and so does an off/on toggle. Acknowledged clients: - -- are still listed in `contenders`, so they are named if the warning comes - up for another reason; -- still raise it through `trafficSeen` when they send; -- drop out of the set when a scan (`.authoritative` or `.advisory`) no - longer lists them, so a relaunched app raises the warning again; - `.unavailable` keeps the set, since it says nothing about who left; -- are forgotten by `start()` and `stop()` (`resetContention()`). + flag the panel shows: `traffic != .none`. **Only replies raise it.** + +A foreign reply is laid at the newcomers of the last scan (others not in +`quietPIDs`), or at every other when all of them are quiet, and added to the +earlier suspects; with no scan to go on it is `.unattributed`, and stays so. +Each scan that is not `.unavailable` (`.authoritative` and `.advisory` +alike) then: + +| `traffic` | effect | +|---|---| +| `.none` | the others not in `suspectedPIDs` join `quietPIDs` | +| `.from(s)` | `s` shrinks to the others still listed; `.none` once empty | +| `.unattributed` | `.none` only when nobody else is listed | + +and prunes `quietPIDs` and `suspectedPIDs` to the others it lists. +`.unavailable` changes nothing. `start()` and `stop()` forget it all +(`resetContention()`). Scans run: @@ -207,9 +203,8 @@ Scans run: `didBecomeKeyNotification` hooks in `MenuPanelView`, which already call `reloadRemotes()`, also call a new `bridge.scanContention()` (scan only, no repaint); -- from **Recheck**: `recheckContention()` clears `trafficSeen`, scans, - acknowledges the `.authoritative` others it found, then repaints as in - Phase 1; +- from **Recheck**: `recheckContention()` clears `traffic`, scans, then + repaints as in Phase 1; - after a foreign reply, to put a name on it: at most one scan per second, so a chatty client cannot turn every reply into a registry walk; - after the device (re)opens in `openDevice()`. @@ -224,8 +219,8 @@ time; `useEmulator` replaces it), like `listAgents`. Panel copy: -- authoritative, or advisory with traffic seen and names known: "Also driving - this pad: Input, Inspector — colours may fight." +- names known: "Also driving this pad: Micro Manager Inspector — colours may + fight." - names unknown: today's text. - **Recheck** stays in every case. @@ -484,6 +479,76 @@ Notes: raise (authoritative-others, open, unplug, off, passive holder, unacknowledged, quit app forgotten, stop forgets); an acknowledged client hides `trafficSeen` (acknowledged reply). +- **Passive holders (bug fix after PR #20).** Reported: right after launch, + with nothing else open, the panel read "Also driving this pad: input — + colours may fight."; Recheck cleared it; with the Inspector open it read + "input, Micro Manager Inspector". Cause: Work Louder's Input + (`/Applications/input.app`) kept running in the background after its + window was closed, and held a client on the vendor interface + (`ioreg`: `"pid 24802, input"` under `PrimaryUsagePage` 65280, the same + pid as in Phase 2's verify) without sending. The rule "an `.authoritative` + scan with others raises the warning by itself" turned that into a warning + on every launch, since `start()` forgets what Recheck acknowledged, and + `acknowledgedPIDs` only hid it until the next off/on or relaunch. Holding + is not driving — "colours may fight" needs a sender — so the fix drops the + presence rule: only foreign replies raise the warning, and the scan says + who may have sent them (Part 1). Laying a reply at every other client + would have kept the warning up after the Inspector quit for as long as + Input runs, which breaks Phase 3's clear-on-reopen in exactly this setup; + hence `quietPIDs` and the newcomer rule. `suspectedPIDs` keeps a Recheck + (or a panel scan after it) with the Inspector still open from making the + Inspector quiet, after which its next reply would be laid at Input too. + `acknowledgedPIDs` is gone, and so is the bridge's distinction between + `.authoritative` and `.advisory`; Recheck's help text is now "Clear the + warning, check again for other apps, and repaint the pad. The warning + comes back if another app sends to the pad again." +- Known limits of the newcomer rule: a client that was already holding the + pad when it was first seen (at open, say) counts as quiet, so its first + reply is laid at every other client too, and the warning then stays until + they have all gone or Recheck is pressed. A reply within the reply-scan + window is laid at the clients of the last scan, so an app that opened and + sent within that second goes unblamed until its next reply. +- Mutation checks for the fix, each restored afterwards: presence raises + again (holding-alone, newcomer, quiet-at-all, recheck-suspected, + recheck-scans, unavailable-keeps); no newcomer rule (newcomer, + quiet-at-all, recheck-suspected, unavailable-keeps); newcomers not + suspected (recheck-suspected); quiet not pruned (holder-left); suspects + not pruned (suspect-left); reset keeps suspects (stop-forgets); + `.unattributed` clears on any scan, and `.unattributed` turned into + `.from` by a later reply (both unattributed-needs-empty); earlier suspects + replaced, not added to (suspects-add-up); quiet holders blamed on nobody + (quiet-sends, quiet-at-all); no quiet marking (newcomer, quiet-at-all, + recheck-suspected, unavailable-keeps); `.unavailable` clears `quietPIDs` + (unavailable-keeps); names not filtered by the suspects (newcomer, + quiet-at-all, recheck-suspected, unavailable-keeps); Recheck keeps the + traffic (recheck tests, stop); `openDevice()` does not scan + (holding-alone, newcomer, limit, and others). +- No hardware or UI check of the fix was done in-session. User checks: with + Input running in the background, launch → no warning; open the Inspector + → "Also driving this pad: Micro Manager Inspector"; quit it and reopen + the panel → warning gone. +- **Input's focus announcements (second report on the same PR).** After the + fix above, opening and quitting the Inspector left "Also driving this pad: + input" until Recheck. A live probe (a test process holding the pad, + logging every reply and scan while the Inspector was opened and quit, then + while only the frontmost app was switched) showed a foreign reply shortly + after each frontmost-app change (0.27 s after the Inspector quit), Inspector or not, with a random id + and `"result": null`. Input sends `host.focused_app` on every app switch; + the probe's own `host.focused_app` got the same null back, while an empty + `v.oai.thstatus` got `{"ok":1}` and `sys.version` an object. The reply + after the Inspector quit found only Input in the scan; Input was quiet, so + the fallback blamed it, and the warning stayed for as long as Input runs. + Fix: `noteResponse(id:result:error:)` ignores a foreign reply whose result + is null (nil or `NSNull`) and that carries no error, after the own-id check. + Error replies still count. `PadEmulator` now answers `host.focused_app` + with `NSNull()`, as the device does. +- Mutation checks for it, each restored afterwards: no null filter + (null-reply); filter before the own-id check (own-null-consumed); errors + ignored too (error-reply); only `NSNull` ignored, not nil (null-reply); + emulator answers `{"ok":1}` (emulator-null); `wire(_:)` drops the result + (device-reply-result). +- Not checked on hardware after the fix: whether the warning stays down + through an Inspector round trip with Input running (user check). ### Done diff --git a/docs/hacking.md b/docs/hacking.md index 7b96e9a..d62942f 100644 --- a/docs/hacking.md +++ b/docs/hacking.md @@ -691,6 +691,18 @@ too, so its list mixes both. `WLDevice.otherClients()` does this walk in Swift (`Sources/WLKit/WLDevice+Clients.swift`). `IOUserClientCreator` is not documented API, and the kernel may cut a long process name short. +Holding is not driving: Work Louder's Input app keeps its client on the vendor +interface for as long as it runs, window or not, so a listed process is only +someone who *could* send. The bridge warns on replies only, and uses the list +to say who may have sent them and when they have gone. + +Not every foreign reply is about the lighting, either. Input sends +`host.focused_app` on every frontmost-app change, window or not, and the +firmware answers it with a bare `"result": null` (checked on 2026-10-10; the +lighting calls answer `{"ok":1}`, `sys.version` an object). So a foreign reply +with a null result is ignored. Request ids are no help in telling senders +apart: Input's look random, while `WLDevice` counts 1…998. + **Variant gating is real.** These vendor methods are registered per hardware variant. If `v.oai.thstatus` returns `Method not found` rather than `{"ok":1}`, this firmware genuinely does not have it. Older Creator Micro 2 firmware