ros / ros_tutorials

Code used in tutorials found on ROS wiki
http://wiki.ros.org/ros_tutorials
805 stars 540 forks source link

Add holonomic motion disable option (ROS 2 turtlesim) #131

Closed Tiryoh closed 2 years ago

Tiryoh commented 3 years ago

This PR adds an option to turtlesim to disable holonomic motion.

Related issue: https://github.com/ros/ros_tutorials/issues/127

Usage

ros2 run turtlesim turtlesim_node --ros-args -p holonomic:=false

Screenshot from 2021-07-08 00-32-42

Note that the right terminal shows the teleop_twist_keyboard, which is not contained in this PR.

Tiryoh commented 2 years ago

@sloretz Sorry to bother you but is there any chance of merging this PR and ROS 1's PR (#130)?

sloretz commented 2 years ago

@sloretz Sorry to bother you but is there any chance of merging this PR and ROS 1's PR (#130)?

I don't have a moment to look at this right now, but at first glance it seems like an API change / ABI break. Those can be merged into ROS Rolling (ROS 2), but not ROS 1 as there's no future ROS 1 distro to target.

jacobperron commented 2 years ago

@Tiryoh I've rebased your branch and pushed an additional commit that simplifies the logic (maintaining ABI compatibility). It also changes the default behavior such that the holonomic feature is off by default, maintaining the current behavior.

I'm tagging current maintainers to review since I've made significant changes.

Tiryoh commented 2 years ago

@jacobperron Thank you for the update.

audrow commented 2 years ago

Here's CI:

audrow commented 2 years ago

Thanks for the PR @Tiryoh, and thanks for the changes and for merging @jacobperron!