diff --git a/.gitmodules b/.gitmodules index f241a4c7eae..5d544b18cce 100644 --- a/.gitmodules +++ b/.gitmodules @@ -421,3 +421,6 @@ [submodule "frozen/CircuitPython_edupico2_paj7620"] path = frozen/CircuitPython_edupico2_paj7620 url = https://github.com/CytronTechnologies/CircuitPython_edupico2_paj7620.git +[submodule "frozen/Adafruit_CircuitPython_BLE_File_Transfer"] + path = frozen/Adafruit_CircuitPython_BLE_File_Transfer + url = https://github.com/adafruit/Adafruit_CircuitPython_BLE_File_Transfer.git diff --git a/frozen/Adafruit_CircuitPython_BLE_File_Transfer b/frozen/Adafruit_CircuitPython_BLE_File_Transfer new file mode 160000 index 00000000000..14c0870bc91 --- /dev/null +++ b/frozen/Adafruit_CircuitPython_BLE_File_Transfer @@ -0,0 +1 @@ +Subproject commit 14c0870bc915aba90cf6e8a4002adeb563bc95fe diff --git a/ports/zephyr-cp/boards/nrf54lm20bsim_nrf54lm20a_cpuapp.conf b/ports/zephyr-cp/boards/nrf54lm20bsim_nrf54lm20a_cpuapp.conf index 784cb782b4d..e2a9595c2a3 100644 --- a/ports/zephyr-cp/boards/nrf54lm20bsim_nrf54lm20a_cpuapp.conf +++ b/ports/zephyr-cp/boards/nrf54lm20bsim_nrf54lm20a_cpuapp.conf @@ -12,6 +12,14 @@ CONFIG_BT_CTLR_TX_PWR_DYNAMIC_CONTROL=y # Override Kconfig default CONFIG_BT_BUF_CMD_TX_COUNT=2 +# The bsim native_sim does not drive the BLE controller ticker time-slot +# mechanism used for radio-synchronized flash operations. With the default +# SOC_FLASH_NRF_RADIO_SYNC_TICKER, flash erase/write waits on a semaphore +# that is never given, hanging for ~34s (FLASH_TIMEOUT_MS) per operation. +# Disable radio sync so flash operations run directly, matching nrf5340bsim +# (which uses an IPC-based controller and so defaults to _NONE). +CONFIG_SOC_FLASH_NRF_RADIO_SYNC_NONE=y + CONFIG_TRACING=y CONFIG_TRACING_PERFETTO=y CONFIG_TRACING_SYNC=y diff --git a/ports/zephyr-cp/boards/nrf54lm20dk_nrf54lm20a_cpuapp.conf b/ports/zephyr-cp/boards/nrf54lm20dk_nrf54lm20a_cpuapp.conf index e6749ae6399..19f8cea27a2 100644 --- a/ports/zephyr-cp/boards/nrf54lm20dk_nrf54lm20a_cpuapp.conf +++ b/ports/zephyr-cp/boards/nrf54lm20dk_nrf54lm20a_cpuapp.conf @@ -1 +1,10 @@ CONFIG_BT_CTLR_TX_PWR_DYNAMIC_CONTROL=y + +# Work around a latent bug in soc_flash_nrf_ticker.c: with the default +# NRF_RRAM_WRITE_BUFFER_SIZE=1, FLASH_SLOT_WRITE (500 us) < +# FLASH_SYNC_SWITCHING_TIME (1700 us), so `interval = duration - +# FLASH_SYNC_SWITCHING_TIME` underflows and the next flash slot is scheduled +# ~71 minutes out. Any write larger than a single 16-byte slot then times out +# (-ETIMEDOUT / -116), e.g. persisting bond keys right after pairing. +# See zephyr/tests/boards/nrf/rram/overlay-radio_sync.conf. +CONFIG_NRF_RRAM_WRITE_BUFFER_SIZE=32 diff --git a/ports/zephyr-cp/common-hal/_bleio/Adapter.c b/ports/zephyr-cp/common-hal/_bleio/Adapter.c index ac2b1948eb7..6518c5ac114 100644 --- a/ports/zephyr-cp/common-hal/_bleio/Adapter.c +++ b/ports/zephyr-cp/common-hal/_bleio/Adapter.c @@ -12,6 +12,7 @@ #include #include #include +#include #include #include #include @@ -20,18 +21,41 @@ #include "py/runtime.h" #include "bindings/zephyr_kernel/__init__.h" #include "shared-bindings/_bleio/__init__.h" +#include "common-hal/_bleio/__init__.h" #include "shared-bindings/_bleio/Adapter.h" #include "shared-bindings/_bleio/Address.h" #include "shared-module/_bleio/Address.h" #include "shared-module/_bleio/ScanResults.h" +#include "supervisor/background_callback.h" +#include "supervisor/shared/bluetooth/bluetooth.h" #include "supervisor/shared/tick.h" bleio_connection_internal_t bleio_connections[BLEIO_TOTAL_CONNECTION_COUNT]; +// Background pump: drains the BLE file-transfer / serial PacketBuffers by +// calling supervisor_bluetooth_background(). Queued from GATT write callbacks +// and connection events so the VM processes incoming data promptly. +static background_callback_t bluetooth_background_cb = {NULL, NULL}; + +static void bluetooth_adapter_background(void *data) { + (void)data; + supervisor_bluetooth_background(); +} + +void bleio_request_bluetooth_background(void) { + if (bluetooth_background_cb.fun != NULL) { + background_callback_add_core(&bluetooth_background_cb); + } +} + static bool scan_callbacks_registered = false; static bleio_scanresults_obj_t *active_scan_results = NULL; static struct bt_le_scan_cb scan_callbacks; static bool ble_advertising = false; +// True when advertising was started by the BLE workflow (supervisor) rather +// 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; #define BLEIO_ADV_MAX_FIELDS 16 @@ -142,27 +166,65 @@ static void bleio_connection_release(bleio_connection_internal_t *connection, ui common_hal_bleio_adapter_obj.connection_objs = NULL; } +// Per-connection ATT MTU exchange parameters. The params struct must persist +// until the exchange callback fires, so it lives for the lifetime of the slot. +static struct bt_gatt_exchange_params mtu_exchange_params[BLEIO_TOTAL_CONNECTION_COUNT]; + +static void on_mtu_exchanged(struct bt_conn *conn, uint8_t err, + struct bt_gatt_exchange_params *params) { + (void)conn; + (void)params; + if (err == 0) { + // Wake the workflow so outgoing_packet_length is recomputed with the + // now-larger negotiated MTU. + bleio_request_bluetooth_background(); + } +} + static void bleio_connected_cb(struct bt_conn *conn, uint8_t err) { if (err != 0) { return; } - if (bleio_connection_track(conn) == NULL) { + bleio_connection_internal_t *connection = bleio_connection_track(conn); + if (connection == NULL) { bt_conn_disconnect(conn, BT_HCI_ERR_CONN_LIMIT_EXCEEDED); return; } + // Initiate an ATT MTU exchange so the negotiated MTU reflects the larger + // payload our stack supports (CONFIG_BT_L2CAP_TX_MTU). Many centrals do + // this themselves, but if they don't we'd be stuck at the default 23-byte + // MTU (20-byte payload). That forces the file-transfer workflow to split + // protocol messages across notifications in ways peers can't reassemble + // (e.g. a listdir_entry whose second fragment begins with a 0x00 flags + // byte is misread as "unknown command 0x00"). bt_gatt_exchange_mtu returns + // -EALREADY if the peer already initiated, so this is safe either way. + size_t idx = (size_t)(connection - bleio_connections); + mtu_exchange_params[idx].func = on_mtu_exchanged; + int mtu_err = bt_gatt_exchange_mtu(conn, &mtu_exchange_params[idx]); + (void)mtu_err; + // 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 // thread context). ble_advertising = false; + ble_advertising_internal = false; common_hal_bleio_adapter_obj.connection_objs = NULL; + + // Pump the workflow once now, and arm the recurring background callback + // so future GATT writes / events get drained by the VM. + bluetooth_background_cb.fun = bluetooth_adapter_background; + bluetooth_background_cb.data = NULL; + bluetooth_adapter_background(NULL); } static void bleio_disconnected_cb(struct bt_conn *conn, uint8_t reason) { + bleio_connection_discovery_abort(); bleio_connection_release(bleio_connection_find_by_conn(conn), reason); + bleio_request_bluetooth_background(); } static void bleio_security_changed_cb(struct bt_conn *conn, bt_security_t level, @@ -240,7 +302,7 @@ static size_t bleio_parse_adv_data(const uint8_t *raw, size_t raw_len, struct bt if (offset + field_len + 1 > raw_len || count >= out_len || storage_offset + data_len > storage_len) { - mp_raise_ValueError(MP_ERROR_TEXT("Invalid advertising data")); + return 0; } uint8_t type = raw[offset + 1]; memcpy(storage + storage_offset, raw + offset + 2, data_len); @@ -328,13 +390,15 @@ mp_int_t common_hal_bleio_adapter_get_tx_power(bleio_adapter_obj_t *self) { return power; } -void common_hal_bleio_adapter_set_tx_power(bleio_adapter_obj_t *self, mp_int_t tx_power) { +// Non-raising variant of common_hal_bleio_adapter_set_tx_power for use from the +// BLE workflow, which runs outside the VM. Returns 0 on success. +static int bleio_adapter_set_tx_power_noraise(mp_int_t tx_power) { struct bt_hci_cp_vs_write_tx_power_level *cp; struct net_buf *buf, *rsp = NULL; buf = bt_hci_cmd_alloc(K_MSEC(3000)); if (!buf) { - mp_raise_msg(&mp_type_MemoryError, NULL); + return -ENOMEM; } cp = net_buf_add(buf, sizeof(*cp)); cp->handle_type = BT_HCI_VS_LL_HANDLE_TYPE_ADV; @@ -343,10 +407,18 @@ void common_hal_bleio_adapter_set_tx_power(bleio_adapter_obj_t *self, mp_int_t t int err = bt_hci_cmd_send_sync(BT_HCI_OP_VS_WRITE_TX_POWER_LEVEL, buf, &rsp); if (err) { - raise_zephyr_error(err); + return err; } net_buf_unref(rsp); + return 0; +} + +void common_hal_bleio_adapter_set_tx_power(bleio_adapter_obj_t *self, mp_int_t tx_power) { + int err = bleio_adapter_set_tx_power_noraise(tx_power); + if (err) { + raise_zephyr_error(err); + } } bleio_address_obj_t *common_hal_bleio_adapter_get_address(bleio_adapter_obj_t *self) { @@ -380,25 +452,31 @@ void common_hal_bleio_adapter_set_name(bleio_adapter_obj_t *self, const char *na } } -void common_hal_bleio_adapter_start_advertising(bleio_adapter_obj_t *self, - bool connectable, bool anonymous, uint32_t timeout, mp_float_t interval, - mp_buffer_info_t *advertising_data_bufinfo, - mp_buffer_info_t *scan_response_data_bufinfo, +// Internal start_advertising used by the BLE workflow (file transfer + serial +// services). Runs outside the VM, so it must not raise. Returns 0 on success or +// a positive errno on failure. A timeout of 0 means advertise indefinitely. +// This is the core implementation; common_hal_bleio_adapter_start_advertising() +// delegates here and translates errors into exceptions. +uint32_t _common_hal_bleio_adapter_start_advertising(bleio_adapter_obj_t *self, + bool connectable, bool anonymous, uint32_t timeout, float interval, + const uint8_t *advertising_data, uint16_t advertising_data_len, + const uint8_t *scan_response_data, uint16_t scan_response_data_len, mp_int_t tx_power, const bleio_address_obj_t *directed_to) { (void)directed_to; (void)interval; + (void)anonymous; + (void)timeout; - if (advertising_data_bufinfo->len > BLEIO_ADV_MAX_DATA_LEN || - scan_response_data_bufinfo->len > BLEIO_ADV_MAX_DATA_LEN) { - mp_raise_NotImplementedError(NULL); - } - - if (timeout != 0) { - mp_raise_NotImplementedError(NULL); + if (advertising_data_len > BLEIO_ADV_MAX_DATA_LEN || + scan_response_data_len > BLEIO_ADV_MAX_DATA_LEN) { + return (uint32_t)EINVAL; } + // Don't disturb advertising that is already active (either user code or the + // workflow's own previous advert). The caller is responsible for stopping + // first if a restart is desired. if (ble_advertising) { - raise_zephyr_error(-EALREADY); + return (uint32_t)EBUSY; } bt_addr_le_t id_addrs[CONFIG_BT_ID_MAX]; @@ -407,30 +485,31 @@ void common_hal_bleio_adapter_start_advertising(bleio_adapter_obj_t *self, if (id_count == 0 || bt_addr_le_eq(&id_addrs[BT_ID_DEFAULT], BT_ADDR_LE_ANY)) { int id = bt_id_create(NULL, NULL); if (id < 0) { - printk("Failed to create identity address: %d\n", id); - raise_zephyr_error(id); + return (uint32_t)(-id); } } - size_t adv_count = bleio_parse_adv_data(advertising_data_bufinfo->buf, - advertising_data_bufinfo->len, + size_t adv_count = bleio_parse_adv_data(advertising_data, + advertising_data_len, adv_data, BLEIO_ADV_MAX_FIELDS, adv_data_storage, sizeof(adv_data_storage)); + if (adv_count == 0) { + return (uint32_t)EINVAL; + } size_t scan_resp_count = 0; - if (scan_response_data_bufinfo->len > 0) { - scan_resp_count = bleio_parse_adv_data(scan_response_data_bufinfo->buf, - scan_response_data_bufinfo->len, + if (scan_response_data_len > 0) { + scan_resp_count = bleio_parse_adv_data(scan_response_data, + scan_response_data_len, scan_resp_data, BLEIO_ADV_MAX_FIELDS, scan_resp_storage, sizeof(scan_resp_storage)); - } - - if (anonymous) { - mp_raise_NotImplementedError(NULL); + if (scan_resp_count == 0) { + return (uint32_t)EINVAL; + } } struct bt_le_adv_param adv_params; @@ -454,15 +533,75 @@ void common_hal_bleio_adapter_start_advertising(bleio_adapter_obj_t *self, NULL); } - common_hal_bleio_adapter_set_tx_power(self, tx_power); + // Best-effort TX power: the vendor HCI command may not exist on all + // controllers, so ignore failures here. + (void)bleio_adapter_set_tx_power_noraise(tx_power); - raise_zephyr_error(bt_le_adv_start(&adv_params, + int err = bt_le_adv_start(&adv_params, adv_data, adv_count, scan_resp_count > 0 ? scan_resp_data : NULL, - scan_resp_count)); + scan_resp_count); + if (err) { + return (uint32_t)(-err); + } ble_advertising = true; + // Default to workflow-owned; the public wrapper overrides this for user code. + ble_advertising_internal = true; + return 0; +} + +void common_hal_bleio_adapter_start_advertising(bleio_adapter_obj_t *self, + bool connectable, bool anonymous, uint32_t timeout, mp_float_t interval, + mp_buffer_info_t *advertising_data_bufinfo, + mp_buffer_info_t *scan_response_data_bufinfo, + mp_int_t tx_power, const bleio_address_obj_t *directed_to) { + (void)directed_to; + (void)interval; + + if (advertising_data_bufinfo->len > BLEIO_ADV_MAX_DATA_LEN || + scan_response_data_bufinfo->len > BLEIO_ADV_MAX_DATA_LEN) { + mp_raise_NotImplementedError(NULL); + } + + if (timeout != 0) { + mp_raise_NotImplementedError(NULL); + } + + if (anonymous) { + mp_raise_NotImplementedError(NULL); + } + + if (ble_advertising) { + if (!ble_advertising_internal) { + // User code is already advertising. + raise_zephyr_error(-EALREADY); + } + // The workflow is advertising. Stop it so user code can take over. + common_hal_bleio_adapter_stop_advertising(self); + } + + uint32_t status = _common_hal_bleio_adapter_start_advertising(self, + connectable, + anonymous, + timeout, + interval, + advertising_data_bufinfo->buf, + advertising_data_bufinfo->len, + scan_response_data_bufinfo->buf, + scan_response_data_bufinfo->len, + tx_power, + directed_to); + if (status == (uint32_t)EINVAL) { + mp_raise_ValueError(MP_ERROR_TEXT("Invalid advertising data")); + } + if (status != 0) { + raise_zephyr_error(-(int)status); + } + + // Mark as user-owned so the workflow won't clobber it. + ble_advertising_internal = false; } void common_hal_bleio_adapter_stop_advertising(bleio_adapter_obj_t *self) { @@ -472,6 +611,7 @@ void common_hal_bleio_adapter_stop_advertising(bleio_adapter_obj_t *self) { } bt_le_adv_stop(); ble_advertising = false; + ble_advertising_internal = false; } bool common_hal_bleio_adapter_get_advertising(bleio_adapter_obj_t *self) { @@ -737,6 +877,7 @@ void bleio_adapter_reset(bleio_adapter_obj_t *adapter) { adapter->connection_objs = NULL; active_scan_results = NULL; ble_advertising = false; + ble_advertising_internal = false; ble_adapter_enabled = bt_is_ready(); } diff --git a/ports/zephyr-cp/common-hal/_bleio/Adapter.h b/ports/zephyr-cp/common-hal/_bleio/Adapter.h index c15c698e2a5..d7411d2950e 100644 --- a/ports/zephyr-cp/common-hal/_bleio/Adapter.h +++ b/ports/zephyr-cp/common-hal/_bleio/Adapter.h @@ -27,3 +27,7 @@ typedef struct { 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. +void bleio_request_bluetooth_background(void); diff --git a/ports/zephyr-cp/common-hal/_bleio/Characteristic.c b/ports/zephyr-cp/common-hal/_bleio/Characteristic.c index af315f9f6b0..93ce48ac22f 100644 --- a/ports/zephyr-cp/common-hal/_bleio/Characteristic.c +++ b/ports/zephyr-cp/common-hal/_bleio/Characteristic.c @@ -12,6 +12,7 @@ #include #include "py/runtime.h" +#include "py/gc.h" #include "bindings/zephyr_kernel/__init__.h" #include "shared-bindings/_bleio/__init__.h" #include "shared-bindings/_bleio/Characteristic.h" @@ -95,6 +96,7 @@ uint16_t bleio_security_to_zephyr_perm( ssize_t bleio_char_read_cb(struct bt_conn *conn, const struct bt_gatt_attr *attr, void *buf, uint16_t len, uint16_t offset) { bleio_characteristic_obj_t *self = attr->user_data; + (void)conn; return bt_gatt_attr_read(conn, attr, buf, len, offset, self->current_value, self->current_value_len); } @@ -103,6 +105,7 @@ ssize_t bleio_char_write_cb(struct bt_conn *conn, const struct bt_gatt_attr *attr, const void *buf, uint16_t len, uint16_t offset, uint8_t flags) { bleio_characteristic_obj_t *self = attr->user_data; + (void)flags; if (offset + len > self->max_length) { return BT_GATT_ERR(BT_ATT_ERR_INVALID_OFFSET); } @@ -134,6 +137,9 @@ bleio_characteristic_properties_t common_hal_bleio_characteristic_get_properties } mp_obj_tuple_t *common_hal_bleio_characteristic_get_descriptors(bleio_characteristic_obj_t *self) { + if (self->descriptor_list == NULL) { + return mp_obj_new_tuple(0, NULL); + } return mp_obj_new_tuple(self->descriptor_list->len, self->descriptor_list->items); } @@ -189,10 +195,18 @@ void common_hal_bleio_characteristic_construct(bleio_characteristic_obj_t *self, self->max_length = max_length; self->fixed_length = fixed_length; self->observer = mp_const_none; - self->descriptor_list = mp_obj_new_list(0, NULL); + // The descriptor list is an mp_obj (GC object). When constructed before + // gc_init() (e.g. the BLE workflow at boot), the GC heap isn't available, + // so leave it NULL and lazily create it on first use. Matches the nordic + // port's handling. + if (gc_alloc_possible()) { + self->descriptor_list = mp_obj_new_list(0, NULL); + } else { + self->descriptor_list = NULL; + } // Allocate value buffer - self->current_value = m_malloc(max_length); + self->current_value = port_malloc(max_length, false); memset(self->current_value, 0, max_length); self->current_value_alloc = max_length; self->current_value_len = 0; @@ -247,6 +261,12 @@ bool common_hal_bleio_characteristic_deinited(bleio_characteristic_obj_t *self) void common_hal_bleio_characteristic_deinit(bleio_characteristic_obj_t *self) { // Nothing to do - service handles unregistration + if (self->current_value != NULL) { + port_free(self->current_value); + self->current_value = NULL; + self->current_value_alloc = 0; + self->current_value_len = 0; + } } // Struct for tracking GATT notification subscriptions on remote characteristics. @@ -374,6 +394,9 @@ void common_hal_bleio_characteristic_set_value(bleio_characteristic_obj_t *self, void common_hal_bleio_characteristic_add_descriptor(bleio_characteristic_obj_t *self, bleio_descriptor_obj_t *descriptor) { + if (self->descriptor_list == NULL) { + self->descriptor_list = mp_obj_new_list(0, NULL); + } mp_obj_list_append(MP_OBJ_FROM_PTR(self->descriptor_list), MP_OBJ_FROM_PTR(descriptor)); // Descriptors added after characteristic construction would need diff --git a/ports/zephyr-cp/common-hal/_bleio/Connection.c b/ports/zephyr-cp/common-hal/_bleio/Connection.c index 8381fe28a24..77942728a5f 100644 --- a/ports/zephyr-cp/common-hal/_bleio/Connection.c +++ b/ports/zephyr-cp/common-hal/_bleio/Connection.c @@ -77,6 +77,18 @@ static discovery_context_t *active_discovery_ctx; static sys_slist_t discovered_list; static struct bt_gatt_discover_params discovery_params; +// Called from the disconnect callback (BT workqueue context) to abort any +// in-flight GATT discovery. Without this the main thread's spin loop would +// hang waiting for a callback that will never come, and the caller would then +// NULL-dereference the now-cleared connection in bt_gatt_discover(). +void bleio_connection_discovery_abort(void) { + discovery_context_t *ctx = active_discovery_ctx; + if (ctx != NULL) { + ctx->err = -ENOTCONN; + ctx->done = true; + } +} + static uint8_t on_service_discovered(struct bt_conn *conn, const struct bt_gatt_attr *attr, struct bt_gatt_discover_params *params) { @@ -152,6 +164,8 @@ typedef struct { // Forward declaration for use by descriptor discovery. static void free_discovered_list(void); +static void bleio_discovery_check_connected(struct bt_conn *conn, + discovery_context_t *ctx); // Callback for descriptor discovery. static uint8_t on_descriptor_discovered(struct bt_conn *conn, @@ -237,6 +251,8 @@ static void create_descriptors_from_discovered(bleio_characteristic_obj_t *chara static void discover_descriptors_for_characteristic(struct bt_conn *conn, discovery_context_t *ctx, bleio_characteristic_obj_t *characteristic, uint16_t end_handle) { + // Bail cleanly if the link dropped before this phase began. + bleio_discovery_check_connected(conn, ctx); uint16_t start = characteristic->handle + 1; if (start > end_handle) { return; @@ -266,6 +282,9 @@ static void discover_descriptors_for_characteristic(struct bt_conn *conn, RUN_BACKGROUND_TASKS; } + // The link may have dropped while we were waiting. + bleio_discovery_check_connected(conn, ctx); + create_descriptors_from_discovered(characteristic); } @@ -332,9 +351,25 @@ static void free_discovered_list(void) { } } +// The link dropped during discovery: the disconnect callback cleared +// connection->conn (so a subsequent bt_gatt_discover() would NULL-deref, since +// CONFIG_ASSERT is off) and/or aborted the active discovery (ctx->err set). +// Stop discovery cleanly with a "Not connected" exception instead of +// crashing or hanging on the spin loop. +static void bleio_discovery_check_connected(struct bt_conn *conn, + discovery_context_t *ctx) { + if (conn == NULL || ctx->err != 0) { + free_discovered_list(); + active_discovery_ctx = NULL; + mp_raise_bleio_BluetoothError(MP_ERROR_TEXT("Not connected")); + } +} + // Discover characteristics for a single remote service. static void discover_characteristics_for_service(struct bt_conn *conn, discovery_context_t *ctx, bleio_service_obj_t *service) { + // Bail cleanly if the link dropped before this phase began. + bleio_discovery_check_connected(conn, ctx); // Need at least 2 handles: one for the service declaration, one for a characteristic if (service->end_handle <= service->start_handle) { return; @@ -365,6 +400,9 @@ static void discover_characteristics_for_service(struct bt_conn *conn, RUN_BACKGROUND_TASKS; } + // The link may have dropped while we were waiting. + bleio_discovery_check_connected(conn, ctx); + // Create CP objects outside of callback context where MP allocations are safe. // This drains and frees the list nodes. char_with_decl_t chars[16]; @@ -576,6 +614,9 @@ mp_obj_tuple_t *common_hal_bleio_connection_discover_remote_services(bleio_conne while (!ctx.done) { RUN_BACKGROUND_TASKS; } + + // The link may have dropped during primary discovery. + bleio_discovery_check_connected(connection->conn, &ctx); } else { mp_obj_iter_buf_t iter_buf; mp_obj_t iterable = mp_getiter(service_uuids_whitelist, &iter_buf); @@ -618,6 +659,10 @@ mp_obj_tuple_t *common_hal_bleio_connection_discover_remote_services(bleio_conne while (!ctx.done) { RUN_BACKGROUND_TASKS; } + + // The link may have dropped during this UUID's discovery; stop + // before the next iteration clears ctx.err and crashes on a NULL conn. + bleio_discovery_check_connected(connection->conn, &ctx); } } @@ -640,6 +685,9 @@ mp_obj_tuple_t *common_hal_bleio_connection_discover_remote_services(bleio_conne for (size_t i = 0; i < result_list->len; i++) { bleio_service_obj_t *svc = MP_OBJ_TO_PTR(result_list->items[i]); if (svc->start_handle < svc->end_handle) { + // discover_characteristics_for_service re-checks, but guard here + // too so we don't enter the phase after a mid-loop disconnect. + bleio_discovery_check_connected(connection->conn, &ctx); discover_characteristics_for_service(connection->conn, &ctx, svc); } } diff --git a/ports/zephyr-cp/common-hal/_bleio/PacketBuffer.c b/ports/zephyr-cp/common-hal/_bleio/PacketBuffer.c index 02e593fabfe..d7773193187 100644 --- a/ports/zephyr-cp/common-hal/_bleio/PacketBuffer.c +++ b/ports/zephyr-cp/common-hal/_bleio/PacketBuffer.c @@ -9,6 +9,7 @@ #include #include +#include #include #include "py/runtime.h" @@ -21,17 +22,23 @@ #include "shared-bindings/_bleio/PacketBuffer.h" #include "supervisor/shared/tick.h" +#include "supervisor/port_heap.h" #include "common-hal/_bleio/Characteristic.h" #include "common-hal/_bleio/PacketBuffer.h" +#include "common-hal/_bleio/Adapter.h" +#include "common-hal/_bleio/Connection.h" +#include "common-hal/_bleio/__init__.h" +#include "bindings/zephyr_kernel/__init__.h" // Zephyr's ring_buf is safe for single-producer/single-consumer without -// locks. The GATT callbacks (system workqueue) are the sole producer; -// the CircuitPython VM (main thread) is the sole consumer. +// locks. The GATT callbacks (system workqueue) are the sole producer of +// incoming data; the CircuitPython VM (main thread) is the sole consumer. // Forward declarations. static bool conn_is_valid(bleio_packet_buffer_obj_t *self); -static bool send_pending(bleio_packet_buffer_obj_t *self); +static void packet_buffer_send_work_handler(struct k_work *work); +static void notify_complete_cb(struct bt_conn *conn, void *user_data); // Called from Zephyr GATT callbacks (system workqueue context). // Wraps incoming data with a uint16_t length prefix and pushes into ringbuf. @@ -82,6 +89,9 @@ void bleio_packet_buffer_extend(bleio_packet_buffer_obj_t *self, ring_buf_put(&self->ringbuf, (uint8_t *)&packet_len, sizeof(uint16_t)); ring_buf_put(&self->ringbuf, data, len); + + // Wake the VM background task so it drains the ring buffer promptly. + bleio_request_bluetooth_background(); } void bleio_packet_buffer_set_conn(bleio_packet_buffer_obj_t *self, @@ -89,12 +99,16 @@ void bleio_packet_buffer_set_conn(bleio_packet_buffer_obj_t *self, self->conn = conn; } -// Completion callback for bt_gatt_notify_cb — called when the PDU has been -// sent (or the buffer freed). Drains any accumulated pending data. +// Completion callback for bt_gatt_notify_cb. Runs on the system workqueue (per +// the Zephyr bt_gatt_notify_cb contract) — the same context send_work runs in +// — so notify_in_flight is single-threaded and needs no lock. static void notify_complete_cb(struct bt_conn *conn, void *user_data) { + (void)conn; bleio_packet_buffer_obj_t *self = (bleio_packet_buffer_obj_t *)user_data; - self->packet_queued = false; - send_pending(self); + self->notify_in_flight = false; + // Drain any data that accumulated in the pipe while this notify was in + // flight. k_work_submit is a no-op if the work is already queued. + k_work_submit(&self->send_work); } // Returns true if the tracked connection is still connected. @@ -112,88 +126,135 @@ static bool conn_is_valid(bleio_packet_buffer_obj_t *self) { return true; } -// Send the pending outgoing buffer via GATT notify. -// Returns true if sent successfully (or terminal failure). -static bool send_pending(bleio_packet_buffer_obj_t *self) { - if (self->pending_size == 0) { - return true; +// The deferred sender. Runs only on the system workqueue (submitted by the VM +// on write/flush and by notify_complete_cb on completion). It is the sole +// consumer of outgoing_pipe, and the only toucher of send_size / +// notify_in_flight / outgoing_buffer (server-side), so those need no lock. +static void packet_buffer_send_work_handler(struct k_work *work) { + bleio_packet_buffer_obj_t *self = + CONTAINER_OF(work, bleio_packet_buffer_obj_t, send_work); + + // A notification is awaiting its completion callback; it will resubmit us. + if (self->notify_in_flight) { + return; } - if (self->characteristic == NULL || - self->characteristic->service == NULL || - self->characteristic->service->is_remote) { - self->pending_size = 0; - return true; + + // If nothing is staged from a previous attempt, pull from the pipe. + if (self->send_size == 0) { + bleio_characteristic_obj_t *c = self->characteristic; + if (c == NULL || c->service == NULL || c->service->is_remote) { + return; // server-side notify path only; clients write directly + } + if (!(c->props & CHAR_PROP_NOTIFY) || !c->service->registered) { + return; + } + + mp_int_t opl = common_hal_bleio_packet_buffer_get_outgoing_packet_length(self); + if (opl <= 0) { + return; // no connection / MTU yet + } + size_t to_read = MIN((size_t)opl, self->max_packet_size); + int n = k_pipe_read(&self->outgoing_pipe, self->outgoing_buffer, + to_read, K_NO_WAIT); + if (n <= 0) { + return; // pipe empty (-EAGAIN or 0) + } + self->send_size = (size_t)n; } + // We have send_size bytes staged in outgoing_buffer. Try to notify. bleio_characteristic_obj_t *c = self->characteristic; - if (!(c->props & CHAR_PROP_NOTIFY) || !c->service->registered) { - self->pending_size = 0; - return true; - } + + // If the tracked connection is stale, clear it. + conn_is_valid(self); struct bt_gatt_notify_params params = { .attr = &c->service->attrs[c->value_attr_index], .data = self->outgoing_buffer, - .len = self->pending_size, + .len = self->send_size, .func = notify_complete_cb, .user_data = self, }; - // If the tracked connection is stale, clear it. - conn_is_valid(self); - int err = bt_gatt_notify_cb(self->conn, ¶ms); if (err == 0) { - self->pending_size = 0; - self->packet_queued = true; - return true; + // Accepted by the controller; the buffer was copied and will be sent + // on-air at the next connection event. Count it as handed off. + atomic_sub(&self->outgoing_pending, (atomic_val_t)self->send_size); + self->send_size = 0; + self->notify_in_flight = true; + // notify_complete_cb clears notify_in_flight and resubmits to drain + // any further accumulated data. + return; } if (err == -ENOTCONN) { - // Peer disconnected — clear tracking, discard pending. + // Peer disconnected — discard everything pending. (We're here only + // when notify_in_flight is clear, so no in-flight completion is owed.) self->conn = NULL; - self->pending_size = 0; - return true; + self->send_size = 0; + k_pipe_reset(&self->outgoing_pipe); // drop unread data + atomic_set(&self->outgoing_pending, 0); + return; } - // -ENOMEM (no TX buffer) — leave pending, caller will retry. - return false; + // -ENOMEM (no TX buffer available right now) — leave send_size staged and + // rely on the next completion (a TX buffer will free up, since -ENOMEM + // means the pool is full i.e. notifications are in flight), the next + // write(), or flush() to resubmit. We must NOT resubmit here: that would + // busy-loop the workqueue against a full pool. } -void common_hal_bleio_packet_buffer_construct( - bleio_packet_buffer_obj_t *self, bleio_characteristic_obj_t *characteristic, - size_t buffer_size, size_t max_packet_size) { +// Shared core for both the Python-facing (allocating) and workflow +// (caller-supplied static buffer) constructors. Wires the ring buffer, +// outgoing buffers, characteristic observer, and (for client-side) CCCD +// subscription. No GC heap allocation happens here. +static void packet_buffer_init_common(bleio_packet_buffer_obj_t *self, + bleio_characteristic_obj_t *characteristic, + uint8_t *ringbuf_data, size_t ringbuf_size, bool owns_ringbuf_data, + uint8_t *outgoing_buffer, bool owns_outgoing_buffer, + uint8_t *pipe_buffer, size_t pipe_size, bool owns_pipe_buffer, + size_t max_packet_size) { self->characteristic = characteristic; self->timeout_ms = 0; self->max_packet_size = max_packet_size; self->conn = NULL; self->client = (characteristic->service != NULL && characteristic->service->is_remote); - self->pending_size = 0; - self->packet_queued = false; - - // Allocate ring buffer: buffer_size packets, each with 2-byte length prefix - self->ringbuf_size = buffer_size * (sizeof(uint16_t) + max_packet_size); - self->ringbuf_data = m_malloc_without_collect(self->ringbuf_size); - ring_buf_init(&self->ringbuf, self->ringbuf_size, self->ringbuf_data); - - // Allocate outgoing buffer for pending writes - bleio_characteristic_properties_t props = - common_hal_bleio_characteristic_get_properties(characteristic); - if (self->client) { - // Client-side: we write to remote characteristic - self->outgoing_buffer = m_malloc_without_collect(max_packet_size); + self->outgoing_buffer = outgoing_buffer; + self->owns_outgoing_buffer = owns_outgoing_buffer; + self->pipe_buffer = pipe_buffer; + self->pipe_size = pipe_size; + self->owns_pipe_buffer = owns_pipe_buffer; + self->send_size = 0; + self->notify_in_flight = false; + atomic_set(&self->outgoing_pending, 0); + + self->ringbuf_data = ringbuf_data; + self->ringbuf_size = ringbuf_size; + self->owns_ringbuf_data = owns_ringbuf_data; + if (ringbuf_data != NULL && ringbuf_size > 0) { + ring_buf_init(&self->ringbuf, ringbuf_size, ringbuf_data); } else { - // Server-side: we notify via local characteristic - if (props & (CHAR_PROP_NOTIFY | CHAR_PROP_INDICATE)) { - self->outgoing_buffer = m_malloc_without_collect(max_packet_size); - } else { - self->outgoing_buffer = NULL; - } + // No incoming buffer (e.g. a server-side NOTIFY-only characteristic). + ring_buf_init(&self->ringbuf, 0, NULL); + } + + // The pipe feeds the deferred notifier. Only server-side notify + // characteristics need it; for client-side it's left empty (the client + // writes directly and synchronously). + if (pipe_buffer != NULL && pipe_size > 0) { + k_pipe_init(&self->outgoing_pipe, pipe_buffer, pipe_size); + k_work_init(&self->send_work, packet_buffer_send_work_handler); } - // Set ourselves as the characteristic's observer + // Set ourselves as the characteristic's observer so GATT write/notify + // callbacks push incoming data into our ring buffer. bleio_characteristic_set_observer(characteristic, MP_OBJ_FROM_PTR(self)); - // For client-side characteristics with NOTIFY/INDICATE, subscribe to notifications + bleio_characteristic_properties_t props = + common_hal_bleio_characteristic_get_properties(characteristic); + + // For client-side characteristics with NOTIFY/INDICATE, subscribe to + // notifications from the remote peer. if (self->client && (props & (CHAR_PROP_NOTIFY | CHAR_PROP_INDICATE))) { bool do_notify = (props & CHAR_PROP_NOTIFY) != 0; bool do_indicate = (props & CHAR_PROP_INDICATE) != 0; @@ -201,21 +262,95 @@ void common_hal_bleio_packet_buffer_construct( } } -// Allocation-free version for BLE workflow use (not yet implemented for Zephyr). +void common_hal_bleio_packet_buffer_construct( + bleio_packet_buffer_obj_t *self, bleio_characteristic_obj_t *characteristic, + size_t buffer_size, size_t max_packet_size) { + + bleio_characteristic_properties_t props = + common_hal_bleio_characteristic_get_properties(characteristic); + bool client = (characteristic->service != NULL && characteristic->service->is_remote); + + // Allocate ring buffer: buffer_size packets, each with 2-byte length prefix + size_t ringbuf_size = buffer_size * (sizeof(uint16_t) + max_packet_size); + uint8_t *ringbuf_data = port_malloc(ringbuf_size, false); + + // Allocate outgoing buffers. Client-side writes go out directly and only + // need one scratch buffer. Server-side notifications use a k_pipe (backed + // by pipe_buffer) plus a notify buffer (outgoing_buffer) the workqueue + // drains into. + uint8_t *outgoing_buffer = NULL; + uint8_t *pipe_buffer = NULL; + bool owns_outgoing = false; + bool owns_pipe = false; + if (client) { + outgoing_buffer = port_malloc(max_packet_size, false); + owns_outgoing = true; + } else if (props & (CHAR_PROP_NOTIFY | CHAR_PROP_INDICATE)) { + outgoing_buffer = port_malloc(max_packet_size, false); + pipe_buffer = port_malloc(max_packet_size, false); + owns_outgoing = true; + owns_pipe = true; + } + + packet_buffer_init_common(self, characteristic, + ringbuf_data, ringbuf_size, true, + outgoing_buffer, owns_outgoing, + pipe_buffer, max_packet_size, owns_pipe, + max_packet_size); +} + +// Allocation-free version for BLE workflow use. The caller supplies static +// buffers so this can run before gc_init() without touching the GC heap. +// outgoing_buffer1 backs the pipe; outgoing_buffer2 is the notify buffer. void _common_hal_bleio_packet_buffer_construct( bleio_packet_buffer_obj_t *self, bleio_characteristic_obj_t *characteristic, uint32_t *incoming_buffer, size_t incoming_buffer_size, uint32_t *outgoing_buffer1, uint32_t *outgoing_buffer2, size_t max_packet_size, ble_event_handler_t *static_handler_entry) { - (void)self; - (void)characteristic; - (void)incoming_buffer; - (void)incoming_buffer_size; - (void)outgoing_buffer1; - (void)outgoing_buffer2; - (void)max_packet_size; (void)static_handler_entry; - mp_raise_NotImplementedError(NULL); + + uint8_t *ringbuf_data = (uint8_t *)incoming_buffer; + size_t ringbuf_size = incoming_buffer_size; + + bleio_characteristic_properties_t props = + common_hal_bleio_characteristic_get_properties(characteristic); + bool client = (characteristic->service != NULL && characteristic->service->is_remote); + + uint8_t *outgoing_buffer = NULL; // client: write scratch; server: notify buf + uint8_t *pipe_buffer = NULL; // server: k_pipe backing store + bool owns_outgoing = false; + bool owns_pipe = false; + + if (client) { + // Client-side: outgoing_buffer1 is the write scratch; no pipe needed. + if (outgoing_buffer1 != NULL) { + outgoing_buffer = (uint8_t *)outgoing_buffer1; + } else { + outgoing_buffer = port_malloc(max_packet_size, false); + owns_outgoing = true; + } + } else if (props & (CHAR_PROP_NOTIFY | CHAR_PROP_INDICATE)) { + // Server-side: outgoing_buffer1 backs the pipe, outgoing_buffer2 is the + // notify buffer the workqueue drains into. + if (outgoing_buffer1 != NULL) { + pipe_buffer = (uint8_t *)outgoing_buffer1; + } else { + pipe_buffer = port_malloc(max_packet_size, false); + owns_pipe = true; + } + if (outgoing_buffer2 != NULL) { + outgoing_buffer = (uint8_t *)outgoing_buffer2; + } else { + outgoing_buffer = port_malloc(max_packet_size, false); + owns_outgoing = true; + } + } + + packet_buffer_init_common(self, characteristic, + ringbuf_data, ringbuf_size, false, + outgoing_buffer, owns_outgoing, + pipe_buffer, max_packet_size, owns_pipe, + max_packet_size); } mp_int_t common_hal_bleio_packet_buffer_readinto(bleio_packet_buffer_obj_t *self, @@ -283,32 +418,106 @@ mp_int_t common_hal_bleio_packet_buffer_write(bleio_packet_buffer_obj_t *self, MP_QSTR_max_packet_size); } - // If no room to append, wait until pending is sent. - if (len + self->pending_size > (size_t)outgoing_packet_length) { - while (self->pending_size != 0 && - !mp_hal_is_interrupted()) { - RUN_BACKGROUND_TASKS; + // Client-side (remote characteristic): write the request directly to the + // remote GATT server. The server-side path below buffers outgoing data in a + // k_pipe and drains it from the system workqueue via send_work, so handle + // client writes separately (and synchronously). + if (self->characteristic != NULL && + self->characteristic->service != NULL && + self->characteristic->service->is_remote) { + bleio_characteristic_obj_t *c = self->characteristic; + bleio_connection_obj_t *connection = MP_OBJ_TO_PTR(c->service->connection); + if (connection == NULL || connection->connection == NULL || + connection->connection->conn == NULL) { + return -1; } + struct bt_conn *conn = connection->connection->conn; + + // Combine header + data into the outgoing buffer (max_packet_size). + memcpy(self->outgoing_buffer, header, header_len); + memcpy(self->outgoing_buffer + header_len, data, len); + size_t total = header_len + len; + + if (c->props & CHAR_PROP_WRITE_NO_RESPONSE) { + // Fire-and-forget write. Retry on transient "no TX buffer" + // (-EAGAIN) so paced protocols (e.g. BLE file transfer) don't + // silently drop data. + int err; + while ((err = bt_gatt_write_without_response(conn, c->handle, + self->outgoing_buffer, total, false)) == -EAGAIN) { + RUN_BACKGROUND_TASKS; + } + if (err != 0) { + raise_zephyr_error(err); + } + } else if (c->props & CHAR_PROP_WRITE) { + bleio_gattc_write_sync(conn, c->handle, self->outgoing_buffer, total); + } else { + // No write property; nothing to send. + return -1; + } + return (mp_int_t)total; } + + // Server-side notify path: push bytes into the k_pipe. The pipe's built-in + // spinlock serializes us (sole producer) against send_work (sole consumer), + // so no manual lock is needed. If the pipe is full (send_work hasn't + // drained the previous packet yet), spin cooperatively until it has room. + // + // The header is a per-packet prefix: it's only written when the outgoing + // packet is empty (nothing pending), matching the documented write() + // semantics. outgoing_pending is the same notion as the old pending_size: + // bytes the controller hasn't accepted yet. + bool include_header = (atomic_get(&self->outgoing_pending) == 0); + if (mp_hal_is_interrupted()) { return -1; } - size_t num_bytes_written = 0; - - if (self->pending_size == 0) { - memcpy(self->outgoing_buffer, header, header_len); - self->pending_size += header_len; - num_bytes_written += header_len; + // Push header (if included) then data into the pipe, accounting each chunk + // in outgoing_pending as it lands. send_work may preempt us during + // RUN_BACKGROUND_TASKS (the system workqueue runs at higher priority) and + // drain+decrement those same bytes, so the increment must precede the + // yield to keep the counter from going negative. + mp_int_t num_bytes_written = 0; + const uint8_t *segs[2]; + size_t seglens[2]; + int nsegs = 0; + if (include_header && header_len > 0) { + segs[nsegs] = header; + seglens[nsegs] = header_len; + nsegs++; } - memcpy(self->outgoing_buffer + self->pending_size, data, len); - self->pending_size += len; - num_bytes_written += len; - - // Send immediately if no write is queued. - if (!self->packet_queued) { - send_pending(self); + segs[nsegs] = data; + seglens[nsegs] = len; + nsegs++; + + for (int s = 0; s < nsegs && !mp_hal_is_interrupted(); s++) { + const uint8_t *p = segs[s]; + size_t left = seglens[s]; + while (left > 0 && !mp_hal_is_interrupted()) { + int wrote = k_pipe_write(&self->outgoing_pipe, p, left, K_NO_WAIT); + if (wrote < 0) { + wrote = 0; // -EAGAIN: try again after yielding + } + if (wrote > 0) { + atomic_add(&self->outgoing_pending, (atomic_val_t)wrote); + num_bytes_written += wrote; + p += wrote; + left -= wrote; + } + if (left > 0) { + RUN_BACKGROUND_TASKS; // let send_work drain the pipe + } + } } + if (mp_hal_is_interrupted()) { + return -1; + } + + // Kick the deferred sender. k_work_submit is a no-op if already queued, so + // a burst of writes only schedules one drain. + k_work_submit(&self->send_work); return num_bytes_written; } @@ -345,24 +554,36 @@ mp_int_t common_hal_bleio_packet_buffer_get_outgoing_packet_length( !self->characteristic->service->is_remote && (common_hal_bleio_characteristic_get_properties(self->characteristic) & (CHAR_PROP_INDICATE | CHAR_PROP_NOTIFY))) { - // We are sending to a client via NOTIFY/INDICATE. - // Use max_packet_size since we don't track MTU dynamically here. - return MIN(self->max_packet_size, self->characteristic->max_length); + // We are sending to a client via NOTIFY/INDICATE. The maximum payload + // per packet is bounded by the negotiated ATT MTU (ATT_MTU - 3 for the + // opcode and handle in a Handle Value Notification PDU). Without a + // current connection we can't know the MTU, so return -1 to signal + // that writes aren't possible yet. + if (!conn_is_valid(self)) { + return -1; + } + uint16_t mtu = bt_gatt_get_mtu(self->conn); + if (mtu < 3) { + return -1; + } + mp_int_t mtu_payload = (mp_int_t)mtu - 3; + return MIN(MIN(mtu_payload, (mp_int_t)self->max_packet_size), + (mp_int_t)self->characteristic->max_length); } // Writing to remote characteristic or local without NOTIFY return MIN(self->characteristic->max_length, self->max_packet_size); } void common_hal_bleio_packet_buffer_flush(bleio_packet_buffer_obj_t *self) { - // With the completion callback, writes drain automatically. - // flush() just waits for any queued data to be sent. - while (self->pending_size > 0 && + // Wait until everything written has been handed to the controller (accepted + // by bt_gatt_notify_cb). send_work runs on the system workqueue, which is + // higher priority than this thread, so k_work_submit lets it preempt us; + // RUN_BACKGROUND_TASKS advances simulated time / lets completion callbacks + // fire so notify_in_flight clears. + while (atomic_get(&self->outgoing_pending) > 0 && !mp_hal_is_interrupted()) { + k_work_submit(&self->send_work); RUN_BACKGROUND_TASKS; - if (!send_pending(self)) { - // Couldn't send — wait and retry. - RUN_BACKGROUND_TASKS; - } } } @@ -376,12 +597,20 @@ void common_hal_bleio_packet_buffer_deinit(bleio_packet_buffer_obj_t *self) { } bleio_characteristic_clear_observer(self->characteristic); self->characteristic = NULL; - // Free ringbuf_data allocated with m_malloc_without_collect - m_free(self->ringbuf_data); + // Free buffers if we own them (port_malloc'd). The BLE workflow path + // supplies static buffers that must not be freed. + if (self->owns_ringbuf_data && self->ringbuf_data != NULL) { + port_free(self->ringbuf_data); + } self->ringbuf_data = NULL; - // Free outgoing buffer - m_free(self->outgoing_buffer); + if (self->owns_outgoing_buffer && self->outgoing_buffer != NULL) { + port_free(self->outgoing_buffer); + } self->outgoing_buffer = NULL; + if (self->owns_pipe_buffer && self->pipe_buffer != NULL) { + port_free(self->pipe_buffer); + } + self->pipe_buffer = NULL; } bool common_hal_bleio_packet_buffer_connected(bleio_packet_buffer_obj_t *self) { diff --git a/ports/zephyr-cp/common-hal/_bleio/PacketBuffer.h b/ports/zephyr-cp/common-hal/_bleio/PacketBuffer.h index 491852ec5fd..7d2e5a86168 100644 --- a/ports/zephyr-cp/common-hal/_bleio/PacketBuffer.h +++ b/ports/zephyr-cp/common-hal/_bleio/PacketBuffer.h @@ -9,6 +9,8 @@ #include +#include +#include #include #include "py/obj.h" @@ -26,12 +28,40 @@ typedef struct { uint8_t *ringbuf_data; size_t ringbuf_size; size_t max_packet_size; - // Outgoing pending buffer + + // Outgoing path. No manual mutex: the structures below all carry their own + // locking (k_pipe) or synchronization (k_work), and the remaining state is + // confined to a single execution context. + // + // Client-side (remote characteristic): outgoing_buffer is a VM-local + // scratch used to assemble header+data before a direct, synchronous GATT + // write. It is only ever touched by the VM thread. + // + // Server-side (local NOTIFY/INDICATE characteristic): the VM is the sole + // producer writing bytes into outgoing_pipe (a k_pipe, whose built-in + // spinlock serializes producer vs. consumer). send_work drains that pipe on + // the system workqueue — the same context bt_gatt_notify_cb runs its + // completion callback on — so send_size/notify_in_flight are touched only + // there. k_work's built-in submit deduplication replaces the old + // packet_queued flag. outgoing_pending counts bytes not yet accepted by the + // controller so flush() can poll for "all handed off". uint8_t *outgoing_buffer; - uint16_t pending_size; - bool packet_queued; + struct k_pipe outgoing_pipe; + uint8_t *pipe_buffer; + size_t pipe_size; + struct k_work send_work; + size_t send_size; + bool notify_in_flight; + atomic_t outgoing_pending; struct bt_conn *conn; bool client; + + // Ownership: true if the buffer was port_malloc'd and should be freed in + // deinit; false if it is a caller-supplied static buffer (the BLE workflow + // path, which runs before gc_init()). + bool owns_ringbuf_data; + bool owns_outgoing_buffer; + bool owns_pipe_buffer; } bleio_packet_buffer_obj_t; // Called from GATT callbacks (system workqueue context) to push diff --git a/ports/zephyr-cp/common-hal/_bleio/Service.c b/ports/zephyr-cp/common-hal/_bleio/Service.c index ee8cf35deee..5f0c7ee268b 100644 --- a/ports/zephyr-cp/common-hal/_bleio/Service.c +++ b/ports/zephyr-cp/common-hal/_bleio/Service.c @@ -25,6 +25,7 @@ #include "py/gc.h" #include "py/runtime.h" +#include "supervisor/port_heap.h" #include "bindings/zephyr_kernel/__init__.h" #include "shared-bindings/_bleio/Characteristic.h" #include "shared-bindings/_bleio/Service.h" @@ -52,8 +53,8 @@ static void service_ensure_capacity(bleio_service_obj_t *self, size_t needed) { while (new_capacity < self->attr_count + needed) { new_capacity *= 2; } - struct bt_gatt_attr *new_attrs = m_realloc(self->attrs, - new_capacity * sizeof(struct bt_gatt_attr)); + struct bt_gatt_attr *new_attrs = port_realloc(self->attrs, + new_capacity * sizeof(struct bt_gatt_attr), false); self->attrs = new_attrs; self->attr_capacity = new_capacity; } @@ -75,7 +76,7 @@ uint32_t _common_hal_bleio_service_construct(bleio_service_obj_t *self, // Allocate attrs array self->attr_capacity = INITIAL_ATTR_CAPACITY; - self->attrs = m_malloc(self->attr_capacity * sizeof(struct bt_gatt_attr)); + self->attrs = port_malloc(self->attr_capacity * sizeof(struct bt_gatt_attr), false); memset(self->attrs, 0, self->attr_capacity * sizeof(struct bt_gatt_attr)); self->attr_count = 0; @@ -105,6 +106,12 @@ void common_hal_bleio_service_deinit(bleio_service_obj_t *self) { bt_gatt_service_unregister(&self->zephyr_service); self->registered = false; } + if (self->attrs != NULL) { + port_free(self->attrs); + self->attrs = NULL; + self->attr_capacity = 0; + self->attr_count = 0; + } } void common_hal_bleio_service_from_remote_service(bleio_service_obj_t *self, diff --git a/ports/zephyr-cp/common-hal/_bleio/__init__.h b/ports/zephyr-cp/common-hal/_bleio/__init__.h index 63eec415311..72dc249d142 100644 --- a/ports/zephyr-cp/common-hal/_bleio/__init__.h +++ b/ports/zephyr-cp/common-hal/_bleio/__init__.h @@ -40,3 +40,8 @@ size_t bleio_gattc_read_sync(struct bt_conn *conn, uint16_t handle, uint8_t *buf, size_t len); void bleio_gattc_write_sync(struct bt_conn *conn, uint16_t handle, const uint8_t *data, size_t len); + +// Abort any in-flight remote GATT discovery; called from the disconnect +// callback so discover_remote_services() fails cleanly instead of hanging +// or NULL-dereferencing the cleared connection. +void bleio_connection_discovery_abort(void); diff --git a/ports/zephyr-cp/cptools/build_circuitpython.py b/ports/zephyr-cp/cptools/build_circuitpython.py index 55c069a1ddc..150a5d81266 100644 --- a/ports/zephyr-cp/cptools/build_circuitpython.py +++ b/ports/zephyr-cp/cptools/build_circuitpython.py @@ -479,8 +479,16 @@ async def build_circuitpython(): # noqa: C901 supervisor_source = [pathlib.Path(p) for p in supervisor_source] supervisor_source.extend(board_info["source_files"]) supervisor_source.extend(top.glob("supervisor/shared/*.c")) - if "_bleio" in enabled_modules: + ble_workflow_enabled = "_bleio" in enabled_modules + if ble_workflow_enabled: supervisor_source.append(top / "supervisor/shared/bluetooth/bluetooth.c") + # BLE workflow = file transfer + serial services, matching other ports. + supervisor_source.append(top / "supervisor/shared/bluetooth/file_transfer.c") + supervisor_source.append(top / "supervisor/shared/bluetooth/serial.c") + circuitpython_flags.append(f"-DCIRCUITPY_BLE_FILE_SERVICE={1 if ble_workflow_enabled else 0}") + circuitpython_flags.append( + f"-DCIRCUITPY_BLE_SERIAL_SERVICE={1 if ble_workflow_enabled else 0}" + ) supervisor_source.append(top / "supervisor/shared/translate/translate.c") if web_workflow_enabled: supervisor_source.extend(top.glob("supervisor/shared/web_workflow/*.c")) diff --git a/ports/zephyr-cp/debug.conf b/ports/zephyr-cp/debug.conf index ab6ae95d416..2f12db119f8 100644 --- a/ports/zephyr-cp/debug.conf +++ b/ports/zephyr-cp/debug.conf @@ -13,6 +13,13 @@ CONFIG_FRAME_POINTER=y CONFIG_FLASH_LOG_LEVEL_DBG=y CONFIG_LOG_MODE_IMMEDIATE=y +# Quiet the UDC device-controller drivers in the debug build. The DWC2 driver +# logs "Prepare RX 0x%02x doeptsiz 0x%x" (LOG_INF) on every OUT endpoint RX +# prepare, which is noisy. Drop UDC_DRIVER to WRN to keep errors/warnings +# while compiling out INF/DBG. (Shared by all udc drivers; only one is active +# per board.) +CONFIG_UDC_DRIVER_LOG_LEVEL_WRN=y + # Bluetooth: enable BT host debug logging so success paths print, not just # LOG_ERR. In particular bt_keys logs "Stored keys for " (LOG_DBG) on a # successful bt_keys_store(); without this only the failure message diff --git a/ports/zephyr-cp/prj.conf b/ports/zephyr-cp/prj.conf index abefcb199b9..864af4c6112 100644 --- a/ports/zephyr-cp/prj.conf +++ b/ports/zephyr-cp/prj.conf @@ -54,8 +54,15 @@ CONFIG_FPU=y CONFIG_MBEDTLS=y -# Override Kconfig default not taking effect +# Override Kconfig default not taking effect. The BLE file-transfer workflow +# packs up to BLEIO_PACKET_BUFFER_MAX_PACKET_SIZE (512) bytes per notification, +# so the L2CAP/ATT MTU must accommodate a 512-byte payload plus the 3-byte +# ATT header (opcode + handle). Without this the default with SMP is only 65, +# and notifications larger than the negotiated MTU fail with +# "No ATT channel for MTU". +CONFIG_BT_L2CAP_TX_MTU=515 CONFIG_BT_BUF_ACL_RX_SIZE=255 +CONFIG_BT_BUF_ACL_TX_SIZE=251 CONFIG_BT_RX_STACK_SIZE=2048 CONFIG_BT_LONG_WQ_STACK_SIZE=3072 CONFIG_MBEDTLS_BUILTIN=y diff --git a/ports/zephyr-cp/tests/bsim/test_bsim_basics.py b/ports/zephyr-cp/tests/bsim/test_bsim_basics.py index 96f40a86b40..3ef22470a38 100644 --- a/ports/zephyr-cp/tests/bsim/test_bsim_basics.py +++ b/ports/zephyr-cp/tests/bsim/test_bsim_basics.py @@ -14,7 +14,7 @@ @pytest.mark.circuitpy_drive({"code.py": BSIM_CODE}) @pytest.mark.circuitpy_drive({"code.py": BSIM_CODE}) -@pytest.mark.duration(3) +@pytest.mark.duration(5) def test_bsim_dual_instance_connect(bsim_phy, circuitpython1, circuitpython2, board): """Run two bsim instances on the same sim id and verify UART output.""" diff --git a/ports/zephyr-cp/tests/bsim/test_bsim_ble_packet_buffer.py b/ports/zephyr-cp/tests/bsim/test_bsim_ble_packet_buffer.py index efda0637aa3..450bd610948 100644 --- a/ports/zephyr-cp/tests/bsim/test_bsim_ble_packet_buffer.py +++ b/ports/zephyr-cp/tests/bsim/test_bsim_ble_packet_buffer.py @@ -3,6 +3,8 @@ """PacketBuffer tests for bsim.""" +import re + import pytest @@ -592,8 +594,12 @@ def test_bsim_packet_buffer_write_header(bsim_phy, circuitpython1, circuitpython # incoming_packet_length reflects the max we can receive (characteristic max_length) print("incoming", pb.incoming_packet_length) -# outgoing_packet_length is capped by max_packet_size -print("outgoing", pb.outgoing_packet_length) +# outgoing_packet_length requires a connection to know the negotiated ATT MTU, +# so without one it raises ValueError (server-side NOTIFY is MTU-bounded). +try: + print("outgoing", pb.outgoing_packet_length) +except ValueError as e: + print("outgoing valueerror", e) print("done") """ @@ -607,9 +613,9 @@ def test_bsim_packet_buffer_packet_lengths(bsim_phy, circuitpython): # Server-side local characteristic: # incoming = max_length = 20 - # outgoing = min(max_packet_size, max_length) = min(15, 20) = 15 + # outgoing requires a connection to know the ATT MTU, so it raises without one. assert "incoming 20" in output - assert "outgoing 15" in output + assert "outgoing valueerror" in output assert "done" in output @@ -783,3 +789,352 @@ def test_bsim_packet_buffer_reconnect(bsim_phy, circuitpython1, circuitpython2): assert "wrote first" in client_output assert "wrote second" in client_output + + +# ---- Test 8: outgoing_packet_length bounded by negotiated MTU ---- +# +# Regression test for the "No ATT channel for MTU" bug. The server-side +# PacketBuffer's outgoing_packet_length must be bounded by the negotiated +# ATT MTU (ATT_MTU - 3), not just max_packet_size. Without that bounding, +# the workflow packs a notification larger than the MTU and bt_gatt_notify_cb +# fails silently ("No ATT channel for MTU"), so the client never receives it. +# +# The characteristic is configured with max_length=600 and max_packet_size=600, +# deliberately larger than any plausible negotiated ATT MTU (the stack supports +# up to CONFIG_BT_L2CAP_TX_MTU = 515, payload 512). So without the fix, +# outgoing_packet_length returns 600 and the 600-byte notification always +# exceeds the MTU and is dropped — regardless of whether an MTU exchange +# happened. With the fix, outgoing_packet_length is ATT_MTU - 3 (≤ 512) and the +# notification fits and is delivered. + +BSIM_PB_MTU_SERVER_CODE = """\ +import _bleio +import time + +adapter = _bleio.adapter + +svc = _bleio.Service(_bleio.UUID(0xFFE0)) +char = _bleio.Characteristic.add_to_service( + svc, _bleio.UUID(0xFFE1), + properties=_bleio.Characteristic.WRITE | _bleio.Characteristic.NOTIFY, + read_perm=_bleio.Attribute.NO_ACCESS, + write_perm=_bleio.Attribute.OPEN, + max_length=600, fixed_length=False, +) + +# max_packet_size deliberately larger than any negotiated ATT MTU so that an +# unbounded outgoing_packet_length always produces an oversized, undeliverable +# notification regardless of MTU-exchange timing. +pb = _bleio.PacketBuffer(char, buffer_size=4, max_packet_size=600) +print("service created") + +name = b"CPPBMT" +advertisement = bytes((2, 0x01, 0x06, len(name) + 1, 0x09)) + name +adapter.start_advertising(advertisement, connectable=True) +print("advertising") + +for _ in range(80): + if adapter.connected: + break + time.sleep(0.1) +print("connected", adapter.connected) + +# Wait for the client to subscribe (CCCD write) and send a trigger write. +data = bytearray(600) +n = 0 +deadline = time.monotonic() + 5.0 +while n == 0 and time.monotonic() < deadline: + n = pb.readinto(data) + if n == 0: + time.sleep(0.05) +print("trigger", data[:n]) + +# The negotiated ATT MTU determines the largest notification payload. The +# connection's max_packet_length is ATT_MTU - 3. +conn = adapter.connections[0] +mtu_payload = conn.max_packet_length +print("mtu_payload", mtu_payload) + +# outgoing_packet_length must be bounded by the MTU, not max_packet_size. +opl = pb.outgoing_packet_length +print("outgoing", opl) + +# Send a response sized exactly to outgoing_packet_length, like the BLE +# file-transfer workflow does. With the fix this fits the MTU and is delivered. +response = bytes([0xAA]) * opl +pb.write(response) +print("sent", len(response)) + +for _ in range(80): + if not adapter.connected: + break + time.sleep(0.1) +print("done") +""" + +BSIM_PB_MTU_CLIENT_CODE = """\ +import _bleio +import time + +adapter = _bleio.adapter + +target = None +for entry in adapter.start_scan(timeout=6.0, active=True): + if entry.connectable and b"CPPBMT" in entry.advertisement_bytes: + target = entry.address + print("found server") + break +adapter.stop_scan() + +connection = adapter.connect(target, timeout=5.0) +print("connected", connection.connected) + +services = connection.discover_remote_services([_bleio.UUID(0xFFE0)]) +remote_char = services[0].characteristics[0] +print("found char, props", remote_char.properties) + +# Creating a client-side PacketBuffer on a NOTIFY characteristic writes the +# CCCD to subscribe, which gives the server a connection to notify. +client_pb = _bleio.PacketBuffer(remote_char, buffer_size=4, max_packet_size=600) +print("subscribed") + +# Trigger the server's response with a direct write. +remote_char.value = b"GO" +print("wrote trigger") + +# Read the notification response. Without the MTU fix the server sends an +# oversized notification that the ATT layer drops, so this times out. +buf = bytearray(600) +n = 0 +deadline = time.monotonic() + 5.0 +while n == 0 and time.monotonic() < deadline: + n = client_pb.readinto(buf) + if n == 0: + time.sleep(0.05) +if n > 0: + print("received_ok", n, bytes(buf[:n])) +else: + print("received_none") + +time.sleep(0.5) +connection.disconnect() + +timeout = time.monotonic() + 4.0 +while connection.connected and time.monotonic() < timeout: + time.sleep(0.1) +print("done") +""" + + +@pytest.mark.duration(20) +@pytest.mark.circuitpy_drive({"code.py": BSIM_PB_MTU_SERVER_CODE}) +@pytest.mark.circuitpy_drive({"code.py": BSIM_PB_MTU_CLIENT_CODE}) +def test_bsim_packet_buffer_outgoing_mtu_bounded(bsim_phy, circuitpython1, circuitpython2): + """Server-side outgoing_packet_length is bounded by the negotiated ATT MTU. + + Without the fix, outgoing_packet_length returns max_packet_size (200) and + the resulting notification exceeds the bsim default ATT MTU (23), so the + ATT layer drops it ("No ATT channel for MTU") and the client receives + nothing. With the fix, outgoing_packet_length is ATT_MTU - 3 (20) and the + notification is delivered to the client. + """ + server = circuitpython1 + client = circuitpython2 + + client.wait_until_done() + server.wait_until_done() + + server_output = server.serial.all_output + client_output = client.serial.all_output + + assert "service created" in server_output + assert "connected True" in server_output + assert "trigger" in server_output + + # outgoing_packet_length must not exceed the connection's MTU-derived + # max_packet_length. Without the fix this is 200 vs 20. + mtu_match = re.search(r"mtu_payload (\d+)", server_output) + outgoing_match = re.search(r"outgoing (\d+)", server_output) + assert mtu_match is not None, f"mtu_payload not printed: {server_output}" + assert outgoing_match is not None, f"outgoing not printed: {server_output}" + mtu_payload = int(mtu_match.group(1)) + outgoing = int(outgoing_match.group(1)) + assert outgoing <= mtu_payload, ( + f"outgoing_packet_length {outgoing} exceeds MTU payload {mtu_payload}; " + "notifications this size would be dropped by the ATT layer" + ) + assert outgoing < 600, f"outgoing_packet_length {outgoing} not bounded below max_packet_size" + + # The notification sized to outgoing_packet_length must actually arrive. + assert "received_ok" in client_output, ( + f"client never received the notification (MTU-bound bug): {client_output}" + ) + assert "received_none" not in client_output + + received_match = re.search(r"received_ok (\d+)", client_output) + assert received_match is not None + received = int(received_match.group(1)) + assert received == outgoing, f"client received {received} bytes, server sent {outgoing}" + + +# ---- Test: client-side PacketBuffer write (remote WRITE_NO_RESPONSE) ---- +# +# Covers the client-side write path in common_hal_bleio_packet_buffer_write +# (the is_remote branch): a CP central wraps a remote characteristic in a +# _bleio.PacketBuffer and writes to it via pb.write(...), which must reach the +# peripheral's server-side PacketBuffer. This mirrors how the BLE file-transfer +# library sends commands (raw.write). It also exercises the header+data combine +# in the client write path via pb.write(data, header=...). +# +# The characteristic uses WRITE_NO_RESPONSE | NOTIFY with open permissions, so +# no pairing is required and the test runs on both bsim boards. + +BSIM_PB_CLIENT_WRITE_SERVER_CODE = """\ +import _bleio +import time + +adapter = _bleio.adapter + +svc = _bleio.Service(_bleio.UUID(0xFFE0)) +char = _bleio.Characteristic.add_to_service( + svc, _bleio.UUID(0xFFE1), + properties=_bleio.Characteristic.WRITE_NO_RESPONSE | _bleio.Characteristic.NOTIFY, + read_perm=_bleio.Attribute.OPEN, + write_perm=_bleio.Attribute.OPEN, + max_length=20, fixed_length=False, +) + +# Server-side PacketBuffer: reads incoming WRITE_NO_RESPONSE, echoes via NOTIFY. +pb = _bleio.PacketBuffer(char, buffer_size=4, max_packet_size=20) +print("service created") + +name = b"CPPBCW" +advertisement = bytes((2, 0x01, 0x06, len(name) + 1, 0x09)) + name +adapter.start_advertising(advertisement, connectable=True) +print("advertising") + +for _ in range(80): + if adapter.connected: + break + time.sleep(0.1) +print("connected", adapter.connected) + +data = bytearray(20) +for _ in range(2): + n = 0 + deadline = time.monotonic() + 5.0 + while n == 0 and time.monotonic() < deadline: + n = pb.readinto(data) + if n == 0: + time.sleep(0.05) + print("received", bytes(data[:n])) + # Echo the packet straight back over NOTIFY. + pb.write(bytes(data[:n])) + print("echoed", bytes(data[:n])) + +for _ in range(80): + if not adapter.connected: + break + time.sleep(0.1) +print("done") +""" + +BSIM_PB_CLIENT_WRITE_CLIENT_CODE = """\ +import _bleio +import time + +adapter = _bleio.adapter + +target = None +for entry in adapter.start_scan(timeout=6.0, active=True): + if entry.connectable and b"CPPBCW" in entry.advertisement_bytes: + target = entry.address + print("found server") + break +adapter.stop_scan() + +connection = adapter.connect(target, timeout=5.0) +print("connected", connection.connected) + +services = connection.discover_remote_services([_bleio.UUID(0xFFE0)]) +print("discovered services", len(services)) + +remote_char = services[0].characteristics[0] +print("found char, props", remote_char.properties) + +# Client-side PacketBuffer on the remote characteristic: subscribes to NOTIFY +# (readinto) and writes to the remote (pb.write -> client-side write path). +cpb = _bleio.PacketBuffer(remote_char, buffer_size=4, max_packet_size=20) + +# Write 1: plain write (no header) — exercises bt_gatt_write_without_response. +cpb.write(b"PING") +print("wrote ping") + +# Write 2: write with a header — exercises the header+data combine in the +# client write path. "B" + "ODY" is sent as a single "BODY" packet. +cpb.write(b"ODY", header=b"B") +print("wrote body with header") + +# Read both NOTIFY echoes back. +buf = bytearray(20) +for _ in range(2): + n = 0 + deadline = time.monotonic() + 5.0 + while n == 0 and time.monotonic() < deadline: + n = cpb.readinto(buf) + if n == 0: + time.sleep(0.05) + print("client received", bytes(buf[:n])) + +connection.disconnect() + +timeout = time.monotonic() + 4.0 +while connection.connected and time.monotonic() < timeout: + time.sleep(0.1) +print("done") +""" + + +@pytest.mark.duration(20) +@pytest.mark.circuitpy_drive({"code.py": BSIM_PB_CLIENT_WRITE_SERVER_CODE}) +@pytest.mark.circuitpy_drive({"code.py": BSIM_PB_CLIENT_WRITE_CLIENT_CODE}) +def test_bsim_packet_buffer_client_write(bsim_phy, circuitpython1, circuitpython2): + """A CP central writes to a remote characteristic via a client-side + PacketBuffer (pb.write), and the peripheral echoes it back over NOTIFY. + + Covers the client-side write path (remote WRITE_NO_RESPONSE), including the + header+data combine, mirroring how the BLE file-transfer library sends + commands. + """ + server = circuitpython1 + client = circuitpython2 + + client.wait_until_done() + server.wait_until_done() + + server_output = server.serial.all_output + client_output = client.serial.all_output + + # Server received both client writes (plain + header-combined). + assert "received b'PING'" in server_output, ( + f"server never received the plain client write: {server_output}" + ) + assert "received b'BODY'" in server_output, ( + f"server never received the header-combined client write: {server_output}" + ) + assert "echoed b'PING'" in server_output + assert "echoed b'BODY'" in server_output + assert "done" in server_output + + # Client wrote both and got both echoes back over NOTIFY. + assert "wrote ping" in client_output, f"client plain write did not complete: {client_output}" + assert "wrote body with header" in client_output, ( + f"client header write did not complete: {client_output}" + ) + assert "client received b'PING'" in client_output, ( + f"client never received the PING echo: {client_output}" + ) + assert "client received b'BODY'" in client_output, ( + f"client never received the BODY echo: {client_output}" + ) + assert "done" in client_output diff --git a/ports/zephyr-cp/tests/bsim/test_bsim_ble_workflow_advertising.py b/ports/zephyr-cp/tests/bsim/test_bsim_ble_workflow_advertising.py new file mode 100644 index 00000000000..857dfb03eba --- /dev/null +++ b/ports/zephyr-cp/tests/bsim/test_bsim_ble_workflow_advertising.py @@ -0,0 +1,94 @@ +# SPDX-FileCopyrightText: 2026 Scott Shawcroft for Adafruit Industries +# SPDX-License-Identifier: MIT + +"""Advertising test for the supervisor BLE workflow (bsim). + +The supervisor's built-in BLE workflow (file-transfer + serial services and +advertising) starts automatically at boot, independent of user code. One +CircuitPython device runs an idle code.py so its workflow advertises; a second +CircuitPython device scans with the adafruit_ble library and verifies the +workflow's advertisement carries the File Transfer service UUID (0xFEBB) and +the CIRCUITPY device name. + +The second device disables its own workflow via settings.toml so only the +first device advertises. +""" + +import pytest + +from .conftest import get_library_files + +_ADAFRUIT_BLE = get_library_files("adafruit_ble") + +# Device 1: idle code.py. The supervisor workflow advertises during the sleep. +WORKFLOW_IDLE_CODE = """\ +import time +time.sleep(15) +""" + +# Device 2: scan for the workflow advertisement. The workflow puts the +# File Transfer service UUID (0xFEBB) in the primary advertisement and the full +# "CIRCUITPYxxxx" name in the scan response, so those arrive as separate +# Advertisement objects (service list vs. scan response). +SCANNER_CODE = """\ +import time +from adafruit_ble import BLERadio +from adafruit_ble.advertising import Advertisement +from adafruit_ble.advertising.standard import ProvideServicesAdvertisement +from adafruit_ble.uuid import StandardUUID + +ble = BLERadio() +print("scan start") +found_ft = False +found_name = False +for adv in ble.start_scan(ProvideServicesAdvertisement, Advertisement, timeout=15, active=True): + if isinstance(adv, ProvideServicesAdvertisement) and StandardUUID(0xFEBB) in adv.services: + found_ft = True + print("ft_uuid febb") + name = adv.complete_name or adv.short_name or "" + if name.startswith("CIRCUITPY"): + found_name = True + print("cp_name", name) + if found_ft and found_name: + print("workflow found") + break +ble.stop_scan() +print("scan done", found_ft, found_name) +""" + +# Disable the workflow on the scanner so it doesn't advertise alongside device 1. +SCANNER_SETTINGS = "CIRCUITPY_BLE_WORKFLOW = false\n" + + +@pytest.mark.duration(20) +@pytest.mark.circuitpy_drive({"code.py": WORKFLOW_IDLE_CODE}) +@pytest.mark.circuitpy_drive( + { + "code.py": SCANNER_CODE, + "settings.toml": SCANNER_SETTINGS, + **_ADAFRUIT_BLE, + } +) +def test_bsim_workflow_advertises(bsim_phy, circuitpython1, circuitpython2): + """The supervisor BLE workflow advertises 0xFEBB and the CIRCUITPY name.""" + workflow = circuitpython1 + scanner = circuitpython2 + + scanner.wait_until_done() + + scanner_output = scanner.serial.all_output + assert "ft_uuid febb" in scanner_output, ( + f"File Transfer service UUID 0xFEBB not observed: {scanner_output}" + ) + assert "cp_name CIRCUITPY" in scanner_output, ( + f"CIRCUITPY device name not observed: {scanner_output}" + ) + assert "workflow found" in scanner_output, ( + f"workflow advertisement not fully observed: {scanner_output}" + ) + + # The workflow device should not have entered safe mode. + workflow_output = workflow.serial.all_output + assert "safe mode" not in workflow_output.lower(), ( + f"workflow device entered safe mode: {workflow_output}" + ) diff --git a/ports/zephyr-cp/tests/bsim/test_bsim_ble_workflow_file_transfer.py b/ports/zephyr-cp/tests/bsim/test_bsim_ble_workflow_file_transfer.py new file mode 100644 index 00000000000..1343d119567 --- /dev/null +++ b/ports/zephyr-cp/tests/bsim/test_bsim_ble_workflow_file_transfer.py @@ -0,0 +1,119 @@ +# SPDX-FileCopyrightText: 2026 Scott Shawcroft for Adafruit Industries +# SPDX-License-Identifier: MIT + +"""File Transfer service test for the supervisor BLE workflow (bsim). + +A second CircuitPython device connects to the supervisor's BLE workflow +(advertised with the File Transfer service UUID 0xFEBB), pairs, and uses the +Adafruit_CircuitPython_BLE_File_Transfer library to list the workflow device's +root directory. + +The File Transfer transfer characteristic is encrypted (ENC_NO_MITM), so this +test requires pairing. nrf54lm20bsim LE encryption is not yet functional in +bsim, so this test is restricted to native_nrf5340bsim (matching +test_bsim_ble_pairing.py). Real hardware works on both. +""" + +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") + +# nrf54lm20bsim LE encryption is not yet functional in bsim. The bsim `board` +# fixture parametrizes over both boards (the circuitpython_board marker is +# documentation only here), so skip the non-functional board explicitly. +pytestmark = pytest.mark.circuitpython_board("native_nrf5340bsim") + + +def _skip_unless_nrf5340bsim(board): + if board != "native_nrf5340bsim": + pytest.skip(f"BLE encryption not functional on {board} in bsim") + + +# Device 1: idle code.py; its workflow exposes the File Transfer service. +WORKFLOW_IDLE_CODE = """\ +import time +time.sleep(25) +""" + +# Device 2: connect, pair, and listdir("/") over the File Transfer service. +CLIENT_CODE = """\ +import time +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 + +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) + +service = connection[FileTransferService] +print("ft version", service.version) +client = FileTransferClient(service) + +entries = client.listdir("/") +names = [e[0] for e in entries] +print("ft names", names) +if "code.py" in names: + print("ft code.py listed") +print("ft done") +connection.disconnect() +""" + +CLIENT_SETTINGS = "CIRCUITPY_BLE_WORKFLOW = false\n" + + +@pytest.mark.duration(30) +@pytest.mark.circuitpy_drive({"code.py": WORKFLOW_IDLE_CODE}) +@pytest.mark.circuitpy_drive( + { + "code.py": CLIENT_CODE, + "settings.toml": CLIENT_SETTINGS, + **_ADAFRUIT_BLE_FILE_TRANSFER, + **_ADAFRUIT_BLE, + } +) +def test_bsim_workflow_file_transfer(board, bsim_phy, circuitpython1, circuitpython2): + """The supervisor BLE workflow's File Transfer service can list files.""" + _skip_unless_nrf5340bsim(board) + workflow = circuitpython1 + client = circuitpython2 + + client.wait_until_done() + + client_output = client.serial.all_output + assert "found workflow" in client_output, f"client never found the workflow: {client_output}" + assert "paired True" in client_output, f"pairing did not succeed: {client_output}" + assert "ft version 4" in client_output, ( + f"File Transfer version characteristic is not 4: {client_output}" + ) + assert "ft code.py listed" in client_output, ( + f"listdir did not include code.py: {client_output}" + ) + assert "ft done" in client_output, f"file transfer did not complete: {client_output}" + + workflow_output = workflow.serial.all_output + assert "safe mode" not in workflow_output.lower(), ( + f"workflow device entered safe mode: {workflow_output}" + ) diff --git a/ports/zephyr-cp/tests/bsim/test_bsim_ble_workflow_nus.py b/ports/zephyr-cp/tests/bsim/test_bsim_ble_workflow_nus.py new file mode 100644 index 00000000000..d418ad2f15d --- /dev/null +++ b/ports/zephyr-cp/tests/bsim/test_bsim_ble_workflow_nus.py @@ -0,0 +1,153 @@ +# SPDX-FileCopyrightText: 2026 Scott Shawcroft for Adafruit Industries +# SPDX-License-Identifier: MIT + +"""Serial (Nordic-UART-style) service test for the supervisor BLE workflow +(bsim). + +The supervisor's BLE workflow exposes a CircuitPython serial service whose RX +characteristic accepts console stdin (write, encrypted) and whose TX +characteristic emits console stdout via notifications (encrypted). One +CircuitPython device runs code.py that reads a line with `input()` and echoes +it back with `print()`; a second CircuitPython device connects, pairs, +subscribes to TX, writes a line to RX, and verifies the echo comes back over TX. + +The serial service requires encryption (Just Works pairing). nrf54lm20bsim LE +encryption is not yet functional in bsim, so this test is restricted to +native_nrf5340bsim (matching test_bsim_ble_pairing.py). Real hardware works on +both. +""" + +import pytest + +from .conftest import get_library_files + +_ADAFRUIT_BLE = get_library_files("adafruit_ble") + +# nrf54lm20bsim LE encryption is not yet functional in bsim. The bsim `board` +# fixture parametrizes over both boards (the circuitpython_board marker is +# documentation only here), so skip the non-functional board explicitly. +pytestmark = pytest.mark.circuitpython_board("native_nrf5340bsim") + + +def _skip_unless_nrf5340bsim(board): + if board != "native_nrf5340bsim": + pytest.skip(f"BLE encryption not functional on {board} in bsim") + + +# Device 1: read a line from console stdin (BLE RX) and echo it to console +# stdout (mirrored to BLE TX). `input()` blocks until the central writes, so the +# device stays alive for the exchange. +WORKFLOW_CODE = """\ +import sys +print("nus ready") +sys.stdout.flush() +try: + line = input() +except EOFError: + line = "" +print("nus_echo:" + line) +""" + +# Device 2: connect, pair, discover the CircuitPython serial service, subscribe +# to TX, write a line to RX, and read the echo back over TX. +CLIENT_CODE = """\ +import time +import _bleio +from adafruit_ble import BLERadio +from adafruit_ble.advertising.standard import ProvideServicesAdvertisement +from adafruit_ble.uuid import StandardUUID + +ble = BLERadio() +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) + +# CircuitPython serial service UUID (128-bit, "nhtyPtiucriC" base + 0x0001). +serial_uuid = _bleio.UUID(b"nhtyPtiucriC\\x01\\x00\\xaf\\xad") +services = connection._bleio_connection.discover_remote_services([serial_uuid]) +svc = services[0] +print("serial service", svc.uuid.uuid16) + +rx_char = None +tx_char = None +for ch in svc.characteristics: + if ch.uuid.uuid16 == 0x0002: + rx_char = ch + elif ch.uuid.uuid16 == 0x0003: + tx_char = ch +print("rx", rx_char is not None, "tx", tx_char is not None) + +# Subscribe to TX notifications (client-side PacketBuffer writes the CCCD). +tx_pb = _bleio.PacketBuffer(tx_char, buffer_size=4, max_packet_size=128) + +# Write a line to RX (console stdin). RX has WRITE_NO_RESPONSE, so `.value =` +# does a GATT write without response. +rx_char.value = b"ZZZ\\r" +print("rx written") + +buf = bytearray(128) +got = b"" +deadline = time.monotonic() + 10 +while time.monotonic() < deadline: + n = tx_pb.readinto(buf) + if n: + got += bytes(buf[:n]) + if b"nus_echo:ZZZ" in got: + print("nus_received nus_echo:ZZZ") + break +print("nus done", b"nus_echo:ZZZ" in got) +connection.disconnect() +""" + +CLIENT_SETTINGS = "CIRCUITPY_BLE_WORKFLOW = false\n" + + +@pytest.mark.duration(30) +@pytest.mark.circuitpy_drive({"code.py": WORKFLOW_CODE}) +@pytest.mark.circuitpy_drive( + { + "code.py": CLIENT_CODE, + "settings.toml": CLIENT_SETTINGS, + **_ADAFRUIT_BLE, + } +) +def test_bsim_workflow_nus(board, bsim_phy, circuitpython1, circuitpython2): + """The supervisor BLE workflow's serial service echoes console I/O over BLE.""" + _skip_unless_nrf5340bsim(board) + workflow = circuitpython1 + client = circuitpython2 + + client.wait_until_done() + + client_output = client.serial.all_output + assert "found workflow" in client_output, f"client never found the workflow: {client_output}" + assert "paired True" in client_output, f"pairing did not succeed: {client_output}" + assert "serial service 1" in client_output, ( + f"CircuitPython serial service not discovered: {client_output}" + ) + assert "rx True tx True" in client_output, ( + f"RX/TX characteristics not discovered: {client_output}" + ) + assert "rx written" in client_output, f"RX write failed: {client_output}" + assert "nus_received nus_echo:ZZZ" in client_output, ( + f"echo 'nus_echo:ZZZ' never received over TX: {client_output}" + ) + assert "nus done True" in client_output, f"echo not confirmed: {client_output}" + + workflow_output = workflow.serial.all_output + assert "safe mode" not in workflow_output.lower(), ( + f"workflow device entered safe mode: {workflow_output}" + )