Skip to content

Add parameter to allow controllers with inactive hardware components#2501

Merged
bmagyar merged 22 commits into
ros-controls:masterfrom
pal-robotics-forks:parameterize/controller_switching/inactive/hardware
Sep 27, 2025
Merged

Add parameter to allow controllers with inactive hardware components#2501
bmagyar merged 22 commits into
ros-controls:masterfrom
pal-robotics-forks:parameterize/controller_switching/inactive/hardware

Conversation

@saikishor

@saikishor saikishor commented Aug 27, 2025

Copy link
Copy Markdown
Member

Gives #2484 a possibility to fix with a parameter

In the 27th August, 2025 PMC meeting, it has been decided to maintain the current behavior, but have an way to the old behavior using a controller_manager parameter. As the main fix is regarding the safety concerns, multiple attendees agreed to keep the behavior, but add a parameter to opt-out of it

@saikishor saikishor added the backport-jazzy Triggers PR backport to ROS 2 jazzy. label Aug 27, 2025
@codecov

codecov Bot commented Aug 27, 2025

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 98.38710% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 89.42%. Comparing base (ff52562) to head (2e83f67).
⚠️ Report is 1 commits behind head on master.

Files with missing lines Patch % Lines
hardware_interface/src/resource_manager.cpp 93.75% 0 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #2501      +/-   ##
==========================================
+ Coverage   89.32%   89.42%   +0.10%     
==========================================
  Files         147      147              
  Lines       16589    16637      +48     
  Branches     1396     1400       +4     
==========================================
+ Hits        14818    14878      +60     
+ Misses       1234     1220      -14     
- Partials      537      539       +2     
Flag Coverage Δ
unittests 89.42% <98.38%> (+0.10%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines Coverage Δ
controller_manager/src/controller_manager.cpp 76.19% <100.00%> (+0.77%) ⬆️
...rdware_interface/types/resource_manager_params.hpp 100.00% <ø> (ø)
...t/test_resource_manager_prepare_perform_switch.cpp 100.00% <100.00%> (ø)
hardware_interface/src/resource_manager.cpp 78.71% <93.75%> (-0.07%) ⬇️
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@saikishor saikishor marked this pull request as ready for review August 28, 2025 18:35
@saikishor saikishor requested review from christophfroehlich and removed request for christophfroehlich August 28, 2025 18:49

@MarqRazz MarqRazz left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Thanks for the quick turn around @saikishor! I will try to test it out on my hardware when I get a chance.

Do we want to add any deprecation warnings to this or are we going to support this feature going forward?

@saikishor

saikishor commented Aug 28, 2025

Copy link
Copy Markdown
Member Author

Thanks for the quick turn around @saikishor! I will try to test it out on my hardware when I get a chance.

Do we want to add any deprecation warnings to this or are we going to support this feature going forward?

@MarqRazz yes, there will be a warning printed that it is not the recommended way to do it.

Regarding the deprecation warning, I think we should do it once we have the configuration interfaces ready as it is the point when we will start to drop support to this. As we don't want to remove everything in a single release version. What do you think?

Comment thread controller_manager/src/controller_manager_parameters.yaml Outdated
Comment thread controller_manager/src/controller_manager_parameters.yaml Outdated
Comment thread controller_manager/src/controller_manager_parameters.yaml Outdated
Co-authored-by: Christoph Fröhlich <christophfroehlich@users.noreply.github.com>
Comment thread controller_manager/src/controller_manager_parameters.yaml Outdated
Comment thread controller_manager/src/controller_manager.cpp
Comment thread hardware_interface/include/hardware_interface/types/resource_manager_params.hpp Outdated
saikishor and others added 2 commits September 2, 2025 16:52
Co-authored-by: Christoph Fröhlich <christophfroehlich@users.noreply.github.com>

@MarqRazz MarqRazz left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Two small suggestions.

Comment thread controller_manager/src/controller_manager_parameters.yaml Outdated
Comment thread controller_manager/src/controller_manager_parameters.yaml Outdated
@saikishor

Copy link
Copy Markdown
Member Author

@christophfroehlich how does it look now?

@christophfroehlich christophfroehlich left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM

Comment thread controller_manager/src/controller_manager_parameters.yaml Outdated
Comment thread controller_manager/src/controller_manager_parameters.yaml Outdated
Comment thread controller_manager/src/controller_manager_parameters.yaml Outdated
Comment thread controller_manager/src/controller_manager_parameters.yaml Outdated
Comment thread doc/release_notes.rst
Co-authored-by: Bence Magyar <bence.magyar.robotics@gmail.com>
Comment thread hardware_interface/src/resource_manager.cpp Outdated
bmagyar
bmagyar previously approved these changes Sep 25, 2025

@bmagyar bmagyar left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

just a few docs touchups

Co-authored-by: Bence Magyar <bence.magyar.robotics@gmail.com>
Comment thread hardware_interface_testing/test/test_resource_manager_prepare_perform_switch.cpp Outdated
@bmagyar bmagyar merged commit bb87986 into ros-controls:master Sep 27, 2025
23 of 31 checks passed
@bmagyar bmagyar deleted the parameterize/controller_switching/inactive/hardware branch September 27, 2025 20:33
@github-project-automation github-project-automation Bot moved this from Kilted to Done in Roadmap / Features Sep 27, 2025
mergify Bot pushed a commit that referenced this pull request Sep 27, 2025
saikishor added a commit that referenced this pull request Oct 3, 2025
…2501) (#2584)

(cherry picked from commit bb87986)

Co-authored-by: Sai Kishor Kothakota <sai.kishor@pal-robotics.com>
Co-authored-by: Christoph Fröhlich <christophfroehlich@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

backport-jazzy Triggers PR backport to ROS 2 jazzy. check-prerelease-downstream Runs the pre-release workflow with 1st level downstream dependencies

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

Enable communication to an inactive hardware interface

5 participants