From 02b7bacc6f570a25b63325dc9421a294ce95d04e Mon Sep 17 00:00:00 2001 From: DX23876 <176692557+DX23876@users.noreply.github.com> Date: Fri, 16 Oct 2026 10:36:27 +0200 Subject: [PATCH] fix(ble): park the bond-loop pause's standing connect once the link is down Refs #1539. The #1544 escape never armed on Apple. Every site that trips the pause runs inside didDisconnectPeripheral before that handler sets state.connected to false, and standingConnectWhilePausedIfDue read state.connected right there: the dying link still counted as up, so shouldStandingConnectWhilePaused refused and nothing was parked. The paused branch further down the same handler then refused as well, because the trip had just started the 10-minute floor. With the app backgrounded nothing else re-arms it, so the strap stayed disconnected until the app was next opened, the symptom #1539 reported. Seen on a WHOOP MG: two connection timeouts within 11 s tripped the #617 detector at 07:37:19, the log never printed "parking a standing connect", and the strap stayed disconnected for 49 minutes until the app was reopened, while a workout was being recorded. Android never had this. Its twin posts the check to the handler, so it runs after handleDisconnect has finished the teardown. The Swift side now does the same on the next main-queue turn, which covers all four call sites at once. The paused-disconnect line also said "strap keeps refusing to pair" for every pause, including the #617 drops and the #971/#982 pairing that never finished, where no refusal was observed. Both platforms now record the cause with the latch and name it: "strap keeps refusing to pair", "pairing never finished" or "the link dropped within seconds of pairing N times in a row". Tests: BondLoopPauseParkingTests drives the real call order (trip while the link still reads connected, then tear down) and failed before the change; the wording is pinned on both platforms with identical strings (BondLoopPauseCauseTest). macOS StrandTests and the Android com.noop.ble unit tests pass; the iOS target builds. --- Strand/BLE/BLEManager.swift | 64 +++++++++++++++---- StrandTests/BondLoopPauseParkingTests.swift | 57 +++++++++++++++++ .../java/com/noop/ble/BondLoopPauseCause.kt | 22 +++++++ .../main/java/com/noop/ble/WhoopBleClient.kt | 10 ++- .../com/noop/ble/BondLoopPauseCauseTest.kt | 22 +++++++ 5 files changed, 162 insertions(+), 13 deletions(-) create mode 100644 StrandTests/BondLoopPauseParkingTests.swift create mode 100644 android/app/src/main/java/com/noop/ble/BondLoopPauseCause.kt create mode 100644 android/app/src/test/java/com/noop/ble/BondLoopPauseCauseTest.kt diff --git a/Strand/BLE/BLEManager.swift b/Strand/BLE/BLEManager.swift index 690e394194..18f1f5ad18 100644 --- a/Strand/BLE/BLEManager.swift +++ b/Strand/BLE/BLEManager.swift @@ -134,6 +134,26 @@ struct PostBondTimeoutLoopDetector { } } +/// Why auto-reconnect is paused, so the paused-disconnect line names what was observed rather than one +/// cause for all of them. The pause trips on a refused bond (#747), on a link that drops within seconds of +/// pairing (#617), or (Android only) on a pairing that never finishes (#971/#982); only the first is a +/// refusal. Twin of Kotlin `BondLoopPauseCause`, same phrases. +enum BondLoopPauseCause: Equatable { + case refusedBond + case pairingNeverFinished + case droppedAfterPairing(cycles: Int) + + /// The parenthetical of the paused-disconnect line. + var pausedReason: String { + switch self { + case .refusedBond: return "strap keeps refusing to pair" + case .pairingNeverFinished: return "pairing never finished" + case .droppedAfterPairing(let cycles): + return "the link dropped within seconds of pairing \(cycles) times in a row" + } + } +} + /// #747 / #750: decides when a strap that keeps REFUSING the encrypted bond ("Encryption/Authentication is /// insufficient", no genuine bond in between) has refused enough times that hammering it further is /// pointless. Two responsibilities, both pure so they're unit-testable without a CoreBluetooth seam: @@ -1249,6 +1269,8 @@ public final class BLEManager: NSObject, ObservableObject { /// disconnect() clears it via `bondGiveUp.reset()`. Distinct from `intentionalDisconnect` so a paused /// link still reports its state honestly rather than looking like a user teardown. private var autoReconnectPausedForBondLoop = false + /// What tripped the current pause, for the paused-disconnect line. Set with the latch at every trip site. + private var bondLoopPauseCause: BondLoopPauseCause = .refusedBond /// When the bond-loop pause last tripped (or last salvage-probed). Drives the #78 hole-4 salvage /// probe's 10-minute floor (`shouldSalvageProbe`): a paused strap the user has since FREED self-heals /// on the next app-foreground instead of staying disconnected until a manual Connect, while a strap @@ -1951,17 +1973,35 @@ public final class BLEManager: NSObject, ObservableObject { /// is backgrounded, which is exactly where the foreground probe cannot reach. Re-stamps /// `bondLoopPausedAt` so a strap that is reachable and still refusing gets one attempt per window /// rather than a connect/refuse spin. + /// + /// Evaluated on the next main-queue turn, as the Kotlin twin's `handler.post` is. Every trip site runs + /// inside `didDisconnectPeripheral` before that handler sets `state.connected = false`; read there, the + /// dying link still counted as up, nothing was parked, and the floored re-park in the paused branch then + /// refused because the trip had just started the floor. The pause stayed stranded until the next app + /// foreground, the #1539 symptom this exists to prevent. func standingConnectWhilePausedIfDue(now: Date = Date(), justTripped: Bool = false) { - let since = justTripped ? nil : bondLoopPausedAt.map { now.timeIntervalSince($0) } - guard BLEManager.shouldStandingConnectWhilePaused( - pausedForBondLoop: autoReconnectPausedForBondLoop, - connected: state.connected, - intentionalDisconnect: intentionalDisconnect, - secondsSincePauseTripped: since) else { return } - bondLoopPausedAt = now - log("Bond-loop pause: parking a standing connect so the strap is claimed the moment it is reachable (#1539) - the give-up stays latched") - issueStandingConnect(whilePausedForBondLoop: true) + DispatchQueue.main.async { [weak self] in + guard let self else { return } + let since = justTripped ? nil : self.bondLoopPausedAt.map { now.timeIntervalSince($0) } + guard BLEManager.shouldStandingConnectWhilePaused( + pausedForBondLoop: self.autoReconnectPausedForBondLoop, + connected: self.state.connected, + intentionalDisconnect: self.intentionalDisconnect, + secondsSincePauseTripped: since) else { return } + self.bondLoopPausedAt = now + self.log("Bond-loop pause: parking a standing connect so the strap is claimed the moment it is reachable (#1539) - the give-up stays latched") + self.issueStandingConnect(whilePausedForBondLoop: true) + } + } + + #if DEBUG + /// Test seam: latch the bond-loop pause the way a trip site does, without a CoreBluetooth link. + func latchBondLoopPauseForTesting(cause: BondLoopPauseCause) { + autoReconnectPausedForBondLoop = true + bondLoopPauseCause = cause + bondLoopPausedAt = Date() } + #endif /// Observe the app-foreground notification and run the salvage probe. Installed once per manager from /// init; self-contained here so the probe needs no per-target scene wiring. iOS and macOS only (the @@ -6078,6 +6118,7 @@ extension BLEManager: @preconcurrency CBCentralManagerDelegate { log(BondRefusalGiveUp.helloSuppressedEpitaph(refusals: bondGiveUp.refusals, opaqueId: opaque)) } else { autoReconnectPausedForBondLoop = true + bondLoopPauseCause = .refusedBond bondLoopPausedAt = Date() // starts the #78 hole-4 salvage-probe floor // #1539: park the connect in the same breath as the pause, so this can end while backgrounded. standingConnectWhilePausedIfDue(justTripped: true) @@ -6218,6 +6259,7 @@ extension BLEManager: @preconcurrency CBCentralManagerDelegate { // Connect (connect()) or a genuine bond re-arms it. We do NOT touch the bond/parse path — the bond // is real; the stale OS pairing is the problem, which the guide tells the user how to clear. autoReconnectPausedForBondLoop = true + bondLoopPauseCause = .droppedAfterPairing(cycles: postBondLoop.consecutiveBondTimeouts) bondLoopPausedAt = Date() // the #78 hole-4 salvage probe covers this pause too (one bounded cycle) // #1539: arm the parked connect in the same breath as the pause. The salvage probe only fires on // app-foreground, so without this a pause tripped with the phone in a pocket strands the strap @@ -6373,13 +6415,13 @@ extension BLEManager: @preconcurrency CBCentralManagerDelegate { // #747: the bond keeps being refused, so auto-reconnect is paused: we stop hammering a strap that // can't bond (the epitaph + paused hint were already surfaced when the give-up tripped). The user // re-arms it by tapping Connect. We do NOT schedule a rescan here. - log("Disconnected\(error.map { ": \($0.localizedDescription)" } ?? "")\(BLEManager.bleErrorSuffix(error)); auto-reconnect paused (strap keeps refusing to pair; tap Connect once it's free)") + log("Disconnected\(error.map { ": \($0.localizedDescription)" } ?? "")\(BLEManager.bleErrorSuffix(error)); auto-reconnect paused (\(bondLoopPauseCause.pausedReason); tap Connect once it's free)") // #1539: a connect attempt CONSUMES the parked request, so re-park it — floored, so a reachable // strap that keeps refusing gets one attempt per window instead of a connect/refuse spin. standingConnectWhilePausedIfDue() if TestCentre.active(.connection) { state.append(log: "connect down (uptime ends)", domain: .connection) - state.append(log: "reconnect paused=bondLoop (strap refusing bond)", domain: .connection) + state.append(log: "reconnect paused=bondLoop (\(bondLoopPauseCause.pausedReason))", domain: .connection) } } else if !intentionalDisconnect { log("Disconnected\(error.map { " — \($0.localizedDescription)" } ?? "")\(BLEManager.bleErrorSuffix(error))") diff --git a/StrandTests/BondLoopPauseParkingTests.swift b/StrandTests/BondLoopPauseParkingTests.swift new file mode 100644 index 0000000000..ddcf62d0a1 --- /dev/null +++ b/StrandTests/BondLoopPauseParkingTests.swift @@ -0,0 +1,57 @@ +import XCTest +@testable import Strand + +/// The bond-loop pause (#617/#747) must leave a parked connect behind on the drop that trips it (#1539), and +/// its paused-disconnect line must name the cause that was observed. +/// +/// Every trip site runs inside `didDisconnectPeripheral` before that handler sets `state.connected = false`. +/// `standingConnectWhilePausedIfDue(justTripped:)` read `state.connected` synchronously there, saw the dying +/// link as up and parked nothing; the floored re-park in the paused branch then refused too, because the +/// trip had just started the 10-minute floor. A pause tripped with the phone in a pocket stayed stranded +/// until the app was next opened, which is the symptom #1539 was meant to fix. Android never had this: its +/// twin posts the check to the handler, so it runs after the teardown. +@MainActor +final class BondLoopPauseParkingTests: XCTestCase { + + private func drainMainQueue() async { + await withCheckedContinuation { c in DispatchQueue.main.async { c.resume() } } + } + + private func parked(_ state: LiveState) -> Bool { + state.log.contains { $0.contains("parking a standing connect") } + } + + func testATripInsideTheDisconnectHandlerParksOnceTheLinkIsDown() async { + let state = LiveState() + let manager = BLEManager(state: state, collector: nil) + manager.latchBondLoopPauseForTesting(cause: .droppedAfterPairing(cycles: 2)) + state.connected = true // the handler has not torn the link down yet + manager.standingConnectWhilePausedIfDue(justTripped: true) + state.connected = false // ...and now it has + await drainMainQueue() + XCTAssertTrue(parked(state), "the trip must leave a parked connect behind") + } + + func testNoConnectIsParkedWhileTheLinkReallyStaysUp() async { + let state = LiveState() + let manager = BLEManager(state: state, collector: nil) + manager.latchBondLoopPauseForTesting(cause: .refusedBond) + state.connected = true + manager.standingConnectWhilePausedIfDue(justTripped: true) + await drainMainQueue() + XCTAssertFalse(parked(state), "a live link needs no parked connect") + } + + // MARK: Paused-disconnect wording + + func testEachPauseCauseNamesWhatWasObserved() { + XCTAssertEqual(BondLoopPauseCause.refusedBond.pausedReason, "strap keeps refusing to pair") + XCTAssertEqual(BondLoopPauseCause.pairingNeverFinished.pausedReason, "pairing never finished") + XCTAssertEqual(BondLoopPauseCause.droppedAfterPairing(cycles: 2).pausedReason, + "the link dropped within seconds of pairing 2 times in a row") + } + + func testTheDropsAfterPairingNeverClaimARefusal() { + XCTAssertFalse(BondLoopPauseCause.droppedAfterPairing(cycles: 3).pausedReason.contains("refus")) + } +} diff --git a/android/app/src/main/java/com/noop/ble/BondLoopPauseCause.kt b/android/app/src/main/java/com/noop/ble/BondLoopPauseCause.kt new file mode 100644 index 0000000000..e6de13c59a --- /dev/null +++ b/android/app/src/main/java/com/noop/ble/BondLoopPauseCause.kt @@ -0,0 +1,22 @@ +package com.noop.ble + +/** + * Mirror of the Swift `BondLoopPauseCause` (Strand/BLE/BLEManager.swift), same phrases. + * + * Why auto-reconnect is paused, so the paused-disconnect line names what was observed rather than one + * cause for all of them. The pause trips on a refused bond (#747), on a pairing that never finishes + * (#971/#982) or on a link that drops within seconds of pairing (#617); only the first is a refusal. + */ +sealed class BondLoopPauseCause { + object RefusedBond : BondLoopPauseCause() + object PairingNeverFinished : BondLoopPauseCause() + data class DroppedAfterPairing(val cycles: Int) : BondLoopPauseCause() + + /** The parenthetical of the paused-disconnect line. */ + val pausedReason: String + get() = when (this) { + RefusedBond -> "strap keeps refusing to pair" + PairingNeverFinished -> "pairing never finished" + is DroppedAfterPairing -> "the link dropped within seconds of pairing $cycles times in a row" + } +} diff --git a/android/app/src/main/java/com/noop/ble/WhoopBleClient.kt b/android/app/src/main/java/com/noop/ble/WhoopBleClient.kt index 8fd7e63730..71d7c49b93 100644 --- a/android/app/src/main/java/com/noop/ble/WhoopBleClient.kt +++ b/android/app/src/main/java/com/noop/ble/WhoopBleClient.kt @@ -6900,6 +6900,7 @@ class WhoopBleClient( "the command channel existed) " + bondWatchdogContext() + " — pausing auto-reconnect and surfacing the re-pair guide (#971)") autoReconnectPausedForBondLoop = true + bondLoopPauseCause = BondLoopPauseCause.PairingNeverFinished bondLoopPausedAtMs = System.currentTimeMillis() // the #78 hole-4 salvage probe covers this pause too // #1539: park the connect in the same breath as the pause, so this can end while backgrounded. standingConnectWhilePausedIfDue(justTripped = true) @@ -7025,6 +7026,8 @@ class WhoopBleClient( * `autoReconnectPausedForBondLoop`. */ @Volatile private var autoReconnectPausedForBondLoop = false + /** What tripped the current pause, for the paused-disconnect line. Set with the latch at every trip site. */ + private var bondLoopPauseCause: BondLoopPauseCause = BondLoopPauseCause.RefusedBond /** When the bond-loop pause last tripped (or last salvage-probed), epoch ms. Drives the #78 hole-4 * salvage probe's floor ([BOND_LOOP_SALVAGE_FLOOR_MS] via [shouldSalvageProbe]): a paused strap the @@ -7155,6 +7158,7 @@ class WhoopBleClient( log(BondRefusalGiveUp.helloSuppressedEpitaph(bondGiveUp.refusals, opaque)) } else { autoReconnectPausedForBondLoop = true + bondLoopPauseCause = BondLoopPauseCause.RefusedBond bondLoopPausedAtMs = System.currentTimeMillis() // #78 hole-4 salvage-probe floor starts here // #1539: park the connect in the same breath as the pause, so this can end while backgrounded. standingConnectWhilePausedIfDue(justTripped = true) @@ -11850,6 +11854,7 @@ class WhoopBleClient( // touch the bond/parse path — the bond is real; the stale OS pairing is the problem. Mirrors the // Swift BLEManager #844 fix. autoReconnectPausedForBondLoop = true + bondLoopPauseCause = BondLoopPauseCause.DroppedAfterPairing(postBondLoop.consecutiveBondTimeouts) bondLoopPausedAtMs = System.currentTimeMillis() // the #78 hole-4 salvage probe covers this pause too (one bounded cycle) // #1539: park the connect in the same breath as the pause, so this can end while backgrounded. standingConnectWhilePausedIfDue(justTripped = true) @@ -11907,6 +11912,7 @@ class WhoopBleClient( log("Strap connects and subscribes but never finishes pairing, then self-drops before the bond watchdog fires (${bondWatchdogBackoff.consecutiveBounces} cycles) " + bondWatchdogContext() + " — pausing auto-reconnect and surfacing the re-pair guide (#982/#971)") autoReconnectPausedForBondLoop = true + bondLoopPauseCause = BondLoopPauseCause.PairingNeverFinished bondLoopPausedAtMs = System.currentTimeMillis() // the #78 hole-4 salvage probe covers this pause too // #1539: park the connect in the same breath as the pause, so this can end while backgrounded. standingConnectWhilePausedIfDue(justTripped = true) @@ -12031,13 +12037,13 @@ class WhoopBleClient( // #747: the bond keeps being refused, so auto-reconnect is paused: we stop hammering a strap that // can't bond (the epitaph + paused hint were already surfaced when the give-up tripped). The user // re-arms it by tapping Connect (clearPairingHintForUserConnect). We do NOT schedule a reconnect. - log("Disconnected ${disconnectStatusLabel(status)}; auto-reconnect paused (strap keeps refusing to pair; tap Connect once it's free)") + log("Disconnected ${disconnectStatusLabel(status)}; auto-reconnect paused (${bondLoopPauseCause.pausedReason}; tap Connect once it's free)") // #1539: a connect attempt CONSUMES the parked request, so re-park it — floored, so a reachable // strap that keeps refusing gets one attempt per window instead of a connect/refuse spin. standingConnectWhilePausedIfDue() if (testCentre.active(com.noop.testcentre.TestDomain.CONNECTION)) { log("connect down (uptime ends)", com.noop.testcentre.TestDomain.CONNECTION) - log("reconnect paused=bondLoop (strap refusing bond)", com.noop.testcentre.TestDomain.CONNECTION) + log("reconnect paused=bondLoop (${bondLoopPauseCause.pausedReason})", com.noop.testcentre.TestDomain.CONNECTION) } } else if (!intentionalDisconnect) { // Connection test mode: count + describe the reconnect churn, and mark the link down for the diff --git a/android/app/src/test/java/com/noop/ble/BondLoopPauseCauseTest.kt b/android/app/src/test/java/com/noop/ble/BondLoopPauseCauseTest.kt new file mode 100644 index 0000000000..caebf2d487 --- /dev/null +++ b/android/app/src/test/java/com/noop/ble/BondLoopPauseCauseTest.kt @@ -0,0 +1,22 @@ +package com.noop.ble + +import org.junit.Assert.assertEquals +import org.junit.Assert.assertFalse +import org.junit.Test + +/** Twin of Swift `BondLoopPauseParkingTests` wording tests: the same phrases on both platforms. */ +class BondLoopPauseCauseTest { + + @Test + fun eachPauseCauseNamesWhatWasObserved() { + assertEquals("strap keeps refusing to pair", BondLoopPauseCause.RefusedBond.pausedReason) + assertEquals("pairing never finished", BondLoopPauseCause.PairingNeverFinished.pausedReason) + assertEquals("the link dropped within seconds of pairing 2 times in a row", + BondLoopPauseCause.DroppedAfterPairing(2).pausedReason) + } + + @Test + fun theDropsAfterPairingNeverClaimARefusal() { + assertFalse(BondLoopPauseCause.DroppedAfterPairing(3).pausedReason.contains("refus")) + } +}