specifying attribute length; fix up value setting

11 files changed, 193 insertions, 82 deletions

diff --git a/ports/nrf/common-hal/bleio/Characteristic.c b/ports/nrf/common-hal/bleio/Characteristic.c
index e0ddf1e81..f8b6ce06c 100644
--- a/ports/nrf/common-hal/bleio/Characteristic.c
+++ b/ports/nrf/common-hal/bleio/Characteristic.c
@@ -43,7 +43,6 @@ STATIC uint16_t characteristic_get_cccd(uint16_t cccd_handle, uint16_t conn_hand
 
     const uint32_t err_code = sd_ble_gatts_value_get(conn_handle, cccd_handle, &value);
 
-
     if (err_code == BLE_ERROR_GATTS_SYS_ATTR_MISSING) {
         // CCCD is not set, so say that neither Notify nor Indicate is enabled.
         cccd = 0;
@@ -125,16 +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_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_obj_list_t *descriptor_list) {
     self->service = MP_OBJ_NULL;
     self->uuid = uuid;
-    self->value = mp_const_none;
+    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;
 
+    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"),
+                                 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]);
@@ -151,48 +158,63 @@ bleio_service_obj_t *common_hal_bleio_characteristic_get_service(bleio_character
 }
 
 mp_obj_t common_hal_bleio_characteristic_get_value(bleio_characteristic_obj_t *self) {
-    uint16_t conn_handle = common_hal_bleio_device_get_conn_handle(self->service->device);
-    if (common_hal_bleio_service_get_is_remote(self->service)) {
-        // self->value is set by evt handler.
-        characteristic_gattc_read(self);
-    } else {
-        self->value = common_hal_bleio_gatts_read(self->handle, conn_handle);
+    // 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);
+        if (common_hal_bleio_service_get_is_remote(self->service)) {
+            // self->value is set by evt handler.
+            characteristic_gattc_read(self);
+        } else {
+            self->value = common_hal_bleio_gatts_read(self->handle, conn_handle);
+        }
     }
 
     return self->value;
 }
 
 void common_hal_bleio_characteristic_set_value(bleio_characteristic_obj_t *self, mp_buffer_info_t *bufinfo) {
-    uint16_t conn_handle = common_hal_bleio_device_get_conn_handle(self->service->device);
-
-    if (common_hal_bleio_service_get_is_remote(self->service)) {
-        // Last argument is true if write-no-reponse desired.
-        common_hal_bleio_gattc_write(self->handle, conn_handle, bufinfo,
-                                     (self->props & CHAR_PROP_WRITE_NO_RESPONSE));
-    } else {
-        bool sent = false;
-        uint16_t cccd = 0;
-
-        const bool notify = self->props & CHAR_PROP_NOTIFY;
-        const bool indicate = self->props & CHAR_PROP_INDICATE;
-        if (notify | indicate) {
-            cccd = characteristic_get_cccd(self->cccd_handle, conn_handle);
-        }
+    // 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);
+
+        if (common_hal_bleio_service_get_is_remote(self->service)) {
+            // Last argument is true if write-no-reponse desired.
+            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"));
+            }
 
-        // It's possible that both notify and indicate are set.
-        if (notify && (cccd & BLE_GATT_HVX_NOTIFICATION)) {
-            characteristic_gatts_notify_indicate(self->handle, conn_handle, bufinfo, BLE_GATT_HVX_NOTIFICATION);
-            sent = true;
-        }
-        if (indicate && (cccd & BLE_GATT_HVX_INDICATION)) {
-            characteristic_gatts_notify_indicate(self->handle, conn_handle, bufinfo, BLE_GATT_HVX_INDICATION);
-            sent = true;
-        }
+            bool sent = false;
+            uint16_t cccd = 0;
+
+            const bool notify = self->props & CHAR_PROP_NOTIFY;
+            const bool indicate = self->props & CHAR_PROP_INDICATE;
+            if (notify | indicate) {
+                cccd = characteristic_get_cccd(self->cccd_handle, conn_handle);
+            }
+
+            // It's possible that both notify and indicate are set.
+            if (notify && (cccd & BLE_GATT_HVX_NOTIFICATION)) {
+                characteristic_gatts_notify_indicate(self->handle, conn_handle, bufinfo, BLE_GATT_HVX_NOTIFICATION);
+                sent = true;
+            }
+            if (indicate && (cccd & BLE_GATT_HVX_INDICATION)) {
+                characteristic_gatts_notify_indicate(self->handle, conn_handle, bufinfo, BLE_GATT_HVX_INDICATION);
+                sent = true;
+            }
 
-        if (!sent) {
-            common_hal_bleio_gatts_write(self->handle, conn_handle, bufinfo);
+            if (!sent) {
+                common_hal_bleio_gatts_write(self->handle, conn_handle, bufinfo);
+            }
         }
     }
+
+    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) {
diff --git a/ports/nrf/common-hal/bleio/Characteristic.h b/ports/nrf/common-hal/bleio/Characteristic.h
index 46f46eee5..fb6a4c6ce 100644
--- a/ports/nrf/common-hal/bleio/Characteristic.h
+++ b/ports/nrf/common-hal/bleio/Characteristic.h
@@ -39,6 +39,8 @@ typedef struct {
     bleio_service_obj_t *service;
     bleio_uuid_obj_t *uuid;
     mp_obj_t value;
+    uint16_t max_length;
+    bool fixed_length;
     uint16_t handle;
     bleio_characteristic_properties_t props;
     bleio_attribute_security_mode_t read_perm;
diff --git a/ports/nrf/common-hal/bleio/Descriptor.c b/ports/nrf/common-hal/bleio/Descriptor.c
index d6473a64f..2a2019277 100644
--- a/ports/nrf/common-hal/bleio/Descriptor.c
+++ b/ports/nrf/common-hal/bleio/Descriptor.c
@@ -35,20 +35,28 @@
 
 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) {
+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) {
     self->characteristic = MP_OBJ_NULL;
     self->uuid = uuid;
-    self->value = mp_const_none;
-    self->handle = BLE_CONN_HANDLE_INVALID;
+    self->value = mp_const_empty_bytes;
+    self->handle = BLE_GATT_HANDLE_INVALID;
     self->read_perm = read_perm;
     self->write_perm = write_perm;
+
+    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"),
+                                 max_length_max, fixed_length ? "True" : "False");
+    }
+    self->max_length = max_length;
+    self->fixed_length = fixed_length;
 }
 
 bleio_uuid_obj_t *common_hal_bleio_descriptor_get_uuid(bleio_descriptor_obj_t *self) {
     return self->uuid;
 }
 
-mp_obj_t common_hal_bleio_descriptor_get_characteristic(bleio_descriptor_obj_t *self) {
+bleio_characteristic_obj_t *common_hal_bleio_descriptor_get_characteristic(bleio_descriptor_obj_t *self) {
     return self->characteristic;
 }
 
@@ -97,22 +105,37 @@ STATIC void descriptor_gattc_read(bleio_descriptor_obj_t *descriptor) {
 }
 
 mp_obj_t common_hal_bleio_descriptor_get_value(bleio_descriptor_obj_t *self) {
-    if (common_hal_bleio_service_get_is_remote(self->characteristic->service)) {
-        descriptor_gattc_read(self);
-    } else {
-        self->value = common_hal_bleio_gatts_read(
-            self->handle, common_hal_bleio_device_get_conn_handle(self->characteristic->service->device));
+    // Do GATT operations only if this descriptor has been registered
+    if (self->handle != BLE_GATT_HANDLE_INVALID) {
+        if (common_hal_bleio_service_get_is_remote(self->characteristic->service)) {
+            descriptor_gattc_read(self);
+        } else {
+            self->value = common_hal_bleio_gatts_read(
+                self->handle, common_hal_bleio_device_get_conn_handle(self->characteristic->service->device));
+        }
     }
 
     return self->value;
 }
 
 void common_hal_bleio_descriptor_set_value(bleio_descriptor_obj_t *self, mp_buffer_info_t *bufinfo) {
-    uint16_t conn_handle = common_hal_bleio_device_get_conn_handle(self->characteristic->service->device);
-    if (common_hal_bleio_service_get_is_remote(self->characteristic->service)) {
-        // false means WRITE_REQ, not write-no-response
-        common_hal_bleio_gattc_write(self->handle, conn_handle, bufinfo, false);
-    } else {
-        common_hal_bleio_gatts_write(self->handle, conn_handle, bufinfo);
+    // 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);
+        if (common_hal_bleio_service_get_is_remote(self->characteristic->service)) {
+            // 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 2fae8cba6..6db4d7be3 100644
--- a/ports/nrf/common-hal/bleio/Descriptor.h
+++ b/ports/nrf/common-hal/bleio/Descriptor.h
@@ -40,6 +40,8 @@ typedef struct {
     bleio_characteristic_obj_t *characteristic;
     bleio_uuid_obj_t *uuid;
     mp_obj_t value;
+    uint16_t max_length;
+    bool fixed_length;
     uint16_t handle;
     bleio_attribute_security_mode_t read_perm;
     bleio_attribute_security_mode_t write_perm;
diff --git a/ports/nrf/common-hal/bleio/Peripheral.c b/ports/nrf/common-hal/bleio/Peripheral.c
index 905b26835..640b93180 100644
--- a/ports/nrf/common-hal/bleio/Peripheral.c
+++ b/ports/nrf/common-hal/bleio/Peripheral.c
@@ -149,21 +149,18 @@ STATIC void peripheral_on_ble_evt(ble_evt_t *ble_evt, void *self_in) {
 
         case BLE_GAP_EVT_CONN_SEC_UPDATE: {
             ble_gap_conn_sec_t* conn_sec = &ble_evt->evt.gap_evt.params.conn_sec_update.conn_sec;
-            mp_printf(&mp_plat_print, "sm: %d, lv: %d\n", conn_sec->sec_mode.sm, conn_sec->sec_mode.lv);
             if (conn_sec->sec_mode.sm <= 1 && conn_sec->sec_mode.lv <= 1) {
                 // Security setup did not succeed:
                 // mode 0, level 0 means no access
                 // mode 1, level 1 means open link
                 // mode >=1 and/or level >=1 means encryption is set up
                 self->pair_status = PAIR_NOT_PAIRED;
-                mp_printf(&mp_plat_print, "PAIR_NOT_PAIRED\n");
             } else {
                 // TODO: see Bluefruit lib
                 // if ( !bond_load_cccd(_role, _conn_hdl, _ediv) ) {
                 //    sd_ble_gatts_sys_attr_set(_conn_hdl, NULL, 0, 0);
                 // }
                 self->pair_status = PAIR_PAIRED;
-                mp_printf(&mp_plat_print, "PAIR_PAIRED\n");
             }
             break;
         }
diff --git a/ports/nrf/common-hal/bleio/Service.c b/ports/nrf/common-hal/bleio/Service.c
index 0ac0107f5..a7a3a6db0 100644
--- a/ports/nrf/common-hal/bleio/Service.c
+++ b/ports/nrf/common-hal/bleio/Service.c
@@ -72,6 +72,10 @@ void common_hal_bleio_service_add_all_characteristics(bleio_service_obj_t *self)
         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,
@@ -97,17 +101,22 @@ void common_hal_bleio_service_add_all_characteristics(bleio_service_obj_t *self)
 
         ble_gatts_attr_md_t char_attr_md = {
             .vloc = BLE_GATTS_VLOC_STACK,
-            .vlen = 1,
+            .vlen = !characteristic->fixed_length,
         };
 
         BLE_GAP_CONN_SEC_MODE_SET_OPEN(&char_attr_md.read_perm);
         BLE_GAP_CONN_SEC_MODE_SET_OPEN(&char_attr_md.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 = sizeof(uint8_t),
-            .max_len = GATT_MAX_DATA_LENGTH,
+            .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;
@@ -118,10 +127,6 @@ void common_hal_bleio_service_add_all_characteristics(bleio_service_obj_t *self)
             mp_raise_OSError_msg_varg(translate("Failed to add characteristic, err 0x%04x"), err_code);
         }
 
-        if (characteristic->handle != BLE_GATT_HANDLE_INVALID) {
-            mp_raise_ValueError(translate("Characteristic already in use by another Service."));
-        }
-
         characteristic->user_desc_handle = char_handles.user_desc_handle;
         characteristic->cccd_handle = char_handles.cccd_handle;
         characteristic->sccd_handle = char_handles.sccd_handle;
@@ -138,25 +143,27 @@ void common_hal_bleio_service_add_all_characteristics(bleio_service_obj_t *self)
             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 = 1,
+                .vlen = !descriptor->fixed_length,
             };
 
             BLE_GAP_CONN_SEC_MODE_SET_OPEN(&desc_attr_md.read_perm);
             BLE_GAP_CONN_SEC_MODE_SET_OPEN(&desc_attr_md.write_perm);
 
-            mp_buffer_info_t bufinfo;
-            mp_get_buffer_raise(descriptor->value, &bufinfo, MP_BUFFER_READ);
+            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 = bufinfo.len,
-                .p_value = bufinfo.buf,
+                .init_len = desc_value_bufinfo.len,
+                .p_value = desc_value_bufinfo.buf,
                 .init_offs = 0,
-                .max_len = GATT_MAX_DATA_LENGTH,
+                .max_len = descriptor->max_length,
             };
 
             err_code = sd_ble_gatts_descriptor_add(characteristic->handle, &desc_attr, &descriptor->handle);
-        }
-    }
+
+        } // loop over descriptors
+
+    } // loop over characteristics
 }
diff --git a/ports/nrf/common-hal/bleio/__init__.c b/ports/nrf/common-hal/bleio/__init__.c
index d09b6f5af..2d144f8b5 100644
--- a/ports/nrf/common-hal/bleio/__init__.c
+++ b/ports/nrf/common-hal/bleio/__init__.c
@@ -218,7 +218,9 @@ STATIC void on_char_discovery_rsp(ble_gattc_evt_char_disc_rsp_t *response, mp_ob
 
         // Call common_hal_bleio_characteristic_construct() to initalize some fields and set up evt handler.
         common_hal_bleio_characteristic_construct(
-            characteristic, uuid, props, SEC_MODE_OPEN, SEC_MODE_OPEN, mp_obj_new_list(0, NULL));
+            characteristic, uuid, props, SEC_MODE_OPEN, SEC_MODE_OPEN,
+            GATT_MAX_DATA_LENGTH, false,   // max_length, fixed_length: values may not matter for gattc
+            mp_obj_new_list(0, NULL));
         characteristic->handle = gattc_char->handle_value;
         characteristic->service = m_char_discovery_service;
 
@@ -272,7 +274,8 @@ STATIC void on_desc_discovery_rsp(ble_gattc_evt_desc_disc_rsp_t *response, mp_ob
             // For now, just leave the UUID as NULL.
         }
 
-        common_hal_bleio_descriptor_construct(descriptor, uuid, SEC_MODE_OPEN, SEC_MODE_OPEN);
+        common_hal_bleio_descriptor_construct(descriptor, uuid, SEC_MODE_OPEN, SEC_MODE_OPEN,
+                                              GATT_MAX_DATA_LENGTH, false);
         descriptor->handle = gattc_desc->handle;
         descriptor->characteristic = m_desc_discovery_characteristic;
 
diff --git a/shared-bindings/bleio/Characteristic.c b/shared-bindings/bleio/Characteristic.c
index 4adc897eb..7bcfc6b6d 100644
--- a/shared-bindings/bleio/Characteristic.c
+++ b/shared-bindings/bleio/Characteristic.c
@@ -42,7 +42,7 @@
 //| and writing of the characteristic's value.
 //|
 //|
-//| .. class:: Characteristic(uuid, *, properties=0, read_perm=`Attribute.OPEN`, write_perm=`Attribute.OPEN`, descriptors=None)
+//| .. 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.
 //|
@@ -55,15 +55,22 @@
 //|      `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_descriptors };
+    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 = SEC_MODE_OPEN } },
-        { MP_QSTR_write_perm, MP_ARG_KW_ONLY| MP_ARG_INT, {.u_int = SEC_MODE_OPEN } },
+        { 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 = SEC_MODE_OPEN} },
+        { MP_QSTR_write_perm, MP_ARG_KW_ONLY| MP_ARG_INT, {.u_int = SEC_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} },
     };
 
@@ -110,13 +117,18 @@ STATIC mp_obj_t bleio_characteristic_make_new(const mp_obj_type_t *type, size_t
             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_const_none) {
+        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);
     }
 
-    common_hal_bleio_characteristic_construct(self, uuid, properties, read_perm, write_perm, desc_list);
+    // 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);
 }
diff --git a/shared-bindings/bleio/Characteristic.h b/shared-bindings/bleio/Characteristic.h
index 1d532d748..87942c8e9 100644
--- a/shared-bindings/bleio/Characteristic.h
+++ b/shared-bindings/bleio/Characteristic.h
@@ -34,7 +34,7 @@
 
 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_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_obj_list_t *descriptor_list);
 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);
diff --git a/shared-bindings/bleio/Descriptor.c b/shared-bindings/bleio/Descriptor.c
index 1ab8c9ecd..2d4c8df9f 100644
--- a/shared-bindings/bleio/Descriptor.c
+++ b/shared-bindings/bleio/Descriptor.c
@@ -53,13 +53,19 @@
 //|      `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 };
+    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 = SEC_MODE_OPEN } },
         { MP_QSTR_write_perm, MP_ARG_KW_ONLY| MP_ARG_INT, {.u_int = SEC_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)];
@@ -81,7 +87,10 @@ STATIC mp_obj_t bleio_descriptor_make_new(const mp_obj_type_t *type, size_t n_ar
     self->base.type = type;
     bleio_uuid_obj_t *uuid = MP_OBJ_TO_PTR(uuid_arg);
 
-    common_hal_bleio_descriptor_construct(self, uuid, read_perm, write_perm);
+    // 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);
 }
@@ -123,9 +132,41 @@ const mp_obj_property_t bleio_descriptor_characteristic_obj = {
                (mp_obj_t)&mp_const_none_obj },
 };
 
+//|   .. attribute:: value
+//|
+//|     The value of this descriptor.
+//|
+STATIC mp_obj_t bleio_descriptor_get_value(mp_obj_t self_in) {
+    bleio_descriptor_obj_t *self = MP_OBJ_TO_PTR(self_in);
+
+    return common_hal_bleio_descriptor_get_value(self);
+}
+STATIC MP_DEFINE_CONST_FUN_OBJ_1(bleio_descriptor_get_value_obj, bleio_descriptor_get_value);
+
+STATIC mp_obj_t bleio_descriptor_set_value(mp_obj_t self_in, mp_obj_t value_in) {
+    bleio_descriptor_obj_t *self = MP_OBJ_TO_PTR(self_in);
+
+    mp_buffer_info_t bufinfo;
+    mp_get_buffer_raise(value_in, &bufinfo, MP_BUFFER_READ);
+
+    common_hal_bleio_descriptor_set_value(self, &bufinfo);
+
+    return mp_const_none;
+}
+STATIC MP_DEFINE_CONST_FUN_OBJ_2(bleio_descriptor_set_value_obj, bleio_descriptor_set_value);
+
+const mp_obj_property_t bleio_descriptor_value_obj = {
+    .base.type = &mp_type_property,
+    .proxy = { (mp_obj_t)&bleio_descriptor_get_value_obj,
+               (mp_obj_t)&bleio_descriptor_set_value_obj,
+               (mp_obj_t)&mp_const_none_obj },
+};
+
 STATIC const mp_rom_map_elem_t bleio_descriptor_locals_dict_table[] = {
     // Properties
     { MP_ROM_QSTR(MP_QSTR_uuid), MP_ROM_PTR(&bleio_descriptor_uuid_obj) },
+    { MP_ROM_QSTR(MP_QSTR_characteristic), MP_ROM_PTR(&bleio_descriptor_characteristic_obj) },
+    { MP_ROM_QSTR(MP_QSTR_value), MP_ROM_PTR(&bleio_descriptor_value_obj) },
 };
 
 STATIC MP_DEFINE_CONST_DICT(bleio_descriptor_locals_dict, bleio_descriptor_locals_dict_table);
diff --git a/shared-bindings/bleio/Descriptor.h b/shared-bindings/bleio/Descriptor.h
index eefce03d1..430c0d0b6 100644
--- a/shared-bindings/bleio/Descriptor.h
+++ b/shared-bindings/bleio/Descriptor.h
@@ -34,8 +34,10 @@
 
 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);
+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 bleio_uuid_obj_t *common_hal_bleio_descriptor_get_uuid(bleio_descriptor_obj_t *self);
-extern mp_obj_t common_hal_bleio_descriptor_get_characteristic(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);
+extern void common_hal_bleio_descriptor_set_value(bleio_descriptor_obj_t *self, mp_buffer_info_t *bufinfo);
 
 #endif // MICROPY_INCLUDED_SHARED_BINDINGS_BLEIO_DESCRIPTOR_H