Messages in this thread | | | Date | Sat, 4 Jun 2022 15:11:28 +0200 | Subject | Re: [PATCH 1/2] HID: nintendo: fix face button mappings | From | Martino Fontana <> |
| |
I made a PR: https://github.com/libsdl-org/SDL/pull/5755.
On 31/05/22 17:31, Roderick Colenbrander wrote: > On Tue, May 31, 2022 at 8:19 AM Martino Fontana <tinozzo123@gmail.com> wrote: >> >> > 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. > > It looks like SDL lacks the patch to deal with the upstream driver > properly. The chicken and egg problem SDL there is between the kernel > and SDL, is that SDL supports a device prior to there being a kernel > driver (or there being enough penetration for one). Without a driver, > many devices function with an often strange mapping. That's the > mapping SDL has in its table (for hid-generic). When a kernel driver > comes around with a different, but proper mapping there is an issue. > > At the time we first dealt with this for DualShock 3 / 4 devices. The > trick used then was to patch the 'version' bit of the HID device with > '0x8000'. This resulted in a different mapping line. To understand SDL > uses a GUID as the index for the table, it is composed of > vendor/product id, version id and other fiels. The patched version, > resulted in a different GUID, which then allowed SDL2 to recognize the > device properly as it would use a different mapping line. > > For the Switch controllers, it looks like no patched line was added. > Someone would need to provide a patch. > >> 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@gmail.com> 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@gmail.com> >>>> --- >>>> 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 >>>> >>> >>>
| |