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

9 files changed, 267 insertions, 260 deletions

diff --git a/shared-bindings/bleio/Characteristic.c b/shared-bindings/bleio/Characteristic.c
index 4894ad114..d9d920b76 100644
--- a/shared-bindings/bleio/Characteristic.c
+++ b/shared-bindings/bleio/Characteristic.c
@@ -41,101 +41,18 @@
 //| Stores information about a BLE service characteristic and allows reading
 //| and writing of the characteristic's value.
 //|
+//| A Characteristic cannot be created directly. A new local Characteristic can be created
+//| and attached to a Service by calling `Service.add_characteristic()`.
+//| Remote Characteristic objects are created by `Central.discover_remote_services()`
+//| or `Peripheral.discover_remote_services()` as part of remote Services.
 //|
-//| .. class:: Characteristic(uuid, *, properties=0, read_perm=`Attribute.OPEN`, write_perm=`Attribute.OPEN`, max_length=20, fixed_length=False, descriptors=None)
-//|
-//|   Create a new Characteristic object identified by the specified UUID.
-//|
-//|   :param bleio.UUID uuid: The uuid of the characteristic
-//|   :param int properties: bitmask of these values bitwise-or'd together: `BROADCAST`, `INDICATE`,
-//|     `NOTIFY`, `READ`, `WRITE`, `WRITE_NO_RESPONSE`
-//|   :param int read_perm: Specifies whether the characteristic can be read by a client, and if so, which
-//|      security mode is required. Must be one of the integer values `Attribute.NO_ACCESS`, `Attribute.OPEN`,
-//|      `Attribute.ENCRYPT_NO_MITM`, `Attribute.ENCRYPT_WITH_MITM`, `Attribute.LESC_ENCRYPT_WITH_MITM`,
-//|      `Attribute.SIGNED_NO_MITM`, or `Attribute.SIGNED_WITH_MITM`.
-//|   :param int write_perm: Specifies whether the characteristic can be written by a client, and if so, which
-//|      security mode is required. Values allowed are the same as ``read_perm``.
-//|   :param int max_length: Maximum length in bytes of the characteristic value. The maximum allowed is
-//|      is 512, or possibly 510 if ``fixed_length`` is False. The default, 20, is the maximum
-//|      number of data bytes that fit in a single BLE 4.x ATT packet.
-//|   :param bool fixed_length: True if the characteristic value is of fixed length.
-//|   :param iterable descriptors: BLE descriptors for this characteristic.
-//|
-STATIC mp_obj_t bleio_characteristic_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_uuid, ARG_properties, ARG_read_perm, ARG_write_perm,
-           ARG_max_length, ARG_fixed_length, ARG_descriptors };
-    static const mp_arg_t allowed_args[] = {
-        { MP_QSTR_uuid,  MP_ARG_REQUIRED | MP_ARG_OBJ, {.u_obj = mp_const_none} },
-        { MP_QSTR_properties, MP_ARG_KW_ONLY| MP_ARG_INT, {.u_int = 0} },
-        { MP_QSTR_read_perm, MP_ARG_KW_ONLY| MP_ARG_INT, {.u_int = SECURITY_MODE_OPEN} },
-        { MP_QSTR_write_perm, MP_ARG_KW_ONLY| MP_ARG_INT, {.u_int = SECURITY_MODE_OPEN} },
-        { MP_QSTR_max_length, MP_ARG_KW_ONLY| MP_ARG_INT, {.u_int = 20} },
-        { MP_QSTR_fixed_length, MP_ARG_KW_ONLY| MP_ARG_BOOL, {.u_bool = false} },
-        { MP_QSTR_descriptors, MP_ARG_KW_ONLY| MP_ARG_OBJ, {.u_obj = mp_const_none} },
-    };
-
-    mp_arg_val_t args[MP_ARRAY_SIZE(allowed_args)];
-    mp_arg_parse_all(n_args, pos_args, kw_args, MP_ARRAY_SIZE(allowed_args), allowed_args, args);
-
-    const mp_obj_t uuid_obj = args[ARG_uuid].u_obj;
-
-    if (!MP_OBJ_IS_TYPE(uuid_obj, &bleio_uuid_type)) {
-        mp_raise_ValueError(translate("Expected a UUID"));
-    }
-    bleio_uuid_obj_t *uuid = MP_OBJ_TO_PTR(uuid_obj);
-
-    const bleio_characteristic_properties_t properties = args[ARG_properties].u_int;
-    if (properties & ~CHAR_PROP_ALL) {
-        mp_raise_ValueError(translate("Invalid properties"));
-    }
-
-    const bleio_attribute_security_mode_t read_perm = args[ARG_read_perm].u_int;
-    common_hal_bleio_attribute_security_mode_check_valid(read_perm);
-
-    const bleio_attribute_security_mode_t write_perm = args[ARG_write_perm].u_int;
-    common_hal_bleio_attribute_security_mode_check_valid(write_perm);
-
-    mp_obj_t descriptors = args[ARG_descriptors].u_obj;
-    if (descriptors == mp_const_none) {
-        descriptors = mp_const_empty_tuple;
-    }
-
-    bleio_characteristic_obj_t *self = m_new_obj(bleio_characteristic_obj_t);
-    self->base.type = &bleio_characteristic_type;
-
-    // Copy the descriptors list and validate its items.
-    mp_obj_t desc_list_obj = mp_obj_new_list(0, NULL);
-    mp_obj_list_t *desc_list = MP_OBJ_TO_PTR(desc_list_obj);
-
-    // If descriptors is not an iterable, an exception will be thrown.
-    mp_obj_iter_buf_t iter_buf;
-    mp_obj_t descriptors_iter = mp_getiter(descriptors, &iter_buf);
-
-    mp_obj_t descriptor_obj;
-    while ((descriptor_obj = mp_iternext(descriptors_iter)) != MP_OBJ_STOP_ITERATION) {
-        if (!MP_OBJ_IS_TYPE(descriptor_obj, &bleio_descriptor_type)) {
-            mp_raise_ValueError(translate("descriptors includes an object that is not a Descriptors"));
-        }
-        bleio_descriptor_obj_t *descriptor = MP_OBJ_TO_PTR(descriptor_obj);
-        if (common_hal_bleio_descriptor_get_characteristic(descriptor) != MP_OBJ_NULL) {
-            mp_raise_ValueError(translate("Descriptor is already attached to a Characteristic"));
-        }
-        mp_obj_list_append(desc_list_obj, descriptor_obj);
-    }
-
-    // Range checking on max_length arg is done by the common_hal layer, because
-    // it may vary depending on underlying BLE implementation.
-    common_hal_bleio_characteristic_construct(self, uuid, properties,
-                                              read_perm, write_perm,
-                                              args[ARG_max_length].u_int, args[ARG_fixed_length].u_bool,
-                                              desc_list);
-
-    return MP_OBJ_FROM_PTR(self);
-}
 
 //|   .. attribute:: properties
 //|
-//|     An int bitmask representing which properties are set.
+//|     An int bitmask representing which properties are set, specified as bitwise or'ing of
+//|     of these possible values.
+//|      `~Characteristic.BROADCAST`, `~Characteristic.INDICATE`, `~Characteristic.NOTIFY`,
+//|      `~Characteristic.READ`, `~Characteristic.WRITE`, `~Characteristic.WRITE_NO_RESPONSE`.
 //|
 STATIC mp_obj_t bleio_characteristic_get_properties(mp_obj_t self_in) {
     bleio_characteristic_obj_t *self = MP_OBJ_TO_PTR(self_in);
@@ -222,7 +139,7 @@ const mp_obj_property_t bleio_characteristic_descriptors_obj = {
 
 //|   .. attribute:: service (read-only)
 //|
-//|     The Service this Characteristic is a part of. None if not yet assigned to a Service.
+//|     The Service this Characteristic is a part of.
 //|
 STATIC mp_obj_t bleio_characteristic_get_service(mp_obj_t self_in) {
     bleio_characteristic_obj_t *self = MP_OBJ_TO_PTR(self_in);
@@ -238,6 +155,84 @@ const mp_obj_property_t bleio_characteristic_service_obj = {
                (mp_obj_t)&mp_const_none_obj },
 };
 
+//|   .. method:: add_descriptor(uuid, *, read_perm=`Attribute.OPEN`, write_perm=`Attribute.OPEN`, max_length=20, fixed_length=False, initial_value=b'')
+//|
+//|   Create a new `Descriptor` object, and add it to this Service.
+//|
+//|   :param bleio.UUID uuid: The uuid of the descriptor
+//|   :param int read_perm: Specifies whether the descriptor can be read by a client, and if so, which
+//|      security mode is required. Must be one of the integer values `Attribute.NO_ACCESS`, `Attribute.OPEN`,
+//|      `Attribute.ENCRYPT_NO_MITM`, `Attribute.ENCRYPT_WITH_MITM`, `Attribute.LESC_ENCRYPT_WITH_MITM`,
+//|      `Attribute.SIGNED_NO_MITM`, or `Attribute.SIGNED_WITH_MITM`.
+//|   :param int write_perm: Specifies whether the descriptor can be written by a client, and if so, which
+//|      security mode is required. Values allowed are the same as ``read_perm``.
+//|   :param int max_length: Maximum length in bytes of the descriptor value. The maximum allowed is
+//|      is 512, or possibly 510 if ``fixed_length`` is False. The default, 20, is the maximum
+//|      number of data bytes that fit in a single BLE 4.x ATT packet.
+//|   :param bool fixed_length: True if the descriptor value is of fixed length.
+//|   :param buf initial_value: The initial value for this descriptor.
+//|
+//|   :return: the new `Descriptor`.
+//|
+STATIC mp_obj_t bleio_characteristic_add_descriptor(size_t n_args, const mp_obj_t *pos_args, mp_map_t *kw_args) {
+    bleio_characteristic_obj_t *self = MP_OBJ_TO_PTR(pos_args[0]);
+
+    enum { ARG_uuid, ARG_read_perm, ARG_write_perm,
+           ARG_max_length, ARG_fixed_length, ARG_initial_value };
+    static const mp_arg_t allowed_args[] = {
+        { MP_QSTR_uuid,  MP_ARG_REQUIRED | MP_ARG_OBJ },
+        { MP_QSTR_read_perm, MP_ARG_KW_ONLY| MP_ARG_INT, {.u_int = SECURITY_MODE_OPEN} },
+        { MP_QSTR_write_perm, MP_ARG_KW_ONLY| MP_ARG_INT, {.u_int = SECURITY_MODE_OPEN} },
+        { MP_QSTR_max_length, MP_ARG_KW_ONLY| MP_ARG_INT, {.u_int = 20} },
+        { MP_QSTR_fixed_length, MP_ARG_KW_ONLY| MP_ARG_BOOL, {.u_bool = false} },
+        { MP_QSTR_initial_value, MP_ARG_KW_ONLY| MP_ARG_OBJ, {.u_obj = mp_const_empty_bytes} },
+    };
+
+    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);
+
+    const mp_obj_t uuid_obj = args[ARG_uuid].u_obj;
+
+    if (!MP_OBJ_IS_TYPE(uuid_obj, &bleio_uuid_type)) {
+        mp_raise_ValueError(translate("Expected a UUID"));
+    }
+    bleio_uuid_obj_t *uuid = MP_OBJ_TO_PTR(uuid_obj);
+
+    const bleio_attribute_security_mode_t read_perm = args[ARG_read_perm].u_int;
+    common_hal_bleio_attribute_security_mode_check_valid(read_perm);
+
+    const bleio_attribute_security_mode_t write_perm = args[ARG_write_perm].u_int;
+    common_hal_bleio_attribute_security_mode_check_valid(write_perm);
+
+    const mp_int_t max_length = args[ARG_max_length].u_int;
+    const bool fixed_length =  args[ARG_fixed_length].u_bool;
+    mp_obj_t initial_value = args[ARG_initial_value].u_obj;
+
+    // Length will be validated in common_hal.
+    mp_buffer_info_t initial_value_bufinfo;
+    if (initial_value == mp_const_none) {
+        if (fixed_length && max_length > 0) {
+            initial_value = mp_obj_new_bytes_of_zeros(max_length);
+        } else {
+            initial_value = mp_const_empty_bytes;
+        }
+    }
+    mp_get_buffer_raise(initial_value, &initial_value_bufinfo, MP_BUFFER_READ);
+
+    bleio_descriptor_obj_t *descriptor = m_new_obj(bleio_descriptor_obj_t);
+    descriptor->base.type = &bleio_descriptor_type;
+
+    // Range checking on max_length arg is done by the common_hal layer, because
+    // it may vary depending on underlying BLE implementation.
+    common_hal_bleio_descriptor_construct(
+        descriptor, uuid, read_perm, write_perm, max_length, fixed_length, &initial_value_bufinfo);
+
+    common_hal_bleio_characteristic_add_descriptor(self, descriptor);
+
+    return MP_OBJ_FROM_PTR(descriptor);
+}
+STATIC MP_DEFINE_CONST_FUN_OBJ_KW(bleio_characteristic_add_descriptor_obj, 2, bleio_characteristic_add_descriptor);
+
 //|   .. method:: set_cccd(*, notify=False, indicate=False)
 //|
 //|     Set the remote characteristic's CCCD to enable or disable notification and indication.
@@ -265,10 +260,11 @@ STATIC MP_DEFINE_CONST_FUN_OBJ_KW(bleio_characteristic_set_cccd_obj, 1, bleio_ch
 
 
 STATIC const mp_rom_map_elem_t bleio_characteristic_locals_dict_table[] = {
-    { MP_ROM_QSTR(MP_QSTR_properties), MP_ROM_PTR(&bleio_characteristic_get_properties) },
-    { MP_ROM_QSTR(MP_QSTR_set_cccd),   MP_ROM_PTR(&bleio_characteristic_set_cccd_obj) },
-    { MP_ROM_QSTR(MP_QSTR_uuid),       MP_ROM_PTR(&bleio_characteristic_uuid_obj) },
-    { MP_ROM_QSTR(MP_QSTR_value),      MP_ROM_PTR(&bleio_characteristic_value_obj) },
+    { MP_ROM_QSTR(MP_QSTR_properties),     MP_ROM_PTR(&bleio_characteristic_get_properties) },
+    { MP_ROM_QSTR(MP_QSTR_uuid),           MP_ROM_PTR(&bleio_characteristic_uuid_obj) },
+    { MP_ROM_QSTR(MP_QSTR_value),          MP_ROM_PTR(&bleio_characteristic_value_obj) },
+    { MP_ROM_QSTR(MP_QSTR_add_descriptor), MP_ROM_PTR(&bleio_characteristic_add_descriptor_obj) },
+    { MP_ROM_QSTR(MP_QSTR_set_cccd),       MP_ROM_PTR(&bleio_characteristic_set_cccd_obj) },
 
     // Bitmask constants to represent properties
 //|   .. data:: BROADCAST
@@ -319,7 +315,6 @@ STATIC void bleio_characteristic_print(const mp_print_t *print, mp_obj_t self_in
 const mp_obj_type_t bleio_characteristic_type = {
     { &mp_type_type },
     .name = MP_QSTR_Characteristic,
-    .make_new = bleio_characteristic_make_new,
     .print = bleio_characteristic_print,
     .locals_dict = (mp_obj_dict_t*)&bleio_characteristic_locals_dict,
 };
diff --git a/shared-bindings/bleio/Characteristic.h b/shared-bindings/bleio/Characteristic.h
index 87942c8e9..d9d980b33 100644
--- a/shared-bindings/bleio/Characteristic.h
+++ b/shared-bindings/bleio/Characteristic.h
@@ -29,18 +29,20 @@
 #define MICROPY_INCLUDED_SHARED_BINDINGS_BLEIO_CHARACTERISTIC_H
 
 #include "shared-bindings/bleio/Attribute.h"
+#include "shared-bindings/bleio/Descriptor.h"
 #include "shared-module/bleio/Characteristic.h"
 #include "common-hal/bleio/Characteristic.h"
 
 extern const mp_obj_type_t bleio_characteristic_type;
 
-extern 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);
+extern 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);
 extern mp_obj_t common_hal_bleio_characteristic_get_value(bleio_characteristic_obj_t *self);
 extern void common_hal_bleio_characteristic_set_value(bleio_characteristic_obj_t *self, mp_buffer_info_t *bufinfo);
 extern bleio_characteristic_properties_t common_hal_bleio_characteristic_get_properties(bleio_characteristic_obj_t *self);
 extern bleio_uuid_obj_t *common_hal_bleio_characteristic_get_uuid(bleio_characteristic_obj_t *self);
 extern mp_obj_list_t *common_hal_bleio_characteristic_get_descriptor_list(bleio_characteristic_obj_t *self);
 extern bleio_service_obj_t *common_hal_bleio_characteristic_get_service(bleio_characteristic_obj_t *self);
+extern void common_hal_bleio_characteristic_add_descriptor(bleio_characteristic_obj_t *self, bleio_descriptor_obj_t *descriptor);
 extern void common_hal_bleio_characteristic_set_cccd(bleio_characteristic_obj_t *self, bool notify, bool indicate);
 
 #endif // MICROPY_INCLUDED_SHARED_BINDINGS_BLEIO_CHARACTERISTIC_H
diff --git a/shared-bindings/bleio/Descriptor.c b/shared-bindings/bleio/Descriptor.c
index d7c2a85c7..db8d393d1 100644
--- a/shared-bindings/bleio/Descriptor.c
+++ b/shared-bindings/bleio/Descriptor.c
@@ -38,62 +38,14 @@
 //| =========================================================
 //|
 //| Stores information about a BLE descriptor.
-//| Descriptors are encapsulated by BLE characteristics and provide contextual
+//| Descriptors are attached to BLE characteristics and provide contextual
 //| information about the characteristic.
 //|
-
-//| .. class:: Descriptor(uuid, *, read_perm=`Attribute.OPEN`, write_perm=`Attribute.OPEN`)
-//|
-//|   Create a new descriptor object with the UUID uuid
-//|
-//|   :param bleio.UUID uuid: The uuid of the descriptor
-//|   :param int read_perm: Specifies whether the descriptor can be read by a client, and if so, which
-//|      security mode is required. Must be one of the integer values `Attribute.NO_ACCESS`, `Attribute.OPEN`,
-//|      `Attribute.ENCRYPT_NO_MITM`, `Attribute.ENCRYPT_WITH_MITM`, `Attribute.LESC_ENCRYPT_WITH_MITM`,
-//|      `Attribute.SIGNED_NO_MITM`, or `Attribute.SIGNED_WITH_MITM`.
-//|   :param int write_perm: Specifies whether the descriptor can be written by a client, and if so, which
-//|      security mode is required. Values allowed are the same as ``read_perm``.
-//|   :param int max_length: Maximum length in bytes of the characteristic value. The maximum allowed is
-//|      is 512, or possibly 510 if ``fixed_length`` is False. The default, 20, is the maximum
-//|      number of data bytes that fit in a single BLE 4.x ATT packet.
-//|   :param bool fixed_length: True if the characteristic value is of fixed length.
-//|
-STATIC mp_obj_t bleio_descriptor_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_uuid, ARG_read_perm, ARG_write_perm, ARG_max_length, ARG_fixed_length };
-    static const mp_arg_t allowed_args[] = {
-        { MP_QSTR_uuid, MP_ARG_REQUIRED | MP_ARG_OBJ },
-        { MP_QSTR_read_perm, MP_ARG_KW_ONLY| MP_ARG_INT, {.u_int = SECURITY_MODE_OPEN } },
-        { MP_QSTR_write_perm, MP_ARG_KW_ONLY| MP_ARG_INT, {.u_int = SECURITY_MODE_OPEN } },
-        { MP_QSTR_max_length, MP_ARG_KW_ONLY| MP_ARG_INT, {.u_int = 20} },
-        { MP_QSTR_fixed_length, MP_ARG_KW_ONLY| MP_ARG_BOOL, {.u_bool = false} },
-    };
-
-    mp_arg_val_t args[MP_ARRAY_SIZE(allowed_args)];
-    mp_arg_parse_all(n_args, pos_args, kw_args, MP_ARRAY_SIZE(allowed_args), allowed_args, args);
-
-    const mp_obj_t uuid_arg = args[ARG_uuid].u_obj;
-
-    if (!MP_OBJ_IS_TYPE(uuid_arg, &bleio_uuid_type)) {
-        mp_raise_ValueError(translate("Expected a UUID"));
-    }
-
-    const bleio_attribute_security_mode_t read_perm = args[ARG_read_perm].u_int;
-    common_hal_bleio_attribute_security_mode_check_valid(read_perm);
-
-    const bleio_attribute_security_mode_t write_perm = args[ARG_write_perm].u_int;
-    common_hal_bleio_attribute_security_mode_check_valid(write_perm);
-
-    bleio_descriptor_obj_t *self = m_new_obj(bleio_descriptor_obj_t);
-    self->base.type = type;
-    bleio_uuid_obj_t *uuid = MP_OBJ_TO_PTR(uuid_arg);
-
-    // Range checking on max_length arg is done by the common_hal layer, because
-    // it may vary depending on underlying BLE implementation.
-    common_hal_bleio_descriptor_construct(self, uuid, read_perm, write_perm,
-                                          args[ARG_max_length].u_int, args[ARG_fixed_length].u_bool);
-
-    return MP_OBJ_FROM_PTR(self);
-}
+//| A Descriptors cannot be created directly. A new local Descriptors can be created
+//| and attached to a Descriptors by calling `Service.add_characteristic()`.
+//| Remote Descriptor objects are created by `Central.discover_remote_services()`
+//| or `Peripheral.discover_remote_services()` as part of remote Characteristics
+//| in the remote Services that are discovered.
 
 //|   .. attribute:: uuid
 //|
@@ -116,7 +68,7 @@ const mp_obj_property_t bleio_descriptor_uuid_obj = {
 
 //|   .. attribute:: characteristic (read-only)
 //|
-//|     The Characteristic this Descriptor is a part of. None if not yet assigned to a Characteristic.
+//|     The Characteristic this Descriptor is a part of.
 //|
 STATIC mp_obj_t bleio_descriptor_get_characteristic(mp_obj_t self_in) {
     bleio_descriptor_obj_t *self = MP_OBJ_TO_PTR(self_in);
@@ -185,7 +137,6 @@ STATIC void bleio_descriptor_print(const mp_print_t *print, mp_obj_t self_in, mp
 const mp_obj_type_t bleio_descriptor_type = {
     { &mp_type_type },
     .name = MP_QSTR_Descriptor,
-    .make_new = bleio_descriptor_make_new,
     .print = bleio_descriptor_print,
     .locals_dict = (mp_obj_dict_t*)&bleio_descriptor_locals_dict
 };
diff --git a/shared-bindings/bleio/Descriptor.h b/shared-bindings/bleio/Descriptor.h
index 430c0d0b6..c6d49c06d 100644
--- a/shared-bindings/bleio/Descriptor.h
+++ b/shared-bindings/bleio/Descriptor.h
@@ -29,12 +29,13 @@
 #define MICROPY_INCLUDED_SHARED_BINDINGS_BLEIO_DESCRIPTOR_H
 
 #include "shared-module/bleio/Attribute.h"
+#include "common-hal/bleio/Characteristic.h"
 #include "common-hal/bleio/Descriptor.h"
 #include "common-hal/bleio/UUID.h"
 
 extern const mp_obj_type_t bleio_descriptor_type;
 
-extern 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);
+extern 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);
 extern bleio_uuid_obj_t *common_hal_bleio_descriptor_get_uuid(bleio_descriptor_obj_t *self);
 extern bleio_characteristic_obj_t *common_hal_bleio_descriptor_get_characteristic(bleio_descriptor_obj_t *self);
 extern mp_obj_t common_hal_bleio_descriptor_get_value(bleio_descriptor_obj_t *self);
diff --git a/shared-bindings/bleio/Peripheral.c b/shared-bindings/bleio/Peripheral.c
index b98e7f74b..3a78802ac 100644
--- a/shared-bindings/bleio/Peripheral.c
+++ b/shared-bindings/bleio/Peripheral.c
@@ -64,71 +64,98 @@
 //|    import bleio
 //|    from adafruit_ble.advertising import ServerAdvertisement
 //|
-//|    # Create a Characteristic.
-//|    chara = bleio.Characteristic(bleio.UUID(0x2919), read=True, notify=True)
+//|    # Create a peripheral and start it up.
+//|    peripheral = bleio.Peripheral()
 //|
-//|    # Create a Service providing that one Characteristic.
-//|    serv = bleio.Service(bleio.UUID(0x180f), [chara])
+//|    # Create a Service and add it to this Peripheral.
+//|    service = peripheral.addService(bleio.UUID(0x180f))
 //|
-//|    # Create a peripheral and start it up.
-//|    periph = bleio.Peripheral([serv])
-//|    adv = ServerAdvertisement(periph)
-//|    periph.start_advertising(adv.advertising_data_bytes, adv.scan_response_bytes)
+//|    # Create a Characteristic and add it to the Service.
+//|    characteristic = service.addCharacteristic(
+//|        bleio.UUID(0x2919), properties=Characteristic.READ | Characteristic.NOTIFY)
 //|
-//|    while not periph.connected:
+//|    adv = ServerAdvertisement(peripheral)
+//|    peripheral.start_advertising(adv.advertising_data_bytes, adv.scan_response_bytes)
+//|
+//|    while not peripheral.connected:
 //|        # Wait for connection.
 //|        pass
 //|
-//| .. class:: Peripheral(services=(), \*, name=None)
+//| .. class:: Peripheral(name=None)
 //|
 //|   Create a new Peripheral object.
 //|
-//|   :param iterable services: the Service objects representing services available from this peripheral, if any.
-//|     A non-connectable peripheral will have no services.
 //|   :param str name: The name used when advertising this peripheral. If name is None,
 //|     bleio.adapter.default_name will be used.
 //|
 STATIC mp_obj_t bleio_peripheral_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_services, ARG_name };
+    enum { ARG_name };
     static const mp_arg_t allowed_args[] = {
-        { MP_QSTR_services, MP_ARG_OBJ, {.u_obj = mp_const_empty_tuple} },
-        { MP_QSTR_name, MP_ARG_OBJ | MP_ARG_KW_ONLY, {.u_obj = MP_OBJ_NULL} },
+        { MP_QSTR_name, MP_ARG_OBJ, {.u_obj = mp_const_none} },
     };
 
     mp_arg_val_t args[MP_ARRAY_SIZE(allowed_args)];
     mp_arg_parse_all(n_args, pos_args, kw_args, MP_ARRAY_SIZE(allowed_args), allowed_args, args);
 
-    // If services is not an iterable, an exception will be thrown.
-    mp_obj_iter_buf_t iter_buf;
-    mp_obj_t iterable = mp_getiter(args[ARG_services].u_obj, &iter_buf);
-
     bleio_peripheral_obj_t *self = m_new_obj(bleio_peripheral_obj_t);
     self->base.type = &bleio_peripheral_type;
 
-    // Copy the services list and validate its items.
-    mp_obj_t services_list_obj = mp_obj_new_list(0, NULL);
-    mp_obj_list_t *services_list = MP_OBJ_FROM_PTR(services_list_obj);
-
-    mp_obj_t service;
-    while ((service = mp_iternext(iterable)) != MP_OBJ_STOP_ITERATION) {
-        if (!MP_OBJ_IS_TYPE(service, &bleio_service_type)) {
-            mp_raise_ValueError(translate("non-Service found in services"));
-        }
-        mp_obj_list_append(services_list, service);
-    }
-
     mp_obj_t name = args[ARG_name].u_obj;
-    if (name == MP_OBJ_NULL || name == mp_const_none) {
+    if (name == mp_const_none) {
         name = common_hal_bleio_adapter_get_default_name();
     } else if (!MP_OBJ_IS_STR(name)) {
         mp_raise_ValueError(translate("name must be a string"));
     }
 
-    common_hal_bleio_peripheral_construct(self, services_list, name);
+    common_hal_bleio_peripheral_construct(self, name);
 
     return MP_OBJ_FROM_PTR(self);
 }
 
+//|   .. method:: add_service(uuid, *, secondary=False)
+//|
+//|   Create a new `Service` object, identitied by the specified UUID, and add it to this ``Peripheral``.
+//|
+//|   To mark the service as secondary, pass `True` as :py:data:`secondary`.
+//|
+//|   :param bleio.UUID uuid: The uuid of the service
+//|   :param bool secondary: If the service is a secondary one
+//
+//|   :return: the new `Service`
+//|
+STATIC mp_obj_t bleio_peripheral_add_service(size_t n_args, const mp_obj_t *pos_args, mp_map_t *kw_args) {
+    bleio_peripheral_obj_t *self = MP_OBJ_TO_PTR(pos_args[0]);
+
+    enum { ARG_uuid, ARG_secondary };
+    static const mp_arg_t allowed_args[] = {
+        { MP_QSTR_uuid, MP_ARG_REQUIRED | MP_ARG_OBJ, {.u_obj = mp_const_none} },
+        { MP_QSTR_secondary, MP_ARG_KW_ONLY | MP_ARG_BOOL, {.u_bool = false} },
+    };
+
+    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);
+
+    const mp_obj_t uuid_obj = args[ARG_uuid].u_obj;
+
+    if (!MP_OBJ_IS_TYPE(uuid_obj, &bleio_uuid_type)) {
+        mp_raise_ValueError(translate("Expected a UUID"));
+    }
+
+    const bool is_secondary = args[ARG_secondary].u_bool;
+    bleio_uuid_obj_t *uuid = MP_OBJ_TO_PTR(uuid_obj);
+
+    bleio_service_obj_t *service = m_new_obj(bleio_service_obj_t);
+    service->base.type = &bleio_service_type;
+
+    common_hal_bleio_service_construct(service, uuid, is_secondary);
+
+    common_hal_bleio_peripheral_add_service(self, service);
+
+    return MP_OBJ_FROM_PTR(service);
+}
+STATIC MP_DEFINE_CONST_FUN_OBJ_KW(bleio_peripheral_add_service_obj, 2, bleio_peripheral_add_service);
+
+
 //|   .. attribute:: connected (read-only)
 //|
 //|     True if connected to a BLE Central device.
@@ -320,11 +347,12 @@ STATIC MP_DEFINE_CONST_FUN_OBJ_1(bleio_peripheral_pair_obj, bleio_peripheral_pai
 
 STATIC const mp_rom_map_elem_t bleio_peripheral_locals_dict_table[] = {
     // Methods
+    { MP_ROM_QSTR(MP_QSTR_add_service),              MP_ROM_PTR(&bleio_peripheral_add_service_obj) },
     { MP_ROM_QSTR(MP_QSTR_start_advertising),        MP_ROM_PTR(&bleio_peripheral_start_advertising_obj) },
     { MP_ROM_QSTR(MP_QSTR_stop_advertising),         MP_ROM_PTR(&bleio_peripheral_stop_advertising_obj) },
     { MP_ROM_QSTR(MP_QSTR_disconnect),               MP_ROM_PTR(&bleio_peripheral_disconnect_obj) },
     { MP_ROM_QSTR(MP_QSTR_discover_remote_services), MP_ROM_PTR(&bleio_peripheral_discover_remote_services_obj) },
-    { MP_ROM_QSTR(MP_QSTR_pair)                    , MP_ROM_PTR(&bleio_peripheral_pair_obj) },
+    { MP_ROM_QSTR(MP_QSTR_pair),                     MP_ROM_PTR(&bleio_peripheral_pair_obj) },
 
     // Properties
     { MP_ROM_QSTR(MP_QSTR_connected),       MP_ROM_PTR(&bleio_peripheral_connected_obj) },
diff --git a/shared-bindings/bleio/Peripheral.h b/shared-bindings/bleio/Peripheral.h
index 597b65ce2..a4bc9542b 100644
--- a/shared-bindings/bleio/Peripheral.h
+++ b/shared-bindings/bleio/Peripheral.h
@@ -30,10 +30,12 @@
 
 #include "py/objtuple.h"
 #include "common-hal/bleio/Peripheral.h"
+#include "common-hal/bleio/Service.h"
 
 extern const mp_obj_type_t bleio_peripheral_type;
 
-extern void common_hal_bleio_peripheral_construct(bleio_peripheral_obj_t *self, mp_obj_list_t *service_list, mp_obj_t name);
+extern void common_hal_bleio_peripheral_construct(bleio_peripheral_obj_t *self, mp_obj_t name);
+extern void common_hal_bleio_peripheral_add_service(bleio_peripheral_obj_t *self, bleio_service_obj_t *service);
 extern mp_obj_list_t *common_hal_bleio_peripheral_get_services(bleio_peripheral_obj_t *self);
 extern bool common_hal_bleio_peripheral_get_connected(bleio_peripheral_obj_t *self);
 extern mp_obj_t common_hal_bleio_peripheral_get_name(bleio_peripheral_obj_t *self);
diff --git a/shared-bindings/bleio/Service.c b/shared-bindings/bleio/Service.c
index 6961b0b52..2497e9d47 100644
--- a/shared-bindings/bleio/Service.c
+++ b/shared-bindings/bleio/Service.c
@@ -39,73 +39,11 @@
 //|
 //| Stores information about a BLE service and its characteristics.
 //|
-
-//| .. class:: Service(uuid, characteristics, *, secondary=False)
-//|
-//|   Create a new Service object identified by the specified UUID.
-//|
-//|   To mark the service as secondary, pass `True` as :py:data:`secondary`.
-//|
-//|   :param bleio.UUID uuid: The uuid of the service
-//|   :param iterable characteristics: the Characteristic objects for this service
-//|   :param bool secondary: If the service is a secondary one
+//| A Service cannot be created directly. A new local Service can be created
+//| and attached to a Peripheral by calling `Peripheral.add_service()`.
+//| Remote Service objects are created by `Central.discover_remote_services()`
+//| or `Peripheral.discover_remote_services()`.
 //|
-//|   A Service may be remote (:py:data:`remote` is ``True``), but a remote Service
-//|   cannot be constructed directly. It is created by `Central.discover_remote_services()`
-//|   or `Peripheral.discover_remote_services()`.
-
-STATIC mp_obj_t bleio_service_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_uuid, ARG_characteristics, ARG_secondary };
-    static const mp_arg_t allowed_args[] = {
-        { MP_QSTR_uuid, MP_ARG_REQUIRED | MP_ARG_OBJ, {.u_obj = mp_const_none} },
-        { MP_QSTR_characteristics, MP_ARG_REQUIRED | MP_ARG_OBJ, {.u_obj = mp_const_none} },
-        { MP_QSTR_secondary, MP_ARG_KW_ONLY | MP_ARG_BOOL, {.u_bool = false} },
-    };
-
-    mp_arg_val_t args[MP_ARRAY_SIZE(allowed_args)];
-    mp_arg_parse_all(n_args, pos_args, kw_args, MP_ARRAY_SIZE(allowed_args), allowed_args, args);
-
-    const mp_obj_t uuid_obj = args[ARG_uuid].u_obj;
-
-    if (!MP_OBJ_IS_TYPE(uuid_obj, &bleio_uuid_type)) {
-        mp_raise_ValueError(translate("Expected a UUID"));
-    }
-
-    bleio_service_obj_t *self = m_new_obj(bleio_service_obj_t);
-    self->base.type = &bleio_service_type;
-
-    const bool is_secondary = args[ARG_secondary].u_bool;
-    bleio_uuid_obj_t *uuid = MP_OBJ_TO_PTR(uuid_obj);
-
-    // If characteristics is not an iterable, an exception will be thrown.
-    mp_obj_iter_buf_t iter_buf;
-    mp_obj_t iterable = mp_getiter(args[ARG_characteristics].u_obj, &iter_buf);
-    mp_obj_t characteristic_obj;
-
-    // Copy the characteristics list and validate its items.
-    mp_obj_t char_list_obj = mp_obj_new_list(0, NULL);
-    mp_obj_list_t *char_list = MP_OBJ_TO_PTR(char_list_obj);
-
-    while ((characteristic_obj = mp_iternext(iterable)) != MP_OBJ_STOP_ITERATION) {
-        if (!MP_OBJ_IS_TYPE(characteristic_obj, &bleio_characteristic_type)) {
-            mp_raise_ValueError(translate("characteristics includes an object that is not a Characteristic"));
-        }
-        bleio_characteristic_obj_t *characteristic = MP_OBJ_TO_PTR(characteristic_obj);
-        if (common_hal_bleio_uuid_get_uuid128_reference(uuid) !=
-            common_hal_bleio_uuid_get_uuid128_reference(characteristic->uuid)) {
-            // The descriptor base UUID doesn't match the characteristic base UUID.
-            mp_raise_ValueError(translate("Characteristic UUID doesn't match Service UUID"));
-        }
-        if (common_hal_bleio_characteristic_get_service(characteristic) != MP_OBJ_NULL) {
-            mp_raise_ValueError(translate("Characteristic is already attached to a Service"));
-        }
-        mp_obj_list_append(char_list_obj, characteristic_obj);
-    }
-
-    common_hal_bleio_service_construct(self, uuid, char_list, is_secondary);
-
-    return MP_OBJ_FROM_PTR(self);
-}
 
 //|   .. attribute:: characteristics
 //|
@@ -182,10 +120,101 @@ const mp_obj_property_t bleio_service_uuid_obj = {
                (mp_obj_t)&mp_const_none_obj },
 };
 
+//|   .. method:: add_characteristic(uuid, *, properties=0, read_perm=`Attribute.OPEN`, write_perm=`Attribute.OPEN`, max_length=20, fixed_length=False, initial_value=None)
+//|
+//|   Create a new `Characteristic` object, and add it to this Service.
+//|
+//|   :param bleio.UUID uuid: The uuid of the characteristic
+//|   :param int properties: The properties of the characteristic,
+//|      specified as a bitmask of these values bitwise-or'd together:
+//|      `Characteristic.BROADCAST`, `Characteristic.INDICATE`, `Characteristic.NOTIFY`,
+//|      `Characteristic.READ`, `Characteristic.WRITE`, `Characteristic.WRITE_NO_RESPONSE`.
+//|   :param int read_perm: Specifies whether the characteristic can be read by a client, and if so, which
+//|      security mode is required. Must be one of the integer values `Attribute.NO_ACCESS`, `Attribute.OPEN`,
+//|      `Attribute.ENCRYPT_NO_MITM`, `Attribute.ENCRYPT_WITH_MITM`, `Attribute.LESC_ENCRYPT_WITH_MITM`,
+//|      `Attribute.SIGNED_NO_MITM`, or `Attribute.SIGNED_WITH_MITM`.
+//|   :param int write_perm: Specifies whether the characteristic can be written by a client, and if so, which
+//|      security mode is required. Values allowed are the same as ``read_perm``.
+//|   :param int max_length: Maximum length in bytes of the characteristic value. The maximum allowed is
+//|      is 512, or possibly 510 if ``fixed_length`` is False. The default, 20, is the maximum
+//|      number of data bytes that fit in a single BLE 4.x ATT packet.
+//|   :param bool fixed_length: True if the characteristic value is of fixed length.
+//|   :param buf initial_value: The initial value for this characteristic. If not given, will be
+//|      filled with zeros.
+//|
+//|   :return: the new `Characteristic`.
+//|
+STATIC mp_obj_t bleio_service_add_characteristic(size_t n_args, const mp_obj_t *pos_args, mp_map_t *kw_args) {
+    bleio_service_obj_t *self = MP_OBJ_TO_PTR(pos_args[0]);
+
+    enum { ARG_uuid, ARG_properties, ARG_read_perm, ARG_write_perm,
+           ARG_max_length, ARG_fixed_length, ARG_initial_value };
+    static const mp_arg_t allowed_args[] = {
+        { MP_QSTR_uuid,  MP_ARG_REQUIRED | MP_ARG_OBJ },
+        { MP_QSTR_properties, MP_ARG_KW_ONLY| MP_ARG_INT, {.u_int = 0} },
+        { MP_QSTR_read_perm, MP_ARG_KW_ONLY| MP_ARG_INT, {.u_int = SECURITY_MODE_OPEN} },
+        { MP_QSTR_write_perm, MP_ARG_KW_ONLY| MP_ARG_INT, {.u_int = SECURITY_MODE_OPEN} },
+        { MP_QSTR_max_length, MP_ARG_KW_ONLY| MP_ARG_INT, {.u_int = 20} },
+        { MP_QSTR_fixed_length, MP_ARG_KW_ONLY| MP_ARG_BOOL, {.u_bool = false} },
+        { MP_QSTR_initial_value, MP_ARG_KW_ONLY| MP_ARG_OBJ, {.u_obj = mp_const_none} },
+    };
+
+    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);
+
+    const mp_obj_t uuid_obj = args[ARG_uuid].u_obj;
+
+    if (!MP_OBJ_IS_TYPE(uuid_obj, &bleio_uuid_type)) {
+        mp_raise_ValueError(translate("Expected a UUID"));
+    }
+    bleio_uuid_obj_t *uuid = MP_OBJ_TO_PTR(uuid_obj);
+
+    const bleio_characteristic_properties_t properties = args[ARG_properties].u_int;
+    if (properties & ~CHAR_PROP_ALL) {
+        mp_raise_ValueError(translate("Invalid properties"));
+    }
+
+    const bleio_attribute_security_mode_t read_perm = args[ARG_read_perm].u_int;
+    common_hal_bleio_attribute_security_mode_check_valid(read_perm);
+
+    const bleio_attribute_security_mode_t write_perm = args[ARG_write_perm].u_int;
+    common_hal_bleio_attribute_security_mode_check_valid(write_perm);
+
+    const mp_int_t max_length = args[ARG_max_length].u_int;
+    const bool fixed_length =  args[ARG_fixed_length].u_bool;
+    mp_obj_t initial_value = args[ARG_initial_value].u_obj;
+
+    // Length will be validated in common_hal.
+    mp_buffer_info_t initial_value_bufinfo;
+    if (initial_value == mp_const_none) {
+        if (fixed_length && max_length > 0) {
+            initial_value = mp_obj_new_bytes_of_zeros(max_length);
+        } else {
+            initial_value = mp_const_empty_bytes;
+        }
+    }
+    mp_get_buffer_raise(initial_value, &initial_value_bufinfo, MP_BUFFER_READ);
+
+    bleio_characteristic_obj_t *characteristic = m_new_obj(bleio_characteristic_obj_t);
+    characteristic->base.type = &bleio_characteristic_type;
+
+    // Range checking on max_length arg is done by the common_hal layer, because
+    // it may vary depending on underlying BLE implementation.
+    common_hal_bleio_characteristic_construct(
+        characteristic, uuid, properties, read_perm, write_perm,
+        max_length, fixed_length, &initial_value_bufinfo);
+
+    common_hal_bleio_service_add_characteristic(self, characteristic);
+
+    return MP_OBJ_FROM_PTR(characteristic);
+}
+STATIC MP_DEFINE_CONST_FUN_OBJ_KW(bleio_service_add_characteristic_obj, 2, bleio_service_add_characteristic);
+
 STATIC const mp_rom_map_elem_t bleio_service_locals_dict_table[] = {
     { MP_ROM_QSTR(MP_QSTR_characteristics),    MP_ROM_PTR(&bleio_service_characteristics_obj) },
     { MP_ROM_QSTR(MP_QSTR_secondary),          MP_ROM_PTR(&bleio_service_secondary_obj) },
     { MP_ROM_QSTR(MP_QSTR_uuid),               MP_ROM_PTR(&bleio_service_uuid_obj) },
+    { MP_ROM_QSTR(MP_QSTR_add_characteristic), MP_ROM_PTR(&bleio_service_add_characteristic_obj) },
 };
 STATIC MP_DEFINE_CONST_DICT(bleio_service_locals_dict, bleio_service_locals_dict_table);
 
@@ -203,7 +232,6 @@ STATIC void bleio_service_print(const mp_print_t *print, mp_obj_t self_in, mp_pr
 const mp_obj_type_t bleio_service_type = {
     { &mp_type_type },
     .name = MP_QSTR_Service,
-    .make_new = bleio_service_make_new,
     .print = bleio_service_print,
     .locals_dict = (mp_obj_dict_t*)&bleio_service_locals_dict
 };
diff --git a/shared-bindings/bleio/Service.h b/shared-bindings/bleio/Service.h
index 716c2e8a9..91c08fbb8 100644
--- a/shared-bindings/bleio/Service.h
+++ b/shared-bindings/bleio/Service.h
@@ -32,11 +32,11 @@
 
 const mp_obj_type_t bleio_service_type;
 
-extern 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);
+extern void common_hal_bleio_service_construct(bleio_service_obj_t *self, bleio_uuid_obj_t *uuid, bool is_secondary);
 extern bleio_uuid_obj_t *common_hal_bleio_service_get_uuid(bleio_service_obj_t *self);
 extern mp_obj_list_t *common_hal_bleio_service_get_characteristic_list(bleio_service_obj_t *self);
 extern bool common_hal_bleio_service_get_is_remote(bleio_service_obj_t *self);
 extern bool common_hal_bleio_service_get_is_secondary(bleio_service_obj_t *self);
-extern void common_hal_bleio_service_add_all_characteristics(bleio_service_obj_t *self);
+extern void common_hal_bleio_service_add_characteristic(bleio_service_obj_t *self, bleio_characteristic_obj_t *characteristic);
 
 #endif // MICROPY_INCLUDED_SHARED_BINDINGS_BLEIO_SERVICE_H
diff --git a/shared-bindings/bleio/__init__.h b/shared-bindings/bleio/__init__.h
index 89be71093..c84895068 100644
--- a/shared-bindings/bleio/__init__.h
+++ b/shared-bindings/bleio/__init__.h
@@ -41,7 +41,7 @@ extern const super_adapter_obj_t common_hal_bleio_adapter_obj;
 extern void common_hal_bleio_check_connected(uint16_t conn_handle);
 
 extern uint16_t common_hal_bleio_device_get_conn_handle(mp_obj_t device);
-extern mp_obj_list_t *common_hal_bleio_device_get_remote_services_list(mp_obj_t device);
+extern mp_obj_list_t *common_hal_bleio_device_get_remote_service_list(mp_obj_t device);
 extern void common_hal_bleio_device_discover_remote_services(mp_obj_t device, mp_obj_t service_uuids_whitelist);
 
 extern mp_obj_t common_hal_bleio_gatts_read(uint16_t handle, uint16_t conn_handle);