[SDL2] Adding input and FFB support for Logitech G29(PS3) on hidapi

[Kethen]

Description

These changes enable the Logitech G29 wheel to run on hidapi with both SDL_Joystick and SDL_Haptic interfaces.

While it is already possible to use the wheel on Linux in WINE + SDL2 thanks to the in-tree evdev driver as well as new-lg4ff, these set of changes allow the G29 to be used with WINE under MacOS and FreeBSD

These wheels should also be supported, but I can only test them from G29's compat modes: G27, G25, DFGT, DFP, DFEX

Haptic and led support are ported from https://github.com/berarma/new-lg4ff so giving @berarma a ping here

Companion test program for testing SDL_Joystick input events, changing wheel compat mode and testing SDL_Haptic calls

https://github.com/Kethen/sdl_lg4ff_util/

Help wanted for this pull request

• Makefile.os2, Makefile.w32, VisualC project files
• More testing and code review

Help wanted for future pull requests

• Support for G923 and other wheels that runs on the same ffb protocol but cannot be emualted by my G29
• Shifter input, it might already work but I don't own one as of writing so I can't test that

Existing Issue(s)

https://github.com/libsdl-org/SDL/issues/6540

[slouken]

Can you retarget this PR for SDL3?

[Kethen]

Can you retarget this PR for SDL3?

https://github.com/libsdl-org/SDL/pull/11598

[sezero]

I don't know whether or not this will be accepted into SDL2 branch, but here is a patch that adds new files to Makefile.os2 / Makefile.w32, and fixes C89 issues, fixes some warnings, and missing end-of-file issues. (a few bits taken from SDL3 version. Also note the signedness issue in HIDAPI_DriverLg4ff_UpdateDevice() is also present in the SDL3 version.) patch.txt

One issue is SDL_hidapihaptic_lg4ff.c does libc function calls sprintf, abs and llabs, and especially windows can be built without libc support. For that, sprintf can be replaced with SDL_snprintf, which I did in the patch like you did in the SDL3 version. abs() with SDL_abs, but if you want the compiler to optimize it, you can add a static inline for it in the lg4ff source. There is no SDL_llabs though: either it can be added as a new SDL api call, or llabs can be added to lg4ff source as a static inline helper so that the compiler can optimize it. (Note: the abs and llabs() issue of directly calling into libc seems to present in the SDL3 version too.)

[Kethen]

@sezero Thank you so much for the patch, can I ask for a version with git format-patch so that I can include your work as your commits?

As for whether this will be accepted or not, I'm unsure, and there seems to be a sentiment to use sdl2-compat in some linux distros eg. https://fedoraproject.org/wiki/Changes/SDL2onSDL3

[sezero]

@sezero Thank you so much for the patch, can I ask for a version with git format-patch so that I can include your work as your commits?

There you go: 0001.patch.txt

Note that the abs() and llabs() issue is still there. (SDL_abs() is the worst thing added to SDL api...) (Maybe you should add abs32() and abs64() static inlines in there and use them instead.)

[berarma]

Implementing a hardware driver inside SDL is something I would never expect. I'd say it's a bad idea but who knows, maybe I'm old fashioned.

I would have thought porting the driver would be a better idea but maybe it's not possible.

[Kethen]

Hey @berarma , thanks for looking at and commenting on this! Please also see https://github.com/libsdl-org/SDL/pull/11598#issuecomment-2525243373 , as the author of new-lg4ff, you can decide whether code ported from your driver can be part of SDL or not

Implementing a hardware driver inside SDL is something I would never expect. I'd say it's a bad idea but who knows, maybe I'm old fashioned.

I would have thought porting the driver would be a better idea but maybe it's not possible.

On open platforms like FreeBSD, I'd agree, someone well versed in FreeBSD development should be able to develop/port drivers to FreeBSD evdev, yielding a better performing driver, even when a userspace driver might be "good enough"

On other platforms however, it's not always possible

On Android, how the kernel is compiled is up to device manufacturers (and they orphan their kernels way way too often), the Paddleboat API does not have any feedback support beyond vibration motors, there's sure no HAL implemented for more complicated evdev FFB; However it is possible to get raw usb access

On Apple operating systems, they introduced GCRacingWheel in 2022, but it lacks force feedback currently and which wheel they support is entirely up to them; To support custom devices on a per application baisis, they provide IOHIDManager and IOKit

SDL's various hidapi controller implementations can be used in these situations

[slouken]

Yeah, we'll need official permission from @berarma to relicense code based on their work under the Zlib license for SDL.

[berarma]

Yeah, we'll need official permission from @berarma to relicense code based on their work under the Zlib license for SDL.

My work is based on the Linux lg4ff module. I added code to implement more effects other than the constant effect and some tweaking features. I'm not sure I'm in a position to decide this.

[Kethen]

Then I believe yet another person to ping would be @mungewell, going by the header of hid-lg4ff.c

[Kethen]

Happy holiday seasons!

• After reviewing git blame of ported code, ported functions are now individually labeled with more detailed authorship information, license conversion should require permission from @mungewell , @MadCatX and @berarma
• Added native mode switching routine, but not tested on wheels other than the G29
• Fixed building for github action available targets
• Added various other bug fixes

Attaching email exchange with @mungewell , @MadCatX and @slouken here, once again thank you so much @mungewell @MadCatX for being so forth coming with the license conversion:

Permission to port hid-lg4ff to SDL2_3.mbox.txt

Yeah, we'll need official permission from @berarma to relicense code based on their work under the Zlib license for SDL.

My work is based on the Linux lg4ff module. I added code to implement more effects other than the constant effect and some tweaking features. I'm not sure I'm in a position to decide this.

@berarma Your effect rendering code still brings enhancement to the driver, and I'd love to be able to include them in this port. To do so, I'd still need your explicit permission

[sezero]

Here is a minor clean-up patch. (Possibly applies to the SDL3 version too.) Pick from it as you like.

- make _abs,_llabs, get_time_ms, effect_is_periodic, effect_is_condition
  helpers static inline.
- rename _abs and _llabs to abs32 and abs64 to avoid analyzers complain
  about reserved names.
- make the drivers[] array static.
- NULL terminate the drivers[] array to avoid an empty array in case if
  SDL_HAPTIC_HIDAPI_LG4FF isn't configured.
- minor formatting clean-ups here and there.
- remove #endif comments about SDL_JOYSTICK_HIDAPI for readability.
   those #if / #endif blocks are short enough already. 

patch1.txt

[berarma]

I give permission to re-licence my code in new-lg4ff under the Zlib license.

[Kethen]

Here is a minor clean-up patch. (Possibly applies to the SDL3 version too.) Pick from it as you like.

- make _abs,_llabs, get_time_ms, effect_is_periodic, effect_is_condition
  helpers static inline.
- rename _abs and _llabs to abs32 and abs64 to avoid analyzers complain
  about reserved names.
- make the drivers[] array static.
- NULL terminate the drivers[] array to avoid an empty array in case if
  SDL_HAPTIC_HIDAPI_LG4FF isn't configured.
- minor formatting clean-ups here and there.
- remove #endif comments about SDL_JOYSTICK_HIDAPI for readability.
   those #if / #endif blocks are short enough already. 

patch1.txt

Thanks for the patch!

I give permission to re-licence my code in new-lg4ff under the Zlib license.

Thank you!

[slouken]

Hi @Kethen, it sounds like you have permission from the authors to use this code. Great job!

Can you please note that on the SDL3 PR and link to the relevant comments in this pull request?

We're getting close to 3.2.0 release, so I'll bump your SDL3 PR to the 3.4.0 milestone where it is likely to be accepted.

[berarma]

I see that our names don't appear in the headings of the files where our code is used, only above the corresponding functions. Is that correct?

[slouken]

@Kethen, the convention is to add copyright holders to the license at the top, e.g. https://github.com/libsdl-org/SDL/blob/23410debf7ad2d05d4662ed38af110e43b2e3a14/src/camera/pipewire/SDL_camera_pipewire.c#L3-L4

[Kethen]

@Kethen, the convention is to add copyright holders to the license at the top, e.g.

https://github.com/libsdl-org/SDL/blob/23410debf7ad2d05d4662ed38af110e43b2e3a14/src/camera/pipewire/SDL_camera_pipewire.c#L3-L4

Hello @slouken, currently ported functions are annotated this way https://github.com/libsdl-org/SDL/blob/54f4837a77374e3b4e1522e1a5460a8513be263d/src/haptic/hidapi/SDL_hidapihaptic_lg4ff.c#L1140-L1148

What would be appropriate to add to the header of files in this PR (and the SDL3 PR) containing ported code?

[slouken]

Hello @slouken, currently ported functions are annotated this way

https://github.com/libsdl-org/SDL/blob/54f4837a77374e3b4e1522e1a5460a8513be263d/src/haptic/hidapi/SDL_hidapihaptic_lg4ff.c#L1140-L1148

What would be appropriate to add to the header of files in this PR (and the SDL3 PR) containing ported code?

Maybe you can ask the contributors how they would like to be credited?

[berarma]

@Kethen, the convention is to add copyright holders to the license at the top, e.g.

https://github.com/libsdl-org/SDL/blob/23410debf7ad2d05d4662ed38af110e43b2e3a14/src/camera/pipewire/SDL_camera_pipewire.c#L3-L4

This is what I expected. In my very basic understanding of copyright law, in the current form, it looks as if we had transferred copyrights. Sincerely, I don't know how important this might be, but is there any issue in following the convention?

[slouken]

@Kethen, the convention is to add copyright holders to the license at the top, e.g. https://github.com/libsdl-org/SDL/blob/23410debf7ad2d05d4662ed38af110e43b2e3a14/src/camera/pipewire/SDL_camera_pipewire.c#L3-L4

This is what I expected. In my very basic understanding of copyright law, in the current form, it looks as if we had transferred copyrights. Sincerely, I don't know how important this might be, but is there any issue on following the convention?

No, no issue at all.

[Kethen]

@Kethen, the convention is to add copyright holders to the license at the top, e.g. https://github.com/libsdl-org/SDL/blob/23410debf7ad2d05d4662ed38af110e43b2e3a14/src/camera/pipewire/SDL_camera_pipewire.c#L3-L4

This is what I expected. In my very basic understanding of copyright law, in the current form, it looks as if we had transferred copyrights. Sincerely, I don't know how important this might be, but is there any issue on following the convention?

No, no issue at all.

@slouken In that case, copyrights of authors of ported code are now added to license headers

[Lawstorant]

Hmm, but is SDL the place for things like that? I understand the want to make this usable under MacOS and FreeBSD but wouldn't it make SDL do too much work now?

Native software might be out of luck, but maybe hidraw could be used to install Logitech drivers under Wine prefix to run it like that? I know people with Asetek wheelbases relied on hidraw to just pass the PID device to Windows side and let it deal with it.

[Kethen]

Hmm, but is SDL the place for things like that? I understand the want to make this usable under MacOS and FreeBSD but wouldn't it make SDL do too much work now?

I would not know what the scope of SDL should be, but there are multiple existing hid controller drivers in SDL already, and more are being added over time https://github.com/libsdl-org/SDL/tree/main/src/joystick/hidapi

Native software might be out of luck, but maybe hidraw could be used to install Logitech drivers under Wine prefix to run it like that? I know people with Asetek wheelbases relied on hidraw to just pass the PID device to Windows side and let it deal with it.

Wine does have kernel usb driver(through libusb) and hid.dll(sans freebsd, iohid/udev+hidraw only) support, and I do look forward to the day where winusb.dll is also ready, and believe that a lot of windows usb/hid drivers can be eventually made work inside of wine even the funky half kernel half user space ones with dependencies on CEF; but as you said, it's a wine only solution

[slouken]

So this is out of scope for SDL2, but I'll leave a reference to https://github.com/libsdl-org/SDL/pull/11598 here for tracking legal permissions and I plan to accept that for 3.4.0.

Thanks!