bleio: API change to create and connect related objects simulatenously: no orphan bleio objects

10 files changed, 174 insertions, 188 deletions

diff --git a/ports/nrf/common-hal/bleio/Adapter.c b/ports/nrf/common-hal/bleio/Adapter.c
index 74650e212..74bee9e56 100644
--- a/ports/nrf/common-hal/bleio/Adapter.c
+++ b/ports/nrf/common-hal/bleio/Adapter.c
@@ -66,12 +66,14 @@ STATIC uint32_t ble_stack_enable(void) {
     };
 
     uint32_t err_code = sd_softdevice_enable(&clock_config, softdevice_assert_handler);
-    if (err_code != NRF_SUCCESS)
+    if (err_code != NRF_SUCCESS) {
         return err_code;
+    }
 
     err_code = sd_nvic_EnableIRQ(SD_EVT_IRQn);
-    if (err_code != NRF_SUCCESS)
+    if (err_code != NRF_SUCCESS) {
         return err_code;
+    }
 
     // Start with no event handlers, etc.
     ble_drv_reset();
@@ -112,8 +114,9 @@ STATIC uint32_t ble_stack_enable(void) {
         .conn_sup_timeout  = BLE_CONN_SUP_TIMEOUT,
     };
    err_code = sd_ble_gap_ppcp_set(&gap_conn_params);
-   if (err_code != NRF_SUCCESS)
+   if (err_code != NRF_SUCCESS) {
        return err_code;
+   }
 
    err_code = sd_ble_gap_appearance_set(BLE_APPEARANCE_UNKNOWN);
    return err_code;
diff --git a/ports/nrf/common-hal/bleio/Central.c b/ports/nrf/common-hal/bleio/Central.c
index 3c5d97784..0f8432b73 100644
--- a/ports/nrf/common-hal/bleio/Central.c
+++ b/ports/nrf/common-hal/bleio/Central.c
@@ -76,7 +76,7 @@ STATIC void central_on_ble_evt(ble_evt_t *ble_evt, void *central_in) {
 void common_hal_bleio_central_construct(bleio_central_obj_t *self) {
     common_hal_bleio_adapter_set_enabled(true);
 
-    self->remote_services_list = mp_obj_new_list(0, NULL);
+    self->remote_service_list = mp_obj_new_list(0, NULL);
     self->conn_handle = BLE_CONN_HANDLE_INVALID;
 }
 
@@ -134,12 +134,12 @@ bool common_hal_bleio_central_get_connected(bleio_central_obj_t *self) {
 mp_obj_tuple_t *common_hal_bleio_central_discover_remote_services(bleio_central_obj_t *self, mp_obj_t service_uuids_whitelist) {
     common_hal_bleio_device_discover_remote_services(MP_OBJ_FROM_PTR(self), service_uuids_whitelist);
     // Convert to a tuple and then clear the list so the callee will take ownership.
-    mp_obj_tuple_t *services_tuple = mp_obj_new_tuple(self->remote_services_list->len,
-                                                      self->remote_services_list->items);
-    mp_obj_list_clear(self->remote_services_list);
+    mp_obj_tuple_t *services_tuple = mp_obj_new_tuple(self->remote_service_list->len,
+                                                      self->remote_service_list->items);
+    mp_obj_list_clear(self->remote_service_list);
     return services_tuple;
 }
 
 mp_obj_list_t *common_hal_bleio_central_get_remote_services(bleio_central_obj_t *self) {
-    return self->remote_services_list;
+    return self->remote_service_list;
 }
diff --git a/ports/nrf/common-hal/bleio/Central.h b/ports/nrf/common-hal/bleio/Central.h
index b7a9050b8..2e00aa35e 100644
--- a/ports/nrf/common-hal/bleio/Central.h
+++ b/ports/nrf/common-hal/bleio/Central.h
@@ -38,7 +38,7 @@ typedef struct {
     volatile bool waiting_to_connect;
     volatile uint16_t conn_handle;
     // Services discovered after connecting to a remote peripheral.
-    mp_obj_list_t *remote_services_list;
+    mp_obj_list_t *remote_service_list;
 } bleio_central_obj_t;
 
 #endif // MICROPY_INCLUDED_SHARED_MODULE_BLEIO_CENTRAL_H
diff --git a/ports/nrf/common-hal/bleio/Characteristic.c b/ports/nrf/common-hal/bleio/Characteristic.c
index 604d24d73..012029b6c 100644
--- a/ports/nrf/common-hal/bleio/Characteristic.c
+++ b/ports/nrf/common-hal/bleio/Characteristic.c
@@ -124,29 +124,24 @@ STATIC void characteristic_gattc_read(bleio_characteristic_obj_t *characteristic
     ble_drv_remove_event_handler(characteristic_on_gattc_read_rsp_evt, characteristic);
 }
 
-void common_hal_bleio_characteristic_construct(bleio_characteristic_obj_t *self, bleio_uuid_obj_t *uuid, bleio_characteristic_properties_t props, bleio_attribute_security_mode_t read_perm, bleio_attribute_security_mode_t write_perm, mp_int_t max_length, bool fixed_length, mp_obj_list_t *descriptor_list) {
+void common_hal_bleio_characteristic_construct(bleio_characteristic_obj_t *self, bleio_uuid_obj_t *uuid, bleio_characteristic_properties_t props, bleio_attribute_security_mode_t read_perm, bleio_attribute_security_mode_t write_perm, mp_int_t max_length, bool fixed_length, mp_buffer_info_t *initial_value_bufinfo) {
     self->service = MP_OBJ_NULL;
     self->uuid = uuid;
-    self->value = mp_const_empty_bytes;
     self->handle = BLE_GATT_HANDLE_INVALID;
     self->props = props;
     self->read_perm = read_perm;
     self->write_perm = write_perm;
-    self->descriptor_list = descriptor_list;
+    self->descriptor_list = mp_obj_new_list(0, NULL);
 
     const mp_int_t max_length_max = fixed_length ? BLE_GATTS_FIX_ATTR_LEN_MAX : BLE_GATTS_VAR_ATTR_LEN_MAX;
     if (max_length < 0 || max_length > max_length_max) {
-        mp_raise_ValueError_varg(translate("max_length must be 0-%d when fixed_length is %s"),
+        mp_raise_ValueError_varg(translate("mnax_length must be 0-%d when fixed_length is %s"),
                                  max_length_max, fixed_length ? "True" : "False");
     }
     self->max_length = max_length;
     self->fixed_length = fixed_length;
 
-    for (size_t descriptor_idx = 0; descriptor_idx < descriptor_list->len; ++descriptor_idx) {
-        bleio_descriptor_obj_t *descriptor =
-            MP_OBJ_TO_PTR(descriptor_list->items[descriptor_idx]);
-        descriptor->characteristic = self;
-    }
+    common_hal_bleio_characteristic_set_value(self, initial_value_bufinfo);
 }
 
 mp_obj_list_t *common_hal_bleio_characteristic_get_descriptor_list(bleio_characteristic_obj_t *self) {
@@ -173,6 +168,15 @@ mp_obj_t common_hal_bleio_characteristic_get_value(bleio_characteristic_obj_t *s
 }
 
 void common_hal_bleio_characteristic_set_value(bleio_characteristic_obj_t *self, mp_buffer_info_t *bufinfo) {
+    if (self->fixed_length && bufinfo->len != self->max_length) {
+        mp_raise_ValueError(translate("Value length != required fixed length"));
+    }
+    if (bufinfo->len > self->max_length) {
+        mp_raise_ValueError(translate("Value length > max_length"));
+    }
+
+    self->value = mp_obj_new_bytes(bufinfo->buf, bufinfo->len);
+
     // Do GATT operations only if this characteristic has been added to a registered service.
     if (self->handle != BLE_GATT_HANDLE_INVALID) {
         uint16_t conn_handle = common_hal_bleio_device_get_conn_handle(self->service->device);
@@ -182,12 +186,6 @@ void common_hal_bleio_characteristic_set_value(bleio_characteristic_obj_t *self,
             common_hal_bleio_gattc_write(self->handle, conn_handle, bufinfo,
                                          (self->props & CHAR_PROP_WRITE_NO_RESPONSE));
         } else {
-            if (self->fixed_length && bufinfo->len != self->max_length) {
-                mp_raise_ValueError(translate("Value length != required fixed length"));
-            }
-            if (bufinfo->len > self->max_length) {
-                mp_raise_ValueError(translate("Value length > max_length"));
-            }
 
             bool sent = false;
             uint16_t cccd = 0;
@@ -213,8 +211,6 @@ void common_hal_bleio_characteristic_set_value(bleio_characteristic_obj_t *self,
             }
         }
     }
-
-    self->value = mp_obj_new_bytes(bufinfo->buf, bufinfo->len);
 }
 
 bleio_uuid_obj_t *common_hal_bleio_characteristic_get_uuid(bleio_characteristic_obj_t *self) {
@@ -225,6 +221,43 @@ bleio_characteristic_properties_t common_hal_bleio_characteristic_get_properties
     return self->props;
 }
 
+void common_hal_bleio_characteristic_add_descriptor(bleio_characteristic_obj_t *self, bleio_descriptor_obj_t *descriptor) {
+    // Connect descriptor to parent characteristic.
+    descriptor->characteristic = self;
+
+    ble_uuid_t desc_uuid;
+    bleio_uuid_convert_to_nrf_ble_uuid(descriptor->uuid, &desc_uuid);
+
+    ble_gatts_attr_md_t desc_attr_md = {
+        // Data passed is not in a permanent location and should be copied.
+        .vloc = BLE_GATTS_VLOC_STACK,
+        .vlen = !descriptor->fixed_length,
+    };
+
+    bleio_attribute_gatts_set_security_mode(&desc_attr_md.read_perm, descriptor->read_perm);
+    bleio_attribute_gatts_set_security_mode(&desc_attr_md.write_perm, descriptor->write_perm);
+
+    mp_buffer_info_t desc_value_bufinfo;
+    mp_get_buffer_raise(descriptor->value, &desc_value_bufinfo, MP_BUFFER_READ);
+
+    ble_gatts_attr_t desc_attr = {
+        .p_uuid = &desc_uuid,
+        .p_attr_md = &desc_attr_md,
+        .init_len = desc_value_bufinfo.len,
+        .p_value = desc_value_bufinfo.buf,
+        .init_offs = 0,
+        .max_len = descriptor->max_length,
+    };
+
+    uint32_t err_code = sd_ble_gatts_descriptor_add(self->handle, &desc_attr, &descriptor->handle);
+
+    if (err_code != NRF_SUCCESS) {
+        mp_raise_OSError_msg_varg(translate("Failed to add characteristic, err 0x%04x"), err_code);
+    }
+
+    mp_obj_list_append(self->descriptor_list, MP_OBJ_FROM_PTR(descriptor));
+}
+
 void common_hal_bleio_characteristic_set_cccd(bleio_characteristic_obj_t *self, bool notify, bool indicate) {
     if (self->cccd_handle == BLE_GATT_HANDLE_INVALID) {
         mp_raise_ValueError(translate("No CCCD for this Characteristic"));
diff --git a/ports/nrf/common-hal/bleio/Descriptor.c b/ports/nrf/common-hal/bleio/Descriptor.c
index 2a2019277..ee8d840f2 100644
--- a/ports/nrf/common-hal/bleio/Descriptor.c
+++ b/ports/nrf/common-hal/bleio/Descriptor.c
@@ -35,10 +35,9 @@
 
 static volatile bleio_descriptor_obj_t *m_read_descriptor;
 
-void common_hal_bleio_descriptor_construct(bleio_descriptor_obj_t *self, bleio_uuid_obj_t *uuid, bleio_attribute_security_mode_t read_perm, bleio_attribute_security_mode_t write_perm, mp_int_t max_length, bool fixed_length) {
+void common_hal_bleio_descriptor_construct(bleio_descriptor_obj_t *self, bleio_uuid_obj_t *uuid, bleio_attribute_security_mode_t read_perm, bleio_attribute_security_mode_t write_perm, mp_int_t max_length, bool fixed_length, mp_buffer_info_t *initial_value_bufinfo) {
     self->characteristic = MP_OBJ_NULL;
     self->uuid = uuid;
-    self->value = mp_const_empty_bytes;
     self->handle = BLE_GATT_HANDLE_INVALID;
     self->read_perm = read_perm;
     self->write_perm = write_perm;
@@ -50,6 +49,8 @@ void common_hal_bleio_descriptor_construct(bleio_descriptor_obj_t *self, bleio_u
     }
     self->max_length = max_length;
     self->fixed_length = fixed_length;
+
+    common_hal_bleio_descriptor_set_value(self, initial_value_bufinfo);
 }
 
 bleio_uuid_obj_t *common_hal_bleio_descriptor_get_uuid(bleio_descriptor_obj_t *self) {
@@ -119,6 +120,15 @@ mp_obj_t common_hal_bleio_descriptor_get_value(bleio_descriptor_obj_t *self) {
 }
 
 void common_hal_bleio_descriptor_set_value(bleio_descriptor_obj_t *self, mp_buffer_info_t *bufinfo) {
+    if (self->fixed_length && bufinfo->len != self->max_length) {
+        mp_raise_ValueError(translate("Value length != required fixed length"));
+    }
+    if (bufinfo->len > self->max_length) {
+        mp_raise_ValueError(translate("Value length > max_length"));
+    }
+
+    self->value = mp_obj_new_bytes(bufinfo->buf, bufinfo->len);
+
     // Do GATT operations only if this descriptor has been registered.
     if (self->handle != BLE_GATT_HANDLE_INVALID) {
         uint16_t conn_handle = common_hal_bleio_device_get_conn_handle(self->characteristic->service->device);
@@ -126,16 +136,8 @@ void common_hal_bleio_descriptor_set_value(bleio_descriptor_obj_t *self, mp_buff
             // false means WRITE_REQ, not write-no-response
             common_hal_bleio_gattc_write(self->handle, conn_handle, bufinfo, false);
         } else {
-            if (self->fixed_length && bufinfo->len != self->max_length) {
-                mp_raise_ValueError(translate("Value length != required fixed length"));
-            }
-            if (bufinfo->len > self->max_length) {
-                mp_raise_ValueError(translate("Value length > max_length"));
-            }
-
             common_hal_bleio_gatts_write(self->handle, conn_handle, bufinfo);
         }
     }
 
-    self->value = mp_obj_new_bytes(bufinfo->buf, bufinfo->len);
 }
diff --git a/ports/nrf/common-hal/bleio/Descriptor.h b/ports/nrf/common-hal/bleio/Descriptor.h
index 6db4d7be3..db1175146 100644
--- a/ports/nrf/common-hal/bleio/Descriptor.h
+++ b/ports/nrf/common-hal/bleio/Descriptor.h
@@ -31,7 +31,7 @@
 
 #include "py/obj.h"
 
-#include "shared-bindings/bleio/Characteristic.h"
+#include "common-hal/bleio/Characteristic.h"
 #include "common-hal/bleio/UUID.h"
 
 typedef struct {
diff --git a/ports/nrf/common-hal/bleio/Peripheral.c b/ports/nrf/common-hal/bleio/Peripheral.c
index 08a6d5066..6f8a1a2fc 100644
--- a/ports/nrf/common-hal/bleio/Peripheral.c
+++ b/ports/nrf/common-hal/bleio/Peripheral.c
@@ -71,7 +71,7 @@ STATIC void peripheral_on_ble_evt(ble_evt_t *ble_evt, void *self_in) {
     bleio_peripheral_obj_t *self = (bleio_peripheral_obj_t*)self_in;
 
     // For debugging.
-     mp_printf(&mp_plat_print, "Peripheral event: 0x%04x\n", ble_evt->header.evt_id);
+    // mp_printf(&mp_plat_print, "Peripheral event: 0x%04x\n", ble_evt->header.evt_id);
 
     switch (ble_evt->header.evt_id) {
         case BLE_GAP_EVT_CONNECTED: {
@@ -156,10 +156,8 @@ STATIC void peripheral_on_ble_evt(ble_evt_t *ble_evt, void *self_in) {
             ble_gap_evt_auth_status_t* status = &ble_evt->evt.gap_evt.params.auth_status;
             if (BLE_GAP_SEC_STATUS_SUCCESS == status->auth_status) {
                 // TODO _ediv = bonding_keys->own_enc.master_id.ediv;
-                mp_printf(&mp_plat_print, "Pairing succeeded, status: 0x%04x\n", status->auth_status);
                 self->pair_status = PAIR_PAIRED;
             } else {
-                mp_printf(&mp_plat_print, "Pairing failed, status: 0x%04x\n", status->auth_status);
                 self->pair_status = PAIR_NOT_PAIRED;
             }
             break;
@@ -204,17 +202,17 @@ STATIC void peripheral_on_ble_evt(ble_evt_t *ble_evt, void *self_in) {
 
         default:
             // For debugging.
-            mp_printf(&mp_plat_print, "Unhandled peripheral event: 0x%04x\n", ble_evt->header.evt_id);
+            // mp_printf(&mp_plat_print, "Unhandled peripheral event: 0x%04x\n", ble_evt->header.evt_id);
             break;
     }
 }
 
-void common_hal_bleio_peripheral_construct(bleio_peripheral_obj_t *self, mp_obj_list_t *services_list, mp_obj_t name) {
+void common_hal_bleio_peripheral_construct(bleio_peripheral_obj_t *self, mp_obj_t name) {
     common_hal_bleio_adapter_set_enabled(true);
 
-    self->services_list = services_list;
+    self->service_list = mp_obj_new_list(0, NULL);
     // Used only for discovery when acting as a client.
-    self->remote_services_list = mp_obj_new_list(0, NULL);
+    self->remote_service_list = mp_obj_new_list(0, NULL);
     self->name = name;
 
     self->conn_handle = BLE_CONN_HANDLE_INVALID;
@@ -222,35 +220,30 @@ void common_hal_bleio_peripheral_construct(bleio_peripheral_obj_t *self, mp_obj_
     self->pair_status = PAIR_NOT_PAIRED;
 
     memset(&self->bonding_keys, 0, sizeof(self->bonding_keys));
+}
 
-    // Add all the services.
-
-    for (size_t service_idx = 0; service_idx < services_list->len; ++service_idx) {
-        bleio_service_obj_t *service = MP_OBJ_TO_PTR(services_list->items[service_idx]);
-
-        service->device = MP_OBJ_FROM_PTR(self);
-
-        ble_uuid_t uuid;
-        bleio_uuid_convert_to_nrf_ble_uuid(service->uuid, &uuid);
+void common_hal_bleio_peripheral_add_service(bleio_peripheral_obj_t *self, bleio_service_obj_t *service) {
+    service->device = MP_OBJ_FROM_PTR(self);
 
-        uint8_t service_type = BLE_GATTS_SRVC_TYPE_PRIMARY;
-        if (common_hal_bleio_service_get_is_secondary(service)) {
-            service_type = BLE_GATTS_SRVC_TYPE_SECONDARY;
-        }
+    ble_uuid_t uuid;
+    bleio_uuid_convert_to_nrf_ble_uuid(service->uuid, &uuid);
 
-        const uint32_t err_code = sd_ble_gatts_service_add(service_type, &uuid, &service->handle);
-        if (err_code != NRF_SUCCESS) {
-            mp_raise_OSError_msg_varg(translate("Failed to add service, err 0x%04x"), err_code);
-        }
+    uint8_t service_type = BLE_GATTS_SRVC_TYPE_PRIMARY;
+    if (common_hal_bleio_service_get_is_secondary(service)) {
+        service_type = BLE_GATTS_SRVC_TYPE_SECONDARY;
+    }
 
-        // Once the service has been registered, its characteristics can be added.
-        common_hal_bleio_service_add_all_characteristics(service);
+    const uint32_t err_code = sd_ble_gatts_service_add(service_type, &uuid, &service->handle);
+    if (err_code != NRF_SUCCESS) {
+        mp_raise_OSError_msg_varg(translate("Failed to add service, err 0x%04x"), err_code);
     }
+
+    mp_obj_list_append(self->service_list, MP_OBJ_FROM_PTR(service));
 }
 
 
 mp_obj_list_t *common_hal_bleio_peripheral_get_services(bleio_peripheral_obj_t *self) {
-    return self->services_list;
+    return self->service_list;
 }
 
 bool common_hal_bleio_peripheral_get_connected(bleio_peripheral_obj_t *self) {
@@ -344,9 +337,9 @@ void common_hal_bleio_peripheral_disconnect(bleio_peripheral_obj_t *self) {
 mp_obj_tuple_t *common_hal_bleio_peripheral_discover_remote_services(bleio_peripheral_obj_t *self, mp_obj_t service_uuids_whitelist) {
     common_hal_bleio_device_discover_remote_services(MP_OBJ_FROM_PTR(self), service_uuids_whitelist);
     // Convert to a tuple and then clear the list so the callee will take ownership.
-    mp_obj_tuple_t *services_tuple = mp_obj_new_tuple(self->remote_services_list->len,
-                                                      self->remote_services_list->items);
-    mp_obj_list_clear(self->remote_services_list);
+    mp_obj_tuple_t *services_tuple = mp_obj_new_tuple(self->remote_service_list->len,
+                                                      self->remote_service_list->items);
+    mp_obj_list_clear(self->remote_service_list);
     return services_tuple;
 }
 
diff --git a/ports/nrf/common-hal/bleio/Peripheral.h b/ports/nrf/common-hal/bleio/Peripheral.h
index b663c9601..1620874ca 100644
--- a/ports/nrf/common-hal/bleio/Peripheral.h
+++ b/ports/nrf/common-hal/bleio/Peripheral.h
@@ -49,9 +49,9 @@ typedef struct {
     mp_obj_t name;
     volatile uint16_t conn_handle;
     // Services provided by this peripheral.
-    mp_obj_list_t *services_list;
+    mp_obj_list_t *service_list;
     // Remote services discovered when this peripheral is acting as a client.
-    mp_obj_list_t *remote_services_list;
+    mp_obj_list_t *remote_service_list;
     // The advertising data and scan response buffers are held by us, not by the SD, so we must
     // maintain them and not change it. If we need to change the contents during advertising,
     // there are tricks to get the SD to notice (see DevZone - TBS).
diff --git a/ports/nrf/common-hal/bleio/Service.c b/ports/nrf/common-hal/bleio/Service.c
index 28e713cc8..e5201cc46 100644
--- a/ports/nrf/common-hal/bleio/Service.c
+++ b/ports/nrf/common-hal/bleio/Service.c
@@ -34,19 +34,13 @@
 #include "shared-bindings/bleio/Service.h"
 #include "shared-bindings/bleio/Adapter.h"
 
-void common_hal_bleio_service_construct(bleio_service_obj_t *self, bleio_uuid_obj_t *uuid, mp_obj_list_t *characteristic_list, bool is_secondary) {
-    self->device = mp_const_none;
+void common_hal_bleio_service_construct(bleio_service_obj_t *self, bleio_uuid_obj_t *uuid, bool is_secondary) {
     self->handle = 0xFFFF;
     self->uuid = uuid;
-    self->characteristic_list = characteristic_list;
+    self->characteristic_list = mp_obj_new_list(0, NULL);
     self->is_remote = false;
     self->is_secondary = is_secondary;
-
-    for (size_t characteristic_idx = 0; characteristic_idx < characteristic_list->len; ++characteristic_idx) {
-        bleio_characteristic_obj_t *characteristic =
-            MP_OBJ_TO_PTR(characteristic_list->items[characteristic_idx]);
-        characteristic->service = self;
-    }
+    self->device = mp_const_none;
 }
 
 bleio_uuid_obj_t *common_hal_bleio_service_get_uuid(bleio_service_obj_t *self) {
@@ -65,106 +59,67 @@ bool common_hal_bleio_service_get_is_secondary(bleio_service_obj_t *self) {
     return self->is_secondary;
 }
 
-// Call this after the Service has been added to the Peripheral.
-void common_hal_bleio_service_add_all_characteristics(bleio_service_obj_t *self) {
-    // Add all the characteristics.
-    for (size_t characteristic_idx = 0; characteristic_idx < self->characteristic_list->len; ++characteristic_idx) {
-        bleio_characteristic_obj_t *characteristic =
-            MP_OBJ_TO_PTR(self->characteristic_list->items[characteristic_idx]);
-
-        if (characteristic->handle != BLE_GATT_HANDLE_INVALID) {
-            mp_raise_ValueError(translate("Characteristic already in use by another Service."));
-        }
-
-        ble_gatts_char_md_t char_md = {
-            .char_props.broadcast      = (characteristic->props & CHAR_PROP_BROADCAST) ? 1 : 0,
-            .char_props.read           = (characteristic->props & CHAR_PROP_READ) ? 1 : 0,
-            .char_props.write_wo_resp  = (characteristic->props & CHAR_PROP_WRITE_NO_RESPONSE) ? 1 : 0,
-            .char_props.write          = (characteristic->props & CHAR_PROP_WRITE) ? 1 : 0,
-            .char_props.notify         = (characteristic->props & CHAR_PROP_NOTIFY) ? 1 : 0,
-            .char_props.indicate       = (characteristic->props & CHAR_PROP_INDICATE) ? 1 : 0,
-        };
-
-        ble_gatts_attr_md_t cccd_md = {
-            .vloc = BLE_GATTS_VLOC_STACK,
-        };
-
-        ble_uuid_t char_uuid;
-        bleio_uuid_convert_to_nrf_ble_uuid(characteristic->uuid, &char_uuid);
-
-        ble_gatts_attr_md_t char_attr_md = {
-            .vloc = BLE_GATTS_VLOC_STACK,
-            .vlen = !characteristic->fixed_length,
-        };
-
-        if (char_md.char_props.notify || char_md.char_props.indicate) {
-            BLE_GAP_CONN_SEC_MODE_SET_OPEN(&cccd_md.read_perm);
-            // Make CCCD write permission match characteristic read permission.
-            bleio_attribute_gatts_set_security_mode(&cccd_md.write_perm, characteristic->read_perm);
-
-            char_md.p_cccd_md = &cccd_md;
-        }
-
-        bleio_attribute_gatts_set_security_mode(&char_attr_md.read_perm, characteristic->read_perm);
-        bleio_attribute_gatts_set_security_mode(&char_attr_md.write_perm, characteristic->write_perm);
-
-        mp_buffer_info_t char_value_bufinfo;
-        mp_get_buffer_raise(characteristic->value, &char_value_bufinfo, MP_BUFFER_READ);
-
-        ble_gatts_attr_t char_attr = {
-            .p_uuid = &char_uuid,
-            .p_attr_md = &char_attr_md,
-            .init_len = char_value_bufinfo.len,
-            .p_value = char_value_bufinfo.buf,
-            .init_offs = 0,
-            .max_len = characteristic->max_length,
-        };
-
-        ble_gatts_char_handles_t char_handles;
-
-        uint32_t err_code;
-        err_code = sd_ble_gatts_characteristic_add(self->handle, &char_md, &char_attr, &char_handles);
-        if (err_code != NRF_SUCCESS) {
-            mp_raise_OSError_msg_varg(translate("Failed to add characteristic, err 0x%04x"), err_code);
-        }
-
-        characteristic->user_desc_handle = char_handles.user_desc_handle;
-        characteristic->cccd_handle = char_handles.cccd_handle;
-        characteristic->sccd_handle = char_handles.sccd_handle;
-        characteristic->handle = char_handles.value_handle;
-
-        // Add the descriptors for this characteristic.
-        for (size_t descriptor_idx = 0; descriptor_idx < characteristic->descriptor_list->len; ++descriptor_idx) {
-            bleio_descriptor_obj_t *descriptor =
-                MP_OBJ_TO_PTR(characteristic->descriptor_list->items[descriptor_idx]);
-
-            ble_uuid_t desc_uuid;
-            bleio_uuid_convert_to_nrf_ble_uuid(descriptor->uuid, &desc_uuid);
-
-            ble_gatts_attr_md_t desc_attr_md = {
-                // Data passed is not in a permanent location and should be copied.
-                .vloc = BLE_GATTS_VLOC_STACK,
-                .vlen = !descriptor->fixed_length,
-            };
-
-            bleio_attribute_gatts_set_security_mode(&desc_attr_md.read_perm, descriptor->read_perm);
-            bleio_attribute_gatts_set_security_mode(&desc_attr_md.write_perm, descriptor->write_perm);
-
-            mp_buffer_info_t desc_value_bufinfo;
-            mp_get_buffer_raise(descriptor->value, &desc_value_bufinfo, MP_BUFFER_READ);
-
-            ble_gatts_attr_t desc_attr = {
-                .p_uuid = &desc_uuid,
-                .p_attr_md = &desc_attr_md,
-                .init_len = desc_value_bufinfo.len,
-                .p_value = desc_value_bufinfo.buf,
-                .init_offs = 0,
-                .max_len = descriptor->max_length,
-            };
-
-            err_code = sd_ble_gatts_descriptor_add(characteristic->handle, &desc_attr, &descriptor->handle);
-
-        } // loop over descriptors
-
-    } // loop over characteristics
+
+void common_hal_bleio_service_add_characteristic(bleio_service_obj_t *self, bleio_characteristic_obj_t *characteristic) {
+    // Connect characteristic to parent service.
+    characteristic->service = self;
+
+    ble_gatts_char_md_t char_md = {
+        .char_props.broadcast      = (characteristic->props & CHAR_PROP_BROADCAST) ? 1 : 0,
+        .char_props.read           = (characteristic->props & CHAR_PROP_READ) ? 1 : 0,
+        .char_props.write_wo_resp  = (characteristic->props & CHAR_PROP_WRITE_NO_RESPONSE) ? 1 : 0,
+        .char_props.write          = (characteristic->props & CHAR_PROP_WRITE) ? 1 : 0,
+        .char_props.notify         = (characteristic->props & CHAR_PROP_NOTIFY) ? 1 : 0,
+        .char_props.indicate       = (characteristic->props & CHAR_PROP_INDICATE) ? 1 : 0,
+    };
+
+    ble_gatts_attr_md_t cccd_md = {
+        .vloc = BLE_GATTS_VLOC_STACK,
+    };
+
+    ble_uuid_t char_uuid;
+    bleio_uuid_convert_to_nrf_ble_uuid(characteristic->uuid, &char_uuid);
+
+    ble_gatts_attr_md_t char_attr_md = {
+        .vloc = BLE_GATTS_VLOC_STACK,
+        .vlen = !characteristic->fixed_length,
+    };
+
+    if (char_md.char_props.notify || char_md.char_props.indicate) {
+        BLE_GAP_CONN_SEC_MODE_SET_OPEN(&cccd_md.read_perm);
+        // Make CCCD write permission match characteristic read permission.
+        bleio_attribute_gatts_set_security_mode(&cccd_md.write_perm, characteristic->read_perm);
+
+        char_md.p_cccd_md = &cccd_md;
+    }
+
+    bleio_attribute_gatts_set_security_mode(&char_attr_md.read_perm, characteristic->read_perm);
+    bleio_attribute_gatts_set_security_mode(&char_attr_md.write_perm, characteristic->write_perm);
+
+    mp_buffer_info_t char_value_bufinfo;
+    mp_get_buffer_raise(characteristic->value, &char_value_bufinfo, MP_BUFFER_READ);
+
+    ble_gatts_attr_t char_attr = {
+        .p_uuid = &char_uuid,
+        .p_attr_md = &char_attr_md,
+        .init_len = char_value_bufinfo.len,
+        .p_value = char_value_bufinfo.buf,
+        .init_offs = 0,
+        .max_len = characteristic->max_length,
+    };
+
+    ble_gatts_char_handles_t char_handles;
+
+    uint32_t err_code;
+    err_code = sd_ble_gatts_characteristic_add(self->handle, &char_md, &char_attr, &char_handles);
+    if (err_code != NRF_SUCCESS) {
+        mp_raise_OSError_msg_varg(translate("Failed to add characteristic, err 0x%04x"), err_code);
+    }
+
+    characteristic->user_desc_handle = char_handles.user_desc_handle;
+    characteristic->cccd_handle = char_handles.cccd_handle;
+    characteristic->sccd_handle = char_handles.sccd_handle;
+    characteristic->handle = char_handles.value_handle;
+
+    mp_obj_list_append(self->characteristic_list, MP_OBJ_FROM_PTR(characteristic));
 }
diff --git a/ports/nrf/common-hal/bleio/__init__.c b/ports/nrf/common-hal/bleio/__init__.c
index 2e26ac07d..868e0a8e9 100644
--- a/ports/nrf/common-hal/bleio/__init__.c
+++ b/ports/nrf/common-hal/bleio/__init__.c
@@ -75,11 +75,11 @@ uint16_t common_hal_bleio_device_get_conn_handle(mp_obj_t device) {
     }
 }
 
-mp_obj_list_t *common_hal_bleio_device_get_remote_services_list(mp_obj_t device) {
+mp_obj_list_t *common_hal_bleio_device_get_remote_service_list(mp_obj_t device) {
     if (MP_OBJ_IS_TYPE(device, &bleio_peripheral_type)) {
-        return ((bleio_peripheral_obj_t*) MP_OBJ_TO_PTR(device))->remote_services_list;
+        return ((bleio_peripheral_obj_t*) MP_OBJ_TO_PTR(device))->remote_service_list;
     } else if (MP_OBJ_IS_TYPE(device, &bleio_central_type)) {
-        return ((bleio_central_obj_t*) MP_OBJ_TO_PTR(device))->remote_services_list;
+        return ((bleio_central_obj_t*) MP_OBJ_TO_PTR(device))->remote_service_list;
     } else {
         return NULL;
     }
@@ -158,7 +158,7 @@ STATIC void on_primary_srv_discovery_rsp(ble_gattc_evt_prim_srvc_disc_rsp_t *res
         service->base.type = &bleio_service_type;
 
         // Initialize several fields at once.
-        common_hal_bleio_service_construct(service, NULL, mp_obj_new_list(0, NULL), false);
+        common_hal_bleio_service_construct(service, NULL, false);
 
         service->device = device;
         service->is_remote = true;
@@ -179,7 +179,7 @@ STATIC void on_primary_srv_discovery_rsp(ble_gattc_evt_prim_srvc_disc_rsp_t *res
             service->uuid = NULL;
         }
 
-        mp_obj_list_append(common_hal_bleio_device_get_remote_services_list(device), service);
+        mp_obj_list_append(common_hal_bleio_device_get_remote_service_list(device), service);
     }
 
     if (response->count > 0) {
@@ -275,7 +275,7 @@ STATIC void on_desc_discovery_rsp(ble_gattc_evt_desc_disc_rsp_t *response, mp_ob
         }
 
         common_hal_bleio_descriptor_construct(descriptor, uuid, SECURITY_MODE_OPEN, SECURITY_MODE_OPEN,
-                                              GATT_MAX_DATA_LENGTH, false);
+                                              GATT_MAX_DATA_LENGTH, false, mp_const_empty_bytes);
         descriptor->handle = gattc_desc->handle;
         descriptor->characteristic = m_desc_discovery_characteristic;
 
@@ -316,12 +316,12 @@ STATIC void discovery_on_ble_evt(ble_evt_t *ble_evt, mp_obj_t device) {
 
 
 void common_hal_bleio_device_discover_remote_services(mp_obj_t device, mp_obj_t service_uuids_whitelist) {
-    mp_obj_list_t *remote_services_list = common_hal_bleio_device_get_remote_services_list(device);
+    mp_obj_list_t *remote_services_list = common_hal_bleio_device_get_remote_service_list(device);
 
     ble_drv_add_event_handler(discovery_on_ble_evt, device);
 
     // Start over with an empty list.
-    mp_obj_list_clear(MP_OBJ_FROM_PTR(common_hal_bleio_device_get_remote_services_list(device)));
+    mp_obj_list_clear(MP_OBJ_FROM_PTR(common_hal_bleio_device_get_remote_service_list(device)));
 
     if (service_uuids_whitelist == mp_const_none) {
         // List of service UUID's not given, so discover all available services.