From c398e46b29f5c780b8016f2e88afe4c6984c54d8 Mon Sep 17 00:00:00 2001 From: Jim Mussared Date: Tue, 3 Nov 2020 17:46:11 +1100 Subject: [PATCH] extmod/modbluetooth: Combine gattc-data-available callbacks into one. Instead of having the stack indicate a "start", "data"..., "end", pass through the data in one callback as an array of chunks of data. This is because the upcoming non-ringbuffer modbluetooth implementation cannot buffer the data in the ringbuffer and requires instead a single callback with all the data, to pass to the Python callback. Signed-off-by: Jim Mussared --- extmod/btstack/modbluetooth_btstack.c | 15 ++-------- extmod/modbluetooth.c | 41 ++++++++++++++------------- extmod/modbluetooth.h | 6 +--- extmod/nimble/modbluetooth_nimble.c | 35 +++++++++++++++++------ 4 files changed, 53 insertions(+), 44 deletions(-) diff --git a/extmod/btstack/modbluetooth_btstack.c b/extmod/btstack/modbluetooth_btstack.c index fd5d343637..a4cc601746 100644 --- a/extmod/btstack/modbluetooth_btstack.c +++ b/extmod/btstack/modbluetooth_btstack.c @@ -414,30 +414,21 @@ STATIC void btstack_packet_handler(uint8_t packet_type, uint8_t *packet, uint8_t 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_uint_t atomic_state; - len = mp_bluetooth_gattc_on_data_available_start(MP_BLUETOOTH_IRQ_GATTC_READ_RESULT, conn_handle, value_handle, len, &atomic_state); - mp_bluetooth_gattc_on_data_available_chunk(data, len); - mp_bluetooth_gattc_on_data_available_end(atomic_state); + 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); uint16_t value_handle = gatt_event_notification_get_value_handle(packet); uint16_t len = gatt_event_notification_get_value_length(packet); const uint8_t *data = gatt_event_notification_get_value(packet); - mp_uint_t atomic_state; - len = mp_bluetooth_gattc_on_data_available_start(MP_BLUETOOTH_IRQ_GATTC_NOTIFY, conn_handle, value_handle, len, &atomic_state); - mp_bluetooth_gattc_on_data_available_chunk(data, len); - mp_bluetooth_gattc_on_data_available_end(atomic_state); + mp_bluetooth_gattc_on_data_available(MP_BLUETOOTH_IRQ_GATTC_NOTIFY, conn_handle, value_handle, &data, &len, 1); } else if (event_type == GATT_EVENT_INDICATION) { DEBUG_printf(" --> gatt indication\n"); uint16_t conn_handle = gatt_event_indication_get_handle(packet); uint16_t value_handle = gatt_event_indication_get_value_handle(packet); uint16_t len = gatt_event_indication_get_value_length(packet); const uint8_t *data = gatt_event_indication_get_value(packet); - mp_uint_t atomic_state; - len = mp_bluetooth_gattc_on_data_available_start(MP_BLUETOOTH_IRQ_GATTC_INDICATE, conn_handle, value_handle, len, &atomic_state); - mp_bluetooth_gattc_on_data_available_chunk(data, len); - mp_bluetooth_gattc_on_data_available_end(atomic_state); + mp_bluetooth_gattc_on_data_available(MP_BLUETOOTH_IRQ_GATTC_INDICATE, conn_handle, value_handle, &data, &len, 1); } 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); diff --git a/extmod/modbluetooth.c b/extmod/modbluetooth.c index 90ed495462..6bad134959 100644 --- a/extmod/modbluetooth.c +++ b/extmod/modbluetooth.c @@ -1125,31 +1125,34 @@ void mp_bluetooth_gattc_on_discover_complete(uint8_t event, uint16_t conn_handle schedule_ringbuf(atomic_state); } -size_t mp_bluetooth_gattc_on_data_available_start(uint8_t event, uint16_t conn_handle, uint16_t value_handle, size_t data_len, mp_uint_t *atomic_state_out) { +void mp_bluetooth_gattc_on_data_available(uint8_t event, uint16_t conn_handle, uint16_t value_handle, const uint8_t **data, uint16_t *data_len, size_t num) { MICROPY_PY_BLUETOOTH_ENTER - *atomic_state_out = atomic_state; mp_obj_bluetooth_ble_t *o = MP_OBJ_TO_PTR(MP_STATE_VM(bluetooth)); - data_len = MIN(o->irq_data_data_alloc, data_len); - if (enqueue_irq(o, 2 + 2 + 2 + data_len, event)) { + + // Get the total length of the fragmented buffers. + uint16_t total_len = 0; + for (size_t i = 0; i < num; ++i) { + total_len += data_len[i]; + } + + // Truncate the data at what we'll be able to pass to Python. + total_len = MIN(o->irq_data_data_alloc, total_len); + + if (enqueue_irq(o, 2 + 2 + 2 + total_len, event)) { ringbuf_put16(&o->ringbuf, conn_handle); ringbuf_put16(&o->ringbuf, value_handle); - // Length field is 16-bit. - data_len = MIN(UINT16_MAX, data_len); - ringbuf_put16(&o->ringbuf, data_len); - return data_len; - } else { - return 0; - } -} -void mp_bluetooth_gattc_on_data_available_chunk(const uint8_t *data, size_t data_len) { - mp_obj_bluetooth_ble_t *o = MP_OBJ_TO_PTR(MP_STATE_VM(bluetooth)); - for (size_t i = 0; i < data_len; ++i) { - ringbuf_put(&o->ringbuf, data[i]); - } -} + ringbuf_put16(&o->ringbuf, total_len); -void mp_bluetooth_gattc_on_data_available_end(mp_uint_t atomic_state) { + // Copy total_len from the fragments to the ringbuffer. + uint16_t copied_bytes = 0; + for (size_t i = 0; i < num; ++i) { + for (size_t j = 0; i < data_len[i] && copied_bytes < total_len; ++j) { + ringbuf_put(&o->ringbuf, data[i][j]); + ++copied_bytes; + } + } + } schedule_ringbuf(atomic_state); } diff --git a/extmod/modbluetooth.h b/extmod/modbluetooth.h index 618939ab18..dee7186a2d 100644 --- a/extmod/modbluetooth.h +++ b/extmod/modbluetooth.h @@ -289,11 +289,7 @@ void mp_bluetooth_gattc_on_descriptor_result(uint16_t conn_handle, uint16_t hand void mp_bluetooth_gattc_on_discover_complete(uint8_t event, uint16_t conn_handle, uint16_t status); // Notify modbluetooth that a read has completed with data (or notify/indicate data available, use `event` to disambiguate). -// Note: these functions are to be called in a group protected by MICROPY_PY_BLUETOOTH_ENTER/EXIT. -// _start returns the number of bytes to submit to the calls to _chunk, followed by a call to _end. -size_t mp_bluetooth_gattc_on_data_available_start(uint8_t event, uint16_t conn_handle, uint16_t value_handle, size_t data_len, mp_uint_t *atomic_state_out); -void mp_bluetooth_gattc_on_data_available_chunk(const uint8_t *data, size_t data_len); -void mp_bluetooth_gattc_on_data_available_end(mp_uint_t atomic_state); +void mp_bluetooth_gattc_on_data_available(uint8_t event, uint16_t conn_handle, uint16_t value_handle, const uint8_t **data, uint16_t *data_len, size_t num); // Notify modbluetooth that a read or write operation has completed. void mp_bluetooth_gattc_on_read_write_status(uint8_t event, uint16_t conn_handle, uint16_t value_handle, uint16_t status); diff --git a/extmod/nimble/modbluetooth_nimble.c b/extmod/nimble/modbluetooth_nimble.c index acb4c03dc3..621278e0f1 100644 --- a/extmod/nimble/modbluetooth_nimble.c +++ b/extmod/nimble/modbluetooth_nimble.c @@ -798,16 +798,35 @@ int mp_bluetooth_set_preferred_mtu(uint16_t mtu) { #if MICROPY_PY_BLUETOOTH_ENABLE_CENTRAL_MODE STATIC void gattc_on_data_available(uint8_t event, uint16_t conn_handle, uint16_t value_handle, const struct os_mbuf *om) { - size_t len = OS_MBUF_PKTLEN(om); - mp_uint_t atomic_state; - len = mp_bluetooth_gattc_on_data_available_start(event, conn_handle, value_handle, len, &atomic_state); - while (len > 0 && om != NULL) { - size_t n = MIN(om->om_len, len); - mp_bluetooth_gattc_on_data_available_chunk(OS_MBUF_DATA(om, const uint8_t *), n); - len -= n; + // When the HCI data for an ATT payload arrives, the L2CAP channel will + // buffer it into its receive buffer. We set BLE_L2CAP_JOIN_RX_FRAGS=1 in + // syscfg.h so it should be rare that the mbuf is fragmented, but we do need + // to be able to handle it. We pass all the fragments up to modbluetooth.c + // which will create a temporary buffer on the MicroPython heap if necessary + // to re-assemble them. + + // Count how many links are in the mbuf chain. + size_t n = 0; + const struct os_mbuf *elem = om; + while (elem) { + n += 1; + elem = SLIST_NEXT(elem, om_next); + } + + // Grab data pointers and lengths for each of the links. + const uint8_t **data = mp_local_alloc(sizeof(uint8_t *) * n); + uint16_t *data_len = mp_local_alloc(sizeof(uint16_t) * n); + for (size_t i = 0; i < n; ++i) { + data[i] = OS_MBUF_DATA(om, const uint8_t *); + data_len[i] = om->om_len; om = SLIST_NEXT(om, om_next); } - mp_bluetooth_gattc_on_data_available_end(atomic_state); + + // Pass all the fragments together. + mp_bluetooth_gattc_on_data_available(event, conn_handle, value_handle, data, data_len, n); + + mp_local_free(data_len); + mp_local_free(data); } STATIC int gap_scan_cb(struct ble_gap_event *event, void *arg) {