From dd0bc26e65734b8a4fafa3769008e92e2ec6645d Mon Sep 17 00:00:00 2001 From: Damien George Date: Tue, 10 Mar 2020 11:45:03 +1100 Subject: [PATCH] extmod/modbluetooth: Change scan result's "connectable" to "adv_type". This commit changes the BLE _IRQ_SCAN_RESULT data from: addr_type, addr, connectable, rssi, adv_data to: addr_type, addr, adv_type, rssi, adv_data This allows _IRQ_SCAN_RESULT to handle all scan result types (not just connectable and non-connectable passive scans), and to distinguish between them using adv_type which is an integer taking values 0x00-0x04 per the BT specification. This is a breaking change to the API, albeit a very minor one: the existing connectable value was a boolean and True now becomes 0x00, False becomes 0x02. Documentation is updated and a test added. Fixes #5738. --- docs/library/ubluetooth.rst | 12 +++++-- extmod/btstack/modbluetooth_btstack.c | 8 ++--- extmod/modbluetooth.c | 34 +++++++++---------- extmod/modbluetooth.h | 2 +- extmod/nimble/modbluetooth_nimble.c | 13 ++----- tests/multi_bluetooth/ble_gap_advertise.py | 24 ++++++++++--- .../multi_bluetooth/ble_gap_advertise.py.exp | 4 ++- 7 files changed, 55 insertions(+), 42 deletions(-) diff --git a/docs/library/ubluetooth.rst b/docs/library/ubluetooth.rst index 88dc98ecac..a16019e604 100644 --- a/docs/library/ubluetooth.rst +++ b/docs/library/ubluetooth.rst @@ -93,7 +93,7 @@ Event Handling conn_handle, attr_handle = data elif event == _IRQ_SCAN_RESULT: # A single scan result. - addr_type, addr, connectable, rssi, adv_data = data + addr_type, addr, adv_type, rssi, adv_data = data elif event == _IRQ_SCAN_COMPLETE: # Scan duration finished or manually stopped. pass @@ -185,7 +185,15 @@ Observer Role (Scanner) interval and window are 1.28 seconds and 11.25 milliseconds respectively (background scanning). - For each scan result, the ``_IRQ_SCAN_RESULT`` event will be raised. + For each scan result the ``_IRQ_SCAN_RESULT`` event will be raised, with event + data ``(addr_type, addr, adv_type, rssi, adv_data)``. ``adv_type`` values correspond + to the Bluetooth Specification: + + * 0x00 - ADV_IND - connectable and scannable undirected advertising + * 0x01 - ADV_DIRECT_IND - connectable directed advertising + * 0x02 - ADV_SCAN_IND - scannable undirected advertising + * 0x03 - ADV_NONCONN_IND - non-connectable undirected advertising + * 0x04 - SCAN_RSP - scan response When scanning is stopped (either due to the duration finishing or when explicitly stopped), the ``_IRQ_SCAN_COMPLETE`` event will be raised. diff --git a/extmod/btstack/modbluetooth_btstack.c b/extmod/btstack/modbluetooth_btstack.c index 285f2c8166..84e1a85f34 100644 --- a/extmod/btstack/modbluetooth_btstack.c +++ b/extmod/btstack/modbluetooth_btstack.c @@ -115,11 +115,9 @@ STATIC void btstack_packet_handler(uint8_t packet_type, uint16_t channel, uint8_ int8_t rssi = gap_event_advertising_report_get_rssi(packet); uint8_t length = gap_event_advertising_report_get_data_length(packet); const uint8_t *data = gap_event_advertising_report_get_data(packet); - bool connectable = adv_event_type == 0 || adv_event_type == 1; - if (adv_event_type <= 2) { - mp_bluetooth_gap_on_scan_result(address_type, address, connectable, rssi, data, length); - } else if (adv_event_type == 4) { - // TODO: Scan response. + // Emit an event for all advertising types except SCAN_RSP. + if (adv_event_type < 4) { + mp_bluetooth_gap_on_scan_result(address_type, address, adv_event_type, rssi, data, length); } } else if (event_type == HCI_EVENT_DISCONNECTION_COMPLETE) { DEBUG_EVENT_printf(" --> hci disconnect complete\n"); diff --git a/extmod/modbluetooth.c b/extmod/modbluetooth.c index f4b5112118..5b4f698cc6 100644 --- a/extmod/modbluetooth.c +++ b/extmod/modbluetooth.c @@ -774,8 +774,8 @@ 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_b, size_t n_i8, mp_obj_bluetooth_uuid_t *uuid, mp_obj_str_t *bytes_data) { - assert(ringbuf_avail(ringbuf) >= n_u16 * 2 + n_u8 + (bytes_addr ? 6 : 0) + n_b + n_i8 + (uuid ? 1 : 0) + (bytes_data ? 1 : 0)); +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) { + assert(ringbuf_avail(ringbuf) >= n_u16 * 2 + n_u8 + (bytes_addr ? 6 : 0) + n_i8 + (uuid ? 1 : 0) + (bytes_data ? 1 : 0)); int j = 0; for (int i = 0; i < n_u16; ++i) { @@ -792,10 +792,7 @@ STATIC void ringbuf_extract(ringbuf_t *ringbuf, mp_obj_tuple_t *data_tuple, size } data_tuple->items[j++] = MP_OBJ_FROM_PTR(bytes_addr); } - if (n_b) { - data_tuple->items[j++] = mp_obj_new_bool(ringbuf_get(ringbuf)); - } - if (n_i8) { + for (int i = 0; i < n_i8; ++i) { // Note the int8_t got packed into the ringbuf as a uint8_t. data_tuple->items[j++] = MP_OBJ_NEW_SMALL_INT((int8_t)ringbuf_get(ringbuf)); } @@ -843,32 +840,32 @@ STATIC mp_obj_t bluetooth_ble_invoke_irq(mp_obj_t none_in) { if (event == MP_BLUETOOTH_IRQ_CENTRAL_CONNECT || event == MP_BLUETOOTH_IRQ_PERIPHERAL_CONNECT || event == MP_BLUETOOTH_IRQ_CENTRAL_DISCONNECT || event == MP_BLUETOOTH_IRQ_PERIPHERAL_DISCONNECT) { // conn_handle, addr_type, addr - ringbuf_extract(&o->ringbuf, data_tuple, 1, 1, &o->irq_data_addr, 0, 0, NULL, NULL); + ringbuf_extract(&o->ringbuf, data_tuple, 1, 1, &o->irq_data_addr, 0, NULL, NULL); } else if (event == MP_BLUETOOTH_IRQ_GATTS_WRITE) { // conn_handle, value_handle - ringbuf_extract(&o->ringbuf, data_tuple, 2, 0, NULL, 0, 0, NULL, NULL); + ringbuf_extract(&o->ringbuf, data_tuple, 2, 0, NULL, 0, NULL, NULL); #if MICROPY_PY_BLUETOOTH_ENABLE_CENTRAL_MODE } else if (event == MP_BLUETOOTH_IRQ_SCAN_RESULT) { - // addr_type, addr, connectable, rssi, adv_data - ringbuf_extract(&o->ringbuf, data_tuple, 0, 1, &o->irq_data_addr, 1, 1, NULL, &o->irq_data_data); + // addr_type, addr, adv_type, rssi, adv_data + ringbuf_extract(&o->ringbuf, data_tuple, 0, 1, &o->irq_data_addr, 2, NULL, &o->irq_data_data); } else if (event == MP_BLUETOOTH_IRQ_SCAN_COMPLETE) { // No params required. data_tuple->len = 0; } else if (event == MP_BLUETOOTH_IRQ_GATTC_SERVICE_RESULT) { // conn_handle, start_handle, end_handle, uuid - ringbuf_extract(&o->ringbuf, data_tuple, 3, 0, NULL, 0, 0, &o->irq_data_uuid, NULL); + ringbuf_extract(&o->ringbuf, data_tuple, 3, 0, NULL, 0, &o->irq_data_uuid, NULL); } else if (event == MP_BLUETOOTH_IRQ_GATTC_CHARACTERISTIC_RESULT) { // conn_handle, def_handle, value_handle, properties, uuid - ringbuf_extract(&o->ringbuf, data_tuple, 3, 1, NULL, 0, 0, &o->irq_data_uuid, NULL); + ringbuf_extract(&o->ringbuf, data_tuple, 3, 1, NULL, 0, &o->irq_data_uuid, NULL); } else if (event == MP_BLUETOOTH_IRQ_GATTC_DESCRIPTOR_RESULT) { // conn_handle, handle, uuid - ringbuf_extract(&o->ringbuf, data_tuple, 2, 0, NULL, 0, 0, &o->irq_data_uuid, NULL); + ringbuf_extract(&o->ringbuf, data_tuple, 2, 0, NULL, 0, &o->irq_data_uuid, NULL); } else if (event == MP_BLUETOOTH_IRQ_GATTC_READ_RESULT || event == MP_BLUETOOTH_IRQ_GATTC_NOTIFY || event == MP_BLUETOOTH_IRQ_GATTC_INDICATE) { // conn_handle, value_handle, data - ringbuf_extract(&o->ringbuf, data_tuple, 2, 0, NULL, 0, 0, NULL, &o->irq_data_data); + ringbuf_extract(&o->ringbuf, data_tuple, 2, 0, NULL, 0, NULL, &o->irq_data_data); } else if (event == MP_BLUETOOTH_IRQ_GATTC_WRITE_STATUS) { // conn_handle, value_handle, status - ringbuf_extract(&o->ringbuf, data_tuple, 3, 0, NULL, 0, 0, NULL, NULL); + ringbuf_extract(&o->ringbuf, data_tuple, 3, 0, NULL, 0, NULL, NULL); #endif // MICROPY_PY_BLUETOOTH_ENABLE_CENTRAL_MODE } @@ -903,7 +900,7 @@ STATIC bool enqueue_irq(mp_obj_bluetooth_ble_t *o, size_t len, uint16_t event) { // Front of the queue is a scan result, remove it. - // event, addr_type, addr, connectable, rssi + // event, addr_type, addr, adv_type, rssi int n = 2 + 1 + 6 + 1 + 1; for (int i = 0; i < n; ++i) { ringbuf_get(&o->ringbuf); @@ -963,7 +960,7 @@ void mp_bluetooth_gap_on_scan_complete(void) { MICROPY_PY_BLUETOOTH_EXIT } -void mp_bluetooth_gap_on_scan_result(uint8_t addr_type, const uint8_t *addr, bool connectable, const int8_t rssi, const uint8_t *data, size_t data_len) { +void mp_bluetooth_gap_on_scan_result(uint8_t addr_type, const uint8_t *addr, uint8_t adv_type, const int8_t rssi, const uint8_t *data, size_t data_len) { MICROPY_PY_BLUETOOTH_ENTER mp_obj_bluetooth_ble_t *o = MP_OBJ_TO_PTR(MP_STATE_VM(bluetooth)); data_len = MIN(o->irq_data_data_alloc, data_len); @@ -972,7 +969,8 @@ void mp_bluetooth_gap_on_scan_result(uint8_t addr_type, const uint8_t *addr, boo for (int i = 0; i < 6; ++i) { ringbuf_put(&o->ringbuf, addr[i]); } - ringbuf_put(&o->ringbuf, connectable ? 1 : 0); + // The adv_type will get extracted as an int8_t but that's ok because valid values are 0x00-0x04. + ringbuf_put(&o->ringbuf, adv_type); // Note conversion of int8_t rssi to uint8_t. Must un-convert on the way out. ringbuf_put(&o->ringbuf, (uint8_t)rssi); ringbuf_put(&o->ringbuf, data_len); diff --git a/extmod/modbluetooth.h b/extmod/modbluetooth.h index bead2387d6..2055b979bd 100644 --- a/extmod/modbluetooth.h +++ b/extmod/modbluetooth.h @@ -248,7 +248,7 @@ bool mp_bluetooth_gatts_on_read_request(uint16_t conn_handle, uint16_t value_han void mp_bluetooth_gap_on_scan_complete(void); // Notify modbluetooth of a scan result. -void mp_bluetooth_gap_on_scan_result(uint8_t addr_type, const uint8_t *addr, bool connectable, const int8_t rssi, const uint8_t *data, size_t data_len); +void mp_bluetooth_gap_on_scan_result(uint8_t addr_type, const uint8_t *addr, uint8_t adv_type, const int8_t rssi, const uint8_t *data, size_t data_len); // Notify modbluetooth that a service was found (either by discover-all, or discover-by-uuid). void mp_bluetooth_gattc_on_primary_service_result(uint16_t conn_handle, uint16_t start_handle, uint16_t end_handle, mp_obj_bluetooth_uuid_t *service_uuid); diff --git a/extmod/nimble/modbluetooth_nimble.c b/extmod/nimble/modbluetooth_nimble.c index 436783f298..fb8161e782 100644 --- a/extmod/nimble/modbluetooth_nimble.c +++ b/extmod/nimble/modbluetooth_nimble.c @@ -634,16 +634,9 @@ STATIC int gap_scan_cb(struct ble_gap_event *event, void *arg) { return 0; } - if (event->disc.event_type == BLE_HCI_ADV_RPT_EVTYPE_ADV_IND || event->disc.event_type == BLE_HCI_ADV_RPT_EVTYPE_NONCONN_IND) { - bool connectable = event->disc.event_type == BLE_HCI_ADV_RPT_EVTYPE_ADV_IND; - uint8_t addr[6]; - reverse_addr_byte_order(addr, event->disc.addr.val); - mp_bluetooth_gap_on_scan_result(event->disc.addr.type, addr, connectable, event->disc.rssi, event->disc.data, event->disc.length_data); - } else if (event->disc.event_type == BLE_HCI_ADV_RPT_EVTYPE_SCAN_RSP) { - // TODO - } else if (event->disc.event_type == BLE_HCI_ADV_RPT_EVTYPE_SCAN_IND) { - // TODO - } + uint8_t addr[6]; + reverse_addr_byte_order(addr, event->disc.addr.val); + mp_bluetooth_gap_on_scan_result(event->disc.addr.type, addr, event->disc.event_type, event->disc.rssi, event->disc.data, event->disc.length_data); return 0; } diff --git a/tests/multi_bluetooth/ble_gap_advertise.py b/tests/multi_bluetooth/ble_gap_advertise.py index 6b43f275fa..dacb431330 100644 --- a/tests/multi_bluetooth/ble_gap_advertise.py +++ b/tests/multi_bluetooth/ble_gap_advertise.py @@ -11,34 +11,48 @@ ADV_TIME_S = 3 def instance0(): multitest.globals(BDADDR=ble.config("mac")) - print("gap_advertise(20_000)") - ble.gap_advertise(20_000, b"\x02\x01\x06\x04\xffMPY") multitest.next() + + print("gap_advertise(100_000, connectable=False)") + ble.gap_advertise(100_000, b"\x02\x01\x06\x04\xffMPY", connectable=False) time.sleep(ADV_TIME_S) + + print("gap_advertise(20_000, connectable=True)") + ble.gap_advertise(20_000, b"\x02\x01\x06\x04\xffMPY", connectable=True) + time.sleep(ADV_TIME_S) + print("gap_advertise(None)") ble.gap_advertise(None) + ble.active(0) def instance1(): multitest.next() finished = False + adv_types = set() adv_data = None def irq(ev, data): - nonlocal finished, adv_data + nonlocal finished, adv_types, adv_data if ev == _IRQ_SCAN_RESULT: if data[1] == BDADDR: - adv_data = bytes(data[4]) + adv_types.add(data[2]) + if adv_data is None: + adv_data = bytes(data[4]) + else: + if adv_data != data[4]: + adv_data = "MISMATCH" elif ev == _IRQ_SCAN_COMPLETE: finished = True ble.config(rxbuf=2000) ble.irq(irq) - ble.gap_scan(ADV_TIME_S * 1000, 10000, 10000) + ble.gap_scan(2 * ADV_TIME_S * 1000, 10000, 10000) while not finished: machine.idle() ble.active(0) + print("adv_types:", sorted(adv_types)) print("adv_data:", adv_data) diff --git a/tests/multi_bluetooth/ble_gap_advertise.py.exp b/tests/multi_bluetooth/ble_gap_advertise.py.exp index b2ac61eb1b..2eb2a44847 100644 --- a/tests/multi_bluetooth/ble_gap_advertise.py.exp +++ b/tests/multi_bluetooth/ble_gap_advertise.py.exp @@ -1,5 +1,7 @@ --- instance0 --- -gap_advertise(20_000) +gap_advertise(100_000, connectable=False) +gap_advertise(20_000, connectable=True) gap_advertise(None) --- instance1 --- +adv_types: [0, 2] adv_data: b'\x02\x01\x06\x04\xffMPY'