SDL icon indicating copy to clipboard operation
SDL copied to clipboard

Add support for sensor to joystick on Linux (evdev)

Open meyraud705 opened this issue 2 years ago • 5 comments

Description

This patch adds support for sensor to joystick on Linux through the evdev interface.

It stores all sensor devices into a list and try to match the sensor and the joystick device with the unique identifier (see https://github.com/libsdl-org/SDL/issues/6478#issuecomment-1304357557 ) when a SDL_Joystick is opened.

If a new-sensor notification from inotify, evdev callback or fallback detection happens after SDL_JoystickOpen (main device notification -> SDL_JOYDEVICEADDED event -> SDL_JoystickOpen -> sensor device notification) then the SDL_Joystick will not have sensor. It did not happen to me while testing hotplug. I am not sure how much we should wait after a notification to be sure that we get all event sources.

If the sensor is removed, SDL_JOYDEVICEREMOVED event is not sent as I assume both sensor and joy item are removed at the same time.

SDL_EVDEV_GuessDeviceClass is used to find sensor. I don't know if this alone is enough.

I assume that all devices follow the same orientation convention as the DualShock 4.

Existing Issue(s)

This partially fix #6478, you still need to allow read access to sensor device with an udev rule.

Here the udev rule for DualShock 4 Slim:

# DualShock 4 Slim over usb
SUBSYSTEM=="input", ATTRS{idVendor}=="054c", ATTRS{idProduct}=="09cc", MODE="0660", TAG+="uaccess"

# DualShock 4 Slim over bluetooth
SUBSYSTEM=="input", KERNELS=="*054C:09CC*", MODE="0660", TAG+="uaccess"

meyraud705 avatar May 11 '23 18:05 meyraud705

Marked as draft because I noticed I made some mistakes. I will fix them, but not right now.

meyraud705 avatar May 11 '23 19:05 meyraud705

I fixed and tested this with a Dualshock 4. It is ready for review and test with other controllers.

meyraud705 avatar May 12 '23 16:05 meyraud705

I added some comments, but in general this looks good. Thanks!

slouken avatar May 12 '23 17:05 slouken

I force pushed the minor requested changes on the first commit.

In the commit to improve sensor detection, I used names and code from SDL_ShouldIgnoreGamepad() so I moved SDL_endswith() from SDL_gamepad.c to SDL_utils.c.

Closing sensor file when sensors are disabled was simple enough and I think it is worth it because most games don't even enable them.

meyraud705 avatar May 13 '23 17:05 meyraud705

Sorry if I don't provide anything as contributing here, but I would love if someone here who has a 8bitdo's SN30 pro+ (or anyone else who has a Pro Controller for that matter) to test this!

Impeta avatar May 13 '23 21:05 Impeta

Sorry if I don't provide anything as contributing here, but I would love if someone here who has a 8bitdo's SN30 pro+ (or anyone else who has a Pro Controller for that matter) to test this!

If it's possible to refactor the device identification heuristic to be more like SDL_EVDEV_GuessDeviceClass - working from pre-collected information, rather than from a "live" file descriptor - then it will be possible to do some basic testing against the pre-recorded device descriptions in tests/testevdev.c, even for people who don't own the appropriate controller. Obviously that won't be a full-stack functional test and wouldn't detect bugs like "x and y axis are swapped", but it's a lot better than nothing.

We have this information for DualShock 4 and 5 (3-axis accelerometer + 3-axis gyro), DualShock 3 (3-axis accelerometer), and the various Wii accessories (probably not practically useful for PC gaming). We don't currently have this information for any Switch-related controllers. One of my colleagues has more gamepads than I do, and reports that with current Linux kernels, Switch Joycons don't produce an evdev device node for motion controls.

smcv avatar Jun 15 '23 16:06 smcv

you still need to allow read access to sensor device with an udev rule

I have tried to get the equivalent of this udev rule upstream into udev, but it is looking as though the systemd/udev maintainers might insist that accelerometers must be accessed via a new interface that has not yet been invented (either a D-Bus-based portal, or a new interface on the Wayland compositor).

smcv avatar Jun 15 '23 16:06 smcv

I have tried to get the equivalent of this udev rule upstream into udev

I hope that SDL being able to read these devices will help push this request.

meyraud705 avatar Jun 26 '23 11:06 meyraud705

Merged, thanks!

slouken avatar Jun 26 '23 14:06 slouken

We have this information for DualShock 4 and 5 (3-axis accelerometer + 3-axis gyro), DualShock 3 (3-axis accelerometer), and the various Wii accessories (probably not practically useful for PC gaming). We don't currently have this information for any Switch-related controllers. One of my colleagues has more gamepads than I do, and reports that with current Linux kernels, Switch Joycons don't produce an evdev device node for motion controls.

@smcv Yes, that's really the problem. At the moment, nothing of Switch's peripherals, joycons or pro-controllers and such, furthermore 3rd party controllers that are functionally same as Switch pro-controllers, seem to spawn an request for aligning and emulating motion controls, unless using through Cemuhook or the alike. I was hoping with this merged finally, motion controls/anything sensor detectable are natively supported and works OOTB. Just to make sure we're on the same page, sorry, is this what you mean?

Merged, thanks!

@slouken Thank you! For certainty's sake, is this merged into SDL3 or will it be backported/included on current SDL2?

Impeta avatar Jun 26 '23 14:06 Impeta

It needs more testing, but this seems reasonable to backport to SDL2.

slouken avatar Jun 26 '23 14:06 slouken

but this seems reasonable to backport to SDL2.

After testing and bugreporting as you say, please, consider so! Emulators such as CEMU, Yuzu and Ryujinx won't work at all in my end with natively detecting and supporting motion controls, neither through Switch nor DualShock4 modes from my 8bitdo's SN30+ pro controller, unless again I opt for the DSU server and use Cemuhook. https://github.com/cemu-project/Cemu/issues/753

Impeta avatar Jun 26 '23 14:06 Impeta

They should work with hidraw support out of the box.

slouken avatar Jun 26 '23 15:06 slouken

They should work with hidraw support out of the box.

@slouken they do not work with HIDRAW out of the box. the same issue with needing additional UDEV rules applies to HIDRAW as it does the IMU event#. It just happens that most of you (and most users) have steam installed which provides the HIDRAW UDEV rules for many popular controllers and SDL ends up benefitting from that.

See https://github.com/cemu-project/Cemu/issues/1097#issuecomment-1952626670 for more information.

Alternatively to use steam's UDEV rules, the PR that was previously linked should solve this as well https://github.com/systemd/systemd/pull/22860 since it adds the necessary UDEV rules to BOTH the event# and hidraw# device nodes for many controllers (but not all the one ones needed, I notice already that Nintendo Switch Joy-Cons are missing and I am sure many many others are as well).

theofficialgman avatar Feb 19 '24 17:02 theofficialgman