Skip to content

[Spawner] Allow parsing the parameter files parsed from spawner to controllers#3136

Merged
christophfroehlich merged 6 commits intoros-controls:masterfrom
pal-robotics-forks:allow/parsing_param_files/spawner
Mar 25, 2026
Merged

[Spawner] Allow parsing the parameter files parsed from spawner to controllers#3136
christophfroehlich merged 6 commits intoros-controls:masterfrom
pal-robotics-forks:allow/parsing_param_files/spawner

Conversation

@saikishor
Copy link
Copy Markdown
Member

@codecov
Copy link
Copy Markdown

codecov bot commented Mar 23, 2026

Codecov Report

❌ Patch coverage is 85.71429% with 7 lines in your changes missing coverage. Please review.
✅ Project coverage is 89.34%. Comparing base (152ed06) to head (add57bd).
⚠️ Report is 3 commits behind head on master.

Files with missing lines Patch % Lines
controller_manager/controller_manager/spawner.py 80.00% 2 Missing and 3 partials ⚠️
controller_manager/test/test_advanced_spawner.cpp 91.66% 0 Missing and 1 partial ⚠️
controller_manager/test/test_spawner_unspawner.cpp 91.66% 0 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #3136      +/-   ##
==========================================
- Coverage   89.35%   89.34%   -0.01%     
==========================================
  Files         158      158              
  Lines       19392    19441      +49     
  Branches     1573     1583      +10     
==========================================
+ Hits        17327    17369      +42     
- Misses       1419     1421       +2     
- Partials      646      651       +5     
Flag Coverage Δ
unittests 89.34% <85.71%> (-0.01%) ⬇️

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

Files with missing lines Coverage Δ
controller_manager/test/test_advanced_spawner.cpp 94.68% <91.66%> (-0.14%) ⬇️
controller_manager/test/test_spawner_unspawner.cpp 96.00% <91.66%> (-0.13%) ⬇️
controller_manager/controller_manager/spawner.py 73.20% <80.00%> (+0.70%) ⬆️
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Copy link
Copy Markdown
Contributor

@fmauch fmauch left a comment

Choose a reason for hiding this comment

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

I have to say that I find it mildly confusing that --ros-args --params-file <filename> != --param-file <filename>.

Maybe it would be worth mentioning explicitly, that --param-file can be replaced by --ros-args --params-file or more importantly that in a launchfile

                arguments=[
                    "my_controller_name",
                    "--param-file",
                    PathSubstitution(FindPackageShare("my_config_pkg"))
                    / "config"
                    / "controllers.yaml",
                ],

can be written as

                parameters=[
                    ParameterFile(
                        PathSubstitution(FindPackageShare("my_config_pkg"))
                        / "config"
                        / "controllers.yaml",
                    ),
                ],
                arguments=[
                    "my_controller_name",
                ],

Or maybe put more simply: Controller parameters can either be specified by loading a parameter file using --param file or through the spawner's ROS node parameters when running as a ROS node.

But this PR definitively solves my issue.

@saikishor
Copy link
Copy Markdown
Member Author

I have to say that I find it mildly confusing that --ros-args --params-file <filename> != --param-file <filename>.

Maybe it would be worth mentioning explicitly, that --param-file can be replaced by --ros-args --params-file or more importantly that in a launchfile

                arguments=[
                    "my_controller_name",
                    "--param-file",
                    PathSubstitution(FindPackageShare("my_config_pkg"))
                    / "config"
                    / "controllers.yaml",
                ],

can be written as

                parameters=[
                    ParameterFile(
                        PathSubstitution(FindPackageShare("my_config_pkg"))
                        / "config"
                        / "controllers.yaml",
                    ),
                ],
                arguments=[
                    "my_controller_name",
                ],

Or maybe put more simply: Controller parameters can either be specified by loading a parameter file using --param file or through the spawner's ROS node parameters when running as a ROS node.

But this PR definitively solves my issue.

I'll try to update the docs tonight. However, I think it is better I don't say it generic that --param-file can be replaced by this, just because then when you try to load multiple controllers with multiple param files with some wildcard entries, it won't work.

Thank you for testing it 👏🏾

@saikishor saikishor added backport-jazzy Triggers PR backport to ROS 2 jazzy. backport-kilted Triggers PR backport to ROS 2 kilted. labels Mar 24, 2026
@saikishor saikishor requested a review from fmauch March 24, 2026 17:35
Copy link
Copy Markdown
Member

@christophfroehlich christophfroehlich left a comment

Choose a reason for hiding this comment

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

LGTM, thanks!

@christophfroehlich christophfroehlich merged commit a583699 into ros-controls:master Mar 25, 2026
17 of 18 checks passed
@christophfroehlich christophfroehlich deleted the allow/parsing_param_files/spawner branch March 25, 2026 20:42
mergify bot pushed a commit that referenced this pull request Mar 25, 2026
…ntrollers (#3136)

(cherry picked from commit a583699)

# Conflicts:
#	controller_manager/controller_manager/spawner.py
#	controller_manager/doc/userdoc.rst
#	controller_manager/test/test_advanced_spawner.cpp
#	doc/release_notes.rst
mergify bot pushed a commit that referenced this pull request Mar 25, 2026
…ntrollers (#3136)

(cherry picked from commit a583699)

# Conflicts:
#	controller_manager/controller_manager/spawner.py
#	controller_manager/doc/userdoc.rst
#	controller_manager/test/test_advanced_spawner.cpp
#	doc/release_notes.rst
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. backport-kilted Triggers PR backport to ROS 2 kilted.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

System setup using substitutions in controller config broken

3 participants