Skip to content

[Spawner] Allow arguments per controller instead of global args#2895

Merged
bmagyar merged 11 commits intoros-controls:masterfrom
pal-robotics-forks:advanced_spawner
Feb 26, 2026
Merged

[Spawner] Allow arguments per controller instead of global args#2895
bmagyar merged 11 commits intoros-controls:masterfrom
pal-robotics-forks:advanced_spawner

Conversation

@saikishor
Copy link
Copy Markdown
Member

@saikishor saikishor commented Dec 6, 2025

This PR adds some changes to the Spawner to allow it to define the controller args per controller rather than global args for all the controllers.

I've added a new function in the launch_utils and I tested it using the following changes in example_12
https://github.com/ros-controls/ros2_control_demos/blob/advanced_spawner/example_12/bringup/launch/rrbot.launch.py

With this change we can do both the following

ros2 run controller_manager spawner ctrl_1 ctrl_2 --inactive --controller-ros-args ....

and

ros2 run controller_manager spawner --controller ctrl_1 --load-only --controller ctrl_2 --inactive --controller-ros-args ...

@saikishor saikishor changed the title Advanced spawner allowing arguments per controller [Spawner] Allow arguments per controller instead of global args Dec 6, 2025
@saikishor saikishor added backport-jazzy Triggers PR backport to ROS 2 jazzy. backport-kilted Triggers PR backport to ROS 2 kilted. labels Dec 6, 2025
@codecov
Copy link
Copy Markdown

codecov bot commented Dec 6, 2025

Codecov Report

❌ Patch coverage is 81.97115% with 75 lines in your changes missing coverage. Please review.
✅ Project coverage is 89.27%. Comparing base (44cbd9c) to head (227e4a4).
⚠️ Report is 1 commits behind head on master.

Files with missing lines Patch % Lines
...troller_manager/controller_manager/launch_utils.py 2.56% 38 Missing ⚠️
controller_manager/controller_manager/spawner.py 78.50% 15 Missing and 8 partials ⚠️
controller_manager/test/test_advanced_spawner.cpp 94.81% 0 Missing and 14 partials ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #2895      +/-   ##
==========================================
- Coverage   89.40%   89.27%   -0.14%     
==========================================
  Files         157      158       +1     
  Lines       18781    19155     +374     
  Branches     1510     1549      +39     
==========================================
+ Hits        16791    17100     +309     
- Misses       1369     1417      +48     
- Partials      621      638      +17     
Flag Coverage Δ
unittests 89.27% <81.97%> (-0.14%) ⬇️

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.81% <94.81%> (ø)
controller_manager/controller_manager/spawner.py 72.64% <78.50%> (+2.23%) ⬆️
...troller_manager/controller_manager/launch_utils.py 22.97% <2.56%> (-22.75%) ⬇️

... and 1 file with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@christophfroehlich christophfroehlich moved this to Needs review in Review triage Dec 17, 2025
@christophfroehlich christophfroehlich moved this from Needs review to Needs discussion in Review triage Dec 17, 2025
@mergify
Copy link
Copy Markdown
Contributor

mergify bot commented Dec 20, 2025

This pull request is in conflict. Could you fix it @saikishor?

@christophfroehlich christophfroehlich moved this from Needs discussion to Needs review in Review triage Jan 14, 2026
bmagyar
bmagyar previously approved these changes Jan 29, 2026
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.

I think this is a great idea.

I tested this successfully with ros-controls/ros2_control_demos#1030 and have some notes with possible improvements:

  • How can we add a hint to the --controller argument from ros2 run controller_manager spawner --help?
  • does it make sense/would it be possible to give more than one controller with one --controller option "block", see the two pid controllers in the linked example which have identical settings.

@github-project-automation github-project-automation bot moved this from Needs review to WIP in Review triage Jan 31, 2026
@bmagyar
Copy link
Copy Markdown
Member

bmagyar commented Feb 25, 2026

@saikishor @christophfroehlich I've implemented the --controller hint using a dummy arg. The other version was to use an epilogue thing from argparse which would look weird in the user output. Tests keep things working even though the current thing is a liiiiiiiiiiittle hacky but it works nicely. I think we should merge the PR as-is

@bmagyar bmagyar moved this from WIP to Needs review in Review triage Feb 25, 2026
@saikishor
Copy link
Copy Markdown
Member Author

@saikishor @christophfroehlich I've implemented the --controller hint using a dummy arg. The other version was to use an epilogue thing from argparse which would look weird in the user output. Tests keep things working even though the current thing is a liiiiiiiiiiittle hacky but it works nicely. I think we should merge the PR as-is

LGTM. Thanks

@bmagyar bmagyar merged commit cb9a983 into ros-controls:master Feb 26, 2026
16 of 17 checks passed
@bmagyar bmagyar deleted the advanced_spawner branch February 26, 2026 11:49
@github-project-automation github-project-automation bot moved this from Needs review to Done in Review triage Feb 26, 2026
mergify bot pushed a commit that referenced this pull request Feb 26, 2026
mergify bot pushed a commit that referenced this pull request Feb 26, 2026
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

Status: Done

Development

Successfully merging this pull request may close these issues.

3 participants