From 2b4da7fe3d80e665f918697625d09113928111ad Mon Sep 17 00:00:00 2001 From: Kevin Blackburn-Matzen Date: Sat, 18 Jul 2026 20:43:20 -0700 Subject: [PATCH] Make sleep tracking work, and suppress only auto-reconnect rather than discovery MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit setDeviceSleeping wrote through an optional chain into discoveredDevices and connectedDevices. Both are only ever populated by addDiscoveredDevice, which nothing in the codebase calls, so both are permanently empty: the setter was a silent no-op and isDeviceSleeping always returned false. Sleep was therefore never suppressing anything, and the app would immediately re-discover and reconnect a camera the user had just put to sleep. Simply recording the flag is not enough, and neither is any refinement of it. Suppressing discovery for a camera believed asleep makes it permanently unreachable: it is withheld from discoveredGoPros both on disconnect and on every advertisement, and connectToGoPro requires it to be there, so the user has no way to reconnect. Adding a timed grace window does not fix this — a camera still advertising when the window closes gets re-discovered mid-shutdown, undoing the sleep command. Gating on observed silence instead fails the other way: a camera woken before the app notices silence stays invisible forever. The ambiguity is fundamental. A camera still shutting down and a camera that just woke up emit identical advertisements, so no rule over advertisements plus elapsed time can separate them. The guard conflated two concerns — do not auto-reconnect a camera the user slept, and do not show it — and only the first was ever required. Discovery is now never suppressed, so the camera stays visible and manually connectable; the sleep flag gates scheduleReconnectIfNeeded and the straggler retries instead. That needs no assumption about how long shutdown takes. Sleep state now lives in its own dictionary rather than hanging off the device records that are never populated, so it cannot silently no-op again. The same applies to power-down tracking, which was inert for the identical reason. Co-Authored-By: Claude Opus 4.8 Claude-Session: https://claude.ai/code/session_01AqfipPBp3eErDgZboooqxP --- Facett/BLEConnectionHandler.swift | 18 +++--- Facett/BLEDeviceStateManager.swift | 43 +++++++++++++- Facett/BLEManager.swift | 30 ++++++++-- FacettTests/StateMachineTests.swift | 89 +++++++++++++++++++++++++++++ 4 files changed, 162 insertions(+), 18 deletions(-) diff --git a/Facett/BLEConnectionHandler.swift b/Facett/BLEConnectionHandler.swift index ee592bf..9ffb01b 100644 --- a/Facett/BLEConnectionHandler.swift +++ b/Facett/BLEConnectionHandler.swift @@ -32,6 +32,8 @@ class BLEConnectionHandler { // Clear retry status on successful connection on main thread DispatchQueue.main.async { bleManager.connectionRetryStatus.removeValue(forKey: uuid) + // A camera we just connected to is awake, whatever we last told it. + bleManager.setDeviceSleeping(uuid, isSleeping: false) } // UI updates must happen on main thread @@ -71,20 +73,16 @@ class BLEConnectionHandler { let isSleeping = bleManager.isDeviceSleeping(uuid) let wasConnected = bleManager.connectedGoPros[uuid] != nil + // A sleeping camera stays on the discovered list so the user can still + // see and tap it. Withholding it here made it unreachable, because + // connectToGoPro requires the camera to be in discoveredGoPros. The + // sleep flag suppresses automatic reconnection instead, below. if let gopro = bleManager.connectedGoPros[uuid] { bleManager.connectedGoPros.removeValue(forKey: uuid) - if !isSleeping { - bleManager.discoveredGoPros[uuid] = gopro - } else { - ErrorHandler.debug("\(cameraName) is sleeping - not moving to discovered list") - } + bleManager.discoveredGoPros[uuid] = gopro } else if let gopro = bleManager.connectingGoPros[uuid] { bleManager.connectingGoPros.removeValue(forKey: uuid) - if !isSleeping { - bleManager.discoveredGoPros[uuid] = gopro - } else { - ErrorHandler.debug("\(cameraName) is sleeping - not moving to discovered list") - } + bleManager.discoveredGoPros[uuid] = gopro } if bleManager.connectedGoPros.isEmpty { diff --git a/Facett/BLEDeviceStateManager.swift b/Facett/BLEDeviceStateManager.swift index 4ad7203..e299c6c 100644 --- a/Facett/BLEDeviceStateManager.swift +++ b/Facett/BLEDeviceStateManager.swift @@ -36,6 +36,11 @@ class BLEDeviceStateManager: ObservableObject { @Published var connectingDevices: [UUID: DeviceState] = [:] @Published var connectionRetryStatus: [UUID: ConnectionRetryStatus] = [:] + /// Devices deliberately put to sleep, and when the sleep command was sent. + /// Kept independent of the device dictionaries above, which are never populated. + @Published private(set) var sleepingDevices: [UUID: Date] = [:] + @Published private(set) var poweringDownDevices: Set = [] + private var connectionRetryCount: [UUID: Int] = [:] private var connectionRetryTimers: [UUID: Timer] = [:] private var connectionAttemptTimers: [UUID: Timer] = [:] @@ -171,17 +176,47 @@ class BLEDeviceStateManager: ObservableObject { } /// Set device sleeping state + /// + /// Sleep state is held in `sleepingDevices` rather than on `DeviceState`. + /// The device dictionaries are only ever populated by `addDiscoveredDevice`, + /// which nothing calls, so both are permanently empty — writing sleep state + /// through an optional chain into them made every setter a silent no-op and + /// made `isDeviceSleeping` always return false. func setDeviceSleeping(_ uuid: UUID, isSleeping: Bool) { + if isSleeping { + sleepingDevices[uuid] = Date() + } else { + sleepingDevices.removeValue(forKey: uuid) + } discoveredDevices[uuid]?.isSleeping = isSleeping connectedDevices[uuid]?.isSleeping = isSleeping } /// Set device powering down state func setDevicePoweringDown(_ uuid: UUID, isPoweringDown: Bool) { + if isPoweringDown { + poweringDownDevices.insert(uuid) + } else { + poweringDownDevices.remove(uuid) + } discoveredDevices[uuid]?.isPoweringDown = isPoweringDown connectedDevices[uuid]?.isPoweringDown = isPoweringDown } + /// Whether automatic reconnection to this device is suppressed. + /// + /// This is the only thing the sleep flag gates. An earlier design also + /// suppressed *discovery* of a sleeping camera, which cannot be made correct: + /// a camera still shutting down and a camera that just woke up emit identical + /// advertisements, so any rule based on advertisements plus elapsed time + /// either undoes the user's sleep command (if it stops suppressing too early) + /// or hides the camera forever (if it never stops). Suppressing only automatic + /// reconnection avoids the ambiguity entirely: the camera stays visible and + /// manually connectable, and the app simply never reconnects on its own. + func isAutoReconnectSuppressed(for uuid: UUID) -> Bool { + return isDeviceSleeping(uuid) + } + /// Get device state func getDeviceState(for uuid: UUID) -> DeviceState? { return discoveredDevices[uuid] @@ -204,12 +239,12 @@ class BLEDeviceStateManager: ObservableObject { /// Check if device is sleeping func isDeviceSleeping(_ uuid: UUID) -> Bool { - return discoveredDevices[uuid]?.isSleeping ?? false + return sleepingDevices[uuid] != nil } /// Check if device is powering down func isDevicePoweringDown(_ uuid: UUID) -> Bool { - return discoveredDevices[uuid]?.isPoweringDown ?? false + return poweringDownDevices.contains(uuid) } /// Remove device from all collections @@ -217,6 +252,8 @@ class BLEDeviceStateManager: ObservableObject { discoveredDevices.removeValue(forKey: uuid) connectedDevices.removeValue(forKey: uuid) connectingDevices.removeValue(forKey: uuid) + sleepingDevices.removeValue(forKey: uuid) + poweringDownDevices.remove(uuid) connectionRetryStatus.removeValue(forKey: uuid) connectionRetryCount.removeValue(forKey: uuid) @@ -241,6 +278,8 @@ class BLEDeviceStateManager: ObservableObject { discoveredDevices.removeAll() connectedDevices.removeAll() connectingDevices.removeAll() + sleepingDevices.removeAll() + poweringDownDevices.removeAll() connectionRetryStatus.removeAll() connectionRetryCount.removeAll() connectionRetryTimers.removeAll() diff --git a/Facett/BLEManager.swift b/Facett/BLEManager.swift index 06096d2..9f644b5 100644 --- a/Facett/BLEManager.swift +++ b/Facett/BLEManager.swift @@ -745,11 +745,16 @@ class BLEManager: NSObject, ObservableObject, CBCentralManagerDelegate, CBPeriph let gopro = GoPro(peripheral: peripheral) guard connectedGoPros[peripheral.identifier] == nil else { return } - // Don't add sleeping devices to discovered list - guard !deviceStateManager.isDeviceSleeping(peripheral.identifier) else { - ErrorHandler.debug("Ignoring sleeping device: \(peripheral.name ?? peripheral.identifier.uuidString)") - return - } + // Advertisements are always honoured, including from a camera we believe + // is asleep. A camera still shutting down and a camera that just woke up + // emit identical advertisements, so no rule based on advertisements and + // elapsed time can tell them apart — suppressing discovery either undoes + // the user's sleep command or hides the camera permanently, since + // connectToGoPro requires it to be in discoveredGoPros. + // + // The sleep flag instead suppresses only AUTOMATIC reconnection, which is + // the behaviour that actually needed guarding. The camera stays visible + // and the user can always tap to connect. let peripheralId = peripheral.identifier let peripheralName = peripheral.name @@ -1899,6 +1904,11 @@ class BLEManager: NSObject, ObservableObject, CBCentralManagerDelegate, CBPeriph return deviceStateManager.isDeviceSleeping(uuid) } + /// Record whether a device is sleeping + func setDeviceSleeping(_ uuid: UUID, isSleeping: Bool) { + deviceStateManager.setDeviceSleeping(uuid, isSleeping: isSleeping) + } + // MARK: - Query Timer Management func startDeviceQueryTimer() { @@ -2220,9 +2230,12 @@ class BLEManager: NSObject, ObservableObject, CBCentralManagerDelegate, CBPeriph // Find cameras that should be connected but aren't let stragglers = targetConnectedCameras.filter { cameraId in // Camera should be connected if it's discovered but not connected and not currently connecting + // A camera the user asked to sleep is excluded: it stays visible and + // manually connectable, but must not be reconnected automatically. return discoveredGoPros[cameraId] != nil && connectedGoPros[cameraId] == nil && - connectingGoPros[cameraId] == nil + connectingGoPros[cameraId] == nil && + !deviceStateManager.isDeviceSleeping(cameraId) } if !stragglers.isEmpty { @@ -2265,6 +2278,11 @@ class BLEManager: NSObject, ObservableObject, CBCentralManagerDelegate, CBPeriph /// Schedule auto-reconnect for a camera that dropped unexpectedly func scheduleReconnectIfNeeded(for uuid: UUID) { guard targetConnectedCameras.contains(uuid) else { return } + // Never auto-reconnect a camera the user asked to sleep. + guard !deviceStateManager.isDeviceSleeping(uuid) else { + ErrorHandler.debug("Not scheduling reconnect - camera was put to sleep") + return + } let cameraName = CameraIdentityManager.shared.getDisplayName(for: uuid) ErrorHandler.info("Camera \(cameraName) dropped - scheduling reconnect") diff --git a/FacettTests/StateMachineTests.swift b/FacettTests/StateMachineTests.swift index 52b0725..8224814 100644 --- a/FacettTests/StateMachineTests.swift +++ b/FacettTests/StateMachineTests.swift @@ -358,3 +358,92 @@ class StateMachineTests: XCTestCase { return BLEManager() } } + +// MARK: - Device Sleep State + +final class DeviceSleepStateTests: XCTestCase { + + var stateManager: BLEDeviceStateManager! + + override func setUp() { + super.setUp() + stateManager = BLEDeviceStateManager() + } + + override func tearDown() { + stateManager = nil + super.tearDown() + } + + func testSleepStateIsRecorded() { + let cam = UUID() + XCTAssertFalse(stateManager.isDeviceSleeping(cam)) + + stateManager.setDeviceSleeping(cam, isSleeping: true) + + // This previously wrote through an optional chain into a dictionary that + // nothing ever populates, so the setter was a silent no-op and the getter + // always returned false. + XCTAssertTrue(stateManager.isDeviceSleeping(cam)) + } + + func testAutoReconnectSuppressedWhileSleeping() { + let cam = UUID() + XCTAssertFalse(stateManager.isAutoReconnectSuppressed(for: cam)) + + stateManager.setDeviceSleeping(cam, isSleeping: true) + + // The sleep flag gates automatic reconnection and nothing else. It must + // not gate discovery: a camera still shutting down and one that just woke + // emit identical advertisements, so suppressing discovery either undoes + // the sleep command or hides the camera permanently. + XCTAssertTrue(stateManager.isAutoReconnectSuppressed(for: cam)) + } + + func testAutoReconnectResumesAfterWake() { + let cam = UUID() + stateManager.setDeviceSleeping(cam, isSleeping: true) + stateManager.setDeviceSleeping(cam, isSleeping: false) + + XCTAssertFalse(stateManager.isAutoReconnectSuppressed(for: cam)) + } + + func testExplicitWakeClearsSleepState() { + let cam = UUID() + stateManager.setDeviceSleeping(cam, isSleeping: true) + stateManager.setDeviceSleeping(cam, isSleeping: false) + XCTAssertFalse(stateManager.isDeviceSleeping(cam)) + } + + func testSleepStateIsPerDevice() { + let sleeping = UUID() + let awake = UUID() + stateManager.setDeviceSleeping(sleeping, isSleeping: true) + + XCTAssertFalse(stateManager.isDeviceSleeping(awake)) + XCTAssertFalse(stateManager.isAutoReconnectSuppressed(for: awake)) + } + + func testPowerDownStateIsRecorded() { + let cam = UUID() + XCTAssertFalse(stateManager.isDevicePoweringDown(cam)) + + stateManager.setDevicePoweringDown(cam, isPoweringDown: true) + XCTAssertTrue(stateManager.isDevicePoweringDown(cam)) + + stateManager.setDevicePoweringDown(cam, isPoweringDown: false) + XCTAssertFalse(stateManager.isDevicePoweringDown(cam)) + } + + func testCleanupClearsSleepState() { + let cam = UUID() + + stateManager.setDeviceSleeping(cam, isSleeping: true) + stateManager.removeDevice(cam) + XCTAssertFalse(stateManager.isDeviceSleeping(cam)) + + stateManager.setDeviceSleeping(cam, isSleeping: true) + stateManager.clearAllDevices() + XCTAssertFalse(stateManager.isDeviceSleeping(cam)) + } +}