at least original functionality with new API

Author: dhalbert

locale/circuitpython.pot

@@ -91,6 +91,10 @@ msgstr ""
 msgid "%q length must be %q"
 msgstr ""
 
+#: shared-bindings/usb_hid/Device.c
+msgid "%q length must be >= 1"
+msgstr ""
+
 #: shared-bindings/vectorio/Polygon.c
 msgid "%q list must be a list"
 msgstr ""
@@ -1226,7 +1230,7 @@ msgstr ""
 msgid "Internal error #%d"
 msgstr ""
 
-#: shared-bindings/sdioio/SDCard.c
+#: shared-bindings/sdioio/SDCard.c shared-module/usb_hid/Device.c
 msgid "Invalid %q"
 msgstr ""
 

shared-bindings/usb_hid/Device.c

@@ -31,7 +31,7 @@
 //| class Device:
 //|     """HID Device specification"""
 //|
-//|     def __init__(self, *, descriptor: ReadableBuffer, usage_page: int, usage: int, in_report_lengths: Sequence[int], out_report_lengths: Sequence[int]) -> None:
+//|     def __init__(self, *, descriptor: ReadableBuffer, usage_page: int, usage: int, report_ids: Sequence[int], in_report_lengths: Sequence[int], out_report_lengths: Sequence[int]) -> None:
 //|         """Create a description of a USB HID device. The actual device is created when you
 //|         pass a `Device` to `usb_hid.enable()`.
 //|
@@ -40,6 +40,7 @@
 //|         :param int usage_page: The Usage Page value from the descriptor. Must match what is in the descriptor.
 //|         :param int usage: The Usage value from the descriptor. Must match what is in the descriptor.
 //|         :param int report_ids: Sequence of report ids used by the descriptor.
+//|           If the ``report_descriptor`` does not have a report ID, use 0.
 //|         :param int in_report_lengths: Sequence of sizes in bytes of the HIDs report sent to the host.
 //|           The sizes are in order of the ``report_ids``.
 //|           "IN" is with respect to the host.
@@ -59,12 +60,12 @@
 //|     MOUSE: Device
 //|     """Standard mouse device supporting five mouse buttons, X and Y relative movements from -127 to 127
 //|     in each report, and a relative mouse wheel change from -127 to 127 in each report.
-//|     Uses Report ID 2 for its IN reports.
+//|     Uses Report ID 2 for its IN report.
 //|     """
 //|
 //|     CONSUMER_CONTROL: Device
 //|     """Consumer Control device supporting sent values from 1-652, with no rollover.
-//|     Uses Report ID 3 for its IN reports."""
+//|     Uses Report ID 3 for its IN report."""
 //|
 
 STATIC mp_obj_t usb_hid_device_make_new(const mp_obj_type_t *type, size_t n_args, const mp_obj_t *pos_args, mp_map_t *kw_args) {
@@ -100,8 +101,12 @@ STATIC mp_obj_t usb_hid_device_make_new(const mp_obj_type_t *type, size_t n_args
     mp_obj_t out_report_lengths = args[ARG_out_report_lengths].u_obj;
 
     size_t report_ids_count = (size_t)MP_OBJ_SMALL_INT_VALUE(mp_obj_len(report_ids));
-    if (MP_OBJ_SMALL_INT_VALUE(mp_obj_len(in_report_lengths)) != report_ids_count ||
-        MP_OBJ_SMALL_INT_VALUE(mp_obj_len(out_report_lengths)) != report_ids_count) {
+    if (report_ids_count < 1) {
+        mp_raise_ValueError_varg(translate("%q length must be >= 1"), MP_QSTR_report_ids);
+    }
+
+    if ((size_t)MP_OBJ_SMALL_INT_VALUE(mp_obj_len(in_report_lengths)) != report_ids_count ||
+        (size_t)MP_OBJ_SMALL_INT_VALUE(mp_obj_len(out_report_lengths)) != report_ids_count) {
         mp_raise_ValueError_varg(translate("%q, %q, and %q must all be the same length"),
             MP_QSTR_report_ids, MP_QSTR_in_report_lengths, MP_QSTR_out_report_lengths);
     }
@@ -136,7 +141,9 @@ STATIC mp_obj_t usb_hid_device_make_new(const mp_obj_type_t *type, size_t n_args
 
 
 //|     def send_report(self, buf: ReadableBuffer, report_id: Optional[int] = None) -> None:
-//|         """Send an HID report.
+//|         """Send an HID report. If the device descriptor specifies zero or one report id's,
+//|         you can supply `None` (the default) as the value of ``report_id``.
+//|         Otherwise you must specify which report id to use when sending the report.
 //|         """
 //|         ...
 //|
@@ -155,22 +162,23 @@ STATIC mp_obj_t usb_hid_device_send_report(size_t n_args, const mp_obj_t *pos_ar
     mp_buffer_info_t bufinfo;
     mp_get_buffer_raise(args[ARG_buf].u_obj, &bufinfo, MP_BUFFER_READ);
 
-    uint8_t report_id = 0;
+    // -1 asks common_hal to determine the report id if possible.
+    mp_int_t report_id_arg = -1;
     if (args[ARG_report_id].u_obj != mp_const_none) {
-        const mp_int_t report_id_arg = mp_obj_int_get_checked(args[ARG_report_id].u_obj);
-        report_id = mp_arg_validate_int_range(report_id_arg, 1, 255, MP_QSTR_report_id);
+        report_id_arg = mp_obj_int_get_checked(args[ARG_report_id].u_obj);
     }
+    const uint8_t report_id = common_hal_usb_hid_device_validate_report_id(self, report_id_arg);
 
     common_hal_usb_hid_device_send_report(self, ((uint8_t *)bufinfo.buf), bufinfo.len, report_id);
     return mp_const_none;
 }
 MP_DEFINE_CONST_FUN_OBJ_KW(usb_hid_device_send_report_obj, 2, usb_hid_device_send_report);
 
 //|     def get_last_received_report(self, report_id: Optional[int] = None) -> bytes:
-//|     """Get the last received HID OUT report for the given report ID.
-//|     The report ID may be omitted if there is no report ID, or only one report ID.
-//|     Return `None` if nothing received.
-//|     """
+//|         """Get the last received HID OUT report for the given report ID.
+//|         The report ID may be omitted if there is no report ID, or only one report ID.
+//|         Return `None` if nothing received.
+//|         """
 //|         ...
 //|
 STATIC mp_obj_t usb_hid_device_get_last_received_report(size_t n_args, const mp_obj_t *pos_args, mp_map_t *kw_args) {
@@ -184,17 +192,18 @@ STATIC mp_obj_t usb_hid_device_get_last_received_report(size_t n_args, const mp_
     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);
 
-    uint8_t report_id = 0;
+    mp_int_t report_id_arg = -1;
     if (args[ARG_report_id].u_obj != mp_const_none) {
-        report_id = mp_obj_int_get_checked(args[ARG_report_id].u_obj);
+        report_id_arg = mp_obj_int_get_checked(args[ARG_report_id].u_obj);
     }
+    const uint8_t report_id = common_hal_usb_hid_device_validate_report_id(self, report_id_arg);
 
     return common_hal_usb_hid_device_get_last_received_report(self, report_id);
 }
 MP_DEFINE_CONST_FUN_OBJ_KW(usb_hid_device_get_last_received_report_obj, 1, usb_hid_device_get_last_received_report);
 
 //|     last_received_report: bytes
-//|     """The HID OUT report as a `bytes`. (read-only). `None` if nothing received.
+//|     """The HID OUT report as a `bytes` (read-only). `None` if nothing received.
 //|     Same as `get_last_received_report()` with no argument.
 //|
 //|     Deprecated: will be removed in CircutPython 8.0.0. Use `get_last_received_report()` instead.
@@ -203,7 +212,9 @@ MP_DEFINE_CONST_FUN_OBJ_KW(usb_hid_device_get_last_received_report_obj, 1, usb_h
 STATIC mp_obj_t usb_hid_device_obj_get_last_received_report_property(mp_obj_t self_in) {
     usb_hid_device_obj_t *self = MP_OBJ_TO_PTR(self_in);
 
-    return common_hal_usb_hid_device_get_last_received_report(self, 0);
+    // Get the sole report_id, if there is one.
+    const uint8_t report_id = common_hal_usb_hid_device_validate_report_id(self, -1);
+    return common_hal_usb_hid_device_get_last_received_report(self, report_id);
 }
 MP_DEFINE_CONST_FUN_OBJ_1(usb_hid_device_get_last_received_report_property_obj, usb_hid_device_obj_get_last_received_report_property);
 
@@ -215,7 +226,7 @@ const mp_obj_property_t usb_hid_device_last_received_report_obj = {
 };
 
 //|     usage_page: int
-//|     """The usage page of the device as an `int`. Can be thought of a category. (read-only)"""
+//|     """The device usage page identifier, which designates a category of device. (read-only)"""
 //|
 STATIC mp_obj_t usb_hid_device_obj_get_usage_page(mp_obj_t self_in) {
     usb_hid_device_obj_t *self = MP_OBJ_TO_PTR(self_in);
@@ -231,7 +242,7 @@ const mp_obj_property_t usb_hid_device_usage_page_obj = {
 };
 
 //|     usage: int
-//|     """The functionality of the device as an int. (read-only)
+//|     """The device usage identifier, which designates a specific kind of device. (read-only)
 //|
 //|     For example, Keyboard is 0x06 within the generic desktop usage page 0x01.
 //|     Mouse is 0x02 within the same usage page."""

shared-bindings/usb_hid/Device.h

@@ -38,6 +38,6 @@ void common_hal_usb_hid_device_send_report(usb_hid_device_obj_t *self, uint8_t *
 mp_obj_t common_hal_usb_hid_device_get_last_received_report(usb_hid_device_obj_t *self, uint8_t report_id);
 uint8_t common_hal_usb_hid_device_get_usage_page(usb_hid_device_obj_t *self);
 uint8_t common_hal_usb_hid_device_get_usage(usb_hid_device_obj_t *self);
-bool common_hal_usb_hid_device_valid_report_id(usb_hid_device_obj_t *self, uint8_t report_id);
+uint8_t common_hal_usb_hid_device_validate_report_id(usb_hid_device_obj_t *self, mp_int_t report_id);
 
 #endif  // MICROPY_INCLUDED_SHARED_BINDINGS_USB_HID_DEVICE_H

shared-bindings/usb_hid/__init__.c

@@ -71,7 +71,7 @@ MP_DEFINE_CONST_FUN_OBJ_0(usb_hid_disable_obj, usb_hid_disable);
 //|
 //|     If you enable too many devices at once, you will run out of USB endpoints.
 //|     The number of available endpoints varies by microcontroller.
-//|     CircuitPython will go into safe mode after running boot.py to inform you if
+//|     CircuitPython will go into safe mode after running ``boot.py`` to inform you if
 //|     not enough endpoints are available.
 //|     """
 //|     ...

shared-module/usb_hid/Device.c

@@ -79,7 +79,7 @@ const usb_hid_device_obj_t usb_hid_device_keyboard_obj = {
     .usage = 0x06,
     .num_report_ids = 1,
     .report_ids = { 0x01, },
-    .in_report_lengths = { 8, 0, 0, 0, },
+    .in_report_lengths = { 8, 0, 0, 0, 0, 0, },
     .out_report_lengths = { 1, },
 };
 
@@ -129,7 +129,7 @@ const usb_hid_device_obj_t usb_hid_device_mouse_obj = {
     .usage = 0x02,
     .num_report_ids = 1,
     .report_ids = { 0x02, },
-    .in_report_lengths = { 4, 0, 0, 0, },
+    .in_report_lengths = { 4, 0, 0, 0, 0, 0, },
     .out_report_lengths = { 0, },
 };
 
@@ -158,18 +158,30 @@ const usb_hid_device_obj_t usb_hid_device_consumer_control_obj = {
     .usage = 0x01,
     .num_report_ids = 1,
     .report_ids = { 0x03 },
-    .in_report_lengths = { 2, 0, 0, 0 },
-    .out_report_lengths = { 0 },
+    .in_report_lengths = { 2, 0, 0, 0, 0, 0, },
+    .out_report_lengths = { 0, },
 };
 
-
-bool common_hal_usb_hid_device_valid_report_id(usb_hid_device_obj_t *self, uint8_t report_id) {
+STATIC size_t get_report_id_idx(usb_hid_device_obj_t *self, size_t report_id) {
     for (size_t i = 0; i < self->num_report_ids; i++) {
         if (report_id == self->report_ids[i]) {
-            return true;
+            return i;
         }
     }
-    return false;
+    return MAX_REPORT_IDS_PER_DESCRIPTOR;
+}
+
+// See if report_id is used by this device. If it is -1, then return the sole report id used by this device,
+// which might be 0 if no report_id was supplied.
+uint8_t common_hal_usb_hid_device_validate_report_id(usb_hid_device_obj_t *self, mp_int_t report_id_arg) {
+    if (report_id_arg == -1 && self->num_report_ids == 1) {
+        return self->report_ids[0];
+    }
+    if (!(report_id_arg >= 0 &&
+          get_report_id_idx(self, (size_t)report_id_arg) < MAX_REPORT_IDS_PER_DESCRIPTOR)) {
+        mp_raise_ValueError_varg(translate("Invalid %q"), MP_QSTR_report_id);
+    }
+    return (uint8_t)report_id_arg;
 }
 
 void common_hal_usb_hid_device_construct(usb_hid_device_obj_t *self, mp_obj_t report_descriptor, uint8_t usage_page, uint8_t usage, size_t num_report_ids, uint8_t *report_ids, uint8_t *in_report_lengths, uint8_t *out_report_lengths) {
@@ -183,7 +195,7 @@ void common_hal_usb_hid_device_construct(usb_hid_device_obj_t *self, mp_obj_t re
     mp_get_buffer_raise(report_descriptor, &bufinfo, MP_BUFFER_READ);
     self->report_descriptor_length = bufinfo.len;
 
-    // Copy the raw the descriptor bytes into a heap obj. We don't keep the Python descriptor object.
+    // Copy the raw descriptor bytes into a heap obj. We don't keep the Python descriptor object.
 
     uint8_t *descriptor_bytes = gc_alloc(bufinfo.len, false, false);
     memcpy(descriptor_bytes, bufinfo.buf, bufinfo.len);
@@ -206,8 +218,12 @@ uint8_t common_hal_usb_hid_device_get_usage(usb_hid_device_obj_t *self) {
 }
 
 void common_hal_usb_hid_device_send_report(usb_hid_device_obj_t *self, uint8_t *report, uint8_t len, uint8_t report_id) {
-    if (len != self->in_report_length) {
-        mp_raise_ValueError_varg(translate("Buffer incorrect size. Should be %d bytes."), self->in_report_length);
+    // report_id and len have already been validated for this device.
+    size_t id_idx = get_report_id_idx(self, report_id);
+
+    if (len != self->in_report_lengths[id_idx]) {
+        mp_raise_ValueError_varg(translate("Buffer incorrect size. Should be %d bytes."),
+            self->in_report_lengths[id_idx]);
     }
 
     // Wait until interface is ready, timeout = 2 seconds
@@ -225,14 +241,25 @@ void common_hal_usb_hid_device_send_report(usb_hid_device_obj_t *self, uint8_t *
     }
 }
 
+mp_obj_t common_hal_usb_hid_device_get_last_received_report(usb_hid_device_obj_t *self, uint8_t report_id) {
+    // report_id has already been validated for this deveice.
+    size_t id_idx = get_report_id_idx(self, report_id);
+    return mp_obj_new_bytes(self->out_report_buffers[id_idx], self->out_report_lengths[id_idx]);
+}
 
 void usb_hid_device_create_report_buffers(usb_hid_device_obj_t *self) {
-    for (size_t i = 0; i < self->num_report_ids; count++) {
-        if (self->out_report_length > 0) {
-            self->out_report_buffers[i] = self->out_report_lengths[i] > 0
-                ? gc_alloc(self->out_report_length, false, true /*long-lived*/)
-                : NULL;
-        }
+    for (size_t i = 0; i < self->num_report_ids; i++) {
+        // The IN buffers are used only for tud_hid_get_report_cb(),
+        // which is an unusual case. Normally we can just pass the data directly with tud_hid_report().
+        self->in_report_buffers[i] =
+            self->in_report_lengths[i] > 0
+            ? gc_alloc(self->in_report_lengths[i], false, true /*long-lived*/)
+            : NULL;
+
+        self->out_report_buffers[i] =
+            self->out_report_lengths[i] > 0
+            ? gc_alloc(self->out_report_lengths[i], false, true /*long-lived*/)
+            : NULL;
     }
 }
 
@@ -254,25 +281,27 @@ uint16_t tud_hid_get_report_cb(uint8_t itf, uint8_t report_id, hid_report_type_t
         memcpy(buffer, hid_device->in_report_buffers[id_idx], reqlen);
         return reqlen;
     }
+    return 0;
+}
 
 // Callbacks invoked when we received Set_Report request through control endpoint
-    void tud_hid_set_report_cb(uint8_t itf, uint8_t report_id, hid_report_type_t report_type, uint8_t const *buffer, uint16_t bufsize) {
-        (void)itf;
-        if (report_type == HID_REPORT_TYPE_INVALID) {
-            report_id = buffer[0];
-            buffer++;
-            bufsize--;
-        } else if (report_type != HID_REPORT_TYPE_OUTPUT) {
-            return;
-        }
+void tud_hid_set_report_cb(uint8_t itf, uint8_t report_id, hid_report_type_t report_type, uint8_t const *buffer, uint16_t bufsize) {
+    (void)itf;
+    if (report_type == HID_REPORT_TYPE_INVALID) {
+        report_id = buffer[0];
+        buffer++;
+        bufsize--;
+    } else if (report_type != HID_REPORT_TYPE_OUTPUT) {
+        return;
+    }
 
-        usb_hid_device_obj_t *hid_device;
-        size_t id_idx;
-        // Find device with this report id, and get the report id index.
-        if (usb_hid_get_device_with_report_id(report_id, &hid_device, &id_idx)) {
-            // If a report of the correct size has been read, save it in the proper OUT report buffer.
-            if (hid_device && hid_device->out_report_lengths[id_idx] >= bufsize) {
-                memcpy(hid_device->out_report_buffers[id_idx], buffer, bufsize);
-            }
+    usb_hid_device_obj_t *hid_device;
+    size_t id_idx;
+    // Find device with this report id, and get the report id index.
+    if (usb_hid_get_device_with_report_id(report_id, &hid_device, &id_idx)) {
+        // If a report of the correct size has been read, save it in the proper OUT report buffer.
+        if (hid_device && hid_device->out_report_lengths[id_idx] >= bufsize) {
+            memcpy(hid_device->out_report_buffers[id_idx], buffer, bufsize);
         }
     }
+}

shared-module/usb_hid/Device.h

@@ -32,7 +32,8 @@
 
 #include "py/obj.h"
 
-// The most complicated one currently know of is the head and eye tracker, which requires 5:
+// The most complicated device currently known of is the head and eye tracker, which requires 5
+// report ids.
 // https://usb.org/sites/default/files/hutrr74_-_usage_page_for_head_and_eye_trackers_0.pdf
 #define MAX_REPORT_IDS_PER_DESCRIPTOR (6)
 
@@ -43,6 +44,7 @@ typedef struct  {
     uint8_t report_ids[MAX_REPORT_IDS_PER_DESCRIPTOR];
     uint8_t in_report_lengths[MAX_REPORT_IDS_PER_DESCRIPTOR];
     uint8_t out_report_lengths[MAX_REPORT_IDS_PER_DESCRIPTOR];
+    uint8_t *in_report_buffers[MAX_REPORT_IDS_PER_DESCRIPTOR];
     uint8_t *out_report_buffers[MAX_REPORT_IDS_PER_DESCRIPTOR];
     uint16_t report_descriptor_length;
     uint8_t usage_page;