From 5c20f9446dfd6ed21e2b311146d516f4a287ee3b Mon Sep 17 00:00:00 2001 From: voiceagentscc Date: Sat, 17 Oct 2026 22:03:47 -0400 Subject: [PATCH 1/6] fix(crp): coordinate reconnect teardown and recovery (#124) --- .../java/com/pulseloop/ring/CRPReconnect.kt | 66 +++++ .../com/pulseloop/ring/CRPTeardownGate.kt | 79 ++++++ .../com/pulseloop/ring/CRPTeardownMonitor.kt | 90 +++++++ .../java/com/pulseloop/ring/RingBLEClient.kt | 247 +++++++++++++++--- .../java/com/pulseloop/ui/PulseLoopApp.kt | 2 +- .../com/pulseloop/ui/components/AppChrome.kt | 3 +- .../ui/components/DeviceHeroStatus.kt | 5 +- .../com/pulseloop/ring/CRPReconnectTest.kt | 140 ++++++++++ .../com/pulseloop/ring/CRPTeardownGateTest.kt | 192 ++++++++++++++ .../ui/components/DeviceHeroStatusTest.kt | 11 + 10 files changed, 801 insertions(+), 34 deletions(-) create mode 100644 app/src/main/java/com/pulseloop/ring/CRPReconnect.kt create mode 100644 app/src/main/java/com/pulseloop/ring/CRPTeardownGate.kt create mode 100644 app/src/main/java/com/pulseloop/ring/CRPTeardownMonitor.kt create mode 100644 app/src/test/java/com/pulseloop/ring/CRPReconnectTest.kt create mode 100644 app/src/test/java/com/pulseloop/ring/CRPTeardownGateTest.kt diff --git a/app/src/main/java/com/pulseloop/ring/CRPReconnect.kt b/app/src/main/java/com/pulseloop/ring/CRPReconnect.kt new file mode 100644 index 00000000..e438b6dc --- /dev/null +++ b/app/src/main/java/com/pulseloop/ring/CRPReconnect.kt @@ -0,0 +1,66 @@ +package com.pulseloop.ring + +import kotlinx.coroutines.CoroutineScope +import kotlinx.coroutines.CoroutineStart +import kotlinx.coroutines.Job +import kotlinx.coroutines.delay +import kotlinx.coroutines.launch + +/** + * Da Rings retries three seconds after a disconnect (`o3/l.o` in the Android decompile). + * Keep this separate from the 15-second liveness watchdog: a five-second Android connection + * timeout otherwise leaves the user waiting another ten seconds before recovery even starts. + * + * GATT callbacks and lifecycle actions may arrive on different threads. The epoch and monitor + * make cancellation invalidate even a timer that has finished its delay and is waiting to run. + * Callbacks run under that monitor, so cancellation cannot return ahead of a stale retry. + */ +internal class CRPReconnect( + private val scope: CoroutineScope, + private val canRetry: () -> Boolean, + private val onPending: () -> Unit, + private val onRetry: () -> Unit, + private val onAbandoned: () -> Unit, +) { + private val lock = Any() + private var epoch = 0L + private var timer: Job? = null + private var recovering = false + + val isPending: Boolean get() = synchronized(lock) { timer != null } + /** Remains true during a recovery scan, until success, cancellation, or exhausted retries. */ + val isRecovering: Boolean get() = synchronized(lock) { recovering } + + fun schedule(): Boolean = synchronized(lock) { + if (timer != null || !canRetry()) return false + recovering = true + val generation = ++epoch + val job = scope.launch(start = CoroutineStart.LAZY) { + delay(3_000L) + synchronized(lock) { + if (generation != epoch) return@synchronized + timer = null + if (canRetry()) onRetry() else { + recovering = false + onAbandoned() + } + } + } + timer = job + onPending() + job.start() + true + } + + /** A new attempt supersedes the timer but still belongs to the current recovery. */ + fun cancelPending() = synchronized(lock) { + epoch++ + timer?.cancel() + timer = null + } + + fun cancel() = synchronized(lock) { + cancelPending() + recovering = false + } +} diff --git a/app/src/main/java/com/pulseloop/ring/CRPTeardownGate.kt b/app/src/main/java/com/pulseloop/ring/CRPTeardownGate.kt new file mode 100644 index 00000000..095438f6 --- /dev/null +++ b/app/src/main/java/com/pulseloop/ring/CRPTeardownGate.kt @@ -0,0 +1,79 @@ +package com.pulseloop.ring + +import kotlinx.coroutines.CompletableDeferred +import kotlinx.coroutines.withTimeoutOrNull + +/** Physical teardown outlives both the GATT disconnected callback and a worker-owned client. */ +internal class CRPTeardownGate( + private val nowNanos: () -> Long, + private val timeoutMs: Long = 5_000L, +) { + internal class Ticket(val address: String) { + val ready = CompletableDeferred() + var closedAtNanos: Long? = null + var disconnected = false + var scanAfterNanos: Long? = null + } + + private val lock = Any() + private val pending = mutableMapOf() + + fun begin(address: String): Ticket = synchronized(lock) { + // An older close completion must never release a newer teardown for the same ring. + pending.remove(address)?.ready?.complete(false) + Ticket(address).also { pending[address] = it } + } + + fun isPending(address: String): Boolean = synchronized(lock) { address in pending } + + fun gattClosed(ticket: Ticket) = synchronized(lock) { + if (pending[ticket.address] !== ticket) return@synchronized + ticket.closedAtNanos = nowNanos() + finishIfReady(ticket) + } + + fun aclDisconnected(address: String) = synchronized(lock) { + pending[address]?.let { + it.disconnected = true + finishIfReady(it) + } + } + + fun bluetoothOff() = synchronized(lock) { + pending.values.toList().forEach { + it.disconnected = true + finishIfReady(it) + } + } + + /** False means require a fresh scan, never blindly reuse the old link on timeout. */ + suspend fun await(address: String): Boolean { + val ticket = synchronized(lock) { pending[address] } ?: return true + if (synchronized(lock) { ticket.scanAfterNanos != null }) return false + val ready = withTimeoutOrNull(timeoutMs) { ticket.ready.await() } + if (ready != null) return ready + return synchronized(lock) { + if (pending[address] !== ticket) return@synchronized !pending.containsKey(address) + ticket.scanAfterNanos = nowNanos() + false + } + } + + /** Only a post-timeout, freshly observed advertisement can substitute for a missed broadcast. */ + fun freshAdvertisement(address: String, timestampNanos: Long): Boolean = synchronized(lock) { + val ticket = pending[address] ?: return true + val after = ticket.scanAfterNanos ?: return false + val closed = ticket.closedAtNanos ?: return false + if (timestampNanos <= maxOf(after, closed)) return false + pending.remove(address) + ticket.ready.complete(true) + true + } + + private fun finishIfReady(ticket: Ticket) { + if (ticket.closedAtNanos != null && ticket.disconnected) { + pending.remove(ticket.address) + ticket.ready.complete(true) + } + } +} diff --git a/app/src/main/java/com/pulseloop/ring/CRPTeardownMonitor.kt b/app/src/main/java/com/pulseloop/ring/CRPTeardownMonitor.kt new file mode 100644 index 00000000..ba852a3d --- /dev/null +++ b/app/src/main/java/com/pulseloop/ring/CRPTeardownMonitor.kt @@ -0,0 +1,90 @@ +package com.pulseloop.ring + +import android.annotation.SuppressLint +import android.bluetooth.BluetoothAdapter +import android.bluetooth.BluetoothDevice +import android.bluetooth.BluetoothGatt +import android.content.BroadcastReceiver +import android.content.Context +import android.content.Intent +import android.content.IntentFilter +import android.os.Build +import android.os.SystemClock +import android.util.Log +import androidx.core.content.ContextCompat +import kotlinx.coroutines.CoroutineScope +import kotlinx.coroutines.Dispatchers +import kotlinx.coroutines.SupervisorJob +import kotlinx.coroutines.delay +import kotlinx.coroutines.launch + +/** + * One monitor per process, using only the application context. A worker can finish while its + * physical link is still disconnecting; a new foreground client must observe the same gate. + * The receiver intentionally lives for the process, including while no client is connecting. + */ +@SuppressLint("MissingPermission") +internal class CRPTeardownMonitor private constructor(context: Context) { + val gate = CRPTeardownGate(SystemClock::elapsedRealtimeNanos) + private val scope = CoroutineScope(Dispatchers.Main + SupervisorJob()) + + init { + val receiver = object : BroadcastReceiver() { + override fun onReceive(context: Context, intent: Intent) { + when (intent.action) { + BluetoothDevice.ACTION_ACL_DISCONNECTED -> { + // Older Android releases may omit EXTRA_TRANSPORT. CRP rings are LE-only. + if (intent.getIntExtra(BluetoothDevice.EXTRA_TRANSPORT, + BluetoothDevice.TRANSPORT_LE) != BluetoothDevice.TRANSPORT_LE) return + val device = if (Build.VERSION.SDK_INT >= 33) { + intent.getParcelableExtra(BluetoothDevice.EXTRA_DEVICE, BluetoothDevice::class.java) + } else { + @Suppress("DEPRECATION") + intent.getParcelableExtra(BluetoothDevice.EXTRA_DEVICE) + } ?: return + if (gate.isPending(device.address)) { + Log.i("RingBLEClient", "CRP physical disconnect confirmed") + gate.aclDisconnected(device.address) + } + } + BluetoothAdapter.ACTION_STATE_CHANGED -> { + if (intent.getIntExtra(BluetoothAdapter.EXTRA_STATE, -1) == BluetoothAdapter.STATE_OFF) { + gate.bluetoothOff() + } + } + } + } + } + // Bluetooth broadcasts originate in a privileged process outside the system UID. + // These are protected system actions; NOT_EXPORTED can silently miss their delivery. + try { + ContextCompat.registerReceiver( + context.applicationContext, receiver, + IntentFilter(BluetoothDevice.ACTION_ACL_DISCONNECTED).apply { + addAction(BluetoothAdapter.ACTION_STATE_CHANGED) + }, + ContextCompat.RECEIVER_EXPORTED, + ) + } catch (e: Exception) { + Log.w("RingBLEClient", "CRP ACL receiver unavailable; will require fresh advertisement", e) + } + } + + /** Register the wait before disconnecting, and close even if the owning client's scope dies. */ + fun disconnect(gatt: BluetoothGatt, close: () -> Unit) { + val ticket = gate.begin(gatt.device.address) + try { gatt.disconnect() } catch (_: Exception) {} + scope.launch { + delay(500L) + try { close() } finally { gate.gattClosed(ticket) } + } + } + + companion object { + @Volatile private var instance: CRPTeardownMonitor? = null + fun current(): CRPTeardownMonitor? = instance + fun get(context: Context): CRPTeardownMonitor = instance ?: synchronized(this) { + instance ?: CRPTeardownMonitor(context.applicationContext).also { instance = it } + } + } +} diff --git a/app/src/main/java/com/pulseloop/ring/RingBLEClient.kt b/app/src/main/java/com/pulseloop/ring/RingBLEClient.kt index 60c2e4e1..ad51524c 100644 --- a/app/src/main/java/com/pulseloop/ring/RingBLEClient.kt +++ b/app/src/main/java/com/pulseloop/ring/RingBLEClient.kt @@ -208,6 +208,34 @@ class RingBLEClient( private var watchdogJob: Job? = null private var ownershipRetryJob: Job? = null @Volatile private var watchdogReconnectPaused = false + @Volatile private var teardownWaitJob: Job? = null + private var teardownWaitAddress: String? = null + @Volatile private var teardownScanAddress: String? = null + private val teardownWaitEpoch = java.util.concurrent.atomic.AtomicLong() + + private val crpReconnect = CRPReconnect( + scope = scope, + canRetry = { + activeCoordinator?.deviceType == RingDeviceType.CRP && + lastKnownIdentifier != null && !watchdogReconnectPaused && + !prefs.getBoolean(USER_DISCONNECTED_KEY, false) && + !synchronized(forgetLock) { forgetPending || forgetFinalizing } && + bluetoothAdapter.isEnabled && hasPermissions() && + reconnectAttempts < MAX_RECONNECT_ATTEMPTS && + _state.value.connectionState !in setOf( + RingConnectionState.CONNECTED, RingConnectionState.CONNECTING, + ) + }, + onPending = { + recordDiagnostic("CRP reconnect scheduled in 3000ms") + updateState { copy(connectionState = RingConnectionState.RECONNECTING) } + PulseEventBus.publishBlocking( + PulseEvent.DeviceStateChanged(RingConnectionState.RECONNECTING, lastKnownIdentifier) + ) + }, + onRetry = { connectLastKnownInternal() }, + onAbandoned = { finishCrpRecovery() }, + ) // MARK: Service-discovery gate // @@ -236,6 +264,13 @@ class RingBLEClient( } fun startScanning() { + // A pairing scan supersedes automatic recovery. Recovery's own scans use the private + // entry point so their pending presentation survives until the ring is found. + finishCrpRecovery() + startScanningInternal() + } + + private fun startScanningInternal() { if (!hasPermissions()) { updateState { copy(lastError = "BLE permissions not granted") } return @@ -249,7 +284,9 @@ class RingBLEClient( reconnectScanPending = false updateState { copy( - connectionState = RingConnectionState.SCANNING, + connectionState = if (crpReconnect.isRecovering || teardownScanAddress != null) + RingConnectionState.RECONNECTING + else RingConnectionState.SCANNING, discovered = emptyList(), lastError = null, ) @@ -263,9 +300,11 @@ class RingBLEClient( } fun stopScanning() { + finishCrpRecovery() reconnectScanPending = false scanner?.stopScan(scanCallback) - if (_state.value.connectionState == RingConnectionState.SCANNING) { + if (_state.value.connectionState in setOf( + RingConnectionState.SCANNING, RingConnectionState.RECONNECTING)) { updateState { copy(connectionState = RingConnectionState.IDLE) } } } @@ -337,6 +376,9 @@ class RingBLEClient( } private fun connectLastKnownInternal() { + // A callback-driven CRP retry owns the next attempt until its timer fires. + if (crpReconnect.isPending) return + if (teardownWaitJob?.isActive == true) return if (!bluetoothAdapter.isEnabled) return // Honor a user-initiated Disconnect: stay off until the user reconnects ([userConnect]) // or pairs a new ring ([connectTo]). Every auto-reconnect path — foreground @@ -344,7 +386,10 @@ class RingBLEClient( // here, so this one guard makes "Disconnect" actually stick. if (prefs.getBoolean(USER_DISCONNECTED_KEY, false)) return val lastId = lastKnownIdentifier ?: return - if (reconnectAttempts >= MAX_RECONNECT_ATTEMPTS) return + if (reconnectAttempts >= MAX_RECONNECT_ATTEMPTS) { + if (crpReconnect.isRecovering || teardownScanAddress != null) finishCrpRecovery() + return + } // A still-pending reconnect scan means the last attempt never sighted the ring. if (reconnectScanPending) { reconnectScanPending = false @@ -362,7 +407,7 @@ class RingBLEClient( bluetoothAdapter.getRemoteDevice(lastId) } catch (_: Exception) { null } if (device == null) { - startScanning() + startScanningInternal() return } if (reconnectAttempts % 3 == 1) { @@ -373,14 +418,16 @@ class RingBLEClient( advertisedName = try { device.name } catch (_: Exception) { null }, ) } else { - startScanning() - // Set AFTER startScanning (which clears it to protect pairing scans). + startScanningInternal() + // Set AFTER startScanningInternal (which clears it to protect pairing scans). reconnectScanPending = true } } /** Clear the reconnect throttling — called on user action and app foreground. */ private fun resetReconnectBackoff() { + crpReconnect.cancel() + cancelTeardownConnect() reconnectAttempts = 0 sightlessScans = 0 reconnectScanPending = false @@ -395,6 +442,8 @@ class RingBLEClient( fun reconnectIfNeeded() { if (!bluetoothAdapter.isEnabled || !hasPermissions()) return val lastId = lastKnownIdentifier ?: return + // Foregrounding must not restart an already scheduled recovery or abandon its scan. + if (crpReconnect.isRecovering || teardownWaitJob?.isActive == true || teardownScanAddress != null) return // Fresh user attention = fresh retry budget (the cap exists to stop unattended // background churn, not to block a user actively waiting on the connection). resetReconnectBackoff() @@ -433,8 +482,15 @@ class RingBLEClient( } private fun connectionWatchdogTick() { - if (!bluetoothAdapter.isEnabled || !hasPermissions()) return + if (!bluetoothAdapter.isEnabled || !hasPermissions()) { + if (crpReconnect.isRecovering || teardownWaitJob != null || teardownScanAddress != null) { + finishCrpRecovery() + } + return + } if (watchdogReconnectPaused) return + if (crpReconnect.isPending) return + if (teardownWaitJob?.isActive == true) return // Time out a hung CONNECTING attempt FIRST — before the last-known-ring guard below — // so it also covers a first-ever pairing (no stored ring yet). The official QRing app // arms this timeout on every connect (mTimeoutRunnable). Without it, a first pair that @@ -482,6 +538,11 @@ class RingBLEClient( RingConnectionState.DISCONNECTED, RingConnectionState.FAILED, RingConnectionState.IDLE -> connectLastKnownInternal() + RingConnectionState.RECONNECTING -> { + // The short timer handles failed attempts; this advances a recovery scan that + // found nothing for a whole watchdog interval, preserving the retry cap. + if (crpReconnect.isRecovering || teardownScanAddress != null) connectLastKnownInternal() + } // CONNECTING is handled above (before the last-known-ring guard). else -> { // SCANNING: a pairing scan proceeds untouched, but a reconnect scan that @@ -499,11 +560,13 @@ class RingBLEClient( } /** - * Abort a stalled connect attempt that has no stored ring to retry (a first pairing that - * hung). Tears the half-open GATT down and surfaces an error so the pairing UI leaves the - * "Connecting…" state instead of spinning forever. + * Abort an unusable connection setup and tear its half-open GATT down. A known CRP ring + * gets the vendor's delayed recovery even if no further disconnect callback arrives; + * first pairing and other families retain their existing failure behavior. */ private fun failConnectAttempt(reason: String) { + crpReconnect.cancel() + cancelTeardownConnect() val gatt = bluetoothGatt bluetoothGatt = null activeDriver?.connectionDidEnd() @@ -511,8 +574,11 @@ class RingBLEClient( subscriptionGate = null resetOpQueue() if (gatt != null) { - try { gatt.disconnect() } catch (_: Exception) {} - closeGattQuietly(gatt) + if (activeCoordinator?.deviceType == RingDeviceType.CRP) disconnectCrpGatt(gatt) + else { + try { gatt.disconnect() } catch (_: Exception) {} + closeGattQuietly(gatt) + } } releaseConnectionOwnership() connectingStartedAt = 0 @@ -520,6 +586,7 @@ class RingBLEClient( PulseEventBus.publishBlocking( PulseEvent.DeviceStateChanged(RingConnectionState.DISCONNECTED, null) ) + scheduleCrpRecovery() } /** @@ -534,6 +601,9 @@ class RingBLEClient( // Lifecycle/worker teardown is intentional. Leave genuine link-loss callbacks eligible // for watchdog recovery, but do not let this client undo an explicit transient teardown. watchdogReconnectPaused = true + crpReconnect.cancel() + cancelTeardownConnect() + reconnectScanPending = false ownershipRetryJob?.cancel(); ownershipRetryJob = null val shouldPublishDisconnect = _state.value.connectionState != RingConnectionState.DISCONNECTED @@ -544,14 +614,16 @@ class RingBLEClient( activeDriver?.connectionDidEnd() writeChar = null; commandChar = null; notifyChars.clear(); batteryChar = null resetOpQueue() - if (gatt != null) { + if (gatt != null && activeCoordinator?.deviceType == RingDeviceType.CRP) { + disconnectCrpGatt(gatt) + } else if (gatt != null) { try { gatt.disconnect() } catch (_: Exception) {} // The official QRing app waits ~500 ms between disconnect() and close() so // the stack finishes the LL teardown before the client handle disappears. scope.launch { delay(GATT_CLOSE_DELAY_MS) closeGattQuietly(gatt) - releaseConnectionOwnership() + if (bluetoothGatt == null) releaseConnectionOwnership() } } else { releaseConnectionOwnership() @@ -605,6 +677,67 @@ class RingBLEClient( try { gatt.close() } catch (_: Exception) {} } + private fun disconnectCrpGatt(gatt: BluetoothGatt) { + recordDiagnostic("CRP physical teardown started") + CRPTeardownMonitor.get(context).disconnect(gatt) { + closeGattQuietly(gatt) + recordDiagnostic("CRP previous GATT closed") + // A close for the old ring must not release a newer connection's ownership. + if (bluetoothGatt == null) releaseConnectionOwnership() + } + } + + private fun cancelTeardownConnect() { + teardownWaitEpoch.incrementAndGet() + teardownWaitJob?.cancel() + teardownWaitJob = null + teardownWaitAddress = null + teardownScanAddress = null + } + + private fun waitForCrpTeardown( + monitor: CRPTeardownMonitor, + target: BluetoothDevice, + deviceType: RingDeviceType?, + selectedModelID: String?, + advertisedName: String?, + scanDetectedType: RingDeviceType?, + ) { + if (teardownWaitJob?.isActive == true && teardownWaitAddress == target.address) return + cancelTeardownConnect() + crpReconnect.cancelPending() + val epoch = teardownWaitEpoch.get() + recordDiagnostic("CRP waiting for previous physical disconnect") + updateState { copy(connectionState = RingConnectionState.RECONNECTING) } + PulseEventBus.publishBlocking( + PulseEvent.DeviceStateChanged(RingConnectionState.RECONNECTING, target.address) + ) + teardownWaitAddress = target.address + val job = scope.launch(start = CoroutineStart.LAZY) { + val ready = monitor.gate.await(target.address) + if (epoch != teardownWaitEpoch.get()) return@launch + teardownWaitJob = null + teardownWaitAddress = null + if (watchdogReconnectPaused || prefs.getBoolean(USER_DISCONNECTED_KEY, false) || + !bluetoothAdapter.isEnabled || !hasPermissions() || + synchronized(forgetLock) { forgetPending || forgetFinalizing }) { + finishCrpRecovery() + return@launch + } + if (ready) { + recordDiagnostic("CRP teardown finished; resuming connection") + beginConnect(target, deviceType, selectedModelID, advertisedName, scanDetectedType) + } else { + recordDiagnostic("CRP teardown confirmation missing; requiring fresh advertisement") + teardownScanAddress = target.address + startScanningInternal() + reconnectScanPending = true + } + } + teardownWaitJob = job + job.start() + } + /** * Explicit "Forget Ring" action. Mirrors the official app: send the ring-side * UNBOND (0x4B 05) and wait for the ring's UNBOND_ACK before tearing down, so @@ -633,6 +766,8 @@ class RingBLEClient( forgetFinalizing = false forgetRequestGeneration = forgetGeneration } + crpReconnect.cancel() + cancelTeardownConnect() // Clear the known-ring id up front so the watchdog/auto-reconnect can't grab // the ring back during or after the unbind window. prefs.edit() @@ -779,6 +914,23 @@ class RingBLEClient( ) { if (watchdogReconnectPaused) return if (synchronized(forgetLock) { forgetPending || forgetFinalizing }) return + // A hard reconnect can also replace a still-live CRP handle. Start its physical + // teardown before checking the shared gate, rather than reopening against that link. + if (activeCoordinator?.deviceType == RingDeviceType.CRP) { + bluetoothGatt?.let { old -> + bluetoothGatt = null + activeDriver?.connectionDidEnd() + resetOpQueue() + disconnectCrpGatt(old) + } + } + val teardownMonitor = CRPTeardownMonitor.current() + if (teardownMonitor?.gate?.isPending(target.address) == true) { + waitForCrpTeardown(teardownMonitor, target, deviceType, selectedModelID, + advertisedName, scanDetectedType) + return + } + cancelTeardownConnect() if (!claimConnectionOwnership()) { Log.i("RingBLEClient", "Connection skipped: another PulseLoop BLE client owns GATT") updateState { copy(connectionState = RingConnectionState.DISCONNECTED) } @@ -792,6 +944,7 @@ class RingBLEClient( } return } + crpReconnect.cancelPending() ownershipRetryJob = null scanner?.stopScan(scanCallback) // Close any stale GATT from a previous (now-dead) connection before opening a new @@ -1304,16 +1457,21 @@ class RingBLEClient( val matchedModel = com.pulseloop.wearables.WearableModel.modelForAdvertisedName(name) discoveredPeripherals[device.address] = device + val teardownScan = device.address == teardownScanAddress + if (teardownScan && CRPTeardownMonitor.current()?.gate + ?.freshAdvertisement(device.address, result.timestampNanos) == false) return + // Reconnect scan: the known ring is advertising again — grab it now. - if (reconnectScanPending && device.address == lastKnownIdentifier) { + if (reconnectScanPending && (teardownScan || device.address == lastKnownIdentifier)) { + teardownScanAddress = null reconnectScanPending = false sightlessScans = 0 scanner?.stopScan(this) connectingName = name beginConnect( device, - matchedType ?: lastKnownDeviceType, - selectedModelID = lastKnownWearableModelID, + matchedType ?: lastKnownDeviceType ?: activeCoordinator?.deviceType, + selectedModelID = lastKnownWearableModelID ?: _state.value.activeWearableModelID, advertisedName = name, scanDetectedType = matchedType, ) @@ -1405,6 +1563,13 @@ class RingBLEClient( failConnectAttempt("Service discovery failed (GATT $status)") return } + // A manual disconnect/reconnect can reuse an Android link whose ATT channel is + // already gone. The October 17 capture returns GATT_SUCCESS with an empty cache in + // both reproductions. This is failed discovery, not proof that the ring lacks fdd3. + if (gatt.services.isEmpty() && activeCoordinator?.deviceType == RingDeviceType.CRP) { + failConnectAttempt("Ring service discovery returned no services") + return + } // Log all discovered service UUIDs for diagnostics val serviceUuids = gatt.services.map { it.uuid.toString() } @@ -1868,14 +2033,8 @@ class RingBLEClient( // Ignore late callbacks from a GATT we already superseded during a reconnect // (we close the old handle in beginConnect). Acting on them would clobber the // CONNECTING state of the fresh attempt with a spurious DISCONNECTED. - if (bluetoothGatt != null && gatt !== bluetoothGatt) { - closeGattQuietly(gatt) - return - } - // Duplicate callback for a GATT we already tore down (bluetoothGatt is null - // after teardown): don't re-publish DISCONNECTED or re-close. - if (bluetoothGatt == null && - _state.value.connectionState == RingConnectionState.DISCONNECTED) { + if (gatt !== bluetoothGatt) { + if (bluetoothGatt != null) closeGattQuietly(gatt) return } resetOpQueue() @@ -1885,8 +2044,8 @@ class RingBLEClient( // Release the dead client immediately (the link is already down, so no // disconnect/delay needed): refresh the GATT cache and close, matching the // official QRing app's teardown. Holding the old handle for autoConnect is - // what left zombie clients in the stack — the watchdog now owns reconnection - // and always starts from a fresh connectGatt. + // what left zombie clients in the stack — both CRP's short retry and the watchdog + // start from a fresh connectGatt. bluetoothGatt = null writeChar = null; commandChar = null; notifyChars.clear(); batteryChar = null subscriptionGate = null @@ -1908,20 +2067,46 @@ class RingBLEClient( }, ) } + scheduleCrpRecovery() + } + + private fun scheduleCrpRecovery() { + if (!crpReconnect.schedule() && crpReconnect.isRecovering && !crpReconnect.isPending) { + finishCrpRecovery() + } + } + + private fun finishCrpRecovery() { + crpReconnect.cancel() + cancelTeardownConnect() + if (reconnectScanPending) scanner?.stopScan(scanCallback) + reconnectScanPending = false + if (_state.value.connectionState == RingConnectionState.RECONNECTING) { + updateState { copy(connectionState = RingConnectionState.DISCONNECTED) } + PulseEventBus.publishBlocking( + PulseEvent.DeviceStateChanged(RingConnectionState.DISCONNECTED, null) + ) + } } /** Tear down this client. Returns true only if it owned PulseLoop's process-wide GATT slot. */ fun destroy(): Boolean { + watchdogReconnectPaused = true + crpReconnect.cancel() + cancelTeardownConnect() watchdogJob?.cancel() ownershipRetryJob?.cancel(); ownershipRetryJob = null stopKeepalive() scanner?.stopScan(scanCallback) activeDriver?.connectionDidEnd() - // The scope is about to die, so the graceful delayed close in disconnect() - // would never run — tear the GATT down synchronously instead. + // The client scope is about to die. CRP close lives in the shared process monitor so + // a foreground client can wait for it; other families still close synchronously here. bluetoothGatt?.let { gatt -> - try { gatt.disconnect() } catch (_: Exception) {} - closeGattQuietly(gatt) + if (activeCoordinator?.deviceType == RingDeviceType.CRP) disconnectCrpGatt(gatt) + else { + try { gatt.disconnect() } catch (_: Exception) {} + closeGattQuietly(gatt) + } } bluetoothGatt = null val releasedConnection = releaseConnectionOwnership() diff --git a/app/src/main/java/com/pulseloop/ui/PulseLoopApp.kt b/app/src/main/java/com/pulseloop/ui/PulseLoopApp.kt index 6bb2c181..ec098ca1 100644 --- a/app/src/main/java/com/pulseloop/ui/PulseLoopApp.kt +++ b/app/src/main/java/com/pulseloop/ui/PulseLoopApp.kt @@ -199,7 +199,7 @@ fun PulseLoopApp() { // process restart — the live GATT is gone, so the views would otherwise show a // false "Connected". Reset until a real connection re-confirms it. db.deviceDao().current()?.let { dev -> - if (dev.stateRaw == "CONNECTED" || dev.stateRaw == "CONNECTING") { + if (dev.stateRaw in setOf("CONNECTED", "CONNECTING", "RECONNECTING")) { db.deviceDao().upsert(dev.copy(stateRaw = "DISCONNECTED", updatedAt = System.currentTimeMillis())) } } diff --git a/app/src/main/java/com/pulseloop/ui/components/AppChrome.kt b/app/src/main/java/com/pulseloop/ui/components/AppChrome.kt index e19892f2..413a0d8e 100644 --- a/app/src/main/java/com/pulseloop/ui/components/AppChrome.kt +++ b/app/src/main/java/com/pulseloop/ui/components/AppChrome.kt @@ -98,7 +98,8 @@ fun ConnectionStatusPill( val label = when (state) { RingConnectionState.CONNECTED -> if (batteryPercent != null && batteryPercent > 0) "Connected · $batteryPercent%" else "Connected" - RingConnectionState.CONNECTING, RingConnectionState.RECONNECTING -> "Connecting…" + RingConnectionState.CONNECTING -> "Connecting…" + RingConnectionState.RECONNECTING -> "Reconnecting…" RingConnectionState.FAILED -> "Sync failed" else -> "Disconnected" } diff --git a/app/src/main/java/com/pulseloop/ui/components/DeviceHeroStatus.kt b/app/src/main/java/com/pulseloop/ui/components/DeviceHeroStatus.kt index b3b23d03..c3010525 100644 --- a/app/src/main/java/com/pulseloop/ui/components/DeviceHeroStatus.kt +++ b/app/src/main/java/com/pulseloop/ui/components/DeviceHeroStatus.kt @@ -60,9 +60,12 @@ data class DeviceHeroStatus( RingConnectionState.CONNECTED -> { statusLine = "Connected"; statusTint = PulseColors.success } - RingConnectionState.CONNECTING, RingConnectionState.RECONNECTING -> { + RingConnectionState.CONNECTING -> { statusLine = "Connecting…"; statusTint = PulseColors.warning } + RingConnectionState.RECONNECTING -> { + statusLine = "Reconnecting…"; statusTint = PulseColors.warning + } RingConnectionState.SCANNING -> { statusLine = "Searching…"; statusTint = PulseColors.warning } diff --git a/app/src/test/java/com/pulseloop/ring/CRPReconnectTest.kt b/app/src/test/java/com/pulseloop/ring/CRPReconnectTest.kt new file mode 100644 index 00000000..aa27aac4 --- /dev/null +++ b/app/src/test/java/com/pulseloop/ring/CRPReconnectTest.kt @@ -0,0 +1,140 @@ +package com.pulseloop.ring + +import kotlinx.coroutines.ExperimentalCoroutinesApi +import kotlinx.coroutines.test.advanceTimeBy +import kotlinx.coroutines.test.runCurrent +import kotlinx.coroutines.test.runTest +import org.junit.Assert.* +import org.junit.Test + +@OptIn(ExperimentalCoroutinesApi::class) +class CRPReconnectTest { + @Test + fun `five second connection failure retries three seconds later instead of next watchdog tick`() = runTest { + val events = mutableListOf>() + val recovery = CRPReconnect( + scope = this, + canRetry = { true }, + onPending = { events += testScheduler.currentTime to "reconnecting" }, + onRetry = { events += testScheduler.currentTime to "scan" }, + onAbandoned = { fail("retry is eligible") }, + ) + // Replay the failed first attempt in the October 17 capture. + advanceTimeBy(5_000) + assertTrue(recovery.schedule()) + runCurrent() + assertTrue(recovery.isPending) + advanceTimeBy(2_999) + runCurrent() + assertEquals(listOf(5_000L to "reconnecting"), events) + advanceTimeBy(1) + runCurrent() + assertEquals(listOf(5_000L to "reconnecting", 8_000L to "scan"), events) + assertFalse(recovery.isPending) + assertTrue("scan remains part of recovery", recovery.isRecovering) + recovery.cancel() + } + + @Test + fun `duplicate scheduling and watchdog checks cannot dispatch a second attempt`() = runTest { + var attempts = 0 + val recovery = CRPReconnect(this, { true }, {}, { attempts++ }, {}) + assertTrue(recovery.schedule()) + runCurrent() + advanceTimeBy(1_000) + assertFalse(recovery.schedule()) + assertTrue("watchdog must yield to pending retry", recovery.isPending) + advanceTimeBy(2_000) + runCurrent() + assertEquals(1, attempts) + advanceTimeBy(15_000) + runCurrent() + assertEquals("timer is one-shot", 1, attempts) + recovery.cancel() + } + + @Test + fun `cancellation invalidates a timer already ready on the dispatcher`() = runTest { + var attempts = 0 + val recovery = CRPReconnect(this, { true }, {}, { attempts++ }, {}) + recovery.schedule() + runCurrent() + advanceTimeBy(3_000) // Deadline reached, continuation has not run yet. + recovery.cancel() + runCurrent() + assertEquals(0, attempts) + assertFalse(recovery.isPending) + assertFalse(recovery.isRecovering) + } + + @Test + fun `a superseding attempt invalidates the old timer without ending recovery`() = runTest { + var attempts = 0 + val recovery = CRPReconnect(this, { true }, {}, { attempts++ }, {}) + recovery.schedule() + runCurrent() + advanceTimeBy(3_000) + recovery.cancelPending() + runCurrent() + assertEquals(0, attempts) + assertTrue(recovery.isRecovering) + // If the superseding attempt fails, its new deadline must win over the old one. + assertTrue(recovery.schedule()) + runCurrent() + advanceTimeBy(2_999) + runCurrent() + assertEquals(0, attempts) + advanceTimeBy(1) + runCurrent() + assertEquals(1, attempts) + recovery.cancel() + } + + @Test + fun `ineligible recovery never schedules or changes presentation`() = runTest { + val recovery = CRPReconnect(this, { false }, { fail("must stay disconnected") }, + { fail("must not connect") }, { fail("nothing was scheduled") }) + assertFalse(recovery.schedule()) + advanceTimeBy(15_000) + runCurrent() + assertFalse(recovery.isPending) + assertFalse(recovery.isRecovering) + } + + @Test + fun `eligibility revoked before dispatch abandons recovery without a connection attempt`() = runTest { + var eligible = true + var abandoned = 0 + val recovery = CRPReconnect(this, { eligible }, {}, + { fail("eligibility was revoked") }, { abandoned++ }) + recovery.schedule() + runCurrent() + eligible = false + advanceTimeBy(3_000) + runCurrent() + assertEquals(1, abandoned) + assertFalse(recovery.isPending) + assertFalse(recovery.isRecovering) + } + + @Test + fun `repeated cancellation permits only a fresh recovery`() = runTest { + var attempts = 0 + val recovery = CRPReconnect(this, { true }, {}, { attempts++ }, {}) + repeat(4) { + recovery.schedule() + runCurrent() + advanceTimeBy(1_000) + recovery.cancel() + advanceTimeBy(3_000) + runCurrent() + } + assertEquals(0, attempts) + recovery.schedule() + runCurrent() + advanceTimeBy(3_000) + runCurrent() + assertEquals(1, attempts) + recovery.cancel() + } +} diff --git a/app/src/test/java/com/pulseloop/ring/CRPTeardownGateTest.kt b/app/src/test/java/com/pulseloop/ring/CRPTeardownGateTest.kt new file mode 100644 index 00000000..26a17b97 --- /dev/null +++ b/app/src/test/java/com/pulseloop/ring/CRPTeardownGateTest.kt @@ -0,0 +1,192 @@ +package com.pulseloop.ring + +import kotlinx.coroutines.ExperimentalCoroutinesApi +import kotlinx.coroutines.async +import kotlinx.coroutines.test.advanceTimeBy +import kotlinx.coroutines.test.runCurrent +import kotlinx.coroutines.test.runTest +import org.junit.Assert.* +import org.junit.Test + +@OptIn(ExperimentalCoroutinesApi::class) +class CRPTeardownGateTest { + private val ring = "ring" + + @Test + fun `quick reconnect waits for the physical teardown captured at 1472 ms`() = runTest { + val gate = CRPTeardownGate({ testScheduler.currentTime * 1_000_000 }) + val old = gate.begin(ring) + advanceTimeBy(500) + gate.gattClosed(old) + advanceTimeBy(822) // User taps reconnect 1.322 s after Disconnect. + val connect = async { gate.await(ring) } + runCurrent() + assertFalse("GATT close is not proof of physical disconnect", connect.isCompleted) + advanceTimeBy(150) + gate.aclDisconnected(ring) + runCurrent() + assertTrue(connect.await()) + assertEquals(1_472L, testScheduler.currentTime) + assertFalse(gate.isPending(ring)) + } + + @Test + fun `an early physical disconnect still waits for the old GATT close`() = runTest { + val gate = CRPTeardownGate({ testScheduler.currentTime * 1_000_000 }) + val old = gate.begin(ring) + val connect = async { gate.await(ring) } + runCurrent() + gate.aclDisconnected(ring) + runCurrent() + assertFalse(connect.isCompleted) + advanceTimeBy(500) + gate.gattClosed(old) + runCurrent() + assertTrue(connect.await()) + } + + @Test + fun `reconnecting after completed teardown adds no delay`() = runTest { + val gate = CRPTeardownGate({ testScheduler.currentTime * 1_000_000 }) + val old = gate.begin(ring) + gate.gattClosed(old) + gate.aclDisconnected(ring) + assertTrue(gate.await(ring)) + assertEquals(0L, testScheduler.currentTime) + } + + @Test + fun `a different device disconnect cannot release this ring`() = runTest { + val gate = CRPTeardownGate({ testScheduler.currentTime * 1_000_000 }) + val old = gate.begin(ring) + gate.gattClosed(old) + val connect = async { gate.await(ring) } + runCurrent() + gate.aclDisconnected("headphones") + runCurrent() + assertFalse(connect.isCompleted) + gate.aclDisconnected(ring) + runCurrent() + assertTrue(connect.await()) + } + + @Test + fun `a stale GATT close cannot release a newer teardown`() = runTest { + val gate = CRPTeardownGate({ testScheduler.currentTime * 1_000_000 }) + val stale = gate.begin(ring) + val current = gate.begin(ring) + val connect = async { gate.await(ring) } + runCurrent() + gate.gattClosed(stale) + gate.aclDisconnected(ring) + runCurrent() + assertFalse(connect.isCompleted) + gate.gattClosed(current) + runCurrent() + assertTrue(connect.await()) + } + + @Test + fun `cancelling one client's wait preserves teardown for the next client`() = runTest { + val gate = CRPTeardownGate({ testScheduler.currentTime * 1_000_000 }) + val old = gate.begin(ring) + gate.gattClosed(old) + val worker = async { gate.await(ring) } + runCurrent() + worker.cancel() + val foreground = async { gate.await(ring) } + runCurrent() + assertTrue(gate.isPending(ring)) + assertFalse(foreground.isCompleted) + gate.aclDisconnected(ring) + runCurrent() + assertTrue(foreground.await()) + assertTrue(worker.isCancelled) + } + + @Test + fun `a missing broadcast requires scanning at timeout rather than blind reconnect`() = runTest { + val gate = CRPTeardownGate({ testScheduler.currentTime * 1_000_000 }) + val old = gate.begin(ring) + gate.gattClosed(old) + val connect = async { gate.await(ring) } + runCurrent() + advanceTimeBy(5_000) + runCurrent() + assertFalse(connect.await()) + assertTrue("other clients must not bypass the missing confirmation", gate.isPending(ring)) + assertFalse(gate.await(ring)) + assertEquals(5_000L, testScheduler.currentTime) + } + + @Test + fun `cached advertisement cannot substitute for missing disconnect confirmation`() = runTest { + val gate = CRPTeardownGate({ testScheduler.currentTime * 1_000_000 }) + val old = gate.begin(ring) + gate.gattClosed(old) + assertFalse("advertising alone cannot skip the normal physical wait", + gate.freshAdvertisement(ring, 100)) + val connect = async { gate.await(ring) } + runCurrent() + advanceTimeBy(5_000) + runCurrent() + assertFalse(connect.await()) + assertFalse(gate.freshAdvertisement(ring, 4_999_000_000)) + assertFalse(gate.freshAdvertisement(ring, 5_000_000_000)) + assertTrue(gate.freshAdvertisement(ring, 5_001_000_000)) + assertTrue(gate.await(ring)) + } + + @Test + fun `fresh advertisement must also be newer than the actual GATT close`() = runTest { + val gate = CRPTeardownGate({ testScheduler.currentTime * 1_000_000 }) + val old = gate.begin(ring) + val connect = async { gate.await(ring) } + runCurrent() + advanceTimeBy(5_000) + runCurrent() + assertFalse(connect.await()) + assertFalse(gate.freshAdvertisement(ring, 5_001_000_000)) + advanceTimeBy(100) + gate.gattClosed(old) + assertFalse(gate.freshAdvertisement(ring, 5_001_000_000)) + assertTrue(gate.freshAdvertisement(ring, 5_101_000_000)) + } + + @Test + fun `a late disconnect broadcast releases the scan fallback normally`() = runTest { + val gate = CRPTeardownGate({ testScheduler.currentTime * 1_000_000 }) + val old = gate.begin(ring) + gate.gattClosed(old) + val connect = async { gate.await(ring) } + runCurrent() + advanceTimeBy(5_000) + runCurrent() + assertFalse(connect.await()) + gate.aclDisconnected(ring) + assertFalse(gate.isPending(ring)) + assertTrue(gate.await(ring)) + } + + @Test + fun `bluetooth off clears physical waits after their handles close`() = runTest { + val gate = CRPTeardownGate({ testScheduler.currentTime * 1_000_000 }) + val old = gate.begin(ring) + val connect = async { gate.await(ring) } + runCurrent() + gate.bluetoothOff() + runCurrent() + assertFalse(connect.isCompleted) + gate.gattClosed(old) + runCurrent() + assertTrue(connect.await()) // The caller still checks Bluetooth and user intent before connecting. + } + + @Test + fun `an unrelated ring is not delayed by pending CRP teardown`() = runTest { + val gate = CRPTeardownGate({ testScheduler.currentTime * 1_000_000 }) + gate.begin(ring) + assertTrue(gate.await("another-ring")) + assertTrue(gate.isPending(ring)) + } +} diff --git a/app/src/test/java/com/pulseloop/ui/components/DeviceHeroStatusTest.kt b/app/src/test/java/com/pulseloop/ui/components/DeviceHeroStatusTest.kt index 64b47f88..9904a75f 100644 --- a/app/src/test/java/com/pulseloop/ui/components/DeviceHeroStatusTest.kt +++ b/app/src/test/java/com/pulseloop/ui/components/DeviceHeroStatusTest.kt @@ -93,4 +93,15 @@ class DeviceHeroStatusTest { assertEquals(false, s.actionEnabled) } } + + @Test + fun `automatic recovery tells the user it is still reconnecting`() { + val status = DeviceHeroStatus.make( + state = RingConnectionState.RECONNECTING, connectedName = null, + knownName = "Colmi R11", batteryPercent = null, lastSyncAt = null, now = now, + ) + assertEquals("Reconnecting…", status.statusLine) + assertEquals(DeviceHeroStatus.Action.PENDING, status.action) + assertEquals(false, status.actionEnabled) + } } From af87206bfd3a13b9c5ff544fd9a3992c7ab0316e Mon Sep 17 00:00:00 2001 From: Khoa Truong Date: Tue, 20 Oct 2026 15:15:38 -0700 Subject: [PATCH 2/6] fix(crp): offer Disconnect while recovery is pending (#124) Automatic CRP recovery can scan for minutes, but RECONNECTING mapped to an inert PENDING action and Settings hid both Connect and Disconnect. Offer Disconnect in the hero card and Wearable settings; it goes through userDisconnect, which already cancels the retry timer and teardown wait. --- .../com/pulseloop/ui/components/DeviceHeroCard.kt | 13 +++++-------- .../pulseloop/ui/components/DeviceHeroStatus.kt | 11 ++++++----- .../pulseloop/ui/screens/SettingsSubScreens.kt | 15 ++++++++++++--- .../ui/components/DeviceHeroStatusTest.kt | 14 +++++++++++--- 4 files changed, 34 insertions(+), 19 deletions(-) diff --git a/app/src/main/java/com/pulseloop/ui/components/DeviceHeroCard.kt b/app/src/main/java/com/pulseloop/ui/components/DeviceHeroCard.kt index abca86a9..83eaa7b4 100644 --- a/app/src/main/java/com/pulseloop/ui/components/DeviceHeroCard.kt +++ b/app/src/main/java/com/pulseloop/ui/components/DeviceHeroCard.kt @@ -151,8 +151,9 @@ fun DeviceHeroCard( } // The connected card is purely informational and opens Wearable settings, where - // Disconnect lives. Setup/reconnect remain available here only when not connected. - if (status.action != DeviceHeroStatus.Action.DISCONNECT) { + // Disconnect lives. Setup/reconnect remain available here only when not connected, and + // automatic recovery offers Disconnect so the user can stop it (issue #124). + if (!connected) { Row(Modifier.fillMaxWidth(), horizontalArrangement = Arrangement.End) { Text( status.actionTitle, @@ -167,12 +168,8 @@ fun DeviceHeroCard( when (status.action) { DeviceHeroStatus.Action.CONNECT -> onConnect() DeviceHeroStatus.Action.SET_UP -> onSetUp() - // Unreachable today: the row above only renders when action != - // DISCONNECT, and DeviceHeroStatus produces DISCONNECT only for - // the connected state — so this is a tripwire, not a fix (the PR - // description once claimed a dead button here; there never was - // one). Exhaustiveness needs the branch, and if the render - // condition is ever relaxed it must call through, not do nothing. + // Reached while RECONNECTING: stops automatic recovery through + // the same sticky userDisconnect path as the Settings button. DeviceHeroStatus.Action.DISCONNECT -> onDisconnect() // Already inert: actionEnabled is false while a connect is in flight. DeviceHeroStatus.Action.PENDING -> {} diff --git a/app/src/main/java/com/pulseloop/ui/components/DeviceHeroStatus.kt b/app/src/main/java/com/pulseloop/ui/components/DeviceHeroStatus.kt index c3010525..65f20899 100644 --- a/app/src/main/java/com/pulseloop/ui/components/DeviceHeroStatus.kt +++ b/app/src/main/java/com/pulseloop/ui/components/DeviceHeroStatus.kt @@ -22,7 +22,7 @@ data class DeviceHeroStatus( DISCONNECT, CONNECT, SET_UP, - /** A connect attempt is in flight (connecting / reconnecting / scanning). */ + /** A connect attempt is in flight (connecting / scanning). */ PENDING, } @@ -34,8 +34,9 @@ data class DeviceHeroStatus( Action.PENDING -> "Connecting…" } - /** The action button is inert while a connection attempt is in flight, so a tap can't - * cancel or restart an in-progress (re)connect. */ + /** The action button is inert while a single connection attempt is in flight, so a tap + * can't cancel or restart it. Automatic recovery (RECONNECTING) can run for minutes, so it + * offers Disconnect instead (issue #124). */ val actionEnabled: Boolean get() = action != Action.PENDING companion object { @@ -83,9 +84,9 @@ data class DeviceHeroStatus( val syncText = lastSyncAt?.let { "Synced ${relativeShort(it, now)}" } val action = when (state) { - RingConnectionState.CONNECTED -> Action.DISCONNECT + RingConnectionState.CONNECTED, + RingConnectionState.RECONNECTING -> Action.DISCONNECT RingConnectionState.CONNECTING, - RingConnectionState.RECONNECTING, RingConnectionState.SCANNING -> Action.PENDING RingConnectionState.IDLE, RingConnectionState.DISCONNECTED, diff --git a/app/src/main/java/com/pulseloop/ui/screens/SettingsSubScreens.kt b/app/src/main/java/com/pulseloop/ui/screens/SettingsSubScreens.kt index a710a4c0..1b572b5a 100644 --- a/app/src/main/java/com/pulseloop/ui/screens/SettingsSubScreens.kt +++ b/app/src/main/java/com/pulseloop/ui/screens/SettingsSubScreens.kt @@ -1940,9 +1940,18 @@ fun WearableSettingsScreen( Spacer(Modifier.width(4.dp)) Text("Disconnect") } - } else if (bleClient?.hasLastKnownRing == true && - bleState.connectionState != RingConnectionState.RECONNECTING - ) { + } else if (bleState.connectionState == RingConnectionState.RECONNECTING) { + // Automatic recovery can scan for minutes; let the user stop it (issue #124). + Spacer(Modifier.height(12.dp)) + OutlinedButton( + onClick = { bleClient?.userDisconnect() }, + modifier = Modifier.fillMaxWidth(), + ) { + Icon(Icons.Filled.Close, null, Modifier.size(16.dp)) + Spacer(Modifier.width(4.dp)) + Text("Disconnect") + } + } else if (bleClient?.hasLastKnownRing == true) { Spacer(Modifier.height(12.dp)) OutlinedButton( onClick = { bleClient.userConnect() }, diff --git a/app/src/test/java/com/pulseloop/ui/components/DeviceHeroStatusTest.kt b/app/src/test/java/com/pulseloop/ui/components/DeviceHeroStatusTest.kt index 9904a75f..235c498f 100644 --- a/app/src/test/java/com/pulseloop/ui/components/DeviceHeroStatusTest.kt +++ b/app/src/test/java/com/pulseloop/ui/components/DeviceHeroStatusTest.kt @@ -81,7 +81,6 @@ class DeviceHeroStatusTest { fun `pending states disable the action`() { for (state in listOf( RingConnectionState.CONNECTING, - RingConnectionState.RECONNECTING, RingConnectionState.SCANNING, )) { val s = DeviceHeroStatus.make( @@ -101,7 +100,16 @@ class DeviceHeroStatusTest { knownName = "Colmi R11", batteryPercent = null, lastSyncAt = null, now = now, ) assertEquals("Reconnecting…", status.statusLine) - assertEquals(DeviceHeroStatus.Action.PENDING, status.action) - assertEquals(false, status.actionEnabled) + } + + @Test + fun `automatic recovery can be stopped with disconnect`() { + val status = DeviceHeroStatus.make( + state = RingConnectionState.RECONNECTING, connectedName = null, + knownName = "Colmi R11", batteryPercent = null, lastSyncAt = null, now = now, + ) + assertEquals(DeviceHeroStatus.Action.DISCONNECT, status.action) + assertEquals("Disconnect", status.actionTitle) + assertEquals(true, status.actionEnabled) } } From 7cad8408cf98fa60703fdb0e6035598f1918f7b3 Mon Sep 17 00:00:00 2001 From: Khoa Truong Date: Tue, 20 Oct 2026 15:16:50 -0700 Subject: [PATCH 3/6] fix(crp): stop teardown recovery for a forgotten ring (#124) A worker or coach client's queued teardown wait and fallback scan kept its own in-memory state, so a Forget on the foreground client did not stop them, and the scan reconnected any advertisement from the old address. Automatic recovery now re-reads the stored ring from prefs before resuming and in the teardown-scan branch, and stops when it no longer matches. An explicit pairing connectTo is exempt, since its target is only stored once the connection succeeds. --- .../java/com/pulseloop/ring/RingBLEClient.kt | 44 ++++++++++++++++--- 1 file changed, 39 insertions(+), 5 deletions(-) diff --git a/app/src/main/java/com/pulseloop/ring/RingBLEClient.kt b/app/src/main/java/com/pulseloop/ring/RingBLEClient.kt index ad51524c..8b558861 100644 --- a/app/src/main/java/com/pulseloop/ring/RingBLEClient.kt +++ b/app/src/main/java/com/pulseloop/ring/RingBLEClient.kt @@ -211,6 +211,8 @@ class RingBLEClient( @Volatile private var teardownWaitJob: Job? = null private var teardownWaitAddress: String? = null @Volatile private var teardownScanAddress: String? = null + /** The queued teardown wait/scan came from an explicit pairing [connectTo], not recovery. */ + @Volatile private var teardownUserInitiated = false private val teardownWaitEpoch = java.util.concurrent.atomic.AtomicLong() private val crpReconnect = CRPReconnect( @@ -350,6 +352,7 @@ class RingBLEClient( else discoveredRing?.wearableModelID ?: selectedModelID, advertisedName = discoveredRing?.name ?: target.name, scanDetectedType = detectedType, + userInitiated = true, ) } @@ -508,7 +511,13 @@ class RingBLEClient( } return } - if (lastKnownIdentifier == null) return + if (lastKnownIdentifier == null) { + // Another client forgot the ring: stop this client's recovery scan, but leave an + // explicit pairing that is waiting out a teardown alone. + if (crpReconnect.isRecovering || + (teardownScanAddress != null && !teardownUserInitiated)) finishCrpRecovery() + return + } when (_state.value.connectionState) { RingConnectionState.CONNECTED -> { if (activeCoordinator?.deviceType == RingDeviceType.JRING) { @@ -693,8 +702,18 @@ class RingBLEClient( teardownWaitJob = null teardownWaitAddress = null teardownScanAddress = null + teardownUserInitiated = false } + /** + * Automatic recovery may only resume toward the ring that is still stored. Another client + * (the foreground one) can Forget while this one waits; its in-memory state is stale here, + * so re-read the shared prefs. An explicit pairing [connectTo] is exempt: its target is not + * stored until the connection succeeds. + */ + private fun teardownTargetStillWanted(address: String, userInitiated: Boolean): Boolean = + userInitiated || address == lastKnownIdentifier + private fun waitForCrpTeardown( monitor: CRPTeardownMonitor, target: BluetoothDevice, @@ -702,6 +721,7 @@ class RingBLEClient( selectedModelID: String?, advertisedName: String?, scanDetectedType: RingDeviceType?, + userInitiated: Boolean, ) { if (teardownWaitJob?.isActive == true && teardownWaitAddress == target.address) return cancelTeardownConnect() @@ -720,16 +740,19 @@ class RingBLEClient( teardownWaitAddress = null if (watchdogReconnectPaused || prefs.getBoolean(USER_DISCONNECTED_KEY, false) || !bluetoothAdapter.isEnabled || !hasPermissions() || - synchronized(forgetLock) { forgetPending || forgetFinalizing }) { + synchronized(forgetLock) { forgetPending || forgetFinalizing } || + !teardownTargetStillWanted(target.address, userInitiated)) { finishCrpRecovery() return@launch } if (ready) { recordDiagnostic("CRP teardown finished; resuming connection") - beginConnect(target, deviceType, selectedModelID, advertisedName, scanDetectedType) + beginConnect(target, deviceType, selectedModelID, advertisedName, scanDetectedType, + userInitiated) } else { recordDiagnostic("CRP teardown confirmation missing; requiring fresh advertisement") teardownScanAddress = target.address + teardownUserInitiated = userInitiated startScanningInternal() reconnectScanPending = true } @@ -911,6 +934,8 @@ class RingBLEClient( * the attempt didn't come from a fresh scan match. [DriverReroute.shouldRerouteToJring] * needs it to tell a generic-"SMART_RING" guess apart from a confident family match. */ scanDetectedType: RingDeviceType? = null, + /** An explicit pairing [connectTo]; recovery must otherwise target the stored ring. */ + userInitiated: Boolean = false, ) { if (watchdogReconnectPaused) return if (synchronized(forgetLock) { forgetPending || forgetFinalizing }) return @@ -927,7 +952,7 @@ class RingBLEClient( val teardownMonitor = CRPTeardownMonitor.current() if (teardownMonitor?.gate?.isPending(target.address) == true) { waitForCrpTeardown(teardownMonitor, target, deviceType, selectedModelID, - advertisedName, scanDetectedType) + advertisedName, scanDetectedType, userInitiated) return } cancelTeardownConnect() @@ -939,7 +964,8 @@ class RingBLEClient( ownershipRetryJob = scope.launch { delay(PROCESS_OWNER_RETRY_MS) ownershipRetryJob = null - beginConnect(target, deviceType, selectedModelID, advertisedName, scanDetectedType) + beginConnect(target, deviceType, selectedModelID, advertisedName, scanDetectedType, + userInitiated) } } return @@ -1458,12 +1484,19 @@ class RingBLEClient( discoveredPeripherals[device.address] = device val teardownScan = device.address == teardownScanAddress + if (teardownScan && !teardownTargetStillWanted(device.address, teardownUserInitiated)) { + // Forgotten (possibly by another client) while this recovery scan was running. + finishCrpRecovery() + return + } if (teardownScan && CRPTeardownMonitor.current()?.gate ?.freshAdvertisement(device.address, result.timestampNanos) == false) return // Reconnect scan: the known ring is advertising again — grab it now. if (reconnectScanPending && (teardownScan || device.address == lastKnownIdentifier)) { + val userInitiated = teardownScan && teardownUserInitiated teardownScanAddress = null + teardownUserInitiated = false reconnectScanPending = false sightlessScans = 0 scanner?.stopScan(this) @@ -1474,6 +1507,7 @@ class RingBLEClient( selectedModelID = lastKnownWearableModelID ?: _state.value.activeWearableModelID, advertisedName = name, scanDetectedType = matchedType, + userInitiated = userInitiated, ) return } From 3331d910f021c472ecf63ceee8e3d3343518d982 Mon Sep 17 00:00:00 2001 From: Khoa Truong Date: Tue, 20 Oct 2026 15:17:45 -0700 Subject: [PATCH 4/6] fix(crp): skip the broadcast wait when the link is already down (#124) CRPTeardownMonitor.disconnect always required ACTION_ACL_DISCONNECTED. On the watchdog's CONNECTED-but-not-live and hung-CONNECTING reconnects the physical link is already gone, so no broadcast arrives: every such reconnect timed out after 5 s and then needed a fresh advertisement. Probe BluetoothManager's GATT state when the ticket opens and, if the link is not up, wait only for the old handle's close. --- .../com/pulseloop/ring/CRPTeardownGate.kt | 11 ++++++++ .../com/pulseloop/ring/CRPTeardownMonitor.kt | 18 ++++++++++++ .../com/pulseloop/ring/CRPTeardownGateTest.kt | 28 +++++++++++++++++++ 3 files changed, 57 insertions(+) diff --git a/app/src/main/java/com/pulseloop/ring/CRPTeardownGate.kt b/app/src/main/java/com/pulseloop/ring/CRPTeardownGate.kt index 095438f6..68e6cd4f 100644 --- a/app/src/main/java/com/pulseloop/ring/CRPTeardownGate.kt +++ b/app/src/main/java/com/pulseloop/ring/CRPTeardownGate.kt @@ -39,6 +39,17 @@ internal class CRPTeardownGate( } } + /** + * The OS already reported no physical link when this teardown began (a watchdog or hung + * CONNECTING reconnect), so no ACL_DISCONNECTED broadcast will follow. Only the GATT close + * remains to wait for. A stale ticket cannot release a newer teardown. + */ + fun linkAlreadyDown(ticket: Ticket) = synchronized(lock) { + if (pending[ticket.address] !== ticket) return@synchronized + ticket.disconnected = true + finishIfReady(ticket) + } + fun bluetoothOff() = synchronized(lock) { pending.values.toList().forEach { it.disconnected = true diff --git a/app/src/main/java/com/pulseloop/ring/CRPTeardownMonitor.kt b/app/src/main/java/com/pulseloop/ring/CRPTeardownMonitor.kt index ba852a3d..9aed3066 100644 --- a/app/src/main/java/com/pulseloop/ring/CRPTeardownMonitor.kt +++ b/app/src/main/java/com/pulseloop/ring/CRPTeardownMonitor.kt @@ -4,6 +4,8 @@ import android.annotation.SuppressLint import android.bluetooth.BluetoothAdapter import android.bluetooth.BluetoothDevice import android.bluetooth.BluetoothGatt +import android.bluetooth.BluetoothManager +import android.bluetooth.BluetoothProfile import android.content.BroadcastReceiver import android.content.Context import android.content.Intent @@ -27,6 +29,8 @@ import kotlinx.coroutines.launch internal class CRPTeardownMonitor private constructor(context: Context) { val gate = CRPTeardownGate(SystemClock::elapsedRealtimeNanos) private val scope = CoroutineScope(Dispatchers.Main + SupervisorJob()) + private val bluetoothManager: BluetoothManager? = + context.applicationContext.getSystemService(BluetoothManager::class.java) init { val receiver = object : BroadcastReceiver() { @@ -72,7 +76,12 @@ internal class CRPTeardownMonitor private constructor(context: Context) { /** Register the wait before disconnecting, and close even if the owning client's scope dies. */ fun disconnect(gatt: BluetoothGatt, close: () -> Unit) { + // Open the ticket before probing so a broadcast racing the probe is still recorded. val ticket = gate.begin(gatt.device.address) + if (!linkPhysicallyUp(gatt.device)) { + Log.i("RingBLEClient", "CRP link already down; waiting only for GATT close") + gate.linkAlreadyDown(ticket) + } try { gatt.disconnect() } catch (_: Exception) {} scope.launch { delay(500L) @@ -80,6 +89,15 @@ internal class CRPTeardownMonitor private constructor(context: Context) { } } + /** Unknown (no manager, missing permission) counts as up: wait for the broadcast. */ + private fun linkPhysicallyUp(device: BluetoothDevice): Boolean = try { + bluetoothManager?.getConnectionState(device, BluetoothProfile.GATT) + ?.let { it == BluetoothProfile.STATE_CONNECTED || it == BluetoothProfile.STATE_DISCONNECTING } + ?: true + } catch (_: Exception) { + true + } + companion object { @Volatile private var instance: CRPTeardownMonitor? = null fun current(): CRPTeardownMonitor? = instance diff --git a/app/src/test/java/com/pulseloop/ring/CRPTeardownGateTest.kt b/app/src/test/java/com/pulseloop/ring/CRPTeardownGateTest.kt index 26a17b97..086b9138 100644 --- a/app/src/test/java/com/pulseloop/ring/CRPTeardownGateTest.kt +++ b/app/src/test/java/com/pulseloop/ring/CRPTeardownGateTest.kt @@ -45,6 +45,34 @@ class CRPTeardownGateTest { assertTrue(connect.await()) } + @Test + fun `a link already down waits only for the GATT close, not a broadcast`() = runTest { + // Watchdog / hung-CONNECTING reconnect: the OS reports no GATT link, so no + // ACL_DISCONNECTED will arrive. Must not fall through to the 5 s timeout and scan. + val gate = CRPTeardownGate({ testScheduler.currentTime * 1_000_000 }) + val old = gate.begin(ring) + gate.linkAlreadyDown(old) + val connect = async { gate.await(ring) } + runCurrent() + assertFalse("the old handle must still close first", connect.isCompleted) + advanceTimeBy(500) + gate.gattClosed(old) + runCurrent() + assertTrue(connect.await()) + assertEquals(500L, testScheduler.currentTime) + assertFalse(gate.isPending(ring)) + } + + @Test + fun `a stale link-down probe cannot release a newer teardown`() = runTest { + val gate = CRPTeardownGate({ testScheduler.currentTime * 1_000_000 }) + val stale = gate.begin(ring) + val current = gate.begin(ring) + gate.linkAlreadyDown(stale) + gate.gattClosed(current) + assertTrue(gate.isPending(ring)) + } + @Test fun `reconnecting after completed teardown adds no delay`() = runTest { val gate = CRPTeardownGate({ testScheduler.currentTime * 1_000_000 }) From 3ff16c445e1fb05406594789e6d531cfe7387a50 Mon Sep 17 00:00:00 2001 From: Khoa Truong Date: Tue, 20 Oct 2026 15:19:14 -0700 Subject: [PATCH 5/6] fix(crp): resume a teardown fallback scan with the caller's parameters (#124) The fallback scan after a missed teardown broadcast reconnected with the scanner's match and the stored ring's model, dropping what the caller passed. A pairing that honoured the carousel over a generic SMART_RING match (#29) could be rescanned as JRING, and a pairing could pick up the previously stored ring's model. Carry deviceType, selectedModelID and scanDetectedType through to the fallback connect. A user-initiated pairing that waits on a pending teardown now shows Connecting rather than Reconnecting, and is reset to Disconnected if that wait is abandoned. --- .../java/com/pulseloop/ring/RingBLEClient.kt | 40 +++++++++++++++++-- 1 file changed, 37 insertions(+), 3 deletions(-) diff --git a/app/src/main/java/com/pulseloop/ring/RingBLEClient.kt b/app/src/main/java/com/pulseloop/ring/RingBLEClient.kt index 8b558861..10545b17 100644 --- a/app/src/main/java/com/pulseloop/ring/RingBLEClient.kt +++ b/app/src/main/java/com/pulseloop/ring/RingBLEClient.kt @@ -213,6 +213,15 @@ class RingBLEClient( @Volatile private var teardownScanAddress: String? = null /** The queued teardown wait/scan came from an explicit pairing [connectTo], not recovery. */ @Volatile private var teardownUserInitiated = false + /** The caller's connect parameters, carried through a teardown fallback scan so a pairing + * that honoured the carousel over a generic "SMART_RING" match (#29) is not rescanned as + * JRING. Null when no fallback scan is pending. */ + private data class TeardownConnect( + val deviceType: RingDeviceType?, + val selectedModelID: String?, + val scanDetectedType: RingDeviceType?, + ) + @Volatile private var teardownScanConnect: TeardownConnect? = null private val teardownWaitEpoch = java.util.concurrent.atomic.AtomicLong() private val crpReconnect = CRPReconnect( @@ -703,6 +712,7 @@ class RingBLEClient( teardownWaitAddress = null teardownScanAddress = null teardownUserInitiated = false + teardownScanConnect = null } /** @@ -726,11 +736,15 @@ class RingBLEClient( if (teardownWaitJob?.isActive == true && teardownWaitAddress == target.address) return cancelTeardownConnect() crpReconnect.cancelPending() + teardownUserInitiated = userInitiated val epoch = teardownWaitEpoch.get() recordDiagnostic("CRP waiting for previous physical disconnect") - updateState { copy(connectionState = RingConnectionState.RECONNECTING) } + // A fresh pairing is still "Connecting…" to the user; only recovery is "Reconnecting…". + val waitState = if (userInitiated) RingConnectionState.CONNECTING + else RingConnectionState.RECONNECTING + updateState { copy(connectionState = waitState) } PulseEventBus.publishBlocking( - PulseEvent.DeviceStateChanged(RingConnectionState.RECONNECTING, target.address) + PulseEvent.DeviceStateChanged(waitState, target.address) ) teardownWaitAddress = target.address val job = scope.launch(start = CoroutineStart.LAZY) { @@ -753,6 +767,7 @@ class RingBLEClient( recordDiagnostic("CRP teardown confirmation missing; requiring fresh advertisement") teardownScanAddress = target.address teardownUserInitiated = userInitiated + teardownScanConnect = TeardownConnect(deviceType, selectedModelID, scanDetectedType) startScanningInternal() reconnectScanPending = true } @@ -1495,12 +1510,27 @@ class RingBLEClient( // Reconnect scan: the known ring is advertising again — grab it now. if (reconnectScanPending && (teardownScan || device.address == lastKnownIdentifier)) { val userInitiated = teardownScan && teardownUserInitiated + val carried = if (teardownScan) teardownScanConnect else null teardownScanAddress = null teardownUserInitiated = false + teardownScanConnect = null reconnectScanPending = false sightlessScans = 0 scanner?.stopScan(this) connectingName = name + if (carried != null) { + // Resume exactly the attempt that hit the gate; a pairing must not pick up + // the previously stored ring's family or model. + beginConnect( + device, + carried.deviceType ?: matchedType, + selectedModelID = carried.selectedModelID, + advertisedName = name, + scanDetectedType = carried.scanDetectedType ?: matchedType, + userInitiated = userInitiated, + ) + return + } beginConnect( device, matchedType ?: lastKnownDeviceType ?: activeCoordinator?.deviceType, @@ -2111,11 +2141,15 @@ class RingBLEClient( } private fun finishCrpRecovery() { + // A pairing that was waiting out a teardown shows CONNECTING with no GATT yet. + val pairingWait = teardownUserInitiated && bluetoothGatt == null crpReconnect.cancel() cancelTeardownConnect() if (reconnectScanPending) scanner?.stopScan(scanCallback) reconnectScanPending = false - if (_state.value.connectionState == RingConnectionState.RECONNECTING) { + val shown = _state.value.connectionState + if (shown == RingConnectionState.RECONNECTING || + (pairingWait && shown == RingConnectionState.CONNECTING)) { updateState { copy(connectionState = RingConnectionState.DISCONNECTED) } PulseEventBus.publishBlocking( PulseEvent.DeviceStateChanged(RingConnectionState.DISCONNECTED, null) From 9d779cafec2d82ca318595951b55aab50946b959 Mon Sep 17 00:00:00 2001 From: Khoa Truong Date: Tue, 20 Oct 2026 15:19:33 -0700 Subject: [PATCH 6/6] fix(crp): read the ACL broadcast device with IntentCompat (#124) The typed getParcelableExtra(name, Class) overload is unreliable on Android 13; IntentCompat only uses it from API 34. --- .../java/com/pulseloop/ring/CRPTeardownMonitor.kt | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-) diff --git a/app/src/main/java/com/pulseloop/ring/CRPTeardownMonitor.kt b/app/src/main/java/com/pulseloop/ring/CRPTeardownMonitor.kt index 9aed3066..fe09e1c2 100644 --- a/app/src/main/java/com/pulseloop/ring/CRPTeardownMonitor.kt +++ b/app/src/main/java/com/pulseloop/ring/CRPTeardownMonitor.kt @@ -10,10 +10,10 @@ import android.content.BroadcastReceiver import android.content.Context import android.content.Intent import android.content.IntentFilter -import android.os.Build import android.os.SystemClock import android.util.Log import androidx.core.content.ContextCompat +import androidx.core.content.IntentCompat import kotlinx.coroutines.CoroutineScope import kotlinx.coroutines.Dispatchers import kotlinx.coroutines.SupervisorJob @@ -40,12 +40,10 @@ internal class CRPTeardownMonitor private constructor(context: Context) { // Older Android releases may omit EXTRA_TRANSPORT. CRP rings are LE-only. if (intent.getIntExtra(BluetoothDevice.EXTRA_TRANSPORT, BluetoothDevice.TRANSPORT_LE) != BluetoothDevice.TRANSPORT_LE) return - val device = if (Build.VERSION.SDK_INT >= 33) { - intent.getParcelableExtra(BluetoothDevice.EXTRA_DEVICE, BluetoothDevice::class.java) - } else { - @Suppress("DEPRECATION") - intent.getParcelableExtra(BluetoothDevice.EXTRA_DEVICE) - } ?: return + // IntentCompat gates the typed overload on API 34: it is buggy on 33. + val device = IntentCompat.getParcelableExtra( + intent, BluetoothDevice.EXTRA_DEVICE, BluetoothDevice::class.java, + ) ?: return if (gate.isPending(device.address)) { Log.i("RingBLEClient", "CRP physical disconnect confirmed") gate.aclDisconnected(device.address)