[PATCH v2 2/5] winebus.sys: Build device reports for linux event devices

[Roderick Colenbrander]

On Thu, Mar 2, 2017 at 11:01 AM, Aric Stewart <aric at codeweavers.com> wrote:
>
>
> On 3/2/17 12:34 AM, Roderick Colenbrander wrote:
>> Hi Aric,
>>
>> I have had a quick look over this patch and have some initial
>> feedback, maybe more later if time permits.
>>
>> For the absolute and relative axes don't hard code the minimum and
>> maximum values for the report descriptors. These are not correct for
>> many devices e.g. Xbox controllers use 16-bit absolute axes, while
>> DS3/DS4 for example use 8-bit. In addition xbox controllers use signed
>> data, while DS3/DS4 use unsigned (0-255)Also many devices use signed
>> data.
>>
>> You should rely on the absinfo data returned by EVIOCGABS and base
>> your input ranges on that.
>>
>> Thanks,
>> Roderick
>>
>
> Hi Rodrick,
>
> Thanks for looking at these, your input is very valuable given all your experience.
>
> I am a bit swamped right at the moment so I will need a moment to circle back but any in depth review you give will not be ignored. I would love to get this part working.
>
> -aric
>
>> On Mon, Feb 27, 2017 at 9:32 AM, Aric Stewart <aric at codeweavers.com> wrote:
>>>
>>> v2: Updates from Sebastian Lackner
>>>
>>> Signed-off-by: Aric Stewart <aric at codeweavers.com>
>>> ---
>>>  dlls/winebus.sys/bus_udev.c | 438 +++++++++++++++++++++++++++++++++++++++++++-
>>>  1 file changed, 436 insertions(+), 2 deletions(-)
>>>
>>>
>>>
>>>
>>>

Hi Aric,

Had a high-level look at some other aspects. As you are aware mapping
from evdev to HID is not ideal as not everything can be expressed
perfectly and there is limited metadata (in particular for buttons).
For sure we can't restore the original HID reports, so applications
which hard coded reading of raw reports for particular devices just
can't work, so those would need to use hidraw, but that's okay.

The area I'm a bit worried about is the axis mapping. Many devices
initially rely on the hid-core layer of the Linux kernel to do mapping
of axes, buttons etcetera based on the HID reports. Many devices do
report fixups or override the mapping as descriptors are often buggy
and not everything can be expressed well in the descriptors.

One buggy area is the mapping of the second stick. There is no good
way for expressing the second stick in HID. Basically HID (and evdev)
talks about X/Y/Z as being for linear displacements and RX/RY/RZ for
rotational displacements (e.g. flight sticks you can rotate a bit).
Over the years at least Linux started (ab?)using RX/RY/RZ for second
stick and trigger. Some devices e.g. ds3/ds4 that I'm aware of and
some others use Z/RZ for the second stick and ry/rz for the triggers,
though for ds4 I fixed this in Linux 4.10 and ds3 will be fixed in
4.12 (though you need to detect the different mapping, we are or'ing
the version with | 0x8000). It looks like the Switch Pro controller is
doing the same right now (will likely be fixed when an official driver
appears in the kernel). Not sure about other devices.

For dinput this mapping may not be much of a concern as expressing
axes is more flexible. In my opinion the popular console gamepads
which do map well to xinput, should be supported in xinput as well.
This is where some of the concern comes in. You would need a per
device mapping table at minimum and maybe even a per platform one (as
I said mapping can change between drivers for a given device).

Another concern about axes mapping, though less of a concern, is there
are many more axes which some drivers report just because the standard
HID parser is basic and there are not enough good suitable evdev axes.
Some devices will leak into other axes ranges (e.g. ABS_MISC /
ABS_MT), though such axes leaking for typically analog buttons could
be ignored.

How do you intend to report other information which comes in through
HID? A good example is battery information which depending on the
device is part of the regular input reports. On Linux you can only get
this data through sysfs. It is not too important yet, but there should
be a plan.

Similar how do you intend to support force feedback / rumble? Of
course it would be output reports, but I forgot if the HID spec
supports them properly, it is typically done in device specific ways
(at least basic rumble is).

Thanks,
Roderick