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

Controller is not recognized as a haptic device with hotplugging #387

Open orbea opened 1 year ago

orbea commented 1 year ago

Version of xpadneo

https://github.com/atar-axis/xpadneo/commit/48f4b119fdef9479d5e45056dc7a234e8959af0d

Controller Model

Connection mode

Installed Software

N/A

Protocol Information

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

Severity / Impact

Describe the Bug

With SDL2 programs and maybe other programs the rumble does not work with some hotplugging under certain circumstances.

For example with the following test program created by the jgrf developer.

sdl2hp.c.txt

Build it:

cc sdl2hp.c -std=c99 sdl2-config --cflags --libs -o sdl2hp

  1. Run the program: ./sd2hp
  2. Turn on the controller.
  3. The controller physically rumbles while SDL2 believes it is not a haptic device and the rumble does not work in the program.
    $ ./sdl2hp
    Joystick Connected: Xbox 360 Controller
    Instance ID: 0, Player: 0
    Device is not haptic
    Ports: 1 0 0 0

And the second scenario.

  1. Turn on the controller.
  2. Start the program: ./sd2hp
  3. The rumble works.
  4. Turn off the controller and wait for it to disconnect.
  5. Turn the controller back on.
  6. The controller physically rumbles while SDL2 believes it is not a haptic device and the rumble does not work in the program.
    
    $ ./sdl2hp
    Joystick Connected: Xbox 360 Controller
    Instance ID: 0, Player: 0
    Force Feedback Enabled
    Ports: 1 0 0 0

Instance ID: 0 Joystick 0 Disconnected Ports: 0 0 0 0

Joystick Connected: Xbox 360 Controller Instance ID: 1, Player: 0 Device is not haptic Ports: 1 0 0 0

I have an official sony dualshock3 controller which fails in a with an different error message in the first scenario and works in the second scenario so I think this is potentially a driver bug rather than SDL2. Although possibly the test program and/or SDL is doing something wrong?

## Expected Behavior

The rumble should work with the hotplugging.

## Screenshots / GIFs / Videos

N/A

## System Information

```console
$ 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

There is a SDL2 hotplugging example repo here.

https://github.com/carmiker/sdl2-hotplug-example

kakra commented 1 year ago

This should not be a driver bug because the driver initializes with force feedback support enabled, the device should only become available after the rumble notification, and then the feedback driver will already be there. Could you try with module parameter ff_connect_notify=0 please?

orbea commented 1 year ago

I unloaded hid_xpadneo with rmmod and then reloaded it with modprobe hid_xpadneo ff_connect_notify=0.

Then I repeated both scenarios.

1.

$ ./sdl2hp
Joystick Connected: Xbox 360 Controller
    Instance ID: 0, Player: 0
Force Feedback Enable Failed: Haptic: Invalid haptic device identifier
Ports: 1 0 0 0

2.

$ ./sdl2hp
Joystick Connected: Xbox 360 Controller
    Instance ID: 0, Player: 0
Force Feedback Enabled
Ports: 1 0 0 0

Instance ID: 0
Joystick 0 Disconnected
Ports: 0 0 0 0

Joystick Connected: Xbox 360 Controller
    Instance ID: 1, Player: 0
Force Feedback Enabled
Ports: 1 0 0 0

This now matches the behavior with the sony dualshock3 controller where the error message is the same and the second scenario works correctly.

Potentially there are two different bugs in both xpadneo and SDL2? Or perhaps the test program is doing something wrong? I will make an SDL2 issue soon.

orbea commented 1 year ago

I made a SDL2 issue here. https://github.com/libsdl-org/SDL/issues/6250

kakra commented 1 year ago

Well, just because without the rumble notification the force feedback device becomes available sooner, doesn't mean there isn't a race condition in SDL: It should wait until the driver initialization settled. So, yes, xpadneo should probably not delay the initialization as long but SDL should be unaffected by whether it takes 9ms, 90ms, or 900ms: The appearance of such additional features is not atomic in Linux.

Without that rumble notification delay, we are actually doing it exactly the same way as hid-microsoft.

But this delay is a longstanding issue I'd like to avoid anyways. There's a plan on my agenda to fix it when introducing haptic feedback for mouse mode (and normal gamepad usage if someone wants to enable that instead of rumble).

That said, I think it's a racing bug in SDL although xpadneo could and should act faster here.

OTOH, the test program may need to consider looking for the haptic property again later - and thus SDL behavior may be totally valid. What happens if you delay checking SDL_JoystickIsHaptic(...) by 1000ms?

orbea commented 1 year ago

Yes, with this diff for the behavior for the test program now matches the sony dualshock3 controller.

--- a/sdl2hp.c
+++ b/sdl2hp.c
@@ -62,6 +62,8 @@ int main(int argc, char *argv[]) {
                         SDL_JoystickGetPlayerIndex(joystick[port])
                     );

+                    SDL_Delay(1000);
+
                     if (SDL_JoystickIsHaptic(joystick[port])) {
                         haptic[port] =
                             SDL_HapticOpenFromJoystick(joystick[port]);

So as you said xpadneo could improve this delay and SDL could fix this race condition. I will make a second related SDL issue soon.

orbea commented 1 year ago

I made a second SDL issue. https://github.com/libsdl-org/SDL/issues/6253