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.95k stars 112 forks source link

Xbox Wireless S controller mappings #273

Closed notgood closed 3 years ago

notgood commented 3 years ago

Version of xpadneo

v0.9-23-g6162dbc

Describe the bug

Several mapping issues:

  1. evtest reports 12 buttons, including KEY_RECORD which Xbox Wireless S controller (model 1708) doesn't have. xpad driver correctly reports 11 buttons.
  2. pressing Xbox button returns KEY_HOMEPAGE in xpadneo and BTN_MODE in xpad, not sure which is right
  3. Buttons BTN_WEST and BTN_NORTH are swapped, even though xpadneo isn't legacy driver and kernel docs says these shall be reported according to physical location. I see some old discussions in #1, #11, etc., but repeating broken xpad behaviour is still wrong.

https://www.kernel.org/doc/html/latest/input/gamepad.html

4-Button Pad: If all 4 action-buttons are present, they can be aligned in two different formations. If diamond-shaped, they are reported as BTN_NORTH, BTN_WEST, BTN_SOUTH, BTN_EAST according to their physical location.

System information

# uname -a
Linux hydra 5.8.0-44-generic #50-Ubuntu SMP Tue Feb 9 06:29:41 UTC 2021 x86_64 x86_64 x86_64 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 0c 15 00 25 01 75 01 95 0c 81 02 15 00 25 00  ....)...%.u.......%.
000000a0: 75 01 95 04 81 03 05 0c 0a 24 02 15 00 25 01 95 01 75 01 81  u........$...%...u..
000000b4: 02 15 00 25 00 75 07 95 01 81 03 05 0c 09 01 85 02 a1 01 05  ...%.u..............
000000c8: 0c 0a 23 02 15 00 25 01 95 01 75 01 81 02 15 00 25 00 75 07  ..#...%...u.....%.u.
000000dc: 95 01 81 03 c0 05 0f 09 21 85 03 a1 02 09 97 15 00 25 01 75  ........!........%.u
000000f0: 04 95 01 91 02 15 00 25 00 75 04 95 01 91 03 09 70 15 00 25  .......%.u......p..%
00000104: 64 75 08 95 04 91 02 09 50 66 01 10 55 0e 15 00 26 ff 00 75  du......Pf..U...&..u
00000118: 08 95 01 91 02 09 a7 15 00 26 ff 00 75 08 95 01 91 02 65 00  .........&..u.....e.
0000012c: 55 00 09 7c 15 00 26 ff 00 75 08 95 01 91 02 c0 05 06 09 20  U..|..&..u......... 
00000140: 85 04 15 00 26 ff 00 75 08 95 01 81 02 c0                    ....&..u......
3560615638 1558
# dmesg
[<   47,413753>] xpadneo 0005:045E:02FD.000C: report descriptor size: 335 bytes
[<    0,000004>] xpadneo 0005:045E:02FD.000C: fixing up report descriptor size
[<    0,000001>] xpadneo 0005:045E:02FD.000C: fixing up Rx axis
[<    0,000002>] xpadneo 0005:045E:02FD.000C: fixing up Ry axis
[<    0,000001>] xpadneo 0005:045E:02FD.000C: fixing up Z axis
[<    0,000001>] xpadneo 0005:045E:02FD.000C: fixing up Rz axis
[<    0,000001>] xpadneo 0005:045E:02FD.000C: fixing up button mapping
[<    0,005525>] xpadneo 0005:045E:02FD.000C: battery detected
[<    0,000007>] xpadneo 0005:045E:02FD.000C: pretending XB1S Windows wireless mode (changed PID from 0x02FD to 0x02E0)
[<    0,000002>] xpadneo 0005:045E:02FD.000C: enabling compliance with Linux Gamepad Specification
[<    0,000116>] input: Xbox Wireless Controller as /devices/pci0000:00/0000:00:12.2/usb1/1-4/1-4.1/1-4.1:1.0/bluetooth/hci0/hci0:68/0005:045E:02FD.000C/input/input38
[<    0,000380>] xpadneo 0005:045E:02FD.000C: input,hidraw4: BLUETOOTH HID v9.03 Gamepad [Xbox Wireless Controller] on 00:1a:7d:da:71:13
[<    0,000006>] xpadneo 0005:045E:02FD.000C: controller quirks: 0x00000010
[<    0,000001>] xpadneo xpadneo_welcome_rumble start
[<    0,982204>] xpadneo xpadneo_welcome_rumble took 980ms
[<    0,000034>] xpadneo 0005:045E:02FD.000C: Xbox Wireless Controller [5c:ba:37:6e:82:e1] connected
[<    0,220997>] xpadneo 0005:045E:02FD.000C: battery registered
# evtest
Input driver version is 1.0.1
Input device ID: bus 0x5 vendor 0x45e product 0x2e0 version 0x903
Input device name: "Xbox Wireless Controller"
Supported events:
  Event type 0 (EV_SYN)
  Event type 1 (EV_KEY)
    Event code 167 (KEY_RECORD)
    Event code 172 (KEY_HOMEPAGE)
    Event code 304 (BTN_SOUTH)
    Event code 305 (BTN_EAST)
    Event code 307 (BTN_NORTH)
    Event code 308 (BTN_WEST)
    Event code 310 (BTN_TL)
    Event code 311 (BTN_TR)
    Event code 314 (BTN_SELECT)
    Event code 315 (BTN_START)
    Event code 317 (BTN_THUMBL)
    Event code 318 (BTN_THUMBR)
  Event type 3 (EV_ABS)
    Event code 0 (ABS_X)
      Value   1238
      Min   -32768
      Max    32767
      Fuzz      32
      Flat    3072
    Event code 1 (ABS_Y)
      Value    999
      Min   -32768
      Max    32767
      Fuzz      32
      Flat    3072
    Event code 2 (ABS_Z)
      Value      0
      Min        0
      Max     1023
      Fuzz       4
    Event code 3 (ABS_RX)
      Value   2036
      Min   -32768
      Max    32767
      Fuzz      32
      Flat    3072
    Event code 4 (ABS_RY)
      Value   1664
      Min   -32768
      Max    32767
      Fuzz      32
      Flat    3072
    Event code 5 (ABS_RZ)
      Value      0
      Min        0
      Max     1023
      Fuzz       4
    Event code 16 (ABS_HAT0X)
      Value      0
      Min       -1
      Max        1
    Event code 17 (ABS_HAT0Y)
      Value      0
      Min       -1
      Max        1
    Event code 40 (ABS_MISC)
      Value      0
      Min    -1023
      Max     1023
      Fuzz       3
      Flat      63
  Event type 4 (EV_MSC)
    Event code 4 (MSC_SCAN)
  Event type 21 (EV_FF)
    Event code 80 (FF_RUMBLE)
    Event code 81 (FF_PERIODIC)
    Event code 88 (FF_SQUARE)
    Event code 89 (FF_TRIANGLE)
    Event code 90 (FF_SINE)
    Event code 96 (FF_GAIN)
kakra commented 3 years ago
  • evtest reports 12 buttons, including KEY_RECORD which Xbox Wireless S controller (model 1708) doesn't have. xpad driver correctly reports 11 buttons.

Actually, the controllers only have 10 joystick buttons. Additional buttons belong to a sub device and should not be visible to games as part of the gamepad device. A patch is sitting in my patch queue which addresses this and is meant to finally fix that. But we may finally go back to reporting 11 buttons if the subdevice split prevents messed up mappings for different games.

  • pressing Xbox button returns KEY_HOMEPAGE in xpadneo and BTN_MODE in xpad, not sure which is right

The current situation is that games expect button positions 1-10 to be the original gaming buttons, and if button 11 exists, it must be the guide button. But that is not true in the Linux kernel as it orders BTN_MODE before position 10. This messes up some games and older SDL implementations.

So we worked around that and supported even old titles, wine games, etc by mapping BTN_MODE out of the 1-10 range. But exposing it as KEY_HOMEPAGE makes the controller partially a keyboard, or more exactly, that key code is part of consumer control devices.

Luckily, the HID implementation of the controllers actually split their mappings into gamepad, consumer control, and keyboard already, we just ignored that since beginning of time. Button 11 is, depending on the controller model, part of the gamepad, the consumer control, or both. With the patch we are going with 10 gamepad buttons, the additional buttons are mapped to consumer control codes.

  • Buttons BTN_WEST and BTN_NORTH are swapped, even though xpadneo isn't legacy driver and kernel docs says these shall be reported according to physical location. I see some old discussions in #1, #11, etc., but repeating broken xpad behaviour is still wrong.

Well, but we are NOT reporting BTN{SOUTH,EAST,NORTH,WEST}, the driver is actually reporting BTN{A,B,X,Y}. It just happens that Linux is using aliased codes for both symbols. Games expect A,B,X,Y and not the cardinal directions. We have named buttons. The cardinal direction symbols are just there for gamepads which have unnamed/anonymous buttons, or something that doesn't fit one of the other naming schemes. The event codes for those buttons match the original Nintendo gamepad, so it looks like X and Y are swapped, and A,B would be swapped on the original Nintendo gamepad. You can force the modern Nintendo mapping with a quirk flag, but you'll probably notice that A,B and X,Y become swapped in games.

I don't see we are doing anything wrong here. evtest is probably wrong in reporting the cardinal direction symbols although we are using a gamepad with named buttons.

Actually, I think the kernel documents about this were not tested enough because the aliasing of A,B,X,Y and cardinal directions matches neither with the Xbox mapping, nor with the Nintendo mapping. So I consider the code mapping of the cardinal direction symbols broken, they should not be used, and the reasoning about this in the kernel docs is obsolete: The document was created Apr 2017 and written in a way to match the quirky and outdated position layout of event code maps for old digital-axis gamepads in the kernel. The code it is based on is by far older, from a time when hardware manufacturers didn't yet settle on a common scheme for naming buttons (A,B,C,X,Y,Z for up to 6 buttons). Today, everyone is using BTN{A,B,C,X,Y,Z} in their code, which aliases to BTN{SOUTH,EAST,nothing,NORTH,WEST,nothing} - but that aliasing is wrong for both Xbox and Nintendo.

The ASCII art gamepad in the docs resembles a DualShock controller. Using cardinal direction symbols for these works better because the buttons there use shapes instead of alphabet symbols for the buttons. We do not (yet) support these controllers.

Or IOW, there are two types of gamepad layouts in the same event code range in the kernel, one is for gamepads with alphabetic naming scheme, the other for shaped symbols scheme or no names at all.

TL;DR - We use alphabet symbols for the Xbox layout as expected by games. evtest is actually wrong in reporting the cardinal direction names for you. It works correctly, if you map evtest back to the correct naming scheme:

evtest | sed 's/BTN_SOUTH/BTN_A/;s/BTN_EAST/BTN_B/;s/BTN_NORTH/BTN_X/;s/BTN_WEST/BTN_Y/'

See also here to verify that sed command is correct: https://github.com/torvalds/linux/blob/master/include/uapi/linux/input-event-codes.h#L379

kakra commented 3 years ago

Additionally, the HID descriptor of the controller jumps through hoops and loops to get the positional order of buttons and axes right when it detects being connected to Linux. From #1 and #11 you can read that the controller has two modes and can (with older firmware versions) be tricked into using Windows mode when connected to Linux. In that mode, the HID descriptor looks different and shows Windows HID codes that match symbols BTN_A,B,C,X (0x130-0x133) and axes ABS_X,Y,Z,RX,RY,RZ for left stick (X,Y), right stick (Z,RX), left trigger (RY), right trigger (RZ) - which is clearly wrong with respect to code mapping in Linux. So MS went with more buttons in Linux mode (15) and then just never send the button bits at position BTN_C (and some more), and also moved reporting of the axes into left stick (X,Y), left trigger (Z), right stick (RX,RY), right trigger (RZ).

But this breaks the joydev API which sees 15 buttons now, 4 of them being blind/dead buttons, and also it doesn't properly map to the axis positions of the sticks and triggers as joydev expects it. But this is a minor issue as joydev is mostly outdated and games use evdev today.

But evdev is also sort-of-broken: It sees 15 buttons with 4 of them being dead, but the remaining 11 buttons at least map to their correct symbolic names. And the axes are all correct now in evdev. So evdev games would work. So in the end, evdev API is a little quirky but correct. Fine.

But now comes SDL and wine: Both these implementations did not care about evdev symbols at all but just counted buttons. Xbox controllers are expected to have button 1-10 as A,B,X,Y,... (without BTN_MODE being part of the first 10 buttons). This is completely broken with the evdev mapping as that reports 15 buttons, and the 10 first buttons already contain dead buttons from this set of 15 buttons. Also, BTN_MODE is within the range of the first 10 button codes, so it messes up the expected positions of LS,RS,Menu,Guide. Actually, there are not even symbols in Linux evdev gamepad codes that would match the symbols of the center buttons of the Xbox controllers but there are in the consumer control maps. But common sense is that the left center button is BTN_SELECT, the right one is BTN_START. And that's it, there is no button 11 in Windows for gamepads. So we do not report the Guide button as part of our mappings.

So I conclude the following from your report:

evtest should report 10 buttons for your gamepad, not 12. This should be fixed by a patch sitting in my patch queue.

Extra buttons should go to subdevices, that will be share, guide, and probably other buttons that MS may invent. That above mentioned patch will take care of it but we are currently not able to limit the reported buttons to that of the exact model: That'll need another patch. So currently, we will report all buttons that could probably emerge in different models.

Given the fact that modern Xbox controllers report a full keyboard with 127 "buttons" in the keyboard subdevice tells me that we should probably not care about filtering buttons in subdevices at all, rendering the need of a "filter by model" patch as outlined above futile: These controllers have ability to remap buttons to other events like keyboard presses in hardware. I'll go with an emulated software layer instead which can do the same, so you would be able to map buttons to keyboard events later. But I'll have to reverse engineer the hardware protocol first, so our emulation layer for older controllers matches the hardware implementation of the modern models. Implementation has already started: You can switch mapping profiles on emulated controller models by holding the guide button and pressing A,B,X,Y to switch between 4 profiles (watch with dmesg -w). But that's all it does yet.

kakra commented 3 years ago

To use my patch queue, go to your git clone of xpadneo and run:

git remote add kakra https://github.com/kakra/xpadneo.git
git remote update

Then checkout the queue 0.10 branch. It's not part of atar-axis as I'll rebase that branch often so you cannot follow it with a simple git pull. I'm running this branch locally, then cherry-pick commits into a "tested" branch (not public) which I'll push to atar-axis master.

kakra commented 3 years ago

Does current master change behavior for you?

notgood commented 3 years ago

Thank you for detailed answers!

Sorry for the late reply, I'll get to test changes in a day or two and will post then.

notgood commented 3 years ago

Retested with master commit b827510

evtest should report 10 buttons for your gamepad, not 12. This should be fixed by a patch sitting in my patch queue.

Confirming, evdev input device "Xbox Wireless Controller" is now reporting 10 buttons, nice!

Extra buttons should go to subdevices, that will be share, guide, and probably other buttons that MS may invent. That above mentioned patch will take care of it but we are currently not able to limit the reported buttons to that of the exact model: That'll need another patch. So currently, we will report all buttons that could probably emerge in different models. Given the fact that modern Xbox controllers report a full keyboard with 127 "buttons" in the keyboard subdevice tells me that we should probably not care about filtering buttons in subdevices at all, rendering the need of a "filter by model" patch as outlined above futile

Separate input device "Xbox Wireless Controller Consumer Control" is now reporting two buttons: 373 (KEY_MODE) and 167 (KEY_RECORD). There is no KEY_RECORD on model 1708, but I agree that there isn't much benefit in trying to filter out non-existing keys on keyboard devices.

Well, but we are NOT reporting BTN{SOUTH,EAST,NORTH,WEST}, the driver is actually reporting BTN{A,B,X,Y}....

Quite exciting article about gamepad buttons https://gamecrate.com/just-fix-your-controller-layout-already-history-where-heck-x-button/23875 Seems to be a huge historical mess, with all the marketing and mentality differences. And then with all the different OS's and API's, there no one true way to fix it once for all :) I mean, Linux kernel docs described SOUTH/EAST/WEST/NORTH approach seems to be the most logical and neutral one to me. But if in the same time kernel makes assumptions and aliases button locations to letters, there is no point going against the flow I guess.

But as long there is (or will be) the way to remap using profiles on driver/hardware level, it solves this problem, at least for xpadneo supported controllers.

You can switch mapping profiles on emulated controller models by holding the guide button and pressing A,B,X,Y to switch between 4 profiles (watch with dmesg -w). But that's all it does yet.

Yup, getting profile switching messages in dmesg, will gladly test when it does more.

I really hope xpadneo will get mainlined one day, nice job!

kakra commented 3 years ago

Quite exciting article about gamepad buttons https://gamecrate.com/just-fix-your-controller-layout-already-history-where-heck-x-button/23875

Not available in Europe... Nah... But their blocking message is quite dumb: I could still scroll down to the (just dimmed and faded) article and read it, interestingly without any ads.

Seems to be a huge historical mess, with all the marketing and mentality differences. And then with all the different OS's and API's, there no one true way to fix it once for all :)

Yes, it is a mess. xpadneo decided to go with symbolic buttons instead of positional buttons: That matches what games expect and do, and it better matches with what games display since they usually expect an Xbox controller and thus use symbols A,B,X,Y on screen. Thus, there's a patch which actually supports the 8BitDo controllers (which have the Nintendo positions of symbols) and swaps the buttons, so games would still map A,B,X,Y correctly. But that got some loud complaints here in the community so I reverted it: People prefer using the positions they know for their games instead of the correct symbols. But if we were actually going by position as the kernel suggests it, we would have X and Y swapped (while A and B is still correct for some crazy reason).

The article you were linking talks about the first mess. There's a second mess in the kernel which does neither the one nor the other mapping correct. To me it seems like it was "invented" for the sake of some ideology, without looking at real controllers in the market. Ideology is often a problem in the Linux community - it's leads to fragmentation and concepts are stuck in the past.

With that said: If you want to swap A,B and X,Y to their real positions on a Nintendo style remake controller which is Xbox compatible (most 8BitDo controllers in XInput mode), there's a quirk flag for the xpadneo module to actually do that - but it's no longer enabled by default. I may turn it back on when we have mapping profiles in place, maybe shipping a different default profile for 8BitDo. But as a final result I'd like to stay with the "our buttons map to symbols" approach - as this is what every sane game developer does in their code: They write BTN_Y and not BTN_NORTH (while in the kernel it maps to BTN_WEST anyways).

kakra commented 3 years ago

@notgood Are there issues left or can we close this?

notgood commented 3 years ago

No issues left, thank you!

Closing.

kakra commented 3 years ago

I want to keep that opened until the pull request is merged. Thanks for the feedback. I'll convert the commit message to a "Fixes" tag then.

kakra commented 3 years ago

Hmm, nah, let's close this instead. The PR actually doesn't fix this but may affect behavior. I'll close it again. Still, thanks for the feedback, tho.