diff --git a/Magic Switch/Model/Store/BluetoothPeripheralStore.swift b/Magic Switch/Model/Store/BluetoothPeripheralStore.swift index 3e0b80f..86d7f09 100644 --- a/Magic Switch/Model/Store/BluetoothPeripheralStore.swift +++ b/Magic Switch/Model/Store/BluetoothPeripheralStore.swift @@ -13,7 +13,7 @@ protocol BluetoothPeripheralManageable { /// Initiates connection to a peripheral func connectPeripheral(_ peripheral: BluetoothPeripheral) - /// Initiates takeover from the peer Mac, refreshing stale local pairing first + /// Initiates takeover from the peer Mac, re-pairing only if the bonded connect is refused func connectPeripheralFromPeer(_ peripheral: BluetoothPeripheral) /// Disconnects from a peripheral @@ -40,6 +40,9 @@ final class BluetoothPeripheralStore: NSObject, ObservableObject, BluetoothPerip /// hosts) can legitimately take 30-45s, and a false-positive timeout /// is worse than waiting a beat longer. static let pairTimeout: TimeInterval = 60 + /// How long to let `-remove` settle in the Bluetooth daemon before pairing + /// again. Re-pairing inside this window races the unbond and fails. + static let unbondSettle: TimeInterval = 0.5 /// How long after wake to wait before deciding whether the peer holds a /// peripheral we released for sleep. Gives Wi-Fi time to reassociate so a /// peer that's actively using the peripheral doesn't look unreachable and @@ -140,6 +143,14 @@ final class BluetoothPeripheralStore: NSObject, ObservableObject, BluetoothPerip @AppStorage(BluetoothPeripheralStore.autoReconnectDefaultsKey) private var autoReconnect: Bool = true + /// The same setting read off the main thread, where the `@AppStorage` + /// wrapper isn't safe to touch. `@AppStorage` stores through + /// `UserDefaults.standard` under the same key, so an absent value means the + /// user has never toggled it and the wrapper's own default applies. + private var autoReconnectIsOn: Bool { + UserDefaults.standard.object(forKey: Self.autoReconnectDefaultsKey) as? Bool ?? true + } + @Published private(set) var peripherals: [BluetoothPeripheral] = [] { didSet { savePeripherals() @@ -203,6 +214,13 @@ final class BluetoothPeripheralStore: NSObject, ObservableObject, BluetoothPerip /// where that attempt has minted but not yet installed its pair. Main-only. private var pendingPairAttempts: [String: UInt64] = [:] + /// Addresses whose local bond an escalation removed and which haven't + /// reached `.connected` since. A failure while an address is in here means + /// the pairing was torn down and not restored — the one outcome no retry + /// cadence can undo for the user, so it's reported even on the paths that + /// otherwise stay silent. Main-only. + private var bondsAwaitingRepair: Set = [] + /// Disconnect notification observers, keyed by peripheral id. private var disconnectObservers: [String: IOBluetoothUserNotification] = [:] @@ -289,13 +307,22 @@ final class BluetoothPeripheralStore: NSObject, ObservableObject, BluetoothPerip /// 60s pair watchdog — when a peer command starts a fresh attempt). Failure /// paths carry their attempt's token and no-op once it's stale, so the old /// attempt's late death can't cancel the new attempt's watchdog, consume - /// its announce flag, or fail its waiters. Main-only. + /// its announce flag, or fail its waiters. Guarded by `attemptTokenLock` + /// (not main-only) so the Bluetooth queue's preflight can re-check it + /// mid-attempt — a cancel must be able to stop a blocked attempt's + /// destructive steps before they run. private var connectAttemptTokens: [String: UInt64] = [:] - /// Backing counter for `connectAttemptTokens`; lock-guarded so attempts - /// can be minted from any thread. + /// Backing counter for `connectAttemptTokens`, under the same lock. private var connectAttemptCounter: UInt64 = 0 private let attemptTokenLock = NSLock() + /// Ids whose take has the peer release round trip still in flight. A + /// cancel inside that window is refused: the UNREGISTER can't be un-sent, + /// and abandoning its success would strand the peripheral — released by + /// the peer, claimed by no one, with the watcher already stood down. + /// Main-only. + private var takeReleasesInFlight: Set = [] + // MARK: - Computed Properties var availablePeripherals: [BluetoothPeripheral] { @@ -605,7 +632,6 @@ final class BluetoothPeripheralStore: NSObject, ObservableObject, BluetoothPerip connectPeripheral( peripheral, announcePairTimeout: false, - refreshPairingBeforeConnect: false, skipRangeCheck: true, completion: nil ) @@ -624,7 +650,6 @@ final class BluetoothPeripheralStore: NSObject, ObservableObject, BluetoothPerip self.connectPeripheral( peripheral, announcePairTimeout: false, - refreshPairingBeforeConnect: false, skipRangeCheck: true, completion: nil ) @@ -810,6 +835,27 @@ final class BluetoothPeripheralStore: NSObject, ObservableObject, BluetoothPerip } } + /// Aborts the in-flight connect for `peripheral` — the dropdown's + /// "Pairing…" row routes its click here. Superseding the attempt token + /// orphans every path still in flight (the Bluetooth-queue preflight + /// re-checks it before its destructive steps), and the watcher is stood + /// down so a retry doesn't repaint "Pairing…" seconds later. Refused while + /// a take's release round trip is on the wire — see `takeReleasesInFlight`. + func cancelConnect(_ peripheral: BluetoothPeripheral) { + guard Thread.isMainThread else { + DispatchQueue.main.async { [weak self] in self?.cancelConnect(peripheral) } + return + } + let id = peripheral.id + guard connectionState(for: id) == .connecting, + !takeReleasesInFlight.contains(id) + else { return } + _ = beginConnectAttempt(for: id) + tearDownPairAttempt(for: id) + disarmReconnect(id) + setConnectionState(.disconnected, for: id) + } + /// Asks the peer to release just this peripheral, then pairs it /// locally. Used by the Peripheral tab's "Connect to PC" button and by /// the right-click menu's per-peripheral switch. Apple's Magic devices @@ -843,43 +889,52 @@ final class BluetoothPeripheralStore: NSObject, ObservableObject, BluetoothPerip // below re-arms it under its own attempt token. let attempt = beginConnectAttempt(for: peripheral.id) schedulePairWatchdog(for: peripheral, announceTimeout: true, attempt: attempt) + takeReleasesInFlight.insert(peripheral.id) networkStore.executeUnregisterOne(address: peripheral.id, on: device) { [weak self] result in - guard let self = self else { return } - switch result { - case .success: - // Peer released it; grab it locally. Arm the watcher too, so a local - // connect that fails (e.g. the device is in the stuck state and needs - // a power-cycle) keeps retrying instead of leaving it on neither Mac. - // It self-disarms once we're connected. - self.connectPeripheralFromPeer(peripheral) - self.armReconnect(peripheral.id) - case .failure(.connectionFailed), .failure(.connectTimeout): - // We never got a TCP connection up, so the peer's machine is - // unreachable (asleep, off the network, app not running) and isn't - // holding the peripheral anymore — a Mac that drops off the network - // has already released its Bluetooth devices. Pair locally instead - // of stranding the user with an error they can't act on, and arm the - // watcher as the same retry safety net. We deliberately don't grab on - // post-connect failures (next case): if the connection opened, the - // peer's machine is awake and may still actively hold the peripheral. - self.connectPeripheralFromPeer(peripheral) - self.armReconnect(peripheral.id) - case .failure(let err): - // Reachable peer but the release errored, so we can't be sure it let - // go. Don't grab it outright (that could yank it from a peer that did - // take it); arm the HOLDS_ONE-gated watcher, which reclaims it only - // once the peer confirms it isn't holding it — and recovers the case - // where the peer released but the ack was lost. - self.setConnectionState(.disconnected, for: peripheral.id) - self.armReconnect(peripheral.id) - self.setPeripheralError("Switch failed.", for: peripheral.id) - NotificationManager.showNotification( - title: "Couldn't Switch", - body: - "Couldn't ask \(device.name) to release \(peripheral.name): \(err.userMessage)", - identifier: "take-failed-\(peripheral.id)" - ) + // Fires on the connection queue; hop to main for the watcher/state + // work below. A peer command can supersede the attempt mid-flight, and + // a superseded release's outcome must not restart the connect or arm + // the watcher. + DispatchQueue.main.async { + guard let self = self else { return } + self.takeReleasesInFlight.remove(peripheral.id) + guard self.isCurrentAttempt(attempt, for: peripheral.id) else { return } + switch result { + case .success: + // Peer released it; grab it locally. Arm the watcher too, so a local + // connect that fails (e.g. the device is in the stuck state and needs + // a power-cycle) keeps retrying instead of leaving it on neither Mac. + // It self-disarms once we're connected. + self.connectPeripheralFromPeer(peripheral) + self.armReconnect(peripheral.id) + case .failure(.connectionFailed), .failure(.connectTimeout): + // We never got a TCP connection up, so the peer's machine is + // unreachable (asleep, off the network, app not running) and isn't + // holding the peripheral anymore — a Mac that drops off the network + // has already released its Bluetooth devices. Pair locally instead + // of stranding the user with an error they can't act on, and arm the + // watcher as the same retry safety net. We deliberately don't grab on + // post-connect failures (next case): if the connection opened, the + // peer's machine is awake and may still actively hold the peripheral. + self.connectPeripheralFromPeer(peripheral) + self.armReconnect(peripheral.id) + case .failure(let err): + // Reachable peer but the release errored, so we can't be sure it let + // go. Don't grab it outright (that could yank it from a peer that did + // take it); arm the HOLDS_ONE-gated watcher, which reclaims it only + // once the peer confirms it isn't holding it — and recovers the case + // where the peer released but the ack was lost. + self.setConnectionState(.disconnected, for: peripheral.id) + self.armReconnect(peripheral.id) + self.setPeripheralError("Switch failed.", for: peripheral.id) + NotificationManager.showNotification( + title: "Couldn't Switch", + body: + "Couldn't ask \(device.name) to release \(peripheral.name): \(err.userMessage)", + identifier: "take-failed-\(peripheral.id)" + ) + } } } } @@ -1074,7 +1129,6 @@ final class BluetoothPeripheralStore: NSObject, ObservableObject, BluetoothPerip connectPeripheral( peripheral, announcePairTimeout: true, - refreshPairingBeforeConnect: false, refreshStaleBondOnFailedOpen: true, completion: nil ) @@ -1091,7 +1145,8 @@ final class BluetoothPeripheralStore: NSObject, ObservableObject, BluetoothPerip connectPeripheral( peripheral, announcePairTimeout: true, - refreshPairingBeforeConnect: true, + refreshStaleBondOnFailedOpen: true, + skipRangeCheck: true, completion: completion ) } @@ -1102,32 +1157,40 @@ final class BluetoothPeripheralStore: NSObject, ObservableObject, BluetoothPerip /// callers pass `true`; the auto-reconnect watcher passes `false` so its /// retries against a stuck device don't spam notifications or strobe the /// inline row error. - /// - Parameter refreshPairingBeforeConnect: whether to remove a stale local - /// pairing record before pairing. Use this only while taking a peripheral - /// from the peer: Magic peripherals can sit at `paired=true` but refuse - /// `openConnection()` until the target Mac re-pairs. /// - Parameter refreshStaleBondOnFailedOpen: whether a bonded device that - /// refuses `openConnection()` while the RSSI probe can still see it may - /// have its local pairing record removed and re-paired within the same - /// attempt. That combination — alive and in range, yet refusing the - /// bonded connect — is the stale-bond signature: the local record says - /// `paired=true` but the device actually answers to the other Mac (a - /// handoff outside the app, or desynced state). Only interactive local - /// connects pass `true`; the background watcher/reclaim paths keep - /// retrying the plain open instead, so a transient link failure in a - /// retry loop can't repeatedly tear bonds down. + /// refuses `openConnection()` may have its local pairing record removed + /// and re-paired within the same attempt. A record that says + /// `paired=true` while the device refuses the bonded connect is the + /// stale-bond signature: the device actually answers to the other Mac + /// (a handoff outside the app, or desynced state). Interactive local + /// connects and adoption grabs additionally require the RSSI probe to + /// still see the device — a healthy bond whose device is merely off or + /// out of range must survive, or the automatic reconnect macOS performs + /// when it returns is lost. Takeover connects (`skipRangeCheck: true`) + /// escalate without the probe: the peer just released the device or + /// vanished, so a bond that still refuses the open is stale by + /// construction. That blind escalation is conditional on auto-reconnect + /// being on — its retries, given a fresh window by + /// `armReconnectForBondRepair`, are what make a wrong guess recoverable, + /// and with the setting off nothing would re-pair the device at all. The + /// watcher's reclaim retries pass `false` and keep retrying the plain + /// open, so a transient link failure in a retry loop can't repeatedly + /// tear bonds down. /// - Parameter skipRangeCheck: start the pair even when the RSSI probe can't - /// see the device. A peripheral we unpaired for sleep that nothing adopted - /// is bonded to no Mac and invisible to the probe until the user touches + /// see the device. A peripheral the peer just released (a takeover) or + /// one we unpaired for sleep that nothing adopted (the wake reclaim) is + /// bonded to no Mac and invisible to the probe until the user touches /// or power-cycles it — but an in-flight `IOBluetoothDevicePair` pages - /// continuously, so a blind attempt catches that brief window where a - /// 5s-cadence probe misses it. A miss just rides the (silent) pair - /// watchdog into `.disconnected`. Only the wake-time direct reclaim passes - /// `true`; everything else keeps the cheap probe gate. + /// continuously, so a blind attempt catches the window a 5s-cadence + /// probe misses, including a release that lands moments after the peer + /// acked it. A miss just rides the (silent) pair watchdog into + /// `.disconnected`. The watcher's retries keep the cheap probe gate. An + /// attempt that escalates to a bond refresh pairs blind whatever this + /// says: it has already established the device is reachable (or is a + /// takeover), and the bond it would fall back on is gone. private func connectPeripheral( _ peripheral: BluetoothPeripheral, announcePairTimeout: Bool, - refreshPairingBeforeConnect: Bool, refreshStaleBondOnFailedOpen: Bool = false, skipRangeCheck: Bool = false, completion: ((Bool) -> Void)? @@ -1142,7 +1205,7 @@ final class BluetoothPeripheralStore: NSObject, ObservableObject, BluetoothPerip bluetoothQueue.async { [weak self] in guard let self = self else { return } - guard var btDevice = IOBluetoothDevice(addressString: peripheral.id) else { + guard let btDevice = IOBluetoothDevice(addressString: peripheral.id) else { print("\(peripheral.name) not found") self.failConnectAttempt( id: peripheral.id, name: peripheral.name, @@ -1169,16 +1232,6 @@ final class BluetoothPeripheralStore: NSObject, ObservableObject, BluetoothPerip return } - if refreshPairingBeforeConnect, btDevice.isConnected() { - self.setConnectionState(.connected, for: peripheral.id) - self.registerForDisconnect(device: btDevice, address: peripheral.id) - return - } - - if refreshPairingBeforeConnect, btDevice.isPaired() { - btDevice = self.removeStaleBond(of: btDevice, id: peripheral.id, name: peripheral.name) - } - // Already bonded to this Mac. A peripheral we're holding that merely // dropped — power cycle, briefly out of range, wake — keeps its link // key, so macOS reconnects it on its own. Running @@ -1187,12 +1240,11 @@ final class BluetoothPeripheralStore: NSObject, ObservableObject, BluetoothPerip // strands the UI at "(Pairing…)" — the pair callback never fires for an // already-connected device, and `fetchConnectedPeripherals` won't // overwrite the in-flight `.connecting`). So adopt the live connection, - // or just open one — don't re-pair up front. For peer takeovers, a - // stale `paired=true connected=false` record is removed above so this - // branch does not mask the required re-pair; interactive connects can - // instead escalate to that same refresh below, but only after the plain - // open has failed against a device the probe can still see. - if !refreshPairingBeforeConnect, btDevice.isConnected() || btDevice.isPaired() { + // or just open one — never re-pair up front. The bonded open is the + // same cheap connect System Settings performs, and on a takeover it's + // what works the moment the peer's release has landed; only a device + // that refuses it escalates to the bond refresh below. + if btDevice.isConnected() || btDevice.isPaired() { var openResult = kIOReturnSuccess if !btDevice.isConnected() { openResult = btDevice.openConnection() @@ -1203,19 +1255,14 @@ final class BluetoothPeripheralStore: NSObject, ObservableObject, BluetoothPerip return } print("openConnection to bonded \(peripheral.name) failed: \(openResult)") - if refreshStaleBondOnFailedOpen, + // A cancel during the (blocking) open must not escalate into a bond + // teardown the stopped attempt can never pair back. + guard self.isCurrentAttempt(attempt, for: peripheral.id) else { return } + guard refreshStaleBondOnFailedOpen, btDevice.responds(to: Selector(("remove"))), - btDevice.rssi() != Constants.invalidRSSI - { - // Alive and in range, yet refusing the bonded connect — the - // stale-bond signature (see the parameter doc). Break the dead - // record and fall through to a fresh pair. The RSSI gate is what - // makes this safe to do unprompted: a healthy bond whose device is - // merely off or out of range doesn't answer the probe, and removing - // *that* bond would cost the automatic reconnect macOS performs - // when the device comes back. - btDevice = self.removeStaleBond(of: btDevice, id: peripheral.id, name: peripheral.name) - } else { + (skipRangeCheck && self.autoReconnectIsOn) + || btDevice.rssi() != Constants.invalidRSSI + else { // Bonded but didn't come up (still booting / out of range / link // failure). macOS or the watcher's next probe may still bring it // back, but an interactive Connect that lands here previously @@ -1231,84 +1278,169 @@ final class BluetoothPeripheralStore: NSObject, ObservableObject, BluetoothPerip ) return } - } - - if !skipRangeCheck, btDevice.rssi() == Constants.invalidRSSI { - print("\(peripheral.name) is out of range or not responding") - self.failConnectAttempt( - id: peripheral.id, name: peripheral.name, - inline: "Not responding.", - notifyTitle: "Couldn't Connect", - notifyBody: - "\(peripheral.name) isn't responding. It may be off, out of range, or connected to your other Mac.", - attempt: attempt - ) + // Refusing the bonded connect — the stale-bond signature (see the + // parameter doc). Break the dead record and re-pair from scratch. The + // RSSI gate is what makes this safe to do unprompted: a healthy bond + // whose device is merely off or out of range doesn't answer the probe, + // and removing *that* bond would cost the automatic reconnect macOS + // performs when the device comes back. Takeovers skip the gate — the + // device often stays silent until the peer's release lands, and the + // paging pair catches it — but only while auto-reconnect is on, since + // the watcher's retries are the whole reason a wrong guess here is + // survivable. With it off, an unanswered probe keeps its bond. + self.removeStaleBond( + of: btDevice, id: peripheral.id, name: peripheral.name + ) { refreshed in + // The settle yields the queue, so a cancel or a newer attempt can + // land in the gap. + guard self.isCurrentAttempt(attempt, for: peripheral.id) else { return } + // The bond is already gone; re-probing before the pair would only + // widen the window in which no host is claiming the device. + self.startDevicePair( + for: peripheral, device: refreshed, attempt: attempt, skipRangeCheck: true) + } return } - guard let devicePair = IOBluetoothDevicePair(device: btDevice) else { - print("Failed to initialize pairing for \(peripheral.name)") - self.failConnectAttempt( - id: peripheral.id, name: peripheral.name, - inline: "Pairing failed.", - notifyBody: - "Couldn't start pairing with \(peripheral.name). Turn it off and on, then try again.", - attempt: attempt - ) + self.startDevicePair( + for: peripheral, device: btDevice, attempt: attempt, skipRangeCheck: skipRangeCheck) + } + } + + /// Pairs `device` from scratch under `attempt` and installs the resulting + /// `IOBluetoothDevicePair`. Runs on `bluetoothQueue`; the success path + /// continues in `devicePairingFinished(_:error:)`. + private func startDevicePair( + for peripheral: BluetoothPeripheral, + device: IOBluetoothDevice, + attempt: UInt64, + skipRangeCheck: Bool + ) { + if !skipRangeCheck, device.rssi() == Constants.invalidRSSI { + print("\(peripheral.name) is out of range or not responding") + failConnectAttempt( + id: peripheral.id, name: peripheral.name, + inline: "Not responding.", + notifyTitle: "Couldn't Connect", + notifyBody: + "\(peripheral.name) isn't responding. It may be off, out of range, or connected to your other Mac.", + attempt: attempt + ) + return + } + + guard let devicePair = IOBluetoothDevicePair(device: device) else { + print("Failed to initialize pairing for \(peripheral.name)") + failConnectAttempt( + id: peripheral.id, name: peripheral.name, + inline: "Pairing failed.", + notifyBody: + "Couldn't start pairing with \(peripheral.name). Turn it off and on, then try again.", + attempt: attempt + ) + return + } + + devicePair.delegate = self + DispatchQueue.main.async { + // A cancel (or a newer attempt) can supersede this one while the + // preflight above runs; a pair installed after that would page on + // with no watchdog to stop it. + guard self.isCurrentAttempt(attempt, for: peripheral.id) else { + devicePair.stop() return } + self.pendingPairs[peripheral.id]?.stop() + self.pendingPairs[peripheral.id] = devicePair + self.pendingPairAttempts[peripheral.id] = attempt + } - devicePair.delegate = self + // Re-checked right before the start: the install guard above may run + // first and its stop() no-ops on a pair that hasn't started yet. + guard isCurrentAttempt(attempt, for: peripheral.id) else { return } + let pairResult = devicePair.start() + if pairResult != kIOReturnSuccess { + print("Failed to start pairing with \(peripheral.name). Error code: \(pairResult)") DispatchQueue.main.async { - self.pendingPairs[peripheral.id]?.stop() - self.pendingPairs[peripheral.id] = devicePair - self.pendingPairAttempts[peripheral.id] = attempt - } - - let pairResult = devicePair.start() - if pairResult != kIOReturnSuccess { - print("Failed to start pairing with \(peripheral.name). Error code: \(pairResult)") - DispatchQueue.main.async { - // Identity-guarded like the delegate: a newer attempt may already - // have replaced this entry. - if self.pendingPairs[peripheral.id] === devicePair { - self.pendingPairs.removeValue(forKey: peripheral.id) - self.pendingPairAttempts.removeValue(forKey: peripheral.id) - } + // Identity-guarded like the delegate: a newer attempt may already + // have replaced this entry. + if self.pendingPairs[peripheral.id] === devicePair { + self.pendingPairs.removeValue(forKey: peripheral.id) + self.pendingPairAttempts.removeValue(forKey: peripheral.id) } - self.failConnectAttempt( - id: peripheral.id, name: peripheral.name, - inline: "Pairing failed.", - notifyBody: - "Couldn't start pairing with \(peripheral.name) (error \(pairResult)). Turn it off and on, then try again.", - attempt: attempt - ) } - // Success path continues in `devicePairingFinished(_:error:)`. + failConnectAttempt( + id: peripheral.id, name: peripheral.name, + inline: "Pairing failed.", + notifyBody: + "Couldn't start pairing with \(peripheral.name) (error \(pairResult)). Turn it off and on, then try again.", + attempt: attempt + ) } } /// Removes a local pairing record judged stale so the caller can re-pair - /// from scratch. Runs on `bluetoothQueue` (it blocks in a settle sleep). - /// Returns a re-fetched device handle — the old one still reports the - /// removed bond. + /// from scratch, then hands a re-fetched device handle to `completion` back + /// on `bluetoothQueue` — the old handle still reports the removed bond. + /// Called on `bluetoothQueue`. + /// + /// `-remove` tears the bond down asynchronously in the Bluetooth daemon, so + /// the settle waits for it rather than polling (there's no condition to poll + /// — just "give the daemon a moment"). It waits by *yielding* the queue: + /// `bluetoothQueue` is serial and shared by every in-flight attempt, so + /// sleeping on it held every other peripheral's connect behind this one + /// peripheral's unbond — which is what made a multi-peripheral switch fail + /// by queue position, the later rows spending longest bonded to no Mac at + /// all. private func removeStaleBond( - of btDevice: IOBluetoothDevice, id: String, name: String - ) -> IOBluetoothDevice { + of btDevice: IOBluetoothDevice, id: String, name: String, + completion: @escaping (IOBluetoothDevice) -> Void + ) { guard btDevice.responds(to: Selector(("remove"))) else { print("Cannot refresh stale pairing for \(name): remove selector unavailable") - return btDevice + completion(btDevice) + return } btDevice.perform(Selector(("remove"))) print("Removed stale local pairing before taking \(name)") - // `-remove` tears the bond down asynchronously in the Bluetooth - // daemon; re-pairing before it settles can race the unbond and fail. - // A short fixed settle is simpler than a poll loop here (there's no - // condition to poll — just "give the daemon a moment"). We're on - // `bluetoothQueue`, a background serial queue, so this briefly stalls - // other queued BT work but never the main thread / UI. - Thread.sleep(forTimeInterval: 0.5) - return IOBluetoothDevice(addressString: id) ?? btDevice + noteBondAwaitingRepair(id) + armReconnectForBondRepair(id) + bluetoothQueue.asyncAfter(deadline: .now() + Constants.unbondSettle) { + completion(IOBluetoothDevice(addressString: id) ?? btDevice) + } + } + + /// Record that `id`'s local bond is gone until something re-pairs it, so a + /// failure can report the pairing as reset rather than merely failed. + private func noteBondAwaitingRepair(_ id: String) { + let apply: () -> Void = { [weak self] in self?.bondsAwaitingRepair.insert(id) } + if Thread.isMainThread { apply() } else { DispatchQueue.main.async(execute: apply) } + } + + /// Whether `id`'s bond was removed and never restored, clearing the record. + /// Main-only. + private func consumeBondAwaitingRepair(_ id: String) -> Bool { + bondsAwaitingRepair.remove(id) != nil + } + + /// Give `id` a full `reconnectMaxWindow` of watcher retries from now. + /// Removing a bond is a debt this Mac just took on, so the retries that pay + /// it can't inherit whatever was left of the original drop's window — + /// `armReconnect` deliberately preserves that earlier deadline, which on an + /// entry armed minutes ago leaves almost no time to re-pair. A peripheral + /// whose caller never armed the watcher gets armed here for the same reason. + /// Leaves an existing entry's reclaim/adoption flavour alone. + private func armReconnectForBondRepair(_ id: String) { + let apply: () -> Void = { [weak self] in + guard let self = self else { return } + guard self.reconnectWatchlist[id] != nil else { + self.armReconnect(id) + return + } + self.reconnectWatchlist[id] = Date() + print("Auto-reconnect: restarted the retry window for \(id)") + } + if Thread.isMainThread { apply() } else { DispatchQueue.main.async(execute: apply) } } /// Disconnect device. Like `unregisterFromPC`, the IOBluetooth work runs on @@ -1416,8 +1548,13 @@ final class BluetoothPeripheralStore: NSObject, ObservableObject, BluetoothPerip let isConnected = connectedAddresses.contains(id) // Don't overwrite an in-flight .connecting/.releasing state with a // stale read (unless a caller explicitly wants the live value). - if !overrideTransient, - self.connectionStates[id] == .connecting || self.connectionStates[id] == .releasing + // One exception: a `.connecting` row with no pair pending and a + // live connection is a connect made behind our back (System + // Settings, macOS auto-reconnect) — adopt it rather than hold + // "Pairing…" until the watchdog fires. + if !overrideTransient, self.connectionStates[id] == .releasing { continue } + if !overrideTransient, self.connectionStates[id] == .connecting, + self.pendingPairs[id] != nil || !isConnected { continue } @@ -1627,6 +1764,7 @@ final class BluetoothPeripheralStore: NSObject, ObservableObject, BluetoothPerip // inline error; a failure that ends in .disconnected keeps it on screen. if state != .disconnected { self.clearPeripheralError(id) } if state == .connected { + self.bondsAwaitingRepair.remove(id) self.completeConnectResultWaiters(for: id, success: true) } else if state == .disconnected { self.completeConnectResultWaiters(for: id, success: false) @@ -1650,6 +1788,21 @@ final class BluetoothPeripheralStore: NSObject, ObservableObject, BluetoothPerip waiters.forEach { $0(success) } } + /// Drops one address's attempt-tracking state — the pending pair + /// (stopped), its watchdog, and the announce flag — returning whether the + /// consumed flag said to announce. Shared by every terminal arm of an + /// attempt (failure, timeout, cancel) so the per-attempt maps can't drift + /// out of lockstep. Main-only. + @discardableResult + private func tearDownPairAttempt(for id: String) -> Bool { + pendingPairs[id]?.stop() + pendingPairs.removeValue(forKey: id) + pendingPairAttempts.removeValue(forKey: id) + pairTimers[id]?.cancel() + pairTimers.removeValue(forKey: id) + return pairTimeoutShouldAnnounce.removeValue(forKey: id) ?? false + } + /// Terminal failure of a connect attempt before (or without) the pairing /// delegate ever firing: cancel the attempt's watchdog, surface the error, /// and land on `.disconnected` (which also fails any completion waiters). @@ -1677,19 +1830,22 @@ final class BluetoothPeripheralStore: NSObject, ObservableObject, BluetoothPerip attempt: UInt64? ) { DispatchQueue.main.async { - if let attempt, self.connectAttemptTokens[id] != attempt { return } - self.pairTimers[id]?.cancel() - self.pairTimers.removeValue(forKey: id) + if let attempt, !self.isCurrentAttempt(attempt, for: id) { return } // A missing flag means the watchdog already consumed it — the timeout // was reported (or deliberately silenced) for this same attempt, so a // late-arriving failure must not stack a second announcement on top, // and a silent watcher retry must stay silent. Every attempt sets the // flag up front in `schedulePairWatchdog`, so absent-because-never-set // can't happen. - let announce = self.pairTimeoutShouldAnnounce.removeValue(forKey: id) ?? false + let announce = self.tearDownPairAttempt(for: id) + // An attempt that tore the local bond down and then failed leaves the + // peripheral paired to nothing, which no retry cadence can undo on the + // user's behalf — so it is surfaced even where a plain failure stays + // quiet. + let bondWasReset = self.consumeBondAwaitingRepair(id) self.setConnectionState(.disconnected, for: id) - guard announce else { return } - self.setPeripheralError(inline, for: id) + guard announce || bondWasReset else { return } + self.setPeripheralError(bondWasReset ? "Pairing reset." : inline, for: id) // An armed watcher means this failure isn't the end of the attempt: // the watcher keeps probing (a just-released Magic device routinely // misses the first RSSI probe and comes up on a retry seconds later), @@ -1697,38 +1853,43 @@ final class BluetoothPeripheralStore: NSObject, ObservableObject, BluetoothPerip // would be premature noise on the takeover/adoption paths that arm it. // The inline row error above still records the miss; a failure with // no retry pending stays loud. - if notify, self.reconnectWatchlist[id] == nil { - NotificationManager.showNotification( - title: notifyTitle, - body: notifyBody, - identifier: "pair-failed-\(id)" - ) - } + guard notify, bondWasReset || self.reconnectWatchlist[id] == nil else { return } + NotificationManager.showNotification( + title: bondWasReset ? "Pairing Was Reset" : notifyTitle, + body: bondWasReset ? Self.bondResetBody(name) : notifyBody, + identifier: "pair-failed-\(id)" + ) } } + /// Body for the one failure the user has to act on themselves: the local + /// bond was torn down for a re-pair that then didn't happen. + private static func bondResetBody(_ name: String) -> String { + "Magic Switch reset this Mac's pairing for \(name) and couldn't pair it again. " + + "Turn \(name) off and on — if it doesn't come back, pair it again in " + + "System Settings → Bluetooth." + } + /// Mints the token identifying one connect attempt and records it as `id`'s - /// current one. The record lands via the same main-queue FIFO that already - /// orders the announce flag ahead of every failure path, so a failure can - /// never observe a token newer than its own attempt's. + /// current one, atomically under `attemptTokenLock` — the counter is + /// monotonic, so the newest mint always wins and every reader (main or the + /// Bluetooth queue) sees it immediately. private func beginConnectAttempt(for id: String) -> UInt64 { attemptTokenLock.lock() + defer { attemptTokenLock.unlock() } connectAttemptCounter += 1 let token = connectAttemptCounter - attemptTokenLock.unlock() - let apply: () -> Void = { [weak self] in - guard let self = self else { return } - // Newest wins: an off-main mint's record can arrive after a later - // main-side mint applied inline, and must not roll the map back to the - // older attempt (which would orphan the newer one's failure paths). - if (self.connectAttemptTokens[id] ?? 0) < token { - self.connectAttemptTokens[id] = token - } - } - if Thread.isMainThread { apply() } else { DispatchQueue.main.async(execute: apply) } + connectAttemptTokens[id] = token return token } + /// Whether `token` is still the newest connect attempt for `id`. + private func isCurrentAttempt(_ token: UInt64, for id: String) -> Bool { + attemptTokenLock.lock() + defer { attemptTokenLock.unlock() } + return connectAttemptTokens[id] == token + } + /// Set the inline error for a peripheral, and fade it after 5s so it doesn't /// linger on the row. `setConnectionState` clears it sooner on a new attempt. private func setPeripheralError(_ message: String, for id: String) { @@ -1783,33 +1944,39 @@ final class BluetoothPeripheralStore: NSObject, ObservableObject, BluetoothPerip // A superseded attempt's watchdog was cancelled when the newer attempt // re-armed it, but a fire already in flight can still land here — it // must not stop the newer attempt's pair or consume its flag. - guard connectAttemptTokens[address] == attempt else { return } + guard isCurrentAttempt(attempt, for: address) else { return } guard connectionStates[address] == .connecting else { - pairTimers.removeValue(forKey: address) - pairTimeoutShouldAnnounce.removeValue(forKey: address) + tearDownPairAttempt(for: address) return } - pendingPairs[address]?.stop() - pendingPairs.removeValue(forKey: address) - pendingPairAttempts.removeValue(forKey: address) - pairTimers.removeValue(forKey: address) - // `?? false` for the same reason as `failConnectAttempt`: an absent flag - // means another failure path already consumed it — atomically with - // cancelling this timer — so a straggling timeout must stay quiet. - let announce = pairTimeoutShouldAnnounce.removeValue(forKey: address) ?? false + // The teardown's `?? false` matters here for the same reason as in + // `failConnectAttempt`: an absent flag means another failure path already + // consumed it — atomically with cancelling this timer — so a straggling + // timeout must stay quiet. + let announce = tearDownPairAttempt(for: address) + // Same reasoning as `failConnectAttempt`: a timeout on an attempt that + // already tore the local bond down is the user's to repair, so it carries + // through the silent gates below. + let bondWasReset = consumeBondAwaitingRepair(address) setConnectionState(.disconnected, for: address) // A silent watcher retry just tries again on the next probe; only // interactive connects surface the timeout to the user. - guard announce else { return } + guard announce || bondWasReset else { return } // Inline first, like every other announced failure — the notification // below may be denied, and without this the row just quietly unsticks // after 60s as if nothing was ever tried. - setPeripheralError("Pairing timed out.", for: address) + setPeripheralError(bondWasReset ? "Pairing reset." : "Pairing timed out.", for: address) + // Same watcher-aware gate as `failConnectAttempt`: with silent retries + // still pending, the timeout is an interim state, not the outcome — a + // blind takeover pair against a device that's simply off would otherwise + // ride the watchdog into a loud notification on every attempt. + guard bondWasReset || reconnectWatchlist[address] == nil else { return } NotificationManager.showNotification( - title: "Pairing Timed Out", - body: - "Couldn't pair \(name). It may currently be connected to your other Mac — try the menu-bar switch action instead.", - identifier: "pair-timeout-\(address)" + title: bondWasReset ? "Pairing Was Reset" : "Pairing Timed Out", + body: bondWasReset + ? Self.bondResetBody(name) + : "Couldn't pair \(name). It may currently be connected to your other Mac — try the menu-bar switch action instead.", + identifier: bondWasReset ? "pair-failed-\(address)" : "pair-timeout-\(address)" ) } @@ -2065,7 +2232,6 @@ final class BluetoothPeripheralStore: NSObject, ObservableObject, BluetoothPerip connectPeripheral( peripheral, announcePairTimeout: false, - refreshPairingBeforeConnect: false, completion: nil ) return @@ -2093,7 +2259,6 @@ final class BluetoothPeripheralStore: NSObject, ObservableObject, BluetoothPerip self.connectPeripheral( peripheral, announcePairTimeout: false, - refreshPairingBeforeConnect: false, completion: nil ) } @@ -2137,14 +2302,15 @@ final class BluetoothPeripheralStore: NSObject, ObservableObject, BluetoothPerip progress.pairAttempts += 1 adoptionProgress[id] = progress print("Adoption: taking \(peripheral.name) (attempt \(progress.pairAttempts))") - // Adoption is a take-from-peer grab (the peer vanished), so refresh a stale - // local bond like the other take-from-peer callers — otherwise a peripheral - // stuck at `paired=true` with `openConnection()` failing never comes over. + // Adoption is a take-from-peer grab (the peer vanished), so a refused + // bonded open escalates to a bond refresh like the other take-from-peer + // callers — otherwise a peripheral stuck at `paired=true` with + // `openConnection()` failing never comes over. // Stays silent (`announcePairTimeout: false`): this is a background retry. connectPeripheral( peripheral, announcePairTimeout: false, - refreshPairingBeforeConnect: true, + refreshStaleBondOnFailedOpen: true, completion: nil ) } diff --git a/Magic Switch/View/MenuBar/DropdownContentView.swift b/Magic Switch/View/MenuBar/DropdownContentView.swift index 749b93f..7c788fb 100644 --- a/Magic Switch/View/MenuBar/DropdownContentView.swift +++ b/Magic Switch/View/MenuBar/DropdownContentView.swift @@ -53,7 +53,7 @@ final class MenuRowControl: NSControl { setHighlighted(false) // The stores can rebuild the menu while the tracking loop runs; a row // replaced mid-press has no window and its action captures stale state. - if inside, window != nil { onClick() } + if inside, self.window != nil { onClick() } } // MARK: - Hover highlight @@ -306,15 +306,26 @@ final class DropdownContentView: NSView { let state = bluetoothStore.connectionState(for: peripheral.id) let canSwitch = networkStore.networkDevices.contains { networkStore.isSwitchable($0) } let row = MenuRowControl { [weak self] in - self?.bluetoothStore.switchPeripheral(peripheral, direction: .toggle) + guard let self = self else { return } + // A click can race the rebuild that follows a state flip — act only on + // the state the row was showing when it was pressed. + guard self.bluetoothStore.connectionState(for: peripheral.id) == state else { return } + if state == .connecting { + self.bluetoothStore.cancelConnect(peripheral) + } else { + self.bluetoothStore.switchPeripheral(peripheral, direction: .toggle) + } } // A disconnected peripheral is always clickable — take it (locally over // Bluetooth if there's no peer to ask). A connected one can only be *sent*, - // so it greys out when no Mac is reachable to hand it to. A pairing row is - // disabled while in flight. + // so it greys out when no Mac is reachable to hand it to. A pairing row + // stays clickable so the attempt can be cancelled; a releasing one can't + // be — aborting a half-done release could strand the peripheral on + // neither Mac. let enabled: Bool switch state { - case .connecting, .releasing: enabled = false + case .releasing: enabled = false + case .connecting: enabled = true case .connected: enabled = canSwitch case .disconnected: enabled = true } @@ -374,7 +385,7 @@ final class DropdownContentView: NSView { switch state { case .connecting: - row.toolTip = "Pairing \(peripheral.name)…" + row.toolTip = "Pairing \(peripheral.name)… Click to cancel." case .releasing: row.toolTip = "Releasing \(peripheral.name) to the other Mac…" case .connected: