From a6aa7397d8e5b309e5675612143d3c5a5e931333 Mon Sep 17 00:00:00 2001 From: Jim Mussared Date: Fri, 3 Mar 2023 00:48:33 +1100 Subject: [PATCH] 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; }