atar-axis / xpadneo

Advanced Linux Driver for Xbox One Wireless Controller (shipped with Xbox One S)
https://atar-axis.github.io/xpadneo/
GNU General Public License v3.0
1.87k stars 110 forks source link

Issues with the extra trigger axes and the SDL_Joystick API #385

Closed orbea closed 4 months ago

orbea commented 1 year ago

Version of xpadneo

https://github.com/kakra/linux/pull/15

Controller Model

Connection mode

Installed Software

N/A

Protocol Information

Please help us identify at which layer the problem can be found if you want to report mapping errors or if the controller fails to be detected:

Please describe how it is failing below in the next sections.

Severity / Impact

Describe the Bug

In some SDL2 and maybe other programs with their own input config they will erroneously configure the extra rudder (?) axes instead of the axes for the trigger.

For example L2 will activate axes 2+ and axes 6- while R2 has axes 5+ and 6+ where SDL2 will see the extra axes 6 events before the real L2/R2 events for axes 2 and 5. This then will cause runtime issues for the game.

Steps to Reproduce

For example this can be reproduced with the jgrf frontend (https://gitlab.com/jgemu/jgrf) which requires the jg headers (https://gitlab.com/jgemu/jg) and the mednafen core (https://gitlab.com/jgemu/mednafen) which supports L2 and R2 input with any playstation 1 games. This gentoo overlay (https://0xacab.org/orbea/jgemu) may speed up the process.

  1. Load any PSX games: jollygood /path/to/file.cue
  2. Press shift+1 to configure the first input device.
  3. View the generated config in ~/.config/jollygood/input_psx.ini.

Note that jgrf has added a hack to avoid this issue, but there is probably a better way.

https://gitlab.com/jgemu/jgrf/-/commit/bb349aa2eb532b57524e8a374515b9ef8172aa72

Expected Behavior

Would it be possible to switch the order of events so that SDL2 will see axes 2 and 5 before axes 6? Or is there a good reason this can't be done? If so are there any suggested ways for the program to avoid this issue? In these cases the program needs the SDL_Joystick API and the SDL_GameController API is not appropriate.

Screenshots / GIFs / Videos

jstest (No input) xpad

jstest (L2 held down) xpad-l2-down

jstest (L2 released) xpad-l2-up

jstest (R2 held down) xpad-r2-down

jstest (R2 released) xpad-r2-up

System Information

$ uname -a
Linux gentoo 5.15.64-x86_64 #1 SMP PREEMPT Thu Sep 1 15:17:19 PDT 2022 x86_64 Intel(R) Core(TM) i5-7600K CPU @ 3.80GHz GenuineIntel GNU/Linux
# xxd -c20 -g1 /sys/module/hid_xpadneo/drivers/hid:xpadneo/0005:045E:*/report_descriptor | tee >(cksum)
00000000: 05 01 09 05 a1 01 85 01 09 01 a1 00 09 30 09 31 15 00 27 ff  .............0.1..'.
00000014: ff 00 00 95 02 75 10 81 02 c0 09 01 a1 00 09 33 09 34 15 00  .....u.........3.4..
00000028: 27 ff ff 00 00 95 02 75 10 81 02 c0 05 01 09 32 15 00 26 ff  '......u.......2..&.
0000003c: 03 95 01 75 0a 81 02 15 00 25 00 75 06 95 01 81 03 05 01 09  ...u.....%.u........
00000050: 35 15 00 26 ff 03 95 01 75 0a 81 02 15 00 25 00 75 06 95 01  5..&....u.....%.u...
00000064: 81 03 05 01 09 39 15 01 25 08 35 00 46 3b 01 66 14 00 75 04  .....9..%.5.F;.f..u.
00000078: 95 01 81 42 75 04 95 01 15 00 25 00 35 00 45 00 65 00 81 03  ...Bu.....%.5.E.e...
0000008c: 05 09 19 01 29 0a 15 00 25 01 75 01 95 0a 81 02 15 00 25 00  ....)...%.u.......%.
000000a0: 75 06 95 01 81 03 05 01 09 80 85 02 a1 00 09 85 15 00 25 01  u.................%.
000000b4: 95 01 75 01 81 02 15 00 25 00 75 07 95 01 81 03 c0 05 0f 09  ..u.....%.u.........
000000c8: 21 85 03 a1 02 09 97 15 00 25 01 75 04 95 01 91 02 15 00 25  !........%.u.......%
000000dc: 00 75 04 95 01 91 03 09 70 15 00 25 64 75 08 95 04 91 02 09  .u......p..%du......
000000f0: 50 66 01 10 55 0e 15 00 26 ff 00 75 08 95 01 91 02 09 a7 15  Pf..U...&..u........
00000104: 00 26 ff 00 75 08 95 01 91 02 65 00 55 00 09 7c 15 00 26 ff  .&..u.....e.U..|..&.
00000118: 00 75 08 95 01 91 02 c0 85 04 05 06 09 20 15 00 26 ff 00 75  .u........... ..&..u
0000012c: 08 95 01 81 02 c0                                            ......
3445511648 1458

Controller and Bluetooth Information

xpadneo-btmon.txt xpadneo-dmesg.txt xpadneo-lsusb.txt

Additional Context

N/A

orbea commented 1 year ago

The jgemu developer made a simple test program that better shows this.

test.c.txt

Build with: gcc $(pkg-config --cflags --libs sdl2) test.c

And this reveals that the above is slightly wrong, L2 always (?) has the event for axis 2 first while R2 has axis 6.

L2

$ ./a.out 
Joystick Connected: Xbox One S Controller
    Instance ID: 0, Player: 0
Ports: 1 0 0 0

Axis 2
Axis 2
Axis 6
Axis 6
Axis 2
Axis 6
Axis 2
Axis 6
Axis 2
Axis 6
Axis 2
Axis 6

R2

$ ./a.out 
Joystick Connected: Xbox One S Controller
    Instance ID: 0, Player: 0
Ports: 1 0 0 0

Axis 6
Axis 6
Axis 5
Axis 5
Axis 6
Axis 5
Axis 6
Axis 5
Axis 6
Axis 5
Axis 6
Axis 5

This order seems pretty consistent, but I am not sure if it is always so?

kakra commented 1 year ago

The joydev devices always order axes by their event number - which is wrong for HID controllers as it doesn't match the original expectation of joydev (I'd say, that's a flaw internally in the kernel input layers). You can use jscal to re-order the axes but that confuses programs that already fix the broken behavior internally. OTOH, joydev is an old API and should not be used any longer.

I'll probably remove the extra trigger axis with the next update, or at least change its behavior to either only send the non-combined values, or the combined value - but not both.

orbea commented 1 year ago

I'll probably remove the extra trigger axis with the next update, or at least change its behavior to either only send the non-combined values, or the combined value - but not both.

Maybe an option could be added to enable the double axis which would be disabled by default in case someone really needs that? I'm not sure if that is worthwhile.

kakra commented 1 year ago

@orbea Can you retry with latest master?

orbea commented 1 year ago

@kakra Given that I have xpadneo built into the kernel I am not sure the best route to updating it?

kakra commented 1 year ago

If you've built it into the kernel as a module, you can just re-install from this git repo. I'm doing it the same way, haven't updated my kernel patch yet.

You can run make -C hid-xpadneo reinstall from the repository root, should work out-of-the-box with Gentoo. It will also do the module reloading. Run as user, it will automatically ask for sudo permissions.

orbea commented 1 year ago

That was easier than I imagined...

However I see no difference in behavior?

This is with commit 48f4b119fdef9479d5e45056dc7a234e8959af0d and I built it following the generic build instructions.

cd hid-xpadneo && make modules && sudo make modules_install

I explicitly unloaded the xpadneo module with rmmod and manually removed the old module to make sure it is the new module installed.

kakra commented 1 year ago

However I see no difference in behavior?

Okay, I was hoping the PID change may change how SDL behaves for you. So I still need to fix the axis thing. BTW: I updated my previous commit with instructions how I reinstall the module during development.

kakra commented 1 year ago

@orbea Could you check if this is still an issue with the latest versions of SDL2 and xpadneo? If you cannot use SDL2 release candidates, you'd have to wait for SDL2 2.28.