[PATCH v4 0/9] MR1448: hid: parser and HidP_SetUsageValue fix
This fixes two things: * HID report descriptor parser enum for local items. Close inspection of page 40 of [Device Class Definition for HID 1.11](https://www.usb.org/document-library/device-class-definition-hid-111) reveals a jump between Designator Maximum (0101) and String Index (0111). This caused a controller of mine to not get recognized. * HidP_SetUsageValue for items that define multiple controls (with Report Count > 1). Attempting to set one value would result in HIDP_STATUS_BUFFER_TOO_SMALL. -- v4: hid: Fix HidP_(Get|Set)(Scaled)UsageValue with usage ranges and arrays. hid: Move (get|set)_usage_value around and use it for scaled values. winexinput.sys: Fix incorrect HID STRING_INDEX tag value. winebus.sys: Fix incorrect HID STRING_INDEX tag value. ntoskrnl.exe/tests: Fix incorrect HID STRING_INDEX tag value. hidparse.sys: Fix incorrect HID STRING_INDEX tag value. dinput/tests: Fix incorrect HID STRING_INDEX tag value. dinput/tests: Add more HidP_(Set|Get)UsageValue tests with array caps. dinput/tests: Add more tests for HidP_SetUsageValue. https://gitlab.winehq.org/wine/wine/-/merge_requests/1448
From: Matthew Tran <0e4ef622(a)gmail.com> --- dlls/dinput/tests/hid.c | 102 +++++++++++++++++++++++++++++++++++----- 1 file changed, 91 insertions(+), 11 deletions(-) diff --git a/dlls/dinput/tests/hid.c b/dlls/dinput/tests/hid.c index 88565283735..5a8e1565ffd 100644 --- a/dlls/dinput/tests/hid.c +++ b/dlls/dinput/tests/hid.c @@ -1188,7 +1188,7 @@ static void test_hidp_set_feature( HANDLE file, int report_id, ULONG report_len, .report_buf = { report_id,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0, - 0,0,0,0,0,0xcd,0xcd,0xcd,0xcd,0xcd,0xcd,0xcd,0xcd,0xcd,0xcd,0xcd, + 0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0, 0xcd,0xcd,0xcd,0xcd,0xcd,0xcd,0xcd,0xcd,0xcd,0xcd,0xcd, }, .ret_length = 3, @@ -1226,7 +1226,8 @@ static void test_hidp_set_feature( HANDLE file, int report_id, ULONG report_len, { 0x5a,0x5a,0x5a,0x5a,0x5a,0x5a,0x5a,0x5a, 0x5a,0x5a,0x5a,0x5a,0x5a,0x5a,0x5a,0x5a, - 0x5a,0x5a,0x5a,0x5a,0x5a, + 0x5a,0x5a,0x5a,0x5a,0x5a,0x5a,0x5a,0x5a, + 0x5a,0x5a,0x5a,0x5a,0x5a,0x5a,0x5a, }, .ret_length = 3, .ret_status = STATUS_SUCCESS, @@ -1558,8 +1559,8 @@ static void test_hidp( HANDLE file, HANDLE async_file, int report_id, BOOL polle .LinkUsage = HID_USAGE_GENERIC_JOYSTICK, .LinkUsagePage = HID_USAGE_PAGE_GENERIC, .CollectionType = 1, - .NumberOfChildren = 7, - .FirstChild = 9, + .NumberOfChildren = 8, + .FirstChild = 10, }, { .LinkUsage = HID_USAGE_GENERIC_JOYSTICK, @@ -1582,6 +1583,7 @@ static void test_hidp( HANDLE file, HANDLE async_file, int report_id, BOOL polle { .DataIndex = 39, .RawValue = 1, }, }; + DWORD waveform_list, collection_count, generic_output_list; OVERLAPPED overlapped = {0}, overlapped2 = {0}; HIDP_LINK_COLLECTION_NODE collections[16]; PHIDP_PREPARSED_DATA preparsed_data; @@ -1589,8 +1591,6 @@ static void test_hidp( HANDLE file, HANDLE async_file, int report_id, BOOL polle HIDP_BUTTON_CAPS button_caps[32]; HIDP_VALUE_CAPS value_caps[16]; char buffer[200], report[200]; - DWORD collection_count; - DWORD waveform_list; HIDP_DATA data[64]; USAGE usages[16]; ULONG off, value; @@ -2038,7 +2038,7 @@ static void test_hidp( HANDLE file, HANDLE async_file, int report_id, BOOL polle value = HidP_MaxDataListLength( HidP_Output, preparsed_data ); ok( value == 0, "HidP_MaxDataListLength(HidP_Output) returned %ld, expected %d\n", value, 0 ); value = HidP_MaxDataListLength( HidP_Feature, preparsed_data ); - ok( value == 14, "HidP_MaxDataListLength(HidP_Feature) returned %ld, expected %d\n", value, 14 ); + ok( value == 24, "HidP_MaxDataListLength(HidP_Feature) returned %ld, expected %d\n", value, 24 ); value = 1; status = HidP_GetData( HidP_Input, data, &value, preparsed_data, report, caps.InputReportByteLength ); @@ -2271,6 +2271,66 @@ static void test_hidp( HANDLE file, HANDLE async_file, int report_id, BOOL polle ok( status == HIDP_STATUS_SUCCESS, "HidP_GetUsageValue returned %#lx\n", status ); ok( value == 0xfffffd0b, "got value %#lx, expected %#x\n", value, 0xfffffd0b ); + memset( report, 0xcd, sizeof(report) ); + status = HidP_InitializeReportForID( HidP_Feature, report_id, preparsed_data, report, + caps.FeatureReportByteLength ); + ok( status == HIDP_STATUS_SUCCESS, "HidP_InitializeReportForID returned %#lx\n", status ); + + memset( buffer, 0xcd, sizeof(buffer) ); + memset( buffer, 0, caps.FeatureReportByteLength ); + buffer[0] = report_id; + ok( !memcmp( buffer, report, sizeof(buffer) ), "unexpected report data\n" ); + + for (i = 0; i < caps.NumberLinkCollectionNodes; ++i) + { + if (collections[i].LinkUsagePage != HID_USAGE_PAGE_GENERIC) continue; + if (collections[i].LinkUsage == HID_USAGE_GENERIC_SYSTEM_CTL) break; + } + ok( i < caps.NumberLinkCollectionNodes, "HID_USAGE_GENERIC_SYSTEM_CTL collection not found\n" ); + generic_output_list = i; + + for (i = 0; i < 5; ++i) + { + status = HidP_SetUsageValue( HidP_Feature, HID_USAGE_PAGE_ORDINAL, generic_output_list, i + 1, + i, preparsed_data, report, caps.FeatureReportByteLength ); + todo_wine + ok( status == HIDP_STATUS_SUCCESS, "HidP_SetUsageValue returned %#lx\n", status ); + + status = HidP_GetUsageValue( HidP_Feature, HID_USAGE_PAGE_ORDINAL, generic_output_list, i + 1, + &value, preparsed_data, report, caps.FeatureReportByteLength ); + todo_wine + ok( status == HIDP_STATUS_SUCCESS, "HidP_SetUsageValue returned %#lx\n", status ); + todo_wine + ok( value == i, "got value %#lx, expected %#x\n", value, i ); + + status = HidP_SetUsageValueArray( HidP_Feature, HID_USAGE_PAGE_ORDINAL, generic_output_list, i + 1, (void *)&value, + sizeof(value), preparsed_data, report, caps.FeatureReportByteLength ); + ok( status == HIDP_STATUS_NOT_VALUE_ARRAY, "HidP_SetUsageValue returned %#lx\n", status ); + status = HidP_GetUsageValueArray( HidP_Feature, HID_USAGE_PAGE_ORDINAL, generic_output_list, i + 1, (void *)&value, + sizeof(value), preparsed_data, report, caps.FeatureReportByteLength ); + ok( status == HIDP_STATUS_NOT_VALUE_ARRAY, "HidP_SetUsageValue returned %#lx\n", status ); + + buffer[i + 21] = i; + } + for (i = 5; i < 10; ++i) + { + status = HidP_SetScaledUsageValue( HidP_Feature, HID_USAGE_PAGE_ORDINAL, generic_output_list, i + 1, + i * 16, preparsed_data, report, caps.FeatureReportByteLength ); + todo_wine + ok( status == HIDP_STATUS_SUCCESS, "HidP_SetScaledUsageValue returned %#lx\n", status ); + + status = HidP_GetScaledUsageValue( HidP_Feature, HID_USAGE_PAGE_ORDINAL, generic_output_list, i + 1, + (LONG *)&value, preparsed_data, report, caps.FeatureReportByteLength ); + todo_wine + ok( status == HIDP_STATUS_SUCCESS, "HidP_GetScaledUsageValue returned %#lx\n", status ); + todo_wine + ok( value == i * 16, "got value %#lx, expected %#x\n", value, i * 16 ); + + buffer[i + 21] = i; + } + todo_wine + ok( !memcmp( buffer, report, sizeof(buffer) ), "unexpected report data\n" ); + test_hidp_get_input( file, report_id, caps.InputReportByteLength, preparsed_data ); test_hidp_get_feature( file, report_id, caps.FeatureReportByteLength, preparsed_data ); test_hidp_set_feature( file, report_id, caps.FeatureReportByteLength, preparsed_data ); @@ -2864,6 +2924,26 @@ static void test_hid_driver( DWORD report_id, DWORD polled ) FEATURE(1, Data|Var|Abs), END_COLLECTION, + USAGE_PAGE(1, HID_USAGE_PAGE_GENERIC), + USAGE(1, HID_USAGE_GENERIC_SYSTEM_CTL), + COLLECTION(1, Report), + REPORT_ID_OR_USAGE_PAGE(1, report_id, 0), + REPORT_COUNT(1, 10), + REPORT_SIZE(1, 8), + LOGICAL_MINIMUM(2, 0x0000), + LOGICAL_MAXIMUM(2, 0x00ff), + PHYSICAL_MINIMUM(2, 0x0000), + PHYSICAL_MAXIMUM(2, 0x0ff0), + USAGE_PAGE(1, HID_USAGE_PAGE_ORDINAL), + STRING_MINIMUM(1, 6), + STRING_MAXIMUM(1, 16), + USAGE_MINIMUM(1, 1), /* Instance 1 */ + USAGE_MAXIMUM(1, 10), /* Instance 10 */ + FEATURE(1, Data|Var|Abs), + PHYSICAL_MINIMUM(1, 0), + PHYSICAL_MAXIMUM(1, 0), + END_COLLECTION, + USAGE_PAGE(1, HID_USAGE_PAGE_GENERIC), USAGE(1, HID_USAGE_GENERIC_JOYSTICK), COLLECTION(1, Report), @@ -2911,14 +2991,14 @@ static void test_hid_driver( DWORD report_id, DWORD polled ) .UsagePage = HID_USAGE_PAGE_GENERIC, .InputReportByteLength = report_id ? 32 : 33, .OutputReportByteLength = report_id ? 2 : 3, - .FeatureReportByteLength = report_id ? 21 : 22, - .NumberLinkCollectionNodes = 10, + .FeatureReportByteLength = report_id ? 31 : 32, + .NumberLinkCollectionNodes = 11, .NumberInputButtonCaps = 17, .NumberInputValueCaps = 7, .NumberInputDataIndices = 47, .NumberFeatureButtonCaps = 1, - .NumberFeatureValueCaps = 6, - .NumberFeatureDataIndices = 8, + .NumberFeatureValueCaps = 7, + .NumberFeatureDataIndices = 18, }; const struct hid_expect expect_in = { -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/1448
From: Rémi Bernon <rbernon(a)codeweavers.com> --- dlls/dinput/tests/hid.c | 38 +++++++++++++++++++++++++++++++------- 1 file changed, 31 insertions(+), 7 deletions(-) diff --git a/dlls/dinput/tests/hid.c b/dlls/dinput/tests/hid.c index 5a8e1565ffd..b9f0baf510a 100644 --- a/dlls/dinput/tests/hid.c +++ b/dlls/dinput/tests/hid.c @@ -1543,7 +1543,7 @@ static void test_hidp( HANDLE file, HANDLE async_file, int report_id, BOOL polle .LinkUsagePage = HID_USAGE_PAGE_GENERIC, .LinkCollection = 1, .IsAbsolute = TRUE, - .BitSize = 4, + .BitSize = 24, .ReportCount = 2, .LogicalMin = 1, .LogicalMax = 8, @@ -1792,26 +1792,50 @@ static void test_hidp( HANDLE file, HANDLE async_file, int report_id, BOOL polle status = HidP_SetUsageValueArray( HidP_Input, HID_USAGE_PAGE_GENERIC, 0, HID_USAGE_GENERIC_X, buffer, sizeof(buffer), preparsed_data, report, caps.InputReportByteLength ); ok( status == HIDP_STATUS_NOT_VALUE_ARRAY, "HidP_SetUsageValueArray returned %#lx\n", status ); - memset( buffer, 0xcd, sizeof(buffer) ); + memset( buffer, 0xa5, sizeof(buffer) ); status = HidP_SetUsageValueArray( HidP_Input, HID_USAGE_PAGE_GENERIC, 0, HID_USAGE_GENERIC_HATSWITCH, buffer, 0, preparsed_data, report, caps.InputReportByteLength ); ok( status == HIDP_STATUS_BUFFER_TOO_SMALL, "HidP_SetUsageValueArray returned %#lx\n", status ); status = HidP_SetUsageValueArray( HidP_Input, HID_USAGE_PAGE_GENERIC, 0, HID_USAGE_GENERIC_HATSWITCH, buffer, 8, preparsed_data, report, caps.InputReportByteLength ); + ok( status == HIDP_STATUS_SUCCESS, "HidP_SetUsageValueArray returned %#lx\n", status ); + ok( report[16] == (char)0xa5, "got report value %d\n", report[16] ); + ok( report[17] == (char)0xa5, "got report value %d\n", report[17] ); + + report[16] = 0xa5; + report[17] = 0xa5; + status = HidP_SetUsageValue( HidP_Input, HID_USAGE_PAGE_GENERIC, 0, HID_USAGE_GENERIC_HATSWITCH, + 8, preparsed_data, report, caps.InputReportByteLength ); + todo_wine + ok( status == HIDP_STATUS_SUCCESS, "HidP_SetUsageValue returned %#lx\n", status ); + todo_wine + ok( report[16] == (char)8, "got value %#x\n", report[16] ); todo_wine - ok( status == HIDP_STATUS_NOT_IMPLEMENTED, "HidP_SetUsageValueArray returned %#lx\n", status ); + ok( report[17] == (char)0, "got value %#x\n", report[17] ); status = HidP_GetUsageValueArray( HidP_Input, HID_USAGE_PAGE_GENERIC, 0, HID_USAGE_GENERIC_X, buffer, sizeof(buffer), preparsed_data, report, caps.InputReportByteLength ); ok( status == HIDP_STATUS_NOT_VALUE_ARRAY, "HidP_GetUsageValueArray returned %#lx\n", status ); - memset( buffer, 0xcd, sizeof(buffer) ); + memset( buffer, 0xa5, sizeof(buffer) ); status = HidP_GetUsageValueArray( HidP_Input, HID_USAGE_PAGE_GENERIC, 0, HID_USAGE_GENERIC_HATSWITCH, buffer, 0, preparsed_data, report, caps.InputReportByteLength ); ok( status == HIDP_STATUS_BUFFER_TOO_SMALL, "HidP_GetUsageValueArray returned %#lx\n", status ); + report[16] = 0xcd; + report[17] = 0xcd; status = HidP_GetUsageValueArray( HidP_Input, HID_USAGE_PAGE_GENERIC, 0, HID_USAGE_GENERIC_HATSWITCH, buffer, 8, preparsed_data, report, caps.InputReportByteLength ); + ok( status == HIDP_STATUS_SUCCESS, "HidP_GetUsageValueArray returned %#lx\n", status ); + ok( buffer[0] == (char)0xcd, "got report value %#x\n", buffer[0] ); + ok( buffer[1] == (char)0xcd, "got report value %#x\n", buffer[1] ); + + report[16] = 0xff; + report[17] = 0xff; + status = HidP_GetUsageValue( HidP_Input, HID_USAGE_PAGE_GENERIC, 0, HID_USAGE_GENERIC_HATSWITCH, + &value, preparsed_data, report, caps.InputReportByteLength ); + todo_wine + ok( status == HIDP_STATUS_SUCCESS, "HidP_GetUsageValue returned %#lx\n", status ); todo_wine - ok( status == HIDP_STATUS_NOT_IMPLEMENTED, "HidP_GetUsageValueArray returned %#lx\n", status ); + ok( value == 0xffff, "got value %ld\n", value ); value = -128; status = HidP_SetUsageValue( HidP_Input, HID_USAGE_PAGE_GENERIC, 0, HID_USAGE_GENERIC_X, value, @@ -2787,7 +2811,7 @@ static void test_hid_driver( DWORD report_id, DWORD polled ) USAGE(1, HID_USAGE_GENERIC_HATSWITCH), LOGICAL_MINIMUM(1, 1), LOGICAL_MAXIMUM(1, 8), - REPORT_SIZE(1, 4), + REPORT_SIZE(1, 24), REPORT_COUNT(1, 2), INPUT(1, Data|Var|Abs), @@ -2989,7 +3013,7 @@ static void test_hid_driver( DWORD report_id, DWORD polled ) { .Usage = HID_USAGE_GENERIC_JOYSTICK, .UsagePage = HID_USAGE_PAGE_GENERIC, - .InputReportByteLength = report_id ? 32 : 33, + .InputReportByteLength = report_id ? 37 : 38, .OutputReportByteLength = report_id ? 2 : 3, .FeatureReportByteLength = report_id ? 31 : 32, .NumberLinkCollectionNodes = 11, -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/1448
From: Matthew Tran <0e4ef622(a)gmail.com> --- dlls/dinput/tests/psh_hid_macros.h | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/dlls/dinput/tests/psh_hid_macros.h b/dlls/dinput/tests/psh_hid_macros.h index 4623af20598..8e0a2a9dd6b 100644 --- a/dlls/dinput/tests/psh_hid_macros.h +++ b/dlls/dinput/tests/psh_hid_macros.h @@ -79,7 +79,7 @@ #define DESIGNATOR_INDEX(n,data) SHORT_ITEM_##n(0x3,2,data) #define DESIGNATOR_MINIMUM(n,data) SHORT_ITEM_##n(0x4,2,data) #define DESIGNATOR_MAXIMUM(n,data) SHORT_ITEM_##n(0x5,2,data) -#define STRING_INDEX(n,data) SHORT_ITEM_##n(0x6,2,data) -#define STRING_MINIMUM(n,data) SHORT_ITEM_##n(0x7,2,data) -#define STRING_MAXIMUM(n,data) SHORT_ITEM_##n(0x8,2,data) -#define DELIMITER(n,data) SHORT_ITEM_##n(0x9,2,data) +#define STRING_INDEX(n,data) SHORT_ITEM_##n(0x7,2,data) +#define STRING_MINIMUM(n,data) SHORT_ITEM_##n(0x8,2,data) +#define STRING_MAXIMUM(n,data) SHORT_ITEM_##n(0x9,2,data) +#define DELIMITER(n,data) SHORT_ITEM_##n(0xa,2,data) -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/1448
From: Matthew Tran <0e4ef622(a)gmail.com> --- dlls/hidparse.sys/main.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/dlls/hidparse.sys/main.c b/dlls/hidparse.sys/main.c index 9c753c0f365..7058093e03c 100644 --- a/dlls/hidparse.sys/main.c +++ b/dlls/hidparse.sys/main.c @@ -93,7 +93,7 @@ enum TAG_LOCAL_DESIGNATOR_INDEX, TAG_LOCAL_DESIGNATOR_MINIMUM, TAG_LOCAL_DESIGNATOR_MAXIMUM, - TAG_LOCAL_STRING_INDEX, + TAG_LOCAL_STRING_INDEX = 0x7, TAG_LOCAL_STRING_MINIMUM, TAG_LOCAL_STRING_MAXIMUM, TAG_LOCAL_DELIMITER -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/1448
From: Matthew Tran <0e4ef622(a)gmail.com> --- dlls/ntoskrnl.exe/tests/psh_hid_macros.h | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/dlls/ntoskrnl.exe/tests/psh_hid_macros.h b/dlls/ntoskrnl.exe/tests/psh_hid_macros.h index 4623af20598..8e0a2a9dd6b 100644 --- a/dlls/ntoskrnl.exe/tests/psh_hid_macros.h +++ b/dlls/ntoskrnl.exe/tests/psh_hid_macros.h @@ -79,7 +79,7 @@ #define DESIGNATOR_INDEX(n,data) SHORT_ITEM_##n(0x3,2,data) #define DESIGNATOR_MINIMUM(n,data) SHORT_ITEM_##n(0x4,2,data) #define DESIGNATOR_MAXIMUM(n,data) SHORT_ITEM_##n(0x5,2,data) -#define STRING_INDEX(n,data) SHORT_ITEM_##n(0x6,2,data) -#define STRING_MINIMUM(n,data) SHORT_ITEM_##n(0x7,2,data) -#define STRING_MAXIMUM(n,data) SHORT_ITEM_##n(0x8,2,data) -#define DELIMITER(n,data) SHORT_ITEM_##n(0x9,2,data) +#define STRING_INDEX(n,data) SHORT_ITEM_##n(0x7,2,data) +#define STRING_MINIMUM(n,data) SHORT_ITEM_##n(0x8,2,data) +#define STRING_MAXIMUM(n,data) SHORT_ITEM_##n(0x9,2,data) +#define DELIMITER(n,data) SHORT_ITEM_##n(0xa,2,data) -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/1448
From: Matthew Tran <0e4ef622(a)gmail.com> --- dlls/winebus.sys/psh_hid_macros.h | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/dlls/winebus.sys/psh_hid_macros.h b/dlls/winebus.sys/psh_hid_macros.h index 4623af20598..8e0a2a9dd6b 100644 --- a/dlls/winebus.sys/psh_hid_macros.h +++ b/dlls/winebus.sys/psh_hid_macros.h @@ -79,7 +79,7 @@ #define DESIGNATOR_INDEX(n,data) SHORT_ITEM_##n(0x3,2,data) #define DESIGNATOR_MINIMUM(n,data) SHORT_ITEM_##n(0x4,2,data) #define DESIGNATOR_MAXIMUM(n,data) SHORT_ITEM_##n(0x5,2,data) -#define STRING_INDEX(n,data) SHORT_ITEM_##n(0x6,2,data) -#define STRING_MINIMUM(n,data) SHORT_ITEM_##n(0x7,2,data) -#define STRING_MAXIMUM(n,data) SHORT_ITEM_##n(0x8,2,data) -#define DELIMITER(n,data) SHORT_ITEM_##n(0x9,2,data) +#define STRING_INDEX(n,data) SHORT_ITEM_##n(0x7,2,data) +#define STRING_MINIMUM(n,data) SHORT_ITEM_##n(0x8,2,data) +#define STRING_MAXIMUM(n,data) SHORT_ITEM_##n(0x9,2,data) +#define DELIMITER(n,data) SHORT_ITEM_##n(0xa,2,data) -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/1448
From: Matthew Tran <0e4ef622(a)gmail.com> --- dlls/winexinput.sys/psh_hid_macros.h | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/dlls/winexinput.sys/psh_hid_macros.h b/dlls/winexinput.sys/psh_hid_macros.h index 4623af20598..8e0a2a9dd6b 100644 --- a/dlls/winexinput.sys/psh_hid_macros.h +++ b/dlls/winexinput.sys/psh_hid_macros.h @@ -79,7 +79,7 @@ #define DESIGNATOR_INDEX(n,data) SHORT_ITEM_##n(0x3,2,data) #define DESIGNATOR_MINIMUM(n,data) SHORT_ITEM_##n(0x4,2,data) #define DESIGNATOR_MAXIMUM(n,data) SHORT_ITEM_##n(0x5,2,data) -#define STRING_INDEX(n,data) SHORT_ITEM_##n(0x6,2,data) -#define STRING_MINIMUM(n,data) SHORT_ITEM_##n(0x7,2,data) -#define STRING_MAXIMUM(n,data) SHORT_ITEM_##n(0x8,2,data) -#define DELIMITER(n,data) SHORT_ITEM_##n(0x9,2,data) +#define STRING_INDEX(n,data) SHORT_ITEM_##n(0x7,2,data) +#define STRING_MINIMUM(n,data) SHORT_ITEM_##n(0x8,2,data) +#define STRING_MAXIMUM(n,data) SHORT_ITEM_##n(0x9,2,data) +#define DELIMITER(n,data) SHORT_ITEM_##n(0xa,2,data) -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/1448
From: Rémi Bernon <rbernon(a)codeweavers.com> --- dlls/hid/hidp.c | 80 +++++++++++++++++++++++-------------------------- 1 file changed, 38 insertions(+), 42 deletions(-) diff --git a/dlls/hid/hidp.c b/dlls/hid/hidp.c index a1c5805c19f..bf5a6259484 100644 --- a/dlls/hid/hidp.c +++ b/dlls/hid/hidp.c @@ -236,18 +236,33 @@ static LONG sign_extend( ULONG value, const struct hid_value_caps *caps ) return value - ((value & sign) << 1); } -static NTSTATUS get_scaled_usage_value( const struct hid_value_caps *caps, void *user ) +static NTSTATUS get_usage_value( const struct hid_value_caps *caps, void *user ) { struct usage_value_params *params = user; - ULONG unsigned_value = 0, bit_count = caps->bit_size * caps->report_count; - LONG signed_value, *value = params->value_buf; + ULONG bit_count = caps->bit_size * caps->report_count; unsigned char *report_buf; - if ((bit_count + 7) / 8 > sizeof(unsigned_value)) return HIDP_STATUS_BUFFER_TOO_SMALL; - if (sizeof(LONG) > params->value_len) return HIDP_STATUS_BUFFER_TOO_SMALL; + if ((bit_count + 7) / 8 > params->value_len) return HIDP_STATUS_BUFFER_TOO_SMALL; + memset( params->value_buf, 0, params->value_len ); report_buf = (unsigned char *)params->report_buf + caps->start_byte; - copy_bits( (unsigned char *)&unsigned_value, report_buf, bit_count, -caps->start_bit ); + copy_bits( params->value_buf, report_buf, bit_count, -caps->start_bit ); + + return HIDP_STATUS_NULL; +} + +static NTSTATUS get_scaled_usage_value( const struct hid_value_caps *caps, void *user ) +{ + struct usage_value_params *params = user; + LONG signed_value, *value = params->value_buf; + ULONG unsigned_value = 0; + NTSTATUS status; + + params->value_buf = &unsigned_value; + params->value_len = sizeof(unsigned_value); + if ((status = get_usage_value( caps, params )) != HIDP_STATUS_NULL) return status; + + if (sizeof(LONG) > params->value_len) return HIDP_STATUS_BUFFER_TOO_SMALL; signed_value = sign_extend( unsigned_value, caps ); if (caps->logical_min > caps->logical_max || caps->physical_min > caps->physical_max) @@ -280,21 +295,6 @@ NTSTATUS WINAPI HidP_GetScaledUsageValue( HIDP_REPORT_TYPE report_type, USAGE us return enum_value_caps( preparsed, report_type, report_len, &filter, get_scaled_usage_value, ¶ms, &count ); } -static NTSTATUS get_usage_value( const struct hid_value_caps *caps, void *user ) -{ - struct usage_value_params *params = user; - ULONG bit_count = caps->bit_size * caps->report_count; - unsigned char *report_buf; - - if ((bit_count + 7) / 8 > params->value_len) return HIDP_STATUS_BUFFER_TOO_SMALL; - memset( params->value_buf, 0, params->value_len ); - - report_buf = (unsigned char *)params->report_buf + caps->start_byte; - copy_bits( params->value_buf, report_buf, bit_count, -caps->start_bit ); - - return HIDP_STATUS_NULL; -} - NTSTATUS WINAPI HidP_GetUsageValue( HIDP_REPORT_TYPE report_type, USAGE usage_page, USHORT collection, USAGE usage, ULONG *value, PHIDP_PREPARSED_DATA preparsed_data, char *report_buf, ULONG report_len ) { @@ -446,17 +446,28 @@ ULONG WINAPI HidP_MaxUsageListLength( HIDP_REPORT_TYPE report_type, USAGE usage_ return count; } -static NTSTATUS set_scaled_usage_value( const struct hid_value_caps *caps, void *user ) +static NTSTATUS set_usage_value( const struct hid_value_caps *caps, void *user ) { - ULONG bit_count = caps->bit_size * caps->report_count; struct usage_value_params *params = user; + ULONG bit_count = caps->bit_size * caps->report_count; unsigned char *report_buf; + + if ((bit_count + 7) / 8 > params->value_len) return HIDP_STATUS_BUFFER_TOO_SMALL; + + report_buf = (unsigned char *)params->report_buf + caps->start_byte; + copy_bits( report_buf, params->value_buf, bit_count, caps->start_bit ); + + return HIDP_STATUS_NULL; +} + +static NTSTATUS set_scaled_usage_value( const struct hid_value_caps *caps, void *user ) +{ + struct usage_value_params *params = user; LONG value, log_range, phy_range; if (caps->logical_min > caps->logical_max) return HIDP_STATUS_BAD_LOG_PHY_VALUES; if (caps->physical_min > caps->physical_max) return HIDP_STATUS_BAD_LOG_PHY_VALUES; - if ((bit_count + 7) / 8 > sizeof(value)) return HIDP_STATUS_BUFFER_TOO_SMALL; if (sizeof(LONG) > params->value_len) return HIDP_STATUS_BUFFER_TOO_SMALL; value = *(LONG *)params->value_buf; @@ -471,10 +482,9 @@ static NTSTATUS set_scaled_usage_value( const struct hid_value_caps *caps, void value = caps->logical_min + value; } - report_buf = (unsigned char *)params->report_buf + caps->start_byte; - copy_bits( report_buf, (unsigned char *)&value, bit_count, caps->start_bit ); - - return HIDP_STATUS_NULL; + params->value_buf = &value; + params->value_len = sizeof(value); + return set_usage_value( caps, params ); } NTSTATUS WINAPI HidP_SetScaledUsageValue( HIDP_REPORT_TYPE report_type, USAGE usage_page, USHORT collection, @@ -495,20 +505,6 @@ NTSTATUS WINAPI HidP_SetScaledUsageValue( HIDP_REPORT_TYPE report_type, USAGE us return enum_value_caps( preparsed, report_type, report_len, &filter, set_scaled_usage_value, ¶ms, &count ); } -static NTSTATUS set_usage_value( const struct hid_value_caps *caps, void *user ) -{ - struct usage_value_params *params = user; - ULONG bit_count = caps->bit_size * caps->report_count; - unsigned char *report_buf; - - if ((bit_count + 7) / 8 > params->value_len) return HIDP_STATUS_BUFFER_TOO_SMALL; - - report_buf = (unsigned char *)params->report_buf + caps->start_byte; - copy_bits( report_buf, params->value_buf, bit_count, caps->start_bit ); - - return HIDP_STATUS_NULL; -} - NTSTATUS WINAPI HidP_SetUsageValue( HIDP_REPORT_TYPE report_type, USAGE usage_page, USHORT collection, USAGE usage, ULONG value, PHIDP_PREPARSED_DATA preparsed_data, char *report_buf, ULONG report_len ) { -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/1448
From: Matthew Tran <0e4ef622(a)gmail.com> --- dlls/dinput/tests/hid.c | 12 ------------ dlls/hid/hidp.c | 36 ++++++++++++++++++++++-------------- 2 files changed, 22 insertions(+), 26 deletions(-) diff --git a/dlls/dinput/tests/hid.c b/dlls/dinput/tests/hid.c index b9f0baf510a..667e335c631 100644 --- a/dlls/dinput/tests/hid.c +++ b/dlls/dinput/tests/hid.c @@ -1806,11 +1806,8 @@ static void test_hidp( HANDLE file, HANDLE async_file, int report_id, BOOL polle report[17] = 0xa5; status = HidP_SetUsageValue( HidP_Input, HID_USAGE_PAGE_GENERIC, 0, HID_USAGE_GENERIC_HATSWITCH, 8, preparsed_data, report, caps.InputReportByteLength ); - todo_wine ok( status == HIDP_STATUS_SUCCESS, "HidP_SetUsageValue returned %#lx\n", status ); - todo_wine ok( report[16] == (char)8, "got value %#x\n", report[16] ); - todo_wine ok( report[17] == (char)0, "got value %#x\n", report[17] ); status = HidP_GetUsageValueArray( HidP_Input, HID_USAGE_PAGE_GENERIC, 0, HID_USAGE_GENERIC_X, buffer, @@ -1832,9 +1829,7 @@ static void test_hidp( HANDLE file, HANDLE async_file, int report_id, BOOL polle report[17] = 0xff; status = HidP_GetUsageValue( HidP_Input, HID_USAGE_PAGE_GENERIC, 0, HID_USAGE_GENERIC_HATSWITCH, &value, preparsed_data, report, caps.InputReportByteLength ); - todo_wine ok( status == HIDP_STATUS_SUCCESS, "HidP_GetUsageValue returned %#lx\n", status ); - todo_wine ok( value == 0xffff, "got value %ld\n", value ); value = -128; @@ -2317,14 +2312,11 @@ static void test_hidp( HANDLE file, HANDLE async_file, int report_id, BOOL polle { status = HidP_SetUsageValue( HidP_Feature, HID_USAGE_PAGE_ORDINAL, generic_output_list, i + 1, i, preparsed_data, report, caps.FeatureReportByteLength ); - todo_wine ok( status == HIDP_STATUS_SUCCESS, "HidP_SetUsageValue returned %#lx\n", status ); status = HidP_GetUsageValue( HidP_Feature, HID_USAGE_PAGE_ORDINAL, generic_output_list, i + 1, &value, preparsed_data, report, caps.FeatureReportByteLength ); - todo_wine ok( status == HIDP_STATUS_SUCCESS, "HidP_SetUsageValue returned %#lx\n", status ); - todo_wine ok( value == i, "got value %#lx, expected %#x\n", value, i ); status = HidP_SetUsageValueArray( HidP_Feature, HID_USAGE_PAGE_ORDINAL, generic_output_list, i + 1, (void *)&value, @@ -2340,19 +2332,15 @@ static void test_hidp( HANDLE file, HANDLE async_file, int report_id, BOOL polle { status = HidP_SetScaledUsageValue( HidP_Feature, HID_USAGE_PAGE_ORDINAL, generic_output_list, i + 1, i * 16, preparsed_data, report, caps.FeatureReportByteLength ); - todo_wine ok( status == HIDP_STATUS_SUCCESS, "HidP_SetScaledUsageValue returned %#lx\n", status ); status = HidP_GetScaledUsageValue( HidP_Feature, HID_USAGE_PAGE_ORDINAL, generic_output_list, i + 1, (LONG *)&value, preparsed_data, report, caps.FeatureReportByteLength ); - todo_wine ok( status == HIDP_STATUS_SUCCESS, "HidP_GetScaledUsageValue returned %#lx\n", status ); - todo_wine ok( value == i * 16, "got value %#lx, expected %#x\n", value, i * 16 ); buffer[i + 21] = i; } - todo_wine ok( !memcmp( buffer, report, sizeof(buffer) ), "unexpected report data\n" ); test_hidp_get_input( file, report_id, caps.InputReportByteLength, preparsed_data ); diff --git a/dlls/hid/hidp.c b/dlls/hid/hidp.c index bf5a6259484..83a4638b6ad 100644 --- a/dlls/hid/hidp.c +++ b/dlls/hid/hidp.c @@ -224,6 +224,8 @@ NTSTATUS WINAPI HidP_GetCaps( PHIDP_PREPARSED_DATA preparsed_data, HIDP_CAPS *ca struct usage_value_params { + BOOL array; + USAGE usage; void *value_buf; USHORT value_len; void *report_buf; @@ -238,15 +240,18 @@ static LONG sign_extend( ULONG value, const struct hid_value_caps *caps ) static NTSTATUS get_usage_value( const struct hid_value_caps *caps, void *user ) { + unsigned char *report_buf, start_bit = caps->start_bit; + ULONG bit_count = caps->bit_size, bit_offset = 0; struct usage_value_params *params = user; - ULONG bit_count = caps->bit_size * caps->report_count; - unsigned char *report_buf; + + if (params->array) bit_count *= caps->report_count; + else bit_offset = (params->usage - caps->usage_min) * caps->bit_size; if ((bit_count + 7) / 8 > params->value_len) return HIDP_STATUS_BUFFER_TOO_SMALL; memset( params->value_buf, 0, params->value_len ); - report_buf = (unsigned char *)params->report_buf + caps->start_byte; - copy_bits( params->value_buf, report_buf, bit_count, -caps->start_bit ); + report_buf = (unsigned char *)params->report_buf + caps->start_byte + bit_offset / 8; + copy_bits( params->value_buf, report_buf, bit_count, -(start_bit + bit_offset % 8) ); return HIDP_STATUS_NULL; } @@ -280,7 +285,7 @@ NTSTATUS WINAPI HidP_GetScaledUsageValue( HIDP_REPORT_TYPE report_type, USAGE us USAGE usage, LONG *value, PHIDP_PREPARSED_DATA preparsed_data, char *report_buf, ULONG report_len ) { - struct usage_value_params params = {.value_buf = value, .value_len = sizeof(*value), .report_buf = report_buf}; + struct usage_value_params params = {.usage = usage, .value_buf = value, .value_len = sizeof(*value), .report_buf = report_buf}; struct hid_preparsed_data *preparsed = (struct hid_preparsed_data *)preparsed_data; struct caps_filter filter = {.values = TRUE, .usage_page = usage_page, .collection = collection, .usage = usage }; USHORT count = 1; @@ -298,7 +303,7 @@ NTSTATUS WINAPI HidP_GetScaledUsageValue( HIDP_REPORT_TYPE report_type, USAGE us NTSTATUS WINAPI HidP_GetUsageValue( HIDP_REPORT_TYPE report_type, USAGE usage_page, USHORT collection, USAGE usage, ULONG *value, PHIDP_PREPARSED_DATA preparsed_data, char *report_buf, ULONG report_len ) { - struct usage_value_params params = {.value_buf = value, .value_len = sizeof(*value), .report_buf = report_buf}; + struct usage_value_params params = {.usage = usage, .value_buf = value, .value_len = sizeof(*value), .report_buf = report_buf}; struct hid_preparsed_data *preparsed = (struct hid_preparsed_data *)preparsed_data; struct caps_filter filter = {.values = TRUE, .usage_page = usage_page, .collection = collection, .usage = usage}; USHORT count = 1; @@ -316,7 +321,7 @@ NTSTATUS WINAPI HidP_GetUsageValueArray( HIDP_REPORT_TYPE report_type, USAGE usa USAGE usage, char *value_buf, USHORT value_len, PHIDP_PREPARSED_DATA preparsed_data, char *report_buf, ULONG report_len ) { - struct usage_value_params params = {.value_buf = value_buf, .value_len = value_len, .report_buf = report_buf}; + struct usage_value_params params = {.array = TRUE, .usage = usage, .value_buf = value_buf, .value_len = value_len, .report_buf = report_buf}; struct hid_preparsed_data *preparsed = (struct hid_preparsed_data *)preparsed_data; struct caps_filter filter = {.values = TRUE, .array = TRUE, .usage_page = usage_page, .collection = collection, .usage = usage}; USHORT count = 1; @@ -448,14 +453,17 @@ ULONG WINAPI HidP_MaxUsageListLength( HIDP_REPORT_TYPE report_type, USAGE usage_ static NTSTATUS set_usage_value( const struct hid_value_caps *caps, void *user ) { + unsigned char *report_buf, start_bit = caps->start_bit; + ULONG bit_count = caps->bit_size, bit_offset = 0; struct usage_value_params *params = user; - ULONG bit_count = caps->bit_size * caps->report_count; - unsigned char *report_buf; + + if (params->array) bit_count *= caps->report_count; + else bit_offset = (params->usage - caps->usage_min) * caps->bit_size; if ((bit_count + 7) / 8 > params->value_len) return HIDP_STATUS_BUFFER_TOO_SMALL; - report_buf = (unsigned char *)params->report_buf + caps->start_byte; - copy_bits( report_buf, params->value_buf, bit_count, caps->start_bit ); + report_buf = (unsigned char *)params->report_buf + caps->start_byte + bit_offset / 8; + copy_bits( report_buf, params->value_buf, bit_count, start_bit + bit_offset % 8 ); return HIDP_STATUS_NULL; } @@ -491,7 +499,7 @@ NTSTATUS WINAPI HidP_SetScaledUsageValue( HIDP_REPORT_TYPE report_type, USAGE us USAGE usage, LONG value, PHIDP_PREPARSED_DATA preparsed_data, char *report_buf, ULONG report_len ) { - struct usage_value_params params = {.value_buf = &value, .value_len = sizeof(value), .report_buf = report_buf}; + struct usage_value_params params = {.usage = usage, .value_buf = &value, .value_len = sizeof(value), .report_buf = report_buf}; struct hid_preparsed_data *preparsed = (struct hid_preparsed_data *)preparsed_data; struct caps_filter filter = {.values = TRUE, .usage_page = usage_page, .collection = collection, .usage = usage }; USHORT count = 1; @@ -508,7 +516,7 @@ NTSTATUS WINAPI HidP_SetScaledUsageValue( HIDP_REPORT_TYPE report_type, USAGE us NTSTATUS WINAPI HidP_SetUsageValue( HIDP_REPORT_TYPE report_type, USAGE usage_page, USHORT collection, USAGE usage, ULONG value, PHIDP_PREPARSED_DATA preparsed_data, char *report_buf, ULONG report_len ) { - struct usage_value_params params = {.value_buf = &value, .value_len = sizeof(value), .report_buf = report_buf}; + struct usage_value_params params = {.usage = usage, .value_buf = &value, .value_len = sizeof(value), .report_buf = report_buf}; struct hid_preparsed_data *preparsed = (struct hid_preparsed_data *)preparsed_data; struct caps_filter filter = {.values = TRUE, .usage_page = usage_page, .collection = collection, .usage = usage}; USHORT count = 1; @@ -526,7 +534,7 @@ NTSTATUS WINAPI HidP_SetUsageValueArray( HIDP_REPORT_TYPE report_type, USAGE usa USAGE usage, char *value_buf, USHORT value_len, PHIDP_PREPARSED_DATA preparsed_data, char *report_buf, ULONG report_len ) { - struct usage_value_params params = {.value_buf = value_buf, .value_len = value_len, .report_buf = report_buf}; + struct usage_value_params params = {.array = TRUE, .usage = usage, .value_buf = value_buf, .value_len = value_len, .report_buf = report_buf}; struct hid_preparsed_data *preparsed = (struct hid_preparsed_data *)preparsed_data; struct caps_filter filter = {.values = TRUE, .array = TRUE, .usage_page = usage_page, .collection = collection, .usage = usage}; USHORT count = 1; -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/1448
Hi, It looks like your patch introduced the new failures shown below. Please investigate and fix them before resubmitting your patch. If they are not new, fixing them anyway would help a lot. Otherwise please ask for the known failures list to be updated. The tests also ran into some preexisting test failures. If you know how to fix them that would be helpful. See the TestBot job for the details: The full results can be found at: https://testbot.winehq.org/JobDetails.pl?Key=126584 Your paranoid android. === debian11 (build log) === error: patch failed: dlls/dinput/tests/hid.c:1188 error: patch failed: dlls/dinput/tests/hid.c:1543 error: patch failed: dlls/dinput/tests/psh_hid_macros.h:79 error: patch failed: dlls/hidparse.sys/main.c:93 error: patch failed: dlls/ntoskrnl.exe/tests/psh_hid_macros.h:79 error: patch failed: dlls/winebus.sys/psh_hid_macros.h:79 error: patch failed: dlls/winexinput.sys/psh_hid_macros.h:79 error: patch failed: dlls/hid/hidp.c:236 error: patch failed: dlls/dinput/tests/hid.c:1806 error: patch failed: dlls/hid/hidp.c:224 Task: Patch failed to apply
On Mon Nov 21 19:00:32 2022 +0000, Rémi Bernon wrote:
You're right, I've pushed some update to my branch, with new tests to validate that. Fun fact, I remember being confused by the array flavor that returned `HIDP_STATUS_NOT_IMPLEMENTED` when I tried using it. The new tests uncovered that it only does when the elements are smaller than a byte. I don't really see how it is any more difficult to cover that case than to cover it with more bits but well, so be it. Wine just does it better ;). I've updated the MR, just edited the `/* Instance 31 */` comment to `/* Instance 10 */` in the tests.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/1448#note_16781
On Mon Nov 21 18:22:57 2022 +0000, Matthew Tran wrote:
You are using the udev / hidraw backend I presume? Yes, and to that end, I've disabled SDL mapping. If I leave SDL mapping enabled, the controller inputs work, but the HID lights don't. Perhaps that's related to why there's no desktop? Also I was applying these on wine-staging, will update when I try vanilla wine. **EDIT**: Master branch wine is the same. There should be no difference between the HID backends regarding the wineserver behavior. If the messages are coming from winedevice.exe, it should find the services desktop, which isn't very useful but is a valid desktop nonetheless.
-- https://gitlab.winehq.org/wine/wine/-/merge_requests/1448#note_16783
This merge request was approved by Rémi Bernon. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/1448
participants (4)
-
Marvin -
Matthew Tran -
Matthew Tran (＠0e4ef622)
-
Rémi Bernon