From bf3eb9dc3911b8e4e7da3054ffece61ae5e2ef64 Mon Sep 17 00:00:00 2001 From: "David (Pololu)" Date: Wed, 12 Apr 2023 18:58:44 -0700 Subject: [PATCH 001/701] rp2/machine_i2c: Add timeout parameter for machine.I2C(). This commit adds support for the `timeout` keyword argument to machine.I2C on the rp2 port, following how it's done on other ports. The main motivation here is avoid the interpreter crashing due to infinite loops when SDA is stuck low, which is quite common if the board gets reset while reading from an I2C device. A default timeout of 50ms is chosen because it's consistent with: - Commit a707fe50b085ec83722106609f6fd219faf9f030 which used a timeout of 50,000us for zero-length writes on the rp2 port. - The machine.SoftI2C class which uses 50,000us as the default timeout. - The stm32 port's hardware I2C, which uses 50,000us for I2C_POLL_DEFAULT_TIMEOUT_US. This commit also fixes the default timeout on the esp32 port to be consistent with the above, and updates the documentation for machine.I2C to document this keyword argument. --- docs/library/machine.I2C.rst | 4 +++- ports/esp32/machine_i2c.c | 2 +- ports/rp2/machine_i2c.c | 16 ++++++++++------ 3 files changed, 14 insertions(+), 8 deletions(-) diff --git a/docs/library/machine.I2C.rst b/docs/library/machine.I2C.rst index 0eb1b67f58..bfc9f7ebcc 100644 --- a/docs/library/machine.I2C.rst +++ b/docs/library/machine.I2C.rst @@ -52,7 +52,7 @@ Example usage:: Constructors ------------ -.. class:: I2C(id, *, scl, sda, freq=400000) +.. class:: I2C(id, *, scl, sda, freq=400000, timeout=50000) Construct and return a new I2C object using the following parameters: @@ -62,6 +62,8 @@ Constructors - *sda* should be a pin object specifying the pin to use for SDA. - *freq* should be an integer which sets the maximum frequency for SCL. + - *timeout* is the maximum time in microseconds to allow for I2C + transactions. This parameter is not allowed on some ports. Note that some ports/boards will have default values of *scl* and *sda* that can be changed in this constructor. Others will have fixed values diff --git a/ports/esp32/machine_i2c.c b/ports/esp32/machine_i2c.c index c805dab87e..9ec39e5649 100644 --- a/ports/esp32/machine_i2c.c +++ b/ports/esp32/machine_i2c.c @@ -62,7 +62,7 @@ #error "unsupported I2C for ESP32 SoC variant" #endif -#define I2C_DEFAULT_TIMEOUT_US (10000) // 10ms +#define I2C_DEFAULT_TIMEOUT_US (50000) // 50ms typedef struct _machine_hw_i2c_obj_t { mp_obj_base_t base; diff --git a/ports/rp2/machine_i2c.c b/ports/rp2/machine_i2c.c index 85d12c7713..fc4c0723a5 100644 --- a/ports/rp2/machine_i2c.c +++ b/ports/rp2/machine_i2c.c @@ -33,6 +33,7 @@ #include "hardware/i2c.h" #define DEFAULT_I2C_FREQ (400000) +#define DEFAULT_I2C_TIMEOUT (50000) #ifndef MICROPY_HW_I2C0_SCL #if PICO_DEFAULT_I2C == 0 @@ -65,6 +66,7 @@ typedef struct _machine_i2c_obj_t { uint8_t scl; uint8_t sda; uint32_t freq; + uint32_t timeout; } machine_i2c_obj_t; STATIC machine_i2c_obj_t machine_i2c_obj[] = { @@ -74,17 +76,18 @@ STATIC machine_i2c_obj_t machine_i2c_obj[] = { STATIC void machine_i2c_print(const mp_print_t *print, mp_obj_t self_in, mp_print_kind_t kind) { machine_i2c_obj_t *self = MP_OBJ_TO_PTR(self_in); - mp_printf(print, "I2C(%u, freq=%u, scl=%u, sda=%u)", - self->i2c_id, self->freq, self->scl, self->sda); + mp_printf(print, "I2C(%u, freq=%u, scl=%u, sda=%u, timeout=%u)", + self->i2c_id, self->freq, self->scl, self->sda, self->timeout); } mp_obj_t machine_i2c_make_new(const mp_obj_type_t *type, size_t n_args, size_t n_kw, const mp_obj_t *all_args) { - enum { ARG_id, ARG_freq, ARG_scl, ARG_sda }; + enum { ARG_id, ARG_freq, ARG_scl, ARG_sda, ARG_timeout }; static const mp_arg_t allowed_args[] = { { MP_QSTR_id, MP_ARG_REQUIRED | MP_ARG_OBJ }, { MP_QSTR_freq, MP_ARG_INT, {.u_int = DEFAULT_I2C_FREQ} }, { MP_QSTR_scl, MP_ARG_KW_ONLY | MP_ARG_OBJ, {.u_rom_obj = MP_ROM_NONE} }, { MP_QSTR_sda, MP_ARG_KW_ONLY | MP_ARG_OBJ, {.u_rom_obj = MP_ROM_NONE} }, + { MP_QSTR_timeout, MP_ARG_KW_ONLY | MP_ARG_INT, {.u_int = DEFAULT_I2C_TIMEOUT} }, }; // Parse args. @@ -121,6 +124,7 @@ mp_obj_t machine_i2c_make_new(const mp_obj_type_t *type, size_t n_args, size_t n self->freq = args[ARG_freq].u_int; i2c_init(self->i2c_inst, self->freq); self->freq = i2c_set_baudrate(self->i2c_inst, self->freq); + self->timeout = args[ARG_timeout].u_int; gpio_set_function(self->scl, GPIO_FUNC_I2C); gpio_set_function(self->sda, GPIO_FUNC_I2C); gpio_set_pulls(self->scl, true, 0); @@ -135,14 +139,14 @@ STATIC int machine_i2c_transfer_single(mp_obj_base_t *self_in, uint16_t addr, si int ret; bool nostop = !(flags & MP_MACHINE_I2C_FLAG_STOP); if (flags & MP_MACHINE_I2C_FLAG_READ) { - ret = i2c_read_blocking(self->i2c_inst, addr, buf, len, nostop); + ret = i2c_read_timeout_us(self->i2c_inst, addr, buf, len, nostop, self->timeout); } else { if (len == 0) { // Workaround issue with hardware I2C not accepting zero-length writes. mp_machine_soft_i2c_obj_t soft_i2c = { .base = { &mp_machine_soft_i2c_type }, .us_delay = 500000 / self->freq + 1, - .us_timeout = 50000, + .us_timeout = self->timeout, .scl = self->scl, .sda = self->sda, }; @@ -157,7 +161,7 @@ STATIC int machine_i2c_transfer_single(mp_obj_base_t *self_in, uint16_t addr, si gpio_set_function(self->sda, GPIO_FUNC_I2C); return ret; } else { - ret = i2c_write_blocking(self->i2c_inst, addr, buf, len, nostop); + ret = i2c_write_timeout_us(self->i2c_inst, addr, buf, len, nostop, self->timeout); } } if (ret < 0) { From ec1eeccab45af6893f19cd07292808bbe7c62cf5 Mon Sep 17 00:00:00 2001 From: Damien George Date: Fri, 21 Apr 2023 18:17:34 +1000 Subject: [PATCH 002/701] shared/tinyusb: Revert setting of CFG_TUD_CDC_EP_BUFSIZE to 256. This reverts commit 0613d3e3561a82fffa36594647ef916b19bc912b. The value of CFG_TUD_CDC_EP_BUFSIZE should match the endpoint size, otherwise data may stay in the lower layers of the USB stack (and not passed up to the higher layers) until a total of CFG_TUD_CDC_EP_BUFSIZE bytes have been received, which may not happen for a long time. Fixes issue #11253. Signed-off-by: Damien George --- shared/tinyusb/tusb_config.h | 1 - 1 file changed, 1 deletion(-) diff --git a/shared/tinyusb/tusb_config.h b/shared/tinyusb/tusb_config.h index a6410a1c93..28bee09a59 100644 --- a/shared/tinyusb/tusb_config.h +++ b/shared/tinyusb/tusb_config.h @@ -59,7 +59,6 @@ // CDC Configuration #if CFG_TUD_CDC -#define CFG_TUD_CDC_EP_BUFSIZE (256) #define CFG_TUD_CDC_RX_BUFSIZE (256) #define CFG_TUD_CDC_TX_BUFSIZE (256) #endif From 9e6885ad820a9f42bf93df56eb3f3be0c8639622 Mon Sep 17 00:00:00 2001 From: Damien George Date: Sat, 22 Apr 2023 00:39:31 +1000 Subject: [PATCH 003/701] extmod/btstack: Switch to use hci_dump_init instead of hci_dump_open. The latter is no longer available in the version of BTstack now in use by this repository. Signed-off-by: Damien George --- extmod/btstack/btstack.mk | 1 + extmod/btstack/btstack_config_common.h | 1 + ports/stm32/mpbtstackport.c | 3 ++- ports/unix/mpbtstackport_common.c | 3 ++- 4 files changed, 6 insertions(+), 2 deletions(-) diff --git a/extmod/btstack/btstack.mk b/extmod/btstack/btstack.mk index ef730d2cf3..281d032ae1 100644 --- a/extmod/btstack/btstack.mk +++ b/extmod/btstack/btstack.mk @@ -35,6 +35,7 @@ INC += -I$(BTSTACK_DIR)/3rd-party/yxml SRC_BTSTACK_C = \ $(addprefix lib/btstack/src/, $(SRC_FILES)) \ $(addprefix lib/btstack/src/ble/, $(filter-out %_tlv.c, $(SRC_BLE_FILES))) \ + lib/btstack/platform/embedded/hci_dump_embedded_stdout.c \ ifeq ($(MICROPY_BLUETOOTH_BTSTACK_USB),1) ifeq ($(MICROPY_BLUETOOTH_BTSTACK_H4),1) diff --git a/extmod/btstack/btstack_config_common.h b/extmod/btstack/btstack_config_common.h index 8118c4c1a9..0f616f7505 100644 --- a/extmod/btstack/btstack_config_common.h +++ b/extmod/btstack/btstack_config_common.h @@ -7,6 +7,7 @@ #define ENABLE_LE_CENTRAL // #define ENABLE_CLASSIC #define ENABLE_L2CAP_LE_CREDIT_BASED_FLOW_CONTROL_MODE +#define ENABLE_PRINTF_HEXDUMP // #define ENABLE_LOG_INFO // #define ENABLE_LOG_DEBUG #define ENABLE_LOG_ERROR diff --git a/ports/stm32/mpbtstackport.c b/ports/stm32/mpbtstackport.c index 301ac30e20..728594d19c 100644 --- a/ports/stm32/mpbtstackport.c +++ b/ports/stm32/mpbtstackport.c @@ -32,6 +32,7 @@ #include "lib/btstack/src/btstack.h" #include "lib/btstack/src/hci_transport_h4.h" +#include "lib/btstack/platform/embedded/hci_dump_embedded_stdout.h" #include "extmod/mpbthci.h" #include "extmod/btstack/btstack_hci_uart.h" #include "extmod/btstack/modbluetooth_btstack.h" @@ -140,7 +141,7 @@ void mp_bluetooth_hci_poll(void) { void mp_bluetooth_btstack_port_init(void) { btstack_run_loop_init(&mp_btstack_runloop_stm32); - // hci_dump_open(NULL, HCI_DUMP_STDOUT); + // hci_dump_init(hci_dump_embedded_stdout_get_instance()); const hci_transport_t *transport = hci_transport_h4_instance_for_uart(&mp_bluetooth_btstack_hci_uart_block); hci_init(transport, &hci_transport_config_uart); diff --git a/ports/unix/mpbtstackport_common.c b/ports/unix/mpbtstackport_common.c index 66a3a0536a..36412e96f4 100644 --- a/ports/unix/mpbtstackport_common.c +++ b/ports/unix/mpbtstackport_common.c @@ -35,6 +35,7 @@ #include "lib/btstack/platform/embedded/btstack_run_loop_embedded.h" #include "lib/btstack/platform/embedded/hal_cpu.h" #include "lib/btstack/platform/embedded/hal_time_ms.h" +#include "lib/btstack/platform/embedded/hci_dump_embedded_stdout.h" #include "extmod/btstack/modbluetooth_btstack.h" @@ -81,7 +82,7 @@ uint32_t hal_time_ms(void) { void mp_bluetooth_btstack_port_init(void) { btstack_run_loop_init(btstack_run_loop_embedded_get_instance()); - // hci_dump_open(NULL, HCI_DUMP_STDOUT); + // hci_dump_init(hci_dump_embedded_stdout_get_instance()); #if MICROPY_BLUETOOTH_BTSTACK_H4 mp_bluetooth_btstack_port_init_h4(); From bc9ec1cf718361c1f361bb4ff6de3c660d84696d Mon Sep 17 00:00:00 2001 From: Jim Mussared Date: Thu, 2 Mar 2023 15:40:59 +1100 Subject: [PATCH 004/701] extmod/modbluetooth: Merge gatts_notify/indicate implementation. Makes gatts_notify and gatts_indicate work in the same way: by default they send the DB value, but you can manually override the payload. In other words, makes gatts_indicate work the same as gatts_notify. Note: This removes support for queuing notifications and indications on btstack when the ACL buffer is full. This functionality will be reimplemented in a future commit. Signed-off-by: Jim Mussared --- docs/library/bluetooth.rst | 19 ++-- extmod/btstack/modbluetooth_btstack.c | 125 +++++--------------------- extmod/modbluetooth.c | 32 ++++--- extmod/modbluetooth.h | 16 ++-- extmod/nimble/modbluetooth_nimble.c | 51 ++++++----- ports/zephyr/modbluetooth_zephyr.c | 18 +--- 6 files changed, 88 insertions(+), 173 deletions(-) diff --git a/docs/library/bluetooth.rst b/docs/library/bluetooth.rst index 8f7041e8d3..63578af16e 100644 --- a/docs/library/bluetooth.rst +++ b/docs/library/bluetooth.rst @@ -514,19 +514,24 @@ writes from a client to a given characteristic, use Sends a notification request to a connected client. - If *data* is not ``None``, then that value is sent to the client as part of - the notification. The local value will not be modified. + If *data* is ``None`` (the default), then the current local value (as set + with :meth:`gatts_write `) will be sent. - Otherwise, if *data* is ``None``, then the current local value (as - set with :meth:`gatts_write `) will be sent. + Otherwise, if *data* is not ``None``, then that value is sent to the client + as part of the notification. The local value will not be modified. **Note:** The notification will be sent regardless of the subscription status of the client to this characteristic. -.. method:: BLE.gatts_indicate(conn_handle, value_handle, /) +.. method:: BLE.gatts_indicate(conn_handle, value_handle, data=None, /) - Sends an indication request containing the characteristic's current value to - a connected client. + Sends a indication request to a connected client. + + If *data* is ``None`` (the default), then the current local value (as set + with :meth:`gatts_write `) will be sent. + + Otherwise, if *data* is not ``None``, then that value is sent to the client + as part of the indication. The local value will not be modified. On acknowledgment (or failure, e.g. timeout), the ``_IRQ_GATTS_INDICATE_DONE`` event will be raised. diff --git a/extmod/btstack/modbluetooth_btstack.c b/extmod/btstack/modbluetooth_btstack.c index e9c0037394..243b6e38ca 100644 --- a/extmod/btstack/modbluetooth_btstack.c +++ b/extmod/btstack/modbluetooth_btstack.c @@ -130,8 +130,6 @@ STATIC mp_obj_bluetooth_uuid_t create_mp_uuid(uint16_t uuid16, const uint8_t *uu // Pending operation types. enum { // Queued for sending when possible. - MP_BLUETOOTH_BTSTACK_PENDING_NOTIFY, // Waiting for context callback - MP_BLUETOOTH_BTSTACK_PENDING_INDICATE, // Waiting for context callback MP_BLUETOOTH_BTSTACK_PENDING_WRITE_NO_RESPONSE, // Waiting for conn handle // Hold buffer pointer until complete. MP_BLUETOOTH_BTSTACK_PENDING_WRITE, // Waiting for write done event @@ -150,11 +148,7 @@ struct _mp_btstack_pending_op_t { uint16_t conn_handle; uint16_t value_handle; - // For notify/indicate only. - // context_registration.context will point back to this struct. - btstack_context_callback_registration_t context_registration; - - // For notify/indicate/write-without-response, this is the actual buffer to send. + // For write-without-response, this is the actual buffer to send. // For write-with-response, just holding onto the buffer for GC ref. size_t len; uint8_t buf[]; @@ -170,30 +164,6 @@ STATIC void btstack_remove_pending_operation(mp_btstack_pending_op_t *pending_op } } -// Called in response to a gatts_notify/indicate being unable to complete, which then calls -// att_server_request_to_send_notification. -// We now have an opportunity to re-try the operation with an empty ACL buffer. -STATIC void btstack_notify_indicate_ready_handler(void *context) { - MICROPY_PY_BLUETOOTH_ENTER - mp_btstack_pending_op_t *pending_op = (mp_btstack_pending_op_t *)context; - DEBUG_printf("btstack_notify_indicate_ready_handler op_type=%d conn_handle=%d value_handle=%d len=%zu\n", pending_op->op_type, pending_op->conn_handle, pending_op->value_handle, pending_op->len); - if (pending_op->op_type == MP_BLUETOOTH_BTSTACK_PENDING_NOTIFY) { - int err = att_server_notify(pending_op->conn_handle, pending_op->value_handle, pending_op->buf, pending_op->len); - DEBUG_printf("btstack_notify_indicate_ready_handler: sending notification err=%d\n", err); - assert(err == ERROR_CODE_SUCCESS); - (void)err; - } else { - assert(pending_op->op_type == MP_BLUETOOTH_BTSTACK_PENDING_INDICATE); - int err = att_server_indicate(pending_op->conn_handle, pending_op->value_handle, NULL, 0); - DEBUG_printf("btstack_notify_indicate_ready_handler: sending indication err=%d\n", err); - assert(err == ERROR_CODE_SUCCESS); - (void)err; - } - // Can't free the pending op as we're in IRQ context. Leave it for the GC. - btstack_remove_pending_operation(pending_op, false /* del */); - MICROPY_PY_BLUETOOTH_EXIT -} - // Register a pending background operation -- copies the buffer, and makes it known to the GC. STATIC mp_btstack_pending_op_t *btstack_enqueue_pending_operation(uint16_t op_type, uint16_t conn_handle, uint16_t value_handle, const uint8_t *buf, size_t len) { DEBUG_printf("btstack_enqueue_pending_operation op_type=%d conn_handle=%d value_handle=%d len=%zu\n", op_type, conn_handle, value_handle, len); @@ -204,11 +174,6 @@ STATIC mp_btstack_pending_op_t *btstack_enqueue_pending_operation(uint16_t op_ty pending_op->len = len; memcpy(pending_op->buf, buf, len); - if (op_type == MP_BLUETOOTH_BTSTACK_PENDING_NOTIFY || op_type == MP_BLUETOOTH_BTSTACK_PENDING_INDICATE) { - pending_op->context_registration.callback = &btstack_notify_indicate_ready_handler; - pending_op->context_registration.context = pending_op; - } - MICROPY_PY_BLUETOOTH_ENTER bool added = btstack_linked_list_add(&MP_STATE_PORT(bluetooth_btstack_root_pointers)->pending_ops, (btstack_linked_item_t *)pending_op); assert(added); @@ -854,7 +819,7 @@ void mp_bluetooth_set_io_capability(uint8_t capability) { #endif // MICROPY_PY_BLUETOOTH_ENABLE_PAIRING_BONDING size_t mp_bluetooth_gap_get_device_name(const uint8_t **buf) { - uint8_t *value = NULL; + const uint8_t *value = NULL; size_t value_len = 0; mp_bluetooth_gatts_db_read(MP_STATE_PORT(bluetooth_btstack_root_pointers)->gatts_db, BTSTACK_GAP_DEVICE_NAME_HANDLE, &value, &value_len); *buf = value; @@ -1095,7 +1060,7 @@ int mp_bluetooth_gatts_register_service_end(void) { return 0; } -int mp_bluetooth_gatts_read(uint16_t value_handle, uint8_t **value, size_t *value_len) { +int mp_bluetooth_gatts_read(uint16_t value_handle, const uint8_t **value, size_t *value_len) { DEBUG_printf("mp_bluetooth_gatts_read\n"); if (!mp_bluetooth_is_active()) { return ERRNO_BLUETOOTH_NOT_ACTIVE; @@ -1114,85 +1079,41 @@ int mp_bluetooth_gatts_write(uint16_t value_handle, const uint8_t *value, size_t return mp_bluetooth_gatts_db_write(MP_STATE_PORT(bluetooth_btstack_root_pointers)->gatts_db, value_handle, value, value_len); } -int mp_bluetooth_gatts_notify(uint16_t conn_handle, uint16_t value_handle) { - DEBUG_printf("mp_bluetooth_gatts_notify\n"); +int mp_bluetooth_gatts_notify_indicate(uint16_t conn_handle, uint16_t value_handle, int gatts_op, const uint8_t *value, size_t value_len) { + DEBUG_printf("mp_bluetooth_gatts_notify_indicate\n"); if (!mp_bluetooth_is_active()) { return ERRNO_BLUETOOTH_NOT_ACTIVE; } - // Note: btstack doesn't appear to support sending a notification without a value, so include the stored value. - uint8_t *data = NULL; - size_t len = 0; - mp_bluetooth_gatts_db_read(MP_STATE_PORT(bluetooth_btstack_root_pointers)->gatts_db, value_handle, &data, &len); - return mp_bluetooth_gatts_notify_send(conn_handle, value_handle, data, len); -} - -int mp_bluetooth_gatts_notify_send(uint16_t conn_handle, uint16_t value_handle, const uint8_t *value, size_t value_len) { - DEBUG_printf("mp_bluetooth_gatts_notify_send\n"); - - if (!mp_bluetooth_is_active()) { - return ERRNO_BLUETOOTH_NOT_ACTIVE; + if (!value) { + // NULL value means "use DB value". + mp_bluetooth_gatts_db_read(MP_STATE_PORT(bluetooth_btstack_root_pointers)->gatts_db, value_handle, &value, &value_len); } + int err = ERROR_CODE_UNKNOWN_HCI_COMMAND; + // Attempt to send immediately. If it succeeds, btstack will copy the buffer. MICROPY_PY_BLUETOOTH_ENTER - int err = att_server_notify(conn_handle, value_handle, value, value_len); + switch (gatts_op) { + case MP_BLUETOOTH_GATTS_OP_NOTIFY: + err = att_server_notify(conn_handle, value_handle, value, value_len); + break; + case MP_BLUETOOTH_GATTS_OP_INDICATE: + // Indicate will raise ATT_EVENT_HANDLE_VALUE_INDICATION_COMPLETE when + // acknowledged (or timeout/error). + err = att_server_indicate(conn_handle, value_handle, value, value_len); + break; + } MICROPY_PY_BLUETOOTH_EXIT if (err == BTSTACK_ACL_BUFFERS_FULL) { - DEBUG_printf("mp_bluetooth_gatts_notify_send: ACL buffer full, scheduling callback\n"); - // Schedule callback, making a copy of the buffer. - mp_btstack_pending_op_t *pending_op = btstack_enqueue_pending_operation(MP_BLUETOOTH_BTSTACK_PENDING_NOTIFY, conn_handle, value_handle, value, value_len); + DEBUG_printf("mp_bluetooth_gatts_notify_indicate: ACL buffer full, scheduling callback\n"); - err = att_server_request_to_send_notification(&pending_op->context_registration, conn_handle); - - if (err != ERROR_CODE_SUCCESS) { - // Failure. Unref and free the pending operation. - btstack_remove_pending_operation(pending_op, true /* del */); - } - - return 0; - } else { - return btstack_error_to_errno(err); - } -} - -int mp_bluetooth_gatts_indicate(uint16_t conn_handle, uint16_t value_handle) { - DEBUG_printf("mp_bluetooth_gatts_indicate\n"); - - if (!mp_bluetooth_is_active()) { - return ERRNO_BLUETOOTH_NOT_ACTIVE; + // TODO: re-implement the handling for this. } - uint8_t *data = NULL; - size_t len = 0; - mp_bluetooth_gatts_db_read(MP_STATE_PORT(bluetooth_btstack_root_pointers)->gatts_db, value_handle, &data, &len); - - // Indicate will raise ATT_EVENT_HANDLE_VALUE_INDICATION_COMPLETE when - // acknowledged (or timeout/error). - - // Attempt to send immediately, will copy buffer. - MICROPY_PY_BLUETOOTH_ENTER - int err = att_server_indicate(conn_handle, value_handle, data, len); - MICROPY_PY_BLUETOOTH_EXIT - - if (err == BTSTACK_ACL_BUFFERS_FULL) { - DEBUG_printf("mp_bluetooth_gatts_indicate: ACL buffer full, scheduling callback\n"); - // Schedule callback, making a copy of the buffer. - mp_btstack_pending_op_t *pending_op = btstack_enqueue_pending_operation(MP_BLUETOOTH_BTSTACK_PENDING_INDICATE, conn_handle, value_handle, data, len); - - err = att_server_request_to_send_indication(&pending_op->context_registration, conn_handle); - - if (err != ERROR_CODE_SUCCESS) { - // Failure. Unref and free the pending operation. - btstack_remove_pending_operation(pending_op, true /* del */); - } - - return 0; - } else { - return btstack_error_to_errno(err); - } + return btstack_error_to_errno(err); } int mp_bluetooth_gatts_set_buffer(uint16_t value_handle, size_t len, bool append) { diff --git a/extmod/modbluetooth.c b/extmod/modbluetooth.c index 3a51fb8b23..e3ef40d789 100644 --- a/extmod/modbluetooth.c +++ b/extmod/modbluetooth.c @@ -733,7 +733,7 @@ STATIC MP_DEFINE_CONST_FUN_OBJ_VAR_BETWEEN(bluetooth_ble_gap_passkey_obj, 4, 4, STATIC mp_obj_t bluetooth_ble_gatts_read(mp_obj_t self_in, mp_obj_t value_handle_in) { (void)self_in; size_t len = 0; - uint8_t *buf; + const uint8_t *buf; mp_bluetooth_gatts_read(mp_obj_get_int(value_handle_in), &buf, &len); return mp_obj_new_bytes(buf, len); } @@ -751,32 +751,30 @@ STATIC mp_obj_t bluetooth_ble_gatts_write(size_t n_args, const mp_obj_t *args) { } STATIC MP_DEFINE_CONST_FUN_OBJ_VAR_BETWEEN(bluetooth_ble_gatts_write_obj, 3, 4, bluetooth_ble_gatts_write); -STATIC mp_obj_t bluetooth_ble_gatts_notify(size_t n_args, const mp_obj_t *args) { +STATIC mp_obj_t bluetooth_ble_gatts_notify_indicate(size_t n_args, const mp_obj_t *args, int gatts_op) { mp_int_t conn_handle = mp_obj_get_int(args[1]); mp_int_t value_handle = mp_obj_get_int(args[2]); + const uint8_t *value = NULL; + size_t value_len = 0; if (n_args == 4 && args[3] != mp_const_none) { mp_buffer_info_t bufinfo = {0}; mp_get_buffer_raise(args[3], &bufinfo, MP_BUFFER_READ); - int err = mp_bluetooth_gatts_notify_send(conn_handle, value_handle, bufinfo.buf, bufinfo.len); - bluetooth_handle_errno(err); - return mp_const_none; - } else { - int err = mp_bluetooth_gatts_notify(conn_handle, value_handle); - return bluetooth_handle_errno(err); + value = bufinfo.buf; + value_len = bufinfo.len; } + return bluetooth_handle_errno(mp_bluetooth_gatts_notify_indicate(conn_handle, value_handle, gatts_op, value, value_len)); +} + +STATIC mp_obj_t bluetooth_ble_gatts_notify(size_t n_args, const mp_obj_t *args) { + return bluetooth_ble_gatts_notify_indicate(n_args, args, MP_BLUETOOTH_GATTS_OP_NOTIFY); } STATIC MP_DEFINE_CONST_FUN_OBJ_VAR_BETWEEN(bluetooth_ble_gatts_notify_obj, 3, 4, bluetooth_ble_gatts_notify); -STATIC mp_obj_t bluetooth_ble_gatts_indicate(mp_obj_t self_in, mp_obj_t conn_handle_in, mp_obj_t value_handle_in) { - (void)self_in; - mp_int_t conn_handle = mp_obj_get_int(conn_handle_in); - mp_int_t value_handle = mp_obj_get_int(value_handle_in); - - int err = mp_bluetooth_gatts_indicate(conn_handle, value_handle); - return bluetooth_handle_errno(err); +STATIC mp_obj_t bluetooth_ble_gatts_indicate(size_t n_args, const mp_obj_t *args) { + return bluetooth_ble_gatts_notify_indicate(n_args, args, MP_BLUETOOTH_GATTS_OP_INDICATE); } -STATIC MP_DEFINE_CONST_FUN_OBJ_3(bluetooth_ble_gatts_indicate_obj, bluetooth_ble_gatts_indicate); +STATIC MP_DEFINE_CONST_FUN_OBJ_VAR_BETWEEN(bluetooth_ble_gatts_indicate_obj, 3, 4, bluetooth_ble_gatts_indicate); STATIC mp_obj_t bluetooth_ble_gatts_set_buffer(size_t n_args, const mp_obj_t *args) { mp_int_t value_handle = mp_obj_get_int(args[1]); @@ -1718,7 +1716,7 @@ mp_bluetooth_gatts_db_entry_t *mp_bluetooth_gatts_db_lookup(mp_gatts_db_t db, ui return MP_OBJ_TO_PTR(elem->value); } -int mp_bluetooth_gatts_db_read(mp_gatts_db_t db, uint16_t handle, uint8_t **value, size_t *value_len) { +int mp_bluetooth_gatts_db_read(mp_gatts_db_t db, uint16_t handle, const uint8_t **value, size_t *value_len) { MICROPY_PY_BLUETOOTH_ENTER mp_bluetooth_gatts_db_entry_t *entry = mp_bluetooth_gatts_db_lookup(db, handle); if (entry) { diff --git a/extmod/modbluetooth.h b/extmod/modbluetooth.h index ca6436b678..fc7012ecfb 100644 --- a/extmod/modbluetooth.h +++ b/extmod/modbluetooth.h @@ -186,6 +186,10 @@ #define MP_BLUETOOTH_PASSKEY_ACTION_DISPLAY (3) #define MP_BLUETOOTH_PASSKEY_ACTION_NUMERIC_COMPARISON (4) +// These are the ops for mp_bluetooth_gatts_notify_indicate. +#define MP_BLUETOOTH_GATTS_OP_NOTIFY (1) +#define MP_BLUETOOTH_GATTS_OP_INDICATE (2) + /* These aren't included in the module for space reasons, but can be used in your Python code if necessary. @@ -333,15 +337,11 @@ int mp_bluetooth_gatts_register_service(mp_obj_bluetooth_uuid_t *service_uuid, m int mp_bluetooth_gatts_register_service_end(void); // Read the value from the local gatts db (likely this has been written by a central). -int mp_bluetooth_gatts_read(uint16_t value_handle, uint8_t **value, size_t *value_len); +int mp_bluetooth_gatts_read(uint16_t value_handle, const uint8_t **value, size_t *value_len); // Write a value to the local gatts db (ready to be queried by a central). Optionally send notifications/indications. int mp_bluetooth_gatts_write(uint16_t value_handle, const uint8_t *value, size_t value_len, bool send_update); -// Notify the central that it should do a read. -int mp_bluetooth_gatts_notify(uint16_t conn_handle, uint16_t value_handle); -// Notify the central, including a data payload. (Note: does not set the gatts db value). -int mp_bluetooth_gatts_notify_send(uint16_t conn_handle, uint16_t value_handle, const uint8_t *value, size_t value_len); -// Indicate the central. -int mp_bluetooth_gatts_indicate(uint16_t conn_handle, uint16_t value_handle); +// Send a notification/indication to the central, optionally with custom payload (otherwise the DB value is used). +int mp_bluetooth_gatts_notify_indicate(uint16_t conn_handle, uint16_t value_handle, int gatts_op, const uint8_t *value, size_t value_len); // Resize and enable/disable append-mode on a value. // Append-mode means that remote writes will append and local reads will clear after reading. @@ -508,7 +508,7 @@ STATIC inline void mp_bluetooth_gatts_db_reset(mp_gatts_db_t db) { void mp_bluetooth_gatts_db_create_entry(mp_gatts_db_t db, uint16_t handle, size_t len); mp_bluetooth_gatts_db_entry_t *mp_bluetooth_gatts_db_lookup(mp_gatts_db_t db, uint16_t handle); -int mp_bluetooth_gatts_db_read(mp_gatts_db_t db, uint16_t handle, uint8_t **value, size_t *value_len); +int mp_bluetooth_gatts_db_read(mp_gatts_db_t db, uint16_t handle, const uint8_t **value, size_t *value_len); int mp_bluetooth_gatts_db_write(mp_gatts_db_t db, uint16_t handle, const uint8_t *value, size_t value_len); int mp_bluetooth_gatts_db_resize(mp_gatts_db_t db, uint16_t handle, size_t len, bool append); diff --git a/extmod/nimble/modbluetooth_nimble.c b/extmod/nimble/modbluetooth_nimble.c index b0194446bd..b2667300ca 100644 --- a/extmod/nimble/modbluetooth_nimble.c +++ b/extmod/nimble/modbluetooth_nimble.c @@ -1008,7 +1008,7 @@ int mp_bluetooth_gap_disconnect(uint16_t conn_handle) { return ble_hs_err_to_errno(ble_gap_terminate(conn_handle, BLE_ERR_REM_USER_CONN_TERM)); } -int mp_bluetooth_gatts_read(uint16_t value_handle, uint8_t **value, size_t *value_len) { +int mp_bluetooth_gatts_read(uint16_t value_handle, const uint8_t **value, size_t *value_len) { if (!mp_bluetooth_is_active()) { return ERRNO_BLUETOOTH_NOT_ACTIVE; } @@ -1026,35 +1026,40 @@ int mp_bluetooth_gatts_write(uint16_t value_handle, const uint8_t *value, size_t return err; } -// TODO: Could use ble_gatts_chr_updated to send to all subscribed centrals. - -int mp_bluetooth_gatts_notify(uint16_t conn_handle, uint16_t value_handle) { +int mp_bluetooth_gatts_notify_indicate(uint16_t conn_handle, uint16_t value_handle, int gatts_op, const uint8_t *value, size_t value_len) { if (!mp_bluetooth_is_active()) { return ERRNO_BLUETOOTH_NOT_ACTIVE; } - // Confusingly, notify/notify_custom/indicate are "gattc" function (even though they're used by peripherals (i.e. gatt servers)). + + int err = BLE_HS_EINVAL; + + // NULL om in the _custom methods means "use DB value" (NimBLE will call + // back into mp_bluetooth_gatts_read for us). + struct os_mbuf *om = NULL; + + if (value) { + om = ble_hs_mbuf_from_flat(value, value_len); + if (om == NULL) { + return MP_ENOMEM; + } + } + + // Note: Confusingly, Nimble's notify/notify_custom and indicate/indicate_custom + // are "gattc" functions (even though they're used by peripherals, i.e. gatt servers). // See https://www.mail-archive.com/dev@mynewt.apache.org/msg01293.html - return ble_hs_err_to_errno(ble_gattc_notify(conn_handle, value_handle)); -} -int mp_bluetooth_gatts_notify_send(uint16_t conn_handle, uint16_t value_handle, const uint8_t *value, size_t value_len) { - if (!mp_bluetooth_is_active()) { - return ERRNO_BLUETOOTH_NOT_ACTIVE; + switch (gatts_op) { + case MP_BLUETOOTH_GATTS_OP_NOTIFY: + err = ble_gattc_notify_custom(conn_handle, value_handle, om); + break; + case MP_BLUETOOTH_GATTS_OP_INDICATE: + // This will raise BLE_GAP_EVENT_NOTIFY_TX with a status when it is + // acknowledged (or timeout/error). + err = ble_gattc_indicate_custom(conn_handle, value_handle, om); + break; } - struct os_mbuf *om = ble_hs_mbuf_from_flat(value, value_len); - if (om == NULL) { - return MP_ENOMEM; - } - return ble_hs_err_to_errno(ble_gattc_notify_custom(conn_handle, value_handle, om)); -} -int mp_bluetooth_gatts_indicate(uint16_t conn_handle, uint16_t value_handle) { - if (!mp_bluetooth_is_active()) { - return ERRNO_BLUETOOTH_NOT_ACTIVE; - } - // This will raise BLE_GAP_EVENT_NOTIFY_TX with a status when it is - // acknowledged (or timeout/error). - return ble_hs_err_to_errno(ble_gattc_indicate(conn_handle, value_handle)); + return ble_hs_err_to_errno(err); } int mp_bluetooth_gatts_set_buffer(uint16_t value_handle, size_t len, bool append) { diff --git a/ports/zephyr/modbluetooth_zephyr.c b/ports/zephyr/modbluetooth_zephyr.c index 4d4b19a1d9..279e4ca9a0 100644 --- a/ports/zephyr/modbluetooth_zephyr.c +++ b/ports/zephyr/modbluetooth_zephyr.c @@ -301,7 +301,7 @@ int mp_bluetooth_gap_disconnect(uint16_t conn_handle) { return MP_EOPNOTSUPP; } -int mp_bluetooth_gatts_read(uint16_t value_handle, uint8_t **value, size_t *value_len) { +int mp_bluetooth_gatts_read(uint16_t value_handle, const uint8_t **value, size_t *value_len) { if (!mp_bluetooth_is_active()) { return ERRNO_BLUETOOTH_NOT_ACTIVE; } @@ -318,21 +318,7 @@ int mp_bluetooth_gatts_write(uint16_t value_handle, const uint8_t *value, size_t return mp_bluetooth_gatts_db_write(MP_STATE_PORT(bluetooth_zephyr_root_pointers)->gatts_db, value_handle, value, value_len); } -int mp_bluetooth_gatts_notify(uint16_t conn_handle, uint16_t value_handle) { - if (!mp_bluetooth_is_active()) { - return ERRNO_BLUETOOTH_NOT_ACTIVE; - } - return MP_EOPNOTSUPP; -} - -int mp_bluetooth_gatts_notify_send(uint16_t conn_handle, uint16_t value_handle, const uint8_t *value, size_t value_len) { - if (!mp_bluetooth_is_active()) { - return ERRNO_BLUETOOTH_NOT_ACTIVE; - } - return MP_EOPNOTSUPP; -} - -int mp_bluetooth_gatts_indicate(uint16_t conn_handle, uint16_t value_handle) { +int mp_bluetooth_gatts_notify_indicate(uint16_t conn_handle, uint16_t value_handle, int gatts_op, const uint8_t *value, size_t value_len) { if (!mp_bluetooth_is_active()) { return ERRNO_BLUETOOTH_NOT_ACTIVE; } From 256f47e2f8348d08b53e3c69461cf07903b00367 Mon Sep 17 00:00:00 2001 From: Jim Mussared Date: Thu, 2 Mar 2023 17:32:13 +1100 Subject: [PATCH 005/701] extmod/btstack: Fix indicate/notify queuing. This adds a mechanism to track a pending notify/indicate operation that is deferred due to the send buffer being full. This uses a tracked alloc that is passed as the content arg to the callback. This replaces the previous mechanism that did this via the global pending op queue, shared with client read/write ops. Signed-off-by: Jim Mussared --- extmod/btstack/modbluetooth_btstack.c | 69 +++++++++++++++++-- ports/rp2/mpconfigport.h | 2 +- ports/stm32/mpconfigport.h | 2 +- ports/unix/mpconfigport.h | 3 + .../unix/variants/coverage/mpconfigvariant.h | 2 +- py/gc.c | 4 +- 6 files changed, 73 insertions(+), 9 deletions(-) diff --git a/extmod/btstack/modbluetooth_btstack.c b/extmod/btstack/modbluetooth_btstack.c index 243b6e38ca..8ce2db74ec 100644 --- a/extmod/btstack/modbluetooth_btstack.c +++ b/extmod/btstack/modbluetooth_btstack.c @@ -166,7 +166,7 @@ STATIC void btstack_remove_pending_operation(mp_btstack_pending_op_t *pending_op // Register a pending background operation -- copies the buffer, and makes it known to the GC. STATIC mp_btstack_pending_op_t *btstack_enqueue_pending_operation(uint16_t op_type, uint16_t conn_handle, uint16_t value_handle, const uint8_t *buf, size_t len) { - DEBUG_printf("btstack_enqueue_pending_operation op_type=%d conn_handle=%d value_handle=%d len=%zu\n", op_type, conn_handle, value_handle, len); + DEBUG_printf("btstack_enqueue_pending_operation op_type=%d conn_handle=%d value_handle=%d len=%lu\n", op_type, conn_handle, value_handle, len); mp_btstack_pending_op_t *pending_op = m_new_obj_var(mp_btstack_pending_op_t, uint8_t, len); pending_op->op_type = op_type; pending_op->conn_handle = conn_handle; @@ -1079,8 +1079,44 @@ int mp_bluetooth_gatts_write(uint16_t value_handle, const uint8_t *value, size_t return mp_bluetooth_gatts_db_write(MP_STATE_PORT(bluetooth_btstack_root_pointers)->gatts_db, value_handle, value, value_len); } +#if !MICROPY_TRACKED_ALLOC +#error "btstack requires MICROPY_TRACKED_ALLOC" +#endif + +typedef struct { + btstack_context_callback_registration_t btstack_registration; + int gatts_op; + uint16_t conn_handle; + uint16_t value_handle; + size_t value_len; + uint8_t value[]; +} notify_indicate_pending_op_t; + +// Called in response to a gatts_notify/indicate being unable to complete, which then calls +// att_server_request_to_send_notification. +STATIC void btstack_notify_indicate_ready_handler(void *context) { + MICROPY_PY_BLUETOOTH_ENTER + notify_indicate_pending_op_t *pending_op = (notify_indicate_pending_op_t *)context; + DEBUG_printf("btstack_notify_indicate_ready_handler gatts_op=%d conn_handle=%d value_handle=%d len=%lu\n", pending_op->gatts_op, pending_op->conn_handle, pending_op->value_handle, pending_op->value_len); + int err = ERROR_CODE_SUCCESS; + switch (pending_op->gatts_op) { + case MP_BLUETOOTH_GATTS_OP_NOTIFY: + err = att_server_notify(pending_op->conn_handle, pending_op->value_handle, pending_op->value, pending_op->value_len); + DEBUG_printf("btstack_notify_indicate_ready_handler: sending notification err=%d\n", err); + break; + case MP_BLUETOOTH_GATTS_OP_INDICATE: + err = att_server_indicate(pending_op->conn_handle, pending_op->value_handle, pending_op->value, pending_op->value_len); + DEBUG_printf("btstack_notify_indicate_ready_handler: sending indication err=%d\n", err); + break; + } + assert(err == ERROR_CODE_SUCCESS); + (void)err; + MICROPY_PY_BLUETOOTH_EXIT + m_tracked_free(pending_op); +} + int mp_bluetooth_gatts_notify_indicate(uint16_t conn_handle, uint16_t value_handle, int gatts_op, const uint8_t *value, size_t value_len) { - DEBUG_printf("mp_bluetooth_gatts_notify_indicate\n"); + DEBUG_printf("mp_bluetooth_gatts_notify_indicate: gatts_op=%d\n", gatts_op); if (!mp_bluetooth_is_active()) { return ERRNO_BLUETOOTH_NOT_ACTIVE; @@ -1107,10 +1143,33 @@ int mp_bluetooth_gatts_notify_indicate(uint16_t conn_handle, uint16_t value_hand } MICROPY_PY_BLUETOOTH_EXIT - if (err == BTSTACK_ACL_BUFFERS_FULL) { - DEBUG_printf("mp_bluetooth_gatts_notify_indicate: ACL buffer full, scheduling callback\n"); + if (err == BTSTACK_ACL_BUFFERS_FULL || err == ATT_HANDLE_VALUE_INDICATION_IN_PROGRESS) { + DEBUG_printf("mp_bluetooth_gatts_notify_indicate: ACL buffer full / indication in progress, scheduling callback\n"); - // TODO: re-implement the handling for this. + // Copy the value and ask btstack to let us know when it can be sent. + notify_indicate_pending_op_t *pending_op = m_tracked_calloc(1, sizeof(notify_indicate_pending_op_t) + value_len); + pending_op->btstack_registration.context = pending_op; + pending_op->btstack_registration.callback = &btstack_notify_indicate_ready_handler; + pending_op->gatts_op = gatts_op; + pending_op->conn_handle = conn_handle; + pending_op->value_handle = value_handle; + pending_op->value_len = value_len; + memcpy(pending_op->value, value, value_len); + + MICROPY_PY_BLUETOOTH_ENTER + switch (gatts_op) { + case MP_BLUETOOTH_GATTS_OP_NOTIFY: + err = att_server_request_to_send_notification(&pending_op->btstack_registration, conn_handle); + break; + case MP_BLUETOOTH_GATTS_OP_INDICATE: + err = att_server_request_to_send_indication(&pending_op->btstack_registration, conn_handle); + break; + } + MICROPY_PY_BLUETOOTH_EXIT + + if (err != ERROR_CODE_SUCCESS) { + m_tracked_free(pending_op); + } } return btstack_error_to_errno(err); diff --git a/ports/rp2/mpconfigport.h b/ports/rp2/mpconfigport.h index d862812f9b..da4548a473 100644 --- a/ports/rp2/mpconfigport.h +++ b/ports/rp2/mpconfigport.h @@ -74,7 +74,7 @@ #define MICROPY_OPT_COMPUTED_GOTO (1) // Python internal features -#define MICROPY_TRACKED_ALLOC (MICROPY_SSL_MBEDTLS) +#define MICROPY_TRACKED_ALLOC (MICROPY_SSL_MBEDTLS || MICROPY_BLUETOOTH_BTSTACK) #define MICROPY_READER_VFS (1) #define MICROPY_ENABLE_GC (1) #define MICROPY_ENABLE_EMERGENCY_EXCEPTION_BUF (1) diff --git a/ports/stm32/mpconfigport.h b/ports/stm32/mpconfigport.h index ee12ab8140..e3283c8f57 100644 --- a/ports/stm32/mpconfigport.h +++ b/ports/stm32/mpconfigport.h @@ -65,7 +65,7 @@ #endif // Python internal features -#define MICROPY_TRACKED_ALLOC (MICROPY_SSL_MBEDTLS) +#define MICROPY_TRACKED_ALLOC (MICROPY_SSL_MBEDTLS || MICROPY_BLUETOOTH_BTSTACK) #define MICROPY_READER_VFS (1) #define MICROPY_ENABLE_GC (1) #define MICROPY_ENABLE_EMERGENCY_EXCEPTION_BUF (1) diff --git a/ports/unix/mpconfigport.h b/ports/unix/mpconfigport.h index a0b9192bfc..8eafb6119b 100644 --- a/ports/unix/mpconfigport.h +++ b/ports/unix/mpconfigport.h @@ -117,6 +117,9 @@ typedef long mp_off_t; #define MICROPY_HELPER_LEXER_UNIX (1) #define MICROPY_VFS_POSIX (1) #define MICROPY_READER_POSIX (1) +#ifndef MICROPY_TRACKED_ALLOC +#define MICROPY_TRACKED_ALLOC (MICROPY_BLUETOOTH_BTSTACK) +#endif // VFS stat functions should return time values relative to 1970/1/1 #define MICROPY_EPOCH_IS_1970 (1) diff --git a/ports/unix/variants/coverage/mpconfigvariant.h b/ports/unix/variants/coverage/mpconfigvariant.h index 6107a4a556..2a48e57e7f 100644 --- a/ports/unix/variants/coverage/mpconfigvariant.h +++ b/ports/unix/variants/coverage/mpconfigvariant.h @@ -39,6 +39,6 @@ // Enable additional features. #define MICROPY_DEBUG_PARSE_RULE_NAME (1) -#define MICROPY_TRACKED_ALLOC (1) +#define MICROPY_TRACKED_ALLOC (1) #define MICROPY_WARNINGS_CATEGORY (1) #define MICROPY_PY_UCRYPTOLIB_CTR (1) diff --git a/py/gc.c b/py/gc.c index ba5c569d50..ad3e110407 100644 --- a/py/gc.c +++ b/py/gc.c @@ -746,7 +746,9 @@ void *gc_alloc_with_finaliser(mp_uint_t n_bytes) { // TODO: freeing here does not call finaliser void gc_free(void *ptr) { if (MP_STATE_THREAD(gc_lock_depth) > 0) { - // TODO how to deal with this error? + // Cannot free while the GC is locked. However free is an optimisation + // to reclaim the memory immediately, this means it will now be left + // until the next collection. return; } From a6aa7397d8e5b309e5675612143d3c5a5e931333 Mon Sep 17 00:00:00 2001 From: Jim Mussared Date: Fri, 3 Mar 2023 00:48:33 +1100 Subject: [PATCH 006/701] extmod/btstack: Include value handle in client read/write events. This replaces the previous pending operation queue (that used to also be shared with pending server notify/indicate ops) with a single pending operation per connection. This allows the value handle to be correctly passed to the Python-level events. Also re-structure GATT client event handling to simplify the packet handler functions. Signed-off-by: Jim Mussared --- docs/library/bluetooth.rst | 2 - extmod/btstack/modbluetooth_btstack.c | 404 ++++++++++++++------------ extmod/btstack/modbluetooth_btstack.h | 9 +- extmod/modbluetooth.c | 3 +- extmod/modbluetooth.h | 2 +- extmod/nimble/modbluetooth_nimble.c | 6 +- 6 files changed, 222 insertions(+), 204 deletions(-) diff --git a/docs/library/bluetooth.rst b/docs/library/bluetooth.rst index 63578af16e..dd0f5ffd62 100644 --- a/docs/library/bluetooth.rst +++ b/docs/library/bluetooth.rst @@ -183,12 +183,10 @@ Event Handling conn_handle, value_handle, char_data = data elif event == _IRQ_GATTC_READ_DONE: # A gattc_read() has completed. - # Note: The value_handle will be zero on btstack (but present on NimBLE). # Note: Status will be zero on success, implementation-specific value otherwise. conn_handle, value_handle, status = data elif event == _IRQ_GATTC_WRITE_DONE: # A gattc_write() has completed. - # Note: The value_handle will be zero on btstack (but present on NimBLE). # Note: Status will be zero on success, implementation-specific value otherwise. conn_handle, value_handle, status = data elif event == _IRQ_GATTC_NOTIFY: diff --git a/extmod/btstack/modbluetooth_btstack.c b/extmod/btstack/modbluetooth_btstack.c index 8ce2db74ec..1c43589252 100644 --- a/extmod/btstack/modbluetooth_btstack.c +++ b/extmod/btstack/modbluetooth_btstack.c @@ -93,121 +93,57 @@ STATIC mp_obj_bluetooth_uuid_t create_mp_uuid(uint16_t uuid16, const uint8_t *uu } #endif // MICROPY_PY_BLUETOOTH_ENABLE_CENTRAL_MODE -// Notes on supporting background ops (e.g. an attempt to gatts_notify while -// an existing notification is in progress): - -// GATTS Notify/Indicate (att_server_notify/indicate) -// * When available, copies buffer immediately. -// * Otherwise fails with BTSTACK_ACL_BUFFERS_FULL -// * Use att_server_request_to_send_notification/indication to get callback -// * Takes btstack_context_callback_registration_t (and takes ownership) and conn_handle. -// * Callback is invoked with just the context member of the btstack_context_callback_registration_t - -// GATTC Write without response (gatt_client_write_value_of_characteristic_without_response) -// * When available, copies buffer immediately. -// * Otherwise, fails with GATT_CLIENT_BUSY. -// * Use gatt_client_request_can_write_without_response_event to get callback -// * Takes btstack_packet_handler_t (function pointer) and conn_handle -// * Callback is invoked, use gatt_event_can_write_without_response_get_handle to get the conn_handle (no other context) -// * There can only be one pending gatt_client_request_can_write_without_response_event (otherwise we fail with EALREADY). - -// GATTC Write with response (gatt_client_write_value_of_characteristic) -// * When peripheral is available, takes ownership of buffer. -// * Otherwise, fails with GATT_CLIENT_IN_WRONG_STATE (we fail the operation). -// * Raises GATT_EVENT_QUERY_COMPLETE to the supplied packet handler. - -// For notify/indicate/write-without-response that proceed immediately, nothing extra required. -// For all other cases, buffer needs to be copied and protected from GC. -// For notify/indicate: -// * btstack_context_callback_registration_t: -// * needs to be malloc'ed -// * needs to be protected from GC -// * context arg needs to point back to the callback registration so it can be freed and un-protected -// For write-without-response -// * only the conn_handle is available in the callback -// * so we need a queue of conn_handle->(value_handle, copied buffer) - -// Pending operation types. -enum { - // Queued for sending when possible. - MP_BLUETOOTH_BTSTACK_PENDING_WRITE_NO_RESPONSE, // Waiting for conn handle - // Hold buffer pointer until complete. - MP_BLUETOOTH_BTSTACK_PENDING_WRITE, // Waiting for write done event -}; - -// Pending operation: -// - Holds a GC reference to the copied outgoing buffer. -// - Provides enough information for the callback handler to execute the desired operation. -struct _mp_btstack_pending_op_t { +#if MICROPY_PY_BLUETOOTH_ENABLE_GATT_CLIENT +typedef struct _mp_btstack_active_connection_t { btstack_linked_item_t *next; // Must be first field to match btstack_linked_item. - // See enum above. - uint16_t op_type; - - // For all op types. uint16_t conn_handle; - uint16_t value_handle; - // For write-without-response, this is the actual buffer to send. - // For write-with-response, just holding onto the buffer for GC ref. - size_t len; - uint8_t buf[]; -}; + // Read/write. + uint16_t pending_value_handle; -// Must hold MICROPY_PY_BLUETOOTH_ENTER. -STATIC void btstack_remove_pending_operation(mp_btstack_pending_op_t *pending_op, bool del) { - bool removed = btstack_linked_list_remove(&MP_STATE_PORT(bluetooth_btstack_root_pointers)->pending_ops, (btstack_linked_item_t *)pending_op); - assert(removed); - (void)removed; - if (del) { - m_del_var(mp_btstack_pending_op_t, uint8_t, pending_op->len, pending_op); - } -} + // Write only. Buffer must be retained until the operation completes. + uint8_t *pending_write_value; + size_t pending_write_value_len; +} mp_btstack_active_connection_t; -// Register a pending background operation -- copies the buffer, and makes it known to the GC. -STATIC mp_btstack_pending_op_t *btstack_enqueue_pending_operation(uint16_t op_type, uint16_t conn_handle, uint16_t value_handle, const uint8_t *buf, size_t len) { - DEBUG_printf("btstack_enqueue_pending_operation op_type=%d conn_handle=%d value_handle=%d len=%lu\n", op_type, conn_handle, value_handle, len); - mp_btstack_pending_op_t *pending_op = m_new_obj_var(mp_btstack_pending_op_t, uint8_t, len); - pending_op->op_type = op_type; - pending_op->conn_handle = conn_handle; - pending_op->value_handle = value_handle; - pending_op->len = len; - memcpy(pending_op->buf, buf, len); - - MICROPY_PY_BLUETOOTH_ENTER - bool added = btstack_linked_list_add(&MP_STATE_PORT(bluetooth_btstack_root_pointers)->pending_ops, (btstack_linked_item_t *)pending_op); - assert(added); +STATIC mp_btstack_active_connection_t *create_active_connection(uint16_t conn_handle) { + DEBUG_printf("create_active_connection: conn_handle=%d\n", conn_handle); + mp_btstack_active_connection_t *conn = m_new(mp_btstack_active_connection_t, 1); + conn->conn_handle = conn_handle; + conn->pending_value_handle = 0xffff; + conn->pending_write_value = NULL; + conn->pending_write_value_len = 0; + bool added = btstack_linked_list_add(&MP_STATE_PORT(bluetooth_btstack_root_pointers)->active_connections, (btstack_linked_item_t *)conn); (void)added; - MICROPY_PY_BLUETOOTH_EXIT - - return pending_op; + assert(added); + return conn; } -#if MICROPY_PY_BLUETOOTH_ENABLE_GATT_CLIENT - -// Cleans up a pending op of the specified type for this conn_handle (and if specified, value_handle). -// Used by MP_BLUETOOTH_BTSTACK_PENDING_WRITE and MP_BLUETOOTH_BTSTACK_PENDING_WRITE_NO_RESPONSE. -// At the moment, both will set value_handle=0xffff as the events do not know their value_handle. -// TODO: Can we make btstack give us the value_handle for regular write (with response) so that we -// know for sure that we're using the correct entry. -STATIC mp_btstack_pending_op_t *btstack_finish_pending_operation(uint16_t op_type, uint16_t conn_handle, uint16_t value_handle, bool del) { - MICROPY_PY_BLUETOOTH_ENTER - DEBUG_printf("btstack_finish_pending_operation op_type=%d conn_handle=%d value_handle=%d\n", op_type, conn_handle, value_handle); +STATIC mp_btstack_active_connection_t *find_active_connection(uint16_t conn_handle) { + DEBUG_printf("find_active_connection: conn_handle=%d\n", conn_handle); btstack_linked_list_iterator_t it; - btstack_linked_list_iterator_init(&it, &MP_STATE_PORT(bluetooth_btstack_root_pointers)->pending_ops); + btstack_linked_list_iterator_init(&it, &MP_STATE_PORT(bluetooth_btstack_root_pointers)->active_connections); + mp_btstack_active_connection_t *conn = NULL; while (btstack_linked_list_iterator_has_next(&it)) { - mp_btstack_pending_op_t *pending_op = (mp_btstack_pending_op_t *)btstack_linked_list_iterator_next(&it); - - if (pending_op->op_type == op_type && pending_op->conn_handle == conn_handle && (value_handle == 0xffff || pending_op->value_handle == value_handle)) { - DEBUG_printf("btstack_finish_pending_operation: found value_handle=%d len=%zu\n", pending_op->value_handle, pending_op->len); - btstack_remove_pending_operation(pending_op, del); - MICROPY_PY_BLUETOOTH_EXIT - return del ? NULL : pending_op; + conn = (mp_btstack_active_connection_t *)btstack_linked_list_iterator_next(&it); + DEBUG_printf(" --> iter conn %d\n", conn->conn_handle); + if (conn->conn_handle == conn_handle) { + break; } } - DEBUG_printf("btstack_finish_pending_operation: not found\n"); - MICROPY_PY_BLUETOOTH_EXIT - return NULL; + return conn; +} + +STATIC void remove_active_connection(uint16_t conn_handle) { + DEBUG_printf("remove_active_connection: conn_handle=%d\n", conn_handle); + mp_btstack_active_connection_t *conn = find_active_connection(conn_handle); + if (conn) { + bool removed = btstack_linked_list_remove(&MP_STATE_PORT(bluetooth_btstack_root_pointers)->active_connections, (btstack_linked_item_t *)conn); + (void)removed; + assert(removed); + m_del(mp_btstack_active_connection_t, conn, 1); + } } #endif @@ -255,8 +191,10 @@ STATIC bool controller_static_addr_available = false; STATIC const uint8_t read_static_address_command_complete_prefix[] = { 0x0e, 0x1b, 0x01, 0x09, 0xfc }; #endif -STATIC void btstack_packet_handler(uint8_t packet_type, uint8_t *packet, uint8_t irq) { - DEBUG_printf("btstack_packet_handler(packet_type=%u, packet=%p)\n", packet_type, packet); +STATIC void btstack_packet_handler_generic(uint8_t packet_type, uint16_t channel, uint8_t *packet, uint16_t size) { + (void)channel; + (void)size; + DEBUG_printf("btstack_packet_handler_generic(packet_type=%u, packet=%p)\n", packet_type, packet); if (packet_type != HCI_EVENT_PACKET) { return; } @@ -279,6 +217,9 @@ STATIC void btstack_packet_handler(uint8_t packet_type, uint8_t *packet, uint8_t // Slave role. irq_event = MP_BLUETOOTH_IRQ_CENTRAL_CONNECT; } + #if MICROPY_PY_BLUETOOTH_ENABLE_GATT_CLIENT + create_active_connection(conn_handle); + #endif mp_bluetooth_gap_on_connected_disconnected(irq_event, conn_handle, addr_type, addr); break; } @@ -372,6 +313,9 @@ STATIC void btstack_packet_handler(uint8_t packet_type, uint8_t *packet, uint8_t } uint8_t addr[6] = {0}; mp_bluetooth_gap_on_connected_disconnected(irq_event, conn_handle, 0xff, addr); + #if MICROPY_PY_BLUETOOTH_ENABLE_GATT_CLIENT + remove_active_connection(conn_handle); + #endif #if MICROPY_PY_BLUETOOTH_ENABLE_CENTRAL_MODE } else if (event_type == GAP_EVENT_ADVERTISING_REPORT) { DEBUG_printf(" --> gap advertising report\n"); @@ -390,51 +334,6 @@ STATIC void btstack_packet_handler(uint8_t packet_type, uint8_t *packet, uint8_t uint16_t conn_handle = gatt_event_mtu_get_handle(packet); uint16_t mtu = gatt_event_mtu_get_MTU(packet); mp_bluetooth_gatts_on_mtu_exchanged(conn_handle, mtu); - } else if (event_type == GATT_EVENT_QUERY_COMPLETE) { - uint16_t conn_handle = gatt_event_query_complete_get_handle(packet); - uint16_t status = gatt_event_query_complete_get_att_status(packet); - DEBUG_printf(" --> gatt query complete irq=%d conn_handle=%d status=%d\n", irq, conn_handle, status); - if (irq == MP_BLUETOOTH_IRQ_GATTC_READ_DONE || irq == MP_BLUETOOTH_IRQ_GATTC_WRITE_DONE) { - // TODO there is no value_handle available to pass here. - // TODO try and get this implemented in btstack. - mp_bluetooth_gattc_on_read_write_status(irq, conn_handle, 0xffff, status); - // Unref the saved buffer for the write operation on this conn_handle. - if (irq == MP_BLUETOOTH_IRQ_GATTC_WRITE_DONE) { - btstack_finish_pending_operation(MP_BLUETOOTH_BTSTACK_PENDING_WRITE, conn_handle, 0xffff, false /* del */); - } - } else if (irq == MP_BLUETOOTH_IRQ_GATTC_SERVICE_DONE || - irq == MP_BLUETOOTH_IRQ_GATTC_CHARACTERISTIC_DONE || - irq == MP_BLUETOOTH_IRQ_GATTC_DESCRIPTOR_DONE) { - mp_bluetooth_gattc_on_discover_complete(irq, conn_handle, status); - } - } else if (event_type == GATT_EVENT_SERVICE_QUERY_RESULT) { - DEBUG_printf(" --> gatt service query result\n"); - uint16_t conn_handle = gatt_event_service_query_result_get_handle(packet); - gatt_client_service_t service; - gatt_event_service_query_result_get_service(packet, &service); - mp_obj_bluetooth_uuid_t service_uuid = create_mp_uuid(service.uuid16, service.uuid128); - mp_bluetooth_gattc_on_primary_service_result(conn_handle, service.start_group_handle, service.end_group_handle, &service_uuid); - } else if (event_type == GATT_EVENT_CHARACTERISTIC_QUERY_RESULT) { - DEBUG_printf(" --> gatt characteristic query result\n"); - uint16_t conn_handle = gatt_event_characteristic_query_result_get_handle(packet); - gatt_client_characteristic_t characteristic; - gatt_event_characteristic_query_result_get_characteristic(packet, &characteristic); - mp_obj_bluetooth_uuid_t characteristic_uuid = create_mp_uuid(characteristic.uuid16, characteristic.uuid128); - mp_bluetooth_gattc_on_characteristic_result(conn_handle, characteristic.value_handle, characteristic.end_handle, characteristic.properties, &characteristic_uuid); - } else if (event_type == GATT_EVENT_ALL_CHARACTERISTIC_DESCRIPTORS_QUERY_RESULT) { - DEBUG_printf(" --> gatt descriptor query result\n"); - uint16_t conn_handle = gatt_event_all_characteristic_descriptors_query_result_get_handle(packet); - gatt_client_characteristic_descriptor_t descriptor; - gatt_event_all_characteristic_descriptors_query_result_get_characteristic_descriptor(packet, &descriptor); - mp_obj_bluetooth_uuid_t descriptor_uuid = create_mp_uuid(descriptor.uuid16, descriptor.uuid128); - mp_bluetooth_gattc_on_descriptor_result(conn_handle, descriptor.handle, &descriptor_uuid); - } else if (event_type == GATT_EVENT_CHARACTERISTIC_VALUE_QUERY_RESULT) { - DEBUG_printf(" --> gatt characteristic value query result\n"); - uint16_t conn_handle = gatt_event_characteristic_value_query_result_get_handle(packet); - uint16_t value_handle = gatt_event_characteristic_value_query_result_get_value_handle(packet); - uint16_t len = gatt_event_characteristic_value_query_result_get_value_length(packet); - const uint8_t *data = gatt_event_characteristic_value_query_result_get_value(packet); - mp_bluetooth_gattc_on_data_available(MP_BLUETOOTH_IRQ_GATTC_READ_RESULT, conn_handle, value_handle, &data, &len, 1); } else if (event_type == GATT_EVENT_NOTIFICATION) { DEBUG_printf(" --> gatt notification\n"); uint16_t conn_handle = gatt_event_notification_get_handle(packet); @@ -452,28 +351,24 @@ STATIC void btstack_packet_handler(uint8_t packet_type, uint8_t *packet, uint8_t } else if (event_type == GATT_EVENT_CAN_WRITE_WITHOUT_RESPONSE) { uint16_t conn_handle = gatt_event_can_write_without_response_get_handle(packet); DEBUG_printf(" --> gatt can write without response %d\n", conn_handle); - mp_btstack_pending_op_t *pending_op = btstack_finish_pending_operation(MP_BLUETOOTH_BTSTACK_PENDING_WRITE_NO_RESPONSE, conn_handle, 0xffff, false /* !del */); - if (pending_op) { - DEBUG_printf(" --> ready for value_handle=%d len=%zu\n", pending_op->value_handle, pending_op->len); - gatt_client_write_value_of_characteristic_without_response(pending_op->conn_handle, pending_op->value_handle, pending_op->len, (uint8_t *)pending_op->buf); - // Note: Can't "del" the pending_op from IRQ context. Leave it for the GC. + mp_btstack_active_connection_t *conn = find_active_connection(conn_handle); + if (!conn || conn->pending_value_handle == 0xffff || !conn->pending_write_value) { + return; } - + DEBUG_printf(" --> ready for value_handle=%d len=%lu\n", conn->pending_value_handle, conn->pending_write_value_len); + int err = gatt_client_write_value_of_characteristic_without_response(conn_handle, conn->pending_value_handle, conn->pending_write_value_len, conn->pending_write_value); + (void)err; + assert(err == ERROR_CODE_SUCCESS); + conn->pending_value_handle = 0xffff; + m_del(uint8_t, conn->pending_write_value, conn->pending_write_value_len); + conn->pending_write_value = NULL; + conn->pending_write_value_len = 0; #endif // MICROPY_PY_BLUETOOTH_ENABLE_GATT_CLIENT } else { DEBUG_printf(" --> hci event type: unknown (0x%02x)\n", event_type); } } -// Because the packet handler callbacks don't support an argument, we use a specific -// handler when we need to provide additional state to the handler (in the "irq" parameter). -// This is the generic handler for when you don't need extra state. -STATIC void btstack_packet_handler_generic(uint8_t packet_type, uint16_t channel, uint8_t *packet, uint16_t size) { - (void)channel; - (void)size; - btstack_packet_handler(packet_type, packet, 0); -} - STATIC btstack_packet_callback_registration_t hci_event_callback_registration = { .callback = &btstack_packet_handler_generic }; @@ -483,35 +378,121 @@ STATIC btstack_packet_callback_registration_t hci_event_callback_registration = STATIC void btstack_packet_handler_discover_services(uint8_t packet_type, uint16_t channel, uint8_t *packet, uint16_t size) { (void)channel; (void)size; - btstack_packet_handler(packet_type, packet, MP_BLUETOOTH_IRQ_GATTC_SERVICE_DONE); + if (packet_type != HCI_EVENT_PACKET) { + return; + } + uint8_t event_type = hci_event_packet_get_type(packet); + if (event_type == GATT_EVENT_SERVICE_QUERY_RESULT) { + DEBUG_printf(" --> gatt service query result\n"); + uint16_t conn_handle = gatt_event_service_query_result_get_handle(packet); + gatt_client_service_t service; + gatt_event_service_query_result_get_service(packet, &service); + mp_obj_bluetooth_uuid_t service_uuid = create_mp_uuid(service.uuid16, service.uuid128); + mp_bluetooth_gattc_on_primary_service_result(conn_handle, service.start_group_handle, service.end_group_handle, &service_uuid); + } else if (event_type == GATT_EVENT_QUERY_COMPLETE) { + uint16_t conn_handle = gatt_event_query_complete_get_handle(packet); + uint16_t status = gatt_event_query_complete_get_att_status(packet); + DEBUG_printf(" --> gatt query services complete conn_handle=%d status=%d\n", conn_handle, status); + mp_bluetooth_gattc_on_discover_complete(MP_BLUETOOTH_IRQ_GATTC_SERVICE_DONE, conn_handle, status); + } } // For when the handler is being used for characteristic discovery. STATIC void btstack_packet_handler_discover_characteristics(uint8_t packet_type, uint16_t channel, uint8_t *packet, uint16_t size) { (void)channel; (void)size; - btstack_packet_handler(packet_type, packet, MP_BLUETOOTH_IRQ_GATTC_CHARACTERISTIC_DONE); + if (packet_type != HCI_EVENT_PACKET) { + return; + } + uint8_t event_type = hci_event_packet_get_type(packet); + if (event_type == GATT_EVENT_CHARACTERISTIC_QUERY_RESULT) { + DEBUG_printf(" --> gatt characteristic query result\n"); + uint16_t conn_handle = gatt_event_characteristic_query_result_get_handle(packet); + gatt_client_characteristic_t characteristic; + gatt_event_characteristic_query_result_get_characteristic(packet, &characteristic); + mp_obj_bluetooth_uuid_t characteristic_uuid = create_mp_uuid(characteristic.uuid16, characteristic.uuid128); + mp_bluetooth_gattc_on_characteristic_result(conn_handle, characteristic.value_handle, characteristic.end_handle, characteristic.properties, &characteristic_uuid); + } else if (event_type == GATT_EVENT_QUERY_COMPLETE) { + uint16_t conn_handle = gatt_event_query_complete_get_handle(packet); + uint16_t status = gatt_event_query_complete_get_att_status(packet); + DEBUG_printf(" --> gatt query characteristics complete conn_handle=%d status=%d\n", conn_handle, status); + mp_bluetooth_gattc_on_discover_complete(MP_BLUETOOTH_IRQ_GATTC_CHARACTERISTIC_DONE, conn_handle, status); + } } // For when the handler is being used for descriptor discovery. STATIC void btstack_packet_handler_discover_descriptors(uint8_t packet_type, uint16_t channel, uint8_t *packet, uint16_t size) { (void)channel; (void)size; - btstack_packet_handler(packet_type, packet, MP_BLUETOOTH_IRQ_GATTC_DESCRIPTOR_DONE); + if (packet_type != HCI_EVENT_PACKET) { + return; + } + uint8_t event_type = hci_event_packet_get_type(packet); + if (event_type == GATT_EVENT_ALL_CHARACTERISTIC_DESCRIPTORS_QUERY_RESULT) { + DEBUG_printf(" --> gatt descriptor query result\n"); + uint16_t conn_handle = gatt_event_all_characteristic_descriptors_query_result_get_handle(packet); + gatt_client_characteristic_descriptor_t descriptor; + gatt_event_all_characteristic_descriptors_query_result_get_characteristic_descriptor(packet, &descriptor); + mp_obj_bluetooth_uuid_t descriptor_uuid = create_mp_uuid(descriptor.uuid16, descriptor.uuid128); + mp_bluetooth_gattc_on_descriptor_result(conn_handle, descriptor.handle, &descriptor_uuid); + } else if (event_type == GATT_EVENT_QUERY_COMPLETE) { + uint16_t conn_handle = gatt_event_query_complete_get_handle(packet); + uint16_t status = gatt_event_query_complete_get_att_status(packet); + DEBUG_printf(" --> gatt query descriptors complete conn_handle=%d status=%d\n", conn_handle, status); + mp_bluetooth_gattc_on_discover_complete(MP_BLUETOOTH_IRQ_GATTC_DESCRIPTOR_DONE, conn_handle, status); + } } // For when the handler is being used for a read query. STATIC void btstack_packet_handler_read(uint8_t packet_type, uint16_t channel, uint8_t *packet, uint16_t size) { (void)channel; (void)size; - btstack_packet_handler(packet_type, packet, MP_BLUETOOTH_IRQ_GATTC_READ_DONE); + if (packet_type != HCI_EVENT_PACKET) { + return; + } + uint8_t event_type = hci_event_packet_get_type(packet); + if (event_type == GATT_EVENT_QUERY_COMPLETE) { + uint16_t conn_handle = gatt_event_query_complete_get_handle(packet); + uint16_t status = gatt_event_query_complete_get_att_status(packet); + DEBUG_printf(" --> gatt query read complete conn_handle=%d status=%d\n", conn_handle, status); + mp_btstack_active_connection_t *conn = find_active_connection(conn_handle); + if (!conn) { + return; + } + mp_bluetooth_gattc_on_read_write_status(MP_BLUETOOTH_IRQ_GATTC_READ_DONE, conn_handle, conn->pending_value_handle, status); + conn->pending_value_handle = 0xffff; + } else if (event_type == GATT_EVENT_CHARACTERISTIC_VALUE_QUERY_RESULT) { + DEBUG_printf(" --> gatt characteristic value query result\n"); + uint16_t conn_handle = gatt_event_characteristic_value_query_result_get_handle(packet); + uint16_t value_handle = gatt_event_characteristic_value_query_result_get_value_handle(packet); + uint16_t len = gatt_event_characteristic_value_query_result_get_value_length(packet); + const uint8_t *data = gatt_event_characteristic_value_query_result_get_value(packet); + mp_bluetooth_gattc_on_data_available(MP_BLUETOOTH_IRQ_GATTC_READ_RESULT, conn_handle, value_handle, &data, &len, 1); + } } // For when the handler is being used for write-with-response. STATIC void btstack_packet_handler_write_with_response(uint8_t packet_type, uint16_t channel, uint8_t *packet, uint16_t size) { (void)channel; (void)size; - btstack_packet_handler(packet_type, packet, MP_BLUETOOTH_IRQ_GATTC_WRITE_DONE); + if (packet_type != HCI_EVENT_PACKET) { + return; + } + uint8_t event_type = hci_event_packet_get_type(packet); + if (event_type == GATT_EVENT_QUERY_COMPLETE) { + uint16_t conn_handle = gatt_event_query_complete_get_handle(packet); + uint16_t status = gatt_event_query_complete_get_att_status(packet); + DEBUG_printf(" --> gatt query write complete conn_handle=%d status=%d\n", conn_handle, status); + mp_btstack_active_connection_t *conn = find_active_connection(conn_handle); + if (!conn) { + return; + } + mp_bluetooth_gattc_on_read_write_status(MP_BLUETOOTH_IRQ_GATTC_WRITE_DONE, conn_handle, conn->pending_value_handle, status); + conn->pending_value_handle = 0xffff; + m_del(uint8_t, conn->pending_write_value, conn->pending_write_value_len); + conn->pending_write_value = NULL; + conn->pending_write_value_len = 0; + } } #endif // MICROPY_PY_BLUETOOTH_ENABLE_GATT_CLIENT @@ -898,8 +879,8 @@ int mp_bluetooth_gatts_register_service_begin(bool append) { att_db_util_add_service_uuid16(GAP_SERVICE_UUID); uint16_t handle = att_db_util_add_characteristic_uuid16(GAP_DEVICE_NAME_UUID, ATT_PROPERTY_READ | ATT_PROPERTY_DYNAMIC, ATT_SECURITY_NONE, ATT_SECURITY_NONE, NULL, 0); - assert(handle == BTSTACK_GAP_DEVICE_NAME_HANDLE); (void)handle; + assert(handle == BTSTACK_GAP_DEVICE_NAME_HANDLE); att_db_util_add_service_uuid16(0x1801); att_db_util_add_characteristic_uuid16(0x2a05, ATT_PROPERTY_READ, ATT_SECURITY_NONE, ATT_SECURITY_NONE, NULL, 0); @@ -1372,49 +1353,90 @@ int mp_bluetooth_gattc_read(uint16_t conn_handle, uint16_t value_handle) { if (!mp_bluetooth_is_active()) { return ERRNO_BLUETOOTH_NOT_ACTIVE; } - return btstack_error_to_errno(gatt_client_read_value_of_characteristic_using_value_handle(&btstack_packet_handler_read, conn_handle, value_handle)); + + // There can only be a single pending GATT client operation per connection. + mp_btstack_active_connection_t *conn = find_active_connection(conn_handle); + if (!conn) { + DEBUG_printf(" --> no active connection %d\n", conn_handle); + return MP_ENOTCONN; + } + if (conn->pending_value_handle != 0xffff) { + // There's either a read in progress, a write-with-response in progress, or a pending can-write-without-response request outstanding. + DEBUG_printf("--> busy\n"); + return MP_EALREADY; + } + conn->pending_value_handle = value_handle; + int err = gatt_client_read_value_of_characteristic_using_value_handle(&btstack_packet_handler_read, conn_handle, value_handle); + if (err != ERROR_CODE_SUCCESS) { + DEBUG_printf("--> can't send read %d\n", err); + conn->pending_value_handle = 0xffff; + } + return btstack_error_to_errno(err); } -int mp_bluetooth_gattc_write(uint16_t conn_handle, uint16_t value_handle, const uint8_t *value, size_t *value_len, unsigned int mode) { +int mp_bluetooth_gattc_write(uint16_t conn_handle, uint16_t value_handle, const uint8_t *value, size_t value_len, unsigned int mode) { DEBUG_printf("mp_bluetooth_gattc_write\n"); if (!mp_bluetooth_is_active()) { return ERRNO_BLUETOOTH_NOT_ACTIVE; } - // We should be distinguishing between gatt_client_write_value_of_characteristic vs + // Note: We should be distinguishing between gatt_client_write_value_of_characteristic vs // gatt_client_write_characteristic_descriptor_using_descriptor_handle. // However both are implemented using send_gatt_write_attribute_value_request under the hood, // and we get the exact same event to the packet handler. // Same story for the "without response" version. int err; - mp_btstack_pending_op_t *pending_op = NULL; if (mode == MP_BLUETOOTH_WRITE_MODE_NO_RESPONSE) { - // If possible, this will send immediately, copying the buffer directly to the ACL buffer. - err = gatt_client_write_value_of_characteristic_without_response(conn_handle, value_handle, *value_len, (uint8_t *)value); - if (err == GATT_CLIENT_BUSY) { - DEBUG_printf("mp_bluetooth_gattc_write: client busy\n"); - // Can't send right now, need to take a copy of the buffer and add it to the queue. - pending_op = btstack_enqueue_pending_operation(MP_BLUETOOTH_BTSTACK_PENDING_WRITE_NO_RESPONSE, conn_handle, value_handle, value, *value_len); - // Notify when this conn_handle can write. - err = gatt_client_request_can_write_without_response_event(&btstack_packet_handler_generic, conn_handle); - } else { - DEBUG_printf("mp_bluetooth_gattc_write: other failure: %d\n", err); + // Simplest case -- if the write can be dispatched directly, then the buffer is copied directly to the ACL buffer. + err = gatt_client_write_value_of_characteristic_without_response(conn_handle, value_handle, value_len, (uint8_t *)value); + if (err != GATT_CLIENT_BUSY) { + DEBUG_printf("--> can't send write-without-response %d\n", err); + return btstack_error_to_errno(err); } + } + + // There can only be a single pending read/write request per connection. + mp_btstack_active_connection_t *conn = find_active_connection(conn_handle); + if (!conn) { + DEBUG_printf(" --> no active connection %d\n", conn_handle); + return MP_ENOTCONN; + } + if (conn->pending_value_handle != 0xffff) { + // There's either a read in progress, a write-with-response in progress, or a pending can-write-without-response request outstanding. + DEBUG_printf(" --> busy\n"); + return MP_EALREADY; + } + conn->pending_value_handle = value_handle; + conn->pending_write_value_len = value_len; + conn->pending_write_value = m_new(uint8_t, value_len); + memcpy(conn->pending_write_value, value, value_len); + + if (mode == MP_BLUETOOTH_WRITE_MODE_NO_RESPONSE) { + DEBUG_printf(" --> client busy\n"); + // Raise the GATT_EVENT_CAN_WRITE_WITHOUT_RESPONSE event when + // write-without-response will succeed. The only way this fails is if + // there's an outstanding request (unlike for the server-equivalent, + // att_server_request_to_send_notification, which has a queue) but + // we've already checked that there isn't one. + err = gatt_client_request_can_write_without_response_event(&btstack_packet_handler_generic, conn_handle); } else if (mode == MP_BLUETOOTH_WRITE_MODE_WITH_RESPONSE) { - // Pending operation copies the value buffer and keeps a GC reference - // until the response comes back (there is always a response). - pending_op = btstack_enqueue_pending_operation(MP_BLUETOOTH_BTSTACK_PENDING_WRITE, conn_handle, value_handle, value, *value_len); - err = gatt_client_write_value_of_characteristic(&btstack_packet_handler_write_with_response, conn_handle, value_handle, pending_op->len, pending_op->buf); + // Attempt to write immediately. This can fail if there's another + // client operation in progress (e.g. discover). + err = gatt_client_write_value_of_characteristic(&btstack_packet_handler_write_with_response, conn_handle, value_handle, value_len, conn->pending_write_value); } else { return MP_EINVAL; } - if (pending_op && err != ERROR_CODE_SUCCESS) { - // Failure. Unref and free the pending operation. - btstack_remove_pending_operation(pending_op, true /* del */); + if (err != ERROR_CODE_SUCCESS) { + DEBUG_printf("--> write failed %d\n", err); + // We knew that there was no read/write in progress, but some other + // client operation is in progress, so release the pending state. + m_del(uint8_t, conn->pending_write_value, value_len); + conn->pending_write_value_len = 0; + conn->pending_value_handle = 0xffff; } return btstack_error_to_errno(err); diff --git a/extmod/btstack/modbluetooth_btstack.h b/extmod/btstack/modbluetooth_btstack.h index 7890bbfae2..7f4a182073 100644 --- a/extmod/btstack/modbluetooth_btstack.h +++ b/extmod/btstack/modbluetooth_btstack.h @@ -33,8 +33,6 @@ #include "lib/btstack/src/btstack.h" -typedef struct _mp_btstack_pending_op_t mp_btstack_pending_op_t; - typedef struct _mp_bluetooth_btstack_root_pointers_t { // This stores both the advertising data and the scan response data, concatenated together. uint8_t *adv_data; @@ -44,11 +42,12 @@ typedef struct _mp_bluetooth_btstack_root_pointers_t { // Characteristic (and descriptor) value storage. mp_gatts_db_t gatts_db; - btstack_linked_list_t pending_ops; - - #if MICROPY_PY_BLUETOOTH_ENABLE_CENTRAL_MODE + #if MICROPY_PY_BLUETOOTH_ENABLE_GATT_CLIENT // Registration for notify/indicate events. gatt_client_notification_t notification; + + // Active connections (only used for GATT clients). + btstack_linked_list_t active_connections; #endif } mp_bluetooth_btstack_root_pointers_t; diff --git a/extmod/modbluetooth.c b/extmod/modbluetooth.c index e3ef40d789..e65851d6b4 100644 --- a/extmod/modbluetooth.c +++ b/extmod/modbluetooth.c @@ -841,12 +841,11 @@ STATIC mp_obj_t bluetooth_ble_gattc_write(size_t n_args, const mp_obj_t *args) { mp_obj_t data = args[3]; mp_buffer_info_t bufinfo = {0}; mp_get_buffer_raise(data, &bufinfo, MP_BUFFER_READ); - size_t len = bufinfo.len; unsigned int mode = MP_BLUETOOTH_WRITE_MODE_NO_RESPONSE; if (n_args == 5) { mode = mp_obj_get_int(args[4]); } - return bluetooth_handle_errno(mp_bluetooth_gattc_write(conn_handle, value_handle, bufinfo.buf, &len, mode)); + return bluetooth_handle_errno(mp_bluetooth_gattc_write(conn_handle, value_handle, bufinfo.buf, bufinfo.len, mode)); } STATIC MP_DEFINE_CONST_FUN_OBJ_VAR_BETWEEN(bluetooth_ble_gattc_write_obj, 4, 5, bluetooth_ble_gattc_write); diff --git a/extmod/modbluetooth.h b/extmod/modbluetooth.h index fc7012ecfb..6fe8d66ed3 100644 --- a/extmod/modbluetooth.h +++ b/extmod/modbluetooth.h @@ -391,7 +391,7 @@ int mp_bluetooth_gattc_discover_descriptors(uint16_t conn_handle, uint16_t start int mp_bluetooth_gattc_read(uint16_t conn_handle, uint16_t value_handle); // Write the value to the remote peripheral. -int mp_bluetooth_gattc_write(uint16_t conn_handle, uint16_t value_handle, const uint8_t *value, size_t *value_len, unsigned int mode); +int mp_bluetooth_gattc_write(uint16_t conn_handle, uint16_t value_handle, const uint8_t *value, size_t value_len, unsigned int mode); // Initiate MTU exchange for a specific connection using the preferred MTU. int mp_bluetooth_gattc_exchange_mtu(uint16_t conn_handle); diff --git a/extmod/nimble/modbluetooth_nimble.c b/extmod/nimble/modbluetooth_nimble.c index b2667300ca..f806f33176 100644 --- a/extmod/nimble/modbluetooth_nimble.c +++ b/extmod/nimble/modbluetooth_nimble.c @@ -1451,15 +1451,15 @@ STATIC int ble_gattc_attr_write_cb(uint16_t conn_handle, const struct ble_gatt_e } // Write the value to the remote peripheral. -int mp_bluetooth_gattc_write(uint16_t conn_handle, uint16_t value_handle, const uint8_t *value, size_t *value_len, unsigned int mode) { +int mp_bluetooth_gattc_write(uint16_t conn_handle, uint16_t value_handle, const uint8_t *value, size_t value_len, unsigned int mode) { if (!mp_bluetooth_is_active()) { return ERRNO_BLUETOOTH_NOT_ACTIVE; } int err; if (mode == MP_BLUETOOTH_WRITE_MODE_NO_RESPONSE) { - err = ble_gattc_write_no_rsp_flat(conn_handle, value_handle, value, *value_len); + err = ble_gattc_write_no_rsp_flat(conn_handle, value_handle, value, value_len); } else if (mode == MP_BLUETOOTH_WRITE_MODE_WITH_RESPONSE) { - err = ble_gattc_write_flat(conn_handle, value_handle, value, *value_len, &ble_gattc_attr_write_cb, NULL); + err = ble_gattc_write_flat(conn_handle, value_handle, value, value_len, &ble_gattc_attr_write_cb, NULL); } else { err = BLE_HS_EINVAL; } From dcb863ebfbce3251188ce0cedf584a120d8aeb9e Mon Sep 17 00:00:00 2001 From: Jim Mussared Date: Wed, 8 Mar 2023 16:44:43 +1100 Subject: [PATCH 007/701] tests/multi_bluetooth: Use time.sleep_ms instead of time.sleep. On unix, time.sleep is implemented as select(timeout=