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.

Sign up for GitHub

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

Jump to bottom

Add flydigi vader3 support (no doubt, clobbering xbox support) #451

Closed
wants to merge 2 commits into from
Closed

Add flydigi vader3 support (no doubt, clobbering xbox support) #451

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
6484951
Add flydigi vader3 support (no doubt, clobbering xbox support)
ahungry Jan 12, 2024
091d8bc
Use a relative deadzone size
ahungry Jan 12, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions hid-xpadneo/src/hid-ids.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,5 +2,6 @@
#define HID_IDS_H_FILE

#define USB_VENDOR_ID_MICROSOFT 0x045e
#define USB_VENDOR_ID_FLYDIGI 0xD7D7

#endif
63 changes: 45 additions & 18 deletions hid-xpadneo/src/hid-xpadneo.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -112,23 +112,33 @@ struct usage_map {
#define USAGE_IGN(u) USAGE_MAP(u, MAP_IGNORE, 0, 0)

static const struct usage_map xpadneo_usage_maps[] = {
// 90009 and 9000a, sent when trigger fully released
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure what happens here... Is this a special function of your model? Then we should document why it exists and how to fit it into the event model.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Honestly, not sure, it was observed while watching the evtest output.

Copy link
Collaborator

@kakra kakra Jan 12, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

From the video presentations, the triggers seem to have microswitches, maybe at the end or at the beginning of the pressure range. It should make a click sound if you press the trigger: Try to figure out how it reacts on pressure and release.

The Steam controller has a similar feature where you could use this feature to aim down sights holding the trigger, then give a little more pressure to fire using the microswitches at the end of the pressure range.

/* fixup buttons to Linux codes */
USAGE_MAP(0x90001, MAP_STATIC, EV_KEY, BTN_A), /* A */
USAGE_MAP(0x90002, MAP_STATIC, EV_KEY, BTN_B), /* B */
USAGE_MAP(0x90003, MAP_STATIC, EV_KEY, BTN_X), /* X */
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This structure should not be changed, it is essential to support exactly those keymaps (because we patch the VID/PID). Instead, patch the HID descriptor in xpadneo_report_fixup() and the HID reports in xpadneo_raw_event() to match the xpadneo_usage_maps[].

Copy link
Collaborator

@kakra kakra Jan 12, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looking at pictures of the device, it has two additional buttons at the center (house and circle), and also buttons C and Z. These are not part of the Xbox spec and must be moved outside the range of the first 10 buttons: The Xbox spec supports exactly 10 well defined buttons in exactly that order: A,B,X,Y,LB,RB,Select,Start,LS,RS - no more, and none inserted. There's an additional Guide button at position 11 - but it's actually not meant to be dealt with by games, thus it's not part of the 10 well defined buttons. The Windows XINPUT spec (and thus what Proton uses) supports exactly 16 buttons maximum, no more. So every excess button in the keymap must be masked.

So either your device MUST not report as an Xbox controller, or exactly match those expectations.

You can patch the HID descriptors and HID reports to move the additional buttons to the BTN_TRIGGER_HAPPY range. We already moved some buttons there, especially the paddles have been moved there.

Other than that, I can only repeat: DO NOT misuse xpadneo to support this device if you don't take the above comments serious. DO NOT fork this project to support the controller, if you do not remove the VID/PID patching and do not remove support for MS devices. Really, I DO NOT want this device show up in random SDL controllerdb mappings destroying all the effort we've put into unifying all Xbox devices into a single virtual model currently properly supported by SDL and Steam.

Everything you do is reverting all these efforts and making the device show up like it would do with the hid-generic driver probably, but then probably (mis-)use our VID/PID patching technique to make the device visible to SDL and games as an Xbox controller - which it isn't in HID mode (aka Bluetooth). Instead, stick to hid-generic and add a SDL controllerdb mapping to make it visible in Steam and games. Ask the SDL project to properly support your model with hid-generic. It may be the easier way.

Please, do NOT submit a controllerdb patch to SDL while using this Frankenstein patched xpadneo driver.

Don't get me wrong: If done properly, we could support your device in xpadneo. But it is important to know the consequences of doing it the wrong way. A lot of effort went into getting to the point where we are now.

See also #286. This is also about how we should probably avoid HID patching in the future and thus stop VID/PID patching, too. That change would then render your controller useless with xpadneo because your initial idea of using xpadneo is probably because games do not see D7D7:0041 as an Xbox controller. OTOH, there's an idea of xpadneo converting other supported controllers to Xbox controllers (e.g. Switch mode of the Gulikit controllers). In this case, your controller would fit the bill of xpadneo. But it MUST be done properly: No changes to the 10 first buttons in the map, no changes to the axes.

USAGE_MAP(0x90004, MAP_STATIC, EV_KEY, BTN_Y), /* Y */
USAGE_MAP(0x90005, MAP_STATIC, EV_KEY, BTN_TL), /* LB */
USAGE_MAP(0x90006, MAP_STATIC, EV_KEY, BTN_TR), /* RB */
USAGE_MAP(0x90007, MAP_STATIC, EV_KEY, BTN_SELECT), /* Back */
USAGE_MAP(0x90008, MAP_STATIC, EV_KEY, BTN_START), /* Menu */
USAGE_MAP(0x90009, MAP_STATIC, EV_KEY, BTN_THUMBL), /* LS */
USAGE_MAP(0x9000A, MAP_STATIC, EV_KEY, BTN_THUMBR), /* RS */
// USAGE_MAP(0x90003, MAP_STATIC, EV_KEY, BTN_X), /* X */
USAGE_MAP(0x90004, MAP_STATIC, EV_KEY, BTN_X), /* Y */
USAGE_MAP(0x90005, MAP_STATIC, EV_KEY, BTN_Y), /* Y */

// The Flydigi Vader3 button set is different here
USAGE_MAP(0x90007, MAP_STATIC, EV_KEY, BTN_TL), /* LB */
USAGE_MAP(0x90008, MAP_STATIC, EV_KEY, BTN_TR), /* RB */
USAGE_MAP(0x9000B, MAP_STATIC, EV_KEY, BTN_SELECT), /* Back */
USAGE_MAP(0x9000C, MAP_STATIC, EV_KEY, BTN_START), /* Menu */

/* USAGE_MAP(0x90005, MAP_STATIC, EV_KEY, BTN_TL), /\* LB *\/ */
/* USAGE_MAP(0x90006, MAP_STATIC, EV_KEY, BTN_TR), /\* RB *\/ */
/* USAGE_MAP(0x90007, MAP_STATIC, EV_KEY, BTN_SELECT), /\* Back *\/ */
/* USAGE_MAP(0x90008, MAP_STATIC, EV_KEY, BTN_START), /\* Menu *\/ */

USAGE_MAP(0x9000E, MAP_STATIC, EV_KEY, BTN_THUMBL), /* LS */
USAGE_MAP(0x9000F, MAP_STATIC, EV_KEY, BTN_THUMBR), /* RS */

/* fixup the Xbox logo button */
USAGE_MAP(0x9000B, MAP_STATIC, EV_KEY, BTN_XBOX), /* Xbox */
// USAGE_MAP(0x9000B, MAP_STATIC, EV_KEY, BTN_XBOX), /* Xbox */

/* fixup the Share button */
USAGE_MAP(0x9000C, MAP_STATIC, EV_KEY, BTN_SHARE), /* Share */
// USAGE_MAP(0x9000C, MAP_STATIC, EV_KEY, BTN_SHARE), /* Share */

/* fixup code "Sys Main Menu" from Windows report descriptor */
USAGE_MAP(0x10085, MAP_STATIC, EV_KEY, BTN_XBOX),
Expand All @@ -142,6 +152,16 @@ static const struct usage_map xpadneo_usage_maps[] = {
/* map special buttons without HID bitmaps, corrected in event handler */
USAGE_MAP(0xC0081, MAP_STATIC, EV_KEY, BTN_PADDLES(0)), /* Four paddles */

// Flydigi Vader3 C and Z buttons
USAGE_MAP(0xC0036, MAP_STATIC, EV_KEY, BTN_C),
USAGE_MAP(0xC0084, MAP_STATIC, EV_KEY, BTN_Z),

// From left to right, the flydigi paddles
USAGE_MAP(0xC009C, MAP_STATIC, EV_KEY, BTN_TRIGGER_HAPPY1), /* Four paddles */
USAGE_MAP(0xC0089, MAP_STATIC, EV_KEY, BTN_TRIGGER_HAPPY2), /* Four paddles */
USAGE_MAP(0xC009D, MAP_STATIC, EV_KEY, BTN_TRIGGER_HAPPY3), /* Four paddles */
USAGE_MAP(0xC00B8, MAP_STATIC, EV_KEY, BTN_TRIGGER_HAPPY4), /* Four paddles */

/* hardware features handled at the raw report level */
USAGE_IGN(0xC0085), /* Profile switcher */
USAGE_IGN(0xC0099), /* Trigger scale switches */
Expand Down Expand Up @@ -865,20 +885,23 @@ static int xpadneo_input_configured(struct hid_device *hdev, struct hid_input *h

if (param_gamepad_compliance) {
hid_info(hdev, "enabling compliance with Linux Gamepad Specification\n");
abs_min = -32768;
abs_max = 32767;
deadzone = 12;
/* abs_min = -32768; */
/* abs_max = 32767; */
abs_min = -127;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If your model reports 0..255 as the axis range, you need to switch cases here for your model. But I rather suggest to instead rescale to the Xbox controller range and patch the HID reports accordingly because we patch the VID/PID during device discovery. Otherwise your changes won't match games expectations.

abs_max = 128;
}

input_set_abs_params(xdata->gamepad, ABS_X, abs_min, abs_max, 32, deadzone);
input_set_abs_params(xdata->gamepad, ABS_Y, abs_min, abs_max, 32, deadzone);
input_set_abs_params(xdata->gamepad, ABS_RX, abs_min, abs_max, 32, deadzone);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Probably the same as above: Do not change the keymap/axismap structure and this should be okay and stay untouched.

input_set_abs_params(xdata->gamepad, ABS_RY, abs_min, abs_max, 32, deadzone);
/* input_set_abs_params(xdata->gamepad, ABS_RX, abs_min, abs_max, 32, deadzone); */
/* input_set_abs_params(xdata->gamepad, ABS_RY, abs_min, abs_max, 32, deadzone); */

input_set_abs_params(xdata->gamepad, ABS_Z, 0, 1023, 4, 0);
input_set_abs_params(xdata->gamepad, ABS_RZ, 0, 1023, 4, 0);
input_set_abs_params(xdata->gamepad, ABS_Z, abs_min, abs_max, 32, deadzone);
input_set_abs_params(xdata->gamepad, ABS_RZ, abs_min, abs_max, 32, deadzone);

/* combine triggers to form a rudder, use ABS_MISC to order after dpad */
input_set_abs_params(xdata->gamepad, ABS_MISC, -1023, 1023, 3, 63);
// input_set_abs_params(xdata->gamepad, ABS_MISC, -1023, 1023, 3, 63);

/* do not report the keyboard buttons as part of the gamepad */
__clear_bit(BTN_SHARE, xdata->gamepad->keybit);
Expand Down Expand Up @@ -919,7 +942,8 @@ static int xpadneo_event(struct hid_device *hdev, struct hid_field *field,
case ABS_RY:
/* Linux Gamepad Specification */
if (param_gamepad_compliance) {
input_report_abs(gamepad, usage->code, value - 32768);
Copy link
Collaborator

@kakra kakra Jan 12, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This reverts param_gamepad_compliance. Do not change this. If you want to report 0..65535, disable param_gamepad_compliance instead. The reason behind this is that the HID descriptor says the axis have ranges from 0..65535 although it could perfectly fine say -32768..32767 (HID support negative values but they require a few more bytes, so that's probably why it wasn't used). But Linux gamepad spec says that gamepads need to have auto-centering sticks idling at position 0, so we just subtract 32768 - and also corrected the axis scale above.

// input_report_abs(gamepad, usage->code, value - 32768);
input_report_abs(gamepad, usage->code, value);
/* no need to sync here */
goto stop_processing;
}
Expand Down Expand Up @@ -1254,6 +1278,9 @@ static const struct hid_device_id xpadneo_devices[] = {

/* XBOX ONE Elite Series 2 */
{ HID_BLUETOOTH_DEVICE(USB_VENDOR_ID_MICROSOFT, 0x0B05) },

{ HID_BLUETOOTH_DEVICE(USB_VENDOR_ID_FLYDIGI, 0x0041)},

{ HID_BLUETOOTH_DEVICE(USB_VENDOR_ID_MICROSOFT, 0x0B22),
.driver_data = XPADNEO_QUIRK_SHARE_BUTTON },

Expand Down