Add support for HyperX Clutch gamepad

[iiol]

No description provided.

[iiol]

Added support for HyperX Clutch
lsusb shows the following vid and pids:
Bus 003 Device 109: ID 03f0:038d HP, Inc HyperX Clutch
when connected with wireless connector and
Bus 003 Device 110: ID 03f0:048d HP, Inc HyperX Clutch
when connected with wire.

[nondebug]

Would XPAD_XBOX360_VENDOR(0x03f0) work? I'm looking at adding another HyperX Clutch gamepad and wondering why you used USB_DEVICE.

HyperX Clutch Gladiate:
Bus 003 Device 006: ID 03f0:0495 Hewlett-Packard HyperX Clutch Gladiate Wired Controller

[hxpanda]

Would we be able to add both:
XPAD_XBOX360_VENDOR(0x03f0) and XPAD_XBOXONE_VENDOR(0x03f0)

Vendor should be HP, Inc rather than Hewlett-Packard.

Do we also need to add support for the controller on the device list?
{ 0x03F0, 0x0495, "HyperX Clutch Gladiate Wired Controller", 0, XTYPE_XBOXONE },

[iiol]

Would XPAD_XBOX360_VENDOR(0x03f0) work? I'm looking at adding another HyperX Clutch gamepad and wondering why you used USB_DEVICE.

Isn't it a good practice to add it exact with usb device? What if it'll include other non-gamepad devices?

[HP-HX-maxster]

I had couple question about the device identifiers.

1. What is the difference between identifying the controller as XTYPE_XBOX360 as compared to XTYPE_XBOXONE?
2. What does XTYPE value "0" indicate?

[hxpanda]

Isn't it a good practice to add it exact with usb device? What if it'll include other non-gamepad devices?

I would agree that it's good to add exact device information. However, would that be the purpose of xpad_device[]? To help filter the devices?

Also curious whether any other vendors has ran into issues by adding only their vendor IDs to xpad_table[]?

Do we also need to add support for the controller on the device list?
{ 0x03F0, 0x0495, "HyperX Clutch Gladiate Wired Controller", 0, XTYPE_XBOXONE },

^ This was for xpad_device[].

[nondebug]

Isn't it a good practice to add it exact with usb device? What if it'll include other non-gamepad devices?

For the XPAD_XBOX360_VENDOR or XBOX_XBOXONE_VENDOR rules to match a device, the device needs to have a matching vendor ID and it also needs to have matching USB interfaces and protocols. It's possible that other HP devices use the same interfaces and protocols without being Xbox-compatible but I'd guess it's pretty unlikely. We don't seem to use USB_DEVICE rules for other vendors so my guess is no conflicting devices have been found yet.

Would we be able to add both: XPAD_XBOX360_VENDOR(0x03f0) and XPAD_XBOXONE_VENDOR(0x03f0)

That's correct if the vendor makes both Xbox 360 and Xbox One compatible devices.

I suspect HyperX Clutch is actually only Xbox 360 compatible since it doesn't advertise Xbox One compatibility. We should confirm it's Xbox One compatible before adding XPAD_XBOXONE_VENDOR. It would be helpful if someone with access to the device can share the output of lsusb.

I would agree that it's good to add exact device information. However, would that be the purpose of xpad_device[]? To help filter the devices?

xpad_device isn't used for filtering. xpad_table defines the filters, xpad_device provides supplemental information to improve compatibility for devices that already match the filters. The supplemental information includes the product name and flags that tweak the mapping. The xpad_table entry can also override xtype for controllers that need special handling, like wireless Xbox 360 controllers.

What is the difference between identifying the controller as XTYPE_XBOX360 as compared to XTYPE_XBOXONE?

Xbox 360 and Xbox One controllers have different capabilities and use different protocols. If xtype is set wrong then xpad will probably hit an error when trying to access a non-existent USB interface.

Note that if xtype is XTYPE_UNKNOWN then xpad tries to automatically detect the type based on the device's USB interfaces. This can happen if xpad_table matches a device but there's no matching entry in xpad_device. In that case the device will use the default name "Generic X-Box pad". (I suspect that's what @iiol sees with the current patch.)

What does XTYPE value "0" indicate?

Zero is XTYPE_XBOX which indicates a controller for the original Microsoft Xbox. The controllers used a proprietary interface based on USB but don't use USB plugs.

Do we also need to add support for the controller on the device list?

Yes, we should add an entry to xpad_device so the controller gets a unique product name.

Vendor should be HP, Inc rather than Hewlett-Packard.

Yes, the "Hewlett-Packard" string is old and should be "HP, Inc" on newer installs. The string was captured on a ChromeOS device with an outdated version of usb.ids. It looks like the database was updated in 2018.

[HP-HX-maxster]

When does a new commit of the xpad.c file get pushed out to the public?

[hxpanda]

xpad_device isn't used for filtering. xpad_table defines the filters, xpad_device provides supplemental information to improve compatibility for devices that already match the filters. The supplemental information includes the product name and flags that tweak the mapping. The xpad_table entry can also override xtype for controllers that need special handling, like wireless Xbox 360 controllers.

Sorry, yes you're right. This overlooked what the for-loop starting in Ln 2211 actually does. But from the looks of it, there isn't a need to specify each and every PID.

I suspect HyperX Clutch is actually only Xbox 360 compatible since it doesn't advertise Xbox One compatibility.

I think I can see were the confusion lies. The pull request is currently only for HyperX Clutch but we like to also add support for HyperX Clutch Gladiate which does support Xbox One.

To summarize, we should have the following then?
xpad_table:
{ XPAD_XBOX360_VENDOR(0x03f0) }, /* HP/HyperX X-Box 360 Controllers */
{ XPAD_XBOXONE_VENDOR(0x03f0) }, /* HP/HyperX X-Box One Controllers */
xpad_device:
{ 0x03F0, 0x0495, "HyperX Clutch Gladiate Wired Controller", 0, XTYPE_XBOXONE }

Zero is XTYPE_XBOX which indicates a controller for the original Microsoft Xbox. The controllers used a proprietary interface based on USB but don't use USB plugs.

I think @HP-HX-maxster is referring to the zero for the button mapping in
{ 0x03F0, 0x0495, "HyperX Clutch Gladiate Wired Controller", 0, XTYPE_XBOXONE }

[nondebug]

Oops, yes, it seems HyperX Clutch Gladiate is Xbox One compatible. I would expect this (assuming HyperX Clutch is also Xbox One compatible):

xpad_table:

{ XPAD_XBOXONE_VENDOR(0x03f0) },		/* HP/HyperX controllers */

xpad_device:

{ 0x03f0, 0x038d, "HyperX Clutch receiver", 0, XTYPE_XBOXONE },
{ 0x03f0, 0x048d, "HyperX Clutch", 0, XTYPE_XBOXONE },
{ 0x03f0, 0x0495, "HyperX Clutch Gladiate", 0, XTYPE_XBOXONE },

Or this (if HyperX Clutch is Xbox 360 compatible):

xpad_table:

{ XPAD_XBOX360_VENDOR(0x03f0) },		/* HP/HyperX Xbox 360 controllers */
{ XPAD_XBOXONE_VENDOR(0x03f0) },		/* HP/HyperX Xbox One controllers */

xpad_device:

{ 0x03f0, 0x038d, "HyperX Clutch receiver", 0, XTYPE_XBOX360 },
{ 0x03f0, 0x048d, "HyperX Clutch", 0, XTYPE_XBOX360 },
{ 0x03f0, 0x0495, "HyperX Clutch Gladiate", 0, XTYPE_XBOXONE },

But from the looks of it, there isn't a need to specify each and every PID.

I think xpad_device should have an entry for each PID since that's how xpad sets the gamepad's name string.

I think @HP-HX-maxster is referring to the zero for the button mapping in

The 0 for mapping means there are no special mapping flags for this device. xpad defines some flags to improve compatibility with DDR dance mats, arcade-style fight pads, and Xbox Elite rear paddles. I don't think these controllers need any special mapping flags.

When does a new commit of the xpad.c file get pushed out to the public?

I'm not sure, it seems this repo and the kernel's xpad have gotten out of sync. Maybe @paroj can clarify.

I think it is quicker to make the changes to the Linux kernel's xpad since that's the version most distros will use. When I added Amazon Luna controller, I added it to the kernel's xpad and @paroj merged it into this repo, but that might not be the preferred way.

[HP-HX-maxster]

After internal discussion, we will remove HyperX Clutch so that it just detects as a generic gamepad. So we would like the following to be added.

xpad_table:

{ XPAD_XBOX360_VENDOR(0x03f0) }, /* HP/HyperX Xbox 360 controllers /
{ XPAD_XBOXONE_VENDOR(0x03f0) }, / HP/HyperX Xbox One controllers */

xpad_device:

{ 0x03f0, 0x0495, "HyperX Clutch Gladiate", 0, XTYPE_XBOXONE },

Does Android and ChromeOS use the Linux kernel's xpad file? If it is quicker, then we can open our own pull request for the Linux kernel. It will be beneficial for us to learn to do so for future controllers as well. Let us know your thoughts @paroj.

[paroj]

So we would like the following to be added.

please create an according PR here

Does Android and ChromeOS use the Linux kernel's xpad file?

yes. SteamOS also.

If it is quicker, then we can open our own pull request for the Linux kernel.

You can skip this repo altogether and contribute to the kernel directly. However, you will need to use this process:
https://www.kernel.org/doc/html/v4.17/process/submitting-patches.html

which neither includes github nor a pull request.