Tweaks from review

py/circuitpy_defns.mk

@@ -646,6 +646,7 @@ endif
 SRC_SHARED_MODULE_INTERNAL = \
 $(filter $(SRC_PATTERNS), \
 	displayio/display_core.c \
+	usb/utf16le.c \
 )
 
 SRC_COMMON_HAL_INTERNAL = \

shared-bindings/usb/core/Device.c

@@ -163,7 +163,7 @@ const mp_obj_property_t usb_core_device_manufacturer_obj = {
 STATIC mp_obj_t usb_core_device_write(size_t n_args, const mp_obj_t *pos_args, mp_map_t *kw_args) {
     enum { ARG_endpoint, ARG_data, ARG_timeout };
     static const mp_arg_t allowed_args[] = {
-        { MP_QSTR_endpoint, MP_ARG_REQUIRED | MP_ARG_OBJ },
+        { MP_QSTR_endpoint, MP_ARG_REQUIRED | MP_ARG_INT },
         { MP_QSTR_data, MP_ARG_REQUIRED | MP_ARG_OBJ },
         { MP_QSTR_timeout, MP_ARG_INT, {.u_int = 0} },
     };
@@ -191,7 +191,7 @@ MP_DEFINE_CONST_FUN_OBJ_KW(usb_core_device_write_obj, 2, usb_core_device_write);
 STATIC mp_obj_t usb_core_device_read(size_t n_args, const mp_obj_t *pos_args, mp_map_t *kw_args) {
     enum { ARG_endpoint, ARG_size_or_buffer, ARG_timeout };
     static const mp_arg_t allowed_args[] = {
-        { MP_QSTR_endpoint, MP_ARG_REQUIRED | MP_ARG_OBJ },
+        { MP_QSTR_endpoint, MP_ARG_REQUIRED | MP_ARG_INT },
         { MP_QSTR_size_or_buffer, MP_ARG_REQUIRED | MP_ARG_OBJ },
         { MP_QSTR_timeout, MP_ARG_INT, {.u_int = 0} },
     };

shared-bindings/usb/core/__init__.c

@@ -25,6 +25,7 @@
  */
 
 #include <stdarg.h>
+#include <string.h>
 
 #include "py/obj.h"
 #include "py/objexcept.h"
@@ -97,8 +98,6 @@ STATIC mp_obj_t _next_device(usb_core_devices_obj_t *iter) {
         usb_core_device_obj_t *self = m_new_obj(usb_core_device_obj_t);
         self->base.type = &usb_core_device_type;
 
-        mp_printf(&mp_plat_print, "USB device %d matches\n", i);
-
         common_hal_usb_core_device_construct(self, i);
         iter->next_index = i + 1;
         return MP_OBJ_FROM_PTR(self);
@@ -132,29 +131,31 @@ STATIC mp_obj_t usb_core_find(size_t n_args, const mp_obj_t *pos_args, mp_map_t
     enum { ARG_find_all, ARG_idVendor, ARG_idProduct };
     static const mp_arg_t allowed_args[] = {
         { MP_QSTR_find_all, MP_ARG_BOOL, {.u_bool = false} },
-        { MP_QSTR_idVendor, MP_ARG_INT, {.u_int = 0x10000} },
+        { MP_QSTR_idVendor, MP_ARG_OBJ, {.u_obj = mp_const_none} },
-        { MP_QSTR_idProduct, MP_ARG_INT, {.u_int = 0x10000} },
+        { MP_QSTR_idProduct, 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);
 
-    bool find_all = args[ARG_find_all].u_bool;
+    const bool find_all = args[ARG_find_all].u_bool;
     usb_core_devices_obj_t temp_iter;
-    usb_core_devices_obj_t *iter;
+    temp_iter.base.type = &usb_core_devices_type;
+    temp_iter.next_index = 1;
+    if (!mp_obj_get_int_maybe(args[ARG_idVendor].u_obj, &temp_iter.vid)) {
+        temp_iter.vid = 0x10000;
+    }
+    if (!mp_obj_get_int_maybe(args[ARG_idProduct].u_obj, &temp_iter.pid)) {
+        temp_iter.pid = 0x10000;
+    }
     if (find_all) {
-        iter = m_new_obj(usb_core_devices_obj_t);
+        // Copy the temp iter contents to a heap object before we return it.
-        iter->base.type = &usb_core_devices_type;
+        // We could do this up front but GCC falsely detects that we may return
-    } else {
+        // the stack copy.
-        iter = &temp_iter;
+        usb_core_devices_obj_t *iter = m_new_obj(usb_core_devices_obj_t);
+        memcpy(iter, &temp_iter, sizeof(usb_core_devices_obj_t));
+        return MP_OBJ_FROM_PTR(iter);
     }
-    iter->next_index = 1;
+    return _next_device(&temp_iter);
-    iter->vid = args[ARG_idVendor].u_int;
-    iter->pid = args[ARG_idProduct].u_int;
-    if (!find_all) {
-        return _next_device(iter);
-    }
-
-    return MP_OBJ_FROM_PTR(iter);
 }
 MP_DEFINE_CONST_FUN_OBJ_KW(usb_core_find_obj, 0, usb_core_find);
 

shared-module/usb/core/Device.c

@@ -32,6 +32,7 @@
 #include "py/runtime.h"
 #include "shared/runtime/interrupt_char.h"
 #include "shared-bindings/usb/core/__init__.h"
+#include "shared-module/usb/utf16le.h"
 #include "supervisor/shared/tick.h"
 
 bool common_hal_usb_core_device_construct(usb_core_device_obj_t *self, uint8_t device_number) {
@@ -49,7 +50,6 @@ uint16_t common_hal_usb_core_device_get_idVendor(usb_core_device_obj_t *self) {
     uint16_t vid;
     uint16_t pid;
     tuh_vid_pid_get(self->device_number, &vid, &pid);
-    mp_printf(&mp_plat_print, "%d vid %04x pid %04x\n", self->device_number, vid, pid);
     return vid;
 }
 
@@ -62,60 +62,13 @@ uint16_t common_hal_usb_core_device_get_idProduct(usb_core_device_obj_t *self) {
 
 STATIC xfer_result_t _get_string_result;
 STATIC bool _transfer_done_cb(uint8_t daddr, tusb_control_request_t const *request, xfer_result_t result) {
+    // Store the result so we stop waiting for the transfer. We don't need the other data for now.
     (void)daddr;
     (void)request;
     _get_string_result = result;
     return true;
 }
 
-
-STATIC void _convert_utf16le_to_utf8(const uint16_t *utf16, size_t utf16_len, uint8_t *utf8, size_t utf8_len) {
-    // TODO: Check for runover.
-    (void)utf8_len;
-
-    for (size_t i = 0; i < utf16_len; i++) {
-        uint16_t chr = utf16[i];
-        if (chr < 0x80) {
-            *utf8++ = chr & 0xff;
-        } else if (chr < 0x800) {
-            *utf8++ = (uint8_t)(0xC0 | (chr >> 6 & 0x1F));
-            *utf8++ = (uint8_t)(0x80 | (chr >> 0 & 0x3F));
-        } else if (chr < 0x10000) {
-            // TODO: Verify surrogate.
-            *utf8++ = (uint8_t)(0xE0 | (chr >> 12 & 0x0F));
-            *utf8++ = (uint8_t)(0x80 | (chr >> 6 & 0x3F));
-            *utf8++ = (uint8_t)(0x80 | (chr >> 0 & 0x3F));
-        } else {
-            // TODO: Handle UTF-16 code points that take two entries.
-            uint32_t hc = ((chr & 0xFFFF0000) - 0xD8000000) >> 6;    /* Get high 10 bits */
-            chr = (chr & 0xFFFF) - 0xDC00;                  /* Get low 10 bits */
-            chr = (hc | chr) + 0x10000;
-            *utf8++ = (uint8_t)(0xF0 | (chr >> 18 & 0x07));
-            *utf8++ = (uint8_t)(0x80 | (chr >> 12 & 0x3F));
-            *utf8++ = (uint8_t)(0x80 | (chr >> 6 & 0x3F));
-            *utf8++ = (uint8_t)(0x80 | (chr >> 0 & 0x3F));
-        }
-    }
-}
-
-// Count how many bytes a utf-16-le encoded string will take in utf-8.
-STATIC mp_int_t _count_utf8_bytes(const uint16_t *buf, size_t len) {
-    size_t total_bytes = 0;
-    for (size_t i = 0; i < len; i++) {
-        uint16_t chr = buf[i];
-        if (chr < 0x80) {
-            total_bytes += 1;
-        } else if (chr < 0x800) {
-            total_bytes += 2;
-        } else if (chr < 0x10000) {
-            total_bytes += 3;
-        } else {
-            total_bytes += 4;
-        }
-    }
-    return total_bytes;
-}
-
 STATIC void _wait_for_callback(void) {
     while (!mp_hal_is_interrupted() &&
            _get_string_result == 0xff) {
@@ -130,14 +83,7 @@ STATIC mp_obj_t _get_string(const uint16_t *temp_buf) {
     if (utf16_len == 0) {
         return mp_const_none;
     }
-    size_t size = _count_utf8_bytes(temp_buf + 1, utf16_len);
+    return utf16le_to_string(temp_buf + 1, utf16_len);
-    vstr_t vstr;
-    vstr_init_len(&vstr, size + 1);
-    byte *p = (byte *)vstr.buf;
-    // Null terminate.
-    p[size] = '\0';
-    _convert_utf16le_to_utf8(temp_buf + 1, utf16_len, p, size);
-    return mp_obj_new_str_from_vstr(&mp_type_str, &vstr);
 }
 
 mp_obj_t common_hal_usb_core_device_get_serial_number(usb_core_device_obj_t *self) {

shared-module/usb/utf16le.c

@@ -0,0 +1,85 @@
+/*
+ * This file is part of the MicroPython project, http://micropython.org/
+ *
+ * The MIT License (MIT)
+ *
+ * Copyright (c) 2022 Scott Shawcroft for Adafruit Industries
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a copy
+ * of this software and associated documentation files (the "Software"), to deal
+ * in the Software without restriction, including without limitation the rights
+ * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+ * copies of the Software, and to permit persons to whom the Software is
+ * furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
+ * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
+ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
+ * THE SOFTWARE.
+ */
+
+#include "shared-module/usb/utf16le.h"
+
+STATIC void _convert_utf16le_to_utf8(const uint16_t *utf16, size_t utf16_len, uint8_t *utf8, size_t utf8_len) {
+    // TODO: Check for runover.
+    (void)utf8_len;
+
+    for (size_t i = 0; i < utf16_len; i++) {
+        uint16_t chr = utf16[i];
+        if (chr < 0x80) {
+            *utf8++ = chr & 0xff;
+        } else if (chr < 0x800) {
+            *utf8++ = (uint8_t)(0xC0 | (chr >> 6 & 0x1F));
+            *utf8++ = (uint8_t)(0x80 | (chr >> 0 & 0x3F));
+        } else if (chr < 0x10000) {
+            // TODO: Verify surrogate.
+            *utf8++ = (uint8_t)(0xE0 | (chr >> 12 & 0x0F));
+            *utf8++ = (uint8_t)(0x80 | (chr >> 6 & 0x3F));
+            *utf8++ = (uint8_t)(0x80 | (chr >> 0 & 0x3F));
+        } else {
+            // TODO: Handle UTF-16 code points that take two entries.
+            uint32_t hc = ((chr & 0xFFFF0000) - 0xD8000000) >> 6;    /* Get high 10 bits */
+            chr = (chr & 0xFFFF) - 0xDC00;                  /* Get low 10 bits */
+            chr = (hc | chr) + 0x10000;
+            *utf8++ = (uint8_t)(0xF0 | (chr >> 18 & 0x07));
+            *utf8++ = (uint8_t)(0x80 | (chr >> 12 & 0x3F));
+            *utf8++ = (uint8_t)(0x80 | (chr >> 6 & 0x3F));
+            *utf8++ = (uint8_t)(0x80 | (chr >> 0 & 0x3F));
+        }
+    }
+}
+
+// Count how many bytes a utf-16-le encoded string will take in utf-8.
+STATIC mp_int_t _count_utf8_bytes(const uint16_t *buf, size_t len) {
+    size_t total_bytes = 0;
+    for (size_t i = 0; i < len; i++) {
+        uint16_t chr = buf[i];
+        if (chr < 0x80) {
+            total_bytes += 1;
+        } else if (chr < 0x800) {
+            total_bytes += 2;
+        } else if (chr < 0x10000) {
+            total_bytes += 3;
+        } else {
+            total_bytes += 4;
+        }
+    }
+    return total_bytes;
+}
+
+mp_obj_t utf16le_to_string(const uint16_t *buf, size_t utf16_len) {
+    size_t size = _count_utf8_bytes(buf, utf16_len);
+    vstr_t vstr;
+    vstr_init_len(&vstr, size + 1);
+    byte *p = (byte *)vstr.buf;
+    // Null terminate.
+    p[size] = '\0';
+    _convert_utf16le_to_utf8(buf, utf16_len, p, size);
+    return mp_obj_new_str_from_vstr(&mp_type_str, &vstr);
+}

shared-module/usb/utf16le.h

@@ -0,0 +1,34 @@
+/*
+ * This file is part of the MicroPython project, http://micropython.org/
+ *
+ * The MIT License (MIT)
+ *
+ * Copyright (c) 2022 Scott Shawcroft for Adafruit Industries
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a copy
+ * of this software and associated documentation files (the "Software"), to deal
+ * in the Software without restriction, including without limitation the rights
+ * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+ * copies of the Software, and to permit persons to whom the Software is
+ * furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
+ * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
+ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
+ * THE SOFTWARE.
+ */
+
+#ifndef MICROPY_INCLUDED_SHARED_MODULE_USB_UTF16LE_H
+#define MICROPY_INCLUDED_SHARED_MODULE_USB_UTF16LE_H
+
+#include "py/obj.h"
+
+mp_obj_t utf16le_to_string(const uint16_t *buf, size_t utf16_len);
+
+#endif // MICROPY_INCLUDED_SHARED_MODULE_USB_UTF16LE_H

tests/circuitpython-manual/usb/device_info.py

@@ -13,7 +13,7 @@ h = usb_host.Port(board.USB_HOST_DP, board.USB_HOST_DM)
 
 while True:
     for device in usb.core.find(find_all=True):
-        print(device.idVendor, device.idProduct)
+        print(f"{device.idVendor:04x}:{device.idProduct:04x}")
         print(device.manufacturer, device.product)
         print(device.serial_number)
     print()