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

Fix Ros2ControlManager chained controller logic #3301

Merged
merged 2 commits into from
Feb 4, 2025

Conversation

pac48
Copy link
Contributor

@pac48 pac48 commented Feb 3, 2025

Description

It looks like this PR might have broken the chained controller management logic.
Previously, claimed_interfaces were being used to determine which joints each controller was associated with, but now required_command_interfaces is used. That seems reasonable, but it was not updated everywhere. Specifically, there is some logic to handle ros2_control's chained controllers that was not updated. This PR fixes that issue.

@pac48 pac48 requested a review from sjahr February 3, 2025 20:21
@codecov-commenter
Copy link

codecov-commenter commented Feb 3, 2025

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

Codecov Report

Attention: Patch coverage is 0% with 6 lines in your changes missing coverage. Please review.

Project coverage is 45.57%. Comparing base (bb7463a) to head (4dc0530).
Report is 1 commits behind head on main.

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

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

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #3301      +/-   ##
==========================================
- Coverage   45.59%   45.57%   -0.02%     
==========================================
  Files         716      716              
  Lines       62385    62387       +2     
  Branches     7546     7544       -2     
==========================================
- Hits        28441    28429      -12     
- Misses      33778    33791      +13     
- Partials      166      167       +1     

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

@sea-bass sea-bass added backport-humble Mergify label that triggers a PR backport to Humble backport-jazzy Mergify label that triggers a PR backport to Jazzy labels Feb 3, 2025
Copy link
Contributor

@sjahr sjahr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

@sjahr sjahr added this pull request to the merge queue Feb 4, 2025
Merged via the queue into moveit:main with commit cf1d3e0 Feb 4, 2025
9 checks passed
mergify bot pushed a commit that referenced this pull request Feb 4, 2025
Signed-off-by: Paul Gesel <[email protected]>
(cherry picked from commit cf1d3e0)
mergify bot pushed a commit that referenced this pull request Feb 4, 2025
Signed-off-by: Paul Gesel <[email protected]>
(cherry picked from commit cf1d3e0)
sea-bass pushed a commit that referenced this pull request Feb 4, 2025
Signed-off-by: Paul Gesel <[email protected]>
(cherry picked from commit cf1d3e0)

Co-authored-by: Paul Gesel <[email protected]>
sea-bass pushed a commit that referenced this pull request Feb 4, 2025
Signed-off-by: Paul Gesel <[email protected]>
(cherry picked from commit cf1d3e0)

Co-authored-by: Paul Gesel <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport-humble Mergify label that triggers a PR backport to Humble backport-jazzy Mergify label that triggers a PR backport to Jazzy
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants