More UUID work; use mp_raise for exceptions

7 files changed, 91 insertions, 53 deletions

diff --git a/shared-bindings/bleio/Address.c b/shared-bindings/bleio/Address.c
index ec23ff207..d4c24e4e2 100644
--- a/shared-bindings/bleio/Address.c
+++ b/shared-bindings/bleio/Address.c
@@ -33,8 +33,12 @@
 #include "shared-bindings/bleio/Address.h"
 #include "shared-module/bleio/Address.h"
 
-#define ADDRESS_LONG_LEN 17 // XX:XX:XX:XX:XX:XX
-#define ADDRESS_SHORT_LEN 12 // XXXXXXXXXXXX
+#define ADDRESS_BYTE_LEN 12
+
+STATIC uint8_t xdigit_8b_value(byte nibble1, byte nibble2) {
+    return unichar_xdigit_value(nibble1) | (unichar_xdigit_value(nibble2) << 4);
+}
+
 
 //| .. currentmodule:: bleio
 //|
@@ -49,7 +53,7 @@
 //|   Create a new Address object encapsulating the address value.
 //|   The value itself can be one of:
 //|
-//|   - a `str` value in the format of 'XXXXXXXXXXXX' or 'XX:XX:XX:XX:XX'
+//|   - a `str` value in the format of 'XXXXXXXXXXXX' or 'XX:XX:XX:XX:XX:XX' (12 hex digits)
 //|   - a `bytes` or `bytearray` containing 6 bytes
 //|   - another Address object
 //|
@@ -85,26 +89,41 @@ STATIC mp_obj_t bleio_address_make_new(const mp_obj_type_t *type, size_t n_args,
     const mp_obj_t address = args[ARG_address].u_obj;
 
     if (MP_OBJ_IS_STR(address)) {
-        GET_STR_DATA_LEN(address, str_data, str_len);
-        const bool is_long = (str_len == ADDRESS_LONG_LEN);
-        const bool is_short = (str_len == ADDRESS_SHORT_LEN);
+        GET_STR_DATA_LEN(address, str, str_len);
+
+        size_t value_index = 0;
+        size_t str_index = str_len;
+        bool error = false;
+
+        // Loop until fewer than two characters left.
+        while (str_index >= 1 && value_index < sizeof(self->value)) {
+            if (str[str_index] == ':') {
+                // Skip colon separators.
+                str_index--;
+                continue;
+            }
 
-        if (is_long || is_short) {
-            size_t i = str_len - 1;
-            for (size_t b = 0; b < BLEIO_ADDRESS_BYTES; ++b) {
-                self->value[b] = unichar_xdigit_value(str_data[i]) |
-                    unichar_xdigit_value(str_data[i - 1]) << 4;
+            if (!unichar_isxdigit(str[str_index]) ||
+                !unichar_isxdigit(str[str_index-1])) {
+                error = true;
+                break;
+            }
 
-                i -= is_long ? 3 : 2;
+            self->value[value_index] = xdigit_8b_value(str[str_index],
+                                                       str[str_index-1]);
+            value_index += 1;
+            str_index -= 2;
             }
-        } else {
-            mp_raise_ValueError(translate("Wrong address length"));
+            // Check for correct number of hex digits and no parsing errors.
+        if (error || value_index != ADDRESS_BYTE_LEN || str_index != -1) {
+            mp_raise_ValueError_varg(translate("Address is not %d bytes long or is in wrong format"),
+                                     ADDRESS_BYTE_LEN);
         }
     } else if (MP_OBJ_IS_TYPE(address, &mp_type_bytearray) || MP_OBJ_IS_TYPE(address, &mp_type_bytes)) {
             mp_buffer_info_t buf_info;
             mp_get_buffer_raise(address, &buf_info, MP_BUFFER_READ);
             if (buf_info.len != BLEIO_ADDRESS_BYTES) {
-                mp_raise_ValueError(translate("Wrong number of bytes provided"));
+                mp_raise_ValueError_varg(translate("Address must be %d bytes long"), BLEIO_ADDRESS_BYTES);
             }
 
             for (size_t b = 0; b < BLEIO_ADDRESS_BYTES; ++b) {
diff --git a/shared-bindings/bleio/Characteristic.c b/shared-bindings/bleio/Characteristic.c
index 2dd1d4270..4b4a4a924 100644
--- a/shared-bindings/bleio/Characteristic.c
+++ b/shared-bindings/bleio/Characteristic.c
@@ -91,7 +91,7 @@ STATIC void bleio_characteristic_print(const mp_print_t *print, mp_obj_t self_in
     bleio_characteristic_obj_t *self = MP_OBJ_TO_PTR(self_in);
 
     mp_printf(print, "Characteristic(");
-    common_hal_bleio_uuid_print(print, self->uuid);
+    common_hal_bleio_uuid_print(self->uuid, print);
     mp_printf(print, ")");
 }
 
@@ -115,16 +115,11 @@ STATIC mp_obj_t bleio_characteristic_make_new(const mp_obj_type_t *type, size_t
 
     const mp_obj_t uuid = args[ARG_uuid].u_obj;
 
-    if (uuid == mp_const_none) {
-        return MP_OBJ_FROM_PTR(self);
-    }
-
-    if (MP_OBJ_IS_TYPE(uuid, &bleio_uuid_type)) {
-        self->uuid = MP_OBJ_TO_PTR(uuid);
-    } else {
+    if (!MP_OBJ_IS_TYPE(uuid, &bleio_uuid_type)) {
         mp_raise_ValueError(translate("Expected a UUID"));
     }
 
+    self->uuid = MP_OBJ_TO_PTR(uuid);
     common_hal_bleio_characteristic_construct(self);
 
     return MP_OBJ_FROM_PTR(self);
diff --git a/shared-bindings/bleio/Descriptor.c b/shared-bindings/bleio/Descriptor.c
index df32a00e7..d58bbd275 100644
--- a/shared-bindings/bleio/Descriptor.c
+++ b/shared-bindings/bleio/Descriptor.c
@@ -61,7 +61,6 @@ enum {
 //| .. class:: Descriptor(uuid)
 //|
 //|   Create a new descriptor object with the UUID uuid.
-//|   The value can be either of type `bleio.UUID` or any value allowed by the `bleio.UUID` constructor.
 
 //|   .. attribute:: handle
 //|
@@ -90,13 +89,11 @@ STATIC mp_obj_t bleio_descriptor_make_new(const mp_obj_type_t *type, size_t n_ar
 
     const mp_obj_t uuid_arg = args[ARG_uuid].u_obj;
 
-    bleio_uuid_obj_t *uuid;
-    if (MP_OBJ_IS_TYPE(uuid_arg, &bleio_uuid_type)) {
-        uuid = MP_OBJ_TO_PTR(uuid_arg);
-    } else {
-        uuid = MP_OBJ_TO_PTR(bleio_uuid_type.make_new(&bleio_uuid_type, 1, 0, &uuid_arg));
+    if (!MP_OBJ_IS_TYPE(uuid_arg, &bleio_uuid_type)) {
+        mp_raise_ValueError(translate("Expected a UUID"));
     }
 
+    bleio_uuid_obj_t *uuid = MP_OBJ_TO_PTR(uuid_arg);
     common_hal_bleio_descriptor_construct(self, uuid);
 
     return MP_OBJ_FROM_PTR(self);
@@ -104,7 +101,6 @@ STATIC mp_obj_t bleio_descriptor_make_new(const mp_obj_type_t *type, size_t n_ar
 
 STATIC void bleio_descriptor_print(const mp_print_t *print, mp_obj_t self_in, mp_print_kind_t kind) {
     bleio_descriptor_obj_t *self = MP_OBJ_TO_PTR(self_in);
-
     common_hal_bleio_descriptor_print(self, print);
 }
 
@@ -124,9 +120,7 @@ const mp_obj_property_t bleio_descriptor_handle_obj = {
 
 STATIC mp_obj_t bleio_descriptor_get_uuid(mp_obj_t self_in) {
     bleio_descriptor_obj_t *self = MP_OBJ_TO_PTR(self_in);
-    const mp_obj_t uuid = mp_obj_new_int(common_hal_bleio_descriptor_get_uuid(self));
-
-    return bleio_uuid_type.make_new(&bleio_uuid_type, 1, 0, &uuid);
+    return common_hal_bleio_descriptor_get_uuid(self);
 }
 MP_DEFINE_CONST_FUN_OBJ_1(bleio_descriptor_get_uuid_obj, bleio_descriptor_get_uuid);
 
diff --git a/shared-bindings/bleio/Descriptor.h b/shared-bindings/bleio/Descriptor.h
index 85310d304..bef91061b 100644
--- a/shared-bindings/bleio/Descriptor.h
+++ b/shared-bindings/bleio/Descriptor.h
@@ -35,6 +35,6 @@ 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);
 extern void common_hal_bleio_descriptor_print(bleio_descriptor_obj_t *self, const mp_print_t *print);
 extern mp_int_t common_hal_bleio_descriptor_get_handle(bleio_descriptor_obj_t *self);
-extern mp_int_t common_hal_bleio_descriptor_get_uuid(bleio_descriptor_obj_t *self);
+extern mp_obj_t common_hal_bleio_descriptor_get_uuid(bleio_descriptor_obj_t *self);
 
 #endif // MICROPY_INCLUDED_SHARED_BINDINGS_BLEIO_DESCRIPTOR_H
diff --git a/shared-bindings/bleio/Service.c b/shared-bindings/bleio/Service.c
index 727350215..275a61eec 100644
--- a/shared-bindings/bleio/Service.c
+++ b/shared-bindings/bleio/Service.c
@@ -67,8 +67,8 @@ STATIC void bleio_service_print(const mp_print_t *print, mp_obj_t self_in, mp_pr
     bleio_service_obj_t *self = MP_OBJ_TO_PTR(self_in);
 
     mp_printf(print, "Service(");
-    common_hal_bleio_uuid_print(print, self->uuid);
-    mp_print(print, ")");
+    common_hal_bleio_uuid_print(self->uuid, print);
+    mp_printf(print, ")");
 }
 
 STATIC mp_obj_t bleio_service_make_new(const mp_obj_type_t *type, size_t n_args, size_t n_kw, const mp_obj_t *pos_args) {
@@ -112,9 +112,10 @@ STATIC mp_obj_t bleio_service_add_characteristic(mp_obj_t self_in, mp_obj_t char
     bleio_service_obj_t *self = MP_OBJ_TO_PTR(self_in);
     bleio_characteristic_obj_t *characteristic = MP_OBJ_TO_PTR(characteristic_in);
 
-    if (self->uuid->type == UUID_TYPE_128BIT) {
-        characteristic->uuid->type = UUID_TYPE_128BIT;
-        characteristic->uuid->uuid_vs_idx = self->uuid->uuid_vs_idx;
+    if (common_hal_bleio_uuid_get_uuid128_handle(self->uuid) !=
+        common_hal_bleio_uuid_get_uuid128_handle(characteristic->uuid)) {
+        // The descriptor base UUID doesn't match the characteristic base UUID.
+        mp_raise_ValueError(translate("Characteristic UUID doesn't match Descriptor UUID"));
     }
 
     characteristic->service = self;
diff --git a/shared-bindings/bleio/UUID.c b/shared-bindings/bleio/UUID.c
index 33d280304..a326518e6 100644
--- a/shared-bindings/bleio/UUID.c
+++ b/shared-bindings/bleio/UUID.c
@@ -27,6 +27,7 @@
  */
 
 #include "py/objproperty.h"
+#include "py/objstr.h"
 #include "py/runtime.h"
 #include "shared-bindings/bleio/UUID.h"
 
@@ -63,6 +64,9 @@ STATIC uint8_t xdigit_8b_value(byte nibble1, byte nibble2) {
 STATIC mp_obj_t bleio_uuid_make_new(const mp_obj_type_t *type, size_t n_args, size_t n_kw, const mp_obj_t *pos_args) {
     mp_arg_check_num(n_args, n_kw, 1, 1, true);
 
+    bleio_uuid_obj_t *self = m_new_obj(bleio_uuid_obj_t);
+    self->base.type = type;
+
     mp_map_t kw_args;
     mp_map_init_fixed_table(&kw_args, n_kw, pos_args + n_args);
 
@@ -98,7 +102,7 @@ STATIC mp_obj_t bleio_uuid_make_new(const mp_obj_type_t *type, size_t n_args, si
             while (str_index >= 1 && uuid128_index < UUID128_BYTE_LEN) {
                 if (str[str_index] == '-') {
                     // Skip hyphen separators.
-                    str--;
+                    str_index--;
                     continue;
                 }
 
@@ -126,6 +130,8 @@ STATIC mp_obj_t bleio_uuid_make_new(const mp_obj_type_t *type, size_t n_args, si
     } else {
         mp_raise_ValueError(translate("UUID value is not int or string"));
     }
+    // Unreachable.
+    return mp_const_none;
 }
 
 STATIC void bleio_uuid_print(const mp_print_t *print, mp_obj_t self_in, mp_print_kind_t kind) {
@@ -138,42 +144,64 @@ STATIC void bleio_uuid_print(const mp_print_t *print, mp_obj_t self_in, mp_print
 //|
 //|     The 16-bit part of the UUID. (read-only)
 //|
-STATIC void bleio_uuid_get_uuid16(mp_obj_t self_in) {
+STATIC mp_obj_t bleio_uuid_get_uuid16(mp_obj_t self_in) {
     bleio_uuid_obj_t *self = MP_OBJ_TO_PTR(self_in);
     return MP_OBJ_NEW_SMALL_INT(common_hal_bleio_uuid_get_uuid16(self));
 }
 
 MP_DEFINE_CONST_FUN_OBJ_1(bleio_uuid_get_uuid16_obj, bleio_uuid_get_uuid16);
 
-const mp_obj_property_t bleio_uuid16_obj = {
+const mp_obj_property_t bleio_uuid_uuid16_obj = {
     .base.type = &mp_type_property,
     .proxy = {(mp_obj_t)&bleio_uuid_get_uuid16_obj,
               (mp_obj_t)&mp_const_none_obj,
               (mp_obj_t)&mp_const_none_obj},
 };
 
-//|   .. attribute:: vendor_specific
+//|   .. attribute:: uuid128_handle
+//|
+//|     An opaque handle representing the 128-bit UUID. (read-only)
+//|     Returns None if this is a 16-bit UUID.
+//|
+STATIC mp_obj_t bleio_uuid_get_uuid128_handle(mp_obj_t self_in) {
+    bleio_uuid_obj_t *self = MP_OBJ_TO_PTR(self_in);
+    uint32_t handle = common_hal_bleio_uuid_get_uuid128_handle(self);
+    return handle == 0 ? mp_const_none : MP_OBJ_NEW_SMALL_INT(handle);
+}
+
+MP_DEFINE_CONST_FUN_OBJ_1(bleio_uuid_get_uuid128_handle_obj, bleio_uuid_get_uuid128_handle);
+
+const mp_obj_property_t bleio_uuid_uuid128_handle_obj = {
+    .base.type = &mp_type_property,
+    .proxy = {(mp_obj_t)&bleio_uuid_get_uuid128_handle_obj,
+              (mp_obj_t)&mp_const_none_obj,
+              (mp_obj_t)&mp_const_none_obj},
+};
+
+//|   .. attribute:: size
 //|
-//|     True if this UUID represents a 128-bit vendor-specific UUID.
-//|     False if this UUID represents a 16-bit Bluetooth SIG assigned UUID. (read-only)
+//|     Returns 128 if this UUID represents a 128-bit vendor-specific UUID.
+//|     Returns 16 if this UUID represents a 16-bit Bluetooth SIG assigned UUID. (read-only)
+//|     32-bit UUIDs are not currently supported.
 //|
-STATIC mp_obj_t bleio_uuid_get_vendor_specific(mp_obj_t self_in) {
+STATIC mp_obj_t bleio_uuid_get_size(mp_obj_t self_in) {
     bleio_uuid_obj_t *self = MP_OBJ_TO_PTR(self_in);
-    return mp_obj_new_bool(common_hal_bleio_uuid_get_vendor_specific(self));
+    return MP_OBJ_NEW_SMALL_INT(common_hal_bleio_uuid_get_size(self));
 }
 
-MP_DEFINE_CONST_FUN_OBJ_1(bleio_uuid_get_vendor_specific_obj, bleio_uuid_get_vendor_specific);
+MP_DEFINE_CONST_FUN_OBJ_1(bleio_uuid_get_size_obj, bleio_uuid_get_size);
 
-const mp_obj_property_t bleio_uuid_vendor_specific_obj = {
+const mp_obj_property_t bleio_uuid_size_obj = {
     .base.type = &mp_type_property,
-    .proxy = {(mp_obj_t)&bleio_uuid_get_vendor_specific_obj,
+    .proxy = {(mp_obj_t)&bleio_uuid_get_size_obj,
               (mp_obj_t)&mp_const_none_obj,
               (mp_obj_t)&mp_const_none_obj},
 };
 
 STATIC const mp_rom_map_elem_t bleio_uuid_locals_dict_table[] = {
-    { MP_ROM_QSTR(MP_QSTR_uuid16), MP_ROM_PTR(&bleio_uuid16_obj) },
-    { MP_ROM_QSTR(MP_QSTR_vendor_specific), MP_ROM_PTR(&bleio_uuid_vendor_specific_obj) },
+    { MP_ROM_QSTR(MP_QSTR_uuid16), MP_ROM_PTR(&bleio_uuid_uuid16_obj) },
+    { MP_ROM_QSTR(MP_QSTR_uuid128_handle), MP_ROM_PTR(&bleio_uuid_uuid128_handle_obj) },
+    { MP_ROM_QSTR(MP_QSTR_size), MP_ROM_PTR(&bleio_uuid_size_obj) },
 };
 
 STATIC MP_DEFINE_CONST_DICT(bleio_uuid_locals_dict, bleio_uuid_locals_dict_table);
diff --git a/shared-bindings/bleio/UUID.h b/shared-bindings/bleio/UUID.h
index e27058516..79810dc97 100644
--- a/shared-bindings/bleio/UUID.h
+++ b/shared-bindings/bleio/UUID.h
@@ -33,7 +33,8 @@ extern const mp_obj_type_t bleio_uuid_type;
 
 extern void common_hal_bleio_uuid_construct(bleio_uuid_obj_t *self, mp_int_t uuid16, uint8_t uuid128[]);
 extern void common_hal_bleio_uuid_print(bleio_uuid_obj_t *self, const mp_print_t *print);
-extern bool common_hal_bleio_uuid_get_vendor_specific(bleio_uuid_obj_t *self);
 extern uint32_t common_hal_bleio_uuid_get_uuid16(bleio_uuid_obj_t *self);
+extern uint32_t common_hal_bleio_uuid_get_uuid128_handle(bleio_uuid_obj_t *self);
+extern uint32_t common_hal_bleio_uuid_get_size(bleio_uuid_obj_t *self);
 
 #endif // MICROPY_INCLUDED_SHARED_BINDINGS_BLEIO_UUID_H