8000 Various Espressif BLE fixes by dhalbert · Pull Request #9374 · adafruit/circuitpython · GitHub
[go: up one dir, main page]
Skip to content

Various Espressif BLE fixes #9374

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 4 commits into from
Jun 26, 2024
Merged

Various Espressif BLE fixes #9374

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
ea9a8df
espressif/common-hal/_bleio: use waiting loops instead of TaskNotify
dhalbert Jun 25, 2024
1019e91
espressif/common-hal/_bleio: int Characteristic observer properly; re…
dhalbert Jun 25, 2024
6bf9ec7
espressif/common-hal/_bleio/Adapter.c: use busy-waiting, to allow bac…
dhalbert Jun 25, 2024
b228cba
espress/common-hal/_bleio: Adapter.c: handle BLE_HS_EBUSY on scan
dhalbert Jun 25, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 6 additions & 1 deletion locale/circuitpython.pot
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2007,6 +2007,10 @@ msgstr ""
msgid "Too many channels in sample."
msgstr ""

#: ports/espressif/common-hal/_bleio/Characteristic.c
msgid "Too many descriptors"
msgstr ""

#: shared-module/displayio/__init__.c
msgid "Too many display busses; forgot displayio.release_displays() ?"
msgstr ""
Expand Down Expand Up @@ -2215,8 +2219,9 @@ msgstr ""
msgid "Unsupported hash algorithm"
msgstr ""

#: ports/espressif/common-hal/_bleio/Adapter.c
#: ports/espressif/common-hal/dualbank/__init__.c
msgid "Update Failed"
msgid "Update failed"
msgstr ""

#: ports/espressif/common-hal/_bleio/Characteristic.c
Expand Down
85 changes: 59 additions & 26 deletions ports/espressif/common-hal/_bleio/Adapter.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@
#include "shared-bindings/_bleio/Connection.h"
#include "shared-bindings/_bleio/ScanEntry.h"
#include "shared-bindings/time/__init__.h"
#include "shared/runtime/interrupt_char.h"

#include "controller/ble_ll_adv.h"
#include "nimble/hci_common.h"
Expand All @@ -47,22 +48,23 @@
#include "shared-module/os/__init__.h"
#endif

bleio_connection_internal_t bleio_connections[BLEIO_TOTAL_CONNECTION_COUNT];
// Status variables used while busy-waiting for events.
static volatile bool _nimble_sync;
static volatile int _connection_status;

bool ble_active = false;
bleio_connection_internal_t bleio_connections[BLEIO_TOTAL_CONNECTION_COUNT];

static void nimble_host_task(void *param) {
nimble_port_run();
nimble_port_freertos_deinit();
}

static TaskHandle_t cp_task = NULL;

static void _on_sync(void) {
int rc = ble_hs_util_ensure_addr(false);
assert(rc == 0);

xTaskNotifyGive(cp_task);
_nimble_sync = true;
}

// All examples have this. It'd make sense in a header.
Expand Down Expand Up @@ -133,15 +135,26 @@ void common_hal_bleio_adapter_set_enabled(bleio_adapter_obj_t *self, bool enable

ble_store_config_init();

cp_task = xTaskGetCurrentTaskHandle();

_nimble_sync = false;
nimble_port_freertos_init(nimble_host_task);
// Wait for sync.
ulTaskNotifyTake(pdTRUE, pdMS_TO_TICKS(200));
// Wait for sync from nimble task.
const uint64_t timeout_time_ms = common_hal_time_monotonic_ms() + 200;
while (!_nimble_sync && (common_hal_time_monotonic_ms() < timeout_time_ms)) {
RUN_BACKGROUND_TASKS;
if (mp_hal_is_interrupted()) {
// Return prematurely. Then the interrupt will be raised.
return;
}
}

if (!_nimble_sync) {
mp_raise_RuntimeError(MP_ERROR_TEXT("Update failed"));
}
} else {
int ret = nimble_port_stop();
while (xTaskGetHandle("nimble_host") != NULL) {
vTaskDelay(pdMS_TO_TICKS(2));
while (xTaskGetHandle("nimble_host") != NULL && !mp_hal_is_interrupted()) {
RUN_BACKGROUND_TASKS;
common_hal_time_delay_ms(2);
}
if (ret == 0) {
nimble_port_deinit();
Expand Down Expand Up @@ -277,8 +290,19 @@ mp_obj_t common_hal_bleio_adapter_start_scan(bleio_adapter_obj_t *self, uint8_t
duration_ms = BLE_HS_FOREVER;
}

CHECK_NIMBLE_ERROR(ble_gap_disc(own_addr_type, duration_ms, &disc_params,
_scan_event, self->scan_results));
int tries = 5;
int status;
// BLE_HS_EBUSY may occasionally occur, indicating something has not finished. Retry a few times if so.
do {
status = ble_gap_disc(own_addr_type, duration_ms, &disc_params,
_scan_event, self->scan_results);
if (status != BLE_HS_EBUSY) {
break;
}
common_hal_time_delay_ms(50);
RUN_BACKGROUND_TASKS;
} while (tries-- > 0);
CHECK_NIMBLE_ERROR(status);

return MP_OBJ_FROM_PTR(self->scan_results);
}
Expand Down Expand Up @@ -309,15 +333,16 @@ static int _mtu_reply(uint16_t conn_handle,
if (error->status == 0) {
connection->mtu = mtu;
}
xTaskNotify(cp_task, conn_handle, eSetValueWithOverwrite);
// Set status var to connection handle to report that connection is now established.
// Another routine is waiting for this.
_connection_status = conn_handle;
return 0;
}

static void _new_connection(uint16_t conn_handle) {
// Set the tx_power for the connection higher than the advertisement.
esp_ble_tx_power_set(conn_handle, ESP_PWR_LVL_N0);


// Find an empty connection. One should always be available because the SD has the same
// total connection limit.
bleio_connection_internal_t *connection = NULL;
Expand Down Expand Up @@ -353,13 +378,14 @@ static int _connect_event(struct ble_gap_event *event, void *self_in) {
switch (event->type) {
case BLE_GAP_EVENT_CONNECT:
if (event->connect.status == 0) {
// This triggers an MTU exchange. Its reply will unblock CP.
// This triggers an MTU exchange. Its reply will exit the loop waiting for a connection.
_new_connection(event->connect.conn_handle);
// Set connections objs back to NULL since we have a new
// connection and need a new tuple.
self->connection_objs = NULL;
} else {
xTaskNotify(cp_task, -event->connect.status, eSetValueWithOverwrite);
// The loop waiting for the connection to be comnpleted will stop when _connection_status changes.
_connection_status = -event->connect.status;
}
break;

Expand Down Expand Up @@ -397,24 +423,31 @@ mp_obj_t common_hal_bleio_adapter_connect(bleio_adapter_obj_t *self, bleio_addre
ble_addr_t addr;
_convert_address(address, &addr);

cp_task = xTaskGetCurrentTaskHandle();
// Make sure we don't have a pending notification from a previous time. This
// can happen if a previous wait timed out before the notification was given.
xTaskNotifyStateClear(cp_task);
const int timeout_ms = SEC_TO_UNITS(timeout, UNIT_1_MS) + 0.5f;
CHECK_NIMBLE_ERROR(
ble_gap_connect(own_addr_type, &addr,
SEC_TO_UNITS(timeout, UNIT_1_MS) + 0.5f,
timeout_ms,
&conn_params,
_connect_event, self));

int error_code;
// Wait an extra 50 ms to give the connect method the opportunity to time out.
CHECK_NOTIFY(xTaskNotifyWait(0, 0, (uint32_t *)&error_code, pdMS_TO_TICKS(timeout * 1000 + 50)));

const uint64_t timeout_time_ms = common_hal_time_monotonic_ms() + timeout_ms;
// _connection_status gets set to either a positive connection handle or a negative error code.
_connection_status = BLEIO_HANDLE_INVALID;
while (_connection_status == BLEIO_HANDLE_INVALID && (common_hal_time_monotonic_ms() < timeout_time_ms)) {
RUN_BACKGROUND_TASKS;
if (mp_hal_is_interrupted()) {
// Return prematurely. Then the interrupt exception will be raised.
return mp_const_none;
}
}

// Negative values are error codes, connection handle otherwise.
if (error_code < 0) {
CHECK_BLE_ERROR(-error_code);
if (_connection_status < 0) {
CHECK_BLE_ERROR(-_connection_status);
}
uint16_t conn_handle = error_code;
const uint16_t conn_handle = _connection_status;

// TODO: If we have keys, then try and encrypt the connection.

Expand Down
61 changes: 43 additions & 18 deletions ports/espressif/common-hal/_bleio/Characteristic.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,20 +7,47 @@
#include <string.h>

#include "py/runtime.h"
#include "shared/runtime/interrupt_char.h"

#include "shared-bindings/_bleio/__init__.h"
#include "shared-bindings/_bleio/Characteristic.h"
#include "shared-bindings/_bleio/CharacteristicBuffer.h"
#include "shared-bindings/_bleio/Descriptor.h"
#include "shared-bindings/_bleio/PacketBuffer.h"
#include "shared-bindings/_bleio/Service.h"
#include "shared-bindings/time/__init__.h"

#include "common-hal/_bleio/Adapter.h"
#include "common-hal/_bleio/Service.h"


static int characteristic_on_ble_gap_evt(struct ble_gap_event *event, void *param);

static volatile int _completion_status;
static uint64_t _timeout_start_time;

static void _reset_completion_status(void) {
_completion_status = 0;
}

// Wait for a status change, recorded in a callback.
// Try twice because sometimes we get a BLE_HS_EAGAIN.
// Maybe we should try more than twice.
static int _wait_for_completion(uint32_t timeout_msecs) {
for (int tries = 1; tries <= 2; tries++) {
_timeout_start_time = common_hal_time_monotonic_ms();
while ((_completion_status == 0) &&
(common_hal_time_monotonic_ms() < _timeout_start_time + timeout_msecs) &&
!mp_hal_is_interrupted()) {
RUN_BACKGROUND_TASKS;
}
if (_completion_status != BLE_HS_EAGAIN) {
// Quit, because either the status is either zero (OK) or it's an error.
break;
}
}
return _completion_status;
}

void common_hal_bleio_characteristic_construct(bleio_characteristic_obj_t *self, bleio_service_obj_t *service,
uint16_t handl F438 e, bleio_uuid_obj_t *uuid, bleio_characteristic_properties_t props,
bleio_attribute_security_mode_t read_perm, bleio_attribute_security_mode_t write_perm,
Expand All @@ -34,7 +61,7 @@ void common_hal_bleio_characteristic_construct(bleio_characteristic_obj_t *self,
self->props = props;
self->read_perm = read_perm;
self->write_perm = write_perm;
self->observer = NULL;
self->observer = mp_const_none;

// Map CP's property values to Nimble's flag values.
self->flags = 0;
Expand Down Expand Up @@ -125,7 +152,6 @@ bleio_service_obj_t *common_hal_bleio_characteristic_get_service(bleio_character
}

typedef struct {
TaskHandle_t task;
uint8_t *buf;
uint16_t len;
} _read_info_t;
Expand All @@ -148,9 +174,9 @@ static int _read_cb(uint16_t conn_handle,
// For debugging.
mp_printf(&mp_plat_print, "Read status: %d\n", error->status);
#endif
xTaskNotify(read_info->task, error->status, eSetValueWithOverwrite);
break;
}
_completion_status = error->status;

return 0;
}
Expand All @@ -163,14 +189,12 @@ size_t common_hal_bleio_characteristic_get_value(bleio_characteristic_obj_t *sel
uint16_t conn_handle = bleio_connection_get_conn_handle(self->service->connection);
if (common_hal_bleio_service_get_is_remote(self->service)) {
_read_info_t read_info = {
.task = xTaskGetCurrentTaskHandle(),
.buf = buf,
.len = len
};
_reset_completion_status();
CHECK_NIMBLE_ERROR(ble_gattc_read(conn_handle, self->handle, _read_cb, &read_info));
int error_code;
xTaskNotifyWait(0, 0, (uint32_t *)&error_code, 200);
CHECK_BLE_ERROR(error_code);
CHECK_NIMBLE_ERROR(_wait_for_completion(2000));
return read_info.len;
} else {
len = MIN(self->current_value_len, len);
Expand All @@ -189,8 +213,7 @@ static int _write_cb(uint16_t conn_handle,
const struct ble_gatt_error *error,
struct ble_gatt_attr *attr,
void *arg) {
TaskHandle_t task = (TaskHandle_t)arg;
xTaskNotify(task, error->status, eSetValueWithOverwrite);
_completion_status = error->status;

return 0;
}
Expand All @@ -201,10 +224,9 @@ void common_hal_bleio_characteristic_set_value(bleio_characteristic_obj_t *self,
if ((self->props & CHAR_PROP_WRITE_NO_RESPONSE) != 0) {
CHECK_NIMBLE_ERROR(ble_gattc_write_no_rsp_flat(conn_handle, self->handle, bufinfo->buf, bufinfo->len));
} else {
CHECK_NIMBLE_ERROR(ble_gattc_write_flat(conn_handle, self->handle, bufinfo->buf, bufinfo->len, _write_cb, xTaskGetCurrentTaskHandle()));
int error_code;
xTaskNotifyWait(0, 0, (uint32_t *)&error_code, 200);
CHECK_BLE_ERROR(error_code);
_reset_completion_status();
CHECK_NIMBLE_ERROR(ble_gattc_write_flat(conn_handle, self->handle, bufinfo->buf, bufinfo->len, _write_cb, NULL));
CHECK_NIMBLE_ERROR(_wait_for_completion(2000));
}
} else {
// Validate data length for local characteristics only.
Expand Down Expand Up @@ -338,6 +360,10 @@ bleio_characteristic_properties_t common_hal_bleio_characteristic_get_properties
void common_hal_bleio_characteristic_add_descriptor(bleio_characteristic_obj_t *self,
bleio_descriptor_obj_t *descriptor) {
size_t i = self->descriptor_list->len;
if (i >= MAX_DESCRIPTORS) {
mp_raise_bleio_BluetoothError(MP_ERROR_TEXT("Too many descriptors"));
}

mp_obj_list_append(MP_OBJ_FROM_PTR(self->descriptor_list),
MP_OBJ_FROM_PTR(descriptor));

Expand Down Expand Up @@ -368,10 +394,9 @@ void common_hal_bleio_characteristic_set_cccd(bleio_characteristic_obj_t *self,
(notify ? 1 << 0 : 0) |
(indicate ? 1 << 1: 0);

CHECK_NIMBLE_ERROR(ble_gattc_write_flat(conn_handle, self->cccd_handle, &cccd_value, 2, _write_cb, xTaskGetCurrentTaskHandle()));
int error_code;
xTaskNotifyWait(0, 0, (uint32_t *)&error_code, 200);
CHECK_BLE_ERROR(error_code);
_reset_completion_status();
CHECK_NIMBLE_ERROR(ble_gattc_write_flat(conn_handle, self->cccd_handle, &cccd_value, 2, _write_cb, NULL));
CHECK_NIMBLE_ERROR(_wait_for_completion(2000));
}

void bleio_characteristic_set_observer(bleio_characteristic_obj_t *self, mp_obj_t observer) {
Expand Down
1 change: 0 additions & 1 deletion ports/espressif/common-hal/_bleio/CharacteristicBuffer.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,6 @@ uint32_t common_hal_bleio_characteristic_buffer_read(bleio_characteristic_buffer
}

uint32_t num_bytes_read = ringbuf_get_n(&self->ringbuf, data, len);

return num_bytes_read;
}

Expand Down
Loading
Loading
0