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 }