Keep the BLE workflow connection alive across VM restarts - #11255
Merged
Conversation
Typing at the "Press any key to enter the REPL" prompt over BLE workflow serial ended the session after exactly one character: the first byte breaks main.c's wait loop, which calls bleio_reset(), and the full stack disable/enable cycle there drops every connection, the workflow's included. Ctrl-D reloads dropped it the same way. bleio_user_reset() now tears down user BLE state individually on every VM reset: it stops user scanning and advertising (as before), and now also disconnects the connections that user code initiated or accepted with its own advertising. Connections are marked as user-owned when they are created: only user code connects in the central role, and a peripheral connection belongs to whoever started the advertising it answered. The BLE workflow connection is not user-owned and stays up. This does espressif's old "TODO: Don't stop BLE workflow connection." bleio_reset() still runs the full stack cycle, but only when user code created a GATT service, tracked by a flag set in the shared-bindings Service constructor. The cycle exists only to clear user services from the GATT table: the SoftDevice can do that no other way, and espressif for now matches it, though its NimBLE can delete individual services (a possible follow-up). When no user services exist, there is nothing to tear down and the workflow session survives. The supervisor constructs its workflow services through common-hal directly and does not set the flag. Since the full cycle no longer always runs, bleio_user_reset() also clears the adapter's and connections' pointers into the VM heap (connection objects, remote service lists, nordic's advertising data buffers), which the cycle used to clear implicitly. The adapter struct is a GC root, so a pointer left over from a dead heap would be scanned as a live object in the next VM. Also make the Service Changed indication work, so a bonded host discards its cached GATT table when a reload changed the services. nordic sent it only when the bonded system-attribute load succeeded, and the call never worked anyway: sd_ble_gatts_service_changed() rejects handles outside the application-populated attribute range, and it was called with (0xC, 0xFFFF), the error ignored. The populated range is now tracked as attributes are added and indicated on every bonded reconnect. espressif never signaled at all; it now calls ble_svc_gatt_changed() after the cycle rebuilds the table, which indicates connected peers immediately and bonded peers when they reconnect. Verified with btmon on both ports: the indication goes out with the real range, and the host confirms, rediscovers, and resubscribes. Applies to nordic and espressif, the ports that run the BLE workflow. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Delete main.c's reset_devices(), which only handled CIRCUITPY_BLEIO_HCI. At VM cleanup it double-reset those builds, because ble_hci's bleio_user_reset() does a full reset itself; at startup it stood in for HCI initialization, which now happens in common_hal_bleio_init() when user code imports _bleio. HCI is unusable before user code supplies an adapter, so import time is early enough, and bleio_reset() no longer re-creates the CCCD UUID that init now owns. Remove silabs declarations of vm_used_ble and cccd_uuid, copied long ago from ble_hci and never defined or used in that port. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…flow # Conflicts: # ports/nordic/common-hal/_bleio/Adapter.h
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
tannewt
requested changes
Aug 26, 2026
tannewt
left a comment
Member
There was a problem hiding this comment.
Two minor things. Thanks for the fixes.
…ed() Per review. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Collaborator
Author
|
I'm going to merge this anyway because the zephyr test timeouts I think are fixed by changes in #11226. |
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
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 wrote the code and found the underlying bugs; I directed, reviewed, and tested, and rewrote this post and some code comments.
(Part of the series of small PRs replacing #11178, following #11225, #11236, and #11237.)
Problem
Typing at the "Press any key to enter the REPL" prompt over BLE workflow serial closed the connection after exactly one character: the first byte breaks main.c's wait loop, which calls
bleio_reset(). That disables and re-enables the full BLE stack, which drops every connection, including the workflow's. Ctrl-D reloads dropped it the same way.Reset Fixes
bleio_reset()now restarts the BLE stack only when user code created a GATT service. That is tracked by a flag set in the shared-bindingsServiceconstructor. BLE workflow services are created through common-hal directly and do not set the flag.The only reason to restart the BLE stack is to clear user services from the GATT table. There is no way on nordic to clear those services; there is such an API on espressif, but we don't use it yet (could be a future PR).
bleio_user_reset()now tears down user BLE state individually on every VM reset: it stops user scanning and advertising (as before), and now also disconnects the connections that user code initiated or accepted with its own advertising. Auser_ownedflag was added to the Connection object. The BLE workflow connection is not user-owned and stays up. This implements espressif's old "TODO: Don't stop BLE workflow connection."Since the restart no longer always runs,
bleio_user_reset()also clears the adapter's and connections' pointers into the VM heap, which the restart used to clear implicitly.Related cleanup
ble_hcireset by deletingreset_devices()in main.c and moving some code frombleio_reset()tocommon_hal_bleio_init(). This avoided a double reset (which was harmless). These changes don't affect the other ports. (Noticed by @dhalbert)vm_used_bleandcccd_uuiddeclarations fromsilabs, copied long ago from ble_hci and never used.Service Changed Problem
(This was an additional problem discovered while debugging. It's related to resets so @dhalbert included it in this PR.)
If you run
_bleiocode that creates services while the BLE workflow is running, the set of services presented by your board changes. A bonded host caches the GATT table across reconnects, and assumes the services are the same. To tell the host that the services change, the device should send a Service Changed indication.Service Changed was done wrong on nordic and not done at all on espressif, so a reload that added or removed services left hosts writing to stale handles. That is unreportable at the ATT layer and caused the REPL serial connection to look dead.
Service Changed Fixes
sd_ble_gatts_service_changed()failed, but we ignored the error. Now the populated range is tracked as attributes are added, and Service Changed is sent with that range on every bonded reconnect.ble_svc_gatt_changed()is called after the cycle rebuilds the table; NimBLE indicates connected peers immediately and records the change for bonded peers, indicating them when they reconnect.Both verified with
btmon: a correct Service Changed goes out, and the host confirms, rediscovers, and resubscribes.Not being afraid of disconnects
In the #11178 review, @tannewt suggested accepting connection drops on reload, since BLE is designed for flaky connections and bonding makes reconnection cheap. This PR takes a middle position: it keeps the disconnects that mean something and removes the ones that don't.
Testing
Tested on a Feather nRF52840 Express and a Metro ESP32-S3 with the web editor on Linux, verified with btmon. Some testing on Windows and macOS also done. macOS is by far the most robust.
Note: a known, pre-existing espressif PacketBuffer send bug, to be fixed in a following PR, can hang reloads while the web editor is connected. It also affects long file-directory listings on current main.
There are still problems with web-editor: it is slow to notice board-initiated disconnects, and after its automatic reconnect the serial pane does not re-acquire its characteristic, so typing goes nowhere. I'll file fixes for that later. But the fixes here will allow those fixes to succeed.