zephyr-cp: keep the BLE workflow connection across reloads; fix bonding at boot - #11366
Open
dhalbert wants to merge 2 commits into
Open
zephyr-cp: keep the BLE workflow connection across reloads; fix bonding at boot#11366dhalbert wants to merge 2 commits into
dhalbert wants to merge 2 commits into
Conversation
…ng at boot Port the nordic/espressif BLE workflow fixes (adafruit#11225, adafruit#11255) to zephyr-cp and fix several port-specific problems found on the nrf54lm20dk. - Bonds were invisible at boot. `supervisor_bluetooth_init()` checks and erases bonds before the stack is enabled, and on zephyr-cp the keys are only loaded by `settings_load()` in `set_enabled(true)`. So a bonded board never started the workflow on a plain boot, and a blue-flash reset never erased the bond. `is_bonded_to_central()` and `erase_bonding()` now bring the stack up first via `bleio_adapter_ensure_stack_ready()`. - Any keypress at the "Press any key" prompt or Ctrl-D dropped the workflow connection: `bleio_reset()` restarted the whole workflow on every VM reset. That restart also never removed user-created services, since `set_enabled(false)` does not call `bt_disable()`, leaving Zephyr's GATT database pointing into the freed VM heap. User services are now kept in a retained list (the espressif pattern) and unregistered in `bleio_user_reset()`; Zephyr sends Service Changed itself. Connections get a `user_owned` flag and only those are disconnected. `bleio_reset()` no longer restarts anything. - `import _bleio` stopped workflow advertising and dropped connections, because `common_hal_bleio_init()` called `bleio_adapter_reset()`. Removed; init is now idempotent. - The PacketBuffer send worker retried every error other than `-ENOTCONN` forever, so `-EPERM`/`-EINVAL` from `gatt_notify()` wedged the buffer, and a notification pending at disconnect left `packet_queued` set because Zephyr never calls the ATT callback for a PDU destroyed by a disconnect. Drop on non-transient errors and clear the state when the connection is gone. - The CCC descriptor was writable on an unencrypted link even when reading the characteristic requires encryption, so an unpaired central could subscribe and never be made to pair. Its write permission now requires the same encryption or authentication as reading the value, espressif's adafruit#11236 policy. NO_ACCESS on read leaves it writable, unlike nordic. - `CONFIG_BT_MAX_CONN` defaulted to 1. Zephyr's host reserves a connection object for a connectable advertiser, and the controller's RX node pool term `BT_CTLR_MAX_CONNECTABLE` is `1 + MIN(MAX_CONN - 1, ADV_SET)`, so while the workflow advertised `adapter.connect()` failed with `ENOMEM` and `start_scan()` with a timeout failed with `OSError` (HCI 0x2042 status 0x07, no node for the scan-termination event). Set 5 to match nordic; about 2.9 KB of RAM per connection. `CONFIG_BT_MAX_PAIRED` was 1, so a second bond evicted the workflow's; set 3, the NimBLE default. - Tests: new `test_bsim_ble_workflow_reload.py` checks the connection survives a Ctrl-D reload with and without a user service, and that the service is gone afterwards. `test_bsim_adapter_state_after_reload` expected `advertising False`, which only held because the import used to kill the workflow's advertising; it also needed a short delay so the sim does not exit before the second run's UART output is delivered. Tested on nrf54lm20dk with the web editor (pairing, Ctrl-D, user-service reload, Bluetooth off/on reconnect) and with the nrf54lm20 bsim suite (61 passed). The feather_nrf52840_zephyr build was checked for size. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
`supervisor_bluetooth_init()` took `start_ticks` before calling `common_hal_bleio_adapter_is_bonded_to_central()`. On zephyr-cp that call brings up the BLE stack (`bt_enable()` + `settings_load()`), and the time it takes was subtracted from the 1000 ms discovery window, so the blue flash could be short or missing. Take `start_ticks` after the bond check so the window is the full 1000 ms on every port. The saved word is set before either, so a reset during stack bring-up still enters discovery mode. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
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
1 participant
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.
Claude found these bugs and wrote the code and tests; I directed, reviewed, and tested on hardware. This note was extensively reworked for brevity and clarity. The commit message is Claude's.
This fixes BLE workflow bugs that were similar to what I encountered on espressif and nordic. In addition some other fixes were needed.
Port BLE and BLE workflow fixes done for other ports in Fix several independent BLE bugs, split from #11178 #11225 and Keep the BLE workflow connection alive across VM restarts #11255.
Fixed bonding handling at boot time.
Fix
import _bleiodoing things that killed the workflow.bleio_adapter_reset()was removed and replaced with more specific logic.Fixed reloads dropping the workflow connection. This same bug was previously fixed in nordic and espressif.
Keep track of user-created services and their attributes and remove them from the Zephyr GATT table when the VM shuts down. Espressif has similar registration but does not yet remove services individually (a TODO: it could).
Problems with PacketBuffer sending were fixed. Reported send errors are now handled better.
Fix handling a pending notification at disconnect.
Disallow unencrypted writing of CCCD of an encrypted characteristic. Same fix as espressif's Require encryption to write the CCCD of an encrypted characteristic (espressif) #11236.
Increase some limits in
prj.conf:CONFIG_BT_MAX_CONNdefaulted to 1, which also limited other things besides connections. If the BLE workflow was running or advertising (not even connected), nothing else could connect, and scanning didn't work either. Increased it to 5 (same as nordic). About 2.9 KB of RAM is reserved per connection.CONFIG_BT_MAX_PAIREDdefaulted to 1, so bonding to a second peer evicted the workflow's bond. Set to 3, the NimBLE default. Nordic keeps dozens.Tests
tests/bsim/test_bsim_ble_workflow_reload.py: a client pairs, lists files, the workflow device is reloaded with Ctrl-D, and the connection must survive with file transfer still working. A second variant hascode.pycreate a Battery Service on its first run and checks the client no longer sees it after the reload.test_bsim_adapter_state_after_reloadexpectedadvertising False, which was only true becauseimport _bleioused to kill the workflow's advertising. It also needed a short delay at the end ofcode.py, since the simulation exits right after the last VM cleanup and could lose the second run's UART output.