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

[WineHQ Bugzilla]

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

--- Comment #6 from Rémi Bernon <rbernon at codeweavers.com> ---
(In reply to Ivo Ivanov from comment #5)
> (In reply to Rémi Bernon from comment #4)
> > Thanks, this looks correct indeed I'll try to add some tests as well and
> > upstream a fix.
> 
> Glad I can help! Pretty excited about the HID PID joystick implementation in
> dinput :) Will test it thoroughly when it's ready.
> 
> I'd like to mention another similar issue with this device descriptor, which
> I spotted while observing the log.
> This is part of Output report 1:
> 
> /*          Usage (57h),                        */
> /*          Collection (Logical),               */
> /*              Usage (000A0001h),              */
> /*              Usage (000A0002h),              */
> /*              Unit (Degrees),                 */
> /*              Unit Exponent (-2),             */
> /*              Logical Minimum (0),            */
> /*              Logical Maximum (180),          */
> /*              Physical Minimum (0),           */
> /*              Physical Maximum (36000),       */
> /*              Unit,                           */
> /*              Report Size (8),                */
> /*              Report Count (1),               */
> /*              Output (Variable),              */
> /*              Unit Exponent (0),              */
> /*              Unit,                           */
> /*          End Collection,                     */
> 
> There are two ordered items Usages, but the Report Count is only 1.
> hidparse.sys parses this as two usages, with the second one being with RCnt
> 0. It's visible in the provided log, slightly above the previous issue. I
> believe this is correct in this situation (RCnt 0). Don't know how this
> would affect other parts of the logic though.
> Anyway, there is this line of code in parse_new_value_caps()
> (hidparse.sys/main.c):
> 
> if (!(is_array = HID_VALUE_CAPS_IS_ARRAY( &state->items )))
> state->items.report_count -= usages_size - 1;
> 
> if usages_size is >= 3 and state->items.report_count is 1, then report_count
> would be a negative value if it was a signed integer, otherwise it would
> wrap around leading to undesired behavior. In my opinion this needs to be
> looked at too.
> 

Hmm indeed... maybe a few tests would show what Windows is really doing but in
any case we should probably make sure not to wrap around.

I didn't bother too much (yet) with arguably invalid report descriptors, as
Windows is already quite brittle in some cases (for instance I could see that
it doesn't like very much descriptors which skip some logical/physical
min/max).

> The descriptor doesn't look correct though. I'll contact Granite Devices for
> this issue, and hopefully they'll fix it themselves.
> 
> P.S. I have few more bugs (with patches) to report, mostly in hidclass.sys
> and one in winebus.sys, which affect mostly such HID devices with multiple
> reports/ReportIDs. Hopefully won't disturb you, or the other devs too much.

Thanks! I will be happy to have a look, even before I finish sending the
patches, maybe there's some fixes that could be worth including early.

I couldn't test the PID code very extensively, as I don't have any device
actually supporting such kind of reports. Thanks to the HID test driver we can
confirm the behavior on Windows even without one, but maybe it's supposed to
differ a bit depending on the devices.

For instance, I can see that your device is using the "new effect block" /
"load block" / "free block" reports, which I initially intended to implement
too, but the tests showed that even when these reports are declared Windows
dinput only seems to be using the "set effect block" with its own effect
indexes.

Maybe I just couldn't figure the exact required report declaration to make it
use them, but I tried with a SideWinder Force Feedback 2 report descriptor,
which has them as well, and it's still not doing it.

So I just implemented the same behavior, and hopefully it'll work here. If it
doesn't then we'll have to implement these reports, it's a bit more verbose and
I still do not completely understand how they are supposed to work though.

Same goes with the "set condition" report, with the weird "type specific block
offset" and "type specific parameter offset", which seems to be simply ignored
on Windows, although required. The PID documentation is also pretty cryptic
about them...

-- 
Do not reply to this email, post in Bugzilla using the
above URL to reply.
You are receiving this mail because:
You are watching all bug changes.