Merge pull request #89 from kmatzen/fix/sleep-visibility

Make sleep tracking work, and suppress only auto-reconnect rather than discovery

Author: kmatzen

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 {

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<UUID> = []
+
     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,19 +239,21 @@ 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
     func removeDevice(_ uuid: UUID) {
         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()

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")

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