Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add Xbox Series X controller bluetooth profile #5406

Closed
wants to merge 1 commit into from

Conversation

jelly
Copy link

@jelly jelly commented Mar 14, 2022

My Xbox Series X controller has a different guid then the existsing SDL entry which means the buttons are mappend wrong.

Description

My Xbox series X controller runs the latest firmware and the only difference is the last part of the guid:

Existing entry's guid: 050000005e040000130b000011050000
My guid: 050000005e040000130b000009050000

This mapping requires a kernel patch to be applied as well as it fixes the mapping of some axes so they are correctly displayed in evemu-record. I haven't sent the patch upstream yet as it most likely will break the existing SDL entry and I would love to figure out what the difference is between the existing and my guid.

@cgutman
Copy link
Collaborator

cgutman commented Mar 16, 2022

The kernel patch might be dicey if you're breaking other userspace applications that expect the old mapping.

What happens without the patch?

@jelly
Copy link
Author

jelly commented Mar 16, 2022

The kernel patch might be dicey if you're breaking other userspace applications that expect the old mapping.

What happens without the patch?

The axes are mapped incorrectly currently from a Linux point of view. The driver could in theory have a module option to retain the old behaviour but that does not sound ideal either. I can't really test the difference here for the existing binding, do you know why the guid is different?

@cgutman
Copy link
Collaborator

cgutman commented Mar 17, 2022

The axes are mapped incorrectly currently from a Linux point of view.

Can you elaborate? Which axes? What is your expected mapping vs the actual mapping?

do you know why the guid is different?

It looks like that's the version part of the GUID (likely related to the firmware of the controller).

/* We only need 16 bits for each of these; space them out to fill 128. */
/* Byteswap so devices get same GUID on little/big endian platforms. */
*guid16++ = SDL_SwapLE16(inpid.bustype);
*guid16++ = 0;
if (inpid.vendor && inpid.product) {
*guid16++ = SDL_SwapLE16(inpid.vendor);
*guid16++ = 0;
*guid16++ = SDL_SwapLE16(inpid.product);
*guid16++ = 0;
*guid16++ = SDL_SwapLE16(inpid.version);
*guid16++ = 0;

What firmware version is your controller running?

@jelly
Copy link
Author

jelly commented Mar 18, 2022

The axes are mapped incorrectly currently from a Linux point of view.

Can you elaborate? Which axes? What is your expected mapping vs the actual mapping?

The right trigger is mapped to ABS_GAS which is for flight sim joysticks
The left trigger is mapped to ABS_BRAKE which is also for flight sim/racing joysticks. When connecting over USB this is mapped as ABS_Z/ABS_RZ which the userland expects for normal controllers.

The right joystick is mapped to ABS_Z/RZ while it should be the right X/Y axes. The kernel driver corrects this.

So without my kernel driver and profile, SDL maps the right trigger to the right joystick's Y axis:

image

The left trigger is mapped to the right trigger and the right joystick axes are also incorect.

@cgutman
Copy link
Collaborator

cgutman commented Mar 19, 2022

The right trigger is mapped to ABS_GAS which is for flight sim joysticks. The left trigger is mapped to ABS_BRAKE which is also for flight sim/racing joysticks.

Using ABS_GAS and ABS_BRAKE for triggers and ABS_Z and ABS_RZ for the right stick is quite standard in the Android world. It is even documented in the Android gamepad docs. This was very likely a conscious choice by Microsoft to be compatible with Android conventions (and therefore, existing Android games too).

I don't think it's a good idea to try to artificially change that. It creates a scenario where certain kernels will have one mapping and other kernels will have a different mapping, causing havoc for userspace. Consider SDL itself, where the mapping GUID for patched and non-patched kernels would be identical, and we couldn't tell which mapping to use.

It also risks causing behavior to change unexpectedly when Microsoft releases a new firmware (if the fixup is only done for certain firmware versions), or potentially interfering with legitimate HID descriptor changes (if the fixup is done for all firmware versions).

So without my kernel driver and profile, SDL maps the right trigger to the right joystick's Y axis:

We can probably improve SDL's heuristic in LINUX_JoystickGetGamepadMapping() to account for these "Android-style" gamepads that use ABS_BRAKE, ABS_GAS, or ABS_THROTTLE instead of ABS_Z and ABS_RZ.

@slouken
Copy link
Collaborator

slouken commented Mar 19, 2022

We can probably improve SDL's heuristic in LINUX_JoystickGetGamepadMapping() to account for these "Android-style" gamepads that use ABS_BRAKE, ABS_GAS, or ABS_THROTTLE instead of ABS_Z and ABS_RZ.

Yep, that's a great idea.

@slouken slouken added this to the 2.28.0 milestone Nov 23, 2022
@slouken slouken self-assigned this Nov 23, 2022
@slouken slouken modified the milestones: 2.28.0, 3.x Mar 7, 2023
@smcv
Copy link
Contributor

smcv commented Apr 12, 2023

It would be useful to add test data for this Xbox Series X controller using an unpatched kernel (in Bluetooth mode, and also in USB mode if different) to test/testevdev.c, to make sure our heuristic for recognising gamepads works correctly for it.

The easiest thing to convert into testevdev format is the output of evemu-describe /dev/input/eventX for each device node eventX associated with this controller.

Another thing that can be useful when investigating new controllers is the steam-runtime-input-monitor tool that comes with Steam, which will list all the device nodes associated with the controller (including any raw HID device nodes, accelerometers, gyros etc. that it might have). The easiest way to use that is to run

~/.steam/root/ubuntu12_32/steam-runtime/run.sh steam-runtime-input-monitor

with this device not connected, and wait for it to finish describing all your other devices (it should say {"all-for-now": true} at the end). Then connect this device, and copy/paste whatever new information is displayed after the {"all-for-now": true} marker. Press Ctrl+C to stop the input monitor when it has finished displaying the new device(s).

smcv added a commit to smcv/SDL that referenced this pull request Jun 9, 2023
This heuristic for gamepads without a more specific mapping already
tried two incompatible conventions for handling triggers: the Linux
Gamepad Specification uses hat switch 2 for the triggers (for whatever
reason), but the de facto standard set by the drivers for older Xbox
and Playstation controllers represents each trigger as the Z-axis of
the nearest analog stick.

Android documentation encourages Bluetooth gamepad manufacturers to use
a third incompatible convention where the left and right triggers are
represented as the brake and gas pedals of a driving simulator
controller. The Android convention also changes the representation of
the right stick: instead of using X and Y rotation as a second pair
of axes, Android uses Z position as a second horizontal axis, and
Z rotation as a second vertical axis.

Try to cope gracefully with all of these. This will hopefully resolve
the issue described in libsdl-org#5406 (when using unpatched kernels).

Signed-off-by: Simon McVittie <smcv@collabora.com>
@smcv
Copy link
Contributor

smcv commented Jun 9, 2023

It would be useful to add test data for this Xbox Series X controller using an unpatched kernel (in Bluetooth mode, and also in USB mode if different) to test/testevdev.c, to make sure our heuristic for recognising gamepads works correctly for it.

#7791 added test data for a "Microsoft Xbox Series S|X Controller model 1914" which is a slightly different device: the one my colleague examined is reported as product 0b12 version 050f via USB or version 0515 via Bluetooth, while @jelly's is reported as product 0b13 version 0509 via Bluetooth (and an unknown product/version via USB).

My colleague's device has different input mappings via USB and Bluetooth. Via USB, it has the typical Xbox-360-style axes (X, Y, Z, RX, RY, RZ), which are presumably X/Y for the left stick, RX/RY for the right stick, Z for the left trigger and RZ for the right trigger, but via Bluetooth, it has X, Y, Z, RZ, GAS and BRAKE, which are presumably the Android-style layout that @cgutman describes.

@jelly's comments suggest that the same could be true for their controller. It would be useful to confirm that via evemu-describe.

We can probably improve SDL's heuristic in LINUX_JoystickGetGamepadMapping() to account for these "Android-style" gamepads that use ABS_BRAKE, ABS_GAS, or ABS_THROTTLE instead of ABS_Z and ABS_RZ.

I think that would be a better solution than merging this PR, particularly if this PR requires a patched kernel. I've proposed #7797.

slouken pushed a commit that referenced this pull request Jun 13, 2023
This heuristic for gamepads without a more specific mapping already
tried two incompatible conventions for handling triggers: the Linux
Gamepad Specification uses hat switch 2 for the triggers (for whatever
reason), but the de facto standard set by the drivers for older Xbox
and Playstation controllers represents each trigger as the Z-axis of
the nearest analog stick.

Android documentation encourages Bluetooth gamepad manufacturers to use
a third incompatible convention where the left and right triggers are
represented as the brake and gas pedals of a driving simulator
controller. The Android convention also changes the representation of
the right stick: instead of using X and Y rotation as a second pair
of axes, Android uses Z position as a second horizontal axis, and
Z rotation as a second vertical axis.

Try to cope gracefully with all of these. This will hopefully resolve
the issue described in #5406 (when using unpatched kernels).

Signed-off-by: Simon McVittie <smcv@collabora.com>
@slouken
Copy link
Collaborator

slouken commented Jun 13, 2023

Fixed by #7797

@slouken slouken closed this Jun 13, 2023
slouken pushed a commit that referenced this pull request Jun 13, 2023
This heuristic for gamepads without a more specific mapping already
tried two incompatible conventions for handling triggers: the Linux
Gamepad Specification uses hat switch 2 for the triggers (for whatever
reason), but the de facto standard set by the drivers for older Xbox
and Playstation controllers represents each trigger as the Z-axis of
the nearest analog stick.

Android documentation encourages Bluetooth gamepad manufacturers to use
a third incompatible convention where the left and right triggers are
represented as the brake and gas pedals of a driving simulator
controller. The Android convention also changes the representation of
the right stick: instead of using X and Y rotation as a second pair
of axes, Android uses Z position as a second horizontal axis, and
Z rotation as a second vertical axis.

Try to cope gracefully with all of these. This will hopefully resolve
the issue described in #5406 (when using unpatched kernels).

Signed-off-by: Simon McVittie <smcv@collabora.com>
(cherry picked from commit cf1dc66)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants