From 7becb9ffe028e8955ee0b699b6fd43b11e219e7e Mon Sep 17 00:00:00 2001 From: Dan Halbert Date: Sun, 13 Sep 2026 12:29:19 -0400 Subject: [PATCH 1/2] zephyr-cp: keep the BLE workflow connection across reloads; fix bonding at boot Port the nordic/espressif BLE workflow fixes (#11225, #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 #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 --- ports/zephyr-cp/common-hal/_bleio/Adapter.c | 103 ++++++---- ports/zephyr-cp/common-hal/_bleio/Adapter.h | 1 - .../zephyr-cp/common-hal/_bleio/Connection.h | 4 + .../common-hal/_bleio/PacketBuffer.c | 73 ++++--- ports/zephyr-cp/common-hal/_bleio/Service.c | 83 +++++++- ports/zephyr-cp/common-hal/_bleio/Service.h | 11 + ports/zephyr-cp/common-hal/_bleio/__init__.c | 45 ++++- ports/zephyr-cp/prj.conf | 11 + .../tests/bsim/test_bsim_ble_adapter.py | 13 +- .../bsim/test_bsim_ble_workflow_reload.py | 190 ++++++++++++++++++ 10 files changed, 449 insertions(+), 85 deletions(-) create mode 100644 ports/zephyr-cp/tests/bsim/test_bsim_ble_workflow_reload.py diff --git a/ports/zephyr-cp/common-hal/_bleio/Adapter.c b/ports/zephyr-cp/common-hal/_bleio/Adapter.c index c3684a3b148..7819e68676e 100644 --- a/ports/zephyr-cp/common-hal/_bleio/Adapter.c +++ b/ports/zephyr-cp/common-hal/_bleio/Adapter.c @@ -56,7 +56,10 @@ static bool ble_advertising = false; // than user code. Lets the workflow restart its own adverts without disturbing // user-initiated advertising. static bool ble_advertising_internal = false; -static bool ble_adapter_enabled = true; +// Set to true when common_hal_bleio_adapter_set_enabled(true) brings the stack up. +// set_enabled(false) clears this flag and stops advertising and scanning, but +// the controller keeps running. +static bool ble_adapter_enabled = false; #define BLEIO_ADV_MAX_FIELDS 16 #define BLEIO_ADV_MAX_DATA_LEN 31 @@ -149,6 +152,7 @@ static void bleio_connection_clear(bleio_connection_internal_t *self) { self->connection_obj = mp_const_none; self->pair_status = PAIR_NOT_PAIRED; self->sec_err = 0; + self->user_owned = false; } static void bleio_connection_release(bleio_connection_internal_t *connection, uint8_t reason) { @@ -205,6 +209,11 @@ static void bleio_connected_cb(struct bt_conn *conn, uint8_t err) { int mtu_err = bt_gatt_exchange_mtu(conn, &mtu_exchange_params[idx]); (void)mtu_err; + // A peripheral connection belongs to whoever started the advertising it + // answered: the BLE workflow (internal) or user code. Central connections + // are marked in common_hal_bleio_adapter_connect(). + connection->user_owned = !ble_advertising_internal; + // When connectable advertising results in a connection, the controller // auto-stops advertising. Clear our flag to match (we cannot call // stop_advertising() here because this callback runs in Zephyr's BT @@ -330,6 +339,32 @@ static uint16_t bleio_validate_and_convert_timeout(mp_float_t timeout) { return (uint16_t)timeout_units; } +// Start the Zephyr Bluetooth host and load its settings (identity, bond keys, +// CCC state) if that hasn't happened yet. Returns 0 or a negative Zephyr errno. +// +// This is separate from set_enabled() because the BLE workflow asks about +// bonds (is_bonded_to_central(), erase_bonding()) in supervisor_bluetooth_init(), +// before it enables the adapter in supervisor_start_bluetooth(). Bond keys live +// in the settings subsystem and are only in RAM after settings_load(), so those +// calls must be able to bring the stack up on their own. Nothing here powers the +// controller down later: set_enabled(false) leaves it running. +static int bleio_adapter_ensure_stack_ready(void) { + if (bt_is_ready()) { + return 0; + } + int err = bt_enable(NULL); + if (err != 0 && err != -EALREADY) { + return err; + } + + // bt_init() returns early without setting BT_DEV_READY when + // CONFIG_BT_SETTINGS=y and no identity is loaded yet. + // Load settings so the BT settings handler fires and calls + // bt_finalize_init() which sets BT_DEV_READY. + settings_load(); + return 0; +} + void common_hal_bleio_adapter_set_enabled(bleio_adapter_obj_t *self, bool enabled) { if (enabled == ble_adapter_enabled) { return; @@ -338,17 +373,9 @@ void common_hal_bleio_adapter_set_enabled(bleio_adapter_obj_t *self, bool enable for (size_t i = 0; i < BLEIO_TOTAL_CONNECTION_COUNT; i++) { bleio_connection_clear(&bleio_connections[i]); } - if (!bt_is_ready()) { - int err = bt_enable(NULL); - if (err != 0 && err != -EALREADY) { - raise_zephyr_error(err); - } - - // bt_init() returns early without setting BT_DEV_READY when - // CONFIG_BT_SETTINGS=y and no identity is loaded yet. - // Load settings so the BT settings handler fires and calls - // bt_finalize_init() which sets BT_DEV_READY. - settings_load(); + int err = bleio_adapter_ensure_stack_ready(); + if (err != 0) { + raise_zephyr_error(err); } // Ensure a local identity exists so advertising/connections work and the // name is stable across reboots. bt_id_create persists the identity when @@ -815,6 +842,9 @@ mp_obj_t common_hal_bleio_adapter_connect(bleio_adapter_obj_t *self, bleio_addre // ref via bleio_connection_track(). Drop the create ref now. bt_conn_unref(conn); + // Only user code connects in the central role. + connection->user_owned = true; + self->connection_objs = NULL; return bleio_connection_new_from_internal(connection); } @@ -840,6 +870,16 @@ static void bond_iterator_check(const struct bt_bond_info *info, void *user_data } void common_hal_bleio_adapter_erase_bonding(bleio_adapter_obj_t *self) { + // Can be called from Python as _bleio.adapter.erase_bonding(), and from + // supervisor_bluetooth_init() on a discovery-mode boot, before the adapter is + // enabled. Bond keys are only visible once the stack is up and settings are + // loaded; without this the loop below finds nothing and the bond reappears + // when the workflow starts. The workflow call runs before the VM, so a failure + // to start the stack is ignored here rather than raised. + if (bleio_adapter_ensure_stack_ready() != 0) { + return; + } + // Unpair all bonded devices for all local identities. for (uint8_t id = 0; id < CONFIG_BT_ID_MAX; id++) { // bt_unpair takes an addr; use bt_foreach_bond to iterate and unpair. @@ -866,7 +906,14 @@ void common_hal_bleio_adapter_erase_bonding(bleio_adapter_obj_t *self) { } bool common_hal_bleio_adapter_is_bonded_to_central(bleio_adapter_obj_t *self) { - // Check if any bond exists for identity 0 + // Called only by the BLE workflow, from supervisor_bluetooth_init() before + // the adapter is enabled and from supervisor_bluetooth_background(); see + // erase_bonding() above. Runs before the VM, so don't raise. + if (bleio_adapter_ensure_stack_ready() != 0) { + return false; + } + + // Check if any bond exists for any local identity. for (uint8_t id = 0; id < CONFIG_BT_ID_MAX; id++) { bool has_bonds = false; bt_foreach_bond(id, bond_iterator_check, &has_bonds); @@ -882,36 +929,6 @@ void bleio_adapter_gc_collect(bleio_adapter_obj_t *adapter) { gc_collect_root((void **)bleio_connections, sizeof(bleio_connections) / sizeof(size_t)); } -void bleio_adapter_reset(bleio_adapter_obj_t *adapter) { - if (adapter == NULL) { - return; - } - - common_hal_bleio_adapter_stop_scan(adapter); - common_hal_bleio_adapter_stop_advertising(adapter); - - for (size_t i = 0; i < BLEIO_TOTAL_CONNECTION_COUNT; i++) { - bleio_connection_internal_t *connection = &bleio_connections[i]; - if (connection->conn != NULL) { - bt_conn_disconnect(connection->conn, BT_HCI_ERR_REMOTE_USER_TERM_CONN); - } - if (connection->connection_obj != MP_OBJ_NULL && - connection->connection_obj != mp_const_none) { - bleio_connection_obj_t *connection_obj = MP_OBJ_TO_PTR(connection->connection_obj); - connection_obj->connection = NULL; - connection_obj->disconnect_reason = BT_HCI_ERR_REMOTE_USER_TERM_CONN; - } - bleio_connection_clear(connection); - } - - adapter->scan_results = NULL; - adapter->connection_objs = NULL; - active_scan_results = NULL; - ble_advertising = false; - ble_advertising_internal = false; - ble_adapter_enabled = bt_is_ready(); -} - bleio_adapter_obj_t *common_hal_bleio_allocate_adapter_or_raise(void) { return &common_hal_bleio_adapter_obj; } diff --git a/ports/zephyr-cp/common-hal/_bleio/Adapter.h b/ports/zephyr-cp/common-hal/_bleio/Adapter.h index 25e1c35d563..ba84e810bfe 100644 --- a/ports/zephyr-cp/common-hal/_bleio/Adapter.h +++ b/ports/zephyr-cp/common-hal/_bleio/Adapter.h @@ -31,7 +31,6 @@ typedef struct { } bleio_adapter_obj_t; void bleio_adapter_gc_collect(bleio_adapter_obj_t *adapter); -void bleio_adapter_reset(bleio_adapter_obj_t *adapter); // Queue a background run of supervisor_bluetooth_background() so the VM drains // incoming BLE PacketBuffer data. Safe to call from Zephyr BT/workqueue context. diff --git a/ports/zephyr-cp/common-hal/_bleio/Connection.h b/ports/zephyr-cp/common-hal/_bleio/Connection.h index 323e9393103..9565fd31b8b 100644 --- a/ports/zephyr-cp/common-hal/_bleio/Connection.h +++ b/ports/zephyr-cp/common-hal/_bleio/Connection.h @@ -24,6 +24,10 @@ typedef struct { mp_obj_t connection_obj; volatile pair_status_t pair_status; uint8_t sec_err; // Security error code from pairing attempt + // True if user code initiated this connection or accepted it with its own + // advertising. bleio_user_reset() disconnects only these; the BLE workflow + // connection is not user-owned and survives VM restarts. + bool user_owned; } bleio_connection_internal_t; typedef struct { diff --git a/ports/zephyr-cp/common-hal/_bleio/PacketBuffer.c b/ports/zephyr-cp/common-hal/_bleio/PacketBuffer.c index 8cdb5fcae76..6e3f42a6729 100644 --- a/ports/zephyr-cp/common-hal/_bleio/PacketBuffer.c +++ b/ports/zephyr-cp/common-hal/_bleio/PacketBuffer.c @@ -134,28 +134,36 @@ static void packet_buffer_send_work_handler(struct k_work *work) { bleio_packet_buffer_obj_t *self = CONTAINER_OF( k_work_delayable_from_work(work), bleio_packet_buffer_obj_t, send_work); - // A notification is awaiting its completion callback; it will resubmit us. - if (self->packet_queued) { - return; - } - if (self->pending_size == 0) { - return; // nothing staged - } - // Server-side notify path only; clients write directly. characteristic is - // NULL after deinit, so bail before touching it (a completion callback can - // resubmit us after teardown). + // NULL after deinit, so bail before touching it (notify_complete_cb() can + // reschedule send_work after teardown). bleio_characteristic_obj_t *c = self->characteristic; if (c == NULL || self->client) { return; } if (!conn_is_valid(self)) { - // Stale connection: drop everything staged. + // The peer is gone: drop everything staged, and clear packet_queued even + // if a notification was handed to bt_gatt_notify_cb() already and its + // notify_complete_cb() has not run. It never will: Zephyr does not call + // the ATT callback for a PDU destroyed by a disconnect (att.c + // att_on_sent_cb(): "Bearer not connected, dropping ATT cb"). If + // packet_queued stayed set, the early return below would skip every + // later send on this buffer, even after the peer reconnects. self->pending_size = 0; self->packet_queued = false; return; } + // A notification has been handed to bt_gatt_notify_cb() and its + // notify_complete_cb() has not run yet. That callback reschedules send_work, + // so this handler will run again once the controller has sent the PDU. + if (self->packet_queued) { + return; + } + if (self->pending_size == 0) { + return; // nothing staged + } + // Staging keeps pending_size <= the negotiated ATT MTU payload, so the // whole staged packet fits in one notification. struct bt_gatt_notify_params params = { @@ -176,27 +184,34 @@ static void packet_buffer_send_work_handler(struct k_work *work) { self->pending_index ^= 1; // VM fills the other buffer next return; } + if (err == -ENOMEM || err == -EAGAIN) { + // No ATT TX buffer right now. The ATT TX pool is shared across all ATT + // traffic on all connections, so it can be full from other + // notifies/indications/responses even when we have nothing in flight. + // Running on the workqueue makes the allocator use K_NO_WAIT: it returns + // NULL and the stack returns -ENOMEM *before* copying or queueing a PDU — + // nothing sent, nothing dropped; the bytes are still in + // outgoing[pending_index]. Leave the data staged and reschedule ourselves + // after a short delay so we retry even when the VM is idle (no write/flush + // to drive us). The delay — not an immediate resubmit — keeps the workqueue + // from busy-looping against a full pool. + k_work_reschedule(&self->send_work, K_MSEC(2)); + return; + } if (err == -ENOTCONN) { - // Peer disconnected — discard everything pending and cancel any - // pending delayed retry. (We're here only when packet_queued is clear, - // so no in-flight completion is owed.) + // Peer disconnected — forget the connection too. (We're here only when + // packet_queued is clear, so no in-flight completion is owed.) self->conn = NULL; - self->pending_size = 0; - self->packet_queued = false; - k_work_cancel_delayable(&self->send_work); - return; } - // -ENOMEM / -EAGAIN: no ATT TX buffer right now. The ATT TX pool is shared - // across all ATT traffic on all connections, so it can be full from other - // notifies/indications/responses even when we have nothing in flight. - // Running on the workqueue makes the allocator use K_NO_WAIT: it returns - // NULL and the stack returns -ENOMEM *before* copying or queueing a PDU — - // nothing sent, nothing dropped; the bytes are still in - // outgoing[pending_index]. Leave the data staged and reschedule ourselves - // after a short delay so we retry even when the VM is idle (no write/flush - // to drive us). The delay — not an immediate resubmit — keeps the workqueue - // from busy-looping against a full pool. - k_work_reschedule(&self->send_work, K_MSEC(2)); + // Any other error is not transient: -ENOTCONN (peer gone), -EPERM (link not + // encrypted and the characteristic requires it), -EINVAL (peer not + // subscribed; CONFIG_BT_GATT_ENFORCE_SUBSCRIPTION), -ENOENT (attribute not + // registered). Retrying would spin forever and wedge this buffer. Drop the + // staged packet, as nordic and espressif do on non-resource errors, and + // cancel any pending delayed retry. + self->pending_size = 0; + self->packet_queued = false; + k_work_cancel_delayable(&self->send_work); } // Shared core for both the Python-facing (allocating) and workflow diff --git a/ports/zephyr-cp/common-hal/_bleio/Service.c b/ports/zephyr-cp/common-hal/_bleio/Service.c index 5f0c7ee268b..4333ee21c25 100644 --- a/ports/zephyr-cp/common-hal/_bleio/Service.c +++ b/ports/zephyr-cp/common-hal/_bleio/Service.c @@ -45,6 +45,82 @@ static const struct bt_uuid_16 _uuid_chrc = BT_UUID_INIT_16(BT_UUID_GATT_CHRC_VA static const struct bt_uuid_16 _uuid_ccc = BT_UUID_INIT_16(BT_UUID_GATT_CCC_VAL); static const struct bt_uuid_16 _uuid_cud = BT_UUID_INIT_16(BT_UUID_GATT_CUD_VAL); +// User-created services live on the GC heap, but once registered, Zephyr's GATT +// database holds a pointer to the bt_gatt_service node inside the object. So a +// registered heap service must stay reachable until it is unregistered, and it +// must be unregistered before the heap it lives in goes away. The zephyr port never +// restarts the Bluetooth stack (set_enabled(false) leaves it running), so the +// database is never cleared for us: bleio_user_reset() unregisters the services at the +// end of every VM run. Same pattern as ports/espressif/common-hal/_bleio/Service.c. +static bleio_service_obj_t *_retained_services; + +static void service_retain(bleio_service_obj_t *self) { + if (!gc_ptr_on_heap((void *)self)) { + // Statically allocated workflow service; the supervisor owns its lifetime. + return; + } + for (bleio_service_obj_t *it = _retained_services; it != NULL; it = it->next_retained) { + if (it == self) { + return; + } + } + self->next_retained = _retained_services; + _retained_services = self; +} + +static void service_release(bleio_service_obj_t *self) { + bleio_service_obj_t **prev = &_retained_services; + for (bleio_service_obj_t *it = *prev; it != NULL; it = it->next_retained) { + if (it == self) { + *prev = it->next_retained; + it->next_retained = NULL; + return; + } + prev = &it->next_retained; + } +} + +// One pointer is enough: the GC traces next_retained through the rest of the chain. +void bleio_service_gc_collect(void) { + gc_collect_ptr(_retained_services); +} + +void bleio_service_unregister_retained(void) { + while (_retained_services != NULL) { + bleio_service_obj_t *service = _retained_services; + // The characteristics' value buffers are on the port heap and nothing + // else frees them once the VM heap is gone. + mp_obj_list_t *list = service->characteristic_list; + if (list != NULL) { + for (size_t i = 0; i < list->len; i++) { + common_hal_bleio_characteristic_deinit(MP_OBJ_TO_PTR(list->items[i])); + } + } + // Unregisters from Zephyr, frees attrs, and removes it from this list. + common_hal_bleio_service_deinit(service); + } +} + +// Zephyr write permission for the CCC descriptor. If reading the characteristic +// value requires encryption or authentication, so does subscribing to it; +// otherwise a client could get the value from notifications without ever being +// made to pair. An unencrypted write then gets Insufficient Encryption or +// Authentication, which is what makes a host pair. Same as espressif (#11236). +// NO_ACCESS on read is not a link-security level, so it leaves the CCC writable; +// nordic differs and makes such a characteristic unsubscribable. +static uint16_t ccc_write_perm(bleio_attribute_security_mode_t read_perm) { + switch (read_perm) { + case SECURITY_MODE_ENC_NO_MITM: + return BT_GATT_PERM_WRITE_ENCRYPT; + case SECURITY_MODE_ENC_WITH_MITM: + return BT_GATT_PERM_WRITE_AUTHEN; + case SECURITY_MODE_LESC_ENC_WITH_MITM: + return BT_GATT_PERM_WRITE_LESC; + default: + return BT_GATT_PERM_WRITE; + } +} + static void service_ensure_capacity(bleio_service_obj_t *self, size_t needed) { if (self->attr_count + needed <= self->attr_capacity) { return; @@ -70,6 +146,7 @@ uint32_t _common_hal_bleio_service_construct(bleio_service_obj_t *self, self->start_handle = 0; self->end_handle = 0; self->registered = false; + self->next_retained = NULL; // Convert UUID to Zephyr format bleio_uuid_to_zephyr(uuid, &self->zephyr_uuid); @@ -106,6 +183,7 @@ void common_hal_bleio_service_deinit(bleio_service_obj_t *self) { bt_gatt_service_unregister(&self->zephyr_service); self->registered = false; } + service_release(self); if (self->attrs != NULL) { port_free(self->attrs); self->attrs = NULL; @@ -124,6 +202,7 @@ void common_hal_bleio_service_from_remote_service(bleio_service_obj_t *self, self->start_handle = 0; self->end_handle = 0; self->registered = false; + self->next_retained = NULL; self->attrs = NULL; self->attr_count = 0; self->attr_capacity = 0; @@ -229,7 +308,7 @@ void common_hal_bleio_service_add_characteristic(bleio_service_obj_t *self, BT_GATT_CCC_MANAGED_USER_DATA_INIT(bleio_ccc_changed_cb, bleio_ccc_write_cb, NULL); self->attrs[idx] = (struct bt_gatt_attr) { .uuid = (const struct bt_uuid *)&_uuid_ccc, - .perm = BT_GATT_PERM_READ | BT_GATT_PERM_WRITE, + .perm = BT_GATT_PERM_READ | ccc_write_perm(characteristic->read_perm), .read = bt_gatt_attr_read_ccc, .write = bt_gatt_attr_write_ccc, .user_data = &characteristic->zephyr_ccc, @@ -262,4 +341,6 @@ void common_hal_bleio_service_add_characteristic(bleio_service_obj_t *self, raise_zephyr_error(err); } self->registered = true; + // Zephyr now points into this object; keep it alive until unregistered. + service_retain(self); } diff --git a/ports/zephyr-cp/common-hal/_bleio/Service.h b/ports/zephyr-cp/common-hal/_bleio/Service.h index 8c820fd204e..75731bc6e09 100644 --- a/ports/zephyr-cp/common-hal/_bleio/Service.h +++ b/ports/zephyr-cp/common-hal/_bleio/Service.h @@ -29,4 +29,15 @@ typedef struct bleio_service_obj { size_t attr_capacity; struct bt_uuid_128 zephyr_uuid; bool registered; + // Link in the list of heap services retained while Zephyr's GATT database + // refers to them. See bleio_service_unregister_retained(). + struct bleio_service_obj *next_retained; } bleio_service_obj_t; + +// Unregister every user-created service still in Zephyr's GATT database and +// free their port-heap buffers. Call from bleio_user_reset(), while the GC heap +// the service objects live in is still valid. +void bleio_service_unregister_retained(void); + +// Mark the retained services as reachable during a GC pass. +void bleio_service_gc_collect(void); diff --git a/ports/zephyr-cp/common-hal/_bleio/__init__.c b/ports/zephyr-cp/common-hal/_bleio/__init__.c index e3c208a7d1d..b51db00835a 100644 --- a/ports/zephyr-cp/common-hal/_bleio/__init__.c +++ b/ports/zephyr-cp/common-hal/_bleio/__init__.c @@ -12,43 +12,70 @@ #include "common-hal/_bleio/__init__.h" #include "bindings/zephyr_kernel/__init__.h" #include "common-hal/_bleio/Connection.h" +#include "common-hal/_bleio/Service.h" #include "supervisor/shared/bluetooth/bluetooth.h" #include "supervisor/shared/tick.h" // The singleton _bleio.Adapter object bleio_adapter_obj_t common_hal_bleio_adapter_obj; +// Called once by the BLE workflow at boot and again on every `import _bleio`, +// so it must not disturb a running adapter: the workflow may be advertising or +// connected by the time user code imports the module. Starting the BLE stack happens in +// common_hal_bleio_adapter_set_enabled(), which the importer calls next. void common_hal_bleio_init(void) { common_hal_bleio_adapter_obj.base.type = &bleio_adapter_type; - bleio_adapter_reset(&common_hal_bleio_adapter_obj); bleio_connection_register_auth_callbacks(); } +// Tear down user BLE state at the end of a VM run, leaving the BLE workflow +// connection up. Runs while the VM heap is still valid. void bleio_user_reset(void) { if (common_hal_bleio_adapter_get_enabled(&common_hal_bleio_adapter_obj)) { // Stop any user scanning or advertising. common_hal_bleio_adapter_stop_scan(&common_hal_bleio_adapter_obj); common_hal_bleio_adapter_stop_advertising(&common_hal_bleio_adapter_obj); + + // Disconnect the connections that user code initiated or accepted with its + // own advertising. The BLE workflow connection is not user-owned and stays up. + // + // Clear each connection's pointer into the VM heap first: the heap is about + // to go away, and a disconnect completes asynchronously, possibly after the heap + // is gone. + for (size_t i = 0; i < BLEIO_TOTAL_CONNECTION_COUNT; i++) { + bleio_connection_internal_t *connection = &bleio_connections[i]; + connection->connection_obj = mp_const_none; + if (connection->conn != NULL && connection->user_owned) { + common_hal_bleio_connection_disconnect(connection); + } + } + + // Remove references to any VM heap objects. + common_hal_bleio_adapter_obj.connection_objs = NULL; + common_hal_bleio_adapter_obj.scan_results = NULL; } + // Remove user-created services from Zephyr's GATT database and free their + // buffers. Zephyr indicates Service Changed to connected peers itself, and + // records it for bonded peers to receive when they next connect. + bleio_service_unregister_retained(); + // Maybe start advertising the BLE workflow. supervisor_bluetooth_background(); } +// Called after the VM heap is gone. On nordic and espressif this restarts the +// BLE stack when user code created GATT services, because their stacks can't +// remove services one at a time. Zephyr can, and bleio_user_reset() already did, +// so there is nothing left that requires dropping the workflow connection. void bleio_reset(void) { common_hal_bleio_adapter_obj.base.type = &bleio_adapter_type; - if (!common_hal_bleio_adapter_get_enabled(&common_hal_bleio_adapter_obj)) { - return; - } - - supervisor_stop_bluetooth(); - bleio_adapter_reset(&common_hal_bleio_adapter_obj); - common_hal_bleio_adapter_set_enabled(&common_hal_bleio_adapter_obj, false); - supervisor_start_bluetooth(); + bleio_clear_user_services_created(); } void common_hal_bleio_gc_collect(void) { bleio_adapter_gc_collect(&common_hal_bleio_adapter_obj); + bleio_service_gc_collect(); } // ======================================================================= diff --git a/ports/zephyr-cp/prj.conf b/ports/zephyr-cp/prj.conf index 864af4c6112..80a28a9c272 100644 --- a/ports/zephyr-cp/prj.conf +++ b/ports/zephyr-cp/prj.conf @@ -76,3 +76,14 @@ CONFIG_PSA_WANT_ALG_SHA_256=y CONFIG_NVS=y CONFIG_SETTINGS_NVS=y CONFIG_SETTINGS_NVS_SECTOR_COUNT=256 + +# Zephyr's host reserves a connection object for a connectable advertiser, and +# the controller sizes its RX node pool from this too. With the default of 1, +# user code cannot connect as a central or start a timed scan while the BLE +# workflow advertises. 5 matches nordic. About 2.9 KB of RAM per connection. +CONFIG_BT_MAX_CONN=5 + +# Number of stored bonds. The default of 1 means bonding to a second peer +# evicts the BLE workflow's bond (BT_KEYS_OVERWRITE_OLDEST). 3 matches +# espressif's NimBLE default; nordic keeps dozens. +CONFIG_BT_MAX_PAIRED=3 diff --git a/ports/zephyr-cp/tests/bsim/test_bsim_ble_adapter.py b/ports/zephyr-cp/tests/bsim/test_bsim_ble_adapter.py index 7651a0ec144..29092b0641c 100644 --- a/ports/zephyr-cp/tests/bsim/test_bsim_ble_adapter.py +++ b/ports/zephyr-cp/tests/bsim/test_bsim_ble_adapter.py @@ -80,6 +80,7 @@ def test_bsim_adapter_disable_stops_advertising(bsim_phy, circuitpython): # --- Adapter state across soft reload --- BSIM_ENABLE_RELOAD_CODE = """\ +import time import _bleio adapter = _bleio.adapter @@ -88,13 +89,21 @@ def test_bsim_adapter_disable_stops_advertising(bsim_phy, circuitpython): print("advertising", adapter.advertising) print("connected", adapter.connected) print("done") +# The sim exits right after the second run's VM cleanup (port_resets). Give the +# UART time to deliver the lines above to the pty before that happens. +time.sleep(0.5) """ @pytest.mark.port_resets(3) +@pytest.mark.duration(30) @pytest.mark.circuitpy_drive({"code.py": BSIM_ENABLE_RELOAD_CODE}) def test_bsim_adapter_state_after_reload(bsim_phy, circuitpython): - """Adapter state is clean after soft reload.""" + """Adapter state is consistent across a soft reload. + + The BLE workflow is advertising on this device, and `import _bleio` must not + disturb it, so adapter.advertising reads True on both runs (as on nordic). + """ circuitpython.serial.wait_for("done") circuitpython.serial.wait_for("Press any key to enter the REPL") circuitpython.serial.write("\x04") @@ -103,7 +112,7 @@ def test_bsim_adapter_state_after_reload(bsim_phy, circuitpython): output = circuitpython.serial.all_output assert output.count("enabled True") >= 2 - assert output.count("advertising False") >= 2 + assert output.count("advertising True") >= 2 assert output.count("connected False") >= 2 diff --git a/ports/zephyr-cp/tests/bsim/test_bsim_ble_workflow_reload.py b/ports/zephyr-cp/tests/bsim/test_bsim_ble_workflow_reload.py new file mode 100644 index 00000000000..cc56b1108e3 --- /dev/null +++ b/ports/zephyr-cp/tests/bsim/test_bsim_ble_workflow_reload.py @@ -0,0 +1,190 @@ +# SPDX-FileCopyrightText: 2026 Dan Halbert for Adafruit Industries +# SPDX-License-Identifier: MIT + +"""The supervisor BLE workflow connection survives a VM reload (bsim). + +A second CircuitPython device connects to the workflow, pairs, and lists the +root directory. The test then interrupts the workflow device's code.py and +reloads it with Ctrl-D, the same path as typing at the "Press any key" prompt +or saving a file from the web editor. The connection must stay up and file +transfer must still work afterwards. + +In the variant where code.py created a `_bleio.Service`, that service must be +gone from the GATT table after the reload, while the workflow connection still +survives: Zephyr can unregister services individually, so no stack restart is +needed. +""" + +import pytest + +from .conftest import get_library_files + +_ADAFRUIT_BLE = get_library_files("adafruit_ble") +_ADAFRUIT_BLE_FILE_TRANSFER = get_library_files("adafruit_ble_file_transfer") + +# Device 1, plain: idle code.py until the test interrupts it. +WORKFLOW_IDLE_CODE = """\ +import supervisor +import time +print("run", supervisor.runtime.run_reason) +print("workflow ready") +# Idle in short sleeps: on zephyr-cp a Ctrl-C from the UART console is only +# noticed when the current time.sleep() ends, so keep each one brief. +while True: + time.sleep(0.5) +""" + +# Device 1, with a user service: create a Battery Service only on the first +# run, so that after the reload it must have been removed by the supervisor, +# not recreated by code.py. +WORKFLOW_SERVICE_CODE = """\ +import supervisor +import time +import _bleio +print("run", supervisor.runtime.run_reason) +if supervisor.runtime.run_reason == supervisor.RunReason.STARTUP: + service = _bleio.Service(_bleio.UUID(0x180F)) + _bleio.Characteristic.add_to_service( + service, _bleio.UUID(0x2A19), + properties=_bleio.Characteristic.READ, + read_perm=_bleio.Attribute.OPEN, + write_perm=_bleio.Attribute.NO_ACCESS, + max_length=1, fixed_length=True, initial_value=b"\\x64", + ) + print("user service created") +print("workflow ready") +# Idle in short sleeps: on zephyr-cp a Ctrl-C from the UART console is only +# noticed when the current time.sleep() ends, so keep each one brief. +while True: + time.sleep(0.5) +""" + +# Device 2: connect, pair, list files, report whether the Battery Service is +# present, then wait through the reload and do it all again on the same +# connection. +CLIENT_CODE = """\ +import time +import _bleio +from adafruit_ble import BLERadio +from adafruit_ble.advertising.standard import ProvideServicesAdvertisement +from adafruit_ble.uuid import StandardUUID +from adafruit_ble_file_transfer import FileTransferService, FileTransferClient + +BATTERY = _bleio.UUID(0x180F) + +ble = BLERadio() + +# Wait for the workflow device's file transfer server to finish starting up +# before scanning. +time.sleep(5) +print("scan start") +target = None +for adv in ble.start_scan(ProvideServicesAdvertisement, timeout=15, active=True): + if StandardUUID(0xFEBB) in adv.services: + target = adv + print("found workflow") + break +ble.stop_scan() +if target is None: + print("no workflow") + raise SystemExit(1) + +connection = ble.connect(target, timeout=10) +print("connected", connection.connected) +connection.pair() +print("paired", connection.paired) + +def battery_present(): + services = connection._bleio_connection.discover_remote_services() + return any(s.uuid == BATTERY for s in services) + +def list_root(tag): + service = connection[FileTransferService] + client = FileTransferClient(service) + names = [e[0] for e in client.listdir("/")] + print("ft names", tag, names) + print("ft code.py listed", tag, "code.py" in names) + +print("battery before", battery_present()) +list_root("before") +print("client ready for reload") + +# The test reloads the workflow device now. Give it time to come back. +time.sleep(20) +print("still connected", connection.connected) +list_root("after") +print("battery after", battery_present()) +print("client done") +connection.disconnect() +""" + +CLIENT_SETTINGS = "CIRCUITPY_BLE_WORKFLOW = false\n" + +CLIENT_DRIVE = { + "code.py": CLIENT_CODE, + "settings.toml": CLIENT_SETTINGS, + **_ADAFRUIT_BLE_FILE_TRANSFER, + **_ADAFRUIT_BLE, +} + + +def _reload_workflow_and_check(workflow, client): + client.serial.wait_for("client ready for reload", timeout=90) + + # Interrupt code.py, then reload from the "Press any key" prompt. Before the + # fix, the first byte at that prompt restarted the BLE stack and dropped the + # workflow connection. + workflow.serial.write("\x03") + workflow.serial.wait_for("KeyboardInterrupt", timeout=30) + workflow.serial.write("\x04") + workflow.serial.wait_for("RunReason.REPL_RELOAD", timeout=30) + + client.serial.wait_for("client done", timeout=90) + + client_output = client.serial.all_output + assert "paired True" in client_output, f"pairing did not succeed: {client_output}" + assert "ft code.py listed before True" in client_output, ( + f"listdir before reload did not include code.py: {client_output}" + ) + assert "still connected True" in client_output, ( + f"workflow connection did not survive the reload: {client_output}" + ) + assert "ft code.py listed after True" in client_output, ( + f"listdir after reload did not include code.py: {client_output}" + ) + + workflow_output = workflow.serial.all_output + assert "safe mode" not in workflow_output.lower(), ( + f"workflow device entered safe mode: {workflow_output}" + ) + return client_output + + +@pytest.mark.port_resets(3) +@pytest.mark.duration(90) +@pytest.mark.circuitpy_drive({"code.py": WORKFLOW_IDLE_CODE}) +@pytest.mark.circuitpy_drive(CLIENT_DRIVE) +def test_bsim_workflow_reload_keeps_connection(board, bsim_phy, circuitpython1, circuitpython2): + """A Ctrl-D reload with no user services leaves the workflow connection up.""" + client_output = _reload_workflow_and_check(circuitpython1, circuitpython2) + assert "battery before False" in client_output + assert "battery after False" in client_output + + +@pytest.mark.port_resets(3) +@pytest.mark.duration(90) +@pytest.mark.circuitpy_drive({"code.py": WORKFLOW_SERVICE_CODE}) +@pytest.mark.circuitpy_drive(CLIENT_DRIVE) +def test_bsim_workflow_reload_removes_user_service( + board, bsim_phy, circuitpython1, circuitpython2 +): + """A reload removes a user-created service without dropping the connection.""" + workflow = circuitpython1 + client_output = _reload_workflow_and_check(workflow, circuitpython2) + assert "user service created" in workflow.serial.all_output + assert "battery before True" in client_output, ( + f"client did not see the user service before the reload: {client_output}" + ) + assert "battery after False" in client_output, ( + f"user service still present after the reload: {client_output}" + ) From dc747565dae5ab7c5d74adc5a4a1a4c5a8cb1a16 Mon Sep 17 00:00:00 2001 From: Dan Halbert Date: Sun, 13 Sep 2026 15:39:51 -0400 Subject: [PATCH 2/2] supervisor: time the BLE discovery window from after the bond check `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 --- supervisor/shared/bluetooth/bluetooth.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/supervisor/shared/bluetooth/bluetooth.c b/supervisor/shared/bluetooth/bluetooth.c index 15cddd7d28c..e5b9c0eb290 100644 --- a/supervisor/shared/bluetooth/bluetooth.c +++ b/supervisor/shared/bluetooth/bluetooth.c @@ -232,13 +232,13 @@ void supervisor_bluetooth_init(void) { #if CIRCUITPY_STATUS_LED status_led_init(); #endif - uint64_t start_ticks = supervisor_ticks_ms64(); - uint64_t diff = 0; if (ble_mode != 0) { boot_in_discovery_mode = true; reset_state = 0x0; } bool bonded = common_hal_bleio_adapter_is_bonded_to_central(&common_hal_bleio_adapter_obj); + uint64_t start_ticks = supervisor_ticks_ms64(); + uint64_t diff = 0; // Don't go into discovery mode when waking from deep sleep. But if we're already bonded, // BLE workflow can continue after deep sleep.