From ab96a726d68dda06bc2d41197729f8c744542ef9 Mon Sep 17 00:00:00 2001 From: Angel Cascarino Date: Mon, 20 Dec 2021 12:51:26 +0000 Subject: [PATCH 1/6] Altered format on test report_descriptor.h --- .../hid_report_descriptor.h | 83 +++++++++++-------- 1 file changed, 47 insertions(+), 36 deletions(-) diff --git a/legacy_tests/app_test_i2s_loopback/hid_report_descriptor.h b/legacy_tests/app_test_i2s_loopback/hid_report_descriptor.h index f133eb7e..6ba8ea01 100644 --- a/legacy_tests/app_test_i2s_loopback/hid_report_descriptor.h +++ b/legacy_tests/app_test_i2s_loopback/hid_report_descriptor.h @@ -5,6 +5,8 @@ #include "xua_hid_report.h" +#define USB_HID_REPORT_ID_CONSUMER ( 0x02 ) + #if 0 /* Existing static report descriptor kept for reference */ unsigned char hidReportDescriptor[] = @@ -29,8 +31,6 @@ unsigned char hidReportDescriptor[] = }; #endif -#define USB_HID_REPORT_ID_CONSUMER ( 0x02 ) - /* * Define non-configurable items in the HID Report descriptor. */ @@ -64,41 +64,56 @@ static const USB_HID_Short_Item_t hidReportCount2 = { static const USB_HID_Short_Item_t hidReportCount6 = { .header = HID_REPORT_SET_HEADER(1, HID_REPORT_ITEM_TYPE_GLOBAL, HID_REPORT_ITEM_TAG_REPORT_COUNT), .data = { 0x06, 0x00 } }; - static const USB_HID_Short_Item_t hidReportSize1 = { .header = HID_REPORT_SET_HEADER(1, HID_REPORT_ITEM_TYPE_GLOBAL, HID_REPORT_ITEM_TAG_REPORT_SIZE), .data = { 0x01, 0x00 } }; + static const USB_HID_Short_Item_t hidUsageConsumerControl = { .header = HID_REPORT_SET_HEADER(1, HID_REPORT_ITEM_TYPE_LOCAL, HID_REPORT_ITEM_TAG_USAGE), .data = { 0x01, 0x00 } }; -static const USB_HID_Short_Item_t hidUsagePageConsumer = { - .header = HID_REPORT_SET_HEADER(1, HID_REPORT_ITEM_TYPE_GLOBAL, HID_REPORT_ITEM_TAG_USAGE_PAGE), - .data = { 0x0C, 0x00 } }; + +/* + * Define the HID Report Descriptor Item, Usage Page, Report ID and length for each HID Report + * For internal purposes, a report element with ID of 0 must be included if report IDs are not being used. + */ +static const USB_HID_Report_Element_t hidReportPageConsumer = { + .item.header = HID_REPORT_SET_HEADER(1, HID_REPORT_ITEM_TYPE_GLOBAL, HID_REPORT_ITEM_TAG_USAGE_PAGE), + .item.data = { USB_HID_USAGE_PAGE_ID_CONSUMER, 0x00 }, + .location = HID_REPORT_SET_LOC( USB_HID_REPORT_ID_CONSUMER, 1, 0, 0 ) }; /* * Define configurable items in the HID Report descriptor. */ -static USB_HID_Report_Element_t hidUsageByte0Bit5 = { .item.header = HID_REPORT_SET_HEADER(1, HID_REPORT_ITEM_TYPE_LOCAL, HID_REPORT_ITEM_TAG_USAGE), .item.data = { 0xE2, 0x00 }, - .location = HID_REPORT_SET_LOC( USB_HID_REPORT_ID_CONSUMER, 0, 0, 5 ) }; // Mute -static USB_HID_Report_Element_t hidUsageByte0Bit4 = { .item.header = HID_REPORT_SET_HEADER(1, HID_REPORT_ITEM_TYPE_LOCAL, HID_REPORT_ITEM_TAG_USAGE), .item.data = { 0xEA, 0x00 }, - .location = HID_REPORT_SET_LOC( USB_HID_REPORT_ID_CONSUMER, 0, 0, 4 ) }; // Vol- -static USB_HID_Report_Element_t hidUsageByte0Bit3 = { .item.header = HID_REPORT_SET_HEADER(1, HID_REPORT_ITEM_TYPE_LOCAL, HID_REPORT_ITEM_TAG_USAGE), .item.data = { 0xE9, 0x00 }, - .location = HID_REPORT_SET_LOC( USB_HID_REPORT_ID_CONSUMER, 0, 0, 3 ) }; // Vol+ -static USB_HID_Report_Element_t hidUsageByte0Bit2 = { .item.header = HID_REPORT_SET_HEADER(1, HID_REPORT_ITEM_TYPE_LOCAL, HID_REPORT_ITEM_TAG_USAGE), .item.data = { 0xB6, 0x00 }, - .location = HID_REPORT_SET_LOC( USB_HID_REPORT_ID_CONSUMER, 0, 0, 2 ) }; // Scan Prev -static USB_HID_Report_Element_t hidUsageByte0Bit1 = { .item.header = HID_REPORT_SET_HEADER(1, HID_REPORT_ITEM_TYPE_LOCAL, HID_REPORT_ITEM_TAG_USAGE), .item.data = { 0xB5, 0x00 }, - .location = HID_REPORT_SET_LOC( USB_HID_REPORT_ID_CONSUMER, 0, 0, 1 ) }; // Scan Next -static USB_HID_Report_Element_t hidUsageByte0Bit0 = { .item.header = HID_REPORT_SET_HEADER(1, HID_REPORT_ITEM_TYPE_LOCAL, HID_REPORT_ITEM_TAG_USAGE), .item.data = { 0xB0, 0x00 }, - .location = HID_REPORT_SET_LOC( USB_HID_REPORT_ID_CONSUMER, 0, 0, 0 ) }; // Play - -static USB_HID_Short_Item_t hidReportID1 = { - .header = HID_REPORT_SET_HEADER(1, HID_REPORT_ITEM_TYPE_GLOBAL, HID_REPORT_ITEM_TAG_REPORT_ID), - .data = { USB_HID_USAGE_PAGE_ID_CONSUMER, 0x00} }; -static const USB_HID_Report_Element_t hidReportConsumer = { - .item.header = HID_REPORT_SET_HEADER(1, HID_REPORT_ITEM_TYPE_GLOBAL, HID_REPORT_ITEM_TAG_USAGE_PAGE), - .item.data = { USB_HID_USAGE_PAGE_ID_CONSUMER, 0x00 }, - .location = HID_REPORT_SET_LOC( USB_HID_REPORT_ID_CONSUMER, 1, 0, 0 ) -}; +static USB_HID_Report_Element_t hidUsageByte0Bit5 = { + .item.header = HID_REPORT_SET_HEADER(1, HID_REPORT_ITEM_TYPE_LOCAL, HID_REPORT_ITEM_TAG_USAGE), + .item.data = { 0xE2, 0x00 }, + .location = HID_REPORT_SET_LOC( USB_HID_REPORT_ID_CONSUMER, 0, 0, 5 ) +}; // Mute +static USB_HID_Report_Element_t hidUsageByte0Bit4 = { + .item.header = HID_REPORT_SET_HEADER(1, HID_REPORT_ITEM_TYPE_LOCAL, HID_REPORT_ITEM_TAG_USAGE), + .item.data = { 0xEA, 0x00 }, + .location = HID_REPORT_SET_LOC( USB_HID_REPORT_ID_CONSUMER, 0, 0, 4 ) +}; // Vol- +static USB_HID_Report_Element_t hidUsageByte0Bit3 = { + .item.header = HID_REPORT_SET_HEADER(1, HID_REPORT_ITEM_TYPE_LOCAL, HID_REPORT_ITEM_TAG_USAGE), + .item.data = { 0xE9, 0x00 }, + .location = HID_REPORT_SET_LOC( USB_HID_REPORT_ID_CONSUMER, 0, 0, 3 ) +}; // Vol+ +static USB_HID_Report_Element_t hidUsageByte0Bit2 = { + .item.header = HID_REPORT_SET_HEADER(1, HID_REPORT_ITEM_TYPE_LOCAL, HID_REPORT_ITEM_TAG_USAGE), + .item.data = { 0xB6, 0x00 }, + .location = HID_REPORT_SET_LOC( USB_HID_REPORT_ID_CONSUMER, 0, 0, 2 ) +}; // Scan Prev +static USB_HID_Report_Element_t hidUsageByte0Bit1 = { + .item.header = HID_REPORT_SET_HEADER(1, HID_REPORT_ITEM_TYPE_LOCAL, HID_REPORT_ITEM_TAG_USAGE), + .item.data = { 0xB5, 0x00 }, + .location = HID_REPORT_SET_LOC( USB_HID_REPORT_ID_CONSUMER, 0, 0, 1 ) +}; // Scan Next +static USB_HID_Report_Element_t hidUsageByte0Bit0 = { + .item.header = HID_REPORT_SET_HEADER(1, HID_REPORT_ITEM_TYPE_LOCAL, HID_REPORT_ITEM_TAG_USAGE), + .item.data = { 0xB0, 0x00 }, + .location = HID_REPORT_SET_LOC( USB_HID_REPORT_ID_CONSUMER, 0, 0, 0 ) +}; // Play /* * List the configurable items in the HID Report descriptor. @@ -113,18 +128,18 @@ static USB_HID_Report_Element_t* const hidConfigurableElements[] = { }; /* - * List Usage pages in the HID Report descriptor, one per byte. + * List HID Reports, one per Report ID. This should be a usage page item with the relevant + * If not using report IDs - still have one with report ID 0 */ -static const USB_HID_Short_Item_t * const hidUsagePages[] = { - &hidUsagePageConsumer +static const USB_HID_Report_Element_t* const hidReports[] = { + &hidReportPageConsumer }; /* * List all items in the HID Report descriptor. */ static const USB_HID_Short_Item_t * const hidReportDescriptorItems[] = { - &(hidReportConsumer.item), - &hidUsagePageConsumer, + &(hidReportPageConsumer.item), &hidUsageConsumerControl, &hidCollectionApplication, &hidLogicalMinimum0, @@ -144,10 +159,6 @@ static const USB_HID_Short_Item_t * const hidReportDescriptorItems[] = { &hidCollectionEnd }; -static const USB_HID_Report_Element_t* const hidReports[] = { - &hidReportConsumer -}; - /* * Define the number of HID Reports * Due to XC not supporting designated initializers, this constant has a hard-coded value. From 548992ab5c46b459e16ece1ef5fc9f4c4a48a17e Mon Sep 17 00:00:00 2001 From: Angel Cascarino Date: Mon, 20 Dec 2021 15:46:15 +0000 Subject: [PATCH 2/6] Removed USB_HID_REPORT_ID_CONSUMER as per review. --- .../hid_report_descriptor.h | 16 +++++++--------- 1 file changed, 7 insertions(+), 9 deletions(-) diff --git a/legacy_tests/app_test_i2s_loopback/hid_report_descriptor.h b/legacy_tests/app_test_i2s_loopback/hid_report_descriptor.h index 6ba8ea01..e0b185f8 100644 --- a/legacy_tests/app_test_i2s_loopback/hid_report_descriptor.h +++ b/legacy_tests/app_test_i2s_loopback/hid_report_descriptor.h @@ -5,8 +5,6 @@ #include "xua_hid_report.h" -#define USB_HID_REPORT_ID_CONSUMER ( 0x02 ) - #if 0 /* Existing static report descriptor kept for reference */ unsigned char hidReportDescriptor[] = @@ -79,7 +77,7 @@ static const USB_HID_Short_Item_t hidUsageConsumerControl = { static const USB_HID_Report_Element_t hidReportPageConsumer = { .item.header = HID_REPORT_SET_HEADER(1, HID_REPORT_ITEM_TYPE_GLOBAL, HID_REPORT_ITEM_TAG_USAGE_PAGE), .item.data = { USB_HID_USAGE_PAGE_ID_CONSUMER, 0x00 }, - .location = HID_REPORT_SET_LOC( USB_HID_REPORT_ID_CONSUMER, 1, 0, 0 ) }; + .location = HID_REPORT_SET_LOC( 0, 1, 0, 0 ) }; /* * Define configurable items in the HID Report descriptor. @@ -87,32 +85,32 @@ static const USB_HID_Report_Element_t hidReportPageConsumer = { static USB_HID_Report_Element_t hidUsageByte0Bit5 = { .item.header = HID_REPORT_SET_HEADER(1, HID_REPORT_ITEM_TYPE_LOCAL, HID_REPORT_ITEM_TAG_USAGE), .item.data = { 0xE2, 0x00 }, - .location = HID_REPORT_SET_LOC( USB_HID_REPORT_ID_CONSUMER, 0, 0, 5 ) + .location = HID_REPORT_SET_LOC( 0, 0, 0, 5 ) }; // Mute static USB_HID_Report_Element_t hidUsageByte0Bit4 = { .item.header = HID_REPORT_SET_HEADER(1, HID_REPORT_ITEM_TYPE_LOCAL, HID_REPORT_ITEM_TAG_USAGE), .item.data = { 0xEA, 0x00 }, - .location = HID_REPORT_SET_LOC( USB_HID_REPORT_ID_CONSUMER, 0, 0, 4 ) + .location = HID_REPORT_SET_LOC( 0, 0, 0, 4 ) }; // Vol- static USB_HID_Report_Element_t hidUsageByte0Bit3 = { .item.header = HID_REPORT_SET_HEADER(1, HID_REPORT_ITEM_TYPE_LOCAL, HID_REPORT_ITEM_TAG_USAGE), .item.data = { 0xE9, 0x00 }, - .location = HID_REPORT_SET_LOC( USB_HID_REPORT_ID_CONSUMER, 0, 0, 3 ) + .location = HID_REPORT_SET_LOC( 0, 0, 0, 3 ) }; // Vol+ static USB_HID_Report_Element_t hidUsageByte0Bit2 = { .item.header = HID_REPORT_SET_HEADER(1, HID_REPORT_ITEM_TYPE_LOCAL, HID_REPORT_ITEM_TAG_USAGE), .item.data = { 0xB6, 0x00 }, - .location = HID_REPORT_SET_LOC( USB_HID_REPORT_ID_CONSUMER, 0, 0, 2 ) + .location = HID_REPORT_SET_LOC( 0, 0, 0, 2 ) }; // Scan Prev static USB_HID_Report_Element_t hidUsageByte0Bit1 = { .item.header = HID_REPORT_SET_HEADER(1, HID_REPORT_ITEM_TYPE_LOCAL, HID_REPORT_ITEM_TAG_USAGE), .item.data = { 0xB5, 0x00 }, - .location = HID_REPORT_SET_LOC( USB_HID_REPORT_ID_CONSUMER, 0, 0, 1 ) + .location = HID_REPORT_SET_LOC( 0, 0, 0, 1 ) }; // Scan Next static USB_HID_Report_Element_t hidUsageByte0Bit0 = { .item.header = HID_REPORT_SET_HEADER(1, HID_REPORT_ITEM_TYPE_LOCAL, HID_REPORT_ITEM_TAG_USAGE), .item.data = { 0xB0, 0x00 }, - .location = HID_REPORT_SET_LOC( USB_HID_REPORT_ID_CONSUMER, 0, 0, 0 ) + .location = HID_REPORT_SET_LOC( 0, 0, 0, 0 ) }; // Play /* From 625123e4a63febe025c153d99a8d2dadf494395c Mon Sep 17 00:00:00 2001 From: Angel Cascarino Date: Mon, 20 Dec 2021 16:45:24 +0000 Subject: [PATCH 3/6] Clarified ep_buffer.xc setup stall Added getter hidIsReportDescriptorPrepared(), allowing more readable logic in the setup phase of ep_buffer.xc --- lib_xua/src/core/buffer/ep/ep_buffer.xc | 18 ++++++------------ lib_xua/src/hid/hid_report.c | 5 +++++ lib_xua/src/hid/xua_hid_report.h | 9 +++++++++ 3 files changed, 20 insertions(+), 12 deletions(-) diff --git a/lib_xua/src/core/buffer/ep/ep_buffer.xc b/lib_xua/src/core/buffer/ep/ep_buffer.xc index 0324bc0b..2eecb6b7 100644 --- a/lib_xua/src/core/buffer/ep/ep_buffer.xc +++ b/lib_xua/src/core/buffer/ep/ep_buffer.xc @@ -375,19 +375,13 @@ void XUA_Buffer_Ep(register chanend c_aud_out, #if( 0 < HID_CONTROLS ) UserHIDInit(); { - int hidReportLength = 0; - unsigned hidReportId; - while(0 == hidReportLength) { - for( hidReportId = 0U; hidReportId < hidGetReportIdLimit(); ++hidReportId) { - hidReportLength = (int) hidGetReportLength(hidReportId); - if(0 < hidReportLength) { - break; - } - } - } + /* Stall until Report Descriptor has been prepared */ + while (!hidIsReportDescriptorPrepared()); - hidReportLength = (int) UserHIDGetData(hidReportId, g_hidData); - XUD_SetReady_In(ep_hid, g_hidData, hidReportLength); + /* Get the last report - we don't really care which it is, so long as there's some data we can grab. */ + int hidReportLength = (int) UserHIDGetData(hidGetReportIdLimit() - 1, g_hidData); + + XUD_SetReady_In(ep_hid, g_hidData, hidReportLength) } #endif diff --git a/lib_xua/src/hid/hid_report.c b/lib_xua/src/hid/hid_report.c index ab35b63b..4f165656 100644 --- a/lib_xua/src/hid/hid_report.c +++ b/lib_xua/src/hid/hid_report.c @@ -213,6 +213,11 @@ unsigned hidGetNextReportTime( const unsigned id ) { return retVal; } +unsigned hidIsReportDescriptorPrepared( void ) +{ + return s_hidReportDescriptorPrepared; +} + unsigned char* hidGetReportDescriptor( void ) { unsigned char* retVal = NULL; diff --git a/lib_xua/src/hid/xua_hid_report.h b/lib_xua/src/hid/xua_hid_report.h index 21bc3aa9..1b2cb4e9 100644 --- a/lib_xua/src/hid/xua_hid_report.h +++ b/lib_xua/src/hid/xua_hid_report.h @@ -142,6 +142,15 @@ typedef struct */ void hidClearChangePending( const unsigned id ); +/** + * @brief Indicate if the HID Report descriptor has been prepared + * + * \returns A Boolean indicating whether the HID Report descriptor has been prepared. + * \retval True The HID Report descriptor has been prepared. + * \retval False The HID Report has not been prepared. + */ + unsigned hidIsReportDescriptorPrepared( void ); + /** * @brief Get the HID Report descriptor * From e71ffdba00af5b855e12f0a46411f2fca98e9f59 Mon Sep 17 00:00:00 2001 From: Angel Cascarino Date: Mon, 20 Dec 2021 17:10:48 +0000 Subject: [PATCH 4/6] Missed a semicolon... --- lib_xua/src/core/buffer/ep/ep_buffer.xc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib_xua/src/core/buffer/ep/ep_buffer.xc b/lib_xua/src/core/buffer/ep/ep_buffer.xc index 2eecb6b7..605ab119 100644 --- a/lib_xua/src/core/buffer/ep/ep_buffer.xc +++ b/lib_xua/src/core/buffer/ep/ep_buffer.xc @@ -381,7 +381,7 @@ void XUA_Buffer_Ep(register chanend c_aud_out, /* Get the last report - we don't really care which it is, so long as there's some data we can grab. */ int hidReportLength = (int) UserHIDGetData(hidGetReportIdLimit() - 1, g_hidData); - XUD_SetReady_In(ep_hid, g_hidData, hidReportLength) + XUD_SetReady_In(ep_hid, g_hidData, hidReportLength); } #endif From 244a7718a1715ff404cc8147c97051e7fee1fea1 Mon Sep 17 00:00:00 2001 From: Angel Cascarino Date: Mon, 20 Dec 2021 17:11:50 +0000 Subject: [PATCH 5/6] Corrected type in documentation --- lib_xua/src/hid/xua_hid_report.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib_xua/src/hid/xua_hid_report.h b/lib_xua/src/hid/xua_hid_report.h index 1b2cb4e9..5bc2cda9 100644 --- a/lib_xua/src/hid/xua_hid_report.h +++ b/lib_xua/src/hid/xua_hid_report.h @@ -147,7 +147,7 @@ void hidClearChangePending( const unsigned id ); * * \returns A Boolean indicating whether the HID Report descriptor has been prepared. * \retval True The HID Report descriptor has been prepared. - * \retval False The HID Report has not been prepared. + * \retval False The HID Report descriptor has not been prepared. */ unsigned hidIsReportDescriptorPrepared( void ); From 77398218b5e69b18e56cda37eefd3ec7868f5993 Mon Sep 17 00:00:00 2001 From: Angel Cascarino Date: Mon, 20 Dec 2021 17:17:34 +0000 Subject: [PATCH 6/6] Apply suggestions from code review Co-authored-by: Ciaran Woodward --- lib_xua/src/core/buffer/ep/ep_buffer.xc | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/lib_xua/src/core/buffer/ep/ep_buffer.xc b/lib_xua/src/core/buffer/ep/ep_buffer.xc index 605ab119..49b96ef5 100644 --- a/lib_xua/src/core/buffer/ep/ep_buffer.xc +++ b/lib_xua/src/core/buffer/ep/ep_buffer.xc @@ -375,10 +375,10 @@ void XUA_Buffer_Ep(register chanend c_aud_out, #if( 0 < HID_CONTROLS ) UserHIDInit(); { - /* Stall until Report Descriptor has been prepared */ - while (!hidIsReportDescriptorPrepared()); + while (!hidIsReportDescriptorPrepared()) + ; - /* Get the last report - we don't really care which it is, so long as there's some data we can grab. */ + /* Get data from the last report, which which we can prep XUD*/ int hidReportLength = (int) UserHIDGetData(hidGetReportIdLimit() - 1, g_hidData); XUD_SetReady_In(ep_hid, g_hidData, hidReportLength);