Conversation
optimizations SDL touchpad support bugfixes
|
The checks failed due to the UDEV_TOUCH_SUPPORT not being defined. I'm not really sure what to do should I add a UDEV_SENSOR_SUPPORT macro for the sensor code? |
|
To make some test |
|
You need to fix the failing CI tasks |
|
Will do |
|
Linux x86 pipeline still failing. |
It seems that sensor API has been introduced in SDL version 2.0.9. And we are using SDL 2.0.4 for the CI as we are building under Xenial. |
|
Even if the build succeeds I don't want the pull request merged yet. It works adequately now for testing but to be usable I need to add a lot of options to adjust sensitivity and mapping. The problem is RN the retropad stack assumes there's only one set of sensors... safe assumption to make on mobile. Not so much with sensor enabled controllers. An example is that on the dualsense is the way it's set up by default you have to point the shoulder buttons up. Most people play games with the face buttons pointing up so it makes playing extremely unintuitive. I only made the pull request so people will be aware that I am working on it and maybe get some more testing on it. Please don't merge it until it's complete. |
|
You can tell GitHub to mark the PR as a draft, which will prevent it from being merged until you tell it that it's ready for review. |
|
CI Linux (i686) still fails. Is SDL_Sensor only available on specific SDL versions? Can we have a compile-time ifdefs for this? |
|
It seems there is a macro SDL_VERSION_ATLEAST so we could use |
|
I was going to say that maybe we should drop support for ancient versions of SDL2, but turns out SDL 2.0.9 released in 2018 so it isn't that old. |
I am confused the release date indicates May 24, 2022 ? |
|
Idk the tags tab says 2018. But when you click on it it says 2022. Maybe they have been pushing commits to it until then. https://github.com/libsdl-org/SDL/tags?after=release-2.0.18 Edit: I found https://www.phoronix.com/news/SDL-2.0.9-Released confirming it's from 2018. |
JesseTG
left a comment
There was a problem hiding this comment.
Getting closer. SDL sensor driver works, minus a few small mistakes I've discovered. I might even try it on Windows for kicks.
The udev driver is another matter, as I still can't get sensor input working in it.
input/drivers/sdl_input.c
Outdated
| "[SDL]: SDL_GetNumTouchDevices: %d\n", | ||
| numTouchDevices=SDL_GetNumTouchDevices() | ||
| ); | ||
| if (!SDL_InitSubSystem(SDL_INIT_GAMECONTROLLER)) { |
There was a problem hiding this comment.
Two things here:
- Don't forget to initialize sensor support with
SDL_InitSubSystem(SDL_INIT_SENSOR). (Yes, it's in SDL2.) - As written, if
SDL_InitSubSystemfails then the issue is silently swallowed. Instead, this function should returnNULL, log the error, and clean up the object you already allocated. This way, the rest of RetroArch can handle the problem.
input/drivers/sdl_input.c
Outdated
| SDL_SensorGetName(sdl->auxiliary_devices[sdl->auxiliary_device_number].dev.sensor) | ||
| ); | ||
| sdl->auxiliary_device_number++; | ||
| sdl->auxiliary_devices=malloc(sizeof(sdl_input_auxiliary_device)*(numSensors+numSensors+numTouchDevices)); |
There was a problem hiding this comment.
Missed a spot; one of these numSensors should be a numJoysticks, which is now initialized a little later. (I noticed this because as written, this causes an immediate crash with my setup.)
input/input_driver.c
Outdated
| int remapped_port=settings->uints.input_sensor_index[port]; | ||
| int invert = (settings->uints.input_sensor_ids[port][id]%2)?-1:1; | ||
| unsigned remapped_input_sensor_id = settings->uints.input_sensor_ids[port][id]/2; |
There was a problem hiding this comment.
I just noticed this now (thanks, AddressSanitizer!), but a core that asks for RETRO_SENSOR_ILLUMINANCE (== 6) will cause undefined behavior, as there's no slot for inverting light sensor mappings. Which is fine, as they're a special case!
But special cases shouldn't risk crashes. Please make sure that this one is handled.
input/drivers/sdl_input.c
Outdated
| int numJoysticks=0,numTouchDevices=0,numSensors=0; | ||
| int i; int sensor_count=0; | ||
|
|
||
| SDL_InitSubSystem(SDL_INIT_GAMECONTROLLER); |
| if (numJoysticks > MAX_USERS) | ||
| numJoysticks=MAX_USERS; | ||
| for (i=0; i<numJoysticks; i++){ | ||
| SDL_GameController * gamepad=SDL_GameControllerOpen(i); |
There was a problem hiding this comment.
Don't forget to at least log the error that occurs if this fails.
input/input_driver.c
Outdated
| else if (id >= RETRO_SENSOR_GYROSCOPE_X && id <= RETRO_SENSOR_GYROSCOPE_Z) | ||
| sensitivity=settings->floats.input_sensor_gyroscope_sensitivity; | ||
| else | ||
| sensitivity=1.f; |
There was a problem hiding this comment.
This should be 0 to accommodate light sensors, because pow(anything, 0) == 1.0; the sensitivity settings for gyroscopes and accelerometers aren't relevant for light sensors, especially since they output different ranges. As written, this is returning incorrect values for the light sensor.
| return NULL; | ||
|
|
||
| if (port < MAX_INPUT_DEVICES) | ||
| dev_index = udev->sensors[port]; |
There was a problem hiding this comment.
On my system, dev->sensors is never updated with sensor indexes; all the values in this array are stuck at -1. This would explain why I can't get sensors with the udev driver working.
There was a problem hiding this comment.
IDK can you set a breakpoint near the for loop where the dev->sensors is set and see what's going on.
| goto error; | ||
| #endif | ||
|
|
||
| if (!open_devices(udev, UDEV_INPUT_SENSOR, udev_handle_sensor)) |
There was a problem hiding this comment.
It looks like ID_INPUT_ACCELEROMETER-typed devices aren't being added if there is no device node. This occurs with my Switch Pro controller. Here's some output from udevadm info:
(deck@steamdeck ~)$ udevadm info --query=all --name=/dev/input/event7 --name=/dev/input/event8 --no-pager
P: /devices/pci0000:00/0000:00:08.1/0000:04:00.3/usb1/1-1/1-1.2/1-1.2:1.0/0003:057E:2009.000F/input/input70/event7
M: event7
R: 7
U: input
D: c 13:71
N: input/event7
L: 0
S: input/by-path/pci-0000:04:00.3-usbv2-0:1.2:1.0-event-joystick
S: input/by-id/usb-Nintendo_Co.__Ltd._Pro_Controller_000000000001-event-joystick
S: input/by-path/pci-0000:04:00.3-usb-0:1.2:1.0-event-joystick
E: DEVPATH=/devices/pci0000:00/0000:00:08.1/0000:04:00.3/usb1/1-1/1-1.2/1-1.2:1.0/0003:057E:2009.000F/input/input70/event7
E: DEVNAME=/dev/input/event7
E: MAJOR=13
E: MINOR=71
E: SUBSYSTEM=input
E: USEC_INITIALIZED=87674495482
E: ID_INPUT=1
E: ID_INPUT_JOYSTICK=1
E: ID_BUS=usb
E: ID_MODEL=Pro_Controller
E: ID_MODEL_ENC=Pro\x20Controller
E: ID_MODEL_ID=2009
E: ID_SERIAL=Nintendo_Co.__Ltd._Pro_Controller_000000000001
E: ID_SERIAL_SHORT=000000000001
E: ID_VENDOR=Nintendo_Co.__Ltd.
E: ID_VENDOR_ENC=Nintendo\x20Co.\x2c\x20Ltd.
E: ID_VENDOR_ID=057e
E: ID_REVISION=0210
E: ID_TYPE=hid
E: ID_USB_MODEL=Pro_Controller
E: ID_USB_MODEL_ENC=Pro\x20Controller
E: ID_USB_MODEL_ID=2009
E: ID_USB_SERIAL=Nintendo_Co.__Ltd._Pro_Controller_000000000001
E: ID_USB_SERIAL_SHORT=000000000001
E: ID_USB_VENDOR=Nintendo_Co.__Ltd.
E: ID_USB_VENDOR_ENC=Nintendo\x20Co.\x2c\x20Ltd.
E: ID_USB_VENDOR_ID=057e
E: ID_USB_REVISION=0210
E: ID_USB_TYPE=hid
E: ID_USB_INTERFACES=:030000:
E: ID_USB_INTERFACE_NUM=00
E: ID_USB_DRIVER=usbhid
E: ID_PATH_WITH_USB_REVISION=pci-0000:04:00.3-usbv2-0:1.2:1.0
E: ID_PATH=pci-0000:04:00.3-usb-0:1.2:1.0
E: ID_PATH_TAG=pci-0000_04_00_3-usb-0_1_2_1_0
E: ID_FOR_SEAT=input-pci-0000_04_00_3-usb-0_1_2_1_0
E: LIBINPUT_DEVICE_GROUP=3/57e/2009:usb-0000:04:00.3-1
E: DEVLINKS=/dev/input/by-path/pci-0000:04:00.3-usbv2-0:1.2:1.0-event-joystick /dev/input/by-id/usb-Nintendo_Co.__Ltd._Pro_Controller_000000000001-event-joystick /dev/input/by-path/pci-0000:04:00.3-usb-0:1.2:1.0-event-joystick
E: TAGS=:uaccess:seat:
E: CURRENT_TAGS=:uaccess:seat:
P: /devices/pci0000:00/0000:00:08.1/0000:04:00.3/usb1/1-1/1-1.2/1-1.2:1.0/0003:057E:2009.000F/input/input71/event8
M: event8
R: 8
U: input
D: c 13:72
N: input/event8
L: 0
S: input/by-id/usb-Nintendo_Co.__Ltd._Pro_Controller_000000000001-event-if00
S: input/by-path/pci-0000:04:00.3-usb-0:1.2:1.0-event
S: input/by-path/pci-0000:04:00.3-usbv2-0:1.2:1.0-event
E: DEVPATH=/devices/pci0000:00/0000:00:08.1/0000:04:00.3/usb1/1-1/1-1.2/1-1.2:1.0/0003:057E:2009.000F/input/input71/event8
E: DEVNAME=/dev/input/event8
E: MAJOR=13
E: MINOR=72
E: SUBSYSTEM=input
E: USEC_INITIALIZED=87674495666
E: ID_INPUT=1
E: ID_INPUT_ACCELEROMETER=1
E: ID_INPUT_WIDTH_MM=15
E: ID_INPUT_HEIGHT_MM=15
E: ID_BUS=usb
E: ID_MODEL=Pro_Controller
E: ID_MODEL_ENC=Pro\x20Controller
E: ID_MODEL_ID=2009
E: ID_SERIAL=Nintendo_Co.__Ltd._Pro_Controller_000000000001
E: ID_SERIAL_SHORT=000000000001
E: ID_VENDOR=Nintendo_Co.__Ltd.
E: ID_VENDOR_ENC=Nintendo\x20Co.\x2c\x20Ltd.
E: ID_VENDOR_ID=057e
E: ID_REVISION=0210
E: ID_TYPE=hid
E: ID_USB_MODEL=Pro_Controller
E: ID_USB_MODEL_ENC=Pro\x20Controller
E: ID_USB_MODEL_ID=2009
E: ID_USB_SERIAL=Nintendo_Co.__Ltd._Pro_Controller_000000000001
E: ID_USB_SERIAL_SHORT=000000000001
E: ID_USB_VENDOR=Nintendo_Co.__Ltd.
E: ID_USB_VENDOR_ENC=Nintendo\x20Co.\x2c\x20Ltd.
E: ID_USB_VENDOR_ID=057e
E: ID_USB_REVISION=0210
E: ID_USB_TYPE=hid
E: ID_USB_INTERFACES=:030000:
E: ID_USB_INTERFACE_NUM=00
E: ID_USB_DRIVER=usbhid
E: ID_PATH_WITH_USB_REVISION=pci-0000:04:00.3-usbv2-0:1.2:1.0
E: ID_PATH=pci-0000:04:00.3-usb-0:1.2:1.0
E: ID_PATH_TAG=pci-0000_04_00_3-usb-0_1_2_1_0
E: LIBINPUT_DEVICE_GROUP=3/57e/2009:usb-0000:04:00.3-1
E: DEVLINKS=/dev/input/by-id/usb-Nintendo_Co.__Ltd._Pro_Controller_000000000001-event-if00 /dev/input/by-path/pci-0000:04:00.3-usb-0:1.2:1.0-event /dev/input/by-path/pci-0000:04:00.3-usbv2-0:1.2:1.0-event
(deck@steamdeck ~)$ udevadm info "/sys/devices/pci0000:00/0000:00:08.1/0000:04:00.3/usb1/1-1/1-1.2/1-1.2:1.0/0003:057E:2009.000F/input/input71" --no-pager
P: /devices/pci0000:00/0000:00:08.1/0000:04:00.3/usb1/1-1/1-1.2/1-1.2:1.0/0003:057E:2009.000F/input/input71
M: input71
R: 71
U: input
E: DEVPATH=/devices/pci0000:00/0000:00:08.1/0000:04:00.3/usb1/1-1/1-1.2/1-1.2:1.0/0003:057E:2009.000F/input/input71
E: SUBSYSTEM=input
E: PRODUCT=3/57e/2009/8111
E: NAME="Nintendo Co., Ltd. Pro Controller (IMU)"
E: PHYS="usb-0000:04:00.3-1.2/input0"
E: UNIQ="48:F1:EB:46:C3:1D"
E: PROP=40
E: EV=19
E: ABS=3f
E: MSC=20
E: MODALIAS=input:b0003v057Ep2009e8111-e0,3,4,kra0,1,2,3,4,5,m5,lsfw
E: USEC_INITIALIZED=87674495593
E: ID_INPUT=1
E: ID_INPUT_ACCELEROMETER=1
E: ID_BUS=usb
E: ID_MODEL=Pro_Controller
E: ID_MODEL_ENC=Pro\x20Controller
E: ID_MODEL_ID=2009
E: ID_SERIAL=Nintendo_Co.__Ltd._Pro_Controller_000000000001
E: ID_SERIAL_SHORT=000000000001
E: ID_VENDOR=Nintendo_Co.__Ltd.
E: ID_VENDOR_ENC=Nintendo\x20Co.\x2c\x20Ltd.
E: ID_VENDOR_ID=057e
E: ID_REVISION=0210
E: ID_TYPE=hid
E: ID_USB_MODEL=Pro_Controller
E: ID_USB_MODEL_ENC=Pro\x20Controller
E: ID_USB_MODEL_ID=2009
E: ID_USB_SERIAL=Nintendo_Co.__Ltd._Pro_Controller_000000000001
E: ID_USB_SERIAL_SHORT=000000000001
E: ID_USB_VENDOR=Nintendo_Co.__Ltd.
E: ID_USB_VENDOR_ENC=Nintendo\x20Co.\x2c\x20Ltd.
E: ID_USB_VENDOR_ID=057e
E: ID_USB_REVISION=0210
E: ID_USB_TYPE=hid
E: ID_USB_INTERFACES=:030000:
E: ID_USB_INTERFACE_NUM=00
E: ID_USB_DRIVER=usbhid
E: ID_PATH_WITH_USB_REVISION=pci-0000:04:00.3-usbv2-0:1.2:1.0
E: ID_PATH=pci-0000:04:00.3-usb-0:1.2:1.0
E: ID_PATH_TAG=pci-0000_04_00_3-usb-0_1_2_1_0
E: ID_FOR_SEAT=input-pci-0000_04_00_3-usb-0_1_2_1_0
E: TAGS=:seat:
E: CURRENT_TAGS=:seat:
(deck@steamdeck ~)$This isn't caused by your code, but is this something you think you'd be able to address? udev_device_get_devnode(udev_device_new_from_syspath(udev->udev, "/sys/devices/pci0000:00/0000:00:08.1/0000:04:00.3/usb1/1-1/1-1.2/1-1.2:1.0/0003:057E:2009.000F/input/input71")) is NULL. Maybe one of the other udev_device_new_from functions can help?
There was a problem hiding this comment.
I have no idea why this wouldn't work I think the steamdeck just doesn't like udev.
|
Please rebase / fix merge conflicts |
|
I gave up on this pull request, it's never getting merged, nobody wants this, my code is trash, and I should feel bad. |
|
Don't give up on this - get it merged, please! Lots of people want this support by default for controllers that support it. |
|
If anyone actually cared about this it wouldn't have sat unmerged for 2 years accumulating merge conflicts. |
I care 👍 I've made some PRs to repos before and they've sat there for a very long time too, I know exactly how that feels. Did you ever try a wiimote (I tried your PR and didn't work for my wiimote) and I note it does say in your notes wiimote is not supported yet. I encourage you to not give up, lets get it rebased on master and fixed up. |
|
I gave up on wiimote it works on a completely different udev system I need to figure out |
If you don't like the pow(2) bit you can delete it if you want.



Description
Implemented Sensor and Touchpad Support for SDL and udev.
Related Issues
#16162
Reviewers
@gouchi @warmenhoven
Known Issues with the pull request
Everything is hardcoded and not adjustable by the user. Which causes problems if an unrelated device such as a mouse takes device 0. Also if you swap the gamepadsOnly the axes are hardcoded now.SDL Device selection doesn't work.Axes are no longer hardcoded but they aren't saved for some reasonRequires UDEV_TOUCH_SUPPORT to be enabled.