From 4f8ff12afafb77982d30d8161de4a325ec6636b8 Mon Sep 17 00:00:00 2001 From: Dan Halbert Date: Wed, 28 Jul 2021 10:31:47 -0400 Subject: [PATCH 1/5] wip --- locale/circuitpython.pot | 21 ++-- py/argcheck.c | 2 +- shared-bindings/usb_hid/Device.c | 185 ++++++++++++++++++++----------- shared-bindings/usb_hid/Device.h | 6 +- shared-module/usb_hid/Device.c | 115 +++++++++++-------- shared-module/usb_hid/Device.h | 15 ++- shared-module/usb_hid/__init__.c | 123 +++++++++++--------- shared-module/usb_hid/__init__.h | 2 +- 8 files changed, 285 insertions(+), 184 deletions(-) diff --git a/locale/circuitpython.pot b/locale/circuitpython.pot index 03e15c6db2..a87bd419b9 100644 --- a/locale/circuitpython.pot +++ b/locale/circuitpython.pot @@ -103,14 +103,6 @@ msgstr "" msgid "%q must be %d-%d" msgstr "" -#: shared-bindings/usb_hid/Device.c -msgid "%q must be 0-255" -msgstr "" - -#: shared-bindings/usb_hid/Device.c -msgid "%q must be 1-255" -msgstr "" - #: py/argcheck.c msgid "%q must be >= %d" msgstr "" @@ -127,10 +119,6 @@ msgstr "" msgid "%q must be >= 1" msgstr "" -#: shared-bindings/usb_hid/Device.c -msgid "%q must be None or between 1 and len(report_descriptor)-1" -msgstr "" - #: py/argcheck.c msgid "%q must be a string" msgstr "" @@ -168,6 +156,10 @@ msgstr "" msgid "%q() takes %d positional arguments but %d were given" msgstr "" +#: shared-bindings/usb_hid/Device.c +msgid "%q, %q, and %q must all be the same length" +msgstr "" + #: ports/esp32s2/bindings/espidf/__init__.c ports/esp32s2/esp_error.c #, c-format msgid "%s error 0x%x" @@ -1518,6 +1510,11 @@ msgstr "" msgid "Missing jmp_pin. Instruction %d jumps on pin" msgstr "" +#: shared-module/usb_hid/Device.c +#, c-format +msgid "More than %d report ids not supported" +msgstr "" + #: shared-bindings/busio/UART.c shared-bindings/displayio/Group.c msgid "Must be a %q subclass." msgstr "" diff --git a/py/argcheck.c b/py/argcheck.c index 0d14c5eff2..ae779a1faa 100644 --- a/py/argcheck.c +++ b/py/argcheck.c @@ -197,7 +197,7 @@ mp_float_t mp_arg_validate_obj_float_non_negative(mp_obj_t float_in, mp_float_t size_t mp_arg_validate_length_with_name(mp_int_t i, size_t length, qstr arg_name, qstr length_name) { if (i != (mp_int_t)length) { - mp_raise_ValueError_varg(translate("%q length must be %q"), MP_QSTR_pressed, MP_QSTR_num_keys); + mp_raise_ValueError_varg(translate("%q length must be %q"), arg_name, length_name); } return (size_t)i; } diff --git a/shared-bindings/usb_hid/Device.c b/shared-bindings/usb_hid/Device.c index 64803f8e13..25377dffed 100644 --- a/shared-bindings/usb_hid/Device.c +++ b/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_length: int, out_report_length: int = 0, report_id_index: Optional[int]) -> None: +//| def __init__(self, *, descriptor: ReadableBuffer, usage_page: int, usage: 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()`. //| @@ -39,38 +39,45 @@ //| not verified for correctness; it is up to you to make sure it is not malformed. //| :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 in_report_length: Size in bytes of the HID report sent to the host. -//| "In" is with respect to the host. -//| :param int out_report_length: Size in bytes of the HID report received from the host. -//| "Out" is with respect to the host. If no reports are expected, use 0. -//| :param int report_id_index: position of byte in descriptor that contains the Report ID. -//| A Report ID will be assigned when the device is created. If there is no -//| Report ID, use ``None``. +//| :param int report_ids: Sequence of report ids used by the descriptor. +//| :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. +//| :param int out_report_lengths: Size in bytes of the HID report received from the host. +//| The sizes are in order of the ``report_ids``. +//| "OUT" is with respect to the host. +//| +//| ``report_ids``, ``in_report_lengths``, and ``out_report_lengths`` must all be the same length. //| """ //| ... //| //| KEYBOARD: Device -//| """Standard keyboard device supporting keycodes 0x00-0xDD, modifiers 0xE-0xE7, and five LED indicators.""" +//| """Standard keyboard device supporting keycodes 0x00-0xDD, modifiers 0xE-0xE7, and five LED indicators. +//| Uses Report ID 1 for its IN and OUT reports. +//| """ //| //| 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.""" +//| in each report, and a relative mouse wheel change from -127 to 127 in each report. +//| Uses Report ID 2 for its IN reports. +//| """ //| //| CONSUMER_CONTROL: Device -//| """Consumer Control device supporting sent values from 1-652, with no rollover.""" +//| """Consumer Control device supporting sent values from 1-652, with no rollover. +//| Uses Report ID 3 for its IN reports.""" //| 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) { usb_hid_device_obj_t *self = m_new_obj(usb_hid_device_obj_t); self->base.type = &usb_hid_device_type; - enum { ARG_report_descriptor, ARG_usage_page, ARG_usage, ARG_in_report_length, ARG_out_report_length, ARG_report_id_index }; + enum { ARG_report_descriptor, ARG_usage_page, ARG_usage, ARG_report_ids, ARG_in_report_lengths, ARG_out_report_lengths }; static const mp_arg_t allowed_args[] = { { MP_QSTR_report_descriptor, MP_ARG_KW_ONLY | MP_ARG_REQUIRED | MP_ARG_OBJ }, { MP_QSTR_usage_page, MP_ARG_KW_ONLY | MP_ARG_REQUIRED | MP_ARG_INT }, { MP_QSTR_usage, MP_ARG_KW_ONLY | MP_ARG_REQUIRED | MP_ARG_INT }, - { MP_QSTR_in_report_length, MP_ARG_KW_ONLY | MP_ARG_REQUIRED | MP_ARG_INT }, - { MP_QSTR_out_report_length, MP_ARG_KW_ONLY | MP_ARG_INT, {.u_int = 0 } }, - { MP_QSTR_report_id_index, MP_ARG_KW_ONLY | MP_ARG_REQUIRED | MP_ARG_OBJ, {.u_obj = mp_const_none } }, + { MP_QSTR_report_ids, MP_ARG_KW_ONLY | MP_ARG_REQUIRED | MP_ARG_OBJ }, + { MP_QSTR_in_report_lengths, MP_ARG_KW_ONLY | MP_ARG_REQUIRED | MP_ARG_OBJ }, + { MP_QSTR_out_report_lengths, MP_ARG_KW_ONLY | MP_ARG_REQUIRED | MP_ARG_OBJ }, }; mp_arg_val_t args[MP_ARRAY_SIZE(allowed_args)]; @@ -81,76 +88,128 @@ STATIC mp_obj_t usb_hid_device_make_new(const mp_obj_type_t *type, size_t n_args mp_obj_t descriptor = mp_obj_new_bytearray(descriptor_bufinfo.len, descriptor_bufinfo.buf); const mp_int_t usage_page_arg = args[ARG_usage_page].u_int; - if (usage_page_arg <= 0 || usage_page_arg > 255) { - mp_raise_ValueError_varg(translate("%q must be 1-255"), MP_QSTR_usage_page); - } + mp_arg_validate_int_range(usage_page_arg, 1, 255, MP_QSTR_usage_page); const uint8_t usage_page = usage_page_arg; const mp_int_t usage_arg = args[ARG_usage].u_int; - if (usage_arg <= 0 || usage_arg > 255) { - mp_raise_ValueError_varg(translate("%q must be 1-255"), MP_QSTR_usage); - } + mp_arg_validate_int_range(usage_arg, 1, 255, MP_QSTR_usage_page); const uint8_t usage = usage_arg; - const mp_int_t in_report_length_arg = args[ARG_in_report_length].u_int; - if (in_report_length_arg <= 0 || in_report_length_arg > 255) { - mp_raise_ValueError_varg(translate("%q must be 1-255"), MP_QSTR_in_report_length); - } - const uint8_t in_report_length = in_report_length_arg; + mp_obj_t report_ids = args[ARG_report_ids].u_obj; + mp_obj_t in_report_lengths = args[ARG_in_report_lengths].u_obj; + mp_obj_t out_report_lengths = args[ARG_out_report_lengths].u_obj; - const mp_int_t out_report_length_arg = args[ARG_out_report_length].u_int; - if (out_report_length_arg < 0 || out_report_length_arg > 255) { - mp_raise_ValueError_varg(translate("%q must be 0-255"), MP_QSTR_out_report_length); + 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) { + 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); } - const uint8_t out_report_length = out_report_length_arg; - const mp_obj_t report_id_index_arg = args[ARG_report_id_index].u_obj; - uint8_t report_id_index = 0; - if (report_id_index_arg != mp_const_none) { - const mp_int_t report_id_index_int = mp_obj_int_get_checked(report_id_index_arg); - if (report_id_index_int <= 0 || (uint32_t)report_id_index_int >= descriptor_bufinfo.len) { - mp_raise_ValueError_varg(translate("%q must be None or between 1 and len(report_descriptor)-1"), - MP_QSTR_report_id_index); - } - report_id_index = report_id_index_int; + uint8_t report_ids_array[report_ids_count]; + uint8_t in_report_lengths_array[report_ids_count]; + uint8_t out_report_lengths_array[report_ids_count]; + + // Validate the ids and lengths are all integers in range. + for (size_t i = 0; i < report_ids_count; i++) { + mp_obj_t i_obj = MP_OBJ_NEW_SMALL_INT(i); + + report_ids_array[i] = (uint8_t)mp_arg_validate_int_range( + // It's not the actual argument that's out of range, but its elements. + // But the error message is close enough. + MP_OBJ_SMALL_INT_VALUE(mp_obj_subscr(report_ids, i_obj, MP_OBJ_SENTINEL)), + 1, 255, MP_QSTR_report_ids); + + in_report_lengths_array[i] = (uint8_t)mp_arg_validate_int_range( + MP_OBJ_SMALL_INT_VALUE(mp_obj_subscr(in_report_lengths_array, i_obj, MP_OBJ_SENTINEL)), + 0, 255, MP_QSTR_in_report_lengths); + + out_report_lengths_array[i] = (uint8_t)mp_arg_validate_int_range( + MP_OBJ_SMALL_INT_VALUE(mp_obj_subscr(out_report_lengths_array, i_obj, MP_OBJ_SENTINEL)), + 0, 255, MP_QSTR_out_report_lengths); } common_hal_usb_hid_device_construct( - self, descriptor, usage_page, usage, in_report_length, out_report_length, report_id_index); + self, descriptor, usage_page, usage, report_ids_count, report_ids_array, in_report_lengths_array, out_report_lengths_array); return (mp_obj_t)self; } -//| def send_report(self, buf: ReadableBuffer) -> None: -//| """Send a HID report.""" +//| def send_report(self, buf: ReadableBuffer, report_id: Optional[int] = None) -> None: +//| """Send an HID report. +//| """ //| ... //| -STATIC mp_obj_t usb_hid_device_send_report(mp_obj_t self_in, mp_obj_t buffer) { - usb_hid_device_obj_t *self = MP_OBJ_TO_PTR(self_in); +STATIC mp_obj_t usb_hid_device_send_report(size_t n_args, const mp_obj_t *pos_args, mp_map_t *kw_args) { + usb_hid_device_obj_t *self = MP_OBJ_TO_PTR(pos_args[0]); + + enum { ARG_buf, ARG_report_id }; + static const mp_arg_t allowed_args[] = { + { MP_QSTR_buf, MP_ARG_REQUIRED | MP_ARG_OBJ }, + { MP_QSTR_report_id, 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); mp_buffer_info_t bufinfo; - mp_get_buffer_raise(buffer, &bufinfo, MP_BUFFER_READ); + mp_get_buffer_raise(args[ARG_buf].u_obj, &bufinfo, MP_BUFFER_READ); - common_hal_usb_hid_device_send_report(self, ((uint8_t *)bufinfo.buf), bufinfo.len); + uint8_t report_id = 0; + 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); + } + + 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_2(usb_hid_device_send_report_obj, usb_hid_device_send_report); +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. +//| """ +//| ... +//| +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) { + usb_hid_device_obj_t *self = MP_OBJ_TO_PTR(pos_args[0]); + + enum { ARG_report_id }; + static const mp_arg_t allowed_args[] = { + { MP_QSTR_report_id, 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); + + uint8_t report_id = 0; + if (args[ARG_report_id].u_obj != mp_const_none) { + report_id = mp_obj_int_get_checked(args[ARG_report_id].u_obj); + } + + 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. //| -STATIC mp_obj_t usb_hid_device_obj_get_last_received_report(mp_obj_t self_in) { +//| Deprecated: will be removed in CircutPython 8.0.0. Use `get_last_received_report()` instead. +//| """ +//| +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); - if (self->out_report_buffer == 0) { - return mp_const_none; - } - return mp_obj_new_bytes(self->out_report_buffer, self->out_report_length); + + return common_hal_usb_hid_device_get_last_received_report(self, 0); } -MP_DEFINE_CONST_FUN_OBJ_1(usb_hid_device_get_last_received_report_obj, usb_hid_device_obj_get_last_received_report); +MP_DEFINE_CONST_FUN_OBJ_1(usb_hid_device_get_last_received_report_property_obj, usb_hid_device_obj_get_last_received_report_property); const mp_obj_property_t usb_hid_device_last_received_report_obj = { .base.type = &mp_type_property, - .proxy = {(mp_obj_t)&usb_hid_device_get_last_received_report_obj, + .proxy = {(mp_obj_t)&usb_hid_device_get_last_received_report_property_obj, MP_ROM_NONE, MP_ROM_NONE}, }; @@ -192,13 +251,15 @@ const mp_obj_property_t usb_hid_device_usage_obj = { }; STATIC const mp_rom_map_elem_t usb_hid_device_locals_dict_table[] = { - { MP_ROM_QSTR(MP_QSTR_send_report), MP_ROM_PTR(&usb_hid_device_send_report_obj) }, - { MP_ROM_QSTR(MP_QSTR_last_received_report), MP_ROM_PTR(&usb_hid_device_last_received_report_obj) }, - { MP_ROM_QSTR(MP_QSTR_usage_page), MP_ROM_PTR(&usb_hid_device_usage_page_obj) }, - { MP_ROM_QSTR(MP_QSTR_usage), MP_ROM_PTR(&usb_hid_device_usage_obj) }, - { MP_ROM_QSTR(MP_QSTR_KEYBOARD), MP_ROM_PTR(&usb_hid_device_keyboard_obj) }, - { MP_ROM_QSTR(MP_QSTR_MOUSE), MP_ROM_PTR(&usb_hid_device_mouse_obj) }, - { MP_ROM_QSTR(MP_QSTR_CONSUMER_CONTROL), MP_ROM_PTR(&usb_hid_device_consumer_control_obj) }, + { MP_ROM_QSTR(MP_QSTR_send_report), MP_ROM_PTR(&usb_hid_device_send_report_obj) }, + { MP_ROM_QSTR(MP_QSTR_get_last_received_report), MP_ROM_PTR(&usb_hid_device_get_last_received_report_obj) }, + { MP_ROM_QSTR(MP_QSTR_last_received_report), MP_ROM_PTR(&usb_hid_device_last_received_report_obj) }, + { MP_ROM_QSTR(MP_QSTR_usage_page), MP_ROM_PTR(&usb_hid_device_usage_page_obj) }, + { MP_ROM_QSTR(MP_QSTR_usage), MP_ROM_PTR(&usb_hid_device_usage_obj) }, + + { MP_ROM_QSTR(MP_QSTR_KEYBOARD), MP_ROM_PTR(&usb_hid_device_keyboard_obj) }, + { MP_ROM_QSTR(MP_QSTR_MOUSE), MP_ROM_PTR(&usb_hid_device_mouse_obj) }, + { MP_ROM_QSTR(MP_QSTR_CONSUMER_CONTROL), MP_ROM_PTR(&usb_hid_device_consumer_control_obj) }, }; STATIC MP_DEFINE_CONST_DICT(usb_hid_device_locals_dict, usb_hid_device_locals_dict_table); diff --git a/shared-bindings/usb_hid/Device.h b/shared-bindings/usb_hid/Device.h index c1af92fd4f..d6069c370e 100644 --- a/shared-bindings/usb_hid/Device.h +++ b/shared-bindings/usb_hid/Device.h @@ -33,9 +33,11 @@ extern const mp_obj_type_t usb_hid_device_type; -void common_hal_usb_hid_device_construct(usb_hid_device_obj_t *self, mp_obj_t descriptor, uint8_t usage_page, uint8_t usage, uint8_t in_report_length, uint8_t out_report_length, uint8_t report_id_index); -void common_hal_usb_hid_device_send_report(usb_hid_device_obj_t *self, uint8_t *report, uint8_t len); +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 report_ids_count,uint8_t *report_ids, uint8_t *in_report_lengths, uint8_t *out_report_lengths); +void common_hal_usb_hid_device_send_report(usb_hid_device_obj_t *self, uint8_t *report, uint8_t len, uint8_t report_id); +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); #endif // MICROPY_INCLUDED_SHARED_BINDINGS_USB_HID_DEVICE_H diff --git a/shared-module/usb_hid/Device.c b/shared-module/usb_hid/Device.c index ad64070448..9690f86586 100644 --- a/shared-module/usb_hid/Device.c +++ b/shared-module/usb_hid/Device.c @@ -39,8 +39,7 @@ static const uint8_t keyboard_report_descriptor[] = { 0x05, 0x01, // 0,1 Usage Page (Generic Desktop Ctrls) 0x09, 0x06, // 2,3 Usage (Keyboard) 0xA1, 0x01, // 4,5 Collection (Application) - 0x85, 0xFF, // 6,7 Report ID [SET AT RUNTIME] -#define KEYBOARD_REPORT_ID_INDEX (7) + 0x85, 0x01, // 6,7 Report ID (1) 0x05, 0x07, // Usage Page (Kbrd/Keypad) 0x19, 0xE0, // Usage Minimum (0xE0) 0x29, 0xE7, // Usage Maximum (0xE7) @@ -78,10 +77,10 @@ const usb_hid_device_obj_t usb_hid_device_keyboard_obj = { .report_descriptor_length = sizeof(keyboard_report_descriptor), .usage_page = 0x01, .usage = 0x06, - .in_report_length = 8, - .out_report_length = 1, - .report_id_index = KEYBOARD_REPORT_ID_INDEX, - + .num_report_ids = 1, + .report_ids = { 0x01, }, + .in_report_lengths = { 8, 0, 0, 0, }, + .out_report_lengths = { 1, }, }; static const uint8_t mouse_report_descriptor[] = { @@ -90,8 +89,7 @@ static const uint8_t mouse_report_descriptor[] = { 0xA1, 0x01, // 4,5 Collection (Application) 0x09, 0x01, // 6,7 Usage (Pointer) 0xA1, 0x00, // 8,9 Collection (Physical) - 0x85, 0xFF, // 10, 11 Report ID [SET AT RUNTIME] -#define MOUSE_REPORT_ID_INDEX (11) + 0x85, 0x02, // 10, 11 Report ID (2) 0x05, 0x09, // Usage Page (Button) 0x19, 0x01, // Usage Minimum (0x01) 0x29, 0x05, // Usage Maximum (0x05) @@ -129,17 +127,17 @@ const usb_hid_device_obj_t usb_hid_device_mouse_obj = { .report_descriptor_length = sizeof(mouse_report_descriptor), .usage_page = 0x01, .usage = 0x02, - .in_report_length = 4, - .out_report_length = 0, - .report_id_index = MOUSE_REPORT_ID_INDEX, + .num_report_ids = 1, + .report_ids = { 0x02, }, + .in_report_lengths = { 4, 0, 0, 0, }, + .out_report_lengths = { 0, }, }; static const uint8_t consumer_control_report_descriptor[] = { 0x05, 0x0C, // 0,1 Usage Page (Consumer) 0x09, 0x01, // 2,3 Usage (Consumer Control) 0xA1, 0x01, // 4,5 Collection (Application) - 0x85, 0xFF, // 6,7 Report ID [SET AT RUNTIME] -#define CONSUMER_CONTROL_REPORT_ID_INDEX (7) + 0x85, 0x03, // 6,7 Report ID (3) 0x75, 0x10, // Report Size (16) 0x95, 0x01, // Report Count (1) 0x15, 0x01, // Logical Minimum (1) @@ -158,15 +156,29 @@ const usb_hid_device_obj_t usb_hid_device_consumer_control_obj = { .report_descriptor_length = sizeof(consumer_control_report_descriptor), .usage_page = 0x0C, .usage = 0x01, - .in_report_length = 2, - .out_report_length = 0, - .report_id_index = CONSUMER_CONTROL_REPORT_ID_INDEX, + .num_report_ids = 1, + .report_ids = { 0x03 }, + .in_report_lengths = { 2, 0, 0, 0 }, + .out_report_lengths = { 0 }, }; -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, uint8_t in_report_length, uint8_t out_report_length, uint8_t report_id_index) { - // report buffer pointers are NULL at start, and are created when USB is initialized. +bool common_hal_usb_hid_device_valid_report_id(usb_hid_device_obj_t *self, uint8_t report_id) { + for (size_t i = 0; i < self->num_report_ids; i++) { + if (report_id == self->report_ids[i]) { + return true; + } + } + return false; +} +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) { + if (num_report_ids > MAX_REPORT_IDS_PER_DESCRIPTOR) { + mp_raise_ValueError_varg(translate("More than %d report ids not supported"), + MAX_REPORT_IDS_PER_DESCRIPTOR); + } + + // report buffer pointers are NULL at start, and are created when USB is initialized. mp_buffer_info_t bufinfo; mp_get_buffer_raise(report_descriptor, &bufinfo, MP_BUFFER_READ); self->report_descriptor_length = bufinfo.len; @@ -179,9 +191,10 @@ void common_hal_usb_hid_device_construct(usb_hid_device_obj_t *self, mp_obj_t re self->usage_page = usage_page; self->usage = usage; - self->in_report_length = in_report_length; - self->out_report_length = out_report_length; - self->report_id_index = report_id_index; + self->num_report_ids = num_report_ids; + memcpy(self->report_ids, report_ids, num_report_ids); + memcpy(self->in_report_lengths, in_report_lengths, num_report_ids); + memcpy(self->out_report_lengths, out_report_lengths, num_report_ids); } uint8_t common_hal_usb_hid_device_get_usage_page(usb_hid_device_obj_t *self) { @@ -192,7 +205,7 @@ uint8_t common_hal_usb_hid_device_get_usage(usb_hid_device_obj_t *self) { return self->usage; } -void common_hal_usb_hid_device_send_report(usb_hid_device_obj_t *self, uint8_t *report, uint8_t len) { +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); } @@ -207,25 +220,24 @@ void common_hal_usb_hid_device_send_report(usb_hid_device_obj_t *self, uint8_t * mp_raise_msg(&mp_type_OSError, translate("USB busy")); } - memcpy(self->in_report_buffer, report, len); - - if (!tud_hid_report(self->report_id, self->in_report_buffer, len)) { + if (!tud_hid_report(report_id, report, len)) { mp_raise_msg(&mp_type_OSError, translate("USB error")); } } void usb_hid_device_create_report_buffers(usb_hid_device_obj_t *self) { - if (self->in_report_length > 0) { - self->in_report_buffer = gc_alloc(self->in_report_length, false, true /*long-lived*/); - } - if (self->out_report_length > 0) { - self->out_report_buffer = gc_alloc(self->out_report_length, false, true /*long-lived*/); + 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; + } } } -// Callbacks invoked when receive Get_Report request through control endpoint +// Callbacks invoked when we received Get_Report request through control endpoint uint16_t tud_hid_get_report_cb(uint8_t itf, uint8_t report_id, hid_report_type_t report_type, uint8_t *buffer, uint16_t reqlen) { (void)itf; // only support Input Report @@ -234,24 +246,33 @@ uint16_t tud_hid_get_report_cb(uint8_t itf, uint8_t report_id, hid_report_type_t } // fill buffer with current report - memcpy(buffer, usb_hid_get_device_with_report_id(report_id)->in_report_buffer, reqlen); - return reqlen; -} -// Callbacks invoked when receive 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; + 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)) { + memcpy(buffer, hid_device->in_report_buffers[id_idx], reqlen); + return reqlen; } - usb_hid_device_obj_t *hid_device = usb_hid_get_device_with_report_id(report_id); +// 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; + } - if (hid_device && hid_device->out_report_length >= bufsize) { - memcpy(hid_device->out_report_buffer, 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); + } + } } -} diff --git a/shared-module/usb_hid/Device.h b/shared-module/usb_hid/Device.h index db3d8ecf65..cc185d3272 100644 --- a/shared-module/usb_hid/Device.h +++ b/shared-module/usb_hid/Device.h @@ -32,19 +32,22 @@ #include "py/obj.h" +// The most complicated one currently know of is the head and eye tracker, which requires 5: +// https://usb.org/sites/default/files/hutrr74_-_usage_page_for_head_and_eye_trackers_0.pdf +#define MAX_REPORT_IDS_PER_DESCRIPTOR (6) + typedef struct { mp_obj_base_t base; // Python buffer object whose contents are the descriptor. const uint8_t *report_descriptor; - uint8_t *in_report_buffer; - uint8_t *out_report_buffer; - uint16_t report_id_index; + 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 *out_report_buffers[MAX_REPORT_IDS_PER_DESCRIPTOR]; uint16_t report_descriptor_length; uint8_t usage_page; uint8_t usage; - uint8_t report_id; - uint8_t in_report_length; - uint8_t out_report_length; + uint8_t num_report_ids; } usb_hid_device_obj_t; extern const usb_hid_device_obj_t usb_hid_device_keyboard_obj; diff --git a/shared-module/usb_hid/__init__.c b/shared-module/usb_hid/__init__.c index 81a62c8eb3..6f903bb657 100644 --- a/shared-module/usb_hid/__init__.c +++ b/shared-module/usb_hid/__init__.c @@ -215,74 +215,91 @@ void usb_hid_build_report_descriptor(uint8_t *report_descriptor_space, size_t re for (mp_int_t i = 0; i < num_hid_devices; i++) { usb_hid_device_obj_t *device = &hid_devices[i]; // Copy the report descriptor for this device. - if (num_hid_devices == 1) { - // There's only one device, so it shouldn't have a report ID. + if (num_hid_devices == 1 && device->num_report_ids == 1) { + // There's only one device, with one report id, so remove it. // Copy the descriptor, but splice out the report id indicator and value (2 bytes). - memcpy(report_descriptor_start, device->report_descriptor, device->report_id_index - 1); - report_descriptor_start += device->report_id_index - 1; - memcpy(report_descriptor_start, device->report_descriptor + device->report_id_index + 1, - device->report_descriptor_length - device->report_id_index - 1); - } else { - // Copy the whole descriptor and fill in the report id. - memcpy(report_descriptor_start, device->report_descriptor, device->report_descriptor_length); - report_descriptor_start[device->report_id_index] = i + 1; + size_t report_id_idx = 0; + for (report_id_idx = 0; report_id_idx < device->report_descriptor_length; report_id_idx++) { + if (report_descriptor_start[report_id_idx] == 0x85) { + break; + } + } + if (report_id_idx < device->report_descriptor_length) { - // Remember the report id that was assigned. - device->report_id = i + 1; - // Advance to the next free chunk for the next report descriptor.x - report_descriptor_start += device->report_descriptor_length; + memcpy(report_descriptor_start, device->report_descriptor, device->report_id_index - 1); + report_descriptor_start += device->report_id_index - 1; + memcpy(report_descriptor_start, device->report_descriptor + device->report_id_index + 1, + device->report_descriptor_length - device->report_id_index - 1); + } else { + // Copy the whole descriptor and fill in the report id. + memcpy(report_descriptor_start, device->report_descriptor, device->report_descriptor_length); + report_descriptor_start[device->report_id_index] = i + 1; + + // Remember the report id that was assigned. + device->report_id = i + 1; + + // Advance to the next free chunk for the next report descriptor.x + report_descriptor_start += device->report_descriptor_length; + } + // Clear the heap pointer to the bytes of the descriptor. + // We don't need it any more and it will get lost when the heap goes away. + device->report_descriptor = NULL; } - // Clear the heap pointer to the bytes of the descriptor. - // We don't need it any more and it will get lost when the heap goes away. - device->report_descriptor = NULL; } -} // Call this after the heap and VM are finished. -void usb_hid_save_report_descriptor(uint8_t *report_descriptor_space, size_t report_descriptor_length) { - if (!usb_hid_enabled()) { - return; + void usb_hid_save_report_descriptor(uint8_t *report_descriptor_space, size_t report_descriptor_length) { + if (!usb_hid_enabled()) { + return; + } + + // Allocate storage that persists across VMs to hold the combined report descriptor. + // and to remember the device details. + + // Copy the descriptor from the temporary area to a supervisor storage allocation that + // will leave between VM instantiations. + hid_report_descriptor_allocation = + allocate_memory(align32_size(report_descriptor_length), + /*high_address*/ false, /*movable*/ false); + memcpy((uint8_t *)hid_report_descriptor_allocation->ptr, report_descriptor_space, report_descriptor_length); } - // Allocate storage that persists across VMs to hold the combined report descriptor. - // and to remember the device details. + void usb_hid_gc_collect(void) { + gc_collect_ptr(hid_devices_tuple); - // Copy the descriptor from the temporary area to a supervisor storage allocation that - // will leave between VM instantiations. - hid_report_descriptor_allocation = - allocate_memory(align32_size(report_descriptor_length), - /*high_address*/ false, /*movable*/ false); - memcpy((uint8_t *)hid_report_descriptor_allocation->ptr, report_descriptor_space, report_descriptor_length); -} + // Mark possible heap pointers in the static device list as in use. + for (mp_int_t device_idx = 0; i < num_hid_devices; i++) { -void usb_hid_gc_collect(void) { - gc_collect_ptr(hid_devices_tuple); + // Cast away the const for .report_descriptor. It could be in flash or on the heap. + // Constant report descriptors must be const so that they are used directly from flash + // and not copied into RAM. + gc_collect_ptr((void *)hid_devices[device_idx].report_descriptor); - // Mark possible heap pointers in the static device list as in use. - for (mp_int_t i = 0; i < num_hid_devices; i++) { - // Cast away the const for .report_descriptor. It could be in flash or on the heap. - // Constant report descriptors must be const so that they are used directly from flash - // and not copied into RAM. - gc_collect_ptr((void *)hid_devices[i].report_descriptor); - gc_collect_ptr(hid_devices[i].in_report_buffer); - gc_collect_ptr(hid_devices[i].out_report_buffer); - } -} - -usb_hid_device_obj_t *usb_hid_get_device_with_report_id(uint8_t report_id) { - for (uint8_t i = 0; i < num_hid_devices; i++) { - usb_hid_device_obj_t *device = &hid_devices[i]; - if (device->report_id == report_id) { - return &hid_devices[i]; + // Collect all the OUT report buffers for this device. + for (size_t id_idx = 0; id_idx < hid_devices[device_idx].report_ids_count; id_idx++) { + gc_collect_ptr(hid_devices[i].out_report_buffers[id_idx]); + } } } - return NULL; -} + + bool usb_hid_get_device_with_report_id(uint8_t report_id, usb_hid_device_obj_t **device_out, size_t *id_idx_out) { + for (uint8_t device_idx = 0; device_idx < num_hid_devices; device_idx++) { + usb_hid_device_obj_t *device = &hid_devices[device_idx]; + for (size_t id_idx = 0; id_idx < device->report_ids_count; id_idx++) { + if (device->report_ids[id_idx] == report_id) { + *device_out = device; + *id_idx_out = id_idx; + return true; + } + } + } + return false; + } // Invoked when GET HID REPORT DESCRIPTOR is received. // Application return pointer to descriptor // Descriptor contents must exist long enough for transfer to complete -uint8_t const *tud_hid_descriptor_report_cb(uint8_t itf) { - return (uint8_t *)hid_report_descriptor_allocation->ptr; -} + uint8_t const *tud_hid_descriptor_report_cb(uint8_t itf) { + return (uint8_t *)hid_report_descriptor_allocation->ptr; + } diff --git a/shared-module/usb_hid/__init__.h b/shared-module/usb_hid/__init__.h index 5069effd3d..6d85b50e9f 100644 --- a/shared-module/usb_hid/__init__.h +++ b/shared-module/usb_hid/__init__.h @@ -44,7 +44,7 @@ size_t usb_hid_report_descriptor_length(void); void usb_hid_build_report_descriptor(uint8_t *report_descriptor_space, size_t report_descriptor_length); void usb_hid_save_report_descriptor(uint8_t *report_descriptor_space, size_t report_descriptor_length); -usb_hid_device_obj_t *usb_hid_get_device_with_report_id(uint8_t report_id); +bool usb_hid_get_device_with_report_id(uint8_t report_id, usb_hid_device_obj_t **device_out, size_t *id_idx_out); void usb_hid_gc_collect(void); From 3dc2b4c2d34ba30e78a50e1e2c6512d3716a3b88 Mon Sep 17 00:00:00 2001 From: Dan Halbert Date: Fri, 13 Aug 2021 21:51:52 -0400 Subject: [PATCH 2/5] at least original functionality with new API --- locale/circuitpython.pot | 6 +- shared-bindings/usb_hid/Device.c | 49 +++++++----- shared-bindings/usb_hid/Device.h | 2 +- shared-bindings/usb_hid/__init__.c | 2 +- shared-module/usb_hid/Device.c | 97 ++++++++++++++-------- shared-module/usb_hid/Device.h | 4 +- shared-module/usb_hid/__init__.c | 124 ++++++++++++----------------- 7 files changed, 152 insertions(+), 132 deletions(-) diff --git a/locale/circuitpython.pot b/locale/circuitpython.pot index a87bd419b9..6114dcfaea 100644 --- a/locale/circuitpython.pot +++ b/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 "" diff --git a/shared-bindings/usb_hid/Device.c b/shared-bindings/usb_hid/Device.c index 25377dffed..718df73241 100644 --- a/shared-bindings/usb_hid/Device.c +++ b/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,11 +162,12 @@ 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; @@ -167,10 +175,10 @@ STATIC mp_obj_t usb_hid_device_send_report(size_t n_args, const mp_obj_t *pos_ar 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.""" diff --git a/shared-bindings/usb_hid/Device.h b/shared-bindings/usb_hid/Device.h index d6069c370e..a5e94c4d52 100644 --- a/shared-bindings/usb_hid/Device.h +++ b/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 diff --git a/shared-bindings/usb_hid/__init__.c b/shared-bindings/usb_hid/__init__.c index f5a7c1cf72..1c8a45e4ea 100644 --- a/shared-bindings/usb_hid/__init__.c +++ b/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. //| """ //| ... diff --git a/shared-module/usb_hid/Device.c b/shared-module/usb_hid/Device.c index 9690f86586..db92bd1c32 100644 --- a/shared-module/usb_hid/Device.c +++ b/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); } } +} diff --git a/shared-module/usb_hid/Device.h b/shared-module/usb_hid/Device.h index cc185d3272..4bdd51d104 100644 --- a/shared-module/usb_hid/Device.h +++ b/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; diff --git a/shared-module/usb_hid/__init__.c b/shared-module/usb_hid/__init__.c index 6f903bb657..2be9a646d6 100644 --- a/shared-module/usb_hid/__init__.c +++ b/shared-module/usb_hid/__init__.c @@ -197,10 +197,6 @@ size_t usb_hid_report_descriptor_length(void) { total_hid_report_descriptor_length += hid_devices[i].report_descriptor_length; } - // Don't need space for a report id if there's only one device. - if (num_hid_devices == 1) { - total_hid_report_descriptor_length -= 2; - } return total_hid_report_descriptor_length; } @@ -215,91 +211,69 @@ void usb_hid_build_report_descriptor(uint8_t *report_descriptor_space, size_t re for (mp_int_t i = 0; i < num_hid_devices; i++) { usb_hid_device_obj_t *device = &hid_devices[i]; // Copy the report descriptor for this device. - if (num_hid_devices == 1 && device->num_report_ids == 1) { - // There's only one device, with one report id, so remove it. - // Copy the descriptor, but splice out the report id indicator and value (2 bytes). - size_t report_id_idx = 0; - for (report_id_idx = 0; report_id_idx < device->report_descriptor_length; report_id_idx++) { - if (report_descriptor_start[report_id_idx] == 0x85) { - break; - } - } - if (report_id_idx < device->report_descriptor_length) { + memcpy(report_descriptor_start, device->report_descriptor, device->report_descriptor_length); + // Advance to the next free chunk for the next report descriptor.x + report_descriptor_start += device->report_descriptor_length; - memcpy(report_descriptor_start, device->report_descriptor, device->report_id_index - 1); - report_descriptor_start += device->report_id_index - 1; - memcpy(report_descriptor_start, device->report_descriptor + device->report_id_index + 1, - device->report_descriptor_length - device->report_id_index - 1); - } else { - // Copy the whole descriptor and fill in the report id. - memcpy(report_descriptor_start, device->report_descriptor, device->report_descriptor_length); - report_descriptor_start[device->report_id_index] = i + 1; - - // Remember the report id that was assigned. - device->report_id = i + 1; - - // Advance to the next free chunk for the next report descriptor.x - report_descriptor_start += device->report_descriptor_length; - } - // Clear the heap pointer to the bytes of the descriptor. - // We don't need it any more and it will get lost when the heap goes away. - device->report_descriptor = NULL; - } + // Clear the heap pointer to the bytes of the descriptor. + // We don't need it any more and it will get lost when the heap goes away. + device->report_descriptor = NULL; } +} // Call this after the heap and VM are finished. - void usb_hid_save_report_descriptor(uint8_t *report_descriptor_space, size_t report_descriptor_length) { - if (!usb_hid_enabled()) { - return; - } - - // Allocate storage that persists across VMs to hold the combined report descriptor. - // and to remember the device details. - - // Copy the descriptor from the temporary area to a supervisor storage allocation that - // will leave between VM instantiations. - hid_report_descriptor_allocation = - allocate_memory(align32_size(report_descriptor_length), - /*high_address*/ false, /*movable*/ false); - memcpy((uint8_t *)hid_report_descriptor_allocation->ptr, report_descriptor_space, report_descriptor_length); +void usb_hid_save_report_descriptor(uint8_t *report_descriptor_space, size_t report_descriptor_length) { + if (!usb_hid_enabled()) { + return; } - void usb_hid_gc_collect(void) { - gc_collect_ptr(hid_devices_tuple); + // Allocate storage that persists across VMs to hold the combined report descriptor. + // and to remember the device details. - // Mark possible heap pointers in the static device list as in use. - for (mp_int_t device_idx = 0; i < num_hid_devices; i++) { + // Copy the descriptor from the temporary area to a supervisor storage allocation that + // will leave between VM instantiations. + hid_report_descriptor_allocation = + allocate_memory(align32_size(report_descriptor_length), + /*high_address*/ false, /*movable*/ false); + memcpy((uint8_t *)hid_report_descriptor_allocation->ptr, report_descriptor_space, report_descriptor_length); +} - // Cast away the const for .report_descriptor. It could be in flash or on the heap. - // Constant report descriptors must be const so that they are used directly from flash - // and not copied into RAM. - gc_collect_ptr((void *)hid_devices[device_idx].report_descriptor); +void usb_hid_gc_collect(void) { + gc_collect_ptr(hid_devices_tuple); - // Collect all the OUT report buffers for this device. - for (size_t id_idx = 0; id_idx < hid_devices[device_idx].report_ids_count; id_idx++) { - gc_collect_ptr(hid_devices[i].out_report_buffers[id_idx]); + // Mark possible heap pointers in the static device list as in use. + for (mp_int_t device_idx = 0; device_idx < num_hid_devices; device_idx++) { + + // Cast away the const for .report_descriptor. It could be in flash or on the heap. + // Constant report descriptors must be const so that they are used directly from flash + // and not copied into RAM. + gc_collect_ptr((void *)hid_devices[device_idx].report_descriptor); + + // Collect all the OUT report buffers for this device. + for (size_t id_idx = 0; id_idx < hid_devices[device_idx].num_report_ids; id_idx++) { + gc_collect_ptr(hid_devices[id_idx].out_report_buffers[id_idx]); + } + } +} + +bool usb_hid_get_device_with_report_id(uint8_t report_id, usb_hid_device_obj_t **device_out, size_t *id_idx_out) { + for (uint8_t device_idx = 0; device_idx < num_hid_devices; device_idx++) { + usb_hid_device_obj_t *device = &hid_devices[device_idx]; + for (size_t id_idx = 0; id_idx < device->num_report_ids; id_idx++) { + if (device->report_ids[id_idx] == report_id) { + *device_out = device; + *id_idx_out = id_idx; + return true; } } } - - bool usb_hid_get_device_with_report_id(uint8_t report_id, usb_hid_device_obj_t **device_out, size_t *id_idx_out) { - for (uint8_t device_idx = 0; device_idx < num_hid_devices; device_idx++) { - usb_hid_device_obj_t *device = &hid_devices[device_idx]; - for (size_t id_idx = 0; id_idx < device->report_ids_count; id_idx++) { - if (device->report_ids[id_idx] == report_id) { - *device_out = device; - *id_idx_out = id_idx; - return true; - } - } - } - return false; - } + return false; +} // Invoked when GET HID REPORT DESCRIPTOR is received. // Application return pointer to descriptor // Descriptor contents must exist long enough for transfer to complete - uint8_t const *tud_hid_descriptor_report_cb(uint8_t itf) { - return (uint8_t *)hid_report_descriptor_allocation->ptr; - } +uint8_t const *tud_hid_descriptor_report_cb(uint8_t itf) { + return (uint8_t *)hid_report_descriptor_allocation->ptr; +} From f37e1d7bf59d76929a77d4f11918599c6501fc56 Mon Sep 17 00:00:00 2001 From: Dan Halbert Date: Sat, 14 Aug 2021 17:36:05 -0400 Subject: [PATCH 3/5] squeeze a couple of boards --- ports/atmel-samd/boards/blm_badge/mpconfigboard.mk | 1 + ports/atmel-samd/boards/sensebox_mcu/mpconfigboard.mk | 4 +++- shared-module/usb_hid/Device.h | 6 +++--- 3 files changed, 7 insertions(+), 4 deletions(-) diff --git a/ports/atmel-samd/boards/blm_badge/mpconfigboard.mk b/ports/atmel-samd/boards/blm_badge/mpconfigboard.mk index 8ba3ae94c3..a85a17890c 100644 --- a/ports/atmel-samd/boards/blm_badge/mpconfigboard.mk +++ b/ports/atmel-samd/boards/blm_badge/mpconfigboard.mk @@ -14,6 +14,7 @@ CIRCUITPY_AUDIOIO = 1 CIRCUITPY_AUDIOBUSIO = 1 # Pins for I2SOut are not available. CIRCUITPY_AUDIOBUSIO_I2SOUT = 0 +CIRCUITPY_BUSIO_SPI = 0 CIRCUITPY_PWMIO = 0 CIRCUITPY_ROTARYIO = 0 CIRCUITPY_RTC = 0 diff --git a/ports/atmel-samd/boards/sensebox_mcu/mpconfigboard.mk b/ports/atmel-samd/boards/sensebox_mcu/mpconfigboard.mk index 94a255529f..4687ce32a0 100644 --- a/ports/atmel-samd/boards/sensebox_mcu/mpconfigboard.mk +++ b/ports/atmel-samd/boards/sensebox_mcu/mpconfigboard.mk @@ -10,4 +10,6 @@ INTERNAL_FLASH_FILESYSTEM = 1 LONGINT_IMPL = NONE CIRCUITPY_FULL_BUILD = 0 -CIRCUITPY_GETPASS = 0 +# There are many pin definitions on this board; it doesn't quite fit on very large translations. +# So remove what might be least likely module to be used. +CIRCUITPY_RAINBOWIO = 0 diff --git a/shared-module/usb_hid/Device.h b/shared-module/usb_hid/Device.h index 4bdd51d104..a3c523acd5 100644 --- a/shared-module/usb_hid/Device.h +++ b/shared-module/usb_hid/Device.h @@ -41,12 +41,12 @@ typedef struct { mp_obj_base_t base; // Python buffer object whose contents are the descriptor. const uint8_t *report_descriptor; - 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 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 usage_page; uint8_t usage; uint8_t num_report_ids; From 38f42818e9a01226cc62d4d1ba7bc4dc264ff5b8 Mon Sep 17 00:00:00 2001 From: Dan Halbert Date: Mon, 16 Aug 2021 18:59:18 -0400 Subject: [PATCH 4/5] fix gc; update KEYBOARD report descriptor The default KEYBOARD report descriptor had a signed/unsigned error, and also could have allowed more keycodes. So I changed it, using the very vanilla descriptor from a very plain extremely common commercial keyboard, modifying it only have 5 LED's instead of 3, and added a report ID. --- shared-module/usb_hid/Device.c | 82 ++++++++++++++++---------------- shared-module/usb_hid/__init__.c | 3 +- 2 files changed, 44 insertions(+), 41 deletions(-) diff --git a/shared-module/usb_hid/Device.c b/shared-module/usb_hid/Device.c index db92bd1c32..50fd93fa68 100644 --- a/shared-module/usb_hid/Device.c +++ b/shared-module/usb_hid/Device.c @@ -36,37 +36,39 @@ #include "tusb.h" static const uint8_t keyboard_report_descriptor[] = { - 0x05, 0x01, // 0,1 Usage Page (Generic Desktop Ctrls) - 0x09, 0x06, // 2,3 Usage (Keyboard) - 0xA1, 0x01, // 4,5 Collection (Application) - 0x85, 0x01, // 6,7 Report ID (1) - 0x05, 0x07, // Usage Page (Kbrd/Keypad) - 0x19, 0xE0, // Usage Minimum (0xE0) - 0x29, 0xE7, // Usage Maximum (0xE7) - 0x15, 0x00, // Logical Minimum (0) - 0x25, 0x01, // Logical Maximum (1) - 0x75, 0x01, // Report Size (1) - 0x95, 0x08, // Report Count (8) - 0x81, 0x02, // Input (Data,Var,Abs,No Wrap,Linear,Preferred State,No Null Position) - 0x81, 0x01, // Input (Const,Array,Abs,No Wrap,Linear,Preferred State,No Null Position) - 0x19, 0x00, // Usage Minimum (0x00) - 0x29, 0xDD, // Usage Maximum (0xDD) - 0x15, 0x00, // Logical Minimum (0) - 0x25, 0xDD, // Logical Maximum (-35) - 0x75, 0x08, // Report Size (8) - 0x95, 0x06, // Report Count (6) - 0x81, 0x00, // Input (Data,Array,Abs,No Wrap,Linear,Preferred State,No Null Position) - 0x05, 0x08, // Usage Page (LEDs) - 0x19, 0x01, // Usage Minimum (Num Lock) - 0x29, 0x05, // Usage Maximum (Kana) - 0x15, 0x00, // Logical Minimum (0) - 0x25, 0x01, // Logical Maximum (1) - 0x75, 0x01, // Report Size (1) - 0x95, 0x05, // Report Count (5) - 0x91, 0x02, // Output (Data,Var,Abs,No Wrap,Linear,Preferred State,No Null Position,Non-volatile) - 0x95, 0x03, // Report Count (3) - 0x91, 0x01, // Output (Const,Array,Abs,No Wrap,Linear,Preferred State,No Null Position,Non-volatile) - 0xC0, // End Collection + 0x05, 0x01, // Usage Page (Generic Desktop Ctrls) + 0x09, 0x06, // Usage (Keyboard) + 0xA1, 0x01, // Collection (Application) + 0x85, 0x01, // Report ID (1) + 0x05, 0x07, // Usage Page (Kbrd/Keypad) + 0x19, 0xE0, // Usage Minimum (0xE0) + 0x29, 0xE7, // Usage Maximum (0xE7) + 0x15, 0x00, // Logical Minimum (0) + 0x25, 0x01, // Logical Maximum (1) + 0x75, 0x01, // Report Size (1) + 0x95, 0x08, // Report Count (8) + 0x81, 0x02, // Input (Data,Var,Abs,No Wrap,Linear,Preferred State,No Null Position) + 0x95, 0x01, // Report Count (1) + 0x75, 0x08, // Report Size (8) + 0x81, 0x01, // Input (Const,Array,Abs,No Wrap,Linear,Preferred State,No Null Position) + 0x95, 0x03, // Report Count (3) + 0x75, 0x01, // Report Size (1) + 0x05, 0x08, // Usage Page (LEDs) + 0x19, 0x01, // Usage Minimum (Num Lock) + 0x29, 0x05, // Usage Maximum (Kana) + 0x91, 0x02, // Output (Data,Var,Abs,No Wrap,Linear,Preferred State,No Null Position,Non-volatile) + 0x95, 0x01, // Report Count (1) + 0x75, 0x05, // Report Size (5) + 0x91, 0x01, // Output (Const,Array,Abs,No Wrap,Linear,Preferred State,No Null Position,Non-volatile) + 0x95, 0x06, // Report Count (6) + 0x75, 0x08, // Report Size (8) + 0x15, 0x00, // Logical Minimum (0) + 0x26, 0xFF, 0x00, // Logical Maximum (255) + 0x05, 0x07, // Usage Page (Kbrd/Keypad) + 0x19, 0x00, // Usage Minimum (0x00) + 0x2A, 0xFF, 0x00, // Usage Maximum (0xFF) + 0x81, 0x00, // Input (Data,Array,Abs,No Wrap,Linear,Preferred State,No Null Position) + 0xC0, // End Collection }; const usb_hid_device_obj_t usb_hid_device_keyboard_obj = { @@ -84,11 +86,11 @@ const usb_hid_device_obj_t usb_hid_device_keyboard_obj = { }; static const uint8_t mouse_report_descriptor[] = { - 0x05, 0x01, // 0,1 Usage Page (Generic Desktop Ctrls) - 0x09, 0x02, // 2,3 Usage (Mouse) - 0xA1, 0x01, // 4,5 Collection (Application) - 0x09, 0x01, // 6,7 Usage (Pointer) - 0xA1, 0x00, // 8,9 Collection (Physical) + 0x05, 0x01, // Usage Page (Generic Desktop Ctrls) + 0x09, 0x02, // Usage (Mouse) + 0xA1, 0x01, // Collection (Application) + 0x09, 0x01, // Usage (Pointer) + 0xA1, 0x00, // Collection (Physical) 0x85, 0x02, // 10, 11 Report ID (2) 0x05, 0x09, // Usage Page (Button) 0x19, 0x01, // Usage Minimum (0x01) @@ -134,10 +136,10 @@ const usb_hid_device_obj_t usb_hid_device_mouse_obj = { }; static const uint8_t consumer_control_report_descriptor[] = { - 0x05, 0x0C, // 0,1 Usage Page (Consumer) - 0x09, 0x01, // 2,3 Usage (Consumer Control) - 0xA1, 0x01, // 4,5 Collection (Application) - 0x85, 0x03, // 6,7 Report ID (3) + 0x05, 0x0C, // Usage Page (Consumer) + 0x09, 0x01, // Usage (Consumer Control) + 0xA1, 0x01, // Collection (Application) + 0x85, 0x03, // Report ID (3) 0x75, 0x10, // Report Size (16) 0x95, 0x01, // Report Count (1) 0x15, 0x01, // Logical Minimum (1) diff --git a/shared-module/usb_hid/__init__.c b/shared-module/usb_hid/__init__.c index 2be9a646d6..e61d01cc16 100644 --- a/shared-module/usb_hid/__init__.c +++ b/shared-module/usb_hid/__init__.c @@ -250,8 +250,9 @@ void usb_hid_gc_collect(void) { // and not copied into RAM. gc_collect_ptr((void *)hid_devices[device_idx].report_descriptor); - // Collect all the OUT report buffers for this device. + // Collect all the report buffers for this device. for (size_t id_idx = 0; id_idx < hid_devices[device_idx].num_report_ids; id_idx++) { + gc_collect_ptr(hid_devices[id_idx].in_report_buffers[id_idx]); gc_collect_ptr(hid_devices[id_idx].out_report_buffers[id_idx]); } } From 1bcf66ff8dc9fe377d38294be4805c558a0b9e15 Mon Sep 17 00:00:00 2001 From: Dan Halbert Date: Tue, 17 Aug 2021 15:34:48 -0400 Subject: [PATCH 5/5] Check buffer existence in tud callbacks (addresses #5020) --- shared-module/usb_hid/Device.c | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/shared-module/usb_hid/Device.c b/shared-module/usb_hid/Device.c index 50fd93fa68..e864b08873 100644 --- a/shared-module/usb_hid/Device.c +++ b/shared-module/usb_hid/Device.c @@ -280,8 +280,11 @@ uint16_t tud_hid_get_report_cb(uint8_t itf, uint8_t report_id, hid_report_type_t 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)) { - memcpy(buffer, hid_device->in_report_buffers[id_idx], reqlen); - return reqlen; + // Make sure buffer exists before trying to copy into it. + if (hid_device->in_report_buffers[id_idx]) { + memcpy(buffer, hid_device->in_report_buffers[id_idx], reqlen); + return reqlen; + } } return 0; } @@ -302,7 +305,9 @@ void tud_hid_set_report_cb(uint8_t itf, uint8_t report_id, hid_report_type_t rep // 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) { + if (hid_device && + hid_device->out_report_buffers[id_idx] && + hid_device->out_report_lengths[id_idx] >= bufsize) { memcpy(hid_device->out_report_buffers[id_idx], buffer, bufsize); } }