Bug 216225

Summary:	hid-nintendo player LED behavior is wrong
Product:	Drivers	Reporter:	tinozzo123
Component:	Input Devices	Assignee:	drivers_input-devices
Status:	RESOLVED IMPLEMENTED
Severity:	normal	CC:	nf.pereira
Priority:	P1
Hardware:	All
OS:	Linux
Kernel Version:	5.18.9	Subsystem:
Regression:	No	Bisected commit-id:

Description tinozzo123 2022-07-09 10:00:02 UTC

The behavior of the Nintendo Switch Pro Controller player LED on Linux is this: the first time it's connected the first led turns on, the second time it's connected the first two LEDs (not just the second) turn on, the third time the first three LEDs (not just the third) turn on, the fourth time all LEDs (not just the fourth) turn on, and then it cycles back to the beginning.

This is completely different from the behavior when connected to the Switch console, where only one player LED ever turns on, and it's the one associated with the player (first LED for player one, second LED for player two...), not the number of times it's been connected.

It seems that joycond, a userspace daemon, does fix this, but there shouldn't be the need of using a daemon for something simple like this.

I don't know if from the kernel it's possible to get the "player number", but it would still be an improvement to only have one LED turning on.

Tested on a Pro Controller 050000007e0500000920000001800000, don't own a Joy-Con (both are managed by hid-nintendo).

Finally, this is just my opinion.

Comment 1 tinozzo123 2022-07-10 20:05:06 UTC

I gave a look to hid-nintendo.c to see why it behaves like this. From what I think I've gathered:

The static variable that controls the LED number is `input_num`.
It increases by 1 (and resets to 1 when it reaches 5) every time after LEDs are initialized (`joycon_leds_create`). It never decreases.
Finally, the reason why multiple LEDs are turned on instead of just one, seems to be in `led->brightness = ((i + 1) <= input_num) ? 1 : 0;` (it's a `<=` instead of a `==`).

Instead of doing this, would it be possible to have a static variable that increases by 1 when a controller is connected (`nintendo_hid_probe`) and decreases by 1 when disconnected `nintendo_hid_remove`? (With the necessary mutexes, of course.)

I don't know how to test kernel stuff, so that's why I'm just leaving this comment.

Comment 2 tinozzo123 2022-07-11 09:26:13 UTC

I just realized that using one global counter would have a problem.
If two controllers are connected, player one disconnects, then a controller is connected, they will both be "player two" according to the leds.

The solution would be to store the controllers in a list, and when a controller disconnects either:
 - All the player of a higher number will shift to the left, and the leds updated accordingly;
 - Leave an empty slot. When a controller is connected, it takes the first empty slot available (this is what's done on the Switch console).

Comment 3 nf.pereira 2022-10-31 22:50:43 UTC

Yes, any of those 2 solutions would be better than the current implementation IMO.

Comment 4 tinozzo123 2024-07-22 11:35:16 UTC

Fixed in Linux 6.10.