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

Report Share button as a BTN_, instead of KEY_RECORD #20

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

mbenkmann
Copy link

KEY_RECORD is a multi-media keyboard key that starts audio recording. This is not the meaning of the Share button.
Having the Share button report KEY_RECORD has side-effects:

  • software gets confused (see e.g. [Linux] The share button on Xbox Series X|S controller produces "double events" libsdl-org/SDL#6296)
  • The Xorg server opens the controller device, keeps it permanently open and reads its buttons and reports share button presses as XF86AudioRecord to applications. Users do not expect this. I grepped the Linux kernel's drivers/input/joystick and there exists NO OTHER joystick type controller that reports a KEY_ type event. They all use BTN_* events.
    I'm sure the person who added the Share button originally just grabbed some event without giving it much thought.

KEY_RECORD is a multi-media keyboard key that starts audio recording.
This is not the meaning of the Share button.
Having the Share button report KEY_RECORD has side-effects:
- software gets confused (see e.g. libsdl-org/SDL#6296)
- The Xorg server opens the controller device, keeps it permanently open and
  reads its buttons and reports share button presses as XF86AudioRecord to
  applications. Users do not expect this.
I grepped the Linux kernel's drivers/input/joystick and there exists
NO OTHER joystick type controller that reports a KEY_ type event.
They all use BTN_* events.
I'm sure the person who added the Share button originally just grabbed
some event without giving it much thought.
@Beanow
Copy link

Beanow commented Oct 7, 2023

Maybe @dlundqvist would be interested in this in their fork?

Can confirm the issue that the share button on official controller maps to media keys, and as a result isn't really usable. A couple of games I've tried so far refuse to even map controls to this.

Another simple test could be: https://github.com/luser/gamepadtest
Which maps everything (including the middle xbox button) except the share button.

Haven't tested the patch yet though.

@Beanow
Copy link

Beanow commented Oct 7, 2023

I've tested against the fork and the patch looks good.
dlundqvist#3

@medusalix
Copy link
Owner

@mbenkmann Hi, thanks for the PR.
I agree that KEY_RECORD might not be the best option for the share button. However, I'm not sure if we should use BTN_TRIGGER_HAPPY11, as that button range is already used by third-party guitars and drum sets in xone.
What do you think about using BTN_BASE instead? It looks like it's used by the Steam Deck's quick access button for example: https://github.com/torvalds/linux/blob/712e14250dd2907346617eba275c46f53db8fae7/drivers/hid/hid-steam.c#L1455

@kakra I saw that xpadneo is currently mapping the share button to KEY_F12. What do you think about changing that to BTN_BASE?

@kakra
Copy link

kakra commented Mar 31, 2024

BTN_BASE is from the joystick range, not from the gamepad range. I only send KEY_F12 in a secondary input device which acts as a keyboard. Using it as part of the gamepad would be wrong. I never got KEY_RECORD to work properly, probably because the consumer input range is not implemented in user-space as actual input but rather for remote control of specific applications. In theory, it should work to do exactly what is needed, but in practice no relevant application seems to implement it.

In that context, BTN_TRIGGER_HAPPY* is probably the correct way to go. You can choose a yet unused position. Since Steam and SDL mostly follow upstream xpad, I'd try to look into where they placed this button, and maybe move the guitar buttons to the end of the range.

SDL currently implements heuristics for HID drivers to detect the physical button associated with the keybit. It also makes some assumptions for Xbox controller drivers in the trigger happy range, so xpadneo will follow xpad and SDL in the next version. This way, it is automatically compatible with Steam because that uses SDL internally.

If we put joystick buttons in a gamepad device, user-space may misidentify the gamepad as a joystick. For the Steam Deck, this is fine, because Valve controls the implementation and can patch SDL/wine to properly work with it. Also, the quick access button is not visible to games, so it will be hidden from games anyways and only the Steam client uses it. But for non-SteamOS implementation, we should try to stay within standards and not bleed into other devices bit ranges.

@medusalix
Copy link
Owner

BTN_BASE is from the joystick range, not from the gamepad range. I only send KEY_F12 in a secondary input device which acts as a keyboard. Using it as part of the gamepad would be wrong. I never got KEY_RECORD to work properly, probably because the consumer input range is not implemented in user-space as actual input but rather for remote control of specific applications. In theory, it should work to do exactly what is needed, but in practice no relevant application seems to implement it.

Thanks for the clarification. It's really unfortunate that the gamepad range doesn't include more (named) buttons apart from BTN_TRIGGER_HAPPY*.

In that context, BTN_TRIGGER_HAPPY* is probably the correct way to go. You can choose a yet unused position. Since Steam and SDL mostly follow upstream xpad, I'd try to look into where they placed this button, and maybe move the guitar buttons to the end of the range.

xpad uses KEY_RECORD for the share button, that's the issue. But AFAICT, SDL handles that just fine. Would you still suggest using the BTN_TRIGGER_HAPPY range instead?

SDL currently implements heuristics for HID drivers to detect the physical button associated with the keybit. It also makes some assumptions for Xbox controller drivers in the trigger happy range, so xpadneo will follow xpad and SDL in the next version. This way, it is automatically compatible with Steam because that uses SDL internally.

So also changing the share button back to KEY_RECORD, following xpad and SDL?

@kakra
Copy link

kakra commented Mar 31, 2024

I haven't made up my mind yet. Currently, xpadneo uses multi input device mode, exposing exclusively gamepad buttons in the gamepad subdevice (excluding the guide button because it's technically not a gamepad button, see XINPUT specification of Microsoft: https://learn.microsoft.com/en-us/windows/win32/api/xinput/ns-xinput-xinput_gamepad).

Everything else, if part of the gamepad input device, should probably go to the trigger happy range.

In xpadneo, we were able to move BTN_MODE back into the gamepad, tho. Current SDL implementations handle that properly: Xbox devices have been removed from the controllerdb entries, and use hints from HID instead, or fall back to xpad.

I still think that KEY_RECORD doesn't belong there the way xpad currently uses it: It's a consumer device keybit, not a gamepad keybit. At least, user-space seems to handle it correctly.

The biggest obstacle for SDL is properly detecting which buttons are physically available. For HID, they rely on the keybits properly set in the mask. SDL tries to avoid looking at the VID/PID to assume a set physical buttons (this is not true in hidraw mode, tho, which is why xpadneo is currently incompatible with that mode in SDL). For non-HID input devices, this is probably simpler: xone should just set buttons in the keymap that actually exist.

As far as I know, the logic goes like this: user-space is supposed to identify the type of device by looking at the first button of a range: BTN_GAMEPAD indicates it's a gamepad (which is an alias of button A). Same goes for BTN_JOYSTICK, BTN_MOUSE, BTN_DIGI (which comes right after the gamepad range, so bleeding into the digitizer range should be avoided), BTN_WHEEL, and so on. You get the idea.

So as long as you avoid mapping any primary buttons of other device ranges, and your only primary button in the map registers in the range of your actual device, you should be "fine". Although, still, KEY_RECORD is from a completely different range and device type.

If adding new buttons, we need to also take into account how mapping buttons to symbols actually works in many games: games usually count bits in the keymap to know the physical position of buttons (and don't look at the kernel symbol name, many Unity games do that). If you insert a button below the trigger happy range, all known buttons in that range will shift up by one position. It may confuse games. It should be fine for SDL games, tho, with their latest logic.

I'm not sure how games detect guitar devices specifically. Isn't this mostly an Xbox controller and the colored buttons map to the Xbox button colors? So I wonder why we need those buttons in the trigger happy range anyways... IOW, if games can properly identify the guitar, we should be able to shift the buttons down back into the gamepad range one by one, and additionally there's technically no reason why a gamepad and a guitar should not use the same button symbols for different purposes. Does a guitar and a gamepad differ in the GIP protocol for handling buttons?

BTW: trigger happy is an extension range for gaming devices with lots of buttons. The naming is more or less a joke (trigger for button, and happy like joy), and it can be used by any range to extent the amount of buttons beyond what the native device range allows. We should not need to care about which trigger happy bit is which button as long as the order of buttons is kept compatible between different drivers. User-space should not assume any specific device type based on bits in the trigger happy range. It should only assume a device type by looking at the primary button of a range (and thus, a guitar should actually set a specific button of a well known range, otherwise it may be mis-identified as accelerometer because such devices have analog axes but no buttons, this is a problem for some gaming pedals, btw).

Of course, technically, SDL does not need to care about what each range actually means. If it supports a device, it can use any bit of the keymap. But for other user-space (like udev, or X11/wayland input drivers) it may actually matter.

@medusalix
Copy link
Owner

I haven't made up my mind yet. Currently, xpadneo uses multi input device mode, exposing exclusively gamepad buttons in the gamepad subdevice (excluding the guide button because it's technically not a gamepad button, see XINPUT specification of Microsoft: learn.microsoft.com/en-us/windows/win32/api/xinput/ns-xinput-xinput_gamepad).

I guess I'll just leave it as it is then for now.

I'm not sure how games detect guitar devices specifically. Isn't this mostly an Xbox controller and the colored buttons map to the Xbox button colors? So I wonder why we need those buttons in the trigger happy range anyways... IOW, if games can properly identify the guitar, we should be able to shift the buttons down back into the gamepad range one by one, and additionally there's technically no reason why a gamepad and a guitar should not use the same button symbols for different purposes. Does a guitar and a gamepad differ in the GIP protocol for handling buttons?

Guitars and drum kits have extra buttons and analog inputs in addition to the standard Xbox gamepad buttons. It's just a different (larger) input packet that is mapped to the BTN_TRIGGER_HAPPY* range by xone. They're not even supported by the Windows driver. But based on the feedback I've got, everything works fine.

@Morbiuzx
Copy link

Is this being worked on?

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.

5 participants