[Bug 51873] Joy.cpl crashes on the FFB tab when the FFB device is selected in the drop down list

[WineHQ Bugzilla]

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

--- Comment #15 from Ivo Ivanov <logos128 at gmail.com> ---
(In reply to Rémi Bernon from comment #13)
> It could be possible to do it the same way as feature reports but the
> problem I'm seeing there is that IOCTL_HID_GET_INPUT_REPORT needs to be
> explicitly requested and I don't see how it would specify the event number
> it's interested in.

I didn't think about the effect id.
Yeah, looks like the most logical way to implement this from manufacturer POV
is to send periodical Input reports on the INTERRUPT IN channel, and seems this
is what the specification suggests. Otherwise they would have written this
explicitly. There is no logic for such report to be implemented on the control
endpoint, without a way to specify the input parameters as effect id, etc. If
the authors of the specification wanted such a thing, they would have
implemented it with feature reports in the same manner as Create New Effect ->
PID Block Load sequence.

I've looked again the usb capture data from the WheelCheck application on
Windows 10, and there is no trace of any Input report through the control
endpoint or any Input report with id 2 (the PID State Report id in Simucube 2)
in the INTERRUPT IN endpoint. So MS doesn't use the control endpoint as well.

IMO this is probably either overlooked by the device manufacturer, or a bug in
the firmware. I'm not on the latest firmware, though. There are few iterations
from the last few months, that mostly add support for new revisions of the
device. I'll install the latest firmware, and let you know for the results.

I plan to contact the manufacturer for the issue with the report descriptor we
talked about a few weeks ago, so I can tell them about this issue too.

> Is HIDIOCGINPUT returning an error if there's no more input report for the
> matching id, or is it blocking until there's one?
> 
> If it's the case, we'll maybe have to implement IOCTL_HID_GET_INPUT_REPORT
> in the same kind of way then, instead of succeeding every time and returning
> the last report seen. This would need to be the case for all the backend
> though, and maybe not completely straightforward.

I have read again the MS documentation about the IOCTL_HID_GET_INPUT_REPORT /
HidD_GetInputReport. They seem to suggest that IOCTL_HID_GET_* / HidD_Get*
routines should be used non regularly only to get the current state of the
device. Something that is always mentioned for the control endpoint in the USB
HID specification. They also always couple the info for
IOCTL_HID_GET_INPUT_REPORT with IOCTL_HID_GET_FEATURE. For continuously
obtaining Input reports, they suggest IRP_MJ_READ / ReadFile. So I would assume
that IRP_MJ_READ uses the highly responsive INTERRUPT IN endpoint, while
IOCTL_HID_GET_* uses the control endpoint.

So, IMO to be more consistent with MS' documentation, we should implement
IOCTL_HID_GET_INPUT_REPORT (using HIDIOCGINPUT) in the same manner as
IOCTL_HID_GET_FEATURE. This way it (HidD_GetInputReport) would do exactly what
the applications expect from it. And some apps might use it just because it
supposedly uses the control endpoint, and not the INTERRUPT IN.

It would also help to better and more easily test this particular issue, of
course.

> The client (dinput) will also need to make a dedicated asynchronous ioctl to
> read effect statuses, instead of reading them in the same way as the device
> state reports.
> 
> Another possible approach would be to do like the SDL backend, where we
> would request HIDIOCGINPUT regularly and push the report in the same way as
> device state reports. That may be simpler from the client perspective as we
> wouldn't have to worry whether a device needs IOCTL_HID_GET_INPUT_REPORT
> requests or just ReadFile calls.

IMO, the proposed patchset is the right way to implement this feature. If the
state of the effect is unknown by the time GetEffectStatus is called, then
there should be an alternative way to calculate this on the dinput side. It
would need the effect start time, loop count (or infinity), duration and
startdelay (if supported) to calculate the current state of the effect, taking
into account if the device is acquired and all the other factors, etc. On every
successful Start it would set the start time, and on every Stop, Unload, Reset,
etc. it would zero it. Every successful SetParameters would set the other time
parameters. Then it's pretty easy to calculate if the effect is currently
playing or not.

-- 
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.