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/native/native_sim/autogen_board_info.toml b/ports/zephyr-cp/boards/native/native_sim/autogen_board_info.toml index 1f0486920fb..69a8dad30d8 100644 --- a/ports/zephyr-cp/boards/native/native_sim/autogen_board_info.toml +++ b/ports/zephyr-cp/boards/native/native_sim/autogen_board_info.toml @@ -1,5 +1,5 @@ # This file is autogenerated when a board is built. Do not edit. Do commit it to git. Other scripts use its info. -name = "POSIX/Native Boards Native simulator - native_sim" +name = "zephyr Native simulator - native_sim" [modules] __future__ = true diff --git a/ports/zephyr-cp/boards/native/nrf5340bsim/autogen_board_info.toml b/ports/zephyr-cp/boards/native/nrf5340bsim/autogen_board_info.toml index dc9608542f6..47edeed3b59 100644 --- a/ports/zephyr-cp/boards/native/nrf5340bsim/autogen_board_info.toml +++ b/ports/zephyr-cp/boards/native/nrf5340bsim/autogen_board_info.toml @@ -1,5 +1,5 @@ # This file is autogenerated when a board is built. Do not edit. Do commit it to git. Other scripts use its info. -name = "POSIX/Native Boards nRF5340 simulated boards (BabbleSim)" +name = "zephyr nRF5340 simulated boards (BabbleSim)" [modules] __future__ = true diff --git a/ports/zephyr-cp/boards/native/nrf54lm20bsim/autogen_board_info.toml b/ports/zephyr-cp/boards/native/nrf54lm20bsim/autogen_board_info.toml index 6a470b5906b..d1feffc835d 100644 --- a/ports/zephyr-cp/boards/native/nrf54lm20bsim/autogen_board_info.toml +++ b/ports/zephyr-cp/boards/native/nrf54lm20bsim/autogen_board_info.toml @@ -1,5 +1,5 @@ # This file is autogenerated when a board is built. Do not edit. Do commit it to git. Other scripts use its info. -name = "POSIX/Native Boards nRF54LM20 simulated boards (BabbleSim)" +name = "zephyr nRF54LM20 simulated boards (BabbleSim)" [modules] __future__ = true 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 7e51cb389fb..c3684a3b148 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,19 +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 @@ -143,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, @@ -241,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); @@ -341,13 +402,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; @@ -356,10 +419,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) { @@ -411,25 +482,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]; @@ -438,30 +515,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; @@ -485,15 +563,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_ValueError(MP_ERROR_TEXT("Data too large for advertisement packet")); + } + + 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) { @@ -503,6 +641,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) { @@ -544,7 +683,11 @@ mp_obj_t common_hal_bleio_adapter_start_scan(bleio_adapter_obj_t *self, uint8_t struct bt_le_scan_param scan_params = { .type = active ? BT_LE_SCAN_TYPE_ACTIVE : BT_LE_SCAN_TYPE_PASSIVE, - .options = BT_LE_SCAN_OPT_FILTER_DUPLICATE, + /* Do not filter duplicates: the application merges advertisement and + * scan-response packets and needs to observe updated advertisements + * from the same device (e.g. when user code replaces the workflow + * advert). */ + .options = 0, .interval = interval_units, .window = window_units, .timeout = (uint16_t)timeout_units, @@ -765,6 +908,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 c9cc7a03897..25e1c35d563 100644 --- a/ports/zephyr-cp/common-hal/_bleio/Adapter.h +++ b/ports/zephyr-cp/common-hal/_bleio/Adapter.h @@ -32,3 +32,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..e1ba8b7349f 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_const_empty_tuple; + } 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..8cdb5fcae76 100644 --- a/ports/zephyr-cp/common-hal/_bleio/PacketBuffer.c +++ b/ports/zephyr-cp/common-hal/_bleio/PacketBuffer.c @@ -19,19 +19,26 @@ #include "shared-bindings/_bleio/__init__.h" #include "shared-bindings/_bleio/Connection.h" #include "shared-bindings/_bleio/PacketBuffer.h" +#include "shared-bindings/microcontroller/__init__.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,14 +99,6 @@ 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. -static void notify_complete_cb(struct bt_conn *conn, void *user_data) { - bleio_packet_buffer_obj_t *self = (bleio_packet_buffer_obj_t *)user_data; - self->packet_queued = false; - send_pending(self); -} - // Returns true if the tracked connection is still connected. // Clears self->conn if the connection is stale. static bool conn_is_valid(bleio_packet_buffer_obj_t *self) { @@ -112,88 +114,139 @@ 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; +// 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 packet_queued is single-threaded here 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; + // Drain the other buffer now that this notify is done. K_NO_WAIT runs + // send_work immediately, cancelling any pending delayed retry. + k_work_reschedule(&self->send_work, K_NO_WAIT); +} + +// 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 +// caller of bt_gatt_notify_cb, so packet_queued / pending_index / pending_size +// are touched here and in the VM's brief critical section — nowhere else. +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->characteristic == NULL || - self->characteristic->service == NULL || - self->characteristic->service->is_remote) { - self->pending_size = 0; - return true; + 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). bleio_characteristic_obj_t *c = self->characteristic; - if (!(c->props & CHAR_PROP_NOTIFY) || !c->service->registered) { + if (c == NULL || self->client) { + return; + } + if (!conn_is_valid(self)) { + // Stale connection: drop everything staged. self->pending_size = 0; - return true; + self->packet_queued = false; + return; } + // Staging keeps pending_size <= the negotiated ATT MTU payload, so the + // whole staged packet fits in one notification. struct bt_gatt_notify_params params = { .attr = &c->service->attrs[c->value_attr_index], - .data = self->outgoing_buffer, + .data = self->outgoing[self->pending_index], .len = self->pending_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; + // Accepted by the controller; the payload was copied into a PDU and + // will go out at the next connection event. Hand this buffer off and + // let the VM fill the other one while it's in flight. self->packet_queued = true; - return true; + self->pending_size = 0; + self->pending_index ^= 1; // VM fills the other buffer next + return; } if (err == -ENOTCONN) { - // Peer disconnected — clear tracking, discard pending. + // 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.) self->conn = NULL; self->pending_size = 0; - return true; + self->packet_queued = false; + k_work_cancel_delayable(&self->send_work); + return; } - // -ENOMEM (no TX buffer) — leave pending, caller will retry. - return false; + // -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)); } -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, the two +// outgoing buffers, the 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 *outgoing0, uint8_t *outgoing1, + bool owns_outgoing0, bool owns_outgoing1, + 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->outgoing[0] = outgoing0; + self->outgoing[1] = outgoing1; + self->owns_outgoing[0] = owns_outgoing0; + self->owns_outgoing[1] = owns_outgoing1; + self->pending_index = 0; 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->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); } - // Set ourselves as the characteristic's observer + // The deferred sender drives server-side notifications. Client-side writes + // go out synchronously and never schedule it, but initializing it always is + // harmless and keeps flush()/deinit() simple. + k_work_init_delayable(&self->send_work, packet_buffer_send_work_handler); + + // 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 +254,91 @@ 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 ping-pong between two + // buffers so the VM can fill one while the other is in flight. + uint8_t *outgoing0 = NULL; + uint8_t *outgoing1 = NULL; + bool owns0 = false; + bool owns1 = false; + if (client) { + outgoing0 = port_malloc(max_packet_size, false); + owns0 = true; + } else if (props & (CHAR_PROP_NOTIFY | CHAR_PROP_INDICATE)) { + outgoing0 = port_malloc(max_packet_size, false); + outgoing1 = port_malloc(max_packet_size, false); + owns0 = true; + owns1 = true; + } + + packet_buffer_init_common(self, characteristic, + ringbuf_data, ringbuf_size, true, + outgoing0, outgoing1, owns0, owns1, + 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 -> outgoing[0], outgoing_buffer2 -> outgoing[1]. 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 *outgoing0 = NULL; + uint8_t *outgoing1 = NULL; + bool owns0 = false; + bool owns1 = false; + + if (client) { + // Client-side: one scratch buffer for synchronous writes. + if (outgoing_buffer1 != NULL) { + outgoing0 = (uint8_t *)outgoing_buffer1; + } else { + outgoing0 = port_malloc(max_packet_size, false); + owns0 = true; + } + } else if (props & (CHAR_PROP_NOTIFY | CHAR_PROP_INDICATE)) { + // Server-side: two buffers to ping-pong between. + if (outgoing_buffer1 != NULL) { + outgoing0 = (uint8_t *)outgoing_buffer1; + } else { + outgoing0 = port_malloc(max_packet_size, false); + owns0 = true; + } + if (outgoing_buffer2 != NULL) { + outgoing1 = (uint8_t *)outgoing_buffer2; + } else { + outgoing1 = port_malloc(max_packet_size, false); + owns1 = true; + } + } + + packet_buffer_init_common(self, characteristic, + ringbuf_data, ringbuf_size, false, + outgoing0, outgoing1, owns0, owns1, + max_packet_size); } mp_int_t common_hal_bleio_packet_buffer_readinto(bleio_packet_buffer_obj_t *self, @@ -261,7 +384,7 @@ mp_int_t common_hal_bleio_packet_buffer_readinto(bleio_packet_buffer_obj_t *self mp_int_t common_hal_bleio_packet_buffer_write(bleio_packet_buffer_obj_t *self, const uint8_t *data, size_t len, uint8_t *header, size_t header_len) { - if (self->outgoing_buffer == NULL) { + if (self->outgoing[0] == NULL) { mp_raise_bleio_BluetoothError(MP_ERROR_TEXT("Writes not supported on Characteristic")); } @@ -283,33 +406,98 @@ 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) { + // Client-side (remote characteristic): write the request directly to the + // remote GATT server. The server-side path below stages into a double + // buffer 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[0], header, header_len); + memcpy(self->outgoing[0] + 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[0], 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[0], total); + } else { + // No write property; nothing to send. + return -1; + } + return (mp_int_t)total; + } + + // Server-side notify path: stage header + data into the pending buffer, + // then hand it to send_work on the system workqueue. bt_gatt_notify_cb + // blocks (K_FOREVER) off the workqueue, so we never call it here; send_work + // does, and the completion callback paces us to one notification in flight. + // The header prefixes each packet, so it's only written when the packet is + // empty (pending_size == 0). pending_size counts bytes staged but not yet + // accepted by the controller. + + if (mp_hal_is_interrupted()) { + return -1; + } + + // If this write would overflow the current packet, wait for send_work to + // drain it (which requires the in-flight notification to complete first). + if ((size_t)len + self->pending_size > (size_t)outgoing_packet_length) { while (self->pending_size != 0 && + conn_is_valid(self) && !mp_hal_is_interrupted()) { + k_work_reschedule(&self->send_work, K_NO_WAIT); RUN_BACKGROUND_TASKS; } } - if (mp_hal_is_interrupted()) { + if (!conn_is_valid(self) || 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; + // send_work (system workqueue) may preempt us and modify pending_index / + // pending_size / packet_queued, so guard the append. + common_hal_mcu_disable_interrupts(); + { + uint8_t *pending = self->outgoing[self->pending_index]; + if (self->pending_size == 0 && header_len > 0) { + memcpy(pending, header, header_len); + self->pending_size += header_len; + num_bytes_written += header_len; + } + memcpy(pending + self->pending_size, data, len); + self->pending_size += len; + num_bytes_written += len; } - memcpy(self->outgoing_buffer + self->pending_size, data, len); - self->pending_size += len; - num_bytes_written += len; + common_hal_mcu_enable_interrupts(); - // Send immediately if no write is queued. + // If no notification is in flight, kick the deferred sender to drain what + // we just staged (K_NO_WAIT runs it immediately, cancelling any pending + // delayed retry). If one is in flight, its completion will resubmit. if (!self->packet_queued) { - send_pending(self); + k_work_reschedule(&self->send_work, K_NO_WAIT); } - return num_bytes_written; + return (mp_int_t)num_bytes_written; } mp_int_t common_hal_bleio_packet_buffer_get_incoming_packet_length( @@ -345,24 +533,39 @@ 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 AND its + // completion has fired (we pace to one notification in flight at a time). + // send_work runs on the system workqueue, which is higher priority than + // this thread, so k_work_reschedule(K_NO_WAIT) lets it preempt us; + // RUN_BACKGROUND_TASKS advances simulated time / lets completion callbacks + // fire so packet_queued clears. For client-side writes (synchronous) there + // is nothing pending, so this returns immediately. + while ((self->pending_size != 0 || self->packet_queued) && + conn_is_valid(self) && !mp_hal_is_interrupted()) { + k_work_reschedule(&self->send_work, K_NO_WAIT); RUN_BACKGROUND_TASKS; - if (!send_pending(self)) { - // Couldn't send — wait and retry. - RUN_BACKGROUND_TASKS; - } } } @@ -376,12 +579,21 @@ 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); + // Cancel any pending delayed retry so it can't fire after we free the + // buffers (or after the object is gone). + k_work_cancel_delayable(&self->send_work); + // 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); - self->outgoing_buffer = NULL; + for (int i = 0; i < 2; i++) { + if (self->owns_outgoing[i] && self->outgoing[i] != NULL) { + port_free(self->outgoing[i]); + } + self->outgoing[i] = 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..67216775257 100644 --- a/ports/zephyr-cp/common-hal/_bleio/PacketBuffer.h +++ b/ports/zephyr-cp/common-hal/_bleio/PacketBuffer.h @@ -9,6 +9,7 @@ #include +#include #include #include "py/obj.h" @@ -22,16 +23,50 @@ typedef struct { mp_obj_base_t base; bleio_characteristic_obj_t *characteristic; uint32_t timeout_ms; + // Store the connection we are talking to. We expect to talk over a single + // connection at a time. + struct bt_conn *conn; + bool client; + // The current MTU negotiated with the active connection. + size_t max_packet_size; + + // ringbuf to store incoming packets. struct ring_buf ringbuf; uint8_t *ringbuf_data; size_t ringbuf_size; - size_t max_packet_size; - // Outgoing pending buffer - uint8_t *outgoing_buffer; - uint16_t pending_size; - bool packet_queued; - struct bt_conn *conn; - bool client; + + // Outgoing path. Two fixed buffers alternate so the VM can keep filling one + // while the other is in flight with the controller. + // + // outgoing[pending_index] is the buffer the VM is filling (pending_size + // bytes staged, not yet sent). packet_queued means a notification has been + // accepted by the controller and its completion callback hasn't fired yet + // — at most one is in flight at a time. + // + uint8_t *outgoing[2]; + uint8_t pending_index; // which buffer the VM is filling + volatile uint16_t pending_size; // bytes staged in outgoing[pending_index] + volatile bool packet_queued; // a notification is in flight + + // send_work is a delayable work item, the only caller of bt_gatt_notify_cb + // (that API blocks (K_FOREVER) off the workqueue but returns -ENOMEM from + // it). It's rescheduled with K_NO_WAIT to drain after a write/completion, + // and with a short delay on -ENOMEM so it self-retries even when the VM is + // idle and the shared ATT TX pool is full from other traffic. It and the + // completion callback both run on the workqueue (serialized with each + // other); the VM is the only other toucher of these fields and guards its + // read-modify-write with common_hal_mcu_disable_interrupts() (the workqueue + // preempts the VM, so the critical section keeps it out while we append). + // bt_gatt_notify_cb copies the payload into a PDU on success, so the source + // buffer may be reused once the call returns 0; the completion callback + // paces us to one in flight and makes flush() accurate. + struct k_work_delayable send_work; + + // 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[2]; } 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 edc9422377b..78cab66fa65 100644 --- a/ports/zephyr-cp/cptools/build_circuitpython.py +++ b/ports/zephyr-cp/cptools/build_circuitpython.py @@ -481,8 +481,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/cptools/zephyr2cp.py b/ports/zephyr-cp/cptools/zephyr2cp.py index a6e2fa53cdc..9525c312111 100644 --- a/ports/zephyr-cp/cptools/zephyr2cp.py +++ b/ports/zephyr-cp/cptools/zephyr2cp.py @@ -585,6 +585,7 @@ def zephyr_dts_to_cp_board(board_id, portdir, builddir, zephyrbuilddir, mpconfig board_names = {} status_led = None status_led_inverted = False + boot_button = None path2chosen = {} chosen2path = {} @@ -730,7 +731,8 @@ def zephyr_dts_to_cp_board(board_id, portdir, builddir, zephyrbuilddir, mpconfig if "gpio-keys" in compatible: for key in node.nodes: - props = node.nodes[key].props + key_node = node.nodes[key] + props = key_node.props ioport = props["gpios"]._markers[1][2] num = int.from_bytes(props["gpios"].value[4:8], "big") @@ -745,10 +747,18 @@ def zephyr_dts_to_cp_board(board_id, portdir, builddir, zephyrbuilddir, mpconfig key_code = props["zephyr,code"].to_num() if key_code in INPUT_KEY_NAMES: board_names[(ioport, num)].append(INPUT_KEY_NAMES[key_code]) - if key in node2alias: - if "sw0" in node2alias[key]: + if key_node in node2alias: + aliases = node2alias[key_node] + if "sw0" in aliases: board_names[(ioport, num)].append("BUTTON") - board_names[(ioport, num)].extend(node2alias[key]) + # The sw0 alias designates the conventional first user + # button, so prefer it as the boot button. + boot_button = (ioport, num) + board_names[(ioport, num)].extend(aliases) + # Default to the first button in device tree order when no sw0 + # alias has designated one yet. + if boot_button is None: + boot_button = (ioport, num) if len(all_ioports) > 1: a, b = all_ioports[:2] @@ -775,6 +785,8 @@ def zephyr_dts_to_cp_board(board_id, portdir, builddir, zephyrbuilddir, mpconfig pin_object_name = f"P{ioport[len(shared_prefix) :].upper()}_{num:02d}" if status_led and (ioport, num) == status_led: status_led = pin_object_name + if boot_button and (ioport, num) == boot_button: + boot_button = pin_object_name pin_defs.append( f"const mcu_pin_obj_t pin_{pin_object_name} = {{ .base.type = &mcu_pin_type, .port = DEVICE_DT_GET(DT_NODELABEL({ioport})), .number = {num}}};" ) @@ -937,6 +949,10 @@ def zephyr_dts_to_cp_board(board_id, portdir, builddir, zephyrbuilddir, mpconfig else: status_led = "" status_led_inverted = "" + if boot_button: + boot_button = f"#define CIRCUITPY_BOOT_BUTTON (&pin_{boot_button})\n" + else: + boot_button = "" ram_list = [] ram_externs = [] max_size = 0 @@ -966,6 +982,7 @@ def zephyr_dts_to_cp_board(board_id, portdir, builddir, zephyrbuilddir, mpconfig #define CIRCUITPY_RAM_DEVICE_COUNT {len(rams)} {status_led} {status_led_inverted} +{boot_button} """ if not header.exists() or header.read_text() != new_header_content: header.write_text(new_header_content) 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/samples/observer/CMakeLists.txt b/ports/zephyr-cp/tests/bsim/samples/observer/CMakeLists.txt new file mode 100644 index 00000000000..59d999d7052 --- /dev/null +++ b/ports/zephyr-cp/tests/bsim/samples/observer/CMakeLists.txt @@ -0,0 +1,10 @@ +# SPDX-License-Identifier: Apache-2.0 + +cmake_minimum_required(VERSION 3.20.0) +find_package(Zephyr REQUIRED HINTS $ENV{ZEPHYR_BASE}) +project(observer) + +target_sources(app PRIVATE + src/main.c + src/observer.c +) diff --git a/ports/zephyr-cp/tests/bsim/samples/observer/prj.conf b/ports/zephyr-cp/tests/bsim/samples/observer/prj.conf new file mode 100644 index 00000000000..a328b3700a9 --- /dev/null +++ b/ports/zephyr-cp/tests/bsim/samples/observer/prj.conf @@ -0,0 +1,2 @@ +CONFIG_BT=y +CONFIG_BT_OBSERVER=y diff --git a/ports/zephyr-cp/tests/bsim/samples/observer/src/main.c b/ports/zephyr-cp/tests/bsim/samples/observer/src/main.c new file mode 100644 index 00000000000..db8ce6bab95 --- /dev/null +++ b/ports/zephyr-cp/tests/bsim/samples/observer/src/main.c @@ -0,0 +1,28 @@ +/* + * Copyright (c) 2022 Nordic Semiconductor ASA + * + * SPDX-License-Identifier: Apache-2.0 + */ + +#include +#include + +int observer_start(void); + +int main(void) { + int err; + + printk("Starting Observer Demo\n"); + + /* Initialize the Bluetooth Subsystem */ + err = bt_enable(NULL); + if (err) { + printk("Bluetooth init failed (err %d)\n", err); + return 0; + } + + (void)observer_start(); + + printk("Exiting %s thread.\n", __func__); + return 0; +} diff --git a/ports/zephyr-cp/tests/bsim/samples/observer/src/observer.c b/ports/zephyr-cp/tests/bsim/samples/observer/src/observer.c new file mode 100644 index 00000000000..fc896bf593d --- /dev/null +++ b/ports/zephyr-cp/tests/bsim/samples/observer/src/observer.c @@ -0,0 +1,139 @@ +/* + * Copyright (c) 2022 Nordic Semiconductor ASA + * Copyright (c) 2015-2016 Intel Corporation + * + * SPDX-License-Identifier: Apache-2.0 + */ + +#include +#include +#include + +static void device_found(const bt_addr_le_t *addr, int8_t rssi, uint8_t type, + struct net_buf_simple *ad) { + printk("Device found: %s (RSSI %d), type %u, AD data len %u\n", + bt_addr_le_str(addr), rssi, type, ad->len); +} + +#if defined(CONFIG_BT_EXT_ADV) +#define NAME_LEN 30 + +static bool data_cb(struct bt_data *data, void *user_data) { + char *name = user_data; + uint8_t len; + + switch (data->type) { + case BT_DATA_NAME_SHORTENED: + case BT_DATA_NAME_COMPLETE: + len = MIN(data->data_len, NAME_LEN - 1); + (void)memcpy(name, data->data, len); + name[len] = '\0'; + return false; + default: + return true; + } +} + +static const char *phy2str(uint8_t phy) { + switch (phy) { + case BT_GAP_LE_PHY_NONE: + return "No packets"; + case BT_GAP_LE_PHY_1M: + return "LE 1M"; + case BT_GAP_LE_PHY_2M: + return "LE 2M"; + case BT_GAP_LE_PHY_CODED: + return "LE Coded"; + default: + return "Unknown"; + } +} + +static void scan_recv(const struct bt_le_scan_recv_info *info, + struct net_buf_simple *buf) { + char name[NAME_LEN]; + uint8_t data_status; + uint16_t data_len; + + (void)memset(name, 0, sizeof(name)); + + data_len = buf->len; + bt_data_parse(buf, data_cb, name); + + data_status = BT_HCI_LE_ADV_EVT_TYPE_DATA_STATUS(info->adv_props); + + printk("[DEVICE]: %s, AD evt type %u, Tx Pwr: %i, RSSI %i " + "Data status: %u, AD data len: %u Name: %s " + "C:%u S:%u D:%u SR:%u E:%u Pri PHY: %s, Sec PHY: %s, " + "Interval: 0x%04x (%u ms), SID: %u\n", + bt_addr_le_str(info->addr), info->adv_type, info->tx_power, info->rssi, + data_status, data_len, name, + (info->adv_props & BT_GAP_ADV_PROP_CONNECTABLE) != 0, + (info->adv_props & BT_GAP_ADV_PROP_SCANNABLE) != 0, + (info->adv_props & BT_GAP_ADV_PROP_DIRECTED) != 0, + (info->adv_props & BT_GAP_ADV_PROP_SCAN_RESPONSE) != 0, + (info->adv_props & BT_GAP_ADV_PROP_EXT_ADV) != 0, + phy2str(info->primary_phy), phy2str(info->secondary_phy), + info->interval, info->interval * 5 / 4, info->sid); +} + +static struct bt_le_scan_cb scan_callbacks = { + .recv = scan_recv, +}; +#endif /* CONFIG_BT_EXT_ADV */ + +static int scan_start(void) { + /* 30 ms continuous active scanning. Duplicate filtering is disabled so + * that updated advertisements from the same device (e.g. when user + * code replaces the workflow advert) are reported again. + */ + struct bt_le_scan_param scan_param = { + .type = BT_LE_SCAN_TYPE_ACTIVE, + .options = 0, + .interval = BT_GAP_SCAN_FAST_INTERVAL_MIN, + .window = BT_GAP_SCAN_FAST_WINDOW, + }; + int err; + + #if defined(CONFIG_BT_EXT_ADV) + scan_param.options |= BT_LE_SCAN_OPT_CODED; + #endif /* CONFIG_BT_EXT_ADV */ + +scan_start_retry: + printk("Starting scanning...\n"); + err = bt_le_scan_start(&scan_param, device_found); + if (err) { + if ((scan_param.options & BT_LE_SCAN_OPT_CODED) != 0U) { + printk("Failed to start scanning with Coded PHY (err %d), retrying " + "without...\n", err); + + scan_param.options &= ~BT_LE_SCAN_OPT_CODED; + + goto scan_start_retry; + } + + printk("Start scanning failed (err %d)\n", err); + + return err; + } + + printk("success.\n"); + + return 0; +} + +int observer_start(void) { + int err; + + #if defined(CONFIG_BT_EXT_ADV) + bt_le_scan_cb_register(&scan_callbacks); + printk("Registered scan callbacks\n"); + #endif /* CONFIG_BT_EXT_ADV */ + + err = scan_start(); + if (err != 0) { + return err; + } + + return 0; +} diff --git a/ports/zephyr-cp/tests/bsim/test_bsim_basics.py b/ports/zephyr-cp/tests/bsim/test_bsim_basics.py index 96f40a86b40..12165750274 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(20) 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_advertising.py b/ports/zephyr-cp/tests/bsim/test_bsim_ble_advertising.py index 40b74ee854a..27fa78a5b7f 100644 --- a/ports/zephyr-cp/tests/bsim/test_bsim_ble_advertising.py +++ b/ports/zephyr-cp/tests/bsim/test_bsim_ble_advertising.py @@ -77,7 +77,56 @@ """ -@pytest.mark.zephyr_sample("bluetooth/observer") +# Advertiser: primary advert carries a distinctive marker; the scan response +# carries a different marker. If the adapter merges them they'd arrive as one +# entry, but the adapter does not merge — they come as separate ScanEntry +# objects distinguished by `scan_response`. +BSIM_ADVERT_SCAN_RESPONSE_ADV_CODE = """\ +import _bleio +import time + +adapter = _bleio.adapter + +name = b"ADVADV" +advertisement = bytes((2, 0x01, 0x06, len(name) + 1, 0x09)) + name + +sname = b"SCANRSP" +scan_response = bytes((len(sname) + 1, 0x09)) + sname + +print("adv start") +adapter.start_advertising(advertisement, scan_response=scan_response, connectable=True) +time.sleep(10) +adapter.stop_advertising() +print("adv done") +""" + +BSIM_ADVERT_SCAN_RESPONSE_SCAN_CODE = """\ +import _bleio + +adapter = _bleio.adapter +print("scan start") +saw_primary = False +saw_scan_response = False +for entry in adapter.start_scan(active=True, timeout=12.0): + data = entry.advertisement_bytes + if b"ADVADV" in data and not entry.scan_response: + saw_primary = True + print("primary") + if b"SCANRSP" in data and entry.scan_response: + saw_scan_response = True + print("scan_response") + if saw_primary and saw_scan_response: + print("both") + break +adapter.stop_scan() +print("scan done", saw_primary, saw_scan_response) +""" + +# Disable the workflow on the advertiser so only its custom advert is on air. +BSIM_ADVERT_SCAN_RESPONSE_SETTINGS = "CIRCUITPY_BLE_WORKFLOW = false\n" + + +@pytest.mark.zephyr_sample("tests/bsim/samples/observer") @pytest.mark.circuitpy_drive({"code.py": BSIM_ADV_CODE}) def test_bsim_advertise_and_scan(bsim_phy, circuitpython, zephyr_sample): """Advertise from CircuitPython and verify Zephyr observer sees traffic.""" @@ -94,9 +143,9 @@ def test_bsim_advertise_and_scan(bsim_phy, circuitpython, zephyr_sample): assert "AD data len 10" in observer_output -@pytest.mark.zephyr_sample("bluetooth/observer") +@pytest.mark.zephyr_sample("tests/bsim/samples/observer") @pytest.mark.code_py_runs(2) -@pytest.mark.duration(25) +@pytest.mark.duration(60) @pytest.mark.circuitpy_drive({"code.py": BSIM_ADV_INTERRUPT_RELOAD_CODE}) def test_bsim_advertise_ctrl_c_reload(bsim_phy, circuitpython, zephyr_sample): """Ensure advertising resumes after Ctrl-C and a reload.""" @@ -107,7 +156,9 @@ def test_bsim_advertise_ctrl_c_reload(bsim_phy, circuitpython, zephyr_sample): observer_count_before = observer.serial.all_output.count("Device found:") circuitpython.serial.write("\x03") - circuitpython.serial.wait_for("KeyboardInterrupt") + # Real AES for the link layer adds CPU overhead that can delay the + # interpreter noticing SIGINT beyond the default wait_for timeout. + circuitpython.serial.wait_for("KeyboardInterrupt", timeout=20) circuitpython.serial.write("\x04") circuitpython.wait_until_done() @@ -125,7 +176,8 @@ def test_bsim_advertise_ctrl_c_reload(bsim_phy, circuitpython, zephyr_sample): assert "Already advertising" not in cp_output -@pytest.mark.zephyr_sample("bluetooth/observer") +@pytest.mark.zephyr_sample("tests/bsim/samples/observer") +@pytest.mark.duration(40) @pytest.mark.circuitpy_drive({"code.py": BSIM_TX_POWER_DEFAULT_CODE}) def test_bsim_tx_power_default_rssi(board, bsim_phy, circuitpython, zephyr_sample): """Verify default TX power produces expected RSSI.""" @@ -151,7 +203,8 @@ def test_bsim_tx_power_default_rssi(board, bsim_phy, circuitpython, zephyr_sampl assert all_rssi[0] == expected_rssi, f"Expected RSSI {expected_rssi}, got {all_rssi[0]}" -@pytest.mark.zephyr_sample("bluetooth/observer") +@pytest.mark.zephyr_sample("tests/bsim/samples/observer") +@pytest.mark.duration(40) @pytest.mark.circuitpy_drive({"code.py": BSIM_TX_POWER_LOW_CODE}) def test_bsim_tx_power_low_rssi(bsim_phy, circuitpython, zephyr_sample): """Verify low TX power reduces RSSI.""" @@ -172,3 +225,37 @@ def test_bsim_tx_power_low_rssi(bsim_phy, circuitpython, zephyr_sample): assert len(all_rssi) > 0, "Observer saw no advertisements" assert all_rssi[0] < -39, f"Expected lower RSSI with -20 dBm TX, got {all_rssi[0]}" + + +@pytest.mark.duration(20) +@pytest.mark.circuitpy_drive( + { + "code.py": BSIM_ADVERT_SCAN_RESPONSE_ADV_CODE, + "settings.toml": BSIM_ADVERT_SCAN_RESPONSE_SETTINGS, + } +) +@pytest.mark.circuitpy_drive({"code.py": BSIM_ADVERT_SCAN_RESPONSE_SCAN_CODE}) +def test_bsim_scan_response_not_merged(bsim_phy, circuitpython1, circuitpython2): + """The adapter does not merge advertisement and scan response. + + The advertiser sends a primary advert (marker "ADVADV") and a scan + response (marker "SCANRSP") as separate packets. With duplicate filtering + disabled and no merging in the adapter, the scanner observes them as two + distinct ScanEntry objects: one with scan_response=False (the primary + advert) and one with scan_response=True (the scan response). + """ + scanner = circuitpython2 + + scanner.wait_until_done() + + scanner_output = scanner.serial.all_output + assert "primary" in scanner_output, ( + f"primary advert entry not observed separately: {scanner_output}" + ) + assert "scan_response" in scanner_output, ( + f"scan response entry not observed separately: {scanner_output}" + ) + assert "both" in scanner_output, ( + f"did not observe both advert and scan response: {scanner_output}" + ) + assert "scan done True True" in scanner_output, f"scan did not confirm both: {scanner_output}" diff --git a/ports/zephyr-cp/tests/bsim/test_bsim_ble_descriptor.py b/ports/zephyr-cp/tests/bsim/test_bsim_ble_descriptor.py index d1b07859133..6d5cf418147 100644 --- a/ports/zephyr-cp/tests/bsim/test_bsim_ble_descriptor.py +++ b/ports/zephyr-cp/tests/bsim/test_bsim_ble_descriptor.py @@ -96,7 +96,7 @@ """ -@pytest.mark.duration(14) +@pytest.mark.duration(30) @pytest.mark.circuitpy_drive({"code.py": BSIM_DESC_SERVER_CODE}) @pytest.mark.circuitpy_drive({"code.py": BSIM_DESC_CLIENT_CODE}) def test_bsim_descriptor_user_description(bsim_phy, circuitpython1, circuitpython2): diff --git a/ports/zephyr-cp/tests/bsim/test_bsim_ble_nus.py b/ports/zephyr-cp/tests/bsim/test_bsim_ble_nus.py index bb589cf9d90..1336d37fbd0 100644 --- a/ports/zephyr-cp/tests/bsim/test_bsim_ble_nus.py +++ b/ports/zephyr-cp/tests/bsim/test_bsim_ble_nus.py @@ -52,7 +52,13 @@ @pytest.mark.zephyr_sample("tests/bsim/samples/central_nus_client") @pytest.mark.duration(14) -@pytest.mark.circuitpy_drive({"code.py": BSIM_NUS_PERIPHERAL_CODE, **_ADAFRUIT_BLE}) +@pytest.mark.circuitpy_drive( + { + "code.py": BSIM_NUS_PERIPHERAL_CODE, + "settings.toml": "CIRCUITPY_BLE_WORKFLOW = false\n", + **_ADAFRUIT_BLE, + } +) def test_bsim_nus_peripheral(bsim_phy, circuitpython, zephyr_sample): """CP hosts NUS peripheral; Zephyr central writes to RX, reads TX notifications.""" circuitpython.wait_until_done() @@ -148,7 +154,7 @@ def test_bsim_nus_peripheral(bsim_phy, circuitpython, zephyr_sample): """ -@pytest.mark.duration(14) +@pytest.mark.duration(30) @pytest.mark.circuitpy_drive({"code.py": BSIM_NUS_SERVER_CODE, **_ADAFRUIT_BLE}) @pytest.mark.circuitpy_drive({"code.py": BSIM_NUS_CLIENT_CODE, **_ADAFRUIT_BLE}) def test_bsim_nus_cp_to_cp(bsim_phy, circuitpython1, circuitpython2): 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..27bb703f379 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 @@ -107,7 +109,7 @@ """ -@pytest.mark.duration(14) +@pytest.mark.duration(25) @pytest.mark.circuitpy_drive({"code.py": BSIM_PB_SERVER_IN_CODE}) @pytest.mark.circuitpy_drive({"code.py": BSIM_PB_CLIENT_IN_CODE}) def test_bsim_packet_buffer_server_incoming(bsim_phy, circuitpython1, circuitpython2): @@ -342,7 +344,7 @@ def test_bsim_packet_buffer_bidirectional(bsim_phy, circuitpython1, circuitpytho """ -@pytest.mark.duration(14) +@pytest.mark.duration(30) @pytest.mark.circuitpy_drive({"code.py": BSIM_PB_QUEUE_SERVER_CODE}) @pytest.mark.circuitpy_drive({"code.py": BSIM_PB_QUEUE_CLIENT_CODE}) def test_bsim_packet_buffer_queue(bsim_phy, circuitpython1, circuitpython2): @@ -451,7 +453,7 @@ def test_bsim_packet_buffer_queue(bsim_phy, circuitpython1, circuitpython2): """ -@pytest.mark.duration(14) +@pytest.mark.duration(25) @pytest.mark.circuitpy_drive({"code.py": BSIM_PB_OVERFLOW_SERVER_CODE}) @pytest.mark.circuitpy_drive({"code.py": BSIM_PB_OVERFLOW_CLIENT_CODE}) def test_bsim_packet_buffer_readinto_overflow(bsim_phy, circuitpython1, circuitpython2): @@ -553,7 +555,7 @@ def test_bsim_packet_buffer_readinto_overflow(bsim_phy, circuitpython1, circuitp """ -@pytest.mark.duration(14) +@pytest.mark.duration(25) @pytest.mark.circuitpy_drive({"code.py": BSIM_PB_HEADER_SERVER_CODE}) @pytest.mark.circuitpy_drive({"code.py": BSIM_PB_HEADER_CLIENT_CODE}) def test_bsim_packet_buffer_write_header(bsim_phy, circuitpython1, circuitpython2): @@ -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 @@ -759,8 +765,12 @@ def test_bsim_packet_buffer_packet_lengths(bsim_phy, circuitpython): @pytest.mark.duration(30) -@pytest.mark.circuitpy_drive({"code.py": BSIM_PB_RECONNECT_SERVER_CODE}) -@pytest.mark.circuitpy_drive({"code.py": BSIM_PB_RECONNECT_CLIENT_CODE}) +@pytest.mark.circuitpy_drive( + {"code.py": BSIM_PB_RECONNECT_SERVER_CODE, "settings.toml": "CIRCUITPY_BLE_WORKFLOW = false\n"} +) +@pytest.mark.circuitpy_drive( + {"code.py": BSIM_PB_RECONNECT_CLIENT_CODE, "settings.toml": "CIRCUITPY_BLE_WORKFLOW = false\n"} +) def test_bsim_packet_buffer_reconnect(bsim_phy, circuitpython1, circuitpython2): """PacketBuffer tracks conn through disconnect / reconnect.""" server = circuitpython1 @@ -783,3 +793,349 @@ 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(30) +@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(35) +@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}" + ) + # Be vague here because PING and BODY may be in separate packets or merged. + assert "b'PING" in client_output, f"client never received the PING echo: {client_output}" + assert "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_pairing.py b/ports/zephyr-cp/tests/bsim/test_bsim_ble_pairing.py index 75f6875ee2e..850b2fc8957 100644 --- a/ports/zephyr-cp/tests/bsim/test_bsim_ble_pairing.py +++ b/ports/zephyr-cp/tests/bsim/test_bsim_ble_pairing.py @@ -1,14 +1,10 @@ # SPDX-FileCopyrightText: 2025 Scott Shawcroft for Adafruit Industries # SPDX-License-Identifier: MIT -"""BLE pairing tests for nrf5340bsim.""" +"""BLE pairing tests for the bsim boards.""" import pytest -# nrf54lm20bsim LE encryption is not yet functional in bsim. Enable it when it -# does. Real hardware works. -pytestmark = pytest.mark.circuitpython_board("native_nrf5340bsim") - BSIM_PERIPHERAL_CODE = """\ import _bleio import time @@ -93,9 +89,13 @@ @pytest.mark.duration(22) -@pytest.mark.circuitpy_drive({"code.py": BSIM_PERIPHERAL_CODE}) -@pytest.mark.circuitpy_drive({"code.py": BSIM_CENTRAL_CODE}) -def test_bsim_pairing_cp_to_cp(bsim_phy, circuitpython1, circuitpython2): +@pytest.mark.circuitpy_drive( + {"code.py": BSIM_PERIPHERAL_CODE, "settings.toml": "CIRCUITPY_BLE_WORKFLOW = false\n"} +) +@pytest.mark.circuitpy_drive( + {"code.py": BSIM_CENTRAL_CODE, "settings.toml": "CIRCUITPY_BLE_WORKFLOW = false\n"} +) +def test_bsim_pairing_cp_to_cp(bsim_phy, board, circuitpython1, circuitpython2): """Two CP instances: device 0 peripheral, device 1 central pairs to it.""" peripheral = circuitpython1 central = circuitpython2 @@ -160,9 +160,16 @@ def test_bsim_pairing_cp_to_cp(bsim_phy, circuitpython1, circuitpython2): @pytest.mark.duration(22) -@pytest.mark.circuitpy_drive({"code.py": BSIM_PERIPHERAL_CODE}) -@pytest.mark.circuitpy_drive({"code.py": BSIM_CENTRAL_PAIRED_PROPERTY_CODE}) -def test_bsim_pairing_paired_property(bsim_phy, circuitpython1, circuitpython2): +@pytest.mark.circuitpy_drive( + {"code.py": BSIM_PERIPHERAL_CODE, "settings.toml": "CIRCUITPY_BLE_WORKFLOW = false\n"} +) +@pytest.mark.circuitpy_drive( + { + "code.py": BSIM_CENTRAL_PAIRED_PROPERTY_CODE, + "settings.toml": "CIRCUITPY_BLE_WORKFLOW = false\n", + } +) +def test_bsim_pairing_paired_property(bsim_phy, board, circuitpython1, circuitpython2): """Verify connection.paired transitions from False to True after pairing.""" peripheral = circuitpython1 central = circuitpython2 @@ -309,9 +316,13 @@ def test_bsim_pairing_paired_property(bsim_phy, circuitpython1, circuitpython2): @pytest.mark.duration(35) -@pytest.mark.circuitpy_drive({"code.py": BSIM_PERIPHERAL_BOND_CODE}) -@pytest.mark.circuitpy_drive({"code.py": BSIM_CENTRAL_BOND_CODE}) -def test_bsim_bonding_persistence(bsim_phy, circuitpython1, circuitpython2): +@pytest.mark.circuitpy_drive( + {"code.py": BSIM_PERIPHERAL_BOND_CODE, "settings.toml": "CIRCUITPY_BLE_WORKFLOW = false\n"} +) +@pytest.mark.circuitpy_drive( + {"code.py": BSIM_CENTRAL_BOND_CODE, "settings.toml": "CIRCUITPY_BLE_WORKFLOW = false\n"} +) +def test_bsim_bonding_persistence(bsim_phy, board, circuitpython1, circuitpython2): """Verify bonds are saved to storage and survive disconnect/reconnect. Central pairs with peripheral, disconnects, then reconnects. diff --git a/ports/zephyr-cp/tests/bsim/test_bsim_ble_peripheral.py b/ports/zephyr-cp/tests/bsim/test_bsim_ble_peripheral.py index 89088fb9f87..a8eb2612b40 100644 --- a/ports/zephyr-cp/tests/bsim/test_bsim_ble_peripheral.py +++ b/ports/zephyr-cp/tests/bsim/test_bsim_ble_peripheral.py @@ -73,7 +73,9 @@ @pytest.mark.zephyr_sample("bluetooth/central") @pytest.mark.duration(14) -@pytest.mark.circuitpy_drive({"code.py": BSIM_PERIPHERAL_CODE}) +@pytest.mark.circuitpy_drive( + {"code.py": BSIM_PERIPHERAL_CODE, "settings.toml": "CIRCUITPY_BLE_WORKFLOW = false\n"} +) def test_bsim_peripheral_zephyr_central(bsim_phy, circuitpython, zephyr_sample): """Advertise as connectable from CP; Zephyr central connects and disconnects.""" central = zephyr_sample diff --git a/ports/zephyr-cp/tests/bsim/test_bsim_ble_scan.py b/ports/zephyr-cp/tests/bsim/test_bsim_ble_scan.py index a2618a851f2..9f58914ade4 100644 --- a/ports/zephyr-cp/tests/bsim/test_bsim_ble_scan.py +++ b/ports/zephyr-cp/tests/bsim/test_bsim_ble_scan.py @@ -111,7 +111,7 @@ def test_bsim_scan_zephyr_beacon(bsim_phy, circuitpython, zephyr_sample): @pytest.mark.zephyr_sample("bluetooth/beacon") @pytest.mark.code_py_runs(2) -@pytest.mark.duration(4) +@pytest.mark.duration(20) @pytest.mark.circuitpy_drive({"code.py": BSIM_SCAN_RELOAD_CODE}) def test_bsim_scan_zephyr_beacon_reload(bsim_phy, circuitpython, zephyr_sample): """Scan for Zephyr beacon, soft reload, and scan again.""" diff --git a/ports/zephyr-cp/tests/bsim/test_bsim_ble_service.py b/ports/zephyr-cp/tests/bsim/test_bsim_ble_service.py index eb83c016b3a..1d4174835d0 100644 --- a/ports/zephyr-cp/tests/bsim/test_bsim_ble_service.py +++ b/ports/zephyr-cp/tests/bsim/test_bsim_ble_service.py @@ -84,7 +84,12 @@ class HeartRateService(Service): @pytest.mark.zephyr_sample("tests/bsim/samples/central_battery_client") @pytest.mark.duration(14) @pytest.mark.circuitpy_drive( - {"code.py": BSIM_SERVICE_CODE, "battery_service.py": BATTERY_LIB, **_ADAFRUIT_BLE} + { + "code.py": BSIM_SERVICE_CODE, + "settings.toml": "CIRCUITPY_BLE_WORKFLOW = false\n", + "battery_service.py": BATTERY_LIB, + **_ADAFRUIT_BLE, + } ) def test_bsim_service_battery(bsim_phy, circuitpython, zephyr_sample): """CP hosts BatteryService; Zephyr central reads battery level.""" @@ -177,7 +182,7 @@ def test_bsim_service_battery(bsim_phy, circuitpython, zephyr_sample): """ -@pytest.mark.duration(14) +@pytest.mark.duration(30) @pytest.mark.circuitpy_drive( {"code.py": BSIM_SERVER_CODE, "battery_service.py": BATTERY_LIB, **_ADAFRUIT_BLE} ) @@ -294,7 +299,7 @@ def test_bsim_service_cp_client(bsim_phy, circuitpython1, circuitpython2): """ -@pytest.mark.duration(14) +@pytest.mark.duration(35) @pytest.mark.circuitpy_drive( { "code.py": BSIM_DISCOVER_ALL_SERVER_CODE, @@ -440,7 +445,7 @@ class MultiService(Service): """ -@pytest.mark.duration(14) +@pytest.mark.duration(25) @pytest.mark.circuitpy_drive( {"code.py": BSIM_MULTI_CHAR_SERVER_CODE, "multi_service.py": MULTI_LIB, **_ADAFRUIT_BLE} ) @@ -510,7 +515,7 @@ def test_bsim_service_write_no_response(bsim_phy, circuitpython1, circuitpython2 """ -@pytest.mark.duration(14) +@pytest.mark.duration(25) @pytest.mark.circuitpy_drive( {"code.py": BSIM_MULTI_CHAR_SERVER_CODE, "multi_service.py": MULTI_LIB, **_ADAFRUIT_BLE} ) @@ -625,7 +630,7 @@ class CustomService(Service): """ -@pytest.mark.duration(14) +@pytest.mark.duration(25) @pytest.mark.circuitpy_drive( {"code.py": BSIM_CUSTOM_UUID_SERVER_CODE, "custom_service.py": CUSTOM_LIB, **_ADAFRUIT_BLE} ) @@ -689,7 +694,7 @@ def test_bsim_service_custom_uuid(bsim_phy, circuitpython1, circuitpython2): """ -@pytest.mark.duration(14) +@pytest.mark.duration(30) @pytest.mark.circuitpy_drive( {"code.py": BSIM_CUSTOM_UUID_SERVER_CODE, "custom_service.py": CUSTOM_LIB, **_ADAFRUIT_BLE} ) 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..a6e9bc6e0dd --- /dev/null +++ b/ports/zephyr-cp/tests/bsim/test_bsim_ble_workflow_advertising.py @@ -0,0 +1,112 @@ +# 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 logging +import re + +import pytest + +from .conftest import get_library_files + +logger = logging.getLogger(__name__) + +_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 +ft_rssi = None +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 + ft_rssi = adv.rssi + print("ft_uuid febb") + name = adv.complete_name or adv.short_name or "" + if name: + found_name = True + print("cp_name", name) + if found_ft and found_name: + print("workflow found") + print("ft_rssi", ft_rssi) + break +ble.stop_scan() +print("scan done", found_ft, found_name, ft_rssi) +""" + +# 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}" + ) + # The device name currently advertises as the controller default ("Zephyr"); + # the CIRCUITPY name is configured separately, at which point this will + # observe the full "CIRCUITPY..." name. Either way a name must be present. + assert "cp_name" in scanner_output, f"device name not observed: {scanner_output}" + assert "workflow found" in scanner_output, ( + f"workflow advertisement not fully observed: {scanner_output}" + ) + + # The workflow advertises its public advert at reduced TX power (-20 dBm), + # so with the 40 dB channel attenuation the RSSI is ~ -59, well below the + # 0 dBm baseline of -39 seen for user advertisements. + rssi_match = re.search(r"ft_rssi (-?\d+)", scanner_output) + assert rssi_match, f"ft_rssi not observed: {scanner_output}" + ft_rssi = int(rssi_match.group(1)) + logger.info("workflow ft_rssi: %d", ft_rssi) + assert ft_rssi < -39, f"Expected reduced-TX-power RSSI (< -39), got {ft_rssi}" + + # 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..a7c383febe1 --- /dev/null +++ b/ports/zephyr-cp/tests/bsim/test_bsim_ble_workflow_file_transfer.py @@ -0,0 +1,102 @@ +# 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. +""" + +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: 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.""" + 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..ebc0bbcc69c --- /dev/null +++ b/ports/zephyr-cp/tests/bsim/test_bsim_ble_workflow_nus.py @@ -0,0 +1,146 @@ +# 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. +""" + +import pytest + +from .conftest import get_library_files + +_ADAFRUIT_BLE = get_library_files("adafruit_ble") + +# 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 time +print("nus ready") +try: + line = input() +except EOFError: + line = "" +print("nus_echo:" + line) +# Keep the VM alive briefly so the BLE TX notification carrying the echo is +# actually transmitted over the link before the device exits. Without this the +# notification is queued but never sent because the process shuts down right +# after code.py finishes. +time.sleep(3) +""" + +# 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() + +# Wait for the workflow device's serial service to finish starting up +# before scanning. Connecting too soon after the workflow's BLE init races +# with a Zephyr connection-parameter update and the link drops shortly +# after pairing. +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) + +# 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.""" + 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}" + ) diff --git a/ports/zephyr-cp/tests/conftest.py b/ports/zephyr-cp/tests/conftest.py index 0efe3bcc3fe..ae796f4da97 100644 --- a/ports/zephyr-cp/tests/conftest.py +++ b/ports/zephyr-cp/tests/conftest.py @@ -325,6 +325,15 @@ def circuitpython(request, board, sim_id, native_sim_binary, native_sim_env, tmp f"-uart{uart_n}_pty", f"-uart{uart_n}_pty_wait_for_readers", "-uart_pty_wait", + # Use the real AES implementation (libCryptov1.so) for the + # link layer instead of BabbleSim's plain-text stand-in. + # The nRF54L HW models route AES through several different + # stand-ins (ECB/CCM copy the data, CRACEN scrambles it), + # which are not consistent with each other, so LE + # encryption only works with real AES. Zephyr's own bsim + # encryption tests pass -RealEncryption=1 for the same + # reason. + "-RealEncryption=1", f"--vm-runs={code_py_runs + 1}", ) )