Closed lights0123 closed 3 years ago
Oh cool, is this the original Xbox Series X controller? I didn't expect it to have Bluetooth... Usually the original controllers bundled with the consoles do not support Bluetooth.
Oh cool, is this the original Xbox Series X controller?
Yep, although I just got the controller by itself because a) all the other controllers were out of stock and b) might as well get the latest one—I don't actually own an Xbox, or plan to.
Also, please dump the device descriptor and document it in
docs/descriptors/
.
Done, although I don't know what I should call it.
Use evtest to test if each button maps correctly
The "change view" button (left button on face, I think also called Back/Select) simply had no output in evtest
. In btmon
, however, I got:
> ACL Data RX: Handle 3585 flags 0x02 dlen 23
ATT: Handle Value Notification (0x1b) len 18
Handle: 0x001e
Data: 857d887d887e06840000000000000000
> ACL Data RX: Handle 3585 flags 0x02 dlen 23
ATT: Handle Value Notification (0x1b) len 18
Handle: 0x001e
Data: 857d887d887e06840000000000000400
The second one (with the 4
) had the button pressed.
Select works. Y appears as West. B appears as East. X appears as North. A appears as South. Assuming that these are supposed to match with a compass, Y and X are swapped.
The DPad gives a positive value for down or right, and negative for up or left. Up and down are on the Y axis, left and right are on the X axis. This may mean that Y needs to be inverted.
The left joystick (ABS_X/ABS_Y) gives a positive value for up or right, and negative for down or left. Up and down are on the X axis, left and right are on the Y axis. This may mean that X and Y need to be swapped. The right joystick (ABS_RX/ABS_RY) has the same behavior.
Left shoulder button sends BTN_TL, right shoulder button sends BTN_TR.
Left trigger (the variable one) sends:
Event: time 1605054206.497768, type 3 (EV_ABS), code 2 (ABS_Z), value 1011
Event: time 1605054206.497768, type 3 (EV_ABS), code 40 (ABS_MISC), value -1011
ABS_Z has a positive value corresponding with how much it's pressed down, while ABS_MISC has its inverse.
Right trigger (the variable one) sends:
Event: time 1605054246.638020, type 3 (EV_ABS), code 40 (ABS_MISC), value 104
Event: time 1605054246.638020, type 3 (EV_ABS), code 5 (ABS_RZ), value 104
ABS_RZ has a positive value corresponding with how much it's pressed down, with ABS_MISC matching.
please also include the share button so we know the code of it
Event: time 1605053344.113912, type 4 (EV_MSC), code 4 (MSC_SCAN), value 90007 Event: time 1605053344.113912, type 1 (EV_KEY), code 314 (BTN_SELECT), value 1 Event: time 1605053344.113912, -------------- SYN_REPORT ------------ Event: time 1605053344.203915, type 4 (EV_MSC), code 4 (MSC_SCAN), value 90007 Event: time 1605053344.203915, type 1 (EV_KEY), code 314 (BTN_SELECT), value 0 Event: time 1605053344.203915, -------------- SYN_REPORT ------------
This looks like it's using the select button code for the share button.
Forgot to mention that the thumb buttons work as well, with THUMBL and THUMBR matching the left and right button clicks respectively.
This looks like it's using the select button code for the share button.
Yeah, I was expecting that. Thus, we need to ignore it for now until we decided how to handle it. I can imagine two solutions: We'd map it to BTN_TRIGGER_HAPPY
range (happy is like joy, thus the name). But I'm unsure at which position: I'd like to have the four paddles of the XBE2 controller at BTN_TRIGGER_HAPPY{1,2,3,4}
. So we'd need to map it to 5. But what if a future controller adds another gaming button?
So we should probably better move it to the keyboard range and use one of the KEY_*
buttons for the share button. I'm not sure which, maybe there a screenshot keycode? At least that sound suitable. And after all: It's not a gaming button thus it should by no means have an BTN_*
keycode.
It could be mapped to Print Screen, but that seems a bit weird. What if we just sent it to a FIFO instead? My keyboard driver does this, although in userspace. That way, you can just wait for new input there and handle it. I don't know if we want to deal with that complexity though for a single button (currently). It also complicates multiuser, as you can't just rely on the standard HID permission magic.
Actually, the behaviors that I noted seem to actually be correct. Just played a game, and everything including rumble seems to work—just the select button is weird.
Now, let's deep dive...
Yep, although I just got the controller by itself because a) all the other controllers were out of stock and b) might as well get the latest one—I don't actually own an Xbox, or plan to.
Ah, okay. We should refer to it as XBSX (Xbox Series X), lining up with XB1S and XBE2 using four-letter codes.
Also, please dump the device descriptor and document it in
docs/descriptors/
.Done, although I don't know what I should call it.
Looks fine. If you don't mind, I'd rename it during merge if I feel it should have a different name.
Use evtest to test if each button maps correctly
The "change view" button (left button on face, I think also called Back/Select) simply had no output in
evtest
. Inbtmon
, however, I got:> ACL Data RX: Handle 3585 flags 0x02 dlen 23 ATT: Handle Value Notification (0x1b) len 18 Handle: 0x001e Data: 857d887d887e06840000000000000000 > ACL Data RX: Handle 3585 flags 0x02 dlen 23 ATT: Handle Value Notification (0x1b) len 18 Handle: 0x001e Data: 857d887d887e06840000000000000400
The second one (with the
4
) had the button pressed.
This looks like it's using a different protocol, not the usual Bluetooth HID protocol. But at least Linux somehow seems to map it to HID so it ends up in xpadneo. Fine.
For the button bits: I wonder where button A is in the report. It is important to remap the keycodes in a way that, when ordered by keycode, we match the order A,B,X,Y,TL,TR,SELECT,START,THUMBL,THUMBR with no other keycodes generated inbetween. Otherwise, games and the kernel will mismatch the symbolic names with the buttons on the controller. This is why we cannot use BTN_MODE
for the logo button because it comes before the thumb button and move their positional code, thus shifting select, start and thumb buttons by one position. This is what you're currently seeing with the share button. So we need to remap the share button out of this range. As I already wrote, and since it isn't a gaming button, I suggest moving it to a KEY_*
range.
See also: https://github.com/torvalds/linux/blob/master/include/uapi/linux/input-event-codes.h
Select works. Y appears as West. B appears as East. X appears as North. A appears as South. Assuming that these are supposed to match with a compass, Y and X are swapped.
Yes, this is what the Linux gamepad specification says. But no, it doesn't match what games expect. Thus, for buttons please ignore the directional symbols, just use the named symbols.
The DPad gives a positive value for down or right, and negative for up or left. Up and down are on the Y axis, left and right are on the X axis. This may mean that Y needs to be inverted.
No, I think that's okay: Up should be negative (this doesn't match with what the thumb axes do).
The left joystick (ABS_X/ABS_Y) gives a positive value for up or right, and negative for down or left. Up and down are on the X axis, left and right are on the Y axis. This may mean that X and Y need to be swapped. The right joystick (ABS_RX/ABS_RY) has the same behavior.
This sounds strange... Let's fix some other stuff first and investigate the HID descriptor for this. Sounds like MS is trying to play tricks again with how Linux maps axes codes to actual symbolic axes. Or one of our HID fixups doesn't play nice with the new report format: We are actually swapping, moving, and shifting some codes around in the report fixup to convert it to a GIP compatible HID report.
Left shoulder button sends BTN_TL, right shoulder button sends BTN_TR.
This is fine, that means there's no other BTN_*
code interfering before these positions. OTOH, there are not many buttons that could do that except BTN_C
and BTN_Z
which are hopefully not assigned by the HID descriptor - and if they are, our report fixup should shift the bits to exclude those.
Left trigger (the variable one) sends:
Event: time 1605054206.497768, type 3 (EV_ABS), code 2 (ABS_Z), value 1011 Event: time 1605054206.497768, type 3 (EV_ABS), code 40 (ABS_MISC), value -1011
ABS_Z has a positive value corresponding with how much it's pressed down, while ABS_MISC has its inverse.
ABS_MISC
is a synthesized axis we added due to a feature request to combine both triggers into a rudder axis. This can be used to assign both triggers to a single rolling axis in simulators. Since its positional code orders after the standard axis, this should create no compatibility problems.
Right trigger (the variable one) sends:
Event: time 1605054246.638020, type 3 (EV_ABS), code 40 (ABS_MISC), value 104 Event: time 1605054246.638020, type 3 (EV_ABS), code 5 (ABS_RZ), value 104
ABS_RZ has a positive value corresponding with how much it's pressed down, with ABS_MISC matching.
Matches what we expect. See above.
please also include the share button so we know the code of it
Event: time 1605053344.113912, type 4 (EV_MSC), code 4 (MSC_SCAN), value 90007 Event: time 1605053344.113912, type 1 (EV_KEY), code 314 (BTN_SELECT), value 1 Event: time 1605053344.113912, -------------- SYN_REPORT ------------ Event: time 1605053344.203915, type 4 (EV_MSC), code 4 (MSC_SCAN), value 90007 Event: time 1605053344.203915, type 1 (EV_KEY), code 314 (BTN_SELECT), value 0 Event: time 1605053344.203915, -------------- SYN_REPORT ------------
Okay, so we cannot ignore it because it uses the code originally intended to be used by BTN_SELECT
. So we need to use a report fixup to shift the reported bits around and order that bit after the standard button bits. This is were it gets a little more challenging now. You want to try?
You'll need to change xpadneo_report_fixup()
to report 12 buttons for your controller. That function patches the descriptor you dumped to the documentation file: It matches some bytes in the report and patches those to report what we want it to report. You can validate the result using the online decoder. It's important to pad the count to a byte boundary probably, this is why we patch 15+1 bits to 11+5 bits, it makes 16 bits = 2 bytes. You'll have to use 12+4 probably.
It'll mess up the mappings or render button completely silent in evtest
now. This is why you also need to patch the raw events from the device which happens in xpadneo_raw_event()
: Adjust the code to shift the bit position 7 to 12: 00 40 ..
is at position 7 (counted backwards from the end of the word in btmon
). It needs to become 08 00
. This change would make button positions 1-11 map correctly, matching what we define in xpadneo_usage_maps
. If you look at evtest
again now, it should report a new keycode for the share button. Now adjust that structure to map it something that is not a BTN_*
code so we get it out of the range of gaming input. This is important because games need to see just 10 buttons, otherwise they may act strange.
It could be mapped to Print Screen, but that seems a bit weird. What if we just sent it to a FIFO instead? My keyboard driver does this, although in userspace. That way, you can just wait for new input there and handle it. I don't know if we want to deal with that complexity though for a single button (currently). It also complicates multiuser, as you can't just rely on the standard HID permission magic.
We should probably move it to the range where the consumer control codes live, maybe KEY_SENDFILE
? Sounds suitable...
I'm going to split the code into different files at the end of the v0.9 cycle, we can decide for more advanced stuff then.
It'll mess up the mappings or render button completely silent in
evtest
now. This is why you also need to patch the raw events from the device which happens inxpadneo_raw_event()
: Adjust the code to shift the bit position 7 to 12:00 40 ..
is at position 7 (counted backwards from the end of the word inbtmon
). It needs to become08 00
. This change would make button positions 1-11 map correctly, matching what we define inxpadneo_usage_maps
. If you look atevtest
again now, it should report a new keycode for the share button. Now adjust that structure to map it something that is not aBTN_*
code so we get it out of the range of gaming input. This is important because games need to see just 10 buttons, otherwise they may act strange.
Actually, the current code may do something wrong with the share button already... But I'm guessing you'll be able to figure it out correctly.
Actually, the behaviors that I noted seem to actually be correct. Just played a game, and everything including rumble seems to work—just the select button is weird.
Yeah, games using evdev
will probably work when they use symboling names to match the buttons. Modern SDL version can do that. But all games using jsdev
or older SDL versions will use positional buttons for mapping, so we need to ensure that the first 10 positions map correctly, and in the best case we stay at 10 positions.
But I was thinking about the share button. It may actually be a game function, right? This is probably why MS decided to report 11 buttons instead of 10, so games will see a button 11 and use that. In that case, it should be mapped to BTN_TRIGGER_HAPPY#
- whatever that number is because XBE2 has the four paddle buttons. We probably need to figure that out later when games start to support the additional buttons. Until then, let's move it to a KEY_*
and leave a comment in the code about that.
After evtest
works correctly, test if jstest
works correctly. It should report no bogus buttons or axes, and match positions correctly with one caveat: You would see axes positions 1-6 as LX,LY,LT,RX,RY,RT
instead of LX,XY,RX,RY,LT,RT
(which is what jsdev games expect). But that's to be fixed with jscal
. We cannot reorder jsdev from within xpadneo in a compatible way. I've tried in https://github.com/atar-axis/xpadneo/issues/220 and it only messed up everything.
We should probably move it to the range where the consumer control codes live, maybe
KEY_SENDFILE
? Sounds suitable...
KEY_RECORD
could make a good code, too. I think the share button is intended to work this way: Short press = record screenshot, long press = record video. So "record" makes most sense to me.
Also, let's not report it as button 11 because some software assumes to find the guide button there.
Here's SDL's change: https://hg.libsdl.org/SDL/rev/38671e8e9a31#l6.34
we match the order A,B,X,Y,TL,TR,SELECT,START,THUMBL,THUMBR
I can confirm that is matched by https://gamepad-tester.com/.
The left joystick (ABS_X/ABS_Y) gives a positive value for up or right, and negative for down or left. Up and down are on the X axis, left and right are on the Y axis. This may mean that X and Y need to be swapped. The right joystick (ABS_RX/ABS_RY) has the same behavior.
This sounds strange... Let's fix some other stuff first and investigate the HID descriptor for this. Sounds like MS is trying to play tricks again with how Linux maps axes codes to actual symbolic axes. Or one of our HID fixups doesn't play nice with the new report format: We are actually swapping, moving, and shifting some codes around in the report fixup to convert it to a GIP compatible HID report.
The left joystick at least worked properly in Rocket League under Wine, so that may also be working as expected.
I'm working on figuring out what modifications I need to make right now.
Here's SDL's change: https://hg.libsdl.org/SDL/rev/38671e8e9a31#l6.34
Interesting, so they are putting it in front of the paddle buttons but conditionally? That sounds like a strange decision because it maps the paddles to position 11-14 for controllers without a share button and 12-15 for controllers with a share button (or +1 if we count the guide button).
we match the order A,B,X,Y,TL,TR,SELECT,START,THUMBL,THUMBR
I can confirm that is matched by https://gamepad-tester.com/.
At least in Chrome, Gamepad Tester is its own story... It makes some wrong assumptions: It reads a mapping from evdev but then uses jsdev to read the actual axes and buttons despite jsdev having an actual API to get the symbolic names. Since that results in some mis-mappings, they've put code in to correct the wrong mappings when some device VID/PID matches - so they are adding an additional layer of hardware abstraction. This doesn't work with xpadneo correctly: It confuses the axes mapping, and you cannot correctly fix it with jscal now because they combine evdev+jsdev with a custom hardware database.
This doesn't work with xpadneo correctly: It confuses the axes mapping, and you cannot correctly fix it with jscal now because they combine evdev+jsdev with a custom hardware database.
Either it works now, or the XBE2 controller doesn't have wrong assumptions in the Chrome Gamepad API.
You'll need to change
xpadneo_report_fixup()
to report 12 buttons for your controller..
How should I gate that, like you do here?
Should I check if there's 11 buttons, and then change it to 12? Should I look at the product ID?
How should I gate that, like you do here?
Yeah, just look if there's 11 buttons and change to 12. That's the correct way to do it. Maybe add a quirk flag for the share button so you can pull it from xdata in the other functions. You'll need 12 bits in xpadneo_raw_event()
to detect the button press. We may need to adjust the existing "11 button instead of 10" to "12 ..." to have a consistent view between models.
I just realized in one of your first comment that "change page" is not "share" - so some of my comments may confuse the bit positions. But I think you get the picture.
wait, I just realized that the descriptor message I posted must include that fixup already, right? dmesg
includes "fixing up button mapping", but the dump I got shows 11 buttons, so that must mean that it already includes its changes.
That means that I can't just rely on a 11→12 change as it appears identical, so I'll have to switch either based on some descriptor data if it's present, or product ID. Also, I should probably re-dump the descriptors. Is there an easy way to do that? Can I unload the kernel module and get it somehow, or should I comment out that block?
Unless you're OK with other controllers also having 12 buttons, which I assume you don't want.
That means that I can't just rely on a 11→12 change as it appears identical, so I'll have to switch either based on some descriptor data if it's present, or product ID. Also, I should probably re-dump the descriptors. Is there an easy way to do that? Can I unload the kernel module and get it somehow, or should I comment out that block?
If your kernel has debug mode enabled, you MAY get the original rdesc from /sys/kernel/debug/hid
, but please double-check. Otherwise you'll have to blacklist xpadneo temporarily to prevent it from loading.
Only rely on PID if no other option is available because MS tends to change the PID with firmware updates sometimes without updating the rdesc, and at least at one point they changed the rdesc without updating the PID. So the only reliable way is checking if the expected bytes at the expected positions are actually present.
Just put your patch above the "15->11" and it should work, maybe put a comment to hint importance of order. Positions 145 and 163 won't match my patch then. Or make it an if/else.
Unless you're OK with other controllers also having 12 buttons, which I assume you don't want.
We can add a quirk flag to tell such cases apart at any time later... In the "rdesc>=164" case we should use if/else to tell both models apart, and either patch 15->11 or XX->12 buttons. Depending on the original unpatched descriptor, you may need to set the linux buttons quirk flag then. Maybe also adjust the documentation to have the unpatched descriptor instead.
If the unpatched descriptor also reports 15 buttons, change the code to report 12. We need to adjust only xpadneo_raw_event()
then to move the share button to bit 12.
BTW: btmon
will report the original unpatched raw event, while /dev/hidrawX
would report the patched report.
Just published the actual descriptor.
or should I comment out that block?
Of course that should work, too.
5808dd3 ignores the share button, but uses the physical select button. Only switching based on PID is implemented so far.
I'm out for the night. I'm not totally sure how to report the share button status—do I just set it as data[15] & BIT(12-8)
?
So, how is the progress coming along? Just got myself the Series X pad yesterday and wondering if these commits will incorporate Bluetooth functionality.
@cow-killer xpadneo does work ONLY with Bluetooth - so yes, it will support this as long as your Bluetooth dongle supports the BLE profile (Bluetooth Low Energy). I think the XBXS controller is BLE-only (or the proprietary 5 GHz dongle but that's not covered by xpadneo currently).
I'm planning to merge this for the v0.9 version of xpadneo.
@cow-killer if you're impatient, the code in this PR does work—the only thing left really is a bit of code cleanup and figuring out what to do with the share button.
@cow-killer This PR is not even 24 hours old - give us a little air to breathe :-D
I've added
if (xdata->quirks & XPADNEO_QUIRK_SHARE_BUTTON)
bits |= (data[16] & BIT(0)) << 11; /* Share */
how do I actually get that to show up in e.g. evtest
now?
how do I actually get that to show up in e.g.
evtest
now?
It should generate a code in evtest
already but the wrong one... Did you test?
I did. With some printk
debugging, I can see that bits
is set to 2048, but no output from evtest
.
In xpadneo_input_configured
, what should I have it change its PID to?
In
xpadneo_input_configured
, what should I have it change its PID to?
This would be needed if SDL mappings are wrong. The use the 0x0B05
case, I'd say. Or pretend a different firmware version: SDL matches devices by firmware version, too, not just VID/PID. Chrome matches by PID/VID only.
I did. With some
printk
debugging, I can see thatbits
is set to 2048, but no output fromevtest
.
This is strange... Maybe this is a result from struct xpadneo_usage_maps
setting an event code to ignore?
Oh yeah, I just added it in xpadneo_usage_maps
to KEY_RECORD
and it works now.
Oh yeah, I just added it in
xpadneo_usage_maps
toKEY_RECORD
and it works now.
\o/ yay! Okay, time for testing now. First things first:
Does evtest
show correct symbols for each and every key and axis?
If yes, does jstest
actually show the expected positional order of buttons (that is, 10 standard buttons followed by optional extra buttons), and does it have the first 6 axis in standard order (X,Y,Z,Rx,Ry,Rz aka left stick x,y, left trigger, right stick x,y, right trigger)?
If yes, test SDL apps, Wine, maybe Gamepad Tester in the browser.
Does
evtest
show correct symbols for each and every key and axis?
yes
If yes, does jstest actually show the expected positional order of buttons (that is, 10 standard buttons followed by optional extra buttons), and does it have the first 6 axis in standard order (X,Y,Z,Rx,Ry,Rz aka left stick x,y, left trigger, right stick x,y, right trigger)?
yes
test SDL apps
Downloading supertuxkart right now, for some reason Arch chose to download from a mirror at less than 3Mbps when I have a gigabit connection. If you have an app that you've tested with, I can do that too.
Wine
Rocket League works
Gamepad tester
works in Firefox and Chrome
Very cool. Thanks for the hard work. I'll mention you in the NEWS file of the next release by full name if that's okay. Also, I'm going to test if my devices still work this evening before merging.
If you want to sqash some commits, or rewrite history, feel free to do so. Otherwise I'm going to squash some of your commits where I feel appropriate.
supertuxkart works, it just doesn't have the button names (says e.g. "Gamepad button 5")
Code Climate has analyzed commit 2e35331e and detected 0 issues on this pull request.
View more on Code Climate.
If you have an app that you've tested with, I can do that too.
I'm always seeing problems with AC:Unity and AC:BlackFlag... These seem to see the triggers as a joystick axis, I don't know why but you'd need to press them half-way to stop erratic movement and turns. But that seems to be a Wine thing, it worked properly with some older versions.
Also, jstest-gtk
will probably display axes wrongly. This can be fixed with jscal by putting a different order. We apparently cannot do that from the driver, and also cannot ship it as a udev rule because it breaks Gamepad Tester.
Side note, what do you think would be the easiest way to hook the share button up to do something? GNOME's hotkey feature doesn't do anything when I press it. Is there some way I can write a script that measures how long it's pressed down and does some action based on that? I could read /dev/input/event*
, but that requires root.
First tests show it's properly working in evtest
and jstest
for my controller. While testing, I found a funny bug: If you depress both thumb stick buttons while the Steam client is running, you're able to move the mouse pointer and do clicks. I've reported that to Valve, I'm not sure why the desktop controller profile of Steam input kicks in when doing that.
Apparently, I don't have too much time this evening, so I'm doing game tests later.
@kakra Getting this when trying to run install.sh
after cloning your repository:
sed: can't read ./lib/../VERSION: No such file or directory
Any chance you could help me out here?
@cow-killer are you sure that you cloned it (rather than using GitHub's download from master), and git
recognizes it as a repository?
Please file a new bug report for this. @cow-killer
@kakra that's a feature.
I'm unable to test this as I experience #166 and #239, but I do get
in my dmesg logs, and I feel the pairing vibration pattern, although it doesn't send any further data.