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

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

Merged
merged 1 commit into from
Feb 12, 2025

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)
@codecov-commenter
Copy link

codecov-commenter commented Feb 12, 2025

⚠️ Please install the 'codecov app svg image' to ensure uploads and comments are reliably processed by Codecov.

Codecov Report

Attention: Patch coverage is 98.01980% with 2 lines in your changes missing coverage. Please review.

Project coverage is 44.41%. Comparing base (611a5a2) to head (dea0514).
Report is 1 commits behind head on jazzy.

Files with missing lines Patch % Lines
...ontrol_interface/src/controller_manager_plugin.cpp 94.60% 2 Missing ⚠️

❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files
@@            Coverage Diff             @@
##            jazzy    #3343      +/-   ##
==========================================
- Coverage   44.52%   44.41%   -0.10%     
==========================================
  Files         700      701       +1     
  Lines       61760    61859      +99     
  Branches     7491     7494       +3     
==========================================
- Hits        27493    27471      -22     
- Misses      34100    34219     +119     
- Partials      167      169       +2     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@sea-bass sea-bass merged commit 7e1fc53 into jazzy Feb 12, 2025
7 checks passed
@sea-bass sea-bass deleted the mergify/bp/jazzy/pr-3332 branch February 12, 2025 22:54
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