[Bug 51822] New: Simucube 2 TrueDrive: Doesn't recognize the steering wheel device

[WineHQ Bugzilla]

https://bugs.winehq.org/show_bug.cgi?id=51822

Bug ID: 51822
           Summary: Simucube 2 TrueDrive: Doesn't recognize the steering
                    wheel device
           Product: Wine
           Version: 6.12
          Hardware: x86-64
                OS: Linux
            Status: UNCONFIRMED
          Severity: minor
          Priority: P2
         Component: hid
          Assignee: [e-mail]
          Reporter: [e-mail]
                CC: [e-mail]
   Regression SHA1: 51560aabcb259cb89659d96654d8ad38edbb528d
      Distribution: ArchLinux

Created attachment 70707
  --> https://bugs.winehq.org/attachment.cgi?id=70707
0001-hidparse.sys-Preserve-the-original-state-items.repor.patch, wine_6.18.log,
SC2_descr.txt

Simucube 2 is FFB steering wheel device using the HID PID specification for
FFB. Thus the support for it is included in the Linux kernel as part of the HID
specification. It still needs some patching of the usbhid kernel module, but in
general is entirely functional in Wine, as long as "DisableInput" and "Enable
SDL" are disabled in the Wine registry, and for the HIDRAW device is granted rw
user access.

TrueDrive is an application for controlling and fine tuning various parameters
of the steering wheel (using HidD/HidP calls), which also includes real time
animation of the steering wheel movement, buttons pushed, etc. The application
is not used/needed during gameplay. The TrueDrive version used is 2020.10. The
application can be downloaded from the manufacturer site:
https://granitedevices.com/wiki/Simucube_2_True_Drive_releases

The mentioned regression commit is the earliest in which the problem occurs.
I've investigated further, and found that state->items.report_count in the
parse_new_value_caps() function (hidclass.sys/descriptor.c) is not preserved
properly. While being used for building the alternate value array it's set to
1, and since "Report Count" is a global item, reset_local_items() at the end of
the function preserves its state. Thus if there are subsequent main items
(Input, Output, etc.) with more than one usages, and with no "Report Count"
specified in between, the parser would calculate wrong bit sizes, report len,
etc.

The particular problem was at the end of the descriptor. Here is an excerpt:

/*      Usage Page (FF00h),                     */
/*      Usage (01h),                            */
/*      Collection (Application),               */
/*          Usage (01h),                        */
/*          Report ID (107),                    */
/*          Report Size (8),                    */
/*          Report Count (60),                  */
/*          Logical Minimum (0),                */
/*          Logical Maximum (255),              */
/*          Output,                             */
/*          Usage (01h),                        */
/*          Report ID (108),                    */
/*          Input,                              */
/*      End Collection,                         */

The Input report 108 follows immediately the Output report 107, without Report
Count specified in between. In our case parse_new_value_caps() would set
erroneously Report Count to 1, after being called on the Output main item. Thus
the next Input report size would be calculated wrongly. This is a vendor
defined report, which they probably use in the process of recognizing the
device.

Since then the issue was mitigated a little bit with commit
7096c26a2e48089424fe90956c80c29617bce9f2, when button array value caps were
introduced, which stopped modifying state->items.report_count if the parsed
item is an array.

The issue is still present in the new hidparse.sys (main.c), where the parsing
code has been moved with commit a290c5bf7c9ddc92af56231693c6d8f00c3efd7b.
It is present in Output report 2. Here is an excerpt:

/*      Usage (5Ah),                            */
/*      Collection (Logical),                   */
/*          Report ID (2),                      */
            ...
/*          Report Count (2),                   */
/*          Output (Variable),                  */
/*          Usage (5Ch),                        */
/*          Usage (5Eh),                        */
/*          Unit (Seconds),                     */
/*          Unit Exponent (-3),                 */
/*          Logical Maximum (32767),            */
/*          Physical Maximum (32767),           */
/*          Report Size (16),                   */
/*          Output (Variable),                  */
/*          Physical Maximum (0),               */
/*          Unit,                               */
/*          Unit Exponent (0),                  */
/*      End Collection,                         */

Since the Output main items are variable, state->items.report_count is still
being modified to 1. So the second Output item ends up with two usages, one 16
bit report, and one 0 bit report. This can be seen in the attached log file
wine_6.18.log. For Output report 2, the RCnt for Usage 0x5E is 0.
The following debug options were used for the log:
WINEDEBUG=+timestamp,+pid,+hid,+hidp,+hid_report,+plugplay. The Wine build is
6.18-107-gbcdb28a563d.
I've attached the Simucube 2 device descriptor in SC2_Descr.txt. Hope it can
help as a real life example of a HID PID device with multiple ReportIDs.

Attached also a patch which is fixing the issue (based on the latest master
aa629c4c7225166f4ee46476d98702df2e142711).