-
Notifications
You must be signed in to change notification settings - Fork 22
Conversation
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.
Superb !! Thank you very much for this PR. People is gonna to love this. The hat/Dpad with predefined buttons are genius. However, you need to change the descriptor to HID_PHYSICAL_MAX_N ( 315 ,2)
since 1 byte is not enough to describe the 315 value.
tinyusb/src/class/hid/hid.h
Outdated
GAMEPAD_BUTTON_MODE = TU_BIT(12), ///< Mode button | ||
GAMEPAD_BUTTON_THUMBL = TU_BIT(13), ///< L3 button | ||
GAMEPAD_BUTTON_THUMBR = TU_BIT(14), ///< R3 button | ||
GAMEPAD_BUTTON_ = TU_BIT(15), ///< Undefined button |
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.
I think we could remove GAMEPAD_BUTTON_
undefined button, this can be used by user as custom as they want.
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.
Ok, let's keep it commented to inform the user the TU_BIT(15) is available if needed.
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.
that is even better
For reference, I just pulled out my PS4 controller and here is its descriptor. Seem like they use hat with unit of degree for analog pad. To be honest, I am not too family with gamepad. I am glad to receive this PR, this will help lots of other people if we could get it work out of the box with most OS. I also found the linux gamepad specification as well, which is very useful https://www.kernel.org/doc/html/latest/input/gamepad.html
|
@hathach Do you think it would better to follow the PS4 Gamepad HID descriptor and adding the missing Rx and Ry variables? If so I will do it. I don't have many idea about HID descriptors and bluetooth but I make this work following the mouse example already written. xD I think it could be interesting to add aswell the battery status descriptor, but I don't know how to do it right now. By the way, which software did you used for extracting the PS4 descriptor? It looks very cool. |
To be honest, I don't use gampepad much and don't fully understand the usage of Rx and Ry. If they are common used then we can surely add it.
HID control page (0x06) does have and battery strength. Though seem like PS4 just use its own vendor defined feature. Though HID usage is rather complicated. It may need a bit of testing, and checking to see how other implement battery status. LED page does has low battery indicator, but it doesn't show the percentage like the above battery strength but look like much easier and could be found more common on consumer device to reverse engineer. Though I think we could leave this battery for another PR I guess. |
I just pulled out another generic/cheap gamepad that laying around just for the reference. They all seem to have X, Y, Z, Rz, Hat before buttons. Maybe it is a good idea to change ours to match the std as well. We could just add Rx, Ry as well, after buttons (again seem to be common layout for gamepad).
|
You are aweseome hathach, thank you very much for all that information. I will modify all to add the rX and rY support and match the "standard" PS4 report. I'm agree to left the battery level feature for another PR. |
I have done a bit of testing with my PS4 controller:
combined Microsoft Xbox controller here that is organized as X,Y, RX, RY, Z, RZ, Hat, Button. I think it make more sense for us to change the layout. I much prefer the layout of PS4 since it is more popular (at least where I live). I will make the changes myself :)
|
The hid reports' order doesn't matters, it's the same to have Buttons, X, Y, Z, RX, RY, RZ, Hat and X,Y, RX, RY, Z, RZ, Hat, Button. What changes is the representation, as PS4 uses uint8_t for X, Y, Z meanwhile Linux/Windows uses int8_t. That's may why we need parsers or external drivers in order to use de PS4/Xbox Controllers on PC. I preferred do it as is to simplify the code because PS4 / Xbox splits the Rx / Ry from the X, Y, in different collection descriptors. Meanwhile the ble controller and the examples works I'm happy with the order you choose. xD From the reports you present here I extract the following representation:
|
Thanks for the sum up, yeah, I know the field order is not important and is defined by hid descriptors. Though I still want to stick with most common format. All x,y,z and its rotate along with hat is mostly fixed, button has potential to grow bigger, therefore moving it to last make sense for me. Also x,y,z,rx seem to stick to each other. So my intended order would be
|
Ok, let's do it. |
wow, look like I push a bit late than you. was doing the same thing :D. I do a merge and push to add the D-pad explanation from linux site, which I found very useful for new user, Hope you don't mind :) |
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.
Thank you very much for this great PR, it will definitely helps lots of user with gamepad usage for both USB and BLE.
yes ahahahahaa thank you for adding the extra information |
@nekuneko This got merged to the core, though actually this repo Note: the local copy here is a bit older than the main repo. |
It's ok @hathach, if you can make the changes faster than me, just do it. :) Please don't forget to review adafruit/Adafruit_nRF52_Arduino#622 and adafruit/Adafruit_TinyUSB_Arduino#72 PR. |
Ok, thanks. Those are next PRs I will review 👍 |
Changes needed to add basic gamepad support. See PR #622 from
adafruit/Adafruit_nRF52_Arduino
repo.