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

[Arkadiusz Hiler]

On Thu, Jul 01, 2021 at 03:11:57PM +0200, Rémi Bernon wrote:
> 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.

You are righ. That would require even more gymnastics, which is probably
not worth it.

-- 
Cheers,
Arek