-
Notifications
You must be signed in to change notification settings - Fork 285
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
improve linux joystick detection #902
base: master
Are you sure you want to change the base?
Conversation
This correctly identifies X/Y/Z axes as well as common button types. More work needs to be done but this also adds enough log information that someone could just send us an allegro.log and we could improve from that. My old xbox 360 controller has 100% of axes and buttons correctly identified with his. Before the patch all the buttons were just named B and the axes were mapped completely wrong, mapping triggers as axes and swapping X and Y axes of the sticks.
Does this work properly with non-Xbox gamepads? I know we've had issues with Xbox controllers in the past where attempting to fix them breaks things with other devices. |
I don't know. But if you look at the changes the old code just was wrong - e.g. it simply ignored whether axes are X/Y/Z and just alternated between X and Y. And it completely ignored the button type. So I would say the chance that the new code is any worse than the old is close to zero :) Still - do you have any more information about those issues you had in the past? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
just a readout
if (strlen(name) < 4) { | ||
al_draw_text(font, fg, x + 13, y + 8, ALLEGRO_ALIGN_CENTRE, name); | ||
char name2[4]; | ||
if (strlen(name) >= 4) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
is this not the case for a strncpy?
strncpy(name2, name, 3);
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think so - strncpy will not copy the final 0.
about the axis auto discovery, I think (cannot test all joysticks in the world) that will probably break something, SDL went to the path to manual configuration instead auto discovery. They probably had a good reason for that. |
They weren't my issues--I just remember there being a good amount of discussion in the past w.r.t. incorrect behavior for Xbox controllers; I'm also pretty sure I remember a fix being reverted because it turned out to break other things. edit: This was the reverted change I was thinking of: #662 |
Let's see if someone can test it with one or two other joysticks. If something currently has 4 axes and designates them all as X axis, it would still show up as 2 sticks with X/Y axis each right now, since the current code completely ignores the designation. My change would make it show up as 4 sticks with a single X axis each. So yes, I can see cases where something would end up worse than before if evdev returns wrong information to begin with. (With my only test device evdev returns correct information.) I looked at the SDL axis/button database and it is very easy to use: https://github.com/gabomdq/SDL_GameControllerDB/blob/master/gamecontrollerdb.txt And it also means the mapping would be identical under Windows and OSX so it would be a big advantage. However it seems SDL is actually using this only in their GameController API - their Joystick API works the same as ours and does not use that database at all, just a direct mapping. So we probably would have to investigate this a bit more and maybe also use an additional API for it. |
Alright, I tested this with 2 of my joysticks, 3 configurations (F310 has a physical switch between DInput and XInput). Current codeF310, XInput
F310, DInput
PSX controller in adapter
New codeF310 XInput
F310 DInput
PSX controller in adapter
So... hit and miss. The only concrete improvement are the button names for one of the controllers (although still not usable as is, as it calls side bumpers triggers). |
I found this Linux kernel doc on joypads that may be helpful. The problem
is that what the linux driver reports and what the button, trigger or axis
really is may not always be easy to map. Nevertheless this doc suggests a
way of doing it which we don't implement. Perhaps we should try what the
doc suggests.
https://www.kernel.org/doc/html/latest/input/gamepad.html
…On Sat, 21 Apr 2018, 22:17 SiegeLord, ***@***.***> wrote:
Alright, I tested this with 2 of my joysticks, 3 configurations (F310 has
a physical switch between DInput and XInput).
Current code FX310, XInput
- 11 buttons (sees the center button)
- 4 2D sticks, triggers and right thumb mixed up between 2 sticks
FX310, DInput
- 12 buttons, (triggers as buttons, no center button)
- 3 2D sticks
PSX controller in adapter
- 12 buttons (triggers as buttons, but this is fine as PSX controller
has no pressure sensitivity for triggers)
- 3 2D sticks
New code F310 XInput
- 11 buttons (have names, center button detected), side bumpers
incorrectly called triggers
- 2 3D sticks (triggers + thumbs combined)
F310 DInput
- 12 buttons (triggers as buttons)
- 1 3D stick, combining left thumb + one axis of right thumb
- 1 1D stick, other axis of right thumb
- 1 2D stick, D-pad
PSX + adapter
- Same as F310 DInput above, screwed up sticks
So... hit and miss. The only concrete improvement are the button names for
one of the controllers (although still not usable as is, as it calls side
buttons triggers).
—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub
<#902 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AAEWecn_bEN6TwLa_g2-Or1uUD10BZR5ks5tq5PfgaJpZM4TU5p5>
.
|
Are we still not doing this with the PR? (except for the part about legacy drivers which is in fact keeping a custom database for those yourself) |
No, not quite. For game pads there seems to be a definite mapping for the
axes of the sticks and the triggers that neither the old code nor this pr
implements.
…On Sun, 22 Apr 2018, 14:36 allefant, ***@***.***> wrote:
Are we still not doing this with the PR? (except for the part about legacy
drivers which is in fact keeping a custom database for those yourself)
—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub
<#902 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AAEWeUBV4soxLDa84aTHQJlHbdKK6hiOks5trHlDgaJpZM4TU5p5>
.
|
I re-read it, I feel that's exactly what the PR does, maybe minus some smaller things. But the main problem is it seems two of SiegeLord's joystick simply do not follow the new Linux kernel guidelines at all, likely using some kind of legacy driver from before those gamepad specifications were written. The only way to make those work seems to be (as that document admits) to keep a database of device uuids and hardcode a mapping for them. |
Well, Allegro already used Allegro.cfg for configuring and dealing with all
sorts of hardware problems. I guess introducing some more settings for
Linux Joysticks is pretty much inevitable if we want all devices to work
well.
…On Mon, 23 Apr 2018, 18:43 elias-pschernig, ***@***.***> wrote:
I re-read it, I feel that's exactly what the PR does, maybe minus some
smaller things. But the main problem is it seems two of SiegeLord's
joystick simply do not follow the new Linux kernel guidelines at all,
likely using some kind of legacy driver from before those gamepad
specifications were written. The only way to make those work seems to be
(as that document admits) to keep a database of device uuids and hardcode a
mapping for them.
—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub
<#902 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AAEWeVUq5M7KNhbhgfPMXlepN38GFQWsks5trgTEgaJpZM4TU5p5>
.
|
I'd argue that none of my controllers are mapped correctly. The correct mapping for F310 (and the XBox controller it is emulating) is 11 butons, 2 1D axes for the triggers and 2 2D axes for the analog sticks. I think we should hold off on merging this code until we got a remapping setup in place. |
I note for the record that Windows itself seems to treat Xbox controllers as having a single 1D axis which is the sum of both triggers. Kind of weird. No idea if this is the same for other platforms. |
Hmm... well, for me on Windows Allegro does treat F310's triggers as 2 1D sticks (interestingly, it treats the D-pad as separate 4 buttons, and doesn't detect the center button). Notable, F310 in DInput mode is not detected at all! |
This correctly identifies X/Y/Z axes as well as common button types.
More work needs to be done but this also adds enough log information
that someone could just send us an allegro.log and we could improve from
that.
My old xbox 360 controller has 100% of axes and buttons correctly
identified with his.
Before the patch all the buttons were just named B and the axes were
mapped completely wrong, mapping triggers as axes and swapping X and Y
axes of the sticks.