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

Fix multi-button commands #84

Merged
merged 3 commits into from
Sep 18, 2023
Merged

Conversation

Noel215
Copy link
Contributor

@Noel215 Noel215 commented May 5, 2023

Hi,

With the current implementation, the multi-button commands are executed if any of the buttons defined is pressed.
I expected the execution of the command if and only if all the buttons are pressed at the same time.

I've added a patch and a test to check the behavior in that situation.

Copy link
Member

@bmagyar bmagyar left a comment

Choose a reason for hiding this comment

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

thanks!

@bmagyar bmagyar merged commit 50c8068 into ros-teleop:master Sep 18, 2023
@sprhawk
Copy link

sprhawk commented Feb 23, 2024

@bmagyar I don't understand why approved this change

if I set buttons 1, 2, 3, only if all these buttons are pressed at the same time, self.active == True, then the topic will be sent. I don't think this is the normal case.

And the check:

self.active &= joy_state.axes[axis] == 1.0

Is the axis used as a button ? The statement means only when axis is in max (1.0), self.active == True, then the topic will be sent. (not the range value generated by axes )

for axes, we should check the last values of axes are changed, if any value of axes changed since last time, the topic should be sent.

in current implementation, the axes are actually acted as a button like request in issue #79

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants