-
-
Notifications
You must be signed in to change notification settings - Fork 115
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
Added Rumble Support for generic devices #620
Conversation
Note: The reason about pytest failling is because (I think) it originally didn't count about BlueRetro printing an Output usage. Below is the test Descriptor parsed by USB Descriptor and Request Parser, and I marked both Output which were ignored on previous version of BlueRetro: Test Descriptor
I didn't change the pytest files because I think that should be done by the maintainers after they accept the Pull, but it's just about updating the expecting string to |
It's minor but in my view a PR should make sure if they affect the test result expectation to modify them to the new expectation. This look really good. Does that make both left & right motor work in a sensible way? I wonder if that would work better than whatever I make for xbox? With an extra commit to fix the test, I would merge this. |
Yeah, I think the same. My guess is that the test didn't expect to read an output usage so that's why it's failing now, as BlueRetro printed an output that were ignored before. (As you can see, before every output usage would be completely ignored, while now it'll be threated as the same way as an input)
It should, but I need more devices to test it. My code assumes that every report field is a motor, so if there are five rumble motors, they all will be turned on/off in the same way. Right now BlueRetro doesn't pass much information to handle them separately. But I tried to use them all, like the delay, the cycle count, and the duration. Some of them I get from BlueRetro itself while the others are from Max/Min values.
I actually based my code from the Xbox one, and I thought the exactly the same, but, again, I don't have any Xbox One controller so I cannot test it neither.
As I said before, the only thing to "fix" the test should be to update the expected output adding the new Output usage |
Yes I know, I would expect you to be the one to modify that. You are the owner of the PR and it's failing the test ;) |
Ok, I modified the expected output according to what I get with a BlueRetro running on a ESP32 and with a BT Device with the exactly the same descriptor as the test. Right now it should be doing the test and I hope it should end successfully |
Ok I will look at more closely later, It doesn't make sense for the report # 1 usage offset to change IMHO. The rumble stuff should be like in another report. |
I think the output is on report # 1 because they're declared before changing the ID. As you can see, all of the inputs (Red) and both Outputs (Blue) are on the same Report (Yellow/1) as the Report 2 was declared after (Orange). About getting rumble from another report, that's completely from the device's side, as the manufacture defines the structure. For example, in Stadia, Google didn't use Collections while Microsoft did. The same thing for Output reports. In a perfect world, yes indeed, those reports should be in a different report, but you know where we live right now ¯_(ツ)_/¯ |
Sorry for my confusion, in my mind the LEDs output case was handled properly all along. So I was surprised when I saw it popping in and shifting all other key usage bit range by one bytes. That means keyboard where kind of broken since day one but it was kind of ok since having the Leds byte as the first key didn't make anything, and missing the last key just reduced the amount of simultaneous key press by one. Anyway your PR fix this! |
0e906fb
to
90df71d
Compare
Thanks again for the PR! |
No problem! I'm very happy to support this amazing project!!! |
Wow. Nice! Thank you @JPZV! I'll give it a try with my Stadia Controller. |
This adds support for generic HID controllers to have rumble Support (#611)
Every device which reports usages from
0F 95
to0F 9C
will be marked as a rumble-capable device (hid_parser.c#L196) along with0F 50
,0F 70
,0F 7C
and0F A7
(hid_parser.c#L197).Please note that there is no effect support, so usages from
0F 99
to0F 9C
have no use.The way it works is by getting the Maximum and the Minimum value for the report. In case the Rumble State is
off
, then the BlueRetro will send the minimum value to every usage. Otherwise, it'll send the maximum value multiplied by a multiplier constant (its value is from 0.0 to 1.0. I leave it there just in case) or the value fromfb_data
.This should work with generic controllers like the Stadia one, Chinese clones/alternatives, and projects like my BluControl (TBA) an BluN64, without needing to integrate them separately.
Main Changes:
RUMBLE
report idUSAGE_GEN_PHYS_INPUT
Usage PageHID_MI_OUTPUT
to the block ofHID_MI_INPUT
(hid_parser.c#L392)Tested with:
There shouldn't be any breaking change as I didn't touch anything related to the current functionalities. Also, I didn't note any type of input lag, and I tried to simplify as much as I could.
If you have any question or feedback, please let me know