Skip to content
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

(Help) Pro-guitar Controller Support #16

Open
2 of 3 tasks
Tracked by #4
EliteAsian123 opened this issue Mar 11, 2023 · 9 comments
Open
2 of 3 tasks
Tracked by #4

(Help) Pro-guitar Controller Support #16

EliteAsian123 opened this issue Mar 11, 2023 · 9 comments

Comments

@EliteAsian123
Copy link
Member

EliteAsian123 commented Mar 11, 2023

Current, the only supported pro-guitar is the WII fender mustang. Obviously, not everyone has a WII one, so I'd like to add support for the Xbox and PS3 fender mustangs. Problem is, since I don't have access to those devices, I don't know the PID and VID for those. This means that YARG can't detect those devices.

If anyone can provide me with the PID and VID's for the Xbox and/or PS3 fender mustang, that would be greatly appreciated. Thanks.

Support for Wii, PS3, and Xbox Fender Mustangs should be working, however I need people to test them. Below are a list of controllers that are confirmed to work:

  • Wii Fender Mustang
  • PS3 Fender Mustang
  • Xbox Fender Mustang

If anyone has any of the missing ones and would like to test it out, please let me know.

@EliteAsian123
Copy link
Member Author

Okay, I found this wonderful repo that has a bunch of information on pro-guitar PID's and VID's!

@EliteAsian123
Copy link
Member Author

In commit 9796998, support for the PS3 and Xbox guitars were added. Now we just need people to test it out.

@EliteAsian123
Copy link
Member Author

Xbox pro-guitar confirmed to work by Discord member.

@EliteAsian123 EliteAsian123 mentioned this issue Mar 26, 2023
19 tasks
@afestini
Copy link

No dice with the PS3 version. Just like the keyboard, it only sends the first few bytes by default unless the proper control transfer is sent. I spent days trying to make that work with HID for the keyboard and now another 2 days with a USB sniffer. The descriptor insists that the out report is 9 bytes and either WinAPI or their driver stubbornly truncates the report accordingly.

After giving up and going the WinUSB route, it's no issue to make it work, but it's obviously not user friendly. "Go and install extra tools to force another driver" is a bit of a pain. In my case, I include inf/cat files exported from Zadig/libwdi to make it easier, but I wouldn't be surprised if some security feature blocks them on other machines (plus, the signature is only valid until 2029).

In your case, it also seems that you only handle HID devices, since my Mustang is no longer showing up in-game after switching to WinUSB. Not sure if you consider it worth the trouble compared to a simple "please connect them as MIDI devices".

@afestini
Copy link

Small update after losing my mind and what was left of my hair. I FINALLY managed to get the PS3 Keyboard and ProGuitar to work in Windows as their default HID self. No more messing around with drivers.

After trying everything under the sun and finally understanding how to send the SetFeature message, they key is that the 40 bytes report to configure them is 5 reports and each needs to be preceded by a 0 report id. The MS API/driver will fail on anything less than the feature report size in the descriptor (9 bytes) and truncate everything above.

Now that it works it looks so obvious and simple, it feels embarrassing it took so long to figure it out. In code:

array<uint8_t, 9> set_config_reports[] {
	{0x00,	0xE9, 0x00, 0x89, 0x1B, 0x00, 0x00, 0x00, 0x02},
	{0x00,	0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00},
	{0x00,	0x00, 0x00, 0x00, 0x00, 0x00, 0x80, 0x00, 0x00},
	{0x00,	0x00, 0x00, 0x89, 0x00, 0x00, 0x00, 0x00, 0x00},
	{0x00,	0xE9, 0x01, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00}};

for (auto& report : set_config_reports)
	HidD_SetFeature(handle, report.data(), static_cast<ULONG>(report.size()));

Side note: I experimented a bit and I can skip the 2nd and 3rd one. However, I didn't check all the feature beyond the keys. I wonder if anyone has any insight what exactly those configurations mean?

@TheNathannator
Copy link
Collaborator

TheNathannator commented May 31, 2023

Interesting, I was under the impression that these feature reports were for enabling the MIDI messages, not the HID reports. Or does sending it multiple times toggle between the two?

Unfortunately the Unity input system doesn't support setting/getting feature reports, so that's not currently possible to send in YARG unless we just entirely replace the HID support with our own (which currently has to be done for Linux anyways, so it wouldn't be too difficult to change that).

@afestini
Copy link

afestini commented Jun 1, 2023

I'm mostly just piecing it together from other projects. My observation of the messages is that without sending the control transfer, only the first few bytes of the HID report are filled (basically the ones for regular controllers). The remaining bytes are all fixed (mostly 0 and some 2s). That's why all the regular buttons work, but none of the "instrument inputs".

After sending the transfers, the report updates all the bytes correctly. No idea why they aren't sent by default like for Wii, so I can only guess that maybe the PS3 handled controllers in a way that required the extra steps.

For use as a MIDI device they usually switch automatically when plugged in. I'm still trying to make sense of the midi notes on the guitar, though. With the buttons only telling you the fret/note on release, I don't think hammer-ons would work.

Side note, since the PlasticBand repo was/is extremely useful. We had surprisingly similar structs for the reports (except I didn't realize I can have unnamed bit fields and had tons of pointless unused1/27...).

One issue with the guitar:
uint8_t lowEFret : 5;
uint8_t aFret : 5;
uint8_t dFret : 5;
bool : 1;

Maybe it's undefined behavior and compiler specific, but having a bitfield spanning across variables didn't work. I tried the above after realizing my uint16_t wasn't aligned and everything was shifted due to padding (and just really wanted to avoid pragma pack). In the end I had to use the pragma and uint16_t.

@TheNathannator
Copy link
Collaborator

TheNathannator commented Jun 2, 2023

In the end I had to use the pragma and uint16_t.

I do note in the docs readme that all the structures are assumed to be byte-packed and unaligned, but I should probably add that explicitly to each of them. I use the structs as just an easy way to lay out the data and don't worry too much about actual compilability, but I also probably shouldn't rely on people having to read the readme in order for them to know the assumptions I use lol

@afestini
Copy link

afestini commented Jun 2, 2023

Oh, the alignment/padding already threw me off before I found your repo, but that was easy to spot after noticing everything is shifted after a given point. It was the part doing
uint8_t : 5;
uint8_t : 5;

That made me wonder "wait, you can do that? That would be much cleaner." Unfortunately you can't and 5:5:5:1 turns into 3 bytes instead of 2. Hence the uint16_t with pragma pack, since I don't see another way to make it work.

Purplo-cf pushed a commit to Purplo-cf/YARG that referenced this issue May 14, 2024
…ial#16)

* Add Pro Guitar track/note info to Moonscraper code

* Implement Pro Guitar parsing
Only has feature parity with YARG's current Pro Guitar parser for now

* Add unit tests for Pro Guitar parsing

* Fix Pro Guitar note value retrieval
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants