Shift to Struct based Method and Constructors, with Executor passed from CM to `on_init()` #2323

soham2560 · 2025-06-09T10:06:25Z

Brief

This addition is the first step towards solving #2141 (also #1732 in some way).
The concept is simple, we are creating a Struct pathway of sorts all the way from the CM to the on_init() call in the hardware component. This enables future additions without too harsh API/ABI breaks.

An example to show this currently is to give the user access to the MultiThreadedExecutor so that they can add nodes to it and publish if necessary by propogating the executor_ from CM all the way to on_init() (refer ab8d08c). A future PR will add a node by default with the hardware_components name

Three Structs have been added to help in the above

HardwareComponentInterfaceParams
- To be passed in on_init() and parsed by user to get all relevant data (hardware_info and executor at this point)
HardwareComponentParams
- To be used for all other initialisation, loading calls up till resource manager
ResourceManagerParams
- To construct Resource Manager

Important Note

This means over time we will have to deprecate and remove the non struct based methods

Side Notes

This does mean we have to update the ros2_control_demos with the updated API
It is expected the user will lock and use the executor as needed
~~documentation is yet to be added, in particular about notifying the user to not abuse the executor (since they can use the cancel() call internally and cripple the services etc for the CM basically )~~

codecov · 2025-06-09T10:47:45Z

Codecov Report

Attention: Patch coverage is 83.08824% with 23 lines in your changes missing coverage. Please review.

Project coverage is 88.95%. Comparing base (4d3362d) to head (6d42540).

Files with missing lines	Patch %	Lines
hardware_interface/src/resource_manager.cpp	78.87%	11 Missing and 4 partials ⚠️
controller_manager/src/controller_manager.cpp	0.00%	8 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #2323      +/-   ##
==========================================
- Coverage   88.96%   88.95%   -0.02%     
==========================================
  Files         144      147       +3     
  Lines       16634    16706      +72     
  Branches     1436     1433       -3     
==========================================
+ Hits        14799    14861      +62     
- Misses       1277     1287      +10     
  Partials      558      558

Flag	Coverage Δ
unittests	`88.95% <83.08%> (-0.02%)`	⬇️

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

Files with missing lines	Coverage Δ
..._interface/include/hardware_interface/actuator.hpp	`100.00% <ø> (ø)`
.../include/hardware_interface/actuator_interface.hpp	`91.30% <100.00%> (+0.34%)`	⬆️
...re_interface/include/hardware_interface/sensor.hpp	`100.00% <ø> (ø)`
...ce/include/hardware_interface/sensor_interface.hpp	`85.98% <100.00%> (+0.98%)`	⬆️
...re_interface/include/hardware_interface/system.hpp	`100.00% <ø> (ø)`
...ce/include/hardware_interface/system_interface.hpp	`84.80% <100.00%> (+0.53%)`	⬆️
...face/types/hardware_component_interface_params.hpp	`100.00% <100.00%> (ø)`
...ware_interface/types/hardware_component_params.hpp	`100.00% <100.00%> (ø)`
...rdware_interface/types/resource_manager_params.hpp	`100.00% <100.00%> (ø)`
hardware_interface/src/actuator.cpp	`80.61% <100.00%> (+0.61%)`	⬆️
... and 4 more

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

saikishor

A partial review here

I think as this is an API breaking change, we should open the corresponding PRs in the packages like gz_ros2_control and also webots_ros2_control probably? We can discuss this part when we finish the final review of this PR

hardware_interface/include/hardware_interface/actuator_interface.hpp

hardware_interface/include/hardware_interface/sensor_interface.hpp

hardware_interface/include/hardware_interface/actuator_interface.hpp

hardware_interface/include/hardware_interface/system_interface.hpp

soham2560 · 2025-06-09T13:24:04Z

we should open the corresponding PRs in the packages like gz_ros2_control

@saikishor have added PR at ros-controls/gz_ros2_control#606

…ce.hpp Co-authored-by: Sai Kishor Kothakota <saisastra3@gmail.com>

….hpp Co-authored-by: Sai Kishor Kothakota <saisastra3@gmail.com>

…ce.hpp Co-authored-by: Sai Kishor Kothakota <saisastra3@gmail.com>

….hpp Co-authored-by: Sai Kishor Kothakota <saisastra3@gmail.com>

saikishor · 2025-06-09T14:07:40Z

we should open the corresponding PRs in the packages like gz_ros2_control

@saikishor have added PR at ros-controls/gz_ros2_control#606

Awesome. Sorry I missed that one

hardware_interface/include/hardware_interface/actuator_interface.hpp

bmagyar · 2025-06-22T08:49:03Z

Good renamings!

saikishor

Partial review

hardware_interface/include/hardware_interface/actuator_interface.hpp

saikishor · 2025-06-22T08:51:39Z

hardware_interface/include/hardware_interface/actuator_interface.hpp

  virtual CallbackReturn on_init(const HardwareInfo & hardware_info)
  {
-    info_ = hardware_info;
+    hardware_interface::HardwareComponentInterfaceParams params;
+    params.hardware_info = hardware_info;
+    return on_init(params);
+  };


For now, we should do the opposite, As this is an overridable method, for the backward compatibility, we need to call this one from the params one. Later once, it is deprecated we can move the logic here.

ooh didn't think that would be okay, but if yes then I will make a note in pr description to change that later on

refer 0137417

saikishor · 2025-06-22T08:55:33Z

hardware_interface/include/hardware_interface/sensor_interface.hpp

+    hardware_interface::HardwareComponentInterfaceParams params;
+    params.hardware_info = hardware_info;
+    return on_init(params);
+  };
+
+  /// Initialization of the hardware interface from data parsed from the robot's URDF.


refer 0137417

saikishor · 2025-06-22T08:55:44Z

hardware_interface/include/hardware_interface/system_interface.hpp

+    hardware_interface::HardwareComponentInterfaceParams params;
+    params.hardware_info = hardware_info;
+    return on_init(params);


refer 0137417

hardware_interface/include/hardware_interface/sensor_interface.hpp

hardware_interface/include/hardware_interface/system_interface.hpp

soham2560 added 2 commits June 9, 2025 09:39

propagated executor from cm to hw.on_init

6e4c3c8

updated tests to include executor

345c055

soham2560 marked this pull request as draft June 9, 2025 10:06

github-actions bot requested review from ARK3r, christophfroehlich, destogl, homalozoa, olivier-stasse, progtologist and VX792 June 9, 2025 10:06

reverted import_component changes

50b42a2

soham2560 marked this pull request as ready for review June 9, 2025 10:50

github-actions bot requested review from livanov93, mcbed, mhubii, moriarty, rosterloh and saikishor June 9, 2025 10:50

saikishor requested changes Jun 9, 2025

View reviewed changes

soham2560 mentioned this pull request Jun 9, 2025

Added support for executor as an arg to on_init() ros-controls/gz_ros2_control#606

Draft

soham2560 and others added 6 commits June 9, 2025 18:55

Update hardware_interface/include/hardware_interface/actuator_interfa…

12ba419

…ce.hpp Co-authored-by: Sai Kishor Kothakota <saisastra3@gmail.com>

Update hardware_interface/include/hardware_interface/sensor_interface…

96cb747

….hpp Co-authored-by: Sai Kishor Kothakota <saisastra3@gmail.com>

Update hardware_interface/include/hardware_interface/sensor_interface…

d098c20

….hpp Co-authored-by: Sai Kishor Kothakota <saisastra3@gmail.com>

Update hardware_interface/include/hardware_interface/actuator_interfa…

72f3253

…ce.hpp Co-authored-by: Sai Kishor Kothakota <saisastra3@gmail.com>

Update hardware_interface/include/hardware_interface/system_interface…

e0a72a4

….hpp Co-authored-by: Sai Kishor Kothakota <saisastra3@gmail.com>

Update hardware_interface/include/hardware_interface/system_interface…

3541ab9

….hpp Co-authored-by: Sai Kishor Kothakota <saisastra3@gmail.com>

soham2560 mentioned this pull request Jun 9, 2025

Added support for executor as an arg to on_init() ros-controls/ros2_control_demos#829

Draft

bmagyar reviewed Jun 17, 2025

View reviewed changes

hardware_interface/include/hardware_interface/actuator_interface.hpp Outdated Show resolved Hide resolved

soham2560 added 17 commits June 19, 2025 07:33

reverted tests

ec2987d

updated with less invasive approach

27e2927

reverted generic system changes

aceb0e1

revert controller_manager

2fdc611

reverted resource manager

8919db5

reordered diffs

9ed42ef

switched to default constructor for rm in cm

ee61346

removed executor

c72b14f

created cascaded structure

84187b6

clock assignment changes

53ea6db

added component params to initialize_hw

22d2e3e

fixed tests

ec9a3b0

added executor

ab8d08c

removed executor from construct params

7ed1c57

removed set and get params

97ca593

removed extra space

1bc8154

added new cotr to CM

292dfed

soham2560 changed the title ~~Added CMs MultiThreadedExecutor as an arg to on_init()~~ Pimpl Approach based Struct Based Methods, with Executor passed from CM to on_init() Jun 21, 2025

soham2560 changed the title ~~Pimpl Approach based Struct Based Methods, with Executor passed from CM to on_init()~~ Shift to Struct based Method and Constructors, with Executor passed from CM to on_init() Jun 21, 2025

soham2560 requested review from saikishor and bmagyar June 21, 2025 18:49

soham2560 and others added 2 commits June 21, 2025 18:50

precommit

96040d0

Rename urdf_string to robot_description

f095a04

saikishor requested changes Jun 22, 2025

View reviewed changes

saikishor reviewed Jun 22, 2025

View reviewed changes

hardware_interface/include/hardware_interface/sensor_interface.hpp Show resolved Hide resolved

saikishor reviewed Jun 22, 2025

View reviewed changes

hardware_interface/include/hardware_interface/system_interface.hpp Show resolved Hide resolved

soham2560 added 2 commits June 22, 2025 10:02

reversed overload on on_init

0137417

Merge branch 'master' into feat/MTEprop

6d42540

Shift to Struct based Method and Constructors, with Executor passed from CM to on_init() #2323

Are you sure you want to change the base?

Shift to Struct based Method and Constructors, with Executor passed from CM to on_init() #2323

Uh oh!

Conversation

soham2560 commented Jun 9, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Brief

Important Note

Side Notes

Uh oh!

codecov bot commented Jun 9, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

saikishor left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

soham2560 commented Jun 9, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

saikishor commented Jun 9, 2025

Uh oh!

Uh oh!

bmagyar commented Jun 22, 2025

Uh oh!

saikishor left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

saikishor Jun 22, 2025

Choose a reason for hiding this comment

Uh oh!

soham2560 Jun 22, 2025

Choose a reason for hiding this comment

Uh oh!

saikishor Jun 22, 2025

Choose a reason for hiding this comment

Uh oh!

soham2560 Jun 22, 2025

Choose a reason for hiding this comment

Uh oh!

saikishor Jun 22, 2025

Choose a reason for hiding this comment

Uh oh!

soham2560 Jun 22, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Shift to Struct based Method and Constructors, with Executor passed from CM to `on_init()` #2323

Shift to Struct based Method and Constructors, with Executor passed from CM to `on_init()` #2323

soham2560 commented Jun 9, 2025 •

edited

Loading

codecov bot commented Jun 9, 2025 •

edited

Loading

soham2560 commented Jun 9, 2025 •

edited

Loading