Add support for the Home LED on NS Controllers#115114
Add support for the Home LED on NS Controllers#115114Repiteo merged 1 commit intogodotengine:masterfrom
Conversation
|
Hello, thank you for your work! I feel like we can also change the behaviour of SDL to make the existing code work with Switch controllers, it doesn't look too hard to fix, see also: godot/thirdparty/sdl/joystick/hidapi/SDL_hidapi_switch.c Lines 1617 to 1628 in 741fb8a godot/thirdparty/sdl/joystick/hidapi/SDL_hidapi_switch.c Lines 734 to 753 in 741fb8a Maybe we can make the Switch driver report having a SDL_PROP_JOYSTICK_CAP_MONO_LED_BOOLEAN capability, similarly to how PS4 controllers work:godot/thirdparty/sdl/joystick/hidapi/SDL_hidapi_ps4.c Lines 891 to 906 in 741fb8a godot/thirdparty/sdl/joystick/hidapi/SDL_hidapijoystick.c Lines 802 to 816 in 741fb8a What do you think? :) Another reason I suggest doing this inside SDL is because the current approach of changing the hint will (probably) modify the home LED of all connected Switch controllers, not just one that was used for |
|
Hey, thanks for your response!
I think that we could, it seems to be the best option now that you mentioned it.
Now that you've mentioned it, yes, I can't really test it since I don't have two or more controllers with this capability but as the hint is changed without providing the joypad/gamepad, it probably changes every controllers LED lights. Also thanks @AThousandShips for the suggested change, it really is better. |
|
As discussed and suggested by @Nintorch , I tried changing the Switch SDL file and it seems to work properly, I just can't test it on multiple controllers but it may work as intended now.
I should note that from my tests, the Pro Controller Home LED has different levels of brightness instead of a continuous range. It isn't an issue but I feel like this should be known. |
|
Looks good to me! May I ask if the code works if you connect both the Pro Controller and the right Joy Con and call |
|
AFAIK it should work as both Pro Controllers and Joy Cons use the same base file ( * The NS2 Joy Cons and NS2 Pro Controllers don't have this LED so they wouldn't work whatsoever. |
|
Ohh, I see! |
Feel free to make the pull request. I need to think about whether this makes sense, since the home LED is treated differently from a mono LED, but I can think about that later once you've proposed it. |
|
@ThiLelito Would you like to make a PR for SDL or would you like me to make it for you, if you want? :) |
|
@Nintorch Feel free to make it :) |
Nintorch
left a comment
There was a problem hiding this comment.
Tested locally, it works as expected! Great job! :D
(Off-topic, but before you made this PR I didn't even know Switch controllers had home LEDs :D )
|
Could you squash your commits? See our pull request guidelines for more information |
Changed the implementation Instead of modifying the hint value, now it works like the PS4 ones, but using MONO instead of RGB. NS Controllers LED Support Added
66f6dd1 to
6ff5f44
Compare
Done :) |
|
aww, it's not in 4.6? sad, because i can't get my mom's ps4 controllers to connect, lol |
|
Thanks! Congratulations on your first merged contribution! 🎉 |
Add support for the Home LED on NS Controllers
With the addition of
Input.set_joy_light()from #111681, I wondered if the LED around the Home Button in some Nintendo Switch controllers could be adjusted. I discovered they could not, because its implementation uses theSDL_SetJoystickLEDfunction while the Home LED uses theSDL_JOYSTICK_HIDAPI_SWITCH_HOME_LEDhint.This PR aims to add support for this LED mentioned.
I only changed the
drivers/sdl/joypad_sdl.cppfile:close_joypadto check if it's a NS controller and turn the LED off if true:SDL_JOYSTICK_HIDAPI_SWITCH_HOME_LEDhint to"0.0";has_joy_lightto make NS controllersreturn true;SDL_JOYSTICK_HIDAPI_SWITCH_HOME_LEDhint with thechar *of the maximum value.I've tested on Windows and it seems to work. I don't know if there are other controllers that use the same hint and are not mentioned in the if statements. If so, feel free to change or improve it.