From 2ee0d1ca5b84f45a267b62a05c88ad6518d60f6b Mon Sep 17 00:00:00 2001 From: Johnny D Date: Wed, 9 Sep 2026 13:29:00 -0400 Subject: [PATCH 1/2] fix(session): preserve command correlation and respect model capabilities --- .../Bluetooth/ControllerSession.swift | 41 +++++++---- tests/session/FrameworkFakes.swift | 42 +++++++++++ tests/session/SessionTests.swift | 70 +++++++++++++++++++ tests/session/run.sh | 20 ++++++ 4 files changed, 158 insertions(+), 15 deletions(-) create mode 100644 tests/session/FrameworkFakes.swift create mode 100644 tests/session/SessionTests.swift create mode 100755 tests/session/run.sh diff --git a/Sources/FinallyTheControllerWorks/Bluetooth/ControllerSession.swift b/Sources/FinallyTheControllerWorks/Bluetooth/ControllerSession.swift index ae1ff9c..598f7d5 100644 --- a/Sources/FinallyTheControllerWorks/Bluetooth/ControllerSession.swift +++ b/Sources/FinallyTheControllerWorks/Bluetooth/ControllerSession.swift @@ -298,7 +298,10 @@ final class ControllerSession: NSObject, @unchecked Sendable { } private func handleCommandResponse(_ data: Data) { - guard let pending = pendingCommand else { return } + // An unrelated notification must not consume the command or its timer. + guard let pending = pendingCommand, data.count >= 8, + data[data.startIndex] == pending.id, + data[data.startIndex + 1] == 0x01 || data[data.startIndex + 1] == 0x02 else { return } commandTimeout?.cancel() pendingCommand = nil // NFC experiments: log the COMPLETE frame (header included) — the @@ -310,11 +313,6 @@ final class ControllerSession: NSObject, @unchecked Sendable { // status/error reply (same shape, payload starts with a status code). // Both correlate to our command — pass the payload up and let the // caller interpret the status byte. - guard data.count >= 8, data[data.startIndex] == pending.id, - data[data.startIndex + 1] == 0x01 || data[data.startIndex + 1] == 0x02 else { - pending.completion(nil) - return - } pending.completion(data.subdata(in: data.startIndex + 8 ..< data.endIndex)) } @@ -323,7 +321,8 @@ final class ControllerSession: NSObject, @unchecked Sendable { let payload = Switch2.memoryReadPayload(length: length, address: address) writeCommand(Switch2.Command.memory, Switch2.Subcommand.memoryRead, payload) { resp in guard let resp, resp.count >= 8 + Int(length), - resp[resp.startIndex] == length else { + resp[resp.startIndex] == length, + Switch2.u32(resp, 4) == address else { completion(nil); return } completion(resp.subdata(in: resp.startIndex + 8 ..< resp.endIndex)) @@ -572,8 +571,7 @@ final class ControllerSession: NSObject, @unchecked Sendable { func setAudioCapture(_ enabled: Bool, completion: @escaping (Bool) -> Void) { queue.async { [weak self] in guard let self, let ch = self.chars[Self.audioInputUUID] else { - completion(false); return - } + completion(false); return } self.peripheral.setNotifyValue(enabled, for: ch) completion(true) } @@ -604,7 +602,9 @@ final class ControllerSession: NSObject, @unchecked Sendable { // can never leave the motor running. if now - rumbleSetAt > 0.5 { strong = 0; weakMag = 0 } - if strong > 0.001 || weakMag > 0.001 || rumbleActive { + // The GameCube model explicitly lacks this motor protocol. Do not + // suppress its LED keep-alive when an unsupported rumble is requested. + if model.hasHDRumble && (strong > 0.001 || weakMag > 0.001 || rumbleActive) { let active = strong > 0.001 || weakMag > 0.001 writeMotor(Switch2.Vibration.waveform(strong: strong, weak: weakMag)) rumbleActive = active @@ -617,7 +617,8 @@ final class ControllerSession: NSObject, @unchecked Sendable { } private func writeMotor(_ vib: Switch2.Vibration) { - guard let motor = chars[Switch2.GATT.vibration(for: model)] else { return } + guard model.hasHDRumble, + let motor = chars[Switch2.GATT.vibration(for: model)] else { return } let packet = Switch2.motorPacket(vib, packetID: vibrationPacketID, model: model) vibrationPacketID &+= 1 peripheral.writeValue(packet, for: motor, type: .withoutResponse) @@ -626,6 +627,16 @@ final class ControllerSession: NSObject, @unchecked Sendable { // MARK: - Input reports + /// Nominal 12-bit range only when factory/user calibration is unavailable. + /// This is degraded, uncalibrated input, not a claim of factory accuracy. + private static func uncalibratedStick(_ raw: (UInt16, UInt16)) -> (Double, Double) { + func axis(_ value: UInt16) -> Double { + let offset = Double(value) - 2048 + return max(-1, min(1, offset / (offset >= 0 ? 2047 : 2048))) + } + return (axis(raw.0), axis(raw.1)) + } + private func handleInputReport(_ data: Data) { guard let report = Switch2.InputReport(data: data) else { return } let now = CFAbsoluteTimeGetCurrent() @@ -641,14 +652,14 @@ final class ControllerSession: NSObject, @unchecked Sendable { switch model { case .joyCon2Left: // One stick, reporting in the first field, calibrated by slot 1. - s.leftStick = leftCal?.apply(report.leftStickRaw) ?? (0, 0) + s.leftStick = leftCal?.apply(report.leftStickRaw) ?? Self.uncalibratedStick(report.leftStickRaw) case .joyCon2Right: // One stick, reporting in the SECOND field — but calibrated by // the unit's slot-1 data (a Joy-Con has no slot-2 calibration). - s.rightStick = leftCal?.apply(report.rightStickRaw) ?? (0, 0) + s.rightStick = leftCal?.apply(report.rightStickRaw) ?? Self.uncalibratedStick(report.rightStickRaw) default: - s.leftStick = leftCal?.apply(report.leftStickRaw) ?? (0, 0) - s.rightStick = rightCal?.apply(report.rightStickRaw) ?? (0, 0) + s.leftStick = leftCal?.apply(report.leftStickRaw) ?? Self.uncalibratedStick(report.leftStickRaw) + s.rightStick = rightCal?.apply(report.rightStickRaw) ?? Self.uncalibratedStick(report.rightStickRaw) } if model.hasAnalogTriggers { s.leftTrigger = report.leftTriggerRaw diff --git a/tests/session/FrameworkFakes.swift b/tests/session/FrameworkFakes.swift new file mode 100644 index 0000000..1ffd74b --- /dev/null +++ b/tests/session/FrameworkFakes.swift @@ -0,0 +1,42 @@ +// Test-only stand-ins. The actual macOS app is separately built with Apple SDKs. +import Foundation + +struct CBCharacteristicProperties: OptionSet { + let rawValue: Int + static let notify = Self(rawValue: 1) +} +final class CBUUID { + let uuidString: String + init(_ uuid: UUID) { uuidString = uuid.uuidString } +} +final class CBCharacteristic { + let uuid: CBUUID + var properties: CBCharacteristicProperties = [.notify] + var value: Data? + var isNotifying = false + init(_ uuid: UUID) { self.uuid = CBUUID(uuid) } +} +final class CBService { var characteristics: [CBCharacteristic]? } +protocol CBPeripheralDelegate: AnyObject {} +enum CBCharacteristicWriteType { case withoutResponse } +final class CBPeripheral { + weak var delegate: CBPeripheralDelegate? + var services: [CBService]? + var canSendWriteWithoutResponse = true + var writes: [(Data, CBCharacteristic)] = [] + var identifier = UUID() + func discoverServices(_ uuids: [CBUUID]?) {} + func discoverCharacteristics(_ uuids: [CBUUID]?, for service: CBService) {} + func setNotifyValue(_ enabled: Bool, for ch: CBCharacteristic) { ch.isNotifying = enabled } + func writeValue(_ data: Data, for ch: CBCharacteristic, type: CBCharacteristicWriteType) { + writes.append((data, ch)) + } + func readRSSI() {} + func maximumWriteValueLength(for type: CBCharacteristicWriteType) -> Int { 180 } +} +final class IOBluetoothHostController { + static func `default`() -> IOBluetoothHostController? { nil } + func addressAsString() -> String? { nil } +} +enum LogLevel { case debug, info, warning, error } +func bridgeLog(_ level: LogLevel, _ category: String, _ message: String) {} diff --git a/tests/session/SessionTests.swift b/tests/session/SessionTests.swift new file mode 100644 index 0000000..d94aa53 --- /dev/null +++ b/tests/session/SessionTests.swift @@ -0,0 +1,70 @@ +import Foundation + +final class Delegate: ControllerSessionDelegate { + var ready = 0 + var failures = 0 + func sessionReady(_ session: ControllerSession) { ready += 1 } + func sessionFailed(_ session: ControllerSession, reason: String) { failures += 1 } + func sessionDidUpdateState(_ session: ControllerSession) {} +} + +@main +enum SessionTests { + static func fixture() -> (ControllerSession, CBPeripheral, DispatchQueue, Delegate) { + let p = CBPeripheral(), q = DispatchQueue(label: "session-test"), d = Delegate() + let s = ControllerSession(peripheral: p, slot: 0, wasPairingMode: false, queue: q, delegate: d) + for uuid in [Switch2.GATT.commandWrite, Switch2.GATT.commandResponse, Switch2.GATT.inputReport, + Switch2.GATT.vibrationPro, Switch2.GATT.vibrationJoyConL, Switch2.GATT.vibrationJoyConR] { + s.chars[uuid] = CBCharacteristic(uuid) + } + return (s, p, q, d) + } + static func main() { + let selected = CommandLine.arguments.last! + func run(_ name: String, _ test: () -> Void) { + if selected == "all" || selected == name { test(); print("PASS \(name)") } + } + run("rumble") { + for model in Switch2.Model.allCases { + let (s, p, q, d) = fixture(); defer { s.teardown(); _ = d } + s.model = model + s.setRumble(strong: 1, weak: 0) + q.sync { s.maintainTick() } + let motors = p.writes.filter { $0.1.uuid.uuidString == Switch2.GATT.vibration(for: model).uuidString } + precondition(motors.isEmpty == !model.hasHDRumble, "GameCube must not receive unsupported HD motor writes") + if !model.hasHDRumble { + precondition(p.writes.contains { $0.0.first == Switch2.Command.leds }, "Unsupported rumble must not suppress keep-alive") + let count = p.writes.count + s.writeMotor(.tone(freqHz: 200, amp: 1)) + precondition(p.writes.count == count, "Direct/experimental motor calls must obey capability") + } + } + } + run("calibration") { + let (s, _, _, d) = fixture(); defer { s.teardown(); _ = d } + var bytes = Data(repeating: 0, count: 63) + bytes[10] = 0xff; bytes[11] = 0x0f; bytes[12] = 0x00 // X=4095, Y=0 + bytes[13] = 0x00; bytes[14] = 0x08; bytes[15] = 0x80 // centered + s.handleInputReport(bytes) + precondition(s.state.leftStick.x == 1 && s.state.leftStick.y == -1, "Missing calibration must not freeze the stick") + precondition(s.state.rightStick == (0, 0)) + } + run("unrelated-response") { + let (s, _, _, d) = fixture(); defer { s.teardown(); _ = d } + var calls = 0 + s.writeCommand(0x09, 0x07, Data()) { _ in calls += 1 } + s.handleCommandResponse(Data([2, 1, 0, 0, 0, 0, 0, 0])) + precondition(s.pendingCommand != nil && calls == 0, "Unrelated reply consumed the active command") + s.handleCommandResponse(Data([9, 1, 0, 0, 0, 0, 0, 0])) + precondition(calls == 1 && s.pendingCommand == nil) + } + run("memory-address") { + let (s, _, _, d) = fixture(); defer { s.teardown(); _ = d } + var succeeded = false + s.readMemory(length: 1, address: 0x13000) { succeeded = $0 != nil } + let frame = Data([2, 1, 0, 0, 0, 0, 0, 0, 1, 0x7e, 0, 0, 0x42, 0x30, 1, 0, 0xaa]) + s.handleCommandResponse(frame) + precondition(!succeeded, "A different memory address must not supply calibration/identity data") + } + } +} diff --git a/tests/session/run.sh b/tests/session/run.sh new file mode 100755 index 0000000..c449f65 --- /dev/null +++ b/tests/session/run.sh @@ -0,0 +1,20 @@ +#!/bin/bash +set -euo pipefail +cd "$(dirname "$0")/../.." +work=$(mktemp -d) +trap 'rm -rf "$work"' EXIT +# Change visibility/imports only, not method bodies. Fake CoreBluetooth objects +# allow the production session callbacks to run without a radio or permission. +python3 - "$work/ControllerSession.swift" <<'PY' +from pathlib import Path +import os, re, sys +source = Path(os.environ.get('SESSION_SOURCE', 'Sources/FinallyTheControllerWorks/Bluetooth/ControllerSession.swift')).read_text() +source = re.sub(r'^import (CoreBluetooth|IOBluetooth)$', '', source, flags=re.M) +source = re.sub(r'\b(?:fileprivate|private)(?:\(set\))?\s+', '', source) +Path(sys.argv[1]).write_text('import CoreFoundation\n' + source) +PY +swiftc -swift-version 5 \ + Sources/FinallyTheControllerWorks/Protocol/Switch2Protocol.swift \ + "$work/ControllerSession.swift" tests/session/FrameworkFakes.swift \ + tests/session/SessionTests.swift -o "$work/session-tests" +"$work/session-tests" "${SESSION_CASE:-all}" From 5b9ccaf6fb9bfd3cc469347dca9bcb23798e8476 Mon Sep 17 00:00:00 2001 From: Johnny D Date: Wed, 9 Sep 2026 20:01:01 -0400 Subject: [PATCH 2/2] fix(session): retire callbacks before teardown and require usable input for readiness (#8) Guard late notifications, queued commands and timers after session retirement. Clear notify completion before invocation to preserve reentrant replacement. Require isNotifying on essential channels and one existing-decoder input report before readiness, retaining the established keep-alive and handshake bytes. Eleven synthetic production-session tests pass locally; five new targeted cases fail against the prior session source. Full Apple-framework build and regressions remain the hosted gate. No new connection deadline or protocol. --- .../Bluetooth/ControllerSession.swift | 79 ++++++++++++++----- docs/session-retirement.md | 30 +++++++ tests/session/SessionTests.swift | 72 +++++++++++++++++ 3 files changed, 160 insertions(+), 21 deletions(-) create mode 100644 docs/session-retirement.md diff --git a/Sources/FinallyTheControllerWorks/Bluetooth/ControllerSession.swift b/Sources/FinallyTheControllerWorks/Bluetooth/ControllerSession.swift index 598f7d5..888190f 100644 --- a/Sources/FinallyTheControllerWorks/Bluetooth/ControllerSession.swift +++ b/Sources/FinallyTheControllerWorks/Bluetooth/ControllerSession.swift @@ -68,6 +68,9 @@ final class ControllerSession: NSObject, @unchecked Sendable { private var chars: [UUID: CBCharacteristic] = [:] private var handshakeStarted = false + private var ended = false + private var handshakeComplete = false + private var readyReported = false private var pendingCommand: (id: UInt8, completion: (Data?) -> Void)? private var commandTimeout: DispatchWorkItem? private var handshakeSteps: [(String, (@escaping (Bool) -> Void) -> Void)] = [] @@ -125,11 +128,18 @@ final class ControllerSession: NSObject, @unchecked Sendable { /// Called by the engine once CoreBluetooth reports the connect. func begin() { + guard !ended else { return } log(.info, "slot \(slot + 1): discovering services") peripheral.discoverServices(nil) } func teardown() { + guard !ended else { return } + ended = true + notifyCompletion = nil + handshakeSteps.removeAll() + onState = nil + onRSSI = nil keepAliveTimer?.cancel() keepAliveTimer = nil commandTimeout?.cancel() @@ -143,12 +153,14 @@ final class ControllerSession: NSObject, @unchecked Sendable { } private func fail(_ reason: String) { + guard !ended else { return } log(.error, "slot \(slot + 1): \(reason)") teardown() delegate?.sessionFailed(self, reason: reason) } private func runHandshake() { + guard !ended else { return } // Order matters and mirrors the console: command-response subscribe // must precede any command; identity before vibration char choice. handshakeSteps = [ @@ -163,15 +175,17 @@ final class ControllerSession: NSObject, @unchecked Sendable { } private func advanceHandshake() { + guard !ended else { return } guard !handshakeSteps.isEmpty else { + handshakeComplete = true + // Keep the existing keep-alive while awaiting the first report. startKeepAlive() - log(.info, "slot \(slot + 1): handshake complete — \(displayName) serial \(serialNumber)") - delegate?.sessionReady(self) + if announceReady() { onState?(slot, state) } return } let (name, step) = handshakeSteps.removeFirst() step { [weak self] ok in - guard let self else { return } + guard let self, !self.ended else { return } if ok { self.advanceHandshake() } else { @@ -180,6 +194,16 @@ final class ControllerSession: NSObject, @unchecked Sendable { } } + /// Notification subscription alone is not usable controller input. + @discardableResult + private func announceReady() -> Bool { + guard !ended, handshakeComplete, reportCount > 0, !readyReported else { return false } + readyReported = true + log(.info, "slot \(slot + 1): handshake and first input complete — \(displayName)") + delegate?.sessionReady(self) + return true + } + // MARK: Handshake steps private func stepReadInfo(_ done: @escaping (Bool) -> Void) { @@ -196,7 +220,7 @@ final class ControllerSession: NSObject, @unchecked Sendable { private func stepReadCalibration(_ done: @escaping (Bool) -> Void) { readStickCalibration(user: Switch2.Address.userStick1, factory: Switch2.Address.factoryStick1) { [weak self] cal in - guard let self else { return } + guard let self, !self.ended else { return } self.leftCal = cal guard self.model.hasSecondStick else { done(true) // single-stick unit: slot-2 holds no valid data @@ -204,7 +228,7 @@ final class ControllerSession: NSObject, @unchecked Sendable { } self.readStickCalibration(user: Switch2.Address.userStick2, factory: Switch2.Address.factoryStick2) { [weak self] cal in - guard let self else { return } + guard let self, !self.ended else { return } self.rightCal = cal done(true) // calibration is best-effort; defaults are usable } @@ -214,7 +238,7 @@ final class ControllerSession: NSObject, @unchecked Sendable { private func readStickCalibration(user: UInt32, factory: UInt32, _ done: @escaping (Switch2.StickCalibration?) -> Void) { readMemory(length: 0x0B, address: user) { [weak self] data in - guard let self else { return } + guard let self, !self.ended else { return } if let data, !Switch2.StickCalibration.isBlank(data) { done(Switch2.StickCalibration(data: data)) return @@ -274,7 +298,7 @@ final class ControllerSession: NSObject, @unchecked Sendable { private func writeCommand(_ command: UInt8, _ subcommand: UInt8, _ data: Data, flag: UInt8 = 0x01, completion: @escaping (Data?) -> Void) { - guard let writeChar = chars[Switch2.GATT.commandWrite] else { + guard !ended, let writeChar = chars[Switch2.GATT.commandWrite] else { completion(nil); return } guard pendingCommand == nil else { @@ -299,7 +323,7 @@ final class ControllerSession: NSObject, @unchecked Sendable { private func handleCommandResponse(_ data: Data) { // An unrelated notification must not consume the command or its timer. - guard let pending = pendingCommand, data.count >= 8, + guard !ended, let pending = pendingCommand, data.count >= 8, data[data.startIndex] == pending.id, data[data.startIndex + 1] == 0x01 || data[data.startIndex + 1] == 0x02 else { return } commandTimeout?.cancel() @@ -349,7 +373,7 @@ final class ControllerSession: NSObject, @unchecked Sendable { /// bypasses persisted patterns. Restores normal LEDs when `nil`. func setRawLEDs(_ pattern: UInt8?) { queue.async { [weak self] in - guard let self else { return } + guard let self, !self.ended else { return } if let pattern { self.writeCommand(Switch2.Command.leds, Switch2.Subcommand.ledsSetPlayer, Data([pattern, 0, 0, 0])) { _ in } @@ -367,7 +391,10 @@ final class ControllerSession: NSObject, @unchecked Sendable { /// Read the current RSSI; result arrives via the rssi callback. var onRSSI: ((Int) -> Void)? func requestRSSI() { - queue.async { [weak self] in self?.peripheral.readRSSI() } + queue.async { [weak self] in + guard let self, !self.ended else { return } + self.peripheral.readRSSI() + } } // MARK: - Experiments (NFC probing, audio capture) @@ -392,7 +419,7 @@ final class ControllerSession: NSObject, @unchecked Sendable { private(set) var promiscuousNotify = false func setPromiscuousNotify(_ enabled: Bool) { queue.async { [weak self] in - guard let self else { return } + guard let self, !self.ended else { return } self.promiscuousNotify = enabled let known: Set = [Switch2.GATT.inputReport, Switch2.GATT.commandResponse, @@ -414,7 +441,7 @@ final class ControllerSession: NSObject, @unchecked Sendable { /// queue only). Returns false when the characteristic is absent. @discardableResult func writeAudioFrame(_ data: Data) -> Bool { - guard let ch = chars[Self.audioOutputUUID] else { return false } + guard !ended, let ch = chars[Self.audioOutputUUID] else { return false } peripheral.writeValue(data, for: ch, type: .withoutResponse) lastWriteAt = CFAbsoluteTimeGetCurrent() return true @@ -463,7 +490,7 @@ final class ControllerSession: NSObject, @unchecked Sendable { private(set) var audioExperimentName: String? func beginAudioExperiment(_ name: String) -> Bool { dispatchPrecondition(condition: .onQueue(queue)) - guard audioExperimentName == nil else { return false } + guard !ended, audioExperimentName == nil else { return false } audioExperimentName = name return true } @@ -487,7 +514,7 @@ final class ControllerSession: NSObject, @unchecked Sendable { next: @escaping () -> Data?, done: @escaping (AudioStreamStats) -> Void) { queue.async { [weak self] in - guard let self else { return } + guard let self, !self.ended else { return } self.audioStreamTimer?.cancel() self.audioStreamQueue.removeAll() self.audioStreamStats = AudioStreamStats(chunkLimit: self.audioWriteChunkLimit) @@ -533,7 +560,7 @@ final class ControllerSession: NSObject, @unchecked Sendable { /// Write queued chunks until the stack refuses; `peripheralIsReady` /// re-enters. Runs on the Bluetooth queue only. fileprivate func drainAudioStream() { - guard audioStreamNext != nil, let ch = chars[Self.audioOutputUUID] else { return } + guard !ended, audioStreamNext != nil, let ch = chars[Self.audioOutputUUID] else { return } while !audioStreamQueue.isEmpty { guard peripheral.canSendWriteWithoutResponse else { audioStreamStats.stalls += 1 @@ -570,7 +597,7 @@ final class ControllerSession: NSObject, @unchecked Sendable { /// Returns false via completion when the firmware doesn't expose it. func setAudioCapture(_ enabled: Bool, completion: @escaping (Bool) -> Void) { queue.async { [weak self] in - guard let self, let ch = self.chars[Self.audioInputUUID] else { + guard let self, !self.ended, let ch = self.chars[Self.audioInputUUID] else { completion(false); return } self.peripheral.setNotifyValue(enabled, for: ch) completion(true) @@ -581,13 +608,14 @@ final class ControllerSession: NSObject, @unchecked Sendable { func setRumble(strong: Double, weak: Double) { queue.async { [weak self] in - guard let self else { return } + guard let self, !self.ended else { return } self.rumbleTarget = (strong, weak) self.rumbleSetAt = CFAbsoluteTimeGetCurrent() } } private func startKeepAlive() { + guard !ended, keepAliveTimer == nil else { return } let timer = DispatchSource.makeTimerSource(queue: queue) timer.schedule(deadline: .now() + 0.05, repeating: 0.05) timer.setEventHandler { [weak self] in self?.maintainTick() } @@ -596,6 +624,7 @@ final class ControllerSession: NSObject, @unchecked Sendable { } private func maintainTick() { + guard !ended else { return } let now = CFAbsoluteTimeGetCurrent() var (strong, weakMag) = rumbleTarget // Failsafe: rumble intents expire after 0.5 s so a crashed consumer @@ -617,7 +646,7 @@ final class ControllerSession: NSObject, @unchecked Sendable { } private func writeMotor(_ vib: Switch2.Vibration) { - guard model.hasHDRumble, + guard !ended, model.hasHDRumble, let motor = chars[Switch2.GATT.vibration(for: model)] else { return } let packet = Switch2.motorPacket(vib, packetID: vibrationPacketID, model: model) vibrationPacketID &+= 1 @@ -638,7 +667,7 @@ final class ControllerSession: NSObject, @unchecked Sendable { } private func handleInputReport(_ data: Data) { - guard let report = Switch2.InputReport(data: data) else { return } + guard !ended, let report = Switch2.InputReport(data: data) else { return } let now = CFAbsoluteTimeGetCurrent() if lastReportAt > 0, now - lastReportAt > 0.100 { gapCount += 1 @@ -694,6 +723,7 @@ final class ControllerSession: NSObject, @unchecked Sendable { } state = s + announceReady() onState?(slot, s) // Battery / rate refresh for the UI at ~1 Hz. @@ -712,6 +742,7 @@ final class ControllerSession: NSObject, @unchecked Sendable { extension ControllerSession: CBPeripheralDelegate { func peripheral(_ peripheral: CBPeripheral, didDiscoverServices error: Error?) { + guard !ended else { return } if let error { fail("service discovery: \(error.localizedDescription)"); return } for service in peripheral.services ?? [] { peripheral.discoverCharacteristics(nil, for: service) @@ -721,6 +752,7 @@ extension ControllerSession: CBPeripheralDelegate { func peripheral(_ peripheral: CBPeripheral, didDiscoverCharacteristicsFor service: CBService, error: Error?) { + guard !ended else { return } if let error { fail("characteristic discovery: \(error.localizedDescription)"); return } for ch in service.characteristics ?? [] { if let uuid = UUID(uuidString: ch.uuid.uuidString) { @@ -741,6 +773,7 @@ extension ControllerSession: CBPeripheralDelegate { func peripheral(_ peripheral: CBPeripheral, didUpdateNotificationStateFor characteristic: CBCharacteristic, error: Error?) { + guard !ended else { return } let uuid = UUID(uuidString: characteristic.uuid.uuidString) if let error { // Only the essential channels are fatal — an experimental @@ -752,11 +785,15 @@ extension ControllerSession: CBPeripheralDelegate { } return } + if uuid == Switch2.GATT.commandResponse || uuid == Switch2.GATT.inputReport { + guard characteristic.isNotifying else { fail("essential notifications stopped"); return } + } if uuid == Switch2.GATT.commandResponse { runHandshake() } else if uuid == Switch2.GATT.inputReport { - notifyCompletion?(true) + let completion = notifyCompletion notifyCompletion = nil + completion?(true) } } @@ -772,7 +809,7 @@ extension ControllerSession: CBPeripheralDelegate { func peripheral(_ peripheral: CBPeripheral, didUpdateValueFor characteristic: CBCharacteristic, error: Error?) { - guard error == nil, let data = characteristic.value else { return } + guard !ended, error == nil, let data = characteristic.value else { return } let uuid = UUID(uuidString: characteristic.uuid.uuidString) if uuid == Switch2.GATT.inputReport { handleInputReport(data) diff --git a/docs/session-retirement.md b/docs/session-retirement.md new file mode 100644 index 0000000..f996f95 --- /dev/null +++ b/docs/session-retirement.md @@ -0,0 +1,30 @@ +# Session retirement and usable readiness + +A controller session now has an explicit terminal state. Teardown marks it +ended before clearing pending notifications, callbacks, handshake steps and +timers. Late CoreBluetooth notifications, queued commands/rumble and audio +work cannot restart or emit input from that retired session. Essential +notification-state callbacks must actually report isNotifying. Completion +storage is cleared before invocation so a reentrant callback cannot erase new +work. + +Readiness now requires both the existing handshake and a report accepted by +the existing decoder. No report lengths, controller bytes, pairing sequence, +GATT identifiers or connection timeouts are changed. The normal keep-alive +starts when the handshake completes even while awaiting input. If input +arrived during the handshake, its latest state is delivered when readiness is +announced. Readiness is announced once, before delivering the first usable +state, allowing the engine to attach output without losing that state. + +Eleven production-session tests pass with fake CoreBluetooth boundaries. Five +new selected failure cases were first run against the prior PR #3 source and +failed: retired notifications, retired input/commands, false readiness, +disabled notifications, and reentrant completion. Additional tests check early +input and stopped keep-alives. The complete app is compiled separately with +Apple SDKs; these fixtures are synthetic, not controller captures. + +Run bash tests/session/run.sh. Physical pairing/reconnect and keep-alive +behavior still need acceptance on the target firmware. Engine-level slot +replacement and delayed player-number callbacks remain separate ownership +boundaries; this session-local guard is not a claim that every lifecycle race +in the application has been eliminated. diff --git a/tests/session/SessionTests.swift b/tests/session/SessionTests.swift index d94aa53..a6b1805 100644 --- a/tests/session/SessionTests.swift +++ b/tests/session/SessionTests.swift @@ -8,6 +8,13 @@ final class Delegate: ControllerSessionDelegate { func sessionDidUpdateState(_ session: ControllerSession) {} } +final class StateCounter: @unchecked Sendable { + private let lock = NSLock() + private var count = 0 + func increment() { lock.lock(); defer { lock.unlock() }; count += 1 } + var value: Int { lock.lock(); defer { lock.unlock() }; return count } +} + @main enum SessionTests { static func fixture() -> (ControllerSession, CBPeripheral, DispatchQueue, Delegate) { @@ -24,6 +31,71 @@ enum SessionTests { func run(_ name: String, _ test: () -> Void) { if selected == "all" || selected == name { test(); print("PASS \(name)") } } + run("retired-notification") { + let (s, _, _, d) = fixture(); defer { _ = d } + var calls = 0 + s.notifyCompletion = { _ in calls += 1 } + s.teardown() + let input = s.chars[Switch2.GATT.inputReport]! + input.isNotifying = true + s.peripheral(s.peripheral, didUpdateNotificationStateFor: input, error: nil) + precondition(calls == 0, "A retired subscription callback resumed its handshake") + } + run("retired-input-command") { + let (s, p, q, d) = fixture(); defer { _ = d } + let states = StateCounter() + s.onState = { _, _ in states.increment() } + s.teardown() + s.handleInputReport(Data(repeating: 0, count: 63)) + s.experimentalCommand(9, 7, payload: Data()) { _ in } + q.sync {} + precondition(states.value == 0 && p.writes.isEmpty, "Retired input/commands must not reach outputs") + } + run("ready-needs-input") { + let (s, _, _, d) = fixture(); defer { s.teardown() } + s.advanceHandshake() + precondition(d.ready == 0, "Subscription/handshake without a valid input report is not ready") + s.handleInputReport(Data(repeating: 0, count: 10)) + precondition(d.ready == 0) + s.handleInputReport(Data(repeating: 0, count: 63)) + precondition(d.ready == 1) + s.handleInputReport(Data(repeating: 0, count: 63)) + precondition(d.ready == 1) + } + run("early-report-is-delivered") { + let (s, _, _, d) = fixture(); defer { s.teardown() } + s.handleInputReport(Data(repeating: 0, count: 63)) + precondition(d.ready == 0) + let states = StateCounter() + s.onState = { _, _ in states.increment() } + s.advanceHandshake() + precondition(d.ready == 1 && states.value == 1, "Input received during handshake must not be lost") + } + run("retired-keepalive") { + let (s, p, q, d) = fixture(); defer { _ = d } + q.sync { + s.advanceHandshake() + s.teardown() + s.maintainTick() + s.begin() + precondition(s.keepAliveTimer == nil && p.writes.isEmpty) + } + } + run("notification-disabled") { + let (s, _, _, d) = fixture(); defer { s.teardown() } + let input = s.chars[Switch2.GATT.inputReport]! + input.isNotifying = false + s.peripheral(s.peripheral, didUpdateNotificationStateFor: input, error: nil) + precondition(d.failures == 1, "Disabled essential notifications are not success") + } + run("notification-completion-reentrancy") { + let (s, _, _, d) = fixture(); defer { s.teardown(); _ = d } + let input = s.chars[Switch2.GATT.inputReport]! + input.isNotifying = true + s.notifyCompletion = { _ in s.notifyCompletion = { _ in } } + s.peripheral(s.peripheral, didUpdateNotificationStateFor: input, error: nil) + precondition(s.notifyCompletion != nil, "A completed callback erased replacement work") + } run("rumble") { for model in Switch2.Model.allCases { let (s, p, q, d) = fixture(); defer { s.teardown(); _ = d }