From 364ee62d108bbdc1d4da8bb646a9332a68964b76 Mon Sep 17 00:00:00 2001 From: Dan Halbert Date: Tue, 16 Jul 2019 19:53:36 -0400 Subject: [PATCH] Address review comments. --- conf.py | 1 + ports/nrf/common-hal/bleio/Scanner.c | 13 ++-- shared-bindings/bleio/Address.c | 52 ++++++++++------ shared-bindings/bleio/Central.c | 19 +++--- shared-bindings/bleio/Characteristic.c | 6 +- shared-bindings/bleio/Descriptor.c | 6 +- shared-bindings/bleio/Peripheral.c | 1 + shared-bindings/bleio/ScanEntry.c | 1 - shared-bindings/bleio/Scanner.c | 8 +-- shared-bindings/bleio/Scanner.h | 3 +- shared-bindings/bleio/Service.c | 6 +- shared-module/bleio/AdvertisementData.h | 79 ------------------------- 12 files changed, 65 insertions(+), 130 deletions(-) delete mode 100644 shared-module/bleio/AdvertisementData.h diff --git a/conf.py b/conf.py index 8586573887..6b0f40da6b 100644 --- a/conf.py +++ b/conf.py @@ -84,6 +84,7 @@ version = release = '0.0.0' # List of patterns, relative to source directory, that match files and # directories to ignore when looking for source files. exclude_patterns = ["**/build*", + ".git", ".venv", ".direnv", "docs/README.md", diff --git a/ports/nrf/common-hal/bleio/Scanner.c b/ports/nrf/common-hal/bleio/Scanner.c index fa4f3923c8..57ce940abd 100644 --- a/ports/nrf/common-hal/bleio/Scanner.c +++ b/ports/nrf/common-hal/bleio/Scanner.c @@ -74,10 +74,9 @@ STATIC void scanner_on_ble_evt(ble_evt_t *ble_evt, void *scanner_in) { } void common_hal_bleio_scanner_construct(bleio_scanner_obj_t *self) { - self->scan_entries = mp_obj_new_list(0, NULL); } -void common_hal_bleio_scanner_scan(bleio_scanner_obj_t *self, mp_float_t timeout, mp_float_t interval, mp_float_t window) { +mp_obj_t common_hal_bleio_scanner_scan(bleio_scanner_obj_t *self, mp_float_t timeout, mp_float_t interval, mp_float_t window) { common_hal_bleio_adapter_set_enabled(true); ble_drv_add_event_handler(scanner_on_ble_evt, self); @@ -87,8 +86,7 @@ void common_hal_bleio_scanner_scan(bleio_scanner_obj_t *self, mp_float_t timeout .scan_phys = BLE_GAP_PHY_1MBPS, }; - // Empty the advertising reports list. - mp_obj_list_clear(self->scan_entries); + self->scan_entries = mp_obj_new_list(0, NULL); uint32_t err_code; err_code = sd_ble_gap_scan_start(&scan_params, &m_scan_buffer); @@ -99,8 +97,9 @@ void common_hal_bleio_scanner_scan(bleio_scanner_obj_t *self, mp_float_t timeout mp_hal_delay_ms(timeout * 1000); sd_ble_gap_scan_stop(); -} -mp_obj_t common_hal_bleio_scanner_get_scan_entries(bleio_scanner_obj_t *self) { - return self->scan_entries; + // Return list, and don't hang on to it, so it can be GC'd. + mp_obj_t entries = self->scan_entries; + self->scan_entries = MP_OBJ_NULL; + return entries; } diff --git a/shared-bindings/bleio/Address.c b/shared-bindings/bleio/Address.c index cfad8fc11d..37f01042ce 100644 --- a/shared-bindings/bleio/Address.c +++ b/shared-bindings/bleio/Address.c @@ -48,13 +48,9 @@ //| The value itself can be one of: //| //| :param buf address: The address value to encapsulate. A buffer object (bytearray, bytes) of 6 bytes. -//| :param int address_type: one of these integers: -//| - ``bleio.Address.PUBLIC`` = 0 -//| - ``bleio.Address.RANDOM_STATIC`` = 1 -//| - ``bleio.Address.RANDOM_PRIVATE_RESOLVABLE`` = 2 -//| - ``bleio.Address.RANDOM_PRIVATE_NON_RESOLVABLE`` = 3 +//| :param int address_type: one of the integer values: `PUBLIC`, `RANDOM_STATIC`, +//| `RANDOM_PRIVATE_RESOLVABLE`, or `RANDOM_PRIVATE_NON_RESOLVABLE`. //| - STATIC mp_obj_t bleio_address_make_new(const mp_obj_type_t *type, size_t n_args, const mp_obj_t *pos_args, mp_map_t *kw_args) { enum { ARG_address, ARG_address_type }; static const mp_arg_t allowed_args[] = { @@ -105,12 +101,9 @@ const mp_obj_property_t bleio_address_address_bytes_obj = { //| .. attribute:: type //| -//| The address type (read-only). One of these integers: -//| -//| - ``bleio.Address.PUBLIC`` = 0 -//| - ``bleio.Address.RANDOM_STATIC`` = 1 -//| - ``bleio.Address.RANDOM_PRIVATE_RESOLVABLE`` = 2 -//| - ``bleio.Address.RANDOM_PRIVATE_NON_RESOLVABLE`` = 3 +//| The address type (read-only). +//| One of the integer values: `PUBLIC`, `RANDOM_STATIC`, +//| `RANDOM_PRIVATE_RESOLVABLE`, or `RANDOM_PRIVATE_NON_RESOLVABLE`. //| STATIC mp_obj_t bleio_address_get_type(mp_obj_t self_in) { bleio_address_obj_t *self = MP_OBJ_TO_PTR(self_in); @@ -154,16 +147,37 @@ STATIC mp_obj_t bleio_address_binary_op(mp_binary_op_t op, mp_obj_t lhs_in, mp_o STATIC void bleio_address_print(const mp_print_t *print, mp_obj_t self_in, mp_print_kind_t kind) { bleio_address_obj_t *self = MP_OBJ_TO_PTR(self_in); - mp_obj_t address_bytes = common_hal_bleio_address_get_address_bytes(self); + if (kind == PRINT_STR) { + mp_buffer_info_t buf_info; + mp_obj_t address_bytes = common_hal_bleio_address_get_address_bytes(self); + mp_get_buffer_raise(address_bytes, &buf_info, MP_BUFFER_READ); - mp_buffer_info_t buf_info; - mp_get_buffer_raise(address_bytes, &buf_info, MP_BUFFER_READ); - const uint8_t *buf = (uint8_t *) buf_info.buf; - mp_printf(print, - "%02x:%02x:%02x:%02x:%02x:%02x", - buf[5], buf[4], buf[3], buf[2], buf[1], buf[0]); + const uint8_t *buf = (uint8_t *) buf_info.buf; + mp_printf(print, + "%02x:%02x:%02x:%02x:%02x:%02x", + buf[5], buf[4], buf[3], buf[2], buf[1], buf[0]); + } else { + mp_printf(print, "
"); + } } +//| .. data:: PUBLIC +//| +//| A publicly known address, with a company ID (high 24 bits)and company-assigned part (low 24 bits). +//| +//| .. data:: RANDOM_STATIC +//| +//| A randomly generated address that does not change often. It may never change or may change after +//| a power cycle. +//| +//| .. data:: RANDOM_PRIVATE_RESOLVABLE +//| +//| An address that is usable when the peer knows the other device's secret Identity Resolving Key (IRK). +//| +//| .. data:: RANDOM_PRIVATE_NON_RESOLVABLE +//| +//| A randomly generated address that changes on every connection. +//| STATIC const mp_rom_map_elem_t bleio_address_locals_dict_table[] = { { MP_ROM_QSTR(MP_QSTR_address_bytes), MP_ROM_PTR(&bleio_address_address_bytes_obj) }, { MP_ROM_QSTR(MP_QSTR_type), MP_ROM_PTR(&bleio_address_type_obj) }, diff --git a/shared-bindings/bleio/Central.c b/shared-bindings/bleio/Central.c index a21f711af7..fc00bce5cc 100644 --- a/shared-bindings/bleio/Central.c +++ b/shared-bindings/bleio/Central.c @@ -56,7 +56,7 @@ //| //| my_entry = None //| for entry in entries: -//| if entry.name is not None and entry.name == 'MyCentral': +//| if entry.name is not None and entry.name == 'MyPeripheral': //| my_entry = entry //| break //| @@ -86,13 +86,13 @@ STATIC mp_obj_t bleio_central_make_new(const mp_obj_type_t *type, size_t n_args, //| //| :param bleio.Address address: The address of the peripheral to connect to //| :param float/int timeout: Try to connect for timeout seconds. -//| :param iterable service_uuids: a collection of :py:class:~`UUID` objects for the services +//| :param iterable service_uuids_whitelist: an iterable of :py:class:~`UUID` objects for the services //| provided by the peripheral that you want to use. //| The peripheral may provide more services, but services not listed are ignored. //| If a service in service_uuids is not found during discovery, it will not //| appear in `remote_services`. //| -//| If service_uuids is None, then all services will undergo discovery, which can be slow. +//| If service_uuids_whitelist is None, then all services will undergo discovery, which can be slow. //| //| If the service UUID is 128-bit, or its characteristic UUID's are 128-bit, you //| you must have already created a :py:class:~`UUID` object for that UUID in order for the @@ -101,11 +101,11 @@ STATIC mp_obj_t bleio_central_make_new(const mp_obj_type_t *type, size_t n_args, STATIC mp_obj_t bleio_central_connect(mp_uint_t n_args, const mp_obj_t *pos_args, mp_map_t *kw_args) { bleio_central_obj_t *self = MP_OBJ_TO_PTR(pos_args[0]); - enum { ARG_address, ARG_timeout, ARG_service_uuids }; + enum { ARG_address, ARG_timeout, ARG_service_uuids_whitelist }; static const mp_arg_t allowed_args[] = { { MP_QSTR_address, MP_ARG_REQUIRED | MP_ARG_OBJ }, { MP_QSTR_timeout, MP_ARG_REQUIRED | MP_ARG_OBJ }, - { MP_QSTR_service_uuids, MP_ARG_KW_ONLY | MP_ARG_OBJ, {.u_obj = mp_const_none} }, + { MP_QSTR_service_uuids_whitelist, MP_ARG_KW_ONLY | MP_ARG_OBJ, {.u_obj = mp_const_none} }, }; mp_arg_val_t args[MP_ARRAY_SIZE(allowed_args)]; @@ -119,7 +119,7 @@ STATIC mp_obj_t bleio_central_connect(mp_uint_t n_args, const mp_obj_t *pos_args mp_float_t timeout = mp_obj_get_float(args[ARG_timeout].u_obj); // common_hal_bleio_central_connect() will validate that services is an iterable or None. - common_hal_bleio_central_connect(self, address, timeout, args[ARG_service_uuids].u_obj); + common_hal_bleio_central_connect(self, address, timeout, args[ARG_service_uuids_whitelist].u_obj); return mp_const_none; } @@ -160,12 +160,15 @@ const mp_obj_property_t bleio_central_connected_obj = { //| .. attribute:: remote_services (read-only) //| -//| Empty until connected, then a list of services provided by the remote peripheral. +//| A tuple of services provided by the remote peripheral. +//| If the Central is not connected, an empty tuple will be returned. //| STATIC mp_obj_t bleio_central_get_remote_services(mp_obj_t self_in) { bleio_central_obj_t *self = MP_OBJ_TO_PTR(self_in); - return MP_OBJ_FROM_PTR(common_hal_bleio_central_get_remote_services(self)); + // Return list as a tuple so user won't be able to change it. + mp_obj_list_t *service_list = common_hal_bleio_central_get_remote_services(self); + return mp_obj_new_tuple(service_list->len, service_list->items); } STATIC MP_DEFINE_CONST_FUN_OBJ_1(bleio_central_get_remote_services_obj, bleio_central_get_remote_services); diff --git a/shared-bindings/bleio/Characteristic.c b/shared-bindings/bleio/Characteristic.c index 0d5f8f79ac..3bf0476ab5 100644 --- a/shared-bindings/bleio/Characteristic.c +++ b/shared-bindings/bleio/Characteristic.c @@ -316,13 +316,13 @@ STATIC MP_DEFINE_CONST_DICT(bleio_characteristic_locals_dict, bleio_characterist STATIC void bleio_characteristic_print(const mp_print_t *print, mp_obj_t self_in, mp_print_kind_t kind) { bleio_characteristic_obj_t *self = MP_OBJ_TO_PTR(self_in); - mp_printf(print, "Characteristic("); if (self->uuid) { + mp_printf(print, "Characteristic("); bleio_uuid_print(print, MP_OBJ_FROM_PTR(self->uuid), kind); + mp_printf(print, ")"); } else { - mp_printf(print, "Unregistered uUID"); + mp_printf(print, ""); } - mp_printf(print, ")"); } const mp_obj_type_t bleio_characteristic_type = { diff --git a/shared-bindings/bleio/Descriptor.c b/shared-bindings/bleio/Descriptor.c index f392838926..f2e5aa01df 100644 --- a/shared-bindings/bleio/Descriptor.c +++ b/shared-bindings/bleio/Descriptor.c @@ -162,13 +162,13 @@ STATIC MP_DEFINE_CONST_DICT(bleio_descriptor_locals_dict, bleio_descriptor_local STATIC void bleio_descriptor_print(const mp_print_t *print, mp_obj_t self_in, mp_print_kind_t kind) { bleio_descriptor_obj_t *self = MP_OBJ_TO_PTR(self_in); - mp_printf(print, "Descriptor("); if (self->uuid) { + mp_printf(print, "Descriptor("); bleio_uuid_print(print, MP_OBJ_FROM_PTR(self->uuid), kind); + mp_printf(print, ")"); } else { - mp_printf(print, "Unregistered uUID"); + mp_printf(print, ""); } - mp_printf(print, ")"); } const mp_obj_type_t bleio_descriptor_type = { diff --git a/shared-bindings/bleio/Peripheral.c b/shared-bindings/bleio/Peripheral.c index b8d4c39753..1302c96523 100644 --- a/shared-bindings/bleio/Peripheral.c +++ b/shared-bindings/bleio/Peripheral.c @@ -63,6 +63,7 @@ static const char default_name[] = "CIRCUITPY"; //| Usage:: //| //| import bleio +//| from adafruit_ble.advertising import ServerAdvertisement //| //| # Create a Characteristic. //| chara = bleio.Characteristic(bleio.UUID(0x2919), read=True, notify=True) diff --git a/shared-bindings/bleio/ScanEntry.c b/shared-bindings/bleio/ScanEntry.c index b7798175cc..6eb02c7166 100644 --- a/shared-bindings/bleio/ScanEntry.c +++ b/shared-bindings/bleio/ScanEntry.c @@ -32,7 +32,6 @@ #include "shared-bindings/bleio/Address.h" #include "shared-bindings/bleio/ScanEntry.h" #include "shared-bindings/bleio/UUID.h" -#include "shared-module/bleio/AdvertisementData.h" #include "shared-module/bleio/ScanEntry.h" //| .. currentmodule:: bleio diff --git a/shared-bindings/bleio/Scanner.c b/shared-bindings/bleio/Scanner.c index 79ab34931e..712425c25f 100644 --- a/shared-bindings/bleio/Scanner.c +++ b/shared-bindings/bleio/Scanner.c @@ -75,8 +75,8 @@ STATIC mp_obj_t bleio_scanner_make_new(const mp_obj_type_t *type, size_t n_args, //| Must be in the range 0.0025 - 40.959375 seconds. //| :param float window: the duration (in seconds) to scan a single BLE channel. //| window must be <= interval. -//| :returns: advertising packets found -//| :rtype: list of :py:class:`bleio.ScanEntry` +//| :returns: an iterable of `bleio.ScanEntry` objects +//| :rtype: iterable //| STATIC mp_obj_t bleio_scanner_scan(size_t n_args, const mp_obj_t *pos_args, mp_map_t *kw_args) { enum { ARG_timeout, ARG_interval, ARG_window }; @@ -110,9 +110,7 @@ STATIC mp_obj_t bleio_scanner_scan(size_t n_args, const mp_obj_t *pos_args, mp_m mp_raise_ValueError(translate("window must be <= interval")); } - common_hal_bleio_scanner_scan(self, timeout, interval, window); - - return common_hal_bleio_scanner_get_scan_entries(self); + return common_hal_bleio_scanner_scan(self, timeout, interval, window); } STATIC MP_DEFINE_CONST_FUN_OBJ_KW(bleio_scanner_scan_obj, 2, bleio_scanner_scan); diff --git a/shared-bindings/bleio/Scanner.h b/shared-bindings/bleio/Scanner.h index 1bbab78f4f..3a0ce7eae4 100644 --- a/shared-bindings/bleio/Scanner.h +++ b/shared-bindings/bleio/Scanner.h @@ -34,8 +34,7 @@ extern const mp_obj_type_t bleio_scanner_type; extern void common_hal_bleio_scanner_construct(bleio_scanner_obj_t *self); -extern void common_hal_bleio_scanner_scan(bleio_scanner_obj_t *self, mp_float_t timeout, mp_float_t interval, mp_float_t window); +extern mp_obj_t common_hal_bleio_scanner_scan(bleio_scanner_obj_t *self, mp_float_t timeout, mp_float_t interval, mp_float_t window); extern void common_hal_bleio_scanner_stop(bleio_scanner_obj_t *self); -extern mp_obj_t common_hal_bleio_scanner_get_scan_entries(bleio_scanner_obj_t *self); #endif // MICROPY_INCLUDED_SHARED_BINDINGS_BLEIO_SCANNER_H diff --git a/shared-bindings/bleio/Service.c b/shared-bindings/bleio/Service.c index af215c6a50..db06069913 100644 --- a/shared-bindings/bleio/Service.c +++ b/shared-bindings/bleio/Service.c @@ -166,13 +166,13 @@ STATIC MP_DEFINE_CONST_DICT(bleio_service_locals_dict, bleio_service_locals_dict STATIC void bleio_service_print(const mp_print_t *print, mp_obj_t self_in, mp_print_kind_t kind) { bleio_service_obj_t *self = MP_OBJ_TO_PTR(self_in); - mp_printf(print, "Service("); if (self->uuid) { + mp_printf(print, "Service("); bleio_uuid_print(print, MP_OBJ_FROM_PTR(self->uuid), kind); + mp_printf(print, ")"); } else { - mp_printf(print, "unregistered UUID"); + mp_printf(print, ""); } - mp_printf(print, ")"); } const mp_obj_type_t bleio_service_type = { diff --git a/shared-module/bleio/AdvertisementData.h b/shared-module/bleio/AdvertisementData.h deleted file mode 100644 index 89cdfb0700..0000000000 --- a/shared-module/bleio/AdvertisementData.h +++ /dev/null @@ -1,79 +0,0 @@ -/* - * This file is part of the MicroPython project, http://micropython.org/ - * - * The MIT License (MIT) - * - * Copyright (c) 2019 Dan Halbert for Adafruit Industries - * Copyright (c) 2018 Artur Pacholec - * - * Permission is hereby granted, free of charge, to any person obtaining a copy - * of this software and associated documentation files (the "Software"), to deal - * in the Software without restriction, including without limitation the rights - * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell - * copies of the Software, and to permit persons to whom the Software is - * furnished to do so, subject to the following conditions: - * - * The above copyright notice and this permission notice shall be included in - * all copies or substantial portions of the Software. - * - * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR - * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, - * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE - * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER - * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, - * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN - * THE SOFTWARE. - */ - -#ifndef MICROPY_INCLUDED_SHARED_MODULE_BLEIO_ADVERTISEMENTDATA_H -#define MICROPY_INCLUDED_SHARED_MODULE_BLEIO_ADVERTISEMENTDATA_H - -#include "py/obj.h" - -// Taken from https://www.bluetooth.com/specifications/assigned-numbers/generic-access-profile -enum { - AdFlags = 0x01, - AdIncompleteListOf16BitServiceClassUUIDs = 0x02, - AdCompleteListOf16BitServiceClassUUIDs = 0x03, - AdIncompleteListOf32BitServiceClassUUIDs = 0x04, - AdCompleteListOf32BitServiceClassUUIDs = 0x05, - AdIncompleteListOf128BitServiceClassUUIDs = 0x06, - AdCompleteListOf128BitServiceClassUUIDs = 0x07, - AdShortenedLocalName = 0x08, - AdCompleteLocalName = 0x09, - AdTxPowerLevel = 0x0A, - AdClassOfDevice = 0x0D, - AdSimplePairingHashC = 0x0E, - AdSimplePairingRandomizerR = 0x0F, - AdSecurityManagerTKValue = 0x10, - AdSecurityManagerOOBFlags = 0x11, - AdSlaveConnectionIntervalRange = 0x12, - AdListOf16BitServiceSolicitationUUIDs = 0x14, - AdListOf128BitServiceSolicitationUUIDs = 0x15, - AdServiceData = 0x16, - AdPublicTargetAddress = 0x17, - AdRandomTargetAddress = 0x18, - AdAppearance = 0x19, - AdAdvertisingInterval = 0x1A, - AdLEBluetoothDeviceAddress = 0x1B, - AdLERole = 0x1C, - AdSimplePairingHashC256 = 0x1D, - AdSimplePairingRandomizerR256 = 0x1E, - AdListOf32BitServiceSolicitationUUIDs = 0x1F, - AdServiceData32BitUUID = 0x20, - AdServiceData128BitUUID = 0x21, - AdLESecureConnectionsConfirmationValue = 0x22, - AdLESecureConnectionsRandomValue = 0x23, - AdURI = 0x24, - AdIndoorPositioning = 0x25, - AdTransportDiscoveryData = 0x26, - AdLESupportedFeatures = 0x27, - AdChannelMapUpdateIndication = 0x28, - AdPBADV = 0x29, - AdMeshMessage = 0x2A, - AdMeshBeacon = 0x2B, - Ad3DInformationData = 0x3D, - AdManufacturerSpecificData = 0xFF, -}; - -#endif // MICROPY_INCLUDED_SHARED_MODULE_BLEIO_ADVERTISEMENTDATA_H