From 81f2162ca0e926c9a4a1787a3863d94d86be0b36 Mon Sep 17 00:00:00 2001 From: Damien George Date: Wed, 23 Sep 2020 23:18:16 +1000 Subject: [PATCH] extmod/modbluetooth: Change module-owned bytes objects to memoryview. A read-only memoryview object is a better representation of the data, which is owned by the ubluetooth module and may change between calls to the user's irq callback function. Signed-off-by: Damien George --- docs/library/ubluetooth.rst | 22 +++++++++++----- extmod/modbluetooth.c | 26 +++++++------------ tests/multi_bluetooth/ble_characteristic.py | 6 ++--- tests/multi_bluetooth/ble_gap_device_name.py | 2 +- .../multi_bluetooth/ble_gatt_data_transfer.py | 2 +- 5 files changed, 31 insertions(+), 27 deletions(-) diff --git a/docs/library/ubluetooth.rst b/docs/library/ubluetooth.rst index 03d03583a1..f94ad3a612 100644 --- a/docs/library/ubluetooth.rst +++ b/docs/library/ubluetooth.rst @@ -88,12 +88,22 @@ Event Handling arguments, ``event`` (which will be one of the codes below) and ``data`` (which is an event-specific tuple of values). - **Note:** the ``addr``, ``adv_data``, ``char_data``, ``notify_data``, and - ``uuid`` entries in the tuples are references to data managed by the - :mod:`ubluetooth` module (i.e. the same instance will be re-used across - multiple calls to the event handler). If your program wants to use this - data outside of the handler, then it must copy them first, e.g. by using - ``bytes(addr)`` or ``bluetooth.UUID(uuid)``. + **Note:** As an optimisation to prevent unnecessary allocations, the ``addr``, + ``adv_data``, ``char_data``, ``notify_data``, and ``uuid`` entries in the + tuples are read-only memoryview instances pointing to ubluetooth's internal + ringbuffer, and are only valid during the invocation of the IRQ handler + function. If your program needs to save one of these values to access after + the IRQ handler has returned (e.g. by saving it in a class instance or global + variable), then it needs to take a copy of the data, either by using ``bytes()`` + or ``bluetooth.UUID()``, like this:: + + connected_addr = bytes(addr) # equivalently: adv_data, char_data, or notify_data + matched_uuid = bluetooth.UUID(uuid) + + For example, the IRQ handler for a scan result might inspect the ``adv_data`` + to decide if it's the correct device, and only then copy the address data to be + used elsewhere in the program. And to print data from within the IRQ handler, + ``print(bytes(addr))`` will be needed. An event handler showing all possible events:: diff --git a/extmod/modbluetooth.c b/extmod/modbluetooth.c index daf9cd0d19..57f69433a1 100644 --- a/extmod/modbluetooth.c +++ b/extmod/modbluetooth.c @@ -32,7 +32,6 @@ #include "py/mphal.h" #include "py/obj.h" #include "py/objarray.h" -#include "py/objstr.h" #include "py/qstr.h" #include "py/runtime.h" #include "extmod/modbluetooth.h" @@ -62,9 +61,8 @@ typedef struct { mp_obj_t irq_data_tuple; uint8_t irq_data_addr_bytes[6]; uint16_t irq_data_data_alloc; - uint8_t *irq_data_data_bytes; - mp_obj_str_t irq_data_addr; - mp_obj_str_t irq_data_data; + mp_obj_array_t irq_data_addr; + mp_obj_array_t irq_data_data; mp_obj_bluetooth_uuid_t irq_data_uuid; ringbuf_t ringbuf; #if MICROPY_PY_BLUETOOTH_GATTS_ON_READ_CALLBACK @@ -262,11 +260,9 @@ STATIC mp_obj_t bluetooth_ble_make_new(const mp_obj_type_t *type, size_t n_args, #endif // Pre-allocated buffers for address, payload and uuid. - o->irq_data_addr.base.type = &mp_type_bytes; - o->irq_data_addr.data = o->irq_data_addr_bytes; + mp_obj_memoryview_init(&o->irq_data_addr, 'B', 0, 0, o->irq_data_addr_bytes); o->irq_data_data_alloc = MICROPY_PY_BLUETOOTH_MAX_EVENT_DATA_BYTES_LEN(MICROPY_PY_BLUETOOTH_RINGBUF_SIZE); - o->irq_data_data.base.type = &mp_type_bytes; - o->irq_data_data.data = m_new(uint8_t, o->irq_data_data_alloc); + mp_obj_memoryview_init(&o->irq_data_data, 'B', 0, 0, m_new(uint8_t, o->irq_data_data_alloc)); o->irq_data_uuid.base.type = &bluetooth_uuid_type; // Allocate the default ringbuf. @@ -352,7 +348,7 @@ STATIC mp_obj_t bluetooth_ble_config(size_t n_args, const mp_obj_t *args, mp_map // Get old buffer sizes and pointers uint8_t *old_ringbuf_buf = self->ringbuf.buf; size_t old_ringbuf_alloc = self->ringbuf.size; - uint8_t *old_irq_data_buf = (uint8_t *)self->irq_data_data.data; + uint8_t *old_irq_data_buf = (uint8_t *)self->irq_data_data.items; size_t old_irq_data_alloc = self->irq_data_data_alloc; // Atomically update the ringbuf and irq data @@ -362,7 +358,7 @@ STATIC mp_obj_t bluetooth_ble_config(size_t n_args, const mp_obj_t *args, mp_map self->ringbuf.iget = 0; self->ringbuf.iput = 0; self->irq_data_data_alloc = irq_data_alloc; - self->irq_data_data.data = irq_data; + self->irq_data_data.items = irq_data; MICROPY_PY_BLUETOOTH_EXIT // Free old buffers @@ -850,7 +846,7 @@ const mp_obj_module_t mp_module_ubluetooth = { #include -STATIC void ringbuf_extract(ringbuf_t *ringbuf, mp_obj_tuple_t *data_tuple, size_t n_u16, size_t n_u8, mp_obj_str_t *bytes_addr, size_t n_i8, mp_obj_bluetooth_uuid_t *uuid, mp_obj_str_t *bytes_data) { +STATIC void ringbuf_extract(ringbuf_t *ringbuf, mp_obj_tuple_t *data_tuple, size_t n_u16, size_t n_u8, mp_obj_array_t *bytes_addr, size_t n_i8, mp_obj_bluetooth_uuid_t *uuid, mp_obj_array_t *bytes_data) { assert(ringbuf_avail(ringbuf) >= n_u16 * 2 + n_u8 + (bytes_addr ? 6 : 0) + n_i8 + (uuid ? 1 : 0) + (bytes_data ? 1 : 0)); size_t j = 0; @@ -863,8 +859,7 @@ STATIC void ringbuf_extract(ringbuf_t *ringbuf, mp_obj_tuple_t *data_tuple, size if (bytes_addr) { bytes_addr->len = 6; for (size_t i = 0; i < bytes_addr->len; ++i) { - // cast away const, this is actually bt->irq_addr_bytes. - ((uint8_t *)bytes_addr->data)[i] = ringbuf_get(ringbuf); + ((uint8_t *)bytes_addr->items)[i] = ringbuf_get(ringbuf); } data_tuple->items[j++] = MP_OBJ_FROM_PTR(bytes_addr); } @@ -880,12 +875,11 @@ STATIC void ringbuf_extract(ringbuf_t *ringbuf, mp_obj_tuple_t *data_tuple, size #endif // The code that enqueues into the ringbuf should ensure that it doesn't // put more than bt->irq_data_data_alloc bytes into the ringbuf, because - // that's what's available here in bt->irq_data_bytes. + // that's what's available here. if (bytes_data) { bytes_data->len = ringbuf_get16(ringbuf); for (size_t i = 0; i < bytes_data->len; ++i) { - // cast away const, this is actually bt->irq_data_bytes. - ((uint8_t *)bytes_data->data)[i] = ringbuf_get(ringbuf); + ((uint8_t *)bytes_data->items)[i] = ringbuf_get(ringbuf); } data_tuple->items[j++] = MP_OBJ_FROM_PTR(bytes_data); } diff --git a/tests/multi_bluetooth/ble_characteristic.py b/tests/multi_bluetooth/ble_characteristic.py index d918d9aefc..026b9d551c 100644 --- a/tests/multi_bluetooth/ble_characteristic.py +++ b/tests/multi_bluetooth/ble_characteristic.py @@ -54,15 +54,15 @@ def irq(event, data): print("_IRQ_GATTC_CHARACTERISTIC_RESULT", data[-1]) value_handle = data[2] elif event == _IRQ_GATTC_READ_RESULT: - print("_IRQ_GATTC_READ_RESULT", data[-1]) + print("_IRQ_GATTC_READ_RESULT", bytes(data[-1])) elif event == _IRQ_GATTC_READ_DONE: print("_IRQ_GATTC_READ_DONE", data[-1]) elif event == _IRQ_GATTC_WRITE_DONE: print("_IRQ_GATTC_WRITE_DONE", data[-1]) elif event == _IRQ_GATTC_NOTIFY: - print("_IRQ_GATTC_NOTIFY", data[-1]) + print("_IRQ_GATTC_NOTIFY", bytes(data[-1])) elif event == _IRQ_GATTC_INDICATE: - print("_IRQ_GATTC_INDICATE", data[-1]) + print("_IRQ_GATTC_INDICATE", bytes(data[-1])) elif event == _IRQ_GATTS_INDICATE_DONE: print("_IRQ_GATTS_INDICATE_DONE", data[-1]) diff --git a/tests/multi_bluetooth/ble_gap_device_name.py b/tests/multi_bluetooth/ble_gap_device_name.py index 92ea94278a..fbc9d80bae 100644 --- a/tests/multi_bluetooth/ble_gap_device_name.py +++ b/tests/multi_bluetooth/ble_gap_device_name.py @@ -36,7 +36,7 @@ def irq(event, data): print("_IRQ_GATTC_CHARACTERISTIC_RESULT", data[-1]) value_handle = data[2] elif event == _IRQ_GATTC_READ_RESULT: - print("_IRQ_GATTC_READ_RESULT", data[-1]) + print("_IRQ_GATTC_READ_RESULT", bytes(data[-1])) if waiting_event is not None: if isinstance(waiting_event, int) and event == waiting_event: diff --git a/tests/multi_bluetooth/ble_gatt_data_transfer.py b/tests/multi_bluetooth/ble_gatt_data_transfer.py index 240f048607..944c9e2d2a 100644 --- a/tests/multi_bluetooth/ble_gatt_data_transfer.py +++ b/tests/multi_bluetooth/ble_gatt_data_transfer.py @@ -67,7 +67,7 @@ def irq(event, data): elif event == _IRQ_GATTC_WRITE_DONE: print("_IRQ_GATTC_WRITE_DONE", data[-1]) elif event == _IRQ_GATTC_NOTIFY: - print("_IRQ_GATTC_NOTIFY", data[-1]) + print("_IRQ_GATTC_NOTIFY", bytes(data[-1])) if waiting_event is not None: if (isinstance(waiting_event, int) and event == waiting_event) or (