Skip to content

Conversation

HTRamsey
Copy link
Collaborator

@HTRamsey HTRamsey commented Oct 3, 2025

Should return false if not pressed
Think this actually conflicts with the getHat stuff since the indices are (maybe?) different for the dpad of gamepad & hat of joystick?

@HTRamsey HTRamsey requested a review from Copilot October 4, 2025 05:37
Copy link
Contributor

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR fixes the gamepad button logic in JoystickSDL to properly return false when a button is not pressed. Previously, the code would only return true for pressed buttons but would fall through to check additional button sources even when the gamepad button query succeeded.


Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

if (SDL_GetGamepadButton(_sdlGamepad, static_cast<SDL_GamepadButton>(idx))) {
return true;
}
return SDL_GetGamepadButton(_sdlGamepad, static_cast<SDL_GamepadButton>(idx));
Copy link

Copilot AI Oct 4, 2025

Choose a reason for hiding this comment

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

SDL_GetGamepadButton returns a Uint8 (0 or 1), but this function returns bool. While this works due to implicit conversion, it's clearer to explicitly convert: return SDL_GetGamepadButton(_sdlGamepad, static_cast<SDL_GamepadButton>(idx)) != 0;

Suggested change
return SDL_GetGamepadButton(_sdlGamepad, static_cast<SDL_GamepadButton>(idx));
return SDL_GetGamepadButton(_sdlGamepad, static_cast<SDL_GamepadButton>(idx)) != 0;

Copilot uses AI. Check for mistakes.

@HTRamsey HTRamsey merged commit 7dd0197 into mavlink:master Oct 4, 2025
13 checks passed
@HTRamsey HTRamsey deleted the dev-joystick-buttoncount branch October 4, 2025 05:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant