Skip to content

sinput: refactor to make unknown controllers fully dynamic #13593

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.

Sign up for GitHub

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

Closed
wants to merge 1 commit into from
Closed

sinput: refactor to make unknown controllers fully dynamic #13593

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
8 changes: 2 additions & 6 deletions src/joystick/SDL_gamepad.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -812,7 +812,8 @@ static GamepadMapping_t *SDL_CreateMappingForHIDAPIGamepad(SDL_GUID guid)
// GC Ultimate Map
SDL_strlcat(mapping_string, "a:b0,b:b2,back:b11,dpdown:h0.4,dpleft:h0.8,dpright:h0.2,dpup:h0.1,guide:b12,leftshoulder:b6,leftstick:b4,lefttrigger:a4,leftx:a0,lefty:a1,misc1:b13,misc2:b14,rightshoulder:b7,rightstick:b5,righttrigger:a5,rightx:a2,righty:a3,start:b10,x:b1,y:b3,misc3:b8,misc4:b9,hint:!SDL_GAMECONTROLLER_USE_GAMECUBE_LABELS:=1,", sizeof(mapping_string));
break;

default:
case USB_PRODUCT_BONJIRICHANNEL_FIREBIRD:
case USB_PRODUCT_HANDHELDLEGEND_SINPUT_GENERIC:
// Apply mapping profile for type
switch (sinput_id) {
Expand Down Expand Up @@ -840,11 +841,6 @@ static GamepadMapping_t *SDL_CreateMappingForHIDAPIGamepad(SDL_GUID guid)
break;
}
break;

default:
case USB_PRODUCT_BONJIRICHANNEL_FIREBIRD:
// Unmapped devices
return NULL;
}
} else {
// All other gamepads have the standard set of 19 buttons and 6 axes
Expand Down
136 changes: 88 additions & 48 deletions src/joystick/hidapi/SDL_hidapi_sinput.c
View file
Open in desktop
Copy link
Contributor

Choose a reason for hiding this comment

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

I think that the changes here, which opt to make the sub-types global for all SInput devices, is not quite the direction I think it should go.

I do think that it would be wiser to have those 32 available to any owner of a particular PID. Seems more extensible that way.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is doable, but it is something i am more likely to be able to fix in #13565

Can we move the discussion for this there?

The only thing this PR does is centralize sub_type handling for the generic, firebird, and other devices. E.g., sub_types should do something by default. It does not preclude using special sub_types per vid:pid, and that is something that can be worked into the switch statements (as is done for the two known controllers).

Original file line number Diff line number Diff line change
Expand Up @@ -239,69 +239,119 @@ static void ProcessSDLFeaturesResponse(SDL_HIDAPI_Device *device, Uint8 *data)

// Obtain protocol version
ctx->protocol_version = EXTRACTUINT16(data, 0);

// Bitfields are not portable, so we unpack them into a struct value
ctx->rumble_supported = (data[2] & 0x01) != 0;
ctx->player_leds_supported = (data[2] & 0x02) != 0;
ctx->accelerometer_supported = (data[2] & 0x04) != 0;
ctx->gyroscope_supported = (data[2] & 0x08) != 0;

ctx->left_analog_stick_supported = (data[2] & 0x10) != 0;
ctx->right_analog_stick_supported = (data[2] & 0x20) != 0;
ctx->left_analog_trigger_supported = (data[2] & 0x40) != 0;
ctx->right_analog_trigger_supported = (data[2] & 0x80) != 0;

ctx->touchpad_supported = (data[3] & 0x01) != 0;
ctx->joystick_rgb_supported = (data[3] & 0x02) != 0;

ctx->is_handheld = (data[3] & 0x04) != 0;

Uint8 *fflags = data + 2;
Uint8 *buttons = data + 12;

//
// Unpack feature flags into context
//
ctx->rumble_supported = (fflags[0] & 0x01) != 0;
ctx->player_leds_supported = (fflags[0] & 0x02) != 0;
ctx->accelerometer_supported = (fflags[0] & 0x04) != 0;
ctx->gyroscope_supported = (fflags[0] & 0x08) != 0;

// Axes cannot be dynamic, so we only sanity check them
bool left_analog_stick_supported = (fflags[0] & 0x10) != 0;
bool right_analog_stick_supported = (fflags[0] & 0x20) != 0;
bool left_analog_trigger_supported = (fflags[0] & 0x40) != 0;
bool right_analog_trigger_supported = (fflags[0] & 0x80) != 0;

ctx->touchpad_supported = (fflags[1] & 0x01) != 0;
ctx->joystick_rgb_supported = (fflags[1] & 0x02) != 0;
ctx->is_handheld = (fflags[1] & 0x04) != 0;

//
// Gamepad Info
//
SDL_GamepadType type = SDL_GAMEPAD_TYPE_UNKNOWN;
type = (SDL_GamepadType)SDL_clamp(data[4], SDL_GAMEPAD_TYPE_UNKNOWN, SDL_GAMEPAD_TYPE_COUNT);
device->type = type;

// The 3 MSB represent a button layout style SDL_GamepadFaceStyle
// The 5 LSB represent a device sub-type
device->guid.data[15] = data[5];

ctx->sub_type = (data[5] & 0x1F);

ctx->polling_rate_ms = data[6];
#if defined(DEBUG_SINPUT_INIT)
SDL_Log("SInput Face Style: %d", (data[5] & 0xE0) >> 5);
SDL_Log("SInput Sub-type: %d", (data[5] & 0x1F));
#endif

//
// IMU Info
//
ctx->polling_rate_ms = data[6];
ctx->accelRange = EXTRACTUINT16(data, 8);
ctx->gyroRange = EXTRACTUINT16(data, 10);

if ((device->product_id == USB_PRODUCT_HANDHELDLEGEND_SINPUT_GENERIC) && (device->vendor_id == USB_VENDOR_RASPBERRYPI)) {

#if defined(DEBUG_SINPUT_INIT)
SDL_Log("SInput Face Style: %d", (data[5] & 0xE0) >> 5);
SDL_Log("SInput Sub-type: %d", (data[5] & 0x1F));
#endif

switch (ctx->sub_type) {
// Default generic device, exposes all buttons
//
// Get mappings based on SDL subtype and assert that they match.
//

// For backwards compatibility with existing firmwares
// when we know the PID, we will passthrough the mask.
// sub_type is only 5 bits, so 0xff can be used.
bool known = (
device->product_id == USB_PRODUCT_HANDHELDLEGEND_PROGCC ||
device->product_id == USB_PRODUCT_HANDHELDLEGEND_GCULTIMATE
);
if (known)
ctx->sub_type = 0xFF;

switch (ctx->sub_type) {
case 0xFF:
ctx->usage_masks[0] = buttons[0];
ctx->usage_masks[1] = buttons[1];
ctx->usage_masks[2] = buttons[2];
ctx->usage_masks[3] = buttons[3];
ctx->left_analog_stick_supported = left_analog_stick_supported;
ctx->right_analog_stick_supported = right_analog_stick_supported;
ctx->left_analog_trigger_supported = left_analog_trigger_supported;
ctx->right_analog_trigger_supported = right_analog_trigger_supported;
break;
default:
case 0:
ctx->usage_masks[0] = 0xFF;
ctx->usage_masks[1] = 0xFF;
ctx->usage_masks[2] = 0xFF;
ctx->usage_masks[3] = 0xFF;
ctx->left_analog_stick_supported = true;
ctx->right_analog_stick_supported = true;
ctx->left_analog_trigger_supported = true;
ctx->right_analog_trigger_supported = true;
break;
}
} else {

// Since SDL uses fixed mappings, unfortunately we cannot use the
// button mask from the protocol. SInput defines a set of predefined
// sub-types for this use. However, we can check it matches our expectations.
if (
// Masks in LSB to MSB
// South, East, West, North, DUp, DDown, DLeft, DRight
ctx->usage_masks[0] = data[12];

// Stick Left, Stick Right, L Shoulder, R Shoulder,
ctx->usage_masks[0] != buttons[0] ||
// Left Stick, Right Stick, L Shoulder, R Shoulder,
// L Trigger, R Trigger, L Paddle 1, R Paddle 1
ctx->usage_masks[1] = data[13];

ctx->usage_masks[1] != buttons[1] ||
// Start, Back, Guide, Capture, L Paddle 2, R Paddle 2, Touchpad L, Touchpad R
ctx->usage_masks[2] = data[14];

ctx->usage_masks[2] != buttons[2] ||
// Power, Misc 4 to 10
ctx->usage_masks[3] = data[15];
ctx->usage_masks[3] != buttons[3] ||
// Check Axes
ctx->left_analog_stick_supported != left_analog_stick_supported ||
ctx->right_analog_stick_supported != right_analog_stick_supported ||
ctx->left_analog_trigger_supported != left_analog_trigger_supported ||
ctx->right_analog_trigger_supported != right_analog_trigger_supported
) {
SDL_LogWarn(
SDL_LOG_CATEGORY_INPUT,
"SInput device %s has different button mask than subtype 0x%.2x or that type is uknown. Found: %.2x%.2x%.2x%.2x-%d%d%d%d and will use: %.2x%.2x%.2x%.2x-%d%d%d%d",
device->name, ctx->sub_type,
buttons[0], buttons[1], buttons[2], buttons[3],
left_analog_stick_supported, right_analog_stick_supported,
left_analog_trigger_supported, right_analog_trigger_supported,
ctx->usage_masks[0], ctx->usage_masks[1], ctx->usage_masks[2], ctx->usage_masks[3],
ctx->left_analog_stick_supported, ctx->right_analog_stick_supported,
ctx->left_analog_trigger_supported, ctx->right_analog_trigger_supported
);
}

// Derive button count from mask
Expand Down Expand Up @@ -533,16 +583,6 @@ static bool HIDAPI_DriverSInput_OpenJoystick(SDL_HIDAPI_Device *device, SDL_Joys
++axes;
}

if ((device->product_id == USB_PRODUCT_HANDHELDLEGEND_SINPUT_GENERIC) && (device->vendor_id == USB_VENDOR_RASPBERRYPI)) {
switch (ctx->sub_type) {
// Default generic device, exposes all axes
default:
case 0:
axes = 6;
break;
}
}

joystick->naxes = axes;

if (ctx->dpad_supported) {
Expand Down
Loading