Activate hardware components by group sequentially

[saikishor]

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.

[codecov]

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]

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]

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]

@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]

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]

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

[firesurfer]

@saikishor sorry for not answering sooner. We decided to actually implement another approach where we do not create multiple hardware interfaces but only one hardware interface which will be differently configured for different hardware combinations.

Even though this goes against the ros2control approach it will be more future proof for our use case.

Nevertheless I think this is an important ros2control feature !