Conversation
There was a problem hiding this comment.
Pull request overview
This PR updates the SP140 firmware BLE behavior toward a “single bonded peer” model, adjusting the button UX and BLE advertising/whitelist + bond management to reduce multi-device reconnection issues.
Changes:
- Removes the 20s “delete all bonds + reboot” button tier; 10s hold now triggers pairing mode (and clears bonds).
- Refactors whitelist syncing to avoid a prior NimBLE EBUSY/infinite-loop scenario by only adding missing bonded addresses.
- On untrusted sessions, attempts to delete “stale” bonds; pairing mode now proactively clears bonds before advertising openly.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
src/sp140/main.cpp |
Simplifies long-hold button handling to a single pairing-mode action and updates perf-mode gating logic. |
src/sp140/ble/ble_core.cpp |
Changes whitelist/bond handling and advertising flow to support a single-bond pairing model and avoid NimBLE whitelist EBUSY loops. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| size_t syncWhiteListFromBonds() { | ||
| clearWhiteList(); | ||
|
|
||
| // Add bonded addresses that aren't already present. Skip addresses already | ||
| // on the whitelist because NimBLE's whiteListAdd() internally calls | ||
| // whiteListRemove() first for duplicates, and whiteListRemove() fails with | ||
| // BLE_HS_EBUSY (rc=524) whenever the controller is advertising — even after | ||
| // an advertising->stop(), which is asynchronous at the HCI level. | ||
| const int bondCount = NimBLEDevice::getNumBonds(); | ||
| for (int i = 0; i < bondCount; ++i) { | ||
| NimBLEDevice::whiteListAdd(NimBLEDevice::getBondedAddress(i)); | ||
| const NimBLEAddress addr = NimBLEDevice::getBondedAddress(i); | ||
| if (!NimBLEDevice::onWhiteList(addr)) { | ||
| NimBLEDevice::whiteListAdd(addr); | ||
| } | ||
| } | ||
|
|
||
| return NimBLEDevice::getWhiteListCount(); |
There was a problem hiding this comment.
syncWhiteListFromBonds() only ever adds bonded addresses; it never removes whitelist entries for bonds that were deleted (e.g., after enterBLEPairingMode() clears bonds). With scan-filter whitelisting enabled (setScanFilter(false, true)), stale whitelist entries can still be allowed to connect, undermining the single-bond model and potentially enabling repeated reconnect/DoS until reboot. Consider explicitly clearing/pruning the whitelist to match current bonds (e.g., clear whitelist when bonds are cleared, or remove any whitelist addresses that are no longer present in the bond list using bounded retries to avoid the prior infinite loop).
| if (deviceConnected && pServer != nullptr && | ||
| connectedHandle != BLE_HS_CONN_HANDLE_NONE) { | ||
| pServer->disconnect(connectedHandle); | ||
| vTaskDelay(pdMS_TO_TICKS(100)); | ||
| } |
There was a problem hiding this comment.
enterBLEPairingMode() disconnects first, but onDisconnect() immediately calls startAdvertising(server). Since pairingModeActive isn’t set until after bond deletion, there’s a window where advertising can restart in BONDED mode with the old bond/whitelist still present, allowing the previous peer to reconnect before bonds are cleared. Consider adding a pairing-transition flag to suppress the onDisconnect advertising restart (or otherwise defer advertising) until after bonds/whitelist are cleared and pairing mode is fully active.
Introduce BLE_PAIRING_HOLD_MS (10000 ms) and replace hard-coded 10000ms checks with this constant. The new macro is used to limit the performance-mode long-hold detection and to trigger BLE pairing mode when disarmed, centralizing the pairing hold duration and removing duplicate magic numbers.
Introduce pairingModeTransitionActive to block advertising/startAdvertising during pairing state transitions. Reconcile the controller whitelist by first pruning stale entries (avoids BLE_HS_EBUSY) and then adding bonded addresses, with comments clarifying the rationale. Improve stale-bond removal to try both identity and peer addresses, remove them from the whitelist when deleted, and update the debug log to include both IDs. Also guard startAdvertising and disconnect restart paths against running during the pairing transition, and set/clear the transition flag in enterBLEPairingMode.
Refactor BLE pairing logic and centralize hold duration constant
|
closing in favor of #107 |
This pull request refactors and simplifies the BLE bonding and advertising logic, focusing on robustness and user experience improvements. The most significant changes include eliminating the manual "delete all bonds" button hold, making pairing mode always clear existing bonds, improving whitelist management to avoid controller errors, and ensuring stale bonds are cleaned up automatically. These changes make BLE pairing more reliable and user-friendly.
BLE Bonding and Pairing Mode Changes:
src/sp140/main.cpp,src/sp140/ble/ble_core.cpp) [1] [2] [3] [4] [5]src/sp140/ble/ble_core.cpp)BLE Whitelist and Advertising Management:
clearWhiteList()function was removed, and the code ensures advertising is stopped before any whitelist changes. (src/sp140/ble/ble_core.cpp) [1] [2]src/sp140/ble/ble_core.cpp) [1] [2] [3]Code Cleanup:
src/sp140/main.cpp) [1] [2] [3]These changes collectively make BLE pairing and bonding more predictable and robust for users and reduce the risk of controller errors during advertising or bond management.