address review; use constructor for HCI Adapter

dhalbert · dhalbert · commit b27d51125134 · 2020-08-30T14:06:48.000-04:00
diff --git a/devices/ble_hci/common-hal/_bleio/Adapter.c b/devices/ble_hci/common-hal/_bleio/Adapter.c
@@ -341,7 +341,8 @@ STATIC void bleio_adapter_hci_init(bleio_adapter_obj_t *self) {
     }
 }
 
-void common_hal_bleio_adapter_hci_uart_init(bleio_adapter_obj_t *self, busio_uart_obj_t *uart, digitalio_digitalinout_obj_t *rts, digitalio_digitalinout_obj_t *cts) {
+void common_hal_bleio_adapter_construct_hci_uart(bleio_adapter_obj_t *self, busio_uart_obj_t *uart, digitalio_digitalinout_obj_t *rts, digitalio_digitalinout_obj_t *cts) {
+    self->allocated = true;
     self->hci_uart = uart;
     self->rts_digitalinout = rts;
     self->cts_digitalinout = cts;
diff --git a/devices/ble_hci/common-hal/_bleio/Adapter.h b/devices/ble_hci/common-hal/_bleio/Adapter.h
@@ -52,6 +52,7 @@ typedef struct _bleio_adapter_obj_t {
     busio_uart_obj_t* hci_uart;
     digitalio_digitalinout_obj_t *rts_digitalinout;
     digitalio_digitalinout_obj_t *cts_digitalinout;
+    bool allocated;  // True when in use.
     bool now_advertising;
     bool extended_advertising;
     bool circuitpython_advertising;
diff --git a/devices/ble_hci/common-hal/_bleio/__init__.c b/devices/ble_hci/common-hal/_bleio/__init__.c
@@ -74,6 +74,9 @@ void bleio_reset() {
         return;
     }
     common_hal_bleio_adapter_set_enabled(&common_hal_bleio_adapter_obj, false);
+    common_hal_bleio_adapter_obj.allocated = false;
+
+    bleio_set_adapter(mp_const_none);
 
     //FIX bonding_reset();
     supervisor_start_bluetooth();
@@ -86,6 +89,13 @@ bleio_adapter_obj_t common_hal_bleio_adapter_obj = {
     },
 };
 
+bleio_adapter_obj_t *common_hal_bleio_allocate_adapter_or_raise(void) {
+    if (common_hal_bleio_adapter_obj.allocated) {
+        mp_raise_RuntimeError(translate("Too many Adapters"));
+    }
+    return &common_hal_bleio_adapter_obj;
+}
+
 void common_hal_bleio_check_connected(uint16_t conn_handle) {
     if (conn_handle == BLE_CONN_HANDLE_INVALID) {
         mp_raise_bleio_ConnectionError(translate("Not connected"));
diff --git a/devices/ble_hci/common-hal/_bleio/hci_include/NOTE.txt b/devices/ble_hci/common-hal/_bleio/hci_include/NOTE.txt
@@ -1,2 +1,2 @@
-The HCI-related include files here are copied from the Zephyr project:
-https://github.com/zephyrproject-rtos/zephyr/tree/master/include/bluetooth
+The HCI-related include files here were copied from the Zephyr project, from this commit:
+https://github.com/zephyrproject-rtos/zephyr/tree/0a87f9359edf1ec1c169626df3e19c2b4a4e9646/include/bluetooth
diff --git a/py/obj.h b/py/obj.h
@@ -303,7 +303,7 @@ typedef struct _mp_rom_obj_t { mp_const_obj_t o; } mp_rom_obj_t;
                   (mp_obj_t)&mp_const_none_obj, \
                   (mp_obj_t)&mp_const_none_obj}, }
 
-// These macros are used to define constant map/dict objects
+// These macros are used to define constant or mutable map/dict objects
 // You can put "static" in front of the definition to make it local
 
 #define MP_DEFINE_CONST_MAP(map_name, table_name) \
@@ -329,6 +329,29 @@ typedef struct _mp_rom_obj_t { mp_const_obj_t o; } mp_rom_obj_t;
         }, \
     }
 
+#define MP_DEFINE_MUTABLE_MAP(map_name, table_name) \
+    mp_map_t map_name = { \
+        .all_keys_are_qstrs = 1, \
+        .is_fixed = 1, \
+        .is_ordered = 1, \
+        .used = MP_ARRAY_SIZE(table_name), \
+        .alloc = MP_ARRAY_SIZE(table_name), \
+        .table = table_name, \
+    }
+
+#define MP_DEFINE_MUTABLE_DICT(dict_name, table_name) \
+    mp_obj_dict_t dict_name = {                 \
+        .base = {&mp_type_dict}, \
+        .map = { \
+            .all_keys_are_qstrs = 1, \
+            .is_fixed = 1, \
+            .is_ordered = 1, \
+            .used = MP_ARRAY_SIZE(table_name), \
+            .alloc = MP_ARRAY_SIZE(table_name), \
+            .table = table_name, \
+        }, \
+    }
+
 // These macros are used to declare and define constant staticmethond and classmethod objects
 // You can put "static" in front of the definitions to make them local
 
diff --git a/shared-bindings/_bleio/Adapter.c b/shared-bindings/_bleio/Adapter.c
@@ -48,46 +48,42 @@
 #define WINDOW_DEFAULT (0.1f)
 
 //| class Adapter:
-//|     """BLE adapter
-//|
-//|     The Adapter manages the discovery and connection to other nearby Bluetooth Low Energy devices.
+//|     """
+//|     The BLE Adapter object manages the discovery and connection to other nearby Bluetooth Low Energy devices.
 //|     This part of the Bluetooth Low Energy Specification is known as Generic Access Profile (GAP).
 //|
 //|     Discovery of other devices happens during a scanning process that listens for small packets of
 //|     information, known as advertisements, that are broadcast unencrypted. The advertising packets
 //|     have two different uses. The first is to broadcast a small piece of data to anyone who cares and
-//|     and nothing more. These are known as Beacons. The second class of advertisement is to promote
+//|     and nothing more. These are known as beacons. The second class of advertisement is to promote
 //|     additional functionality available after the devices establish a connection. For example, a
-//|     BLE keyboard may advertise that it can provide key information, but not what the key info is.
+//|     BLE heart rate monitor would advertise that it provides the standard BLE Heart Rate Service.
 //|
-//|     The built-in BLE adapter can do both parts of this process: it can scan for other device
+//|     The Adapter can do both parts of this process: it can scan for other device
 //|     advertisements and it can advertise its own data. Furthermore, Adapters can accept incoming
 //|     connections and also initiate connections."""
 //|
 
-//|     def __init__(self) -> None:
-//|         """You cannot create an instance of `_bleio.Adapter`.
-//|         Use `_bleio.adapter` to access the sole instance available."""
-//|         ...
-//|
-
-//|     def hci_uart_init(self, *, uart: busio.UART, rts: digitalio.DigitalInOut, cts: digitalio.DigitalInOut, baudrate: int = 115200, buffer_size: int = 256) -> None:
-//|         """On boards that do not have native BLE, you can an use HCI co-processor.
+//|     def __init__(self, *, uart: busio.UART, rts: digitalio.DigitalInOut, cts: digitalio.DigitalInOut) -> None:
+//|         """On boards that do not have native BLE, you can use an HCI co-processor.
 //|         Pass the uart and pins used to communicate with the co-processor, such as an Adafruit AirLift.
 //|         The co-processor must have been reset and put into BLE mode beforehand
 //|         by the appropriate pin manipulation.
 //|         The ``uart``, ``rts``, and ``cts`` objects are used to
 //|         communicate with the HCI co-processor in HCI mode.
+//|         The `Adapter` object is enabled during this call.
 //|
-//|         The `_bleio.adapter` object is enabled during this call.
+//|         After instantiating the Adapter, assign it to _bleio.adapter
 //|
-//|         Raises `RuntimeError` on boards with native BLE.
+//|         On boards with native BLE, you cannot create an instance of `_bleio.Adapter`;
+//|         this constructor will raise `NotImplementedError`.
+//|         Use `_bleio.adapter` to access the sole instance already available."""
 //|         """
 //|         ...
 //|
-STATIC mp_obj_t bleio_adapter_hci_uart_init(mp_uint_t n_args, const mp_obj_t *pos_args, mp_map_t *kw_args) {
+STATIC mp_obj_t bleio_adapter_make_new(const mp_obj_type_t *type, size_t n_args, const mp_obj_t *pos_args, mp_map_t *kw_args) {
 #if CIRCUITPY_BLEIO_HCI
-    bleio_adapter_obj_t *self = MP_OBJ_TO_PTR(pos_args[0]);
+    bleio_adapter_obj_t *self = common_hal_bleio_allocate_adapter_or_raise();
 
     enum { ARG_uart, ARG_rts, ARG_cts };
     static const mp_arg_t allowed_args[] = {
@@ -97,7 +93,7 @@ STATIC mp_obj_t bleio_adapter_hci_uart_init(mp_uint_t n_args, const mp_obj_t *po
     };
 
     mp_arg_val_t args[MP_ARRAY_SIZE(allowed_args)];
-    mp_arg_parse_all(n_args - 1, pos_args + 1, kw_args, MP_ARRAY_SIZE(allowed_args), allowed_args, args);
+    mp_arg_parse_all(n_args, pos_args, kw_args, MP_ARRAY_SIZE(allowed_args), allowed_args, args);
 
     busio_uart_obj_t *uart = args[ARG_uart].u_obj;
     if (!MP_OBJ_IS_TYPE(uart, &busio_uart_type)) {
@@ -112,15 +108,14 @@ STATIC mp_obj_t bleio_adapter_hci_uart_init(mp_uint_t n_args, const mp_obj_t *po
     }
 
     // Will enable the adapter.
-    common_hal_bleio_adapter_hci_uart_init(self, uart, rts, cts);
+    common_hal_bleio_adapter_construct_hci_uart(self, uart, rts, cts);
 
-    return mp_const_none;
+    return MP_OBJ_FROM_PTR(self);
 #else
-    mp_raise_RuntimeError(translate("hci_uart_init not available"));
+    mp_raise_NotImplementedError(translate("Cannot create a new Adapter; use _bleio.adapter;"));
     return mp_const_none;
 #endif // CIRCUITPY_BLEIO_HCI
 }
-STATIC MP_DEFINE_CONST_FUN_OBJ_KW(bleio_adapter_hci_uart_init_obj, 1, bleio_adapter_hci_uart_init);
 
 //|
 //|     enabled: bool
@@ -454,7 +449,6 @@ STATIC mp_obj_t bleio_adapter_erase_bonding(mp_obj_t self_in) {
 STATIC MP_DEFINE_CONST_FUN_OBJ_1(bleio_adapter_erase_bonding_obj, bleio_adapter_erase_bonding);
 
 STATIC const mp_rom_map_elem_t bleio_adapter_locals_dict_table[] = {
-    { MP_ROM_QSTR(MP_QSTR_hci_uart_init), MP_ROM_PTR(&bleio_adapter_hci_uart_init_obj) },
     { MP_ROM_QSTR(MP_QSTR_enabled), MP_ROM_PTR(&bleio_adapter_enabled_obj) },
     { MP_ROM_QSTR(MP_QSTR_address), MP_ROM_PTR(&bleio_adapter_address_obj) },
     { MP_ROM_QSTR(MP_QSTR_name),    MP_ROM_PTR(&bleio_adapter_name_obj) },
@@ -479,5 +473,6 @@ STATIC MP_DEFINE_CONST_DICT(bleio_adapter_locals_dict, bleio_adapter_locals_dict
 const mp_obj_type_t bleio_adapter_type = {
     .base = { &mp_type_type },
     .name = MP_QSTR_Adapter,
+    .make_new = bleio_adapter_make_new,
     .locals_dict = (mp_obj_t)&bleio_adapter_locals_dict,
 };
diff --git a/shared-bindings/_bleio/Adapter.h b/shared-bindings/_bleio/Adapter.h
@@ -38,7 +38,7 @@
 const mp_obj_type_t bleio_adapter_type;
 
 #if CIRCUITPY_BLEIO_HCI
-void common_hal_bleio_adapter_hci_uart_init(bleio_adapter_obj_t *self, busio_uart_obj_t *uart, digitalio_digitalinout_obj_t *rts, digitalio_digitalinout_obj_t *cts);
+void common_hal_bleio_adapter_construct_hci_uart(bleio_adapter_obj_t *self, busio_uart_obj_t *uart, digitalio_digitalinout_obj_t *rts, digitalio_digitalinout_obj_t *cts);
 #endif // CIRCUITPY_BLEIO_HCI
 
 extern bool common_hal_bleio_adapter_get_advertising(bleio_adapter_obj_t *self);
diff --git a/shared-bindings/_bleio/__init__.c b/shared-bindings/_bleio/__init__.c
@@ -108,15 +108,83 @@ NORETURN void mp_raise_bleio_SecurityError(const compressed_string_t* fmt, ...)
 
 // Called when _bleio is imported.
 STATIC mp_obj_t bleio___init__(void) {
+// HCI cannot be enabled on import, because we need to setup the HCI adapter first.
 #if !CIRCUITPY_BLEIO_HCI
-    // HCI cannot be enabled on import, because we need to setup the HCI adapter first.
     common_hal_bleio_adapter_set_enabled(&common_hal_bleio_adapter_obj, true);
 #endif
     return mp_const_none;
 }
 STATIC MP_DEFINE_CONST_FUN_OBJ_0(bleio___init___obj, bleio___init__);
 
 
+// Need a forward reference due to mutual references.
+#if CIRCUITPY_BLEIO_HCI
+STATIC mp_obj_dict_t bleio_module_globals;
+#endif
+
+//| def set_adapter(adapter: Optional[_bleio.Adapter]) -> None:
+//|     """Set the adapter to use for BLE. Not settable when the adapter is a singleton."""
+//|     ...
+//|
+mp_obj_t bleio_set_adapter(mp_obj_t adapter_obj) {
+#if CIRCUITPY_BLEIO_HCI
+    if (adapter_obj != mp_const_none && !MP_OBJ_IS_TYPE(adapter_obj, &bleio_adapter_type)) {
+        mp_raise_TypeError_varg(translate("Expected a %q"), bleio_adapter_type.name);
+    }
+
+    // Equivalent of:
+    // bleio.adapter = adapter_obj
+    mp_map_elem_t *elem = mp_map_lookup(&bleio_module_globals.map, MP_ROM_QSTR(MP_QSTR_adapter), MP_MAP_LOOKUP);
+    if (elem) {
+        elem->value = adapter_obj;
+    }
+#else
+    mp_raise_NotImplementedError(translate("Not settable"));
+#endif
+    return mp_const_none;
+}
+MP_DEFINE_CONST_FUN_OBJ_1(bleio_set_adapter_obj, bleio_set_adapter);
+
+#if CIRCUITPY_BLEIO_HCI
+// Make the module dictionary be in RAM, so that _bleio.adapter can be set.
+
+STATIC mp_map_elem_t bleio_module_globals_table[] = {
+    // Name must be the first entry so that the exception printing below is correct.
+    { MP_ROM_QSTR(MP_QSTR___name__),             MP_ROM_QSTR(MP_QSTR__bleio) },
+    { MP_ROM_QSTR(MP_QSTR_Adapter),              MP_OBJ_FROM_PTR(&bleio_adapter_type) },
+    { MP_ROM_QSTR(MP_QSTR_Address),              MP_OBJ_FROM_PTR(&bleio_address_type) },
+    { MP_ROM_QSTR(MP_QSTR_Attribute),            MP_OBJ_FROM_PTR(&bleio_attribute_type) },
+    { MP_ROM_QSTR(MP_QSTR_Connection),           MP_OBJ_FROM_PTR(&bleio_connection_type) },
+    { MP_ROM_QSTR(MP_QSTR_Characteristic),       MP_OBJ_FROM_PTR(&bleio_characteristic_type) },
+    { MP_ROM_QSTR(MP_QSTR_CharacteristicBuffer), MP_OBJ_FROM_PTR(&bleio_characteristic_buffer_type) },
+    { MP_ROM_QSTR(MP_QSTR_Descriptor),           MP_OBJ_FROM_PTR(&bleio_descriptor_type) },
+    { MP_ROM_QSTR(MP_QSTR_PacketBuffer),         MP_OBJ_FROM_PTR(&bleio_packet_buffer_type) },
+    { MP_ROM_QSTR(MP_QSTR_ScanEntry),            MP_OBJ_FROM_PTR(&bleio_scanentry_type) },
+    { MP_ROM_QSTR(MP_QSTR_ScanResults),          MP_OBJ_FROM_PTR(&bleio_scanresults_type) },
+    { MP_ROM_QSTR(MP_QSTR_Service),              MP_OBJ_FROM_PTR(&bleio_service_type) },
+    { MP_ROM_QSTR(MP_QSTR_UUID),                 MP_OBJ_FROM_PTR(&bleio_uuid_type) },
+
+    // Attributes
+    { MP_ROM_QSTR(MP_QSTR_adapter),              mp_const_none },
+
+    // Functions
+    { MP_ROM_QSTR(MP_QSTR_set_adapter),          (mp_obj_t) &bleio_set_adapter_obj },
+
+    // Errors
+    { MP_ROM_QSTR(MP_QSTR_BluetoothError),       MP_OBJ_FROM_PTR(&mp_type_bleio_BluetoothError) },
+    { MP_ROM_QSTR(MP_QSTR_ConnectionError),      MP_OBJ_FROM_PTR(&mp_type_bleio_ConnectionError) },
+    { MP_ROM_QSTR(MP_QSTR_RoleError),            MP_OBJ_FROM_PTR(&mp_type_bleio_RoleError) },
+    { MP_ROM_QSTR(MP_QSTR_SecurityError),        MP_OBJ_FROM_PTR(&mp_type_bleio_SecurityError) },
+
+    // Initialization
+    { MP_ROM_QSTR(MP_QSTR___init__),             MP_OBJ_FROM_PTR(&bleio___init___obj) },
+};
+
+STATIC MP_DEFINE_MUTABLE_DICT(bleio_module_globals, bleio_module_globals_table);
+
+#else
+// When _bleio.adapter is a singleton, and can't be set.
+
 STATIC const mp_rom_map_elem_t bleio_module_globals_table[] = {
     // Name must be the first entry so that the exception printing below is correct.
     { MP_ROM_QSTR(MP_QSTR___name__),             MP_ROM_QSTR(MP_QSTR__bleio) },
@@ -144,10 +212,10 @@ STATIC const mp_rom_map_elem_t bleio_module_globals_table[] = {
 
     // Initialization
     { MP_ROM_QSTR(MP_QSTR___init__),            MP_ROM_PTR(&bleio___init___obj) },
-
 };
 
 STATIC MP_DEFINE_CONST_DICT(bleio_module_globals, bleio_module_globals_table);
+#endif
 
 void bleio_exception_print(const mp_print_t *print, mp_obj_t o_in, mp_print_kind_t kind) {
     mp_print_kind_t k = kind & ~PRINT_EXC_SUBCLASS;
diff --git a/shared-bindings/_bleio/__init__.h b/shared-bindings/_bleio/__init__.h
@@ -55,11 +55,14 @@ extern const mp_obj_type_t mp_type_bleio_ConnectionError;
 extern const mp_obj_type_t mp_type_bleio_RoleError;
 extern const mp_obj_type_t mp_type_bleio_SecurityError;
 
+extern mp_obj_t bleio_set_adapter(mp_obj_t adapter_obj);
+
 NORETURN void mp_raise_bleio_BluetoothError(const compressed_string_t* msg, ...);
 NORETURN void mp_raise_bleio_ConnectionError(const compressed_string_t* msg, ...);
 NORETURN void mp_raise_bleio_RoleError(const compressed_string_t* msg);
 NORETURN void mp_raise_bleio_SecurityError(const compressed_string_t* msg, ...);
 
+bleio_adapter_obj_t *common_hal_bleio_allocate_adapter_or_raise(void);
 void common_hal_bleio_check_connected(uint16_t conn_handle);
 
 uint16_t common_hal_bleio_device_get_conn_handle(mp_obj_t device);
diff --git a/shared-bindings/usb_midi/__init__.c b/shared-bindings/usb_midi/__init__.c
@@ -51,17 +51,7 @@ mp_map_elem_t usb_midi_module_globals_table[] = {
 };
 
 // This isn't const so we can set ports dynamically.
-mp_obj_dict_t usb_midi_module_globals = {
-    .base = {&mp_type_dict},
-    .map = {
-        .all_keys_are_qstrs = 1,
-        .is_fixed = 1,
-        .is_ordered = 1,
-        .used = MP_ARRAY_SIZE(usb_midi_module_globals_table),
-        .alloc = MP_ARRAY_SIZE(usb_midi_module_globals_table),
-        .table = usb_midi_module_globals_table,
-    },
-};
+MP_DEFINE_MUTABLE_DICT(usb_midi_module_globals, usb_midi_module_globals_table);
 
 const mp_obj_module_t usb_midi_module = {
     .base = { &mp_type_module },

Original file line number	Diff line number	Diff line change
`@@ -341,7 +341,8 @@ STATIC void bleio_adapter_hci_init(bleio_adapter_obj_t *self) {`
`341`	`341`	`}`
`342`	`342`	`}`
`343`	`343`
`344`		`-void common_hal_bleio_adapter_hci_uart_init(bleio_adapter_obj_t self, busio_uart_obj_t uart, digitalio_digitalinout_obj_t rts, digitalio_digitalinout_obj_t cts) {`
	`344`	`+void common_hal_bleio_adapter_construct_hci_uart(bleio_adapter_obj_t self, busio_uart_obj_t uart, digitalio_digitalinout_obj_t rts, digitalio_digitalinout_obj_t cts) {`
	`345`	`+ self->allocated = true;`
`345`	`346`	`self->hci_uart = uart;`
`346`	`347`	`self->rts_digitalinout = rts;`
`347`	`348`	`self->cts_digitalinout = cts;`