-
Notifications
You must be signed in to change notification settings - Fork 2.3k
Implement SInput Device Support #13343
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
Conversation
Command inputs now are 0x03 All command outputs are 0x02
Power States
Assigned ID from Raspberry Pi was 0x10df updated from 0x10de
Ordering of bitfields isn't portable so you shouldn't use those |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks pretty much like #13327 so lgtm, but I may be biased here :P
Will let Sam do a deeper review of the new driver file but CI does seem happy with it; I think all the hidapi targets support the features used but I may be forgetting something.
Understood, I can update the necessary bits to ensure this is consistent from the driver side. |
Co-authored-by: Ethan Lee <[email protected]>
- Remove reliance on bitfield structures, use a fixed unpacking method - Exchange mappings - Implement PID for Firebird controller kit
Reliance on bitfield struct formats have been replaced with static unpacking methods for cross-platform support. |
- At the request of the firebird dev, I've went ahead and implemented all possible buttons - I updated the Firebird specific mapping which allows 6 additional buttons
- At the request of the firebird dev, I've went ahead and implemented all possible buttons - I updated the Firebird specific mapping which allows 6 additional buttons - Update GC Ultimate mapping
- Fixed more bad bracket formatting (oops) - Implement RGB Joystick command pass-through - Only send power state if it's supported/non-zero - Get rid of weird digital trigger handling - Change haptic support flag to rumble support to be more explicit - Implement feature report polling rate
- Remove reliance on paddle inputs. Simply send the digital and analog inputs.
This is looking pretty good. Let me know when you think it's ready for merging? |
I'll take care of those remaining reviews you've pointed out, then I'll spend this week testing and getting some more feedback from colleagues. Thank you for your thorough reviews and helping this shine. |
You’re welcome! |
- Buttons count is now derived from the button mask rather than supplying a separate count. This was done to reduce the chance of bugs with mismatch count compared to the mask. - Return NULL for devices lacking a mapping - Remove masks lookup table and use a bitshift max instead - Add sub-device for a gamepad (Super Gamepad+), which is out of production and does not have a unique PID - Remove unused joystick pointer reference in driver context
@slouken I've gotten enough feedback from some colleagues etc to feel comfortable merging provided that you're happy with everything and no further adjustments are suggested. I appreciate your time, looking forward to getting more devices supported down the line and I'm open to helping any way I can. |
You might want to switch gyro_elapsed_time to be uint32. Right now it can only represent up to 65 ms, which can be exceeded by wireless delay over Bluetooth. I know this is a USB protocol, but someone might want to implement it over Bluetooth at some point. |
Instead of (or in addition to) reporting the SDL gamepad type and subtype, it might make sense to report the face button style and include that in guid[15]. That way you can create controllers that have Xbox style face buttons, Nintendo style face buttons, GameCube style face buttons, etc. all without changing the protocol or requiring a new USB PID. |
- Sub-ID is broken into two 4 bit values - MSB represents a face style - LSB represents a sub-type
I've updated the protocol to contain face style and sub-ID information in GUID byte 15 per your recommendation. Timestamping will remain as-is for this. I think future changes on the driver-side could be implemented to contend with bluetooth lag, but I think it will be worth getting real gameplay feedback and seeing if it's even necessary. I have a feeling it will turn out just fine, because the timestamps are based on what's happening internally on the device since the packets are unreliable. |
Understood-- I've ensured it's NULL return for any invalid mappings. If there's anything else let me know, otherwise I'm happy with it. |
Merged! |
This implements a new SDL HID driver for a format developed by Hand Held Legend for their gamepad devices called SInput
Devices that are supported by this change with well-defined mappings
GC Ultimate ( https://gcultimate.com )
ProGCC ( https://handheldlegend.com/products/progcc-kit-wireless-wired-bundle )
The SInput format is documented here: https://github.com/HandHeldLegend/SInput-HID
Edit: Updated documentation to github.