darthcloud / BlueRetro

Multiplayer Bluetooth controllers adapter for retro video game consoles
https://blueretro.io
Apache License 2.0
1.23k stars 103 forks source link

Rumble doesn't work on Stadia controller #974

Closed asweintraub closed 2 months ago

asweintraub commented 2 months ago

BlueRetro firmware version

1.9.1 and 24.04 beta 21

BlueRetro firmware specification

HW1

BlueRetro firmware variant

Universal

BlueRetro hardware type

External adapter with detachable cord

Manufacturer

blue-retro.com

System used

Nintendo 64

Bluetooth controller brand & name

Google Stadia

What is problem? (only list ONE problem per report)

Vibrations don't work with the Stadia controller.

Previous bugs for this issue were closed, so reopening here.

I don't have a linux machine set up to pull the HID report from the controller, but assuming this HID report is correct, the issue seems to be that USAGE_PAGE16 is not handled in a way that supports the Stadia controller's HID report.

In particular, it looks like the Stadia controller is sending 0x06, 0x0F, 0x00, // Usage Page (PID Page) instead of 0x05, 0x0F, // Usage Page (PID Page) which would be properly handled with the existing code.

I'm going to see if I can get a dev environment set up to test this, but I think changing the USAGE_PAGE16 code to the following might fix the problem:

hid_stack[hid_stack_idx].usage_page = *desc;
desc += 2;

What did you expect to happen?

I would expect vibrations to work with the Stadia controller.

Attach files like logs or Bluetooth traces here

No response

darthcloud commented 2 months ago

I'm very open to receiving PRs ;) Hopefully this help a bit:

Usage page is only stored as a 8bits value in the hid_parser stack: https://github.com/darthcloud/BlueRetro/blob/3c45c1577f4a3d372808838908b1251bc72f875d/main/adapter/hid_parser.c#L16-L22

Would make sense to upgrade it to 32bits since everything else is that, including the global structure used in all other module: https://github.com/darthcloud/BlueRetro/blob/3c45c1577f4a3d372808838908b1251bc72f875d/main/adapter/adapter.h#L469-L477

You would need to cast the pointer to uint16_t * to grab the full value.

hid_stack[hid_stack_idx].usage_page = *(uint16_t *)desc;;
desc += 2;

Then after that you would likely need to edit the generic rumble code to support that usage_page/usage combination: https://github.com/darthcloud/BlueRetro/blob/3c45c1577f4a3d372808838908b1251bc72f875d/main/adapter/wireless/hid_generic.c#L700-L786

asweintraub commented 2 months ago

Thanks for the pointers! I probably won't have time to get around to this for a few weeks, since I don't have the ESP-IDF environment set up and will be quite busy. That said, it looks like a good idea to support 32-bit usage pages, so I'll give that a shot when I have the chance.

Either way, it looks as if the values are little-endian, so 0x06, 0x0F, 0x00 should be synonymous with 0x05, 0x0F, meaning we shouldn't have to modify anything else for this to work with Stadia. In fact, the largest usage page I can find in the HID docs is 0x91, so switching this to 32-bits (instead of dropping the extra bytes) is good for future-proofing, but otherwise unnecessary.

darthcloud commented 2 months ago

Yes indeed it was simply the same value using a bigger sized var.

Rumble.work now!

https://github.com/darthcloud/BlueRetro/actions/runs/8808673421

asweintraub commented 2 months ago

Nice! I didn't realize you had already merged the fix in. I modified the code earlier today but wasn't able to test it until just now. Thank you!

darthcloud commented 2 months ago

Fixed in v24.04