From 281a67408fd5e6aa7c43588b195811f6190c4570 Mon Sep 17 00:00:00 2001 From: Dan Halbert Date: Thu, 11 Jun 2026 20:20:28 -0400 Subject: [PATCH 1/3] Fix slow espressif BLE discovery Avoid writing to remote characteristics during discovery, and make descriptor discovery use per-step timeouts and the correct end handle. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../common-hal/_bleio/Characteristic.c | 2 +- .../espressif/common-hal/_bleio/Connection.c | 53 +++++++++---------- 2 files changed, 25 insertions(+), 30 deletions(-) diff --git a/ports/espressif/common-hal/_bleio/Characteristic.c b/ports/espressif/common-hal/_bleio/Characteristic.c index 736c61c650eb4..ff9ab35c1b9f0 100644 --- a/ports/espressif/common-hal/_bleio/Characteristic.c +++ b/ports/espressif/common-hal/_bleio/Characteristic.c @@ -92,7 +92,7 @@ void common_hal_bleio_characteristic_construct(bleio_characteristic_obj_t *self, self->max_length = max_length; self->fixed_length = fixed_length; - if (initial_value_bufinfo != NULL) { + if (!service->is_remote && initial_value_bufinfo != NULL) { common_hal_bleio_characteristic_set_value(self, initial_value_bufinfo); } diff --git a/ports/espressif/common-hal/_bleio/Connection.c b/ports/espressif/common-hal/_bleio/Connection.c index 42816cffb6fae..d9597bdd73edf 100644 --- a/ports/espressif/common-hal/_bleio/Connection.c +++ b/ports/espressif/common-hal/_bleio/Connection.c @@ -183,7 +183,7 @@ static volatile int _last_discovery_status; static uint64_t _discovery_start_time; -// Give 20 seconds for discovery +// Give 20 seconds for each discovery step. #define DISCOVERY_TIMEOUT_MS 20000 static void _start_discovery_timeout(void) { @@ -199,6 +199,9 @@ static int _wait_for_discovery_step_done(void) { _last_discovery_status = BLE_HS_EDONE; } } + if (_last_discovery_status == 0) { + return BLE_HS_ETIMEOUT; + } return _last_discovery_status; } @@ -207,6 +210,17 @@ static void _set_discovery_step_status(int status) { _last_discovery_status = status; } +static void _check_discovery_status(int status) { + if (status == BLE_HS_EDONE) { + return; + } + if (status < BLE_HS_ERR_ATT_BASE) { + CHECK_NIMBLE_ERROR(status); + return; + } + CHECK_BLE_ERROR(status); +} + static int _discovered_service_cb(uint16_t conn_handle, const struct ble_gatt_error *error, const struct ble_gatt_svc *svc, @@ -280,10 +294,6 @@ static int _discovered_characteristic_cb(uint16_t conn_handle, // Set def_handle directly since it is only used in discovery. characteristic->def_handle = chr->def_handle; - #if CIRCUITPY_VERBOSE_BLE - mp_printf(&mp_plat_print, "_discovered_characteristic_cb: char handle: %d\n", characteristic->handle); - #endif - mp_obj_list_append(MP_OBJ_FROM_PTR(service->characteristic_list), MP_OBJ_FROM_PTR(characteristic)); return 0; @@ -297,12 +307,6 @@ static int _discovered_descriptor_cb(uint16_t conn_handle, bleio_characteristic_obj_t *characteristic = (bleio_characteristic_obj_t *)arg; if (error->status != 0) { - - #if CIRCUITPY_VERBOSE_BLE - mp_printf(&mp_plat_print, "_discovered_descriptor_cb error->status: %d, handle: %d\n", - error->status, error->att_handle); - #endif - // BLE_HS_EDONE or some error has occurred. _set_discovery_step_status(error->status); return 0; @@ -336,11 +340,6 @@ static int _discovered_descriptor_cb(uint16_t conn_handle, GATT_MAX_DATA_LENGTH, false, mp_const_empty_bytes); descriptor->handle = dsc->handle; - #if CIRCUITPY_VERBOSE_BLE - mp_printf(&mp_plat_print, "_discovered_descriptor_cb: char handle: %d, desc handle: %d, uuid type: %d, u16 value: 0x%x\n", - characteristic->handle, descriptor->handle, dsc->uuid.u.type, dsc->uuid.u16.value); - #endif - mp_obj_list_append(MP_OBJ_FROM_PTR(characteristic->descriptor_list), MP_OBJ_FROM_PTR(descriptor)); return 0; @@ -356,14 +355,13 @@ static void discover_remote_services(bleio_connection_internal_t *self, mp_obj_t if (service_uuids_whitelist == mp_const_none) { // Reset discovery status before starting callbacks _set_discovery_step_status(0); + _start_discovery_timeout(); CHECK_NIMBLE_ERROR(ble_gattc_disc_all_svcs(self->conn_handle, _discovered_service_cb, self)); // Wait for _discovered_service_cb() to be called multiple times until it's done. int status = _wait_for_discovery_step_done(); - if (status != BLE_HS_EDONE) { - CHECK_BLE_ERROR(status); - } + _check_discovery_status(status); } else { mp_obj_iter_buf_t iter_buf; mp_obj_t iterable = mp_getiter(service_uuids_whitelist, &iter_buf); @@ -376,15 +374,14 @@ static void discover_remote_services(bleio_connection_internal_t *self, mp_obj_t // Reset discovery status before starting callbacks _set_discovery_step_status(0); + _start_discovery_timeout(); CHECK_NIMBLE_ERROR(ble_gattc_disc_svc_by_uuid(self->conn_handle, &uuid->nimble_ble_uuid.u, _discovered_service_cb, self)); // Wait for _discovered_service_cb() to be called multiple times until it's done. int status = _wait_for_discovery_step_done(); - if (status != BLE_HS_EDONE) { - CHECK_BLE_ERROR(status); - } + _check_discovery_status(status); } } @@ -395,6 +392,7 @@ static void discover_remote_services(bleio_connection_internal_t *self, mp_obj_t // Reset discovery status before starting callbacks _set_discovery_step_status(0); + _start_discovery_timeout(); CHECK_NIMBLE_ERROR(ble_gattc_disc_all_chrs(self->conn_handle, service->start_handle, @@ -404,9 +402,7 @@ static void discover_remote_services(bleio_connection_internal_t *self, mp_obj_t // Wait for _discovered_characteristic_cb() to be called multiple times until it's done. int status = _wait_for_discovery_step_done(); - if (status != BLE_HS_EDONE) { - CHECK_BLE_ERROR(status); - } + _check_discovery_status(status); // Got characteristics for this service. Now discover descriptors for each characteristic. size_t char_list_len = service->characteristic_list->len; @@ -423,7 +419,7 @@ static void discover_remote_services(bleio_connection_internal_t *self, mp_obj_t uint16_t end_handle = next_characteristic == NULL ? service->end_handle - : next_characteristic->handle - 1; + : next_characteristic->def_handle - 1; // Pre-check if there are no descriptors to discover so descriptor discovery doesn't fail if (end_handle <= characteristic->handle) { @@ -432,6 +428,7 @@ static void discover_remote_services(bleio_connection_internal_t *self, mp_obj_t // Reset discovery status before starting callbacks _set_discovery_step_status(0); + _start_discovery_timeout(); // The descriptor handle inclusive range is [characteristic->handle + 1, end_handle], // but ble_gattc_disc_all_dscs() requires starting with characteristic->handle. @@ -441,9 +438,7 @@ static void discover_remote_services(bleio_connection_internal_t *self, mp_obj_t // Wait for _discovered_descriptor_cb to be called multiple times until it's done. status = _wait_for_discovery_step_done(); - if (status != BLE_HS_EDONE) { - CHECK_BLE_ERROR(status); - } + _check_discovery_status(status); } } } From 4433e995fd1765c80579d66ad58635abdeb1dd72 Mon Sep 17 00:00:00 2001 From: Dan Halbert Date: Fri, 12 Jun 2026 07:36:16 -0400 Subject: [PATCH 2/3] restore debugging code; shorten discovery step timeout --- .../espressif/common-hal/_bleio/Connection.c | 19 +++++++++++++++++-- 1 file changed, 17 insertions(+), 2 deletions(-) diff --git a/ports/espressif/common-hal/_bleio/Connection.c b/ports/espressif/common-hal/_bleio/Connection.c index d9597bdd73edf..86ebd6c8e9179 100644 --- a/ports/espressif/common-hal/_bleio/Connection.c +++ b/ports/espressif/common-hal/_bleio/Connection.c @@ -183,8 +183,8 @@ static volatile int _last_discovery_status; static uint64_t _discovery_start_time; -// Give 20 seconds for each discovery step. -#define DISCOVERY_TIMEOUT_MS 20000 +// Give 3 seconds for each discovery step. +#define DISCOVERY_TIMEOUT_MS 3000 static void _start_discovery_timeout(void) { _discovery_start_time = common_hal_time_monotonic_ms(); @@ -294,6 +294,10 @@ static int _discovered_characteristic_cb(uint16_t conn_handle, // Set def_handle directly since it is only used in discovery. characteristic->def_handle = chr->def_handle; + #if CIRCUITPY_VERBOSE_BLE + mp_printf(&mp_plat_print, "_discovered_characteristic_cb: char handle: %d\n", characteristic->handle); + #endif + mp_obj_list_append(MP_OBJ_FROM_PTR(service->characteristic_list), MP_OBJ_FROM_PTR(characteristic)); return 0; @@ -307,6 +311,12 @@ static int _discovered_descriptor_cb(uint16_t conn_handle, bleio_characteristic_obj_t *characteristic = (bleio_characteristic_obj_t *)arg; if (error->status != 0) { + + #if CIRCUITPY_VERBOSE_BLE + mp_printf(&mp_plat_print, "_discovered_descriptor_cb error->status: %d, handle: %d\n", + error->status, error->att_handle); + #endif + // BLE_HS_EDONE or some error has occurred. _set_discovery_step_status(error->status); return 0; @@ -340,6 +350,11 @@ static int _discovered_descriptor_cb(uint16_t conn_handle, GATT_MAX_DATA_LENGTH, false, mp_const_empty_bytes); descriptor->handle = dsc->handle; + #if CIRCUITPY_VERBOSE_BLE + mp_printf(&mp_plat_print, "_discovered_descriptor_cb: char handle: %d, desc handle: %d, uuid type: %d, u16 value: 0x%x\n", + characteristic->handle, descriptor->handle, dsc->uuid.u.type, dsc->uuid.u16.value); + #endif + mp_obj_list_append(MP_OBJ_FROM_PTR(characteristic->descriptor_list), MP_OBJ_FROM_PTR(descriptor)); return 0; From daa8531137283ad96473c438fe69b07683b8f2e5 Mon Sep 17 00:00:00 2001 From: Dan Halbert Date: Fri, 12 Jun 2026 10:24:41 -0400 Subject: [PATCH 3/3] simplify BLE connection timeout; increase back to 20 secs; 3 secs was too short --- .../espressif/common-hal/_bleio/Connection.c | 19 +++---------------- 1 file changed, 3 insertions(+), 16 deletions(-) diff --git a/ports/espressif/common-hal/_bleio/Connection.c b/ports/espressif/common-hal/_bleio/Connection.c index 86ebd6c8e9179..8df4dc26545d3 100644 --- a/ports/espressif/common-hal/_bleio/Connection.c +++ b/ports/espressif/common-hal/_bleio/Connection.c @@ -181,17 +181,11 @@ void common_hal_bleio_connection_set_connection_interval(bleio_connection_intern // Zero when discovery is in process. BLE_HS_EDONE or a BLE_HS_ error code when done. static volatile int _last_discovery_status; -static uint64_t _discovery_start_time; - -// Give 3 seconds for each discovery step. -#define DISCOVERY_TIMEOUT_MS 3000 - -static void _start_discovery_timeout(void) { - _discovery_start_time = common_hal_time_monotonic_ms(); -} +// Give 20 seconds for each step of discovery: services, characteristics, attributes. +#define DISCOVERY_TIMEOUT_MS 20000 static int _wait_for_discovery_step_done(void) { - const uint64_t timeout_time_ms = _discovery_start_time + DISCOVERY_TIMEOUT_MS; + const uint64_t timeout_time_ms = common_hal_time_monotonic_ms() + DISCOVERY_TIMEOUT_MS; while ((_last_discovery_status == 0) && (common_hal_time_monotonic_ms() < timeout_time_ms)) { RUN_BACKGROUND_TASKS; if (mp_hal_is_interrupted()) { @@ -364,13 +358,9 @@ static void discover_remote_services(bleio_connection_internal_t *self, mp_obj_t // Start over with an empty list. self->remote_service_list = mp_obj_new_list(0, NULL); - // Start timeout in case discovery gets stuck. - _start_discovery_timeout(); - if (service_uuids_whitelist == mp_const_none) { // Reset discovery status before starting callbacks _set_discovery_step_status(0); - _start_discovery_timeout(); CHECK_NIMBLE_ERROR(ble_gattc_disc_all_svcs(self->conn_handle, _discovered_service_cb, self)); @@ -389,7 +379,6 @@ static void discover_remote_services(bleio_connection_internal_t *self, mp_obj_t // Reset discovery status before starting callbacks _set_discovery_step_status(0); - _start_discovery_timeout(); CHECK_NIMBLE_ERROR(ble_gattc_disc_svc_by_uuid(self->conn_handle, &uuid->nimble_ble_uuid.u, _discovered_service_cb, self)); @@ -407,7 +396,6 @@ static void discover_remote_services(bleio_connection_internal_t *self, mp_obj_t // Reset discovery status before starting callbacks _set_discovery_step_status(0); - _start_discovery_timeout(); CHECK_NIMBLE_ERROR(ble_gattc_disc_all_chrs(self->conn_handle, service->start_handle, @@ -443,7 +431,6 @@ static void discover_remote_services(bleio_connection_internal_t *self, mp_obj_t // Reset discovery status before starting callbacks _set_discovery_step_status(0); - _start_discovery_timeout(); // The descriptor handle inclusive range is [characteristic->handle + 1, end_handle], // but ble_gattc_disc_all_dscs() requires starting with characteristic->handle.