From 24c6295ed49fa3e328f67771f7dd71ff11dea490 Mon Sep 17 00:00:00 2001 From: Dan Halbert Date: Sat, 2 Oct 2021 22:50:36 -0400 Subject: [PATCH 1/6] Boot protocol and feature reports: compiles; needs to be tested --- ports/espressif/esp-idf | 2 +- shared-bindings/usb_hid/__init__.c | 77 ++++++++++++++++++++++++++---- shared-bindings/usb_hid/__init__.h | 3 +- shared-module/usb_hid/Device.c | 12 ++--- shared-module/usb_hid/__init__.c | 46 +++++++++++++++--- shared-module/usb_hid/__init__.h | 3 +- supervisor/shared/usb/usb_desc.c | 2 +- 7 files changed, 120 insertions(+), 25 deletions(-) diff --git a/ports/espressif/esp-idf b/ports/espressif/esp-idf index e493a4c30e..48ae2309fd 160000 --- a/ports/espressif/esp-idf +++ b/ports/espressif/esp-idf @@ -1 +1 @@ -Subproject commit e493a4c30e1b2f7c54a376327388b9e12262cbbf +Subproject commit 48ae2309fd9cea600ff960b61a029892be1fdc1b diff --git a/shared-bindings/usb_hid/__init__.c b/shared-bindings/usb_hid/__init__.c index d1b4357dc1..0c60015ee7 100644 --- a/shared-bindings/usb_hid/__init__.c +++ b/shared-bindings/usb_hid/__init__.c @@ -60,7 +60,7 @@ STATIC mp_obj_t usb_hid_disable(void) { } MP_DEFINE_CONST_FUN_OBJ_0(usb_hid_disable_obj, usb_hid_disable); -//| def enable(devices: Optional[Sequence[Device]]) -> None: +//| def enable(devices: Optional[Sequence[Device]], boot_device: int = 0) -> None: //| """Specify which USB HID devices that will be available. //| Can be called in ``boot.py``, before USB is connected. //| @@ -68,15 +68,49 @@ MP_DEFINE_CONST_FUN_OBJ_0(usb_hid_disable_obj, usb_hid_disable); //| If `devices` is empty, HID is disabled. The order of the ``Devices`` //| may matter to the host. For instance, for MacOS, put the mouse device //| before any Gamepad or Digitizer HID device or else it will not work. +//| :param int boot_device: If non-zero, inform the host that support for a +//| a boot HID device is available. +//| If ``boot_device=1``, a boot keyboard is available. +//| If ``boot_device=2``, a boot mouse is available. No other values are allowed. +//| See below. //| //| 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 //| not enough endpoints are available. +//| +//| **Boot Devices** +//| +//| Boot devices implement a fixed, predefined report descriptor, defined in +//| https://www.usb.org/sites/default/files/hid1_12.pdf, Appendix B. A USB host +//| can request to use the boot device if the USB device says it is available. +//| Usually only a BIOS or other kind of boot-time, limited-functionality +//| host needs boot keyboard support. +//| +//| For example, to make a boot keyboard available, you can use this code:: +//| +//| usb_hid.enable((Device.KEYBOARD), boot_device=1) +//| +//| If the host requests the boot keyboard, the report descriptor provided by `Device.KEYBOARD` +//| will be ignored, and the predefined report descriptor will be used. +//| But if the host does not request the boot keyboard, +//| the descriptor provided by `Device.KEYBOARD` will be used. //| """ //| ... //| -STATIC mp_obj_t usb_hid_enable(mp_obj_t devices) { +STATIC mp_obj_t usb_hid_enable(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_devices, ARG_boot_device }; + static const mp_arg_t allowed_args[] = { + { MP_QSTR_devices, MP_ARG_REQUIRED | MP_ARG_OBJ }, + { MP_QSTR_boot_device, MP_ARG_OBJ, {.u_int = 0} }, + }; + + 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_obj_t devices = args[ARG_devices].u_obj; const mp_int_t len = mp_obj_get_int(mp_obj_len(devices)); for (mp_int_t i = 0; i < len; i++) { mp_obj_t item = mp_obj_subscr(devices, MP_OBJ_NEW_SMALL_INT(i), MP_OBJ_SENTINEL); @@ -85,21 +119,46 @@ STATIC mp_obj_t usb_hid_enable(mp_obj_t devices) { } } - if (!common_hal_usb_hid_enable(devices)) { + uint8_t boot_device = + (uint8_t)mp_arg_validate_int_range(args[ARG_boot_device].u_int, 0, 2, MP_QSTR_boot_device); + + if (!common_hal_usb_hid_enable(devices, boot_device)) { mp_raise_RuntimeError(translate("Cannot change USB devices now")); } return mp_const_none; } -MP_DEFINE_CONST_FUN_OBJ_1(usb_hid_enable_obj, usb_hid_enable); +MP_DEFINE_CONST_FUN_OBJ_KW(usb_hid_enable_obj, 2, usb_hid_enable); + +//| def get_boot_device() -> int: +//| """ +//| :return: the boot device requested by the host, if any. +//| Returns 0 if the host did not request a boot device, or if `usb_hid.enable()` +//| was called with `boot_device=0`, the default, which disables boot device support. +//| If the host did request aboot device, +//| returns the value of ``boot_device`` set in `usb_hid.enable()`: +//| ``1`` for a boot keyboard, or ``2`` for boot mouse. +//| Your device driver should check and act on this value if the keyboard or mouse +//| device you specified provides reports that differ from the standard boot keyboard or mouse. +//| However, the standard devices provided by CircuitPython, `Device.KEYBOARD` and `Device.MOUSE`, +//| describe reports that match the boot device reports, so you don't need to check this +//| if you are using those devices. +//| :rtype int: +//| """ +//| +STATIC mp_obj_t usb_hid_get_boot_device(void) { + return MP_OBJ_NEW_SMALL_INT(common_hal_usb_hid_get_boot_device()); +} +MP_DEFINE_CONST_FUN_OBJ_0(usb_hid_get_boot_device_obj, usb_hid_get_boot_device); // usb_hid.devices is set once the usb devices are determined, after boot.py runs. STATIC mp_map_elem_t usb_hid_module_globals_table[] = { - { MP_ROM_QSTR(MP_QSTR___name__), MP_OBJ_NEW_QSTR(MP_QSTR_usb_hid) }, - { MP_ROM_QSTR(MP_QSTR_Device), MP_OBJ_FROM_PTR(&usb_hid_device_type) }, - { MP_ROM_QSTR(MP_QSTR_devices), mp_const_none }, - { MP_ROM_QSTR(MP_QSTR_disable), MP_OBJ_FROM_PTR(&usb_hid_disable_obj) }, - { MP_ROM_QSTR(MP_QSTR_enable), MP_OBJ_FROM_PTR(&usb_hid_enable_obj) }, + { MP_ROM_QSTR(MP_QSTR___name__), MP_OBJ_NEW_QSTR(MP_QSTR_usb_hid) }, + { MP_ROM_QSTR(MP_QSTR_Device), MP_OBJ_FROM_PTR(&usb_hid_device_type) }, + { MP_ROM_QSTR(MP_QSTR_devices), mp_const_none }, + { MP_ROM_QSTR(MP_QSTR_disable), MP_OBJ_FROM_PTR(&usb_hid_disable_obj) }, + { MP_ROM_QSTR(MP_QSTR_enable), MP_OBJ_FROM_PTR(&usb_hid_enable_obj) }, + { MP_ROM_QSTR(MP_QSTR_get_boot_device), MP_OBJ_FROM_PTR(&usb_hid_get_boot_device_obj) }, }; STATIC MP_DEFINE_MUTABLE_DICT(usb_hid_module_globals, usb_hid_module_globals_table); diff --git a/shared-bindings/usb_hid/__init__.h b/shared-bindings/usb_hid/__init__.h index d5fc7743a3..feaeed2545 100644 --- a/shared-bindings/usb_hid/__init__.h +++ b/shared-bindings/usb_hid/__init__.h @@ -36,6 +36,7 @@ extern mp_obj_tuple_t common_hal_usb_hid_devices; void usb_hid_set_devices(mp_obj_t devices); bool common_hal_usb_hid_disable(void); -bool common_hal_usb_hid_enable(const mp_obj_t devices_seq); +bool common_hal_usb_hid_enable(const mp_obj_t devices_seq, uint8_t boot_device); +uint8_t common_hal_usb_hid_get_boot_device(void); #endif // SHARED_BINDINGS_USB_HID_H diff --git a/shared-module/usb_hid/Device.c b/shared-module/usb_hid/Device.c index 71c6dea7af..de34d60d0a 100644 --- a/shared-module/usb_hid/Device.c +++ b/shared-module/usb_hid/Device.c @@ -244,7 +244,7 @@ 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. + // report_id has already been validated for this device. 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]); } @@ -266,11 +266,11 @@ void usb_hid_device_create_report_buffers(usb_hid_device_obj_t *self) { } -// Callbacks invoked when we received Get_Report request through control endpoint +// Callback invoked when we receive 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 - if (report_type != HID_REPORT_TYPE_INPUT) { + // Support Input Report and Feature Report + if (report_type != HID_REPORT_TYPE_INPUT && report_type != HID_REPORT_TYPE_FEATURE) { return 0; } @@ -289,14 +289,14 @@ uint16_t tud_hid_get_report_cb(uint8_t itf, uint8_t report_id, hid_report_type_t return 0; } -// Callbacks invoked when we received Set_Report request through control endpoint +// Callback invoked when we 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) { + } else if (report_type != HID_REPORT_TYPE_OUTPUT && report_type != HID_REPORT_TYPE_FEATURE) { return; } diff --git a/shared-module/usb_hid/__init__.c b/shared-module/usb_hid/__init__.c index efe37662ee..fb5c9dd107 100644 --- a/shared-module/usb_hid/__init__.c +++ b/shared-module/usb_hid/__init__.c @@ -44,7 +44,9 @@ static const uint8_t usb_hid_descriptor_template[] = { 0x02, // 4 bNumEndpoints 2 0x03, // 5 bInterfaceClass: HID 0x00, // 6 bInterfaceSubClass: NOBOOT +#define HID_DESCRIPTOR_SUBCLASS_INDEX (6) 0x00, // 7 bInterfaceProtocol: NONE +#define HID_DESCRIPTOR_INTERFACE_PROTOCOL_INDEX (7) 0xFF, // 8 iInterface (String Index) [SET AT RUNTIME] #define HID_DESCRIPTOR_INTERFACE_STRING_INDEX (8) @@ -81,6 +83,14 @@ static usb_hid_device_obj_t hid_devices[MAX_HID_DEVICES]; // If 0, USB HID is disabled. static mp_int_t num_hid_devices; +// Which boot device is available 0: no boot devices, 1: boot keyboard, 2: boot mouse. +// This value is set by usb_hid.enable(), and used to build the HID interface descriptor. +// The value is remembered here from boot.py to code.py. +static uint8_t hid_boot_device; + +// Whether a boot device was requested by a SET_PROTOCOL request from the host. +static bool hid_boot_device_requested; + // This tuple is store in usb_hid.devices. static mp_obj_tuple_t *hid_devices_tuple; @@ -100,9 +110,19 @@ bool usb_hid_enabled(void) { return num_hid_devices > 0; } +uint8_t usb_hid_boot_device(void) { + return hid_boot_device; +} + +// Returns 1 or 2 if host requested a boot device and boot protocol was enabled in the interface descriptor. +uint8_t common_hal_usb_hid_get_boot_device(void) { + return hid_boot_device_requested ? hid_boot_device : 0; +} + void usb_hid_set_defaults(void) { + hid_boot_device = 0; common_hal_usb_hid_enable( - CIRCUITPY_USB_HID_ENABLED_DEFAULT ? &default_hid_devices_tuple : mp_const_empty_tuple); + CIRCUITPY_USB_HID_ENABLED_DEFAULT ? &default_hid_devices_tuple : mp_const_empty_tuple, 0); } // This is the interface descriptor, not the report descriptor. @@ -113,12 +133,17 @@ size_t usb_hid_descriptor_length(void) { static const char usb_hid_interface_name[] = USB_INTERFACE_NAME " HID"; // This is the interface descriptor, not the report descriptor. -size_t usb_hid_add_descriptor(uint8_t *descriptor_buf, descriptor_counts_t *descriptor_counts, uint8_t *current_interface_string, uint16_t report_descriptor_length) { +size_t usb_hid_add_descriptor(uint8_t *descriptor_buf, descriptor_counts_t *descriptor_counts, uint8_t *current_interface_string, uint16_t report_descriptor_length, uint8_t boot_device) { memcpy(descriptor_buf, usb_hid_descriptor_template, sizeof(usb_hid_descriptor_template)); descriptor_buf[HID_DESCRIPTOR_INTERFACE_INDEX] = descriptor_counts->current_interface; descriptor_counts->current_interface++; + if (boot_device > 0) { + descriptor_buf[HID_DESCRIPTOR_SUBCLASS_INDEX] = 1; // BOOT protocol (device) available. + descriptor_buf[HID_DESCRIPTOR_INTERFACE_PROTOCOL_INDEX] = boot_device; // 1: keyboard, 2: mouse + } + usb_add_interface_string(*current_interface_string, usb_hid_interface_name); descriptor_buf[HID_DESCRIPTOR_INTERFACE_STRING_INDEX] = *current_interface_string; (*current_interface_string)++; @@ -151,10 +176,10 @@ static void usb_hid_set_devices_from_hid_devices(void) { } bool common_hal_usb_hid_disable(void) { - return common_hal_usb_hid_enable(mp_const_empty_tuple); + return common_hal_usb_hid_enable(mp_const_empty_tuple, 0); } -bool common_hal_usb_hid_enable(const mp_obj_t devices) { +bool common_hal_usb_hid_enable(const mp_obj_t devices, uint8_t boot_device) { // We can't change the devices once we're connected. if (tud_connected()) { return false; @@ -167,6 +192,8 @@ bool common_hal_usb_hid_enable(const mp_obj_t devices) { num_hid_devices = num_devices; + hid_boot_device = boot_device; + // Remember the devices in static storage so they live across VMs. for (mp_int_t i = 0; i < num_hid_devices; i++) { // devices has already been validated to contain only usb_hid_device_obj_t objects. @@ -182,6 +209,7 @@ bool common_hal_usb_hid_enable(const mp_obj_t devices) { // Called when HID devices are ready to be used, when code.py or the REPL starts running. void usb_hid_setup_devices(void) { + hid_boot_device_requested = false; usb_hid_set_devices_from_hid_devices(); // Create report buffers on the heap. @@ -272,9 +300,15 @@ bool usb_hid_get_device_with_report_id(uint8_t report_id, usb_hid_device_obj_t * return false; } -// Invoked when GET HID REPORT DESCRIPTOR is received. -// Application return pointer to descriptor +// Callback invoked when we receive a GET HID REPORT DESCRIPTOR +// Application returns 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; } + +// Callback invoked when we receive a SET_PROTOCOL request. +// Protocol is either HID_PROTOCOL_BOOT (0) or HID_PROTOCOL_REPORT (1) +void tud_hid_set_protocol_cb(uint8_t instance, uint8_t protocol) { + hid_boot_device_requested = (protocol == HID_PROTOCOL_BOOT); +} diff --git a/shared-module/usb_hid/__init__.h b/shared-module/usb_hid/__init__.h index 6d85b50e9f..1a33540321 100644 --- a/shared-module/usb_hid/__init__.h +++ b/shared-module/usb_hid/__init__.h @@ -33,9 +33,10 @@ extern usb_hid_device_obj_t usb_hid_devices[]; bool usb_hid_enabled(void); +uint8_t usb_hid_boot_device(void); void usb_hid_set_defaults(void); -size_t usb_hid_add_descriptor(uint8_t *descriptor_buf, descriptor_counts_t *descriptor_counts, uint8_t *current_interface_string, uint16_t report_descriptor_length); +size_t usb_hid_add_descriptor(uint8_t *descriptor_buf, descriptor_counts_t *descriptor_counts, uint8_t *current_interface_string, uint16_t report_descriptor_length, uint8_t boot_device); size_t usb_hid_descriptor_length(void); size_t usb_hid_report_descriptor_length(void); diff --git a/supervisor/shared/usb/usb_desc.c b/supervisor/shared/usb/usb_desc.c index bad0d84523..0324be7b8f 100644 --- a/supervisor/shared/usb/usb_desc.c +++ b/supervisor/shared/usb/usb_desc.c @@ -223,7 +223,7 @@ static void usb_build_configuration_descriptor(void) { if (usb_hid_enabled()) { descriptor_buf_remaining += usb_hid_add_descriptor( descriptor_buf_remaining, &descriptor_counts, ¤t_interface_string, - usb_hid_report_descriptor_length()); + usb_hid_report_descriptor_length(), usb_hid_boot_device()); } #endif From 1ae69b5cc5dc545d0369de1768a001e890d2f1c5 Mon Sep 17 00:00:00 2001 From: Dan Halbert Date: Thu, 7 Oct 2021 08:19:53 -0400 Subject: [PATCH 2/6] debugging, with instrumentation --- shared-bindings/microcontroller/Processor.c | 3 +- shared-bindings/usb_hid/Device.c | 2 +- shared-bindings/usb_hid/__init__.c | 19 ++++---- shared-module/usb_hid/__init__.c | 50 ++++++++++++++++++++- 4 files changed, 62 insertions(+), 12 deletions(-) diff --git a/shared-bindings/microcontroller/Processor.c b/shared-bindings/microcontroller/Processor.c index e1d569efca..c1bb626db8 100644 --- a/shared-bindings/microcontroller/Processor.c +++ b/shared-bindings/microcontroller/Processor.c @@ -97,8 +97,9 @@ const mp_obj_property_t mcu_processor_reset_reason_obj = { //| //| Is `None` if the temperature is not available.""" //| +extern volatile float indicator; STATIC mp_obj_t mcu_processor_get_temperature(mp_obj_t self) { - float temperature = common_hal_mcu_processor_get_temperature(); + float temperature = indicator; // common_hal_mcu_processor_get_temperature(); return isnan(temperature) ? mp_const_none : mp_obj_new_float(temperature); } diff --git a/shared-bindings/usb_hid/Device.c b/shared-bindings/usb_hid/Device.c index 44f3216b01..32305afd06 100644 --- a/shared-bindings/usb_hid/Device.c +++ b/shared-bindings/usb_hid/Device.c @@ -136,7 +136,7 @@ STATIC mp_obj_t usb_hid_device_make_new(const mp_obj_type_t *type, size_t n_args // 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); + 0, 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, i_obj, MP_OBJ_SENTINEL)), diff --git a/shared-bindings/usb_hid/__init__.c b/shared-bindings/usb_hid/__init__.c index 0c60015ee7..5c903dda5d 100644 --- a/shared-bindings/usb_hid/__init__.c +++ b/shared-bindings/usb_hid/__init__.c @@ -40,6 +40,11 @@ //| """Tuple of all active HID device interfaces. //| The default set of devices is ``Device.KEYBOARD, Device.MOUSE, Device.CONSUMER_CONTROL``, //| On boards where `usb_hid` is disabled by default, `devices` is an empty tuple. +//| +//| If a boot device is enabled by `usb_hid.enable)`, *and* the host has requested a boot device, +//| the `devices` tuple is *replaced* when ``code.py`` starts with a single-element tuple +//| containing a `Device` that describes the boot device chosen (keyboard or mouse). +//| The request for a boot device overrides any other HID devices. //| """ //| @@ -84,12 +89,12 @@ MP_DEFINE_CONST_FUN_OBJ_0(usb_hid_disable_obj, usb_hid_disable); //| Boot devices implement a fixed, predefined report descriptor, defined in //| https://www.usb.org/sites/default/files/hid1_12.pdf, Appendix B. A USB host //| can request to use the boot device if the USB device says it is available. -//| Usually only a BIOS or other kind of boot-time, limited-functionality +//| Usually only a BIOS or other kind of limited-functionality //| host needs boot keyboard support. //| //| For example, to make a boot keyboard available, you can use this code:: //| -//| usb_hid.enable((Device.KEYBOARD), boot_device=1) +//| usb_hid.enable((Device.KEYBOARD), boot_device=1) # 1 for a keyboard //| //| If the host requests the boot keyboard, the report descriptor provided by `Device.KEYBOARD` //| will be ignored, and the predefined report descriptor will be used. @@ -104,11 +109,11 @@ STATIC mp_obj_t usb_hid_enable(size_t n_args, const mp_obj_t *pos_args, mp_map_t enum { ARG_devices, ARG_boot_device }; static const mp_arg_t allowed_args[] = { { MP_QSTR_devices, MP_ARG_REQUIRED | MP_ARG_OBJ }, - { MP_QSTR_boot_device, MP_ARG_OBJ, {.u_int = 0} }, + { MP_QSTR_boot_device, MP_ARG_INT, {.u_int = 0} }, }; 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_arg_parse_all(n_args, pos_args, kw_args, MP_ARRAY_SIZE(allowed_args), allowed_args, args); mp_obj_t devices = args[ARG_devices].u_obj; const mp_int_t len = mp_obj_get_int(mp_obj_len(devices)); @@ -128,18 +133,16 @@ STATIC mp_obj_t usb_hid_enable(size_t n_args, const mp_obj_t *pos_args, mp_map_t return mp_const_none; } -MP_DEFINE_CONST_FUN_OBJ_KW(usb_hid_enable_obj, 2, usb_hid_enable); +MP_DEFINE_CONST_FUN_OBJ_KW(usb_hid_enable_obj, 1, usb_hid_enable); //| def get_boot_device() -> int: //| """ //| :return: the boot device requested by the host, if any. //| Returns 0 if the host did not request a boot device, or if `usb_hid.enable()` //| was called with `boot_device=0`, the default, which disables boot device support. -//| If the host did request aboot device, +//| If the host did request a boot device, //| returns the value of ``boot_device`` set in `usb_hid.enable()`: //| ``1`` for a boot keyboard, or ``2`` for boot mouse. -//| Your device driver should check and act on this value if the keyboard or mouse -//| device you specified provides reports that differ from the standard boot keyboard or mouse. //| However, the standard devices provided by CircuitPython, `Device.KEYBOARD` and `Device.MOUSE`, //| describe reports that match the boot device reports, so you don't need to check this //| if you are using those devices. diff --git a/shared-module/usb_hid/__init__.c b/shared-module/usb_hid/__init__.c index fb5c9dd107..21bf6e72a1 100644 --- a/shared-module/usb_hid/__init__.c +++ b/shared-module/usb_hid/__init__.c @@ -31,10 +31,12 @@ #include "py/gc.h" #include "py/runtime.h" #include "shared-bindings/usb_hid/__init__.h" -#include "shared-module/usb_hid/Device.h" +#include "shared-bindings/usb_hid/Device.h" #include "supervisor/memory.h" #include "supervisor/usb.h" +volatile float indicator = 0.1f; + static const uint8_t usb_hid_descriptor_template[] = { 0x09, // 0 bLength 0x04, // 1 bDescriptorType (Interface) @@ -86,6 +88,7 @@ static mp_int_t num_hid_devices; // Which boot device is available 0: no boot devices, 1: boot keyboard, 2: boot mouse. // This value is set by usb_hid.enable(), and used to build the HID interface descriptor. // The value is remembered here from boot.py to code.py. + static uint8_t hid_boot_device; // Whether a boot device was requested by a SET_PROTOCOL request from the host. @@ -106,6 +109,38 @@ static mp_obj_tuple_t default_hid_devices_tuple = { }, }; +// These describe the standard descriptors used for boot keyboard and mouse, which don't use report IDs. +// When the host requests a boot device, replace whatever HID devices were enabled with a tuple +// containing just one of these, since the host is uninterested in other devices. +// The driver code will then use the proper report length and send_report() will not send a report ID. +static const usb_hid_device_obj_t boot_keyboard_obj = { + .base = { + .type = &usb_hid_device_type, + }, + .report_descriptor = NULL, + .report_descriptor_length = 0, + .usage_page = 0x01, + .usage = 0x06, + .num_report_ids = 1, + .report_ids = { 0, }, + .in_report_lengths = { 8, }, + .out_report_lengths = { 1, }, +}; + +static const usb_hid_device_obj_t boot_mouse_obj = { + .base = { + .type = &usb_hid_device_type, + }, + .report_descriptor = NULL, + .report_descriptor_length = 0, + .usage_page = 0x01, + .usage = 0x02, + .num_report_ids = 1, + .report_ids = { 0, }, + .in_report_lengths = { 4, }, + .out_report_lengths = { 0, }, +}; + bool usb_hid_enabled(void) { return num_hid_devices > 0; } @@ -121,6 +156,7 @@ uint8_t common_hal_usb_hid_get_boot_device(void) { void usb_hid_set_defaults(void) { hid_boot_device = 0; + hid_boot_device_requested = false; common_hal_usb_hid_enable( CIRCUITPY_USB_HID_ENABLED_DEFAULT ? &default_hid_devices_tuple : mp_const_empty_tuple, 0); } @@ -209,7 +245,17 @@ bool common_hal_usb_hid_enable(const mp_obj_t devices, uint8_t boot_device) { // Called when HID devices are ready to be used, when code.py or the REPL starts running. void usb_hid_setup_devices(void) { - hid_boot_device_requested = false; + + // If the host requested a boot device, replace the current list of devices + // with a single-element tuple containing the proper boot device. + if (hid_boot_device_requested) { + memcpy(&hid_devices[0], + // Will be 1 (keyboard) or 2 (mouse). + hid_boot_device == 1 ? &boot_keyboard_obj : &boot_mouse_obj, + sizeof(usb_hid_device_obj_t)); + num_hid_devices = 1; + } + usb_hid_set_devices_from_hid_devices(); // Create report buffers on the heap. From 8b97ad1b7e050083700cb30ca0276bd3902bf75b Mon Sep 17 00:00:00 2001 From: Dan Halbert Date: Thu, 7 Oct 2021 14:37:57 -0400 Subject: [PATCH 3/6] remove unused self in usb_hid_enable --- shared-bindings/usb_hid/__init__.c | 2 -- 1 file changed, 2 deletions(-) diff --git a/shared-bindings/usb_hid/__init__.c b/shared-bindings/usb_hid/__init__.c index 5c903dda5d..b0d488fd19 100644 --- a/shared-bindings/usb_hid/__init__.c +++ b/shared-bindings/usb_hid/__init__.c @@ -104,8 +104,6 @@ MP_DEFINE_CONST_FUN_OBJ_0(usb_hid_disable_obj, usb_hid_disable); //| ... //| STATIC mp_obj_t usb_hid_enable(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_devices, ARG_boot_device }; static const mp_arg_t allowed_args[] = { { MP_QSTR_devices, MP_ARG_REQUIRED | MP_ARG_OBJ }, From a911cbef51667bd4071d1c525456c72e6ec96c11 Mon Sep 17 00:00:00 2001 From: Dan Halbert Date: Wed, 13 Oct 2021 12:30:01 -0400 Subject: [PATCH 4/6] check that boot device is interface #0; remove instrumentation --- locale/circuitpython.pot | 4 ++++ shared-bindings/microcontroller/Processor.c | 3 +-- shared-bindings/usb_hid/__init__.c | 6 ++++++ shared-module/usb_hid/__init__.c | 5 +---- supervisor/shared/safe_mode.c | 3 +++ supervisor/shared/safe_mode.h | 1 + supervisor/shared/usb/usb_desc.c | 5 +++++ 7 files changed, 21 insertions(+), 6 deletions(-) diff --git a/locale/circuitpython.pot b/locale/circuitpython.pot index 4b8a26a9ac..a177820c1b 100644 --- a/locale/circuitpython.pot +++ b/locale/circuitpython.pot @@ -551,6 +551,10 @@ msgstr "" msgid "Bit depth must be multiple of 8." msgstr "" +#: supervisor/shared/safe_mode.c +msgid "Boot device must be first device (interface #0)." +msgstr "" + #: ports/mimxrt10xx/common-hal/busio/UART.c msgid "Both RX and TX required for flow control" msgstr "" diff --git a/shared-bindings/microcontroller/Processor.c b/shared-bindings/microcontroller/Processor.c index c1bb626db8..e1d569efca 100644 --- a/shared-bindings/microcontroller/Processor.c +++ b/shared-bindings/microcontroller/Processor.c @@ -97,9 +97,8 @@ const mp_obj_property_t mcu_processor_reset_reason_obj = { //| //| Is `None` if the temperature is not available.""" //| -extern volatile float indicator; STATIC mp_obj_t mcu_processor_get_temperature(mp_obj_t self) { - float temperature = indicator; // common_hal_mcu_processor_get_temperature(); + float temperature = common_hal_mcu_processor_get_temperature(); return isnan(temperature) ? mp_const_none : mp_obj_new_float(temperature); } diff --git a/shared-bindings/usb_hid/__init__.c b/shared-bindings/usb_hid/__init__.c index b0d488fd19..19644ac91b 100644 --- a/shared-bindings/usb_hid/__init__.c +++ b/shared-bindings/usb_hid/__init__.c @@ -100,6 +100,12 @@ MP_DEFINE_CONST_FUN_OBJ_0(usb_hid_disable_obj, usb_hid_disable); //| will be ignored, and the predefined report descriptor will be used. //| But if the host does not request the boot keyboard, //| the descriptor provided by `Device.KEYBOARD` will be used. +//| +//| The HID boot device must usually be the first or only device presented by CircuitPython. +//| The HID device will be USB interface number 0. +//| To make sure it is the first device, disable other USB devices, including CDC and MSC (CIRCUITPY). +//| If you specify a non-zero ``boot_device``, and it is not the first device, CircuitPython +//| will enter safe mode to report this error. //| """ //| ... //| diff --git a/shared-module/usb_hid/__init__.c b/shared-module/usb_hid/__init__.c index 21bf6e72a1..89a05c77b3 100644 --- a/shared-module/usb_hid/__init__.c +++ b/shared-module/usb_hid/__init__.c @@ -35,8 +35,6 @@ #include "supervisor/memory.h" #include "supervisor/usb.h" -volatile float indicator = 0.1f; - static const uint8_t usb_hid_descriptor_template[] = { 0x09, // 0 bLength 0x04, // 1 bDescriptorType (Interface) @@ -85,10 +83,9 @@ static usb_hid_device_obj_t hid_devices[MAX_HID_DEVICES]; // If 0, USB HID is disabled. static mp_int_t num_hid_devices; -// Which boot device is available 0: no boot devices, 1: boot keyboard, 2: boot mouse. +// Which boot device is available? 0: no boot devices, 1: boot keyboard, 2: boot mouse. // This value is set by usb_hid.enable(), and used to build the HID interface descriptor. // The value is remembered here from boot.py to code.py. - static uint8_t hid_boot_device; // Whether a boot device was requested by a SET_PROTOCOL request from the host. diff --git a/supervisor/shared/safe_mode.c b/supervisor/shared/safe_mode.c index 646467ce39..59b5ab838d 100644 --- a/supervisor/shared/safe_mode.c +++ b/supervisor/shared/safe_mode.c @@ -169,6 +169,9 @@ void print_safe_mode_message(safe_mode_t reason) { case USB_TOO_MANY_INTERFACE_NAMES: message = translate("USB devices specify too many interface names."); break; + case USB_BOOT_DEVICE_NOT_INTERFACE_ZERO: + message = translate("Boot device must be first device (interface #0)."); + break; case WATCHDOG_RESET: message = translate("Watchdog timer expired."); break; diff --git a/supervisor/shared/safe_mode.h b/supervisor/shared/safe_mode.h index 01aed37d63..600abd7672 100644 --- a/supervisor/shared/safe_mode.h +++ b/supervisor/shared/safe_mode.h @@ -46,6 +46,7 @@ typedef enum { WATCHDOG_RESET, USB_TOO_MANY_ENDPOINTS, USB_TOO_MANY_INTERFACE_NAMES, + USB_BOOT_DEVICE_NOT_INTERFACE_ZERO, NO_HEAP, } safe_mode_t; diff --git a/supervisor/shared/usb/usb_desc.c b/supervisor/shared/usb/usb_desc.c index 3b7382e11e..9fe1eadd4e 100644 --- a/supervisor/shared/usb/usb_desc.c +++ b/supervisor/shared/usb/usb_desc.c @@ -228,6 +228,11 @@ static void usb_build_configuration_descriptor(void) { #if CIRCUITPY_USB_HID if (usb_hid_enabled()) { + if (usb_hid_boot_device() > 0 && descriptor_counts.current_interface > 0) { + // Hosts using boot devices generally to expect them to be at interface zero, + // and will not work properly otherwise. + reset_into_safe_mode(USB_BOOT_DEVICE_NOT_INTERFACE_ZERO); + } descriptor_buf_remaining += usb_hid_add_descriptor( descriptor_buf_remaining, &descriptor_counts, ¤t_interface_string, usb_hid_report_descriptor_length(), usb_hid_boot_device()); From 13210d2cce8f0bdb650fcad2c90279b9af68eaa8 Mon Sep 17 00:00:00 2001 From: Dan Halbert Date: Wed, 13 Oct 2021 15:38:39 -0400 Subject: [PATCH 5/6] feature report doc --- shared-bindings/usb_hid/Device.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/shared-bindings/usb_hid/Device.c b/shared-bindings/usb_hid/Device.c index 30379a25e9..1553efe823 100644 --- a/shared-bindings/usb_hid/Device.c +++ b/shared-bindings/usb_hid/Device.c @@ -50,7 +50,8 @@ //| Use a size of ``0`` for a report that is not an OUT report. //| "OUT" is with respect to the host. //| -//| ``report_ids``, ``in_report_lengths``, and ``out_report_lengths`` must all be the same length. +//| ``report_ids``, ``in_report_lengths``, and ``out_report_lengths`` must all have the +//| same number of elements. //| //| Here is an example of a `Device` with a descriptor that specifies two report IDs, 3 and 4. //| Report ID 3 sends an IN report of length 5, and receives an OUT report of length 6. @@ -192,7 +193,7 @@ 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, 1, 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. +//| """Get the last received HID OUT or feature 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. //| """ From 11c80805773af1c336aeafa64cfc33fe3f64b32b Mon Sep 17 00:00:00 2001 From: Dan Halbert Date: Mon, 18 Oct 2021 15:37:15 -0400 Subject: [PATCH 6/6] touch up doc --- shared-bindings/usb_hid/__init__.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/shared-bindings/usb_hid/__init__.c b/shared-bindings/usb_hid/__init__.c index 19644ac91b..3922ded03c 100644 --- a/shared-bindings/usb_hid/__init__.c +++ b/shared-bindings/usb_hid/__init__.c @@ -41,8 +41,8 @@ //| The default set of devices is ``Device.KEYBOARD, Device.MOUSE, Device.CONSUMER_CONTROL``, //| On boards where `usb_hid` is disabled by default, `devices` is an empty tuple. //| -//| If a boot device is enabled by `usb_hid.enable)`, *and* the host has requested a boot device, -//| the `devices` tuple is *replaced* when ``code.py`` starts with a single-element tuple +//| If a boot device is enabled by `usb_hid.enable()`, *and* the host has requested a boot device, +//| the `devices` tuple is **replaced** when ``code.py`` starts with a single-element tuple //| containing a `Device` that describes the boot device chosen (keyboard or mouse). //| The request for a boot device overrides any other HID devices. //| """ @@ -143,7 +143,7 @@ MP_DEFINE_CONST_FUN_OBJ_KW(usb_hid_enable_obj, 1, usb_hid_enable); //| """ //| :return: the boot device requested by the host, if any. //| Returns 0 if the host did not request a boot device, or if `usb_hid.enable()` -//| was called with `boot_device=0`, the default, which disables boot device support. +//| was called with ``boot_device=0``, the default, which disables boot device support. //| If the host did request a boot device, //| returns the value of ``boot_device`` set in `usb_hid.enable()`: //| ``1`` for a boot keyboard, or ``2`` for boot mouse.