Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Add logic to Ros2ControlManager to match ros2_control (backport #3332) #3342

Open
wants to merge 1 commit into
base: humble
Choose a base branch
from

Conversation

mergify[bot]
Copy link

@mergify mergify bot commented Feb 12, 2025

Description

The logic for the chained controllers still does not match the expectation of ROS2 control. This PR makes a few changes:

  1. Activation/deactivation is expected to be disjoint. For example, if controller B is a dependency of A (A chains to B) but controller B is also a dependency of C (B chains to B), then the switch from A->B to C->B would cause B to be in both the activation and deactivate list. This causes ROS2 control to through an error and reject the switch. The simplifyControllerActivationDeactivation function adds the logic needed to avoid this from happening.
  2. The valid transitions for chained controllers depend asymmetrically on whether the controller is being started or stopped. If A chained to B, then when A is started, B should be automatically started. B is allowed to be started and stopped on its own if A is not running. However, if both A and B are running, B cannot be shut down alone, both must be shut down together or A alone.
  3. Lastly, ROS 2 control should not be told to shut down controllers that are not running or start controllers that are already running. Doing so can cause an error. A check is added to only start controllers that are not already running.

This is an automatic backport of pull request #3332 done by [Mergify](https://mergify.com).

* Add logic to Ros2ControlManager to match ros2_control

Signed-off-by: Paul Gesel <paul.gesel@picknik.ai>

* Add Ros2ControlManager test

Signed-off-by: Paul Gesel <paul.gesel@picknik.ai>

* move simplifyControllerActivationDeactivation to function and add doxygen

Signed-off-by: Paul Gesel <paul.gesel@picknik.ai>

* move queue.pop_back up

Signed-off-by: Paul Gesel <paul.gesel@picknik.ai>

* Update moveit_plugins/moveit_ros_control_interface/src/controller_manager_plugin.cpp

Co-authored-by: Sebastian Castro <4603398+sea-bass@users.noreply.github.com>

* Update moveit_plugins/moveit_ros_control_interface/src/controller_manager_plugin.cpp

Co-authored-by: Sebastian Castro <4603398+sea-bass@users.noreply.github.com>

* Update moveit_plugins/moveit_ros_control_interface/src/controller_manager_plugin.cpp

Co-authored-by: Sebastian Castro <4603398+sea-bass@users.noreply.github.com>

* pr feedback

Signed-off-by: Paul Gesel <paul.gesel@picknik.ai>

* clang fixes

Signed-off-by: Paul Gesel <paul.gesel@picknik.ai>

---------

Signed-off-by: Paul Gesel <paul.gesel@picknik.ai>
Co-authored-by: Sebastian Castro <4603398+sea-bass@users.noreply.github.com>
Co-authored-by: Sebastian Jahr <sebastian.jahr@picknik.ai>
(cherry picked from commit dbf07b1)

# Conflicts:
#	moveit_plugins/moveit_ros_control_interface/CMakeLists.txt
#	moveit_plugins/moveit_ros_control_interface/src/controller_manager_plugin.cpp
@mergify mergify bot added the conflicts label Feb 12, 2025
Copy link
Author

mergify bot commented Feb 12, 2025

Cherry-pick of dbf07b1 has failed:

On branch mergify/bp/humble/pr-3332
Your branch is up to date with 'origin/humble'.

You are currently cherry-picking commit dbf07b1ac.
  (fix conflicts and run "git cherry-pick --continue")
  (use "git cherry-pick --skip" to skip this patch)
  (use "git cherry-pick --abort" to cancel the cherry-pick operation)

Changes to be committed:
	new file:   moveit_plugins/moveit_ros_control_interface/test/CMakeLists.txt
	new file:   moveit_plugins/moveit_ros_control_interface/test/test_controller_manager_plugin.cpp

Unmerged paths:
  (use "git add <file>..." to mark resolution)
	both modified:   moveit_plugins/moveit_ros_control_interface/CMakeLists.txt
	both modified:   moveit_plugins/moveit_ros_control_interface/src/controller_manager_plugin.cpp

To fix up this pull request, you can check it out locally. See documentation: https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/reviewing-changes-in-pull-requests/checking-out-pull-requests-locally

@sea-bass
Copy link
Contributor

@pac48 want to either resolve these or just not backport this to humble? Up to you.

# for free to join this conversation on GitHub. Already have an account? # to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants