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

sdl_joypad: fix hotplug, order ports #13527

Closed
Closed
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
80 changes: 59 additions & 21 deletions input/drivers_joypad/sdl_joypad.c
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@ typedef struct _sdl_joypad
#ifdef HAVE_SDL2
unsigned num_balls;
#endif
int sdl_id;
} sdl_joypad_t;

/* TODO/FIXME - static globals */
Expand All @@ -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);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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);

return SDL_JoystickNameForIndex(sdl_pads[pad].sdl_id);
#else
return SDL_JoystickName(pad);
return SDL_JoystickName(sdl_pads[pad].sdl_id);
#endif
}

Expand Down Expand Up @@ -91,9 +92,27 @@ static int16_t sdl_pad_get_axis(sdl_joypad_t *pad, unsigned axis)
return SDL_JoystickGetAxis(pad->joypad, axis);
}

// id is a SDL-side id
static void sdl_pad_connect(unsigned id)
{
sdl_joypad_t *pad = (sdl_joypad_t*)&sdl_pads[id];
int port_num = -1;

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);
Copy link
Member

@RobLoach RobLoach Apr 18, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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);

return;
}
}

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

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

port_num = i;
break;
}
}

sdl_joypad_t *pad = (sdl_joypad_t*)&sdl_pads[port_num];
bool success = false;
int32_t product = 0;
int32_t vendor = 0;
Expand All @@ -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);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

pad->controller = SDL_GameControllerOpen(id);
pad->joypad = SDL_GameControllerGetJoystick(pad->controller);

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

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

pad->joypad = SDL_JoystickOpen(id);
success = pad->joypad != NULL;
}
Expand All @@ -124,6 +145,7 @@ static void sdl_pad_connect(unsigned id)
SDL_JoystickClose(pad->joypad);

pad->joypad = NULL;
pad->controller = NULL;

return;
}
Expand All @@ -139,23 +161,29 @@ static void sdl_pad_connect(unsigned id)
product = guid_ptr[1];
#endif
#ifdef WEBOS
RARCH_WARN("[SDL_JOYPAD] #%d: vendor: %04x; product: %04x\n", id, vendor, product);
if (vendor == 0x9999 && product == 0x9999)
{
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);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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);

if (pad->controller)
SDL_GameControllerClose(pad->controller);

pad->joypad = NULL;
pad->controller = NULL;
return;
}
#endif
#endif

pad->sdl_id = id;

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

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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);


input_autoconfigure_connect(
sdl_joypad_name(id),
sdl_joypad_name(port_num),
NULL,
sdl_joypad.ident,
id,
port_num,
vendor,
product);

Expand Down Expand Up @@ -189,7 +217,7 @@ static void sdl_pad_connect(unsigned id)
}

pad->haptic = NULL;

if (g_has_haptic)
{
pad->haptic = SDL_HapticOpenFromJoystick(pad->joypad);
Expand Down Expand Up @@ -222,26 +250,26 @@ static void sdl_pad_connect(unsigned id)
#endif
}

static void sdl_pad_disconnect(unsigned id)
static void sdl_pad_disconnect(unsigned port_num)
{
#ifdef HAVE_SDL2
if (sdl_pads[id].haptic)
SDL_HapticClose(sdl_pads[id].haptic);
if (sdl_pads[port_num].haptic)
SDL_HapticClose(sdl_pads[port_num].haptic);

if (sdl_pads[id].controller)
if (sdl_pads[port_num].controller)
{
SDL_GameControllerClose(sdl_pads[id].controller);
input_autoconfigure_disconnect(id, sdl_joypad.ident);
SDL_GameControllerClose(sdl_pads[port_num].controller);
input_autoconfigure_disconnect(port_num, sdl_joypad.ident);
}
else
#endif
if (sdl_pads[id].joypad)
if (sdl_pads[port_num].joypad)
{
SDL_JoystickClose(sdl_pads[id].joypad);
input_autoconfigure_disconnect(id, sdl_joypad.ident);
SDL_JoystickClose(sdl_pads[port_num].joypad);
input_autoconfigure_disconnect(port_num, sdl_joypad.ident);
}

memset(&sdl_pads[id], 0, sizeof(sdl_pads[id]));
memset(&sdl_pads[port_num], 0, sizeof(sdl_pads[port_num]));
}

static void sdl_joypad_destroy(void)
Expand Down Expand Up @@ -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);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

sdl_pad_connect(event.jdevice.which);
break;
case SDL_JOYDEVICEREMOVED:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we consider implementing SDL_ControllerDeviceEvents SDL_CONTROLLERDEVICEADDED and SDL_CONTROLLERDEVICEREMOVED?

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

@RobLoach RobLoach Apr 18, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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


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);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

sdl_pad_disconnect(i);
break;
}
}

break;
}
}
Expand Down