-
Notifications
You must be signed in to change notification settings - Fork 493
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
Port moveit ros control interface to ROS2 #545
Conversation
201d00e
to
9d09875
Compare
Codecov Report
@@ Coverage Diff @@
## main #545 +/- ##
==========================================
- Coverage 54.30% 54.28% -0.01%
==========================================
Files 191 191
Lines 20084 20084
==========================================
- Hits 10904 10900 -4
- Misses 9180 9184 +4
Continue to review full report at Codecov.
|
moveit_ros/planning/trajectory_execution_manager/src/trajectory_execution_manager.cpp
Show resolved
Hide resolved
moveit_plugins/moveit_ros_control_interface/moveit_ros_control_interface_plugins.xml
Show resolved
Hide resolved
moveit_ros/planning/trajectory_execution_manager/src/trajectory_execution_manager.cpp
Show resolved
Hide resolved
moveit_plugins/moveit_ros_control_interface/src/controller_manager_plugin.cpp
Outdated
Show resolved
Hide resolved
moveit_plugins/moveit_ros_control_interface/src/controller_manager_plugin.cpp
Show resolved
Hide resolved
moveit_plugins/moveit_ros_control_interface/src/controller_manager_plugin.cpp
Show resolved
Hide resolved
moveit_plugins/moveit_ros_control_interface/src/controller_manager_plugin.cpp
Show resolved
Hide resolved
9d09875
to
ef622bc
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is there a high-level overview of the difference between these packages somewhere? Frankly, they all sound the same:
- moveit_ros_control_interface: "ros_control controller manager interface for MoveIt"
- moveit_controller_manager: I see that this is COLCON-ignored, probably will be deleted.
- moveit_simple_controller_manager: "A generic, simple controller manager plugin for MoveIt."
It seems like moveit_ros_control_interface
and moveit_simple_controller_manager
should be combined into one package.
I see that moveit_ros_control_interface
depends on moveit_simple_controller_manager
I won't block the PR over this comment, but it should be considered in the future.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I also think a bit of specific documentation about what these packages do would be great. Do they switch controllers when a different move group is used, or ...?
A tutorial would be great.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
testing this right now...
moveit_plugins/moveit_ros_control_interface/src/controller_manager_plugin.cpp
Outdated
Show resolved
Hide resolved
moveit_plugins/moveit_ros_control_interface/src/controller_manager_plugin.cpp
Show resolved
Hide resolved
moveit_plugins/moveit_ros_control_interface/src/gripper_controller_plugin.cpp
Show resolved
Hide resolved
fdbf828
to
319c8c7
Compare
Description
Currently setting
moveit_manage_controllers
toTrue
will not work since controller_manager doesn't report joints names for the inactive controllersChecklist