-
Notifications
You must be signed in to change notification settings - Fork 2.3k
Switch 2 Controller Support #13327
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
base: main
Are you sure you want to change the base?
Switch 2 Controller Support #13327
Conversation
Well, my preferred name would be just Nohzockt or Noah, no email or something special. I feel honoured that you ask me. Thanks a lot. |
Done! I also cleaned up the button reading and made a working gamepad mapping, once the left/right stick are working this is ready to review. |
Quick question. I was able to generate an SDL3.dll file with your WIP stub, would an SDL2.dll be able to be generated as well or this still a work in progress? I am wondering how these can be built for executables like emulators or steam API. |
I have some devices I can test in addition to the GC controller so I might want to add those to this PR if possible. I can create a branch off of your branch, or add them after this is merged. |
Whatever works! I can add commits to this PR if Sam doesn't get to it first. |
int size; | ||
while ((size = SDL_hid_read_timeout(device->dev, packet, sizeof(packet), 0)) > 0) { | ||
if (size > 14) { | ||
Uint64 timestamp = SDL_GetTicksNS(); |
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.
This block should be refactored out into a separate function to handle devices with varying capabilities and report formats.
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 believe all NS2 devices report the same layout (passing 0 for unsupported axes like the GCN triggers) but @endrift can confirm this.
040524d
to
01f8dea
Compare
Does this support Bluetooth? What changes would need to be made for rumble and gyro support? |
I haven't looked over the code yet, but I tested the controller and discovered a few things that might want improvements:
|
Sadly no, it seems to communicate entirely via a proprietary BLE service so we would need to communicate directly via dbus+bluez or something. Rumble/Gyro come down to figuring out any more possible bulk output init commands or hid output commands to get more out of the endpoint than what it shares with us today. |
Rumble is partially documented here: https://gist.github.com/shinyquagsire23/66f006b46c56216acbaac6c1e2279b64#file-00_ns2_sidechannel_notes-txt-L285 |
We probably need to get some sort of factory calibration from the controller, like is done with the Switch 1 controllers.
Yes, share should be mapped to MISC1 and the C button should be mapped to MISC2. |
Huh, I thought I put capture at misc1 and C at misc2, but testcontroller doesn't show these so I couldn't confirm visually. I can confirm the stick range isn't saturated but I wasn't sure if there was a way to get a calibration range just yet. The triggers are showing full range though, but maybe I'm not looking closely enough. |
Testing adding the USB ID for the Pro controller (0x2069) to this, it doesn't actually show up as functional. I haven't dug further yet. Also, Misc1 definitely shows up in testcontroller for the Xbox One controller. |
Misc2 doesn't show up in the gamepad image, but it will show up in the left-hand input list if it's mapped. |
Odd... does anything seem off with the mapping added to gamepad.c? |
Latest push is confirmed working with misc1/misc2. |
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.
Some minor things I noticed, but until calibration is figured out I don't think this will be ready.
const unsigned char DEFAULT_REPORT_DATA[] = { | ||
0x03, 0x91, 0x00, 0x0d, 0x00, 0x08, | ||
0x00, 0x00, 0x01, 0x00, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF | ||
}; | ||
const unsigned char SET_LED_DATA[] = { | ||
0x09, 0x91, 0x00, 0x07, 0x00, 0x08, | ||
0x00, 0x00, 0x01, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00 | ||
}; |
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.
Might want to use some enums for things like the first byte (the command) and the second byte (the direction), but that's not needed for the initial merge.
Thanks to kiddkaffeine for the Xcode updates!
Thanks to Nohzockt for the initial libusb init and hidapi polling work!
Latest push addresses most of the review items, left the question for Sam and didn't touch the init commands just yet; presumably we need at least one more to get it to show us calibration data so we can probably make the change at that point. |
Are you on ndeadly's discord where a bunch of discussion about reversing has been going on? If not, it's probably a good idea to join. |
Went ahead and joined, probably won't be able to add very much under the circumstances but I'll keep up as best as I can! |
I've made a PR on your branch to get the procon working too. However, confining the bulk endpoint comms to hid.c won't be viable for this driver. Things like rumble go over that endpoint, so we need a way to plumb it through to the actual driver. |
timestamp, | ||
joystick, | ||
(Uint8) i, | ||
(packet[buttons[i].byte] & buttons[i].mask) != 0 |
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.
After the latest change to the buttons
array, it seems fairly redundant from a purely functional standpoint.
(packet[buttons[i].byte] & buttons[i].mask) != 0 | |
(packet[3 + (i >> 3)] & (1U << (i & 7))) != 0 |
Merged the ProCon2 support and renamed this accordingly. |
Fixes #13178.
Device init and polling works, but I haven't translated the packets yet since
the public code doesn't line up with the automatic mappingsand I'm intentionally avoiding doing any further reverse engineering myself. Happy to add patches to get this PR working though!udev rule for Linux:
CC @Nohzockt: If you have a preferred name/email for
Co-authored-by
I'll apply it to the patch that implements the init/polling work.