From 91c70cbfee2b75ac95604608794ecbdcd70df994 Mon Sep 17 00:00:00 2001 From: Jeremiah K <17190268+jeremiah-k@users.noreply.github.com> Date: Wed, 26 Aug 2026 00:10:59 +0000 Subject: [PATCH] Use Scan-Only Probes After Prolonged Bonded BLE Reconnect Failures (#6871) --- .../core/network/radio/BleDeviceLocator.kt | 131 ++++++++++ .../core/network/radio/BleRadioTransport.kt | 133 ++++------ .../core/network/radio/ScanOnlyProbeGate.kt | 81 ++++++ .../network/radio/BleRadioTransportTest.kt | 240 +++++++++++++++++- .../network/radio/ScanOnlyProbeGateTest.kt | 111 ++++++++ 5 files changed, 601 insertions(+), 95 deletions(-) create mode 100644 core/network/src/commonMain/kotlin/org/meshtastic/core/network/radio/BleDeviceLocator.kt create mode 100644 core/network/src/commonMain/kotlin/org/meshtastic/core/network/radio/ScanOnlyProbeGate.kt create mode 100644 core/network/src/commonTest/kotlin/org/meshtastic/core/network/radio/ScanOnlyProbeGateTest.kt diff --git a/core/network/src/commonMain/kotlin/org/meshtastic/core/network/radio/BleDeviceLocator.kt b/core/network/src/commonMain/kotlin/org/meshtastic/core/network/radio/BleDeviceLocator.kt new file mode 100644 index 0000000000..e48d3fda6d --- /dev/null +++ b/core/network/src/commonMain/kotlin/org/meshtastic/core/network/radio/BleDeviceLocator.kt @@ -0,0 +1,131 @@ +/* + * Copyright (c) 2026 Meshtastic LLC + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program. If not, see . + */ +@file:Suppress("TooGenericExceptionCaught") + +package org.meshtastic.core.network.radio + +import co.touchlab.kermit.Logger +import kotlinx.coroutines.CancellationException +import kotlinx.coroutines.delay +import kotlinx.coroutines.flow.first +import kotlinx.coroutines.withTimeoutOrNull +import org.meshtastic.core.ble.BleDevice +import org.meshtastic.core.ble.BleScanner +import org.meshtastic.core.ble.BluetoothRepository +import org.meshtastic.core.ble.MeshtasticBleConstants.SERVICE_UUID +import org.meshtastic.core.model.RadioNotConnectedException +import org.meshtastic.core.model.util.anonymize +import kotlin.time.Duration +import kotlin.time.Duration.Companion.seconds + +private const val SCAN_RETRY_COUNT = 3 +private val SCAN_RETRY_DELAY = 1.seconds + +/** + * Bounded scan duration used by both discovery paths in [BleDeviceLocator.findDevice]: + * - Bonded devices get one address-filtered scan before falling back to the bonded handle. + * - Non-bonded retries each use this duration. + * + * Keeping the bonded path to one scanner registration is important on Android, which throttles applications that start + * BLE scans too frequently. A single 5s window still covers multiple advertising intervals for typical power-save slots + * (~1–2s each), resolves immediately when the target advertises, and avoids consuming two scan starts per reconnect. If + * the scan misses, [BleDeviceLocator.findDevice] falls back to the bonded handle and `attemptConnection` keeps that + * patient `autoConnect` path bounded through `CONNECTION_TIMEOUT`. + */ +internal val SCAN_TIMEOUT = 5.seconds + +/** + * Locates the BLE device for one transport address: bonded-handle lookup with a single fresh-advertisement scan, or + * retried bounded scans for non-bonded addresses. + * + * Extracted from [BleRadioTransport] (unchanged behavior) so the transport class stays within detekt's LargeClass + * budget while sibling reconnect fixes continue to grow it. + */ +internal class BleDeviceLocator( + private val scanner: BleScanner, + private val bluetoothRepository: BluetoothRepository, + private val address: String, +) { + /** Robustly finds the device. Checks bonded devices, preferring a fresh scan result when available. */ + @Suppress("ReturnCount") + internal suspend fun findDevice(): BleDevice { + val bondedDevice = + bluetoothRepository.state.value.bondedDevices.firstOrNull { it.address.equals(address, ignoreCase = true) } + + if (bondedDevice != null) { + // Use one bounded, address-filtered scan. Splitting this into a short scan plus an escalated scan consumed + // two Android scanner registrations per reconnect and could hit SCAN_FAILED_SCANNING_TOO_FREQUENTLY when a + // user switched devices while the reconnect policy and the Connections screen were also scanning. + Logger.i { "[${address.anonymize()}] Bonded device found; scanning once for a fresh advertisement" } + scanForFreshDevice(SCAN_TIMEOUT)?.let { + Logger.i { "[${address.anonymize()}] Fresh advertisement found; using scanned device" } + return it + } + + // If the scan misses, fall back to the bonded handle. Bonded-only devices have no fresh advertisement, so + // Kable uses autoConnect=true and Android can patiently wait for the device to advertise again. + // This remains bounded by CONNECTION_TIMEOUT in connectAndAwait(), after which BleReconnectPolicy owns + // retry/backoff. + Logger.w { + "[${address.anonymize()}] No fresh advertisement within $SCAN_TIMEOUT; " + + "falling back to bonded handle for bounded autoConnect" + } + return bondedDevice + } + + // Non-bonded path: preserve existing retry behavior (SCAN_RETRY_COUNT attempts at SCAN_TIMEOUT). + Logger.i { "[${address.anonymize()}] Device not found in bonded list, scanning" } + repeat(SCAN_RETRY_COUNT) { attempt -> + scanForFreshDevice(SCAN_TIMEOUT)?.let { + return it + } + if (attempt < SCAN_RETRY_COUNT - 1) { + delay(SCAN_RETRY_DELAY) + } + } + throw RadioNotConnectedException("Device not found at address ${address.anonymize()}") + } + + /** + * Performs a single BLE scan attempt for the selected [address] and returns the first matching [BleDevice], or null + * if the scan times out or fails. + * + * One scan attempt only — no retry, no backoff. Both bonded and non-bonded paths in [findDevice] share this + * primitive so retry policy stays centralized: + * - Bonded: one address-filtered [SCAN_TIMEOUT] attempt before [findDevice] returns the bonded handle. + * - Non-bonded: [SCAN_RETRY_COUNT] attempts at [SCAN_TIMEOUT] with [SCAN_RETRY_DELAY] between attempts. + * + * The outer [withTimeoutOrNull] is binding: the scanner receives [timeout] as a hint, but this coroutine resumes on + * its own schedule regardless of when (or whether) the scanner honors it. + * + * [CancellationException] is rethrown — coroutine cancellation must never be swallowed. + */ + internal suspend fun scanForFreshDevice(timeout: Duration): BleDevice? = try { + withTimeoutOrNull(timeout) { + // Pass both service UUID and address; the scanner picks whichever filter the platform can honour + // (address natively on Android, service UUID elsewhere) and narrows to the address itself. + scanner.scan(timeout = timeout, serviceUuid = SERVICE_UUID, address = address).first { + it.address.equals(address, ignoreCase = true) + } + } + } catch (e: CancellationException) { + throw e + } catch (e: Exception) { + Logger.v(e) { "[${address.anonymize()}] Scan failed (timeout=$timeout)" } + null + } +} diff --git a/core/network/src/commonMain/kotlin/org/meshtastic/core/network/radio/BleRadioTransport.kt b/core/network/src/commonMain/kotlin/org/meshtastic/core/network/radio/BleRadioTransport.kt index f61e3503b4..688b0b823b 100644 --- a/core/network/src/commonMain/kotlin/org/meshtastic/core/network/radio/BleRadioTransport.kt +++ b/core/network/src/commonMain/kotlin/org/meshtastic/core/network/radio/BleRadioTransport.kt @@ -64,12 +64,9 @@ import org.meshtastic.core.repository.RadioInterfaceService import org.meshtastic.core.repository.RadioTransport import org.meshtastic.core.repository.RadioTransportCallback import kotlin.concurrent.Volatile -import kotlin.time.Duration import kotlin.time.Duration.Companion.milliseconds import kotlin.time.Duration.Companion.seconds -private const val SCAN_RETRY_COUNT = 3 -private val SCAN_RETRY_DELAY = 1.seconds private val CONNECTION_TIMEOUT = 15.seconds /** @@ -83,18 +80,6 @@ private val CONNECTION_TIMEOUT = 15.seconds */ private val HEARTBEAT_DRAIN_DELAY = 200.milliseconds -/** - * Bounded scan duration used by both discovery paths in [findDevice]: - * - Bonded devices get one address-filtered scan before falling back to the bonded handle. - * - Non-bonded retries each use this duration. - * - * Keeping the bonded path to one scanner registration is important on Android, which throttles applications that start - * BLE scans too frequently. A single 5s window still covers multiple advertising intervals for typical power-save slots - * (~1–2s each), resolves immediately when the target advertises, and avoids consuming two scan starts per reconnect. If - * the scan misses, [findDevice] falls back to the bonded handle and [attemptConnection] keeps that patient - * `autoConnect` path bounded through [CONNECTION_TIMEOUT]. - */ -internal val SCAN_TIMEOUT = 5.seconds private val GATT_CLEANUP_TIMEOUT = 5.seconds private val BLE_WRITE_OPERATION_TIMEOUT = 10.seconds private const val BLE_MAX_PENDING_WRITES = 4 @@ -247,6 +232,16 @@ class BleRadioTransport( // recovering (issue #6685). This gate decides when a long failure streak has earned one cache refresh. private val gattCacheInvalidationGate = GattCacheInvalidationGate() + // For the same reason, a bonded radio that stays away would otherwise keep paying the full bonded-fallback price + // (a GATT open against Android's stale-GATT window plus up to CONNECTION_TIMEOUT) on every retry. This gate makes + // most long-streak attempts cheap scan-only probes while periodically yielding to bonded autoConnect as a self-heal + // when Android scanning is unavailable. + private val scanOnlyProbeGate = ScanOnlyProbeGate() + + // Device discovery (bonded-first lookup + bounded scans) lives in BleDeviceLocator so this class stays within + // detekt's LargeClass budget. + private val deviceLocator = BleDeviceLocator(scanner, bluetoothRepository, address) + private val heartbeatSender = HeartbeatSender( // HeartbeatSender owns rejection severity. Suppress the generic offline-admission warning here so one @@ -265,75 +260,6 @@ class BleRadioTransport( // --- Connection & Discovery Logic --- - /** Robustly finds the device. Checks bonded devices, preferring a fresh scan result when available. */ - @Suppress("ReturnCount") - private suspend fun findDevice(): BleDevice { - val bondedDevice = - bluetoothRepository.state.value.bondedDevices.firstOrNull { it.address.equals(address, ignoreCase = true) } - - if (bondedDevice != null) { - // Use one bounded, address-filtered scan. Splitting this into a short scan plus an escalated scan consumed - // two Android scanner registrations per reconnect and could hit SCAN_FAILED_SCANNING_TOO_FREQUENTLY when a - // user switched devices while the reconnect policy and the Connections screen were also scanning. - Logger.i { "[${address.anonymize()}] Bonded device found; scanning once for a fresh advertisement" } - scanForFreshDevice(SCAN_TIMEOUT)?.let { - Logger.i { "[${address.anonymize()}] Fresh advertisement found; using scanned device" } - return it - } - - // If the scan misses, fall back to the bonded handle. Bonded-only devices have no fresh advertisement, so - // Kable uses autoConnect=true and Android can patiently wait for the device to advertise again. - // This remains bounded by CONNECTION_TIMEOUT in connectAndAwait(), after which BleReconnectPolicy owns - // retry/backoff. - Logger.w { - "[${address.anonymize()}] No fresh advertisement within $SCAN_TIMEOUT; " + - "falling back to bonded handle for bounded autoConnect" - } - return bondedDevice - } - - // Non-bonded path: preserve existing retry behavior (SCAN_RETRY_COUNT attempts at SCAN_TIMEOUT). - Logger.i { "[${address.anonymize()}] Device not found in bonded list, scanning" } - repeat(SCAN_RETRY_COUNT) { attempt -> - scanForFreshDevice(SCAN_TIMEOUT)?.let { - return it - } - if (attempt < SCAN_RETRY_COUNT - 1) { - delay(SCAN_RETRY_DELAY) - } - } - throw RadioNotConnectedException("Device not found at address $address") - } - - /** - * Performs a single BLE scan attempt for the selected [address] and returns the first matching [BleDevice], or null - * if the scan times out or fails. - * - * One scan attempt only — no retry, no backoff. Both bonded and non-bonded paths in [findDevice] share this - * primitive so retry policy stays centralized: - * - Bonded: one address-filtered [SCAN_TIMEOUT] attempt before [findDevice] returns the bonded handle. - * - Non-bonded: [SCAN_RETRY_COUNT] attempts at [SCAN_TIMEOUT] with [SCAN_RETRY_DELAY] between attempts. - * - * The outer [withTimeoutOrNull] is binding: the scanner receives [timeout] as a hint, but this coroutine resumes on - * its own schedule regardless of when (or whether) the scanner honors it. - * - * [CancellationException] is rethrown — coroutine cancellation must never be swallowed. - */ - private suspend fun scanForFreshDevice(timeout: Duration): BleDevice? = try { - withTimeoutOrNull(timeout) { - // Pass both service UUID and address; the scanner picks whichever filter the platform can honour - // (address natively on Android, service UUID elsewhere) and narrows to the address itself. - scanner.scan(timeout = timeout, serviceUuid = SERVICE_UUID, address = address).first { - it.address.equals(address, ignoreCase = true) - } - } - } catch (e: CancellationException) { - throw e - } catch (e: Exception) { - Logger.v(e) { "[${address.anonymize()}] Scan failed (timeout=$timeout)" } - null - } - private fun connect() { connectionJob = connectionScope.launch { @@ -385,7 +311,43 @@ class BleRadioTransport( awaitPendingSessionCleanup() sessionFailed.value = false - val device = findDevice() + + // Long-streak probe: once the retry ladder has saturated, a bonded device that is still absent fails most + // full-price attempts (GATT open + connection timeout) identically. Probe by scan alone instead — but + // deliberately yield at a fixed interval so a bonded autoConnect still gets recovery opportunities when Android + // scanning is unavailable (for example Location-off gating on API 26–30 or a revoked scan permission on 31+). + // Any stable connection resets the reconnect streak, which disarms probing automatically. + val consecutiveFailures = reconnectPolicy.consecutiveFailures + val isBonded = bluetoothRepository.isBonded(address) + val shouldProbe = isBonded && scanOnlyProbeGate.shouldProbeInsteadOfBondedConnect(consecutiveFailures) + if (isBonded && consecutiveFailures >= scanOnlyProbeGate.failureThreshold && !shouldProbe) { + Logger.d { + "[${address.anonymize()}] $consecutiveFailures consecutive failures; " + + "trying periodic bonded fallback" + } + } + + val probedDevice = + if (shouldProbe) { + // Entering probe mode earns one Info line; every continuation is Debug so a days-long absence does + // not repeat the same message at Info on every retry (the same noise class the heartbeat demotion + // addresses). + if (consecutiveFailures == scanOnlyProbeGate.failureThreshold) { + Logger.i { + "[${address.anonymize()}] $consecutiveFailures consecutive failures; probing by scan only" + } + } else { + Logger.d { + "[${address.anonymize()}] $consecutiveFailures consecutive failures; probing by scan only" + } + } + deviceLocator.scanForFreshDevice(SCAN_TIMEOUT) + ?: throw RadioNotConnectedException("No advertisement during scan-only probe") + } else { + null + } + + val device = probedDevice ?: deviceLocator.findDevice() bondDeviceBeforeConnect(device) @@ -404,7 +366,6 @@ class BleRadioTransport( // consumeGattCacheInvalidationRequest() is read into a val first: `||` short-circuiting must never skip // consuming the one-shot post-OTA flag. val postOtaRequested = (callback as? RadioInterfaceService)?.consumeGattCacheInvalidationRequest() == true - val consecutiveFailures = reconnectPolicy.consecutiveFailures val staleCacheSuspected = gattCacheInvalidationGate.shouldInvalidateOnAttempt(consecutiveFailures) if (postOtaRequested || staleCacheSuspected) { val triggers = buildList { diff --git a/core/network/src/commonMain/kotlin/org/meshtastic/core/network/radio/ScanOnlyProbeGate.kt b/core/network/src/commonMain/kotlin/org/meshtastic/core/network/radio/ScanOnlyProbeGate.kt new file mode 100644 index 0000000000..04619ee9af --- /dev/null +++ b/core/network/src/commonMain/kotlin/org/meshtastic/core/network/radio/ScanOnlyProbeGate.kt @@ -0,0 +1,81 @@ +/* + * Copyright (c) 2026 Meshtastic LLC + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program. If not, see . + */ +package org.meshtastic.core.network.radio + +/** + * Decides when a bonded device's reconnect attempt should be a cheap scan-only probe instead of the full bonded-handle + * connect. + * + * Background: while a bonded radio stays away (out of range or powered off), every reconnect iteration pays the full + * bonded-fallback price — a fresh GATT open against Android's stale-GATT window plus up to [CONNECTION_TIMEOUT] — only + * to fail again seconds later. Field logs show that pattern repeating every ~83 s for as long as the radio is gone. + * + * Probing is not free either: autoConnect from a bonded handle can succeed without any advertisement at all, so + * skipping it on early failures would trade away legitimate reconnect chances during an ordinary out-of-range blip. + * Like [GattCacheInvalidationGate], this gate therefore arms only after the failure streak has run for *minutes* — once + * the retry ladder has saturated at its 60 s cap and demonstrably stopped working — and disarms automatically when the + * streak ends ([BleReconnectPolicy] resets its counter on any stable connection). + * + * Scan visibility is not equivalent to connectability on Android: API 26–30 can return no scan results when system + * Location is off, and API 31+ can lose `BLUETOOTH_SCAN` while a bonded `autoConnect` remains viable. To avoid making + * scan-only mode a permanent trap in either case, every [BONDED_FALLBACK_INTERVAL]th long-streak attempt deliberately + * yields to the normal bonded fallback. + * + * The gate is stateless; its answer is derived entirely from [consecutiveFailures]. + * + * @param failureThreshold consecutive reconnect failures required before attempts become scan-only probes + */ +internal class ScanOnlyProbeGate(val failureThreshold: Int = DEFAULT_FAILURE_THRESHOLD) { + + init { + require(failureThreshold > 0) { "failureThreshold must be positive, was $failureThreshold" } + } + + /** + * Returns true when the attempt about to run should probe by scan alone rather than connect from the bonded handle. + * + * A [consecutiveFailures] of zero can never reach [failureThreshold] (which is always positive), so a healthy + * connection never probes. Once armed, every [BONDED_FALLBACK_INTERVAL]th long-streak attempt returns false so a + * bonded `autoConnect` still gets periodic recovery opportunities when Android scanning is unavailable. + * + * @param consecutiveFailures failures observed *before* the attempt in progress, i.e. + * [BleReconnectPolicy.consecutiveFailures] read from inside the attempt + */ + fun shouldProbeInsteadOfBondedConnect(consecutiveFailures: Int): Boolean { + if (consecutiveFailures < failureThreshold) return false + + val longStreakAttempt = consecutiveFailures - failureThreshold + 1 + return longStreakAttempt % BONDED_FALLBACK_INTERVAL != 0 + } + + companion object { + /** + * Consecutive failures before reconnect attempts switch to cheap scan-only probes. + * + * Deliberately far above [BleReconnectPolicy.DEFAULT_FAILURE_THRESHOLD] (3), which only marks a disconnect as + * "more than a blip" for the UI. Three failures are reached about 47 s into an ordinary out-of-range gap, where + * autoConnect still deserves its chance. Six is the first count at which [computeReconnectBackoff] has been + * saturated at its 60 s cap for two consecutive cycles — the same "minutes of unbroken failure" bar + * [GattCacheInvalidationGate.DEFAULT_FAILURE_THRESHOLD] uses for stale-cache recovery, so both escalations come + * online together on the seventh attempt. + */ + const val DEFAULT_FAILURE_THRESHOLD = 6 + + /** Long-streak attempts between deliberate retries of the normal bonded-handle connection path. */ + const val BONDED_FALLBACK_INTERVAL = 5 + } +} diff --git a/core/network/src/commonTest/kotlin/org/meshtastic/core/network/radio/BleRadioTransportTest.kt b/core/network/src/commonTest/kotlin/org/meshtastic/core/network/radio/BleRadioTransportTest.kt index 23b31407c9..7cfe56cba5 100644 --- a/core/network/src/commonTest/kotlin/org/meshtastic/core/network/radio/BleRadioTransportTest.kt +++ b/core/network/src/commonTest/kotlin/org/meshtastic/core/network/radio/BleRadioTransportTest.kt @@ -28,11 +28,16 @@ import kotlinx.coroutines.CompletableDeferred import kotlinx.coroutines.CoroutineScope import kotlinx.coroutines.ExperimentalCoroutinesApi import kotlinx.coroutines.async +import kotlinx.coroutines.awaitCancellation +import kotlinx.coroutines.flow.Flow +import kotlinx.coroutines.flow.flow import kotlinx.coroutines.test.TestScope import kotlinx.coroutines.test.advanceTimeBy import kotlinx.coroutines.test.runCurrent import kotlinx.coroutines.test.runTest import kotlinx.coroutines.withTimeout +import org.meshtastic.core.ble.BleDevice +import org.meshtastic.core.ble.BleScanner import org.meshtastic.core.ble.DisconnectReason import org.meshtastic.core.ble.MeshtasticBleConstants.FROMNUM_CHARACTERISTIC import org.meshtastic.core.ble.MeshtasticBleConstants.FROMRADIO_CHARACTERISTIC @@ -53,7 +58,9 @@ import kotlin.test.assertEquals import kotlin.test.assertFalse import kotlin.test.assertNotNull import kotlin.test.assertTrue +import kotlin.time.Duration import kotlin.time.Duration.Companion.seconds +import kotlin.uuid.Uuid @OptIn(ExperimentalCoroutinesApi::class) class BleRadioTransportTest { @@ -608,15 +615,18 @@ class BleRadioTransportTest { } } - private fun bleTransportOn(scope: CoroutineScope, callback: RadioInterfaceService): BleRadioTransport = - BleRadioTransport( - scope = scope, - scanner = scanner, - bluetoothRepository = bluetoothRepository, - connectionFactory = connectionFactory, - callback = callback, - address = address, - ) + private fun bleTransportOn( + scope: CoroutineScope, + callback: RadioInterfaceService, + bleScanner: BleScanner = scanner, + ): BleRadioTransport = BleRadioTransport( + scope = scope, + scanner = bleScanner, + bluetoothRepository = bluetoothRepository, + connectionFactory = connectionFactory, + callback = callback, + address = address, + ) /** * Simulates the platform replaying a stale GATT service table: `profile()` cannot find the Meshtastic service, and @@ -898,4 +908,216 @@ class BleRadioTransportTest { bleTransport.close() } } + + /** + * Once the failure streak has saturated the retry ladder ([ScanOnlyProbeGate.DEFAULT_FAILURE_THRESHOLD]), a bonded + * radio that is still absent must stop paying the full bonded-fallback price on every retry. When the radio starts + * advertising again, the reconnect must use that fresh device instance directly rather than the stale bonded + * handle. + */ + @Test + fun `long absence reconnect uses the fresh advertisement returned by the probe`() = runTest { + val threshold = ScanOnlyProbeGate.DEFAULT_FAILURE_THRESHOLD + val bondedDevice = FakeBleDevice(address = address, name = "Bonded Handle") + val freshDevice = FakeBleDevice(address = address, name = "Fresh Advertisement") + val controllableScanner = ControllableBleScanner() + bluetoothRepository.bond(bondedDevice) + connection.service.addCharacteristic(FROMNUM_CHARACTERISTIC) + connection.service.addCharacteristic(FROMRADIO_CHARACTERISTIC) + connection.failNextN = threshold + + val bleTransport = bleTransportOn(this, service, controllableScanner) + bleTransport.start() + + try { + advanceThroughFirstScanOnlyProbe(threshold) + + assertEquals( + threshold, + connection.connectAndAwaitCalls, + "the first scan-only miss must not reach the bonded-handle connect", + ) + assertEquals( + 0, + connection.invalidateServiceCacheCalls, + "a probe miss never reaches a link, so the GATT cache path must stay untouched", + ) + + controllableScanner.advertisedDevice = freshDevice + advanceThroughCappedReconnect() + + assertEquals(threshold + 1, connection.connectAndAwaitCalls) + assertTrue( + connection.device === freshDevice, + "the recovered connect must use the fresh advertisement, not the stale bonded handle", + ) + } finally { + bleTransport.close() + } + } + + /** + * Regression guard for the threshold boundary itself: every pre-threshold attempt reaches the bonded handle, while + * the first armed probe miss must leave the exact connect count unchanged. + */ + @Test + fun `the first scan-only probe skips the bonded connect exactly at the threshold`() = runTest { + val threshold = ScanOnlyProbeGate.DEFAULT_FAILURE_THRESHOLD + val device = FakeBleDevice(address = address, name = "Bonded Device") + bluetoothRepository.bond(device) + connection.failNextN = 100 + + val bleTransport = + BleRadioTransport( + scope = this, + scanner = scanner, + bluetoothRepository = bluetoothRepository, + connectionFactory = connectionFactory, + callback = service, + address = address, + ) + bleTransport.start() + + try { + advanceTimeBy(elapsedThroughFailedBondedAttempts(threshold).inWholeMilliseconds) + runCurrent() + assertEquals( + threshold, + connection.connectAndAwaitCalls, + "every attempt through the threshold must still use the bonded fallback", + ) + + val firstProbe = computeReconnectBackoff(threshold) + BleReconnectPolicy.DEFAULT_SETTLE_DELAY + SCAN_TIMEOUT + advanceTimeBy(firstProbe.inWholeMilliseconds) + runCurrent() + assertEquals( + threshold, + connection.connectAndAwaitCalls, + "the first missed probe after the threshold must not add another bonded-handle connect", + ) + } finally { + bleTransport.close() + } + } + + /** + * The probe gate is bonded-only. A non-bonded address must keep the existing three-scan discovery sequence even + * after its reconnect failure count passes the probe threshold. + */ + @Test + fun `non bonded absence keeps full discovery retries beyond the probe threshold`() = runTest { + val threshold = ScanOnlyProbeGate.DEFAULT_FAILURE_THRESHOLD + val controllableScanner = ControllableBleScanner() + val bleTransport = bleTransportOn(this, service, controllableScanner) + bleTransport.start() + + try { + val scansPerDiscovery = 3 + val retryDelayCount = scansPerDiscovery - 1 + val discoveryAttemptCost = + BleReconnectPolicy.DEFAULT_SETTLE_DELAY + SCAN_TIMEOUT * scansPerDiscovery + 1.seconds * retryDelayCount + val throughFirstPostThresholdAttempt = + discoveryAttemptCost * (threshold + 1) + + (1..threshold).fold(Duration.ZERO) { total, failures -> total + computeReconnectBackoff(failures) } + + advanceTimeBy(throughFirstPostThresholdAttempt.inWholeMilliseconds) + runCurrent() + + assertEquals(0, connection.connectAndAwaitCalls, "a missing non-bonded device never reaches GATT connect") + assertEquals( + scansPerDiscovery * (threshold + 1), + controllableScanner.scanCalls, + "non-bonded discovery must retain all three scan attempts beyond the bonded probe threshold", + ) + } finally { + bleTransport.close() + } + } + + /** + * Android scan availability can disappear independently of bonded-connect capability. When every scan remains + * empty, scan-only mode must still yield periodically so `autoConnect` gets another chance instead of becoming + * permanently unreachable behind the probe gate. + */ + @Test + fun `prolonged scan misses periodically retry the bonded fallback`() = runTest { + val threshold = ScanOnlyProbeGate.DEFAULT_FAILURE_THRESHOLD + val device = FakeBleDevice(address = address, name = "Bonded Device") + bluetoothRepository.bond(device) + connection.failNextN = 100 + + val bleTransport = + BleRadioTransport( + scope = this, + scanner = scanner, + bluetoothRepository = bluetoothRepository, + connectionFactory = connectionFactory, + callback = service, + address = address, + ) + bleTransport.start() + + try { + val throughPeriodicFallback = + (0 until ScanOnlyProbeGate.BONDED_FALLBACK_INTERVAL).fold( + elapsedThroughFailedBondedAttempts(threshold), + ) { elapsed, offset -> + elapsed + + computeReconnectBackoff(threshold + offset) + + BleReconnectPolicy.DEFAULT_SETTLE_DELAY + + SCAN_TIMEOUT + } + advanceTimeBy(throughPeriodicFallback.inWholeMilliseconds) + runCurrent() + + assertEquals( + threshold + 1, + connection.connectAndAwaitCalls, + "the fifth long-streak attempt must retry bonded autoConnect even when scanning never sees the radio", + ) + } finally { + bleTransport.close() + } + } + + private fun TestScope.advanceThroughFirstScanOnlyProbe(threshold: Int) { + val firstProbeDone = + elapsedThroughFailedBondedAttempts(threshold) + + computeReconnectBackoff(threshold) + + BleReconnectPolicy.DEFAULT_SETTLE_DELAY + + SCAN_TIMEOUT + advanceTimeBy(firstProbeDone.inWholeMilliseconds) + runCurrent() + } + + private fun TestScope.advanceThroughCappedReconnect() { + val budget = + BleReconnectPolicy.RECONNECT_MAX_DELAY + BleReconnectPolicy.DEFAULT_SETTLE_DELAY + SCAN_TIMEOUT + 1.seconds + advanceTimeBy(budget.inWholeMilliseconds) + runCurrent() + } + + private fun elapsedThroughFailedBondedAttempts(attemptCount: Int): Duration { + require(attemptCount > 0) + val attemptCost = BleReconnectPolicy.DEFAULT_SETTLE_DELAY + SCAN_TIMEOUT + val backoffBeforeLast = + (1 until attemptCount).fold(Duration.ZERO) { total, failures -> total + computeReconnectBackoff(failures) } + return attemptCost * attemptCount + backoffBeforeLast + } +} + +private class ControllableBleScanner : BleScanner { + var advertisedDevice: BleDevice? = null + + var scanCalls: Int = 0 + private set + + override fun scan(timeout: Duration, serviceUuid: Uuid?, address: String?): Flow = flow { + scanCalls++ + advertisedDevice?.let { + emit(it) + return@flow + } + awaitCancellation() + } } diff --git a/core/network/src/commonTest/kotlin/org/meshtastic/core/network/radio/ScanOnlyProbeGateTest.kt b/core/network/src/commonTest/kotlin/org/meshtastic/core/network/radio/ScanOnlyProbeGateTest.kt new file mode 100644 index 0000000000..a0ad40a496 --- /dev/null +++ b/core/network/src/commonTest/kotlin/org/meshtastic/core/network/radio/ScanOnlyProbeGateTest.kt @@ -0,0 +1,111 @@ +/* + * Copyright (c) 2026 Meshtastic LLC + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program. If not, see . + */ +package org.meshtastic.core.network.radio + +import kotlin.test.Test +import kotlin.test.assertFailsWith +import kotlin.test.assertFalse +import kotlin.test.assertTrue +import kotlin.time.Duration +import kotlin.time.Duration.Companion.minutes + +class ScanOnlyProbeGateTest { + + @Test + fun `no probing while the failure streak is below the threshold`() { + val gate = ScanOnlyProbeGate(failureThreshold = 3) + + assertFalse(gate.shouldProbeInsteadOfBondedConnect(0), "a healthy connection must never probe") + assertFalse(gate.shouldProbeInsteadOfBondedConnect(-1), "a nonsensical count must never probe") + assertFalse(gate.shouldProbeInsteadOfBondedConnect(1), "one failure is an ordinary out-of-range blip") + assertFalse(gate.shouldProbeInsteadOfBondedConnect(2), "two failures are still below the threshold") + } + + @Test + fun `probing starts at the threshold and periodically yields to bonded fallback`() { + val gate = ScanOnlyProbeGate(failureThreshold = 3) + + assertTrue(gate.shouldProbeInsteadOfBondedConnect(3), "the threshold-th consecutive failure must arm probing") + assertTrue(gate.shouldProbeInsteadOfBondedConnect(4)) + assertTrue(gate.shouldProbeInsteadOfBondedConnect(5)) + assertTrue(gate.shouldProbeInsteadOfBondedConnect(6)) + assertFalse( + gate.shouldProbeInsteadOfBondedConnect(7), + "the fifth long-streak attempt must yield to bonded autoConnect as a scan-unavailable escape", + ) + assertTrue(gate.shouldProbeInsteadOfBondedConnect(8), "probing must resume after the periodic bonded fallback") + } + + @Test + fun `a stable reconnect policy outcome disarms probing`() { + val gate = ScanOnlyProbeGate() + val policy = BleReconnectPolicy() + repeat(ScanOnlyProbeGate.DEFAULT_FAILURE_THRESHOLD) { + policy.processOutcome(BleReconnectPolicy.Outcome.Failed(IllegalStateException("test failure"))) + } + + assertTrue(gate.shouldProbeInsteadOfBondedConnect(policy.consecutiveFailures)) + + policy.processOutcome(BleReconnectPolicy.Outcome.Disconnected(wasStable = true, wasIntentional = false)) + + assertFalse(gate.shouldProbeInsteadOfBondedConnect(policy.consecutiveFailures)) + } + + /** + * Discriminator against the stale-GATT-window cost: a probe pass replaces a full bonded-handle connect (GATT open + * plus up to the platform connection timeout) with a cheap scan miss, so it must only arm once the retry ladder has + * demonstrably stopped working — not while early attempts are still likely to succeed. + */ + @Test + fun `the default threshold is far above the reconnect policy transient-disconnect threshold`() { + assertTrue( + ScanOnlyProbeGate.DEFAULT_FAILURE_THRESHOLD > BleReconnectPolicy.DEFAULT_FAILURE_THRESHOLD, + "probing at the transient-disconnect threshold would skip legitimate autoConnect chances " + + "(probe=${ScanOnlyProbeGate.DEFAULT_FAILURE_THRESHOLD}, " + + "transient=${BleReconnectPolicy.DEFAULT_FAILURE_THRESHOLD})", + ) + } + + /** + * Pins the intent behind the default rather than the number: probing may only start after *minutes* of unbroken + * failure, mirroring [GattCacheInvalidationGate]'s staleness bar. Lower-bound math assumes every pre-probe attempt + * failed instantly after its scan window; slower failures only push the first probe later. + */ + @Test + fun `the default threshold is only reachable after minutes of unbroken failure`() { + val threshold = ScanOnlyProbeGate.DEFAULT_FAILURE_THRESHOLD + val backoff = + (1..threshold).fold(Duration.ZERO) { total, failures -> total + computeReconnectBackoff(failures) } + // One settle delay plus one missed scan window precede every failing attempt. + val elapsedBeforeFirstProbe = (BleReconnectPolicy.DEFAULT_SETTLE_DELAY + SCAN_TIMEOUT) * threshold + backoff + + assertTrue( + elapsedBeforeFirstProbe >= 3.minutes, + "probing must not be reachable inside an ordinary out-of-range gap (reached at $elapsedBeforeFirstProbe)", + ) + assertTrue( + ScanOnlyProbeGate().shouldProbeInsteadOfBondedConnect(threshold), + "the timing discriminator must correspond to an actually armed gate", + ) + } + + @Test + fun `a non-positive threshold is rejected`() { + assertFailsWith { ScanOnlyProbeGate(failureThreshold = 0) } + assertFailsWith { ScanOnlyProbeGate(failureThreshold = -1) } + } +}