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

joy_teleop: Do not require deadman input for axis mapping #71

Open
wants to merge 3 commits into
base: foxy-devel
Choose a base branch
from

Conversation

russkel
Copy link
Contributor

@russkel russkel commented Jul 18, 2021

Hello,

While attempting to use this teleop package for my purposes, I found I couldn't simply map an axis without specifying a "deadman switch" which in this case is more accurately a trigger. These changes improve the behaviour of the mapping.

  • Scale and Offset aren't really required parameters because defaults of 1.0 and 0 are used respectively. Removes tests and checks.
  • Check that all deadman inputs are actually satisfied.
  • Remove requirement for deadman input to map an axis. Publishes on relevant axis/button changes since last iteration.
  • Slight clean ups and refactoring.

This code should work on foxy, but has been tested on galactic only.

Russ

@russkel
Copy link
Contributor Author

russkel commented Aug 28, 2021

@bmagyar could this get a review please?

@@ -152,36 +152,6 @@ def test_teleop_axis_mappings_missing_axis_or_button(self):
self.assertEqual(joy_teleop_process.exit_code, 1)
self.assertTrue('must have an axis, button, or value' in joy_teleop_process.output)

def test_teleop_axis_mappings_missing_offset(self):
Copy link
Member

Choose a reason for hiding this comment

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

I think these tests still hold value in showing that things are handled despite some things missing. Is it ok to leave them?

Copy link
Contributor Author

@russkel russkel Sep 4, 2021

Choose a reason for hiding this comment

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

These tests are expecting an error when there isn't one after the changes in this PR.

Do you mean you also want the checks for empty offset and scale put back into the code?

joy_teleop/launch/example.launch.py Outdated Show resolved Hide resolved
@bmagyar
Copy link
Member

bmagyar commented Sep 3, 2021

Please specify what your target distro is with this

@russkel
Copy link
Contributor Author

russkel commented Sep 4, 2021

Please specify what your target distro is with this

I use galactic, personally. I don't see why this wouldn't work on foxy as well.

@russkel
Copy link
Contributor Author

russkel commented Oct 5, 2021

Hi @bmagyar - poke.

@AdronTech
Copy link

Any updates on this PR?

@russkel
Copy link
Contributor Author

russkel commented Mar 27, 2022

Seems to fail all CI tests now. I will fix this when I get a moment and hopefully it can be merged.

@blaine141
Copy link

I am still interested in this change

Copy link

@blaine141 blaine141 left a comment

Choose a reason for hiding this comment

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

Looks good to me.

@blaine141
Copy link

Appears to be resolving #74

@russkel
Copy link
Contributor Author

russkel commented Mar 12, 2023

Yeah I would have liked to have had these changes merged years ago.

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.

4 participants