From 915f52730d4bd91642bfcf74a8ebfb18536517e9 Mon Sep 17 00:00:00 2001 From: Nathan Conrad Date: Tue, 17 Sep 2019 11:28:29 -0400 Subject: [PATCH 1/6] Implement HID desc request. --- src/class/hid/hid_device.c | 24 +++++++++++++++++++++++- 1 file changed, 23 insertions(+), 1 deletion(-) diff --git a/src/class/hid/hid_device.c b/src/class/hid/hid_device.c index 572f2cad2..2d579f475 100644 --- a/src/class/hid/hid_device.c +++ b/src/class/hid/hid_device.c @@ -202,7 +202,29 @@ bool hidd_control_request(uint8_t rhport, tusb_control_request_t const * p_reque uint8_t const desc_index = tu_u16_low (p_request->wValue); (void) desc_index; - if (p_request->bRequest == TUSB_REQ_GET_DESCRIPTOR && desc_type == HID_DESC_TYPE_REPORT) + if (p_request->bRequest == TUSB_REQ_GET_DESCRIPTOR && desc_type == HID_DESC_TYPE_HID) + { + // FIXME: Should check which is the active configuration, but no callback in usbd currently exists to do that + tusb_desc_configuration_t const* desc_cfg = + (tusb_desc_configuration_t const*) tud_descriptor_configuration_cb(0); + uint8_t const * p_desc = ((uint8_t const*) desc_cfg) + sizeof(tusb_desc_configuration_t); + uint8_t const * desc_end = ((uint8_t const*) desc_cfg) + desc_cfg->wTotalLength; + + while( p_desc < desc_end ) + { + tusb_hid_descriptor_hid_t *p_desc_hid =(tusb_hid_descriptor_hid_t*)p_desc; + if(p_desc_hid->bDescriptorType == HID_DESC_TYPE_HID) { + tud_control_xfer(rhport, p_request, (void*) p_desc_hid, p_desc_hid->bLength); + break; + } + p_desc += p_desc_hid->bLength; // next desc + } + if(p_desc >= desc_end) + { + return false; + } + } + else if (p_request->bRequest == TUSB_REQ_GET_DESCRIPTOR && desc_type == HID_DESC_TYPE_REPORT) { uint8_t const * desc_report = tud_hid_descriptor_report_cb(); tud_control_xfer(rhport, p_request, (void*) desc_report, p_hid->reprot_desc_len); From 05164c5a27eaad25f28a4fc8926297d6cd32e3fc Mon Sep 17 00:00:00 2001 From: Nathan Conrad Date: Thu, 19 Sep 2019 21:04:51 -0400 Subject: [PATCH 2/6] Cache pointer to HID descriptor. --- src/class/hid/hid_device.c | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-) diff --git a/src/class/hid/hid_device.c b/src/class/hid/hid_device.c index 2d579f475..7dabdbc17 100644 --- a/src/class/hid/hid_device.c +++ b/src/class/hid/hid_device.c @@ -46,12 +46,13 @@ typedef struct uint8_t boot_protocol; // Boot mouse or keyboard bool boot_mode; // default = false (Report) uint8_t idle_rate; // up to application to handle idle rate - uint16_t reprot_desc_len; + uint16_t report_desc_len; CFG_TUSB_MEM_ALIGN uint8_t epin_buf[CFG_TUD_HID_BUFSIZE]; CFG_TUSB_MEM_ALIGN uint8_t epout_buf[CFG_TUD_HID_BUFSIZE]; -}hidd_interface_t; + tusb_hid_descriptor_hid_t const * hid_descriptor; +} hidd_interface_t; CFG_TUSB_MEM_SECTION static hidd_interface_t _hidd_itf[CFG_TUD_HID]; @@ -167,8 +168,8 @@ bool hidd_open(uint8_t rhport, tusb_desc_interface_t const * desc_itf, uint16_t //------------- HID descriptor -------------// p_desc = tu_desc_next(p_desc); - tusb_hid_descriptor_hid_t const *desc_hid = (tusb_hid_descriptor_hid_t const *) p_desc; - TU_ASSERT(HID_DESC_TYPE_HID == desc_hid->bDescriptorType); + p_hid->hid_descriptor = (tusb_hid_descriptor_hid_t const *) p_desc; + TU_ASSERT(HID_DESC_TYPE_HID == p_hid->hid_descriptor->bDescriptorType); //------------- Endpoint Descriptor -------------// p_desc = tu_desc_next(p_desc); @@ -178,7 +179,7 @@ bool hidd_open(uint8_t rhport, tusb_desc_interface_t const * desc_itf, uint16_t p_hid->boot_mode = false; // default mode is REPORT p_hid->itf_num = desc_itf->bInterfaceNumber; - memcpy(&p_hid->reprot_desc_len, &desc_hid->wReportLength, 2); + memcpy(&p_hid->report_desc_len, &(p_hid->hid_descriptor->wReportLength), 2); *p_len = sizeof(tusb_desc_interface_t) + sizeof(tusb_hid_descriptor_hid_t) + desc_itf->bNumEndpoints*sizeof(tusb_desc_endpoint_t); @@ -205,6 +206,7 @@ bool hidd_control_request(uint8_t rhport, tusb_control_request_t const * p_reque if (p_request->bRequest == TUSB_REQ_GET_DESCRIPTOR && desc_type == HID_DESC_TYPE_HID) { // FIXME: Should check which is the active configuration, but no callback in usbd currently exists to do that + TU_VERIFY(p_hid->hid_descriptor != NULL); tusb_desc_configuration_t const* desc_cfg = (tusb_desc_configuration_t const*) tud_descriptor_configuration_cb(0); uint8_t const * p_desc = ((uint8_t const*) desc_cfg) + sizeof(tusb_desc_configuration_t); @@ -227,7 +229,7 @@ bool hidd_control_request(uint8_t rhport, tusb_control_request_t const * p_reque else if (p_request->bRequest == TUSB_REQ_GET_DESCRIPTOR && desc_type == HID_DESC_TYPE_REPORT) { uint8_t const * desc_report = tud_hid_descriptor_report_cb(); - tud_control_xfer(rhport, p_request, (void*) desc_report, p_hid->reprot_desc_len); + tud_control_xfer(rhport, p_request, (void*) desc_report, p_hid->report_desc_len); }else { return false; // stall unsupported request @@ -273,7 +275,7 @@ bool hidd_control_request(uint8_t rhport, tusb_control_request_t const * p_reque case HID_REQ_CONTROL_GET_PROTOCOL: { - uint8_t protocol = 1-p_hid->boot_mode; // 0 is Boot, 1 is Report protocol + uint8_t protocol = (uint8_t)(1-p_hid->boot_mode); // 0 is Boot, 1 is Report protocol tud_control_xfer(rhport, p_request, &protocol, 1); } break; From a8a65d6ceae244971405a13bc493ee923861be1f Mon Sep 17 00:00:00 2001 From: Nathan Conrad Date: Fri, 20 Sep 2019 08:46:17 -0400 Subject: [PATCH 3/6] Use cached HID descriptor. --- src/class/hid/hid_device.c | 23 +++-------------------- 1 file changed, 3 insertions(+), 20 deletions(-) diff --git a/src/class/hid/hid_device.c b/src/class/hid/hid_device.c index 7dabdbc17..fee44e30c 100644 --- a/src/class/hid/hid_device.c +++ b/src/class/hid/hid_device.c @@ -205,32 +205,15 @@ bool hidd_control_request(uint8_t rhport, tusb_control_request_t const * p_reque if (p_request->bRequest == TUSB_REQ_GET_DESCRIPTOR && desc_type == HID_DESC_TYPE_HID) { - // FIXME: Should check which is the active configuration, but no callback in usbd currently exists to do that TU_VERIFY(p_hid->hid_descriptor != NULL); - tusb_desc_configuration_t const* desc_cfg = - (tusb_desc_configuration_t const*) tud_descriptor_configuration_cb(0); - uint8_t const * p_desc = ((uint8_t const*) desc_cfg) + sizeof(tusb_desc_configuration_t); - uint8_t const * desc_end = ((uint8_t const*) desc_cfg) + desc_cfg->wTotalLength; - - while( p_desc < desc_end ) - { - tusb_hid_descriptor_hid_t *p_desc_hid =(tusb_hid_descriptor_hid_t*)p_desc; - if(p_desc_hid->bDescriptorType == HID_DESC_TYPE_HID) { - tud_control_xfer(rhport, p_request, (void*) p_desc_hid, p_desc_hid->bLength); - break; - } - p_desc += p_desc_hid->bLength; // next desc - } - if(p_desc >= desc_end) - { - return false; - } + TU_VERIFY(tud_control_xfer(rhport, p_request, (void*) p_hid->hid_descriptor, p_hid->hid_descriptor->bLength)); } else if (p_request->bRequest == TUSB_REQ_GET_DESCRIPTOR && desc_type == HID_DESC_TYPE_REPORT) { uint8_t const * desc_report = tud_hid_descriptor_report_cb(); tud_control_xfer(rhport, p_request, (void*) desc_report, p_hid->report_desc_len); - }else + } + else { return false; // stall unsupported request } From f241ff389f9494836cdc45e621726c0b69b8daed Mon Sep 17 00:00:00 2001 From: Nathan Conrad Date: Fri, 20 Sep 2019 08:56:46 -0400 Subject: [PATCH 4/6] Also need to just return false in the case that it isn't an interface control event. We shouldn't assert. This normally isn't an error, either, so I don't want to use TU_VERIFY. --- src/class/hid/hid_device.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/class/hid/hid_device.c b/src/class/hid/hid_device.c index fee44e30c..b57d6219f 100644 --- a/src/class/hid/hid_device.c +++ b/src/class/hid/hid_device.c @@ -193,6 +193,10 @@ bool hidd_open(uint8_t rhport, tusb_desc_interface_t const * desc_itf, uint16_t // return false to stall control endpoint (e.g unsupported request) bool hidd_control_request(uint8_t rhport, tusb_control_request_t const * p_request) { + if (p_request->bmRequestType_bit.recipient != TUSB_REQ_RCPT_INTERFACE) + { + return false; + } hidd_interface_t* p_hid = get_interface_by_itfnum( (uint8_t) p_request->wIndex ); TU_ASSERT(p_hid); From 2281a514846402af68aa4c97fcd206611c0969a1 Mon Sep 17 00:00:00 2001 From: Nathan Conrad Date: Fri, 20 Sep 2019 12:27:41 -0400 Subject: [PATCH 5/6] Revert "Also need to just return false in the case that it isn't an interface control event. We shouldn't assert. This normally isn't an" This reverts commit f241ff389f9494836cdc45e621726c0b69b8daed. --- src/class/hid/hid_device.c | 4 ---- 1 file changed, 4 deletions(-) diff --git a/src/class/hid/hid_device.c b/src/class/hid/hid_device.c index b57d6219f..fee44e30c 100644 --- a/src/class/hid/hid_device.c +++ b/src/class/hid/hid_device.c @@ -193,10 +193,6 @@ bool hidd_open(uint8_t rhport, tusb_desc_interface_t const * desc_itf, uint16_t // return false to stall control endpoint (e.g unsupported request) bool hidd_control_request(uint8_t rhport, tusb_control_request_t const * p_request) { - if (p_request->bmRequestType_bit.recipient != TUSB_REQ_RCPT_INTERFACE) - { - return false; - } hidd_interface_t* p_hid = get_interface_by_itfnum( (uint8_t) p_request->wIndex ); TU_ASSERT(p_hid); From 8a688cd8d0fed2325aaacdfbb5159a04dfe03ac1 Mon Sep 17 00:00:00 2001 From: Nathan Conrad Date: Fri, 20 Sep 2019 12:58:26 -0400 Subject: [PATCH 6/6] Revert "Revert "Also need to just return false in the case that it isn't an interface control event. We shouldn't assert. This normally isn't an"" This reverts commit 2281a514846402af68aa4c97fcd206611c0969a1. --- src/class/hid/hid_device.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/class/hid/hid_device.c b/src/class/hid/hid_device.c index fee44e30c..b57d6219f 100644 --- a/src/class/hid/hid_device.c +++ b/src/class/hid/hid_device.c @@ -193,6 +193,10 @@ bool hidd_open(uint8_t rhport, tusb_desc_interface_t const * desc_itf, uint16_t // return false to stall control endpoint (e.g unsupported request) bool hidd_control_request(uint8_t rhport, tusb_control_request_t const * p_request) { + if (p_request->bmRequestType_bit.recipient != TUSB_REQ_RCPT_INTERFACE) + { + return false; + } hidd_interface_t* p_hid = get_interface_by_itfnum( (uint8_t) p_request->wIndex ); TU_ASSERT(p_hid);