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)) + } +}