sdl_joypad: fix hotplug, order ports #13527

Informatic · 2022-01-21T23:03:51Z

Description

Minor SDL joypad handling cleanup.

With joypad ignoring code used on webOS we were ending up with joypads registered at port 3+ on certain platforms. This also broke input port device index rebinding (Device Index list was always empty, even for devices on port 3+ which were working properly otherwise; not sure if missing reported devices is a violation of input device reporting API, or it's just a bug there)

This now properly attempts to store joypads continuously, by indexing them based on port number in an internal joypads state array.

Additionally, this fixes hotplug/unplug detection on SDL2. event.jdevice.which on unplug contains a Joystick Instance ID (monotonously increasing on every device connected/disconnected device), not Joystick Index used on open, thus a list of opened joysticks needs to be walked through and compared to received ID. Unplug detection would break after second replug before.

Note: this has not been tested outside of webOS, especially on SDL1.

Related Issues

[Any issues this pull request may be addressing]

Related Pull Requests

[Any other PRs from related repositories that might be needed for this pull request to work]

Reviewers

[If possible @mention all the people that should review your pull request]

TiZ-HugLife · 2023-08-14T04:53:38Z

Is there a particular reason this PR has sat dormant? Hotplugging does not work on Steam Deck. Emudeck configures SDL as the gamepad backend because the normal user doesn't have the permissions necessary for the udev backend to work. This seems like a particularly relevant fix for Deck.

hizzlekizzle · 2023-08-14T13:23:20Z

@sonninnos @LibretroAdmin @Myaats Any thoughts? (including Mats due to Steam/Deck angle)

RobLoach · 2024-04-17T22:06:36Z

input/drivers_joypad/sdl_joypad.c

@@ -55,10 +56,10 @@ static const char *sdl_joypad_name(unsigned pad)

 #ifdef HAVE_SDL2
   if (sdl_pads[pad].controller)
-      return SDL_GameControllerNameForIndex(pad);
-   return SDL_JoystickNameForIndex(pad);
+      return SDL_GameControllerNameForIndex(sdl_pads[pad].sdl_id);


Not really needed, but we could likely use SDL_GameControllerName here.

Suggested change

return SDL_GameControllerNameForIndex(sdl_pads[pad].sdl_id);

return SDL_GameControllerName(sdl_pads[pad].controller);

RobLoach · 2024-04-17T22:09:54Z

input/drivers_joypad/sdl_joypad.c

@@ -447,10 +475,20 @@ static void sdl_joypad_poll(void)
      switch (event.type)
      {
         case SDL_JOYDEVICEADDED:
+            RARCH_WARN("[SDL_JOYPAD] hotplug added: %d\n", event.jdevice.which);
            sdl_pad_connect(event.jdevice.which);
            break;
         case SDL_JOYDEVICEREMOVED:


Should we consider implementing SDL_ControllerDeviceEvents SDL_CONTROLLERDEVICEADDED and SDL_CONTROLLERDEVICEREMOVED?

RobLoach

Looks like a solid change and clean up to SDL's gamepad control system.

sonninnos · 2024-04-17T22:42:01Z

Most of those loggings don't really seem like "warnings" but rather regular "info"..

TiZ-HugLife · 2024-04-18T03:53:54Z

Thanks for the review, @RobLoach. I would love to see this merged in time for the next release.

RobLoach · 2024-04-18T16:43:55Z

input/drivers_joypad/sdl_joypad.c

+
+   for (int i = 0; i < MAX_USERS; i++) {
+       if (sdl_pads[i].joypad != NULL && sdl_pads[i].sdl_id == id) {
+           RARCH_WARN("[SDL_JOYPAD]: %d already registered on port %d\n", id, i);


Suggested change

RARCH_WARN("[SDL_JOYPAD]: %d already registered on port %d\n", id, i);

RARCH_LOG("[SDL_JOYPAD]: %d already registered on port %d\n", id, i);

RobLoach · 2024-04-18T16:44:25Z

input/drivers_joypad/sdl_joypad.c

+
+   for (int i = 0; i < MAX_USERS; i++) {
+       if (sdl_pads[i].joypad == NULL) {
+           RARCH_WARN("[SDL_JOYPAD]: using port: %d\n", i);


Suggested change

RARCH_WARN("[SDL_JOYPAD]: using port: %d\n", i);

RARCH_LOG("[SDL_JOYPAD]: using port: %d\n", i);

RobLoach · 2024-04-18T16:44:46Z

input/drivers_joypad/sdl_joypad.c

@@ -104,6 +123,7 @@ static void sdl_pad_connect(unsigned id)

   if (SDL_IsGameController(id))
   {
+      RARCH_WARN("[SDL_JOYSTICK]: opening #%d as GameController\n", id);


Suggested change

RARCH_WARN("[SDL_JOYSTICK]: opening #%d as GameController\n", id);

RARCH_LOG("[SDL_JOYSTICK]: opening #%d as GameController\n", id);

RobLoach · 2024-04-18T16:45:04Z

input/drivers_joypad/sdl_joypad.c

@@ -112,6 +132,7 @@ static void sdl_pad_connect(unsigned id)
   else
 #endif
   {
+      RARCH_WARN("[SDL_JOYSTICK]: opening #%d as Joystick\n", id);


Suggested change

RARCH_WARN("[SDL_JOYSTICK]: opening #%d as Joystick\n", id);

RARCH_LOG("[SDL_JOYSTICK]: opening #%d as Joystick\n", id);

RobLoach · 2024-04-18T16:45:20Z

input/drivers_joypad/sdl_joypad.c

-      RARCH_WARN("[SDL_JOYPAD]: Ignoring pad #%d (vendor: %d; product: %d)\n", id, vendor, product);
-      if (pad->joypad)
-         SDL_JoystickClose(pad->joypad);
+      RARCH_WARN("[SDL_JOYPAD]: Ignoring pad #%d (vendor: %04x; product: %04x)\n", id, vendor, product);


Suggested change

RARCH_WARN("[SDL_JOYPAD]: Ignoring pad #%d (vendor: %04x; product: %04x)\n", id, vendor, product);

RARCH_LOG("[SDL_JOYPAD]: Ignoring pad #%d (vendor: %04x; product: %04x)\n", id, vendor, product);

RobLoach · 2024-04-18T16:45:41Z

input/drivers_joypad/sdl_joypad.c

      return;
   }
 #endif
 #endif

+   pad->sdl_id = id;
+
+   RARCH_WARN("[SDL_JOYPAD]: registering %d as port %d\n", id, port_num);


Suggested change

RARCH_WARN("[SDL_JOYPAD]: registering %d as port %d\n", id, port_num);

RARCH_LOG("[SDL_JOYPAD]: registering %d as port %d\n", id, port_num);

RobLoach · 2024-04-18T16:46:02Z

input/drivers_joypad/sdl_joypad.c

@@ -447,10 +475,20 @@ static void sdl_joypad_poll(void)
      switch (event.type)
      {
         case SDL_JOYDEVICEADDED:
+            RARCH_WARN("[SDL_JOYPAD] hotplug added: %d\n", event.jdevice.which);


Suggested change

RARCH_WARN("[SDL_JOYPAD] hotplug added: %d\n", event.jdevice.which);

RARCH_LOG("[SDL_JOYPAD] hotplug added: %d\n", event.jdevice.which);

RobLoach · 2024-04-18T16:46:15Z

input/drivers_joypad/sdl_joypad.c

            sdl_pad_connect(event.jdevice.which);
            break;
         case SDL_JOYDEVICEREMOVED:
-            sdl_pad_disconnect(event.jdevice.which);
+            RARCH_WARN("[SDL_JOYPAD] hotplug removed: %d\n", event.jdevice.which);


Suggested change

RARCH_WARN("[SDL_JOYPAD] hotplug removed: %d\n", event.jdevice.which);

RARCH_LOG("[SDL_JOYPAD] hotplug removed: %d\n", event.jdevice.which);

RobLoach · 2024-04-18T16:46:50Z

input/drivers_joypad/sdl_joypad.c

+
+            for (int i = 0 ; i < MAX_USERS ; i++) {
+                if (SDL_JoystickInstanceID(sdl_pads[i].joypad) == event.jdevice.which) {
+                    RARCH_WARN("Found joystick: %d\n", i);


Suggested change

RARCH_WARN("Found joystick: %d\n", i);

RARCH_LOG("[SDL_JOYPAD] Found joystick: %d\n", i);

gouchi · 2024-07-29T21:31:12Z

@Informatic If you can resolve latest suggested changes.

Thank you.

RobLoach · 2024-08-30T03:13:10Z

The changes represented here have been brought into #16941 .

sdl_joypad: fix hotplug, order ports

a5ace01

FishOrBear mentioned this pull request Oct 20, 2023

Steam Deck - No inputs are detected if controller disconnects #14524

Open

RobLoach reviewed Apr 17, 2024

View reviewed changes

RobLoach approved these changes Apr 17, 2024

View reviewed changes

RobLoach reviewed Apr 18, 2024

View reviewed changes

RobLoach mentioned this pull request Aug 30, 2024

Update the SDL Joypad Input Drivery #16941

Draft

3 tasks

RobLoach closed this Aug 30, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

sdl_joypad: fix hotplug, order ports #13527

sdl_joypad: fix hotplug, order ports #13527

Informatic commented Jan 21, 2022 •

edited

Loading

TiZ-HugLife commented Aug 14, 2023 •

edited

Loading

hizzlekizzle commented Aug 14, 2023

RobLoach Apr 17, 2024

RobLoach Apr 17, 2024

RobLoach left a comment

sonninnos commented Apr 17, 2024

TiZ-HugLife commented Apr 18, 2024

RobLoach Apr 18, 2024 •

edited

Loading

RobLoach Apr 18, 2024

RobLoach Apr 18, 2024

RobLoach Apr 18, 2024

RobLoach Apr 18, 2024

RobLoach Apr 18, 2024

RobLoach Apr 18, 2024

RobLoach Apr 18, 2024 •

edited

Loading

RobLoach Apr 18, 2024

gouchi commented Jul 29, 2024 •

edited

Loading

RobLoach commented Aug 30, 2024

	return SDL_GameControllerNameForIndex(sdl_pads[pad].sdl_id);
	return SDL_GameControllerName(sdl_pads[pad].controller);

	RARCH_WARN("[SDL_JOYPAD]: %d already registered on port %d\n", id, i);
	RARCH_LOG("[SDL_JOYPAD]: %d already registered on port %d\n", id, i);

	RARCH_WARN("[SDL_JOYPAD]: using port: %d\n", i);
	RARCH_LOG("[SDL_JOYPAD]: using port: %d\n", i);

	RARCH_WARN("[SDL_JOYSTICK]: opening #%d as GameController\n", id);
	RARCH_LOG("[SDL_JOYSTICK]: opening #%d as GameController\n", id);

	RARCH_WARN("[SDL_JOYPAD]: Ignoring pad #%d (vendor: %04x; product: %04x)\n", id, vendor, product);
	RARCH_LOG("[SDL_JOYPAD]: Ignoring pad #%d (vendor: %04x; product: %04x)\n", id, vendor, product);

	RARCH_WARN("[SDL_JOYPAD]: registering %d as port %d\n", id, port_num);
	RARCH_LOG("[SDL_JOYPAD]: registering %d as port %d\n", id, port_num);

	RARCH_WARN("[SDL_JOYPAD] hotplug added: %d\n", event.jdevice.which);
	RARCH_LOG("[SDL_JOYPAD] hotplug added: %d\n", event.jdevice.which);

	RARCH_WARN("[SDL_JOYPAD] hotplug removed: %d\n", event.jdevice.which);
	RARCH_LOG("[SDL_JOYPAD] hotplug removed: %d\n", event.jdevice.which);

	RARCH_WARN("Found joystick: %d\n", i);
	RARCH_LOG("[SDL_JOYPAD] Found joystick: %d\n", i);

sdl_joypad: fix hotplug, order ports #13527

sdl_joypad: fix hotplug, order ports #13527

Conversation

Informatic commented Jan 21, 2022 • edited Loading

Description

Related Issues

Related Pull Requests

Reviewers

TiZ-HugLife commented Aug 14, 2023 • edited Loading

hizzlekizzle commented Aug 14, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

RobLoach left a comment

Choose a reason for hiding this comment

sonninnos commented Apr 17, 2024

TiZ-HugLife commented Apr 18, 2024

RobLoach Apr 18, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

RobLoach Apr 18, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

gouchi commented Jul 29, 2024 • edited Loading

RobLoach commented Aug 30, 2024

Informatic commented Jan 21, 2022 •

edited

Loading

TiZ-HugLife commented Aug 14, 2023 •

edited

Loading

RobLoach Apr 18, 2024 •

edited

Loading

RobLoach Apr 18, 2024 •

edited

Loading

gouchi commented Jul 29, 2024 •

edited

Loading