[PATCH v3] dinput: Introduce new HID joystick backend.

[Rémi Bernon]

On 7/1/21 12:38 PM, Arkadiusz Hiler wrote:
> On Wed, Jun 30, 2021 at 03:16:13PM +0200, Rémi Bernon wrote:
>> On 6/30/21 2:30 PM, Rémi Bernon wrote:
>>> On 6/30/21 1:08 PM, Arkadiusz Hiler wrote:
>>>> On Wed, Jun 30, 2021 at 10:54:19AM +0200, Rémi Bernon wrote:
>>>>> +
>>>>> +static HRESULT hid_joystick_enum_device( DWORD type, DWORD
>>>>> flags, DIDEVICEINSTANCEW *instance,
>>>>> +                                         DWORD version, int index )
>>>>> +{
>>>>> +    HIDD_ATTRIBUTES attrs = {sizeof(attrs)};
>>>>> +    PHIDP_PREPARSED_DATA preparsed;
>>>>> +    WCHAR device_path[MAX_PATH];
>>>>> +    HIDP_CAPS caps;
>>>>> +    HANDLE device;
>>>>> +    HRESULT hr;
>>>>> +
>>>>> +    TRACE( "type %x, flags %#x, instance %p, version %04x,
>>>>> index %d\n", type, flags, instance, version, index );
>>>>> +
>>>>> +    hr = hid_joystick_device_open( index, 0, 0, device_path,
>>>>> &device, version,
>>>>> +                                   &preparsed, &attrs, &caps,
>>>>> instance );
>>>>
>>>> hid_joystick_device_open() is getting a bit unwieldy with all the
>>>> possible parameter combinations. I don't think it needs the enumeration
>>>> / vidpid lookup logic.
>>>>
>>>> IMO it would read better if hid_joystick_device_open() would take the
>>>> path or RI handle and enum logic would be moved either here or to
>>>> a helper.
>>>>
>>>
>>> I don't think we need yet another device list cache, and I would rather
>>> avoid it if possible. It's really just two call sites.
> 
> Oh sorry, this is not what I was suggesting.
> 
> I was thinking about factoring out the setupapi enumeration / device
> finding logic to a separate function than the one that does all the
> Hid?_* handling.
> 
> Something like:
> 
> HANDLE hid_joystick_find_device( int index, HANDLE ri_handle, int vid_pid, HANDLE *ri_handle_out );
> 
> and then you can either:
> 
> device = hid_joystick_find_device( index, INVALID_HANDLE, -1, &ri_handle );
> device = hid_joystick_find_device( -1, ri_handle, -1, NULL );
> device = hid_joystick_find_device( -1, INVALID_HANDLE, vid_pid, NULL );
> 
> open would then look more like:
> 
> hr = hid_joystick_device_read( device, version, &preparsed, &attrs, &caps );
> 
> Those two should give you all the thing you need to fill
> DIDEVICEINSTANCEW in _enum_ and to fill in hid_joystick in _create_.
> 
> Also I like to use obviously wrong values for parameters that are not used.
> 

Well the issue is that DInput enumeration is a bit weird and it 
enumerates the backend with increasing indexes until it fails.

If, for some reason, we can find a device at the provided index, but the 
the read fails, we need to try again with another index or the 
enumeration will consider there's no more devices.

So I don't think we can do anything different from the current loop, 
where we try to open device until it succeeds, and only consider the 
DInput index when the device could be fully opened.
-- 
Rémi Bernon <rbernon at codeweavers.com>