Skip to content

Conversation

@saikishor
Copy link
Member

Fixes: #2811

When a hardware component group is defined, from now on. It will first configure all the hardware components and then activate them all together. Instead of activating one by one, as requested by 2811.

@saikishor saikishor self-assigned this Jan 25, 2026
@saikishor saikishor added backport-jazzy Triggers PR backport to ROS 2 jazzy. backport-kilted Triggers PR backport to ROS 2 kilted. labels Jan 25, 2026
@codecov
Copy link

codecov bot commented Jan 25, 2026

Codecov Report

❌ Patch coverage is 24.44444% with 34 lines in your changes missing coverage. Please review.
✅ Project coverage is 89.28%. Comparing base (c12c3f2) to head (016dfdb).

Files with missing lines Patch % Lines
controller_manager/src/controller_manager.cpp 24.44% 32 Missing and 2 partials ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #2984      +/-   ##
==========================================
- Coverage   89.42%   89.28%   -0.15%     
==========================================
  Files         157      157              
  Lines       18747    18788      +41     
  Branches     1503     1513      +10     
==========================================
+ Hits        16765    16774       +9     
- Misses       1363     1394      +31     
- Partials      619      620       +1     
Flag Coverage Δ
unittests 89.28% <24.44%> (-0.15%) ⬇️

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.06% <24.44%> (-1.09%) ⬇️
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@firesurfer
Copy link
Contributor

Hey @saikishor great that you created this PR :)

I currently do not have hardware access to test it. Perhaps I managed to try out the PR next week.

Few remarks from my side:

  1. Would it make sense to also synchronize the init() call of each component ?
  2. You mention in Deterministic startup of hardware interfaces #2811 that the group argument is also for:

The current hardware components XML support an argument call group, basically we use to make sure that if one of the group actuator is in error, we take down the whole group.

I would suggest adding the possibility to configure flags for a group.

E.g. combined_shutdown and synchronized_startup
So it would be possible to have groups with the current behavior which are not synchronized.

@saikishor
Copy link
Member Author

Would it make sense to also synchronize the init() call of each component ?

IMO, no, it doesn't make sense

I would suggest adding the possibility to configure flags for a group.

E.g. combined_shutdown and synchronized_startup So it would be possible to have groups with the current behavior which are not synchronized.

Why? Can you explain why this type of setup is needed?

@firesurfer
Copy link
Contributor

firesurfer commented Feb 3, 2026

@saikishor

IMO, no, it doesn't make sense

Could you elaborate why? (Just for my understanding)

Why? Can you explain why this type of setup is needed?

  1. You are changing the behavior of the group implementation
  2. As a general feature: when supporting groups -> supporting flags/settings for a group is the next step

@saikishor
Copy link
Member Author

Could you elaborate why? (Just for my understanding)

Right now, the initialization happens when the loading of the robot description happens, so we have less control over there. Atleast as per my understanding

Why? Can you explain why this type of setup is needed?

1. You are changing the behavior of the group implementation

2. As a general feature: when supporting groups -> supporting flags/settings for a group is the next step

Do you know of a cases that would affect by this change?. This is only at startup, for the people who don't have a singleton approach, do you think it will make a difference?

@firesurfer
Copy link
Contributor

Just pointing out potential pitfalls. I'll try to test this PR the coming week.

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: Needs review

Development

Successfully merging this pull request may close these issues.

Deterministic startup of hardware interfaces

2 participants