Re: [PATCH 1/2] HID: nintendo: fix face button mappings
From: Martino Fontana
Date: Tue May 31 2022 - 11:19:40 EST
> Second, even if the patch was right it would be tricky to merge. The
> problem is that a changed mapping breaks user spaces and in general
> can't do this unless there is a really good reason. It just would
> break existing applications and libraries (often e.g. SDL)
The problem is that the userspace is already broken.
If, out of the box, you attempt to launch something that uses SDL (like
Wine, or Super Tux Kart), the mapping you'll get will be wrong (and not
visually, the buttons are literally swapped).
Right now, this can be worked around (it makes the mapping correct) by
setting an environment variable (which isn't a thing that a user is
supposed to be doing, and the patch will remove the need of it):
```
SDL_GAMECONTROLLERCONFIG=050000007e0500000920000001800000,Nintendo
Switch Pro
Controller,platform:Linux,a:b0,b:b1,x:b3,y:b2,back:b9,guide:b11,start:b10,leftstick:b12,rightstick:b13,leftshoulder:b5,rightshoulder:b6,dpup:h0.1,dpdown:h0.4,dpleft:h0.8,dpright:h0.2,leftx:a0,lefty:a1,rightx:a2,righty:a3,lefttrigger:b7,righttrigger:b8,030000007e0500000920000011810000,Nintendo
Switch Pro
Controller,platform:Linux,a:b0,b:b1,x:b3,y:b2,back:b9,guide:b11,start:b10,leftstick:b12,rightstick:b13,leftshoulder:b5,rightshoulder:b6,dpup:h0.1,dpdown:h0.4,dpleft:h0.8,dpright:h0.2,leftx:a0,lefty:a1,rightx:a2,righty:a3,lefttrigger:b7,righttrigger:b8,060000007e0500000620000000000000,Nintendo
Switch Combined
Joy-Cons,platform:Linux,a:b0,b:b1,x:b3,y:b2,back:b9,guide:b11,start:b10,leftstick:b12,rightstick:b13,leftshoulder:b5,rightshoulder:b6,dpup:b14,dpdown:b15,dpleft:b16,dpright:b17,leftx:a0,lefty:a1,rightx:a2,righty:a3,lefttrigger:b7,righttrigger:b8,
```
The weird thing however is that SDL should already uses a controller
database:
https://github.com/libsdl-org/SDL/blob/main/src/joystick/SDL_gamecontrollerdb.h.
If this problem is caused by an SDL bug (I don't know if it is), then
it's SDL that will need a patch, and the problem will be solved with
zero repercussions. I think that SDL should be investigated before
continuing to discuss this.
If, however, SDL is working as intended (making the patch necessary) and
this patch is merged, it will make the mapping correct out of the box
but it will also break a few things:
- Those who were using this workaround will have to remove it (by the
way, the page on the Arch Wiki should be updated too:
https://wiki.archlinux.org/title/Gamepad#Using_hid-nintendo_with_SDL2_Games);
- Applications that use raw joydev/evdev mappings (like Dolphin
Emulator) will have to be reconfigured.
- Also, some applications use SDL2 mappings in a different way (for
example, PCSX2), so out of the box the mappings are applied and the
controller is mapped as expected. Merging the patch will break
applications like these (in PCSX2's case it will be temporary since its
database is updated weekly, but it will force the user to use the new
kernel version, otherwise they will be stuck with a wrong mapping).
Whether it's a wise idea to merge this as soon as possible in order to
cause the least amount amount of breakage, is not my call.
On 13/05/22 21:58, Roderick Colenbrander wrote:
Hi Max,
Thanks for your patch, however I must say the patch is not correct for
2 reasons.
Over the years different controllers have different layouts. The
standard which this driver (as well as others such as
hid-sony/hid-playstation) follow is the Linux gamepad standard (see
Documentation/input/gamepad.rst). It stays away of the debate what is
A/B/X/Y. It talks about North/west/.., (yes they are macros which map
to A/B/X/Y). In case of the Switch it does mean things are flipped,
but it was not meant to represent an Xbox controller. (Technically one
could argue that the Xbox controller should be flipped as it was the
SNES controller back in the days which introduced X/Y and the Switch
is still consistent with that.)
Second, even if the patch was right it would be tricky to merge. The
problem is that a changed mapping breaks user spaces and in general
can't do this unless there is a really good reason. It just would
break existing applications and libraries (often e.g. SDL)
Thanks,
Roderick
On Wed, May 11, 2022 at 8:12 PM Max Fletcher <fletcher0max@xxxxxxxxx> wrote:
Previously, A and B would match the Xbox layout, but X and Y were incorrectly swapped. This corrects it so that X and Y match the Xbox layout.
Signed-off-by: Max Fletcher <fletcher0max@xxxxxxxxx>
---
drivers/hid/hid-nintendo.c | 10 +++++-----
1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/drivers/hid/hid-nintendo.c b/drivers/hid/hid-nintendo.c
index 2204de889739..7735971ede3f 100644
--- a/drivers/hid/hid-nintendo.c
+++ b/drivers/hid/hid-nintendo.c
@@ -1351,10 +1351,10 @@ static void joycon_parse_report(struct joycon_ctlr *ctlr,
input_report_key(dev, BTN_START, btns & JC_BTN_PLUS);
input_report_key(dev, BTN_THUMBR, btns & JC_BTN_RSTICK);
input_report_key(dev, BTN_MODE, btns & JC_BTN_HOME);
- input_report_key(dev, BTN_WEST, btns & JC_BTN_Y);
- input_report_key(dev, BTN_NORTH, btns & JC_BTN_X);
- input_report_key(dev, BTN_EAST, btns & JC_BTN_A);
- input_report_key(dev, BTN_SOUTH, btns & JC_BTN_B);
+ input_report_key(dev, BTN_X, btns & JC_BTN_Y);
+ input_report_key(dev, BTN_Y, btns & JC_BTN_X);
+ input_report_key(dev, BTN_B, btns & JC_BTN_A);
+ input_report_key(dev, BTN_A, btns & JC_BTN_B);
}
input_sync(dev);
@@ -1578,7 +1578,7 @@ static const unsigned int joycon_button_inputs_l[] = {
static const unsigned int joycon_button_inputs_r[] = {
BTN_START, BTN_MODE, BTN_THUMBR,
- BTN_SOUTH, BTN_EAST, BTN_NORTH, BTN_WEST,
+ BTN_A, BTN_B, BTN_Y, BTN_X,
BTN_TR, BTN_TR2,
0 /* 0 signals end of array */
};
--
2.35.3