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

Improve joy telep support #131

Merged
merged 21 commits into from
Jan 14, 2025
Merged

Improve joy telep support #131

merged 21 commits into from
Jan 14, 2025

Conversation

civerachb-cpr
Copy link
Contributor

@civerachb-cpr civerachb-cpr commented Dec 16, 2024

Overview

  1. Block joy teleop inputs if signal quality is poor
  2. Add configurations for other common controllers

Block inputs when signal quality is poor

Adds a new clearpath_bt_joy package which contains a node that monitors the link quality of the connection to the game controller and uses that to publish a quality_ok topic that can be used a lock to a twist_mux node.

The joy controller topics & nodes are slightly reworked, with some new hidden topics. Everything remains API compatible with the existing layout.

Previously:

  • joy_linux publishes joy_teleop/joy
  • teleop_twist_joy subscribes to joy_teleop/joy, publishes joy_teleop/cmd_vel

Now:

  • joy_linux still publishes joy_teleop/joy
  • teleop_twist_joy publishes joy_teleop/_cmd_vel (hidden topic)
  • clearpath_bt_joy publishes joy_teleop/quality and joy_teleop/bt_quality_stop as new topics
  • a new twist_mux subscribes to joy_teleop/_cmd_vel as the input and joy_teleop/bt_quality_stop as a lock, publishing joy_teleop/cmd_vel as the output

By relying on the bluez package's hcitool lq MAC_ADDR command, we avoid adding any additional systemd jobs or modifying the OS's joy controller drivers; we just passively monitor the link quality and use the twist_mux's lock input to disable the controller input when needed.

Context

Linux game controllers latch their last-received command for a brief period when the connection to the controller is lost. When dealing with a physical robot this can cause a dangerous situation where the robot is literally untrontrollable for a brief window of time.

Previously we used the DS4DRV package to mitigate this, but that package was specific to the PS4/DualShock controller, which we may be moving away from.

Add support for additional common controllers

Adds udev rules & configuration files for PS4, Xbox, Logitech game controllers. PS5 pending, though may be compatible with PS4 (untested so far)

Logitech controller udev rules removed from clearpath_robot and moved here. See clearpathrobotics/clearpath_robot#113

Additional controllers added to clearpath_config here: clearpathrobotics/clearpath_config#105

@civerachb-cpr civerachb-cpr requested a review from a team as a code owner December 16, 2024 17:34
@civerachb-cpr civerachb-cpr requested review from roni-kreinin and hilary-luo and removed request for a team December 16, 2024 17:34
@civerachb-cpr civerachb-cpr changed the title Block joy teleop inputs if signal quality is poor Improve joy telep support Dec 17, 2024
@civerachb-cpr civerachb-cpr marked this pull request as draft December 17, 2024 17:49
@civerachb-cpr civerachb-cpr marked this pull request as ready for review December 19, 2024 21:09
Copy link
Contributor

@roni-kreinin roni-kreinin left a comment

Choose a reason for hiding this comment

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

Other than the hidden topics, LGTM

clearpath_control/launch/teleop_joy.launch.py Outdated Show resolved Hide resolved
clearpath_control/launch/teleop_joy.launch.py Outdated Show resolved Hide resolved
Copy link
Contributor

@hilary-luo hilary-luo left a comment

Choose a reason for hiding this comment

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

A couple more high level comments:

  • Consider adding some additional info logging lines to show if it is still waiting on something else (ex. a service to be available like line 60 in clearpath_bt_joy_cutoff_node.py) vs it is ready and operational
  • Consider whether the quality or timeout should be configurable from the robot.yaml

{'topics.joy.timeout': 0.5},
{'topics.joy.priority': 10},
{'locks.bt_quality.topic': 'joy_teleop/bt_quality_stop'},
{'locks.bt_quality.timeout': 0.0},
Copy link
Contributor

Choose a reason for hiding this comment

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

This has the quality timeout disabled, it would be "safer" to timeout on a stale quality topic. Let's discuss

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I've added a 1s timeout after discussing on Slack.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Made a few further changes to the code for non-bluetooth controllers to work around the timeout. Since if the controller doesn't have a MAC it's most likely using a dongle or it's wired, we just publish full quality. If the device is then unplugged it will stop publishing anyway, so the quality cutoff is irrelevant.

@civerachb-cpr
Copy link
Contributor Author

Updated the MR description to reflect changes to topic names, just in case we ever refer back to this down the road.

…added a timeout to the lock, publish fake quality data when using wired controllers. Log when this happens
Copy link
Contributor

@hilary-luo hilary-luo 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

@civerachb-cpr civerachb-cpr merged commit a1c270b into jazzy-2.0RC Jan 14, 2025
3 of 9 checks passed
@civerachb-cpr civerachb-cpr deleted the better-joy-teleop branch January 14, 2025 16:47
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