This probably merits a fork and cleanup of irrelevant code - maybe pulling in the xow driver as a reference for dongle support, and xpad for usb support (although I didn't even test wired, as I wanted to use wireless).

Hey all - just got a Flydigi Vader 3 Pro a couple hours ago, and as I already did the relevant work to make it usable over bluetooth locally (by using this repo as a reference point, since I had tried it for the xbox elite series 2 a couple weeks ago) - I am creating this draft PR to get advice - do you think there would be benefit to support generic xbox controllers in this code?

Would it be more fitting for me to try to push some of the corrected button mappings to the Linux kernel? (the ones that it registers by default on dinput mode are...odd - no doubt problems on the hardware side - but the Z key comes through as a Return press).

If neither of those seem ideal, maybe a license respecting fork where I focus on more Flydigi Vader 3 support would be beneficial to the controller community?

When this project started, did rumble work with the xbox remotes? That's the one thing that still hasn't worked on this one (rumble + bluetooth) - rumble works via the dongle (but only on x-input mode, not dinput) - and of course, x-input blocks the 6 special keys this controller has from registering, as they're bound controller-side by the app (and can't do multi-key combos).

The linux hid-generic driver with the dongle does not work well however, it doesn't seem to show any activity at all under evtest, but maybe I need to add a custom udev rule.

Code Climate has analyzed commit 091d8bc6 and detected 0 issues on this pull request.

View more on Code Climate.

Hey all - just got a Flydigi Vader 3 Pro a couple hours ago, and as I already did the relevant work to make it usable over bluetooth locally (by using this repo as a reference point, since I had tried it for the xbox elite series 2 a couple weeks ago) - I am creating this draft PR to get advice - do you think there would be benefit to support generic xbox controllers in this code?

As pointed out above: Do not try to mask the controller as an Xbox controller using xpadneo if you don't accept to exactly match the Xbox controller spec for the 6 axis and first 10 buttons.

If you can change your code to match exactly that spec and expose the other buttons in the trigger happy range of key codes, then we can accept the code into xpadneo and allow it to be masked as an Xbox controller using our VID/PID patching technique.

Would it be more fitting for me to try to push some of the corrected button mappings to the Linux kernel? (the ones that it registers by default on dinput mode are...odd - no doubt problems on the hardware side - but the Z key comes through as a Return press).

The problem seems to be that hid-generic doesn't care about game input devices having more buttons than the BTN_GAMEPAD range, and then additional buttons bleed into non-related ranges. A proper patch for the kernel should move these bits to another range, e.g. trigger happy.

If neither of those seem ideal, maybe a license respecting fork where I focus on more Flydigi Vader 3 support would be beneficial to the controller community?

A fork MUST NOT use our VID/PID patching technique, otherwise it would create a mess in the SDL controller support. You can use the driver as a base, with support for VID/PID patching ripped out (or changed to other IDs) and support for MS devices ripped out, too. But then you are probably left with a thin driver that does hardly more than hid-generic.

When this project started, did rumble work with the xbox remotes?

Yes. It was one of the initial ideas of creating this driver in the first place because back at that time, the kernel did not support rumble for Bluetooth controllers (and still doesn't for some models). And also, button mapping was a mess because of very creative HID descriptors which changed with each firmware update - thanks Microsoft.

That's the one thing that still hasn't worked on this one (rumble + bluetooth) - rumble works via the dongle (but only on x-input mode, not dinput) - and of course, x-input blocks the 6 special keys this controller has from registering, as they're bound controller-side by the app (and can't do multi-key combos).

You need to find the HID report ID in the HID descriptor for rumble. Look in the docs folder to get an idea how to find it. In theory, we could detect the ID by looking at the descriptor. But currently it's hardwired to ID 0x03 (in the header file via a macro).

Also, additional buttons MUST NOT bleed into other keymap ranges (see input-event-codes.h in the kernel). This is really a problem with the hid-generic driver but also some other drivers, e.g. hid-microsoft doesn't care about this problem for controllers but some other devices. If this happens, rather move those buttons to the trigger happy range, then talk to the SDL devs to properly auto-assign these to their correct function. This obviously needs a specialized thin HID driver.

The linux hid-generic driver with the dongle does not work well however, it doesn't seem to show any activity at all under evtest, but maybe I need to add a custom udev rule.

Yes, you may need to add a udev rule. In GIP mode (USB or dongle), it may not create an event device but just an input and hidraw device (via the kernel). A udev rule could fix that. Or the device is just missing a proper udev tag/property which obviously a rule could also fix.

Also, hid-generic won't load a rumble driver or implement rumble by itself. hid-microsoft is a driver which uses hid-generic but implements model-specific quirks and also attaches a rumble driver for supported models. You could create a similar driver (actually, I based some ideas of xpadneo off of hid-microsoft and hid-logitech-{dj,hidpp}). Every driver which implements support for known VID/PID will automatically use that driver instead of falling back to hid-generic, at least in newer kernels and if the module is loaded already. For older kernels or unloaded modules, you need to rebind the driver with udev rules as we do in our rules. Obviously, this is rather intended for modules built into the kernel (so VID/PID matching the specialized driver is already known to the kernel).

@kakra thank you so much for taking time to wade through this "Frankenstein" of a patch :laughing: (such an apt term).

This is my first foray into anything driver/controller related, so your feedback is very appreciated.

What is VID/PID? I see the acronym was used often.

Obviously, this effort has been a lot of guess/check/trial-and-error, where I'm slowly understanding more of the inner workings as I fiddle with the current code.

If I can get the rumble to work (trying that today), given that you seem somewhat receptive to the concept (not necessarily the current code :smile: ) I would be interested in cleaning it up and doing it the "right way" to get into the driver itself, and moving beyond "break things to verify the POC is possible".

I do like the controller registering in my limited game tests as an Xbox one (it does so over dongle with the default drivers as well) - at least, in my test game (FF14 via wine), as that causes the in game controller detection and bindings to work out of the box.

It sounds like for the alternatively mapped codes (where many of the codes this controller produces overlap with other codes the Xbox defines) - instead of altering the top level usage map as I have done, there is a different layer (HID?) where I can make special accommodations to alter the codes from this controller to the expected Xbox codes?

@kakra thank you so much for taking time to wade through this "Frankenstein" of a patch 😆 (such an apt term).

This is my first foray into anything driver/controller related, so your feedback is very appreciated.

You're welcome.

What is VID/PID? I see the acronym was used often.

Vendor ID, product ID... It's an 8-digit hex ID used to identify the vendor/product/model in the form of vvvv:pppp, like 045e:028e for a Microsoft (045e) Xbox Controller (028e).

We use PID patching to mask the device as a different device to work around mismatching controllerdb entries in SDL, and also to trick games into using the generic controller layout of the original Xbox controller which has exactly 10 buttons. This way we bypassed problems with games seeing wrong mappings in either dinput or xinput. Nowadays, dinput is mostly gone, older games may use it and Proton probably ships work-arounds for that (dinput doesn't have analog triggers).

Obviously, this effort has been a lot of guess/check/trial-and-error, where I'm slowly understanding more of the inner workings as I fiddle with the current code.

Yes, it will take some effort to wrap your head around that - I've gone through this for a long time. The key is understanding the central role of the mapping struct: This is only used to mask off mappings bits we do not want to show or emulate, and it's used to turn on bits for buttons we emulate/moved. The magic happens in the event handlers which actually swap or move bits in the reports, patch the descriptor, and steal input events from the kernel default handler and process them internally instead.

If I can get the rumble to work (trying that today), given that you seem somewhat receptive to the concept (not necessarily the current code 😄 ) I would be interested in cleaning it up and doing it the "right way" to get into the driver itself, and moving beyond "break things to verify the POC is possible".

There's a plan to support non-Xbox controllers after v0.11 or v0.12. With v0.10 I will reshape the code to be more convenient and modular so we can better support new concepts. That's the time when your concept could actually land in the code. But until then we have to be careful how to present the device to user-space, otherwise we will create an almost unsolvable mess with SDL and Steam Input again.

I've understood your code as POC and thus didn't raise any concerns there, I just wanted to give hints where to move lines properly for better integration. Code style seems fine, and cleanup will follow. It should be possible to get it into a state where it doesn't interfere with functions of existing devices. But as said, there will be refactoring with v0.10 which would require you to manually re-apply your changes to the codebase.

Since v0.11 or v0.12 is far into the future, a separate driver may be more useful. Since some buttons are part of the Switch controller layout, you could look into the Switch HID driver, too. Maybe it's more easy to put some patches there.

Whatever you do: If you plan on modifying an existing driver and plan to merge such a patch as part of the driver, ensure that you do not insert new buttons within existing keymaps because SDL doesn't look at kernel key symbols but it rather counts buttons.

So if we have this bitmap (as xpadneo uses):

ABCXYZ...
110110...

and you enable the C and Z bit, you would make C the third button, that moves X and Y to position 4 and 5, but SDL would continue to think that button 3 and 4 are X and Y, and never see button 5 and 6, and thus your button C would act as X in SDL, and button Y and Z would be dead (or rather, bleed into the following gamepad buttons, and cut off later buttons instead, but you get the idea). I collected a lot of thoughts and notes about that in #286.

Instead, you would need to keep BTN_Z (etc) disabled in the mapping struct but enable trigger happy buttons instead, then either patch the HID report to shuffle bits around as needed, or change the input handler to synthesize the trigger happy events (that's actually a name of a bit range in input-event-codes.h) if you're actually seeing the C bit (etc) set in the HID report.

I do like the controller registering in my limited game tests as an Xbox one (it does so over dongle with the default drivers as well) - at least, in my test game (FF14 via wine), as that causes the in game controller detection and bindings to work out of the box.

Then there's no other way then ensuring we only have the default 10 Xbox buttons in the lower bit range - do not enable bits for C,Z or other none-Xbox-buttons. If you want to still enable the buttons, you'd have to do some tricks in the HID report. You can enable mapping bits beyond BTN_TRIGGER_HAPPY, and then synthesize those events to reports these buttons to user-space.

Important: The XINPUT spec of Windows supports at most 16 buttons. 10 are from the original Xbox layout, an 11th bit is used for the Guide button but v0.10 moves that to a different sub-device. So there are six bits left, XINPUT puts the digipad into this bitmap, too: 4 bits, see https://learn.microsoft.com/en-us/windows/win32/api/xinput/ns-xinput-xinput_gamepad. So this leaves two additional bits which are usually already occupied by e.g. a Share button. But this also doesn't belong in the XINPUT spec so v0.10 moves the Share button to a sub-device, too.

SDL works a little bit different and would have a hard time using sub-devices. So the plan is to move the sub-device buttons back into the main device bitmap but to the trigger happy range, otherwise with xpadneo, SDL cannot see any button beyond the 10 standard buttons. I think, the Guide button already moved to back to the main device.

It sounds like for the alternatively mapped codes (where many of the codes this controller produces overlap with other codes the Xbox defines) - instead of altering the top level usage map as I have done, there is a different layer (HID?) where I can make special accommodations to alter the codes from this controller to the expected Xbox codes?

The struct just enables what user-space sees: It tells user-space which buttons are actually on the controller. The problem is: SDL ignores the symbols attached to the bitmap positions, it just counts buttons. For some older Xbox controllers it expects button 0,1,2,3 for A,B,X,Y, and for newer controllers it expects button 0,1,3,4 for A,B,X,Y - thus it skips a seemingly dead/blind C button. BTW, which is why we patch the PID so we go with the original bitmap layout which also many old games expect on the jsdev device (which is crafted from the evdev device by the kernel).

To respect the original Xbox button layout, all non-default buttons must be enabled in the trigger happy range (otherwise they bleed into the range for digitizer pens). SDL nowadays has heuristics to properly detect our trigger happy buttons by - again - counting enabled bits in the bitmap. So new buttons must go after the range of bits already set, otherwise the mapping implied by SDL and most games shifts across buttons.

You only get proper events if you adjust everything properly. You'd start with only adding your C/Z button (and maybe others) to the end of the struct, using proper trigger happy positions. Then change to event and raw handlers to properly return events if you see those bits. Use a sync event if you're done reporting a full frame of events.

A frame of events is used the following way: The kernel receives a raw HID report, then generates one event per keymap bit in the report, and for each axis change in the report. It then syncs the event to tell user-space that the report has been fully processed. If you don't sync between button press and release, user-space would not see a change in the button state (because it would be pressed and released within the same frame), or rather: it will see just the last event from that frame for that button, and if the resulting state doesn't differ from the previous frame, that event didn't happen. If you sync too often, user-space cannot detect that all events come from a single report packet - but that's probably never an issue. A series of events makes up such a frame, a sync signals the end of the frame.

BTW: The struct only enables base-model buttons. There's logic in the device setup functions which enable model specific mapping bits or disable bits that we report through sub-devices (xpadneo_input_configured(), __{clear,set}bit(), model specific keymap bits go there, not into the struct). See also xpadneo/keyboard.c and similar to see how to enable bits for sub-devices created on demand. SDL actually uses the set of enabled bits to heuristically identify the real controller model and properly setup it's own button mappings.

I think I created a parsed report that matches other controllers:

HID Descriptor for Flydigi Vader3 Pro (Linux mode)

Hex dump of the controller descriptor:

# xxd -c20 -g1 /sys/module/hid_xpadneo/drivers/hid:xpadneo/*:D7D7:0041.*/report_descriptor

05 01 09 05 a1 01 85 01 a1 02 09 30 09 31 09 32 09 35 15 80
25 7f 35 80 45 7f 75 08 95 04 81 02 09 40 09 41 09 42 09 43
15 00 26 ff 00 75 08 95 04 81 02 75 04 95 01 15 01 25 08 46
3b 01 65 14 09 39 81 42 65 00 05 09 95 0c 75 01 25 01 15 00
09 01 09 02 09 04 09 05 09 07 09 08 09 09 09 0a 09 0b 09 0c
09 0e 09 0f 81 02 05 0c 09 36 09 84 09 b8 09 9c 09 9d 09 89
09 8d 09 65 09 82 09 8a 75 01 95 0f 81 02 05 0c 09 69 75 01
95 01 81 02 05 02 15 00 26 ff 00 09 c5 09 c4 95 02 75 08 81
02 c0 c0

Parsed descriptor: (via https://eleccelerator.com/usbdescreqparser/)

0x05, 0x01,        // Usage Page (Generic Desktop Ctrls)
0x09, 0x05,        // Usage (Game Pad)
0xA1, 0x01,        // Collection (Application)
0x85, 0x01,        //   Report ID (1)
0xA1, 0x02,        //   Collection (Logical)
0x09, 0x30,        //     Usage (X)
0x09, 0x31,        //     Usage (Y)
0x09, 0x32,        //     Usage (Z)
0x09, 0x35,        //     Usage (Rz)
0x15, 0x80,        //     Logical Minimum (-128)
0x25, 0x7F,        //     Logical Maximum (127)
0x35, 0x80,        //     Physical Minimum (-128)
0x45, 0x7F,        //     Physical Maximum (127)
0x75, 0x08,        //     Report Size (8)
0x95, 0x04,        //     Report Count (4)
0x81, 0x02,        //     Input (Data,Var,Abs,No Wrap,Linear,Preferred State,No Null Position)
0x09, 0x40,        //     Usage (Vx)
0x09, 0x41,        //     Usage (Vy)
0x09, 0x42,        //     Usage (Vz)
0x09, 0x43,        //     Usage (Vbrx)
0x15, 0x00,        //     Logical Minimum (0)
0x26, 0xFF, 0x00,  //     Logical Maximum (255)
0x75, 0x08,        //     Report Size (8)
0x95, 0x04,        //     Report Count (4)
0x81, 0x02,        //     Input (Data,Var,Abs,No Wrap,Linear,Preferred State,No Null Position)
0x75, 0x04,        //     Report Size (4)
0x95, 0x01,        //     Report Count (1)
0x15, 0x01,        //     Logical Minimum (1)
0x25, 0x08,        //     Logical Maximum (8)
0x46, 0x3B, 0x01,  //     Physical Maximum (315)
0x65, 0x14,        //     Unit (System: English Rotation, Length: Centimeter)
0x09, 0x39,        //     Usage (Hat switch)
0x81, 0x42,        //     Input (Data,Var,Abs,No Wrap,Linear,Preferred State,Null State)
0x65, 0x00,        //     Unit (None)
0x05, 0x09,        //     Usage Page (Button)
0x95, 0x0C,        //     Report Count (12)
0x75, 0x01,        //     Report Size (1)
0x25, 0x01,        //     Logical Maximum (1)
0x15, 0x00,        //     Logical Minimum (0)
0x09, 0x01,        //     Usage (0x01)
0x09, 0x02,        //     Usage (0x02)
0x09, 0x04,        //     Usage (0x04)
0x09, 0x05,        //     Usage (0x05)
0x09, 0x07,        //     Usage (0x07)
0x09, 0x08,        //     Usage (0x08)
0x09, 0x09,        //     Usage (0x09)
0x09, 0x0A,        //     Usage (0x0A)
0x09, 0x0B,        //     Usage (0x0B)
0x09, 0x0C,        //     Usage (0x0C)
0x09, 0x0E,        //     Usage (0x0E)
0x09, 0x0F,        //     Usage (0x0F)
0x81, 0x02,        //     Input (Data,Var,Abs,No Wrap,Linear,Preferred State,No Null Position)
0x05, 0x0C,        //     Usage Page (Consumer)
0x09, 0x36,        //     Usage (Function Buttons)
0x09, 0x84,        //     Usage (Enter Channel)
0x09, 0xB8,        //     Usage (Eject)
0x09, 0x9C,        //     Usage (Channel Increment)
0x09, 0x9D,        //     Usage (Channel Decrement)
0x09, 0x89,        //     Usage (Media Select TV)
0x09, 0x8D,        //     Usage (Media Select Program Guide)
0x09, 0x65,        //     Usage (Snapshot)
0x09, 0x82,        //     Usage (Mode Step)
0x09, 0x8A,        //     Usage (Media Select WWW)
0x75, 0x01,        //     Report Size (1)
0x95, 0x0F,        //     Report Count (15)
0x81, 0x02,        //     Input (Data,Var,Abs,No Wrap,Linear,Preferred State,No Null Position)
0x05, 0x0C,        //     Usage Page (Consumer)
0x09, 0x69,        //     Usage (0x69)
0x75, 0x01,        //     Report Size (1)
0x95, 0x01,        //     Report Count (1)
0x81, 0x02,        //     Input (Data,Var,Abs,No Wrap,Linear,Preferred State,No Null Position)
0x05, 0x02,        //     Usage Page (Sim Ctrls)
0x15, 0x00,        //     Logical Minimum (0)
0x26, 0xFF, 0x00,  //     Logical Maximum (255)
0x09, 0xC5,        //     Usage (Brake)
0x09, 0xC4,        //     Usage (Accelerator)
0x95, 0x02,        //     Report Count (2)
0x75, 0x08,        //     Report Size (8)
0x81, 0x02,        //     Input (Data,Var,Abs,No Wrap,Linear,Preferred State,No Null Position)
0xC0,              //   End Collection
0xC0,              // End Collection

// 163 bytes

Unlike most the others in the docs directory, this seems to only have a single report id (changing the hardcoded header value from 0x03 to 0x01 does not cause rumble to function).

Is there anything that can be done (doing some offset in this report_id to the right area?) - or is this output indicative of a lack of support at the hardware level for rumble over bluetooth?

Also, is the report_descriptor impacted/produced by the driver itself (in which case it would always be incorrect here, as I am piggybacking on the xbox driver for a non-xbox controller), or is it coming from a lower level (direct device hardware data via the kernel or something)?

Also, managed to get my first kernel panic that crashed my desktop in 15 years of GNU/Linux usage by fiddling with rumble settings that were not valid :joy:

I think I created a parsed report that matches other controllers: [...] Parsed descriptor: (via https://eleccelerator.com/usbdescreqparser/) [...] Unlike most the others in the docs directory, this seems to only have a single report id (changing the hardcoded header value from 0x03 to 0x01 does not cause rumble to function).

Yes, there is no rumble function in the descriptor. Report 1 is a report that sends data from the device to the computer. You cannot use it in the other direction. As you can see, the other docs have Output in the description, which describes data being sent from the computer to the device. Rumble reports could be detected by looking at the description of output values 0..100 repeated at least 2 times (main motor, weak motor, but left and right trigger motor is also supported) and 3 timings (attack, sustain, release in units of 10^-2 seconds). Also, it's defined in another usage page.

Is there anything that can be done (doing some offset in this report_id to the right area?) - or is this output indicative of a lack of support at the hardware level for rumble over bluetooth?

There's no such thing as a fantasy offset into the report. The report clearly defines report sizes (in bits) and report counts for different usages (buttons, axes, ...) and a communication direction (input to the host, output from the host). Each of those report usage definitions (sizes, counts) accounts for the offset into the report.

And yes, this means: no rumble for this controller in Bluetooth mode...

Also, is the report_descriptor impacted/produced by the driver itself (in which case it would always be incorrect here, as I am piggybacking on the xbox driver for a non-xbox controller), or is it coming from a lower level (direct device hardware data via the kernel or something)?

Yes and no. The original descriptor comes from the HID device. xpadneo does patch that before user-space sees or the kernel parses it - so yes, it is partially crafted by the driver. Actually, there exist HID devices without HID descriptors. In that case, a binary HID descriptor blob would be hard-coded into the driver. Something similar happens in drivers which support devices with known-broken HID descriptors: sometimes it's easier to just replace it instead of patching it.

Xbox controller HID descriptors have one quirk in common: They end with a bogus null byte. In my patcher, I remove it if I detect it. I also look at specific byte offsets to replace the bytes with HID descriptions that more closely match what Linux user-space expects from an Xbox controller.

Also, managed to get my first kernel panic that crashed my desktop in 15 years of GNU/Linux usage by fiddling with rumble settings that were not valid 😂

This is easy: If you don't close all handles or fail to free memory, or use memory after free, the kernel will probably explode more or less silent in front of you. Dangling handles often cause the kernel to freeze on shutdown because it waits for an event that never happens. Or if you free memory with dangling handles, another subsystem may access freed memory and cause a memory access error which usually results in a kernel panic.

Sometimes it may be better to spawn your test kernel with the module in a qemu machine with a directly booted kernel.

@kakra I did a from-scratch implementation vs a fork, but I did re-use modified versions of your install.sh/uninstall.sh/Makefiles - I hope that's alright (there should not be any remnants that cross over with xpadneo, beyond the notice I put in the README.md to let users know what I assumed were yours/original project author's copyrights on those supplementary files).

https://github.com/ahungry/vader3

In doing that, I got a lot more insight into the raw_event vs event areas, and managed to take your advice (remapping in events vs changing things in the input_mapping), as well as using the TRIGGER_HAPPY ranges.

If/when you want to add official third party xbox-mode controller support for generic devices, feel free to take anything you see for this project (the default bindings to these new ones are worked out at least) or give me a ping and I can aim for that official pull request.

Thanks again!

atar-axis / xpadneo

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

HID Descriptor for Flydigi Vader3 Pro (Linux mode)