fix(ble): harden reconnect sessions and Android scan lifecycle#6384
Merged
jamesarich merged 3 commits intoJul 23, 2026
Conversation
Give BLE retry callers an explicit authority predicate that is checked after each failure and again after backoff. The initial attempt remains unconditional, cancellation still propagates immediately, and revocation rethrows the exact latest operation failure without consuming another attempt. Bind queued radio writes to the Meshtastic profile captured under the write mutex. When disconnect or reconnect replaces that profile, the old write stops retrying and cannot tear down the replacement session; its final stale failure is reduced to debug-level diagnostics. Add deterministic retry-revocation and remote-disconnect coverage, plus fake write-attempt accounting that includes failures occurring before a write is recorded.
Bonded reconnects previously registered a two-second targeted scan and then a separate five-second scan when the first window missed. Reconnect retries, Connections-screen discovery, and rapid device switching could therefore exceed Android's BLE scan-start quota and stall connection progress behind the platform cooldown. Use one five-second address-filtered scan for the bonded path. Return immediately when a fresh advertisement arrives, prefer that fresh device over the stale bonded handle, and retain the bounded autoConnect fallback when the scan misses. Leave the non-bonded retry policy unchanged. Update deterministic reconnect coverage to require exactly one scanner registration for both fresh-device discovery and bonded-handle fallback, including the selected-address and service filters and the bounded scan duration.
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (13)
🚧 Files skipped from review as they are similar to previous changes (11)
📝 WalkthroughWalkthroughAdds Android BLE scan-start rate limiting with rolling-window reservations, retry-after propagation, scanner failure classification, retry cancellation on stale sessions, single bounded bonded-device scans, and cooldown behavior in ChangesBLE lifecycle and scan control
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant ScannerViewModel
participant KableBleScanner
participant AndroidBleScanStartLimiter
ScannerViewModel->>KableBleScanner: start scan
KableBleScanner->>AndroidBleScanStartLimiter: reserveStart()
AndroidBleScanStartLimiter-->>KableBleScanner: success or retryAfter failure
KableBleScanner-->>ScannerViewModel: scan result or BleScanStartException
ScannerViewModel->>ScannerViewModel: apply retry-aware cooldown
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
jeremiah-k
marked this pull request as ready for review
July 23, 2026 12:24
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
core/ble/src/commonTest/kotlin/org/meshtastic/core/ble/KableBleConnectionTest.kt (1)
69-79: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winMake the reservation test assert ordering, not just invocation count.
emptyFlow()has no observable collection side effect, so this test passes even ifreserveScanStart()runs afteradvertisements(...). Record reservation and advertisement-collection events, then assert reservation occurs first.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@core/ble/src/commonTest/kotlin/org/meshtastic/core/ble/KableBleConnectionTest.kt` around lines 69 - 79, Update the test `scan reserves one platform start before collecting advertisements` to record both the scan-start reservation and advertisement collection as ordered events. Configure the scanner’s empty scan-results flow to append an advertisement-collection event when collected, append the reservation event in `BleScanStartLimiter`, and assert the reservation event precedes advertisement collection while retaining the single-reservation assertion.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@core/ble/src/commonMain/kotlin/org/meshtastic/core/ble/BleScanStartException.kt`:
- Around line 37-38: Update the public BleScanStartException class to add an
explicit two-argument constructor accepting reason and cause, delegating to the
existing constructor with retryAfter unset; preserve the current three-argument
constructor and its default-parameter behavior for source compatibility.
---
Nitpick comments:
In
`@core/ble/src/commonTest/kotlin/org/meshtastic/core/ble/KableBleConnectionTest.kt`:
- Around line 69-79: Update the test `scan reserves one platform start before
collecting advertisements` to record both the scan-start reservation and
advertisement collection as ordered events. Configure the scanner’s empty
scan-results flow to append an advertisement-collection event when collected,
append the reservation event in `BleScanStartLimiter`, and assert the
reservation event precedes advertisement collection while retaining the
single-reservation assertion.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: aff6a4d0-c5b1-4d20-b23a-a6b11e221f0b
📒 Files selected for processing (16)
core/ble/src/androidHostTest/kotlin/org/meshtastic/core/ble/AndroidBleScanStartLimiterTest.ktcore/ble/src/androidMain/kotlin/org/meshtastic/core/ble/AndroidBleScanStartLimiter.ktcore/ble/src/commonMain/kotlin/org/meshtastic/core/ble/BleRetry.ktcore/ble/src/commonMain/kotlin/org/meshtastic/core/ble/BleScanStartException.ktcore/ble/src/commonMain/kotlin/org/meshtastic/core/ble/BleScanStartLimiter.ktcore/ble/src/commonMain/kotlin/org/meshtastic/core/ble/KableBleScanner.ktcore/ble/src/commonTest/kotlin/org/meshtastic/core/ble/BleRetryTest.ktcore/ble/src/commonTest/kotlin/org/meshtastic/core/ble/KableBleConnectionTest.ktcore/ble/src/iosMain/kotlin/org/meshtastic/core/ble/BleScanStartLimiter.ios.ktcore/ble/src/jvmMain/kotlin/org/meshtastic/core/ble/BleScanStartLimiter.jvm.ktcore/network/src/commonMain/kotlin/org/meshtastic/core/network/radio/BleRadioTransport.ktcore/network/src/commonTest/kotlin/org/meshtastic/core/network/radio/BleRadioTransportReconnectCrashTest.ktcore/network/src/commonTest/kotlin/org/meshtastic/core/network/radio/BleRadioTransportTest.ktcore/testing/src/commonMain/kotlin/org/meshtastic/core/testing/FakeBle.ktfeature/connections/src/commonMain/kotlin/org/meshtastic/feature/connections/ScannerViewModel.ktfeature/connections/src/commonTest/kotlin/org/meshtastic/feature/connections/ScannerViewModelTest.kt
jeremiah-k
marked this pull request as draft
July 23, 2026 12:38
jeremiah-k
force-pushed
the
bugfix/ble-reconnect-lifecycle
branch
from
July 23, 2026 12:49
8430924 to
1e891fb
Compare
jeremiah-k
marked this pull request as ready for review
July 23, 2026 13:06
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@feature/connections/src/commonMain/kotlin/org/meshtastic/feature/connections/ScannerViewModel.kt`:
- Around line 396-412: The ScanningTooFrequently branch in the exception.reason
mapping must provide a localized quota-specific message that includes
exception.retryAfter rounded up to the required time unit. Add or reuse the
appropriate resource and pass the formatted remaining cooldown through
getStringSuspend, while preserving the existing generic message for
ApplicationRegistrationFailed and the permission message.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 6c04ddfe-2a55-451d-81c3-dc63793fe9ec
📒 Files selected for processing (10)
core/ble/src/androidHostTest/kotlin/org/meshtastic/core/ble/AndroidBleScanStartLimiterTest.ktcore/ble/src/androidMain/kotlin/org/meshtastic/core/ble/AndroidBleScanStartLimiter.ktcore/ble/src/commonMain/kotlin/org/meshtastic/core/ble/BleScanStartException.ktcore/ble/src/commonMain/kotlin/org/meshtastic/core/ble/BleScanStartLimiter.ktcore/ble/src/commonMain/kotlin/org/meshtastic/core/ble/KableBleScanner.ktcore/ble/src/commonTest/kotlin/org/meshtastic/core/ble/KableBleConnectionTest.ktcore/ble/src/iosMain/kotlin/org/meshtastic/core/ble/BleScanStartLimiter.ios.ktcore/ble/src/jvmMain/kotlin/org/meshtastic/core/ble/BleScanStartLimiter.jvm.ktfeature/connections/src/commonMain/kotlin/org/meshtastic/feature/connections/ScannerViewModel.ktfeature/connections/src/commonTest/kotlin/org/meshtastic/feature/connections/ScannerViewModelTest.kt
🚧 Files skipped from review as they are similar to previous changes (8)
- core/ble/src/iosMain/kotlin/org/meshtastic/core/ble/BleScanStartLimiter.ios.kt
- core/ble/src/commonMain/kotlin/org/meshtastic/core/ble/BleScanStartLimiter.kt
- core/ble/src/androidHostTest/kotlin/org/meshtastic/core/ble/AndroidBleScanStartLimiterTest.kt
- core/ble/src/jvmMain/kotlin/org/meshtastic/core/ble/BleScanStartLimiter.jvm.kt
- core/ble/src/androidMain/kotlin/org/meshtastic/core/ble/AndroidBleScanStartLimiter.kt
- core/ble/src/commonTest/kotlin/org/meshtastic/core/ble/KableBleConnectionTest.kt
- core/ble/src/commonMain/kotlin/org/meshtastic/core/ble/KableBleScanner.kt
- feature/connections/src/commonTest/kotlin/org/meshtastic/feature/connections/ScannerViewModelTest.kt
jeremiah-k
marked this pull request as draft
July 23, 2026 13:17
jeremiah-k
force-pushed
the
bugfix/ble-reconnect-lifecycle
branch
from
July 23, 2026 13:26
1e891fb to
53b9945
Compare
jeremiah-k
marked this pull request as ready for review
July 23, 2026 14:01
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@core/ble/src/commonMain/kotlin/org/meshtastic/core/ble/KableBleScanner.kt`:
- Around line 80-81: Move reserveScanStart() into the withTimeoutOrNull(timeout)
block in KableBleScanner so the timeout covers both reservation and
advertisement collection. Preserve the existing scan behavior while ensuring
zero or expired timeouts cannot consume a reservation slot.
In `@core/resources/src/commonMain/composeResources/values/strings.xml`:
- Line 146: Replace the bluetooth_scan_too_frequent string resource with a
plurals resource containing grammatically correct one and other forms, then
update the caller that formats this cooldown message to use the plural-resource
API with the remaining seconds value.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 15f4be60-454e-4646-96f4-93c16ed09ab1
📒 Files selected for processing (12)
.skills/compose-ui/strings-index.txtcore/ble/src/androidHostTest/kotlin/org/meshtastic/core/ble/AndroidBleScanStartLimiterTest.ktcore/ble/src/androidMain/kotlin/org/meshtastic/core/ble/AndroidBleScanStartLimiter.ktcore/ble/src/commonMain/kotlin/org/meshtastic/core/ble/BleScanStartException.ktcore/ble/src/commonMain/kotlin/org/meshtastic/core/ble/BleScanStartLimiter.ktcore/ble/src/commonMain/kotlin/org/meshtastic/core/ble/KableBleScanner.ktcore/ble/src/commonTest/kotlin/org/meshtastic/core/ble/KableBleConnectionTest.ktcore/ble/src/iosMain/kotlin/org/meshtastic/core/ble/BleScanStartLimiter.ios.ktcore/ble/src/jvmMain/kotlin/org/meshtastic/core/ble/BleScanStartLimiter.jvm.ktcore/resources/src/commonMain/composeResources/values/strings.xmlfeature/connections/src/commonMain/kotlin/org/meshtastic/feature/connections/ScannerViewModel.ktfeature/connections/src/commonTest/kotlin/org/meshtastic/feature/connections/ScannerViewModelTest.kt
🚧 Files skipped from review as they are similar to previous changes (9)
- core/ble/src/jvmMain/kotlin/org/meshtastic/core/ble/BleScanStartLimiter.jvm.kt
- core/ble/src/commonMain/kotlin/org/meshtastic/core/ble/BleScanStartLimiter.kt
- core/ble/src/iosMain/kotlin/org/meshtastic/core/ble/BleScanStartLimiter.ios.kt
- core/ble/src/commonTest/kotlin/org/meshtastic/core/ble/KableBleConnectionTest.kt
- feature/connections/src/commonTest/kotlin/org/meshtastic/feature/connections/ScannerViewModelTest.kt
- core/ble/src/commonMain/kotlin/org/meshtastic/core/ble/BleScanStartException.kt
- core/ble/src/androidMain/kotlin/org/meshtastic/core/ble/AndroidBleScanStartLimiter.kt
- core/ble/src/androidHostTest/kotlin/org/meshtastic/core/ble/AndroidBleScanStartLimiterTest.kt
- feature/connections/src/commonMain/kotlin/org/meshtastic/feature/connections/ScannerViewModel.kt
jeremiah-k
marked this pull request as draft
July 23, 2026 14:08
Android applies its BLE scan-start quota across the entire process, while reconnect discovery and Connections-screen discovery previously tracked retries independently. Their combined activity could register a sixth scan inside thirty seconds, after which some platform versions report status 6 and others leave the scan without a useful callback. Reserve native scan starts through one process-wide, mutex-protected rolling window before Kable reaches BluetoothLeScanner. Keep reservation inside the caller timeout so expired requests consume no quota and limiter contention cannot outlive the requested scan. Surface quota exhaustion as a typed failure with the remaining retry delay, retain narrow wrapping for platform-reported failures, and keep JVM and iOS behavior unchanged through no-op implementations. Keep scanner unit tests independent from Android process state through an overridable internal reservation seam. Assert that each production scan reserves exactly once and before advertisement collection, while zero-duration requests do not reserve; dedicated Android limiter tests retain ownership of singleton sharing, exhaustion, rolling expiry, and concurrent admission. Use one effective cooldown for retry admission and a dedicated localized plural message. Round displayed seconds upward so the UI never understates the enforced wait, while preserving permission-specific and generic registration-failure messages. Cover native error mapping, cooldown behavior, and fractional-duration formatting.
jeremiah-k
force-pushed
the
bugfix/ble-reconnect-lifecycle
branch
from
July 23, 2026 14:15
53b9945 to
7eba669
Compare
jeremiah-k
marked this pull request as ready for review
July 23, 2026 14:44
jamesarich
approved these changes
Jul 23, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Overview
This hardens BLE reconnect behavior across transport-session replacement and Android scan throttling.
A write captured from an old BLE session could continue retrying after that session had already been disconnected or replaced. Bonded reconnect also performed two sequential scans before falling back to Android's bonded device handle, increasing scan-start pressure during rapid device switching. Those reconnect scans share Android's process-wide scan quota with user-initiated discovery, but the callers were not coordinated.
This is a focused follow-up to #5795 and #5960. It also extends the Android scan-start failure handling from #6019 to cover the process-wide
SCAN_FAILED_SCANNING_TOO_FREQUENTLYlimit.The change makes write retry authority session-aware, reduces bonded reconnect discovery to one bounded scan, and coordinates Android scan starts through one shared rolling quota.
Review Guide
The three commits are intended to be reviewed in order:
Key Changes
Session-aware write retries
Bonded reconnect discovery
autoConnectconnection attempt.Android scan-start coordination
Testing
Added or updated coverage for:
The exact patch-equivalent source branch passed Android builds, lint and KMP smoke compilation, screenshot validation, all test shards, and the aggregate workflow check.
On-device F-Droid validation exercised repeated BLE-to-TCP and TCP-to-BLE switching. It covered both bonded reconnect outcomes:
BLE profile setup and mesh handshakes completed without Android scan-start rejection, stale database association, Room/SQLite failure, process crash, ANR, or OOM. Delayed callbacks and node work from replaced transport generations were rejected rather than applied to the active session.
Migration Notes
Summary by CodeRabbit
Summary by CodeRabbit