-
Notifications
You must be signed in to change notification settings - Fork 528
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
Support chained controllers #1482
Conversation
…d_octomap_updater.cpp Co-authored-by: AndyZe <andyz@utexas.edu>
…m/JafarAbdi/moveit2 into pr-support-chained-controllers
…ac48/moveit2 into pr-support-chained-controllers
Codecov Report
@@ Coverage Diff @@
## main #1482 +/- ##
==========================================
+ Coverage 51.10% 51.11% +0.01%
==========================================
Files 380 380
Lines 31802 31802
==========================================
+ Hits 16249 16252 +3
+ Misses 15553 15550 -3
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. |
Why? It does not sound like it would be too difficult to extend it? |
The response from the ros2_control list controllers service does indicate which controllers are chained to which. The logic in this PR is really about taking that information and converting it to a format that MoveIt understands. Regarding chaining multiple controllers, MoveIt's controller manager plugin assumes there exists a direct mapping from controllers to controllable interfaces. If the controllers are chained with only one connection, then it is straight forward to propagate the controllable interfaces to the beginning of the chain, which ultimately controls those interfaces. If branching occurs in the chain, then propagating the interfaces is ambiguous. Lastly, I believe the ros2_control maintainers are working towards a simplified service interface to handle chained controllers. When that happens, handling the general case will be more straightforward. |
4ef460a
to
7ed2e6a
Compare
7ed2e6a
to
ce1bc0c
Compare
…ager_plugin.cpp Co-authored-by: AndyZe <andyz@utexas.edu>
…ager_plugin.cpp Co-authored-by: AndyZe <andyz@utexas.edu>
…ager_plugin.cpp Co-authored-by: AndyZe <andyz@utexas.edu>
…ager_plugin.cpp Co-authored-by: AndyZe <andyz@utexas.edu>
…ac48/moveit2 into pr-support-chained-controllers
3b6a2cd
to
fa594a2
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.
One tiny nit. Otherwise, this looks good.
moveit_plugins/moveit_ros_control_interface/src/controller_manager_plugin.cpp
Outdated
Show resolved
Hide resolved
…ager_plugin.cpp Co-authored-by: Tyler Weaver <squirrel428@protonmail.com>
moveit_plugins/moveit_ros_control_interface/src/controller_manager_plugin.cpp
Outdated
Show resolved
Hide resolved
moveit_plugins/moveit_ros_control_interface/src/empty_controller_plugin.cpp
Outdated
Show resolved
Hide resolved
...simple_controller_manager/include/moveit_simple_controller_manager/empty_controller_handle.h
Outdated
Show resolved
Hide resolved
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've tested it now, and also tested that it doesn't break the regular Joint Trajectory Controller. Once you rebase, we should be clear to merge it. :)
* fix controller list if chained controllers exist * add comments and clean code * added additional comments * fix formatting * fix white space * add const reference and chhnage variable name * simplify logic to only work with one layer chain * Don't return false when not finding optional parameter * Update moveit_ros/perception/pointcloud_octomap_updater/src/pointcloud_octomap_updater.cpp Co-authored-by: AndyZe <andyz@utexas.edu> * add debug information * print controller names * print controllers with not known type * load controller dependencies * start chained controllers in switch * reverse order of activate controllers * prevent stoppping controller twice * revert all debug changes * add ROS error if a controller chains to more than one * use loop to index chained connections * update ros_control * add empty controller allocator for admittance controller * fix plugin xml * Update moveit_plugins/moveit_ros_control_interface/src/controller_manager_plugin.cpp Co-authored-by: AndyZe <andyz@utexas.edu> * Update moveit_plugins/moveit_ros_control_interface/src/controller_manager_plugin.cpp Co-authored-by: AndyZe <andyz@utexas.edu> * Update moveit_plugins/moveit_ros_control_interface/src/controller_manager_plugin.cpp Co-authored-by: AndyZe <andyz@utexas.edu> * Update moveit_plugins/moveit_ros_control_interface/src/controller_manager_plugin.cpp Co-authored-by: AndyZe <andyz@utexas.edu> * fix map indexing * add comment * Update moveit_plugins/moveit_ros_control_interface/src/controller_manager_plugin.cpp Co-authored-by: Tyler Weaver <squirrel428@protonmail.com> * Typos Co-authored-by: JafarAbdi <cafer.abdi@gmail.com> Co-authored-by: Jafar <jafar.uruc@gmail.com> Co-authored-by: AndyZe <andyz@utexas.edu> Co-authored-by: Vatan Aksoy Tezer <vatan@picknik.ai> Co-authored-by: Tyler Weaver <squirrel428@protonmail.com> Co-authored-by: AndyZe <zelenak@picknik.ai>
* fix controller list if chained controllers exist * add comments and clean code * added additional comments * fix formatting * fix white space * add const reference and chhnage variable name * simplify logic to only work with one layer chain * Don't return false when not finding optional parameter * Update moveit_ros/perception/pointcloud_octomap_updater/src/pointcloud_octomap_updater.cpp Co-authored-by: AndyZe <andyz@utexas.edu> * add debug information * print controller names * print controllers with not known type * load controller dependencies * start chained controllers in switch * reverse order of activate controllers * prevent stoppping controller twice * revert all debug changes * add ROS error if a controller chains to more than one * use loop to index chained connections * update ros_control * add empty controller allocator for admittance controller * fix plugin xml * Update moveit_plugins/moveit_ros_control_interface/src/controller_manager_plugin.cpp Co-authored-by: AndyZe <andyz@utexas.edu> * Update moveit_plugins/moveit_ros_control_interface/src/controller_manager_plugin.cpp Co-authored-by: AndyZe <andyz@utexas.edu> * Update moveit_plugins/moveit_ros_control_interface/src/controller_manager_plugin.cpp Co-authored-by: AndyZe <andyz@utexas.edu> * Update moveit_plugins/moveit_ros_control_interface/src/controller_manager_plugin.cpp Co-authored-by: AndyZe <andyz@utexas.edu> * fix map indexing * add comment * Update moveit_plugins/moveit_ros_control_interface/src/controller_manager_plugin.cpp Co-authored-by: Tyler Weaver <squirrel428@protonmail.com> * Typos Co-authored-by: JafarAbdi <cafer.abdi@gmail.com> Co-authored-by: Jafar <jafar.uruc@gmail.com> Co-authored-by: AndyZe <andyz@utexas.edu> Co-authored-by: Vatan Aksoy Tezer <vatan@picknik.ai> Co-authored-by: Tyler Weaver <squirrel428@protonmail.com> Co-authored-by: AndyZe <zelenak@picknik.ai> (cherry picked from commit 3db960a)
This PR adds support for ros2_control's chained controllers. Only simple chains are supported currently. That is, controller can only chain to at most one other controller. I also added logic to start and stop the chained controller in the correct order.
I created a demo repository under ros2_control with that uses the chnages made in this PR. Here is a link to the README