Addition of a Default Node for Hardware Component #2348

soham2560 · 2025-06-30T04:53:34Z

Brief

Now that we have access to the CM's executor from #2323, the user can add their nodes to it and create publishers and subscribers from them. The next step, which this PR addresses, is adding a node by default, with the hardware components name, and giving the user access to it through a get_node method.

codecov · 2025-06-30T05:09:23Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 88.88%. Comparing base (c588879) to head (e1724d4).
Report is 1 commits behind head on master.

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #2348      +/-   ##
==========================================
+ Coverage   88.83%   88.88%   +0.05%     
==========================================
  Files         148      148              
  Lines       16896    16938      +42     
  Branches     1440     1443       +3     
==========================================
+ Hits        15009    15056      +47     
+ Misses       1319     1317       -2     
+ Partials      568      565       -3

Flag	Coverage Δ
unittests	`88.88% <100.00%> (+0.05%)`	⬆️

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

Files with missing lines	Coverage Δ
.../include/hardware_interface/actuator_interface.hpp	`91.62% <100.00%> (+0.31%)`	⬆️
...ce/include/hardware_interface/sensor_interface.hpp	`86.84% <100.00%> (+0.86%)`	⬆️
...ce/include/hardware_interface/system_interface.hpp	`85.30% <100.00%> (+0.50%)`	⬆️
...e_interface_testing/test/test_resource_manager.cpp	`99.46% <100.00%> (+0.01%)`	⬆️
...e_interface_testing/test/test_resource_manager.hpp	`95.00% <100.00%> (+0.55%)`	⬆️

... and 2 files with indirect coverage changes

🚀 New features to boost your workflow:

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

hardware_interface/include/hardware_interface/system_interface.hpp

hardware_interface/include/hardware_interface/actuator_interface.hpp

hardware_interface/include/hardware_interface/sensor_interface.hpp

hardware_interface/include/hardware_interface/system_interface.hpp

hardware_interface/include/hardware_interface/actuator_interface.hpp

hardware_interface/include/hardware_interface/sensor_interface.hpp

hardware_interface/include/hardware_interface/system_interface.hpp

…ce.hpp Co-authored-by: Sai Kishor Kothakota <[email protected]>

….hpp Co-authored-by: Sai Kishor Kothakota <[email protected]>

…ce.hpp Co-authored-by: Sai Kishor Kothakota <[email protected]>

….hpp Co-authored-by: Sai Kishor Kothakota <[email protected]>

saikishor · 2025-07-12T10:59:11Z

hardware_interface/include/hardware_interface/actuator_interface.hpp

+    if (auto locked_executor = params.executor.lock())
+    {
+      std::string node_name = params.hardware_info.name;
+      std::replace(node_name.begin(), node_name.end(), '/', '_');


Should we later convert it to lower case just in case?. What's your opinion here?.

I'm okay to merge it as it is and then we can deal with it later.

refer
7750bd7

saikishor

In general looks great :)

hardware_interface_testing/test/test_resource_manager.cpp

christophfroehlich

LGTM

Co-authored-by: Sai Kishor Kothakota <[email protected]>

hardware_interface/doc/writing_new_hardware_component.rst

saikishor · 2025-07-12T13:42:36Z

hardware_interface/doc/writing_new_hardware_component.rst

@@ -48,6 +48,54 @@ The following is a step-by-step guide to create source files, basic tests, and c
      In the first line usually the parents ``on_init`` is called to process standard values, like name. This is done using: ``hardware_interface::(Actuator|Sensor|System)Interface::on_init(info)``.
      If all required parameters are set and valid and everything works fine return ``CallbackReturn::SUCCESS`` or ``return CallbackReturn::ERROR`` otherwise.

+      #. **(Optional) Adding a Diagnostic Publisher**


@soham2560 Can you modify this section here? it would makes sense to be regarding the addition of the node, and how it can be accessed, and the user can use it to add services, publishers etc. May be the below example code, can be an example on what the user can do. It's better to focus on generic things rather than a specific one in the documentation.

Co-authored-by: Sai Kishor Kothakota <[email protected]>

saikishor

The method2 looks good. The described method1 needs some changes

hardware_interface/doc/writing_new_hardware_component.rst

saikishor · 2025-07-12T14:49:10Z

@soham2560 there are some failing tests, can you review them?

Thanks

Co-authored-by: Sai Kishor Kothakota <[email protected]>

christophfroehlich

Just thinking if we can move the logic into any helper methods file, instead of duplicating it in every interface class?

soham2560 · 2025-07-15T05:39:54Z

Just thinking if we can move the logic into any helper methods file, instead of duplicating it in every interface class?

Theres a future PR where we merge system, sensor and actuator since theres a lot of overlapping code, that should take care of this imo, or should it be done here regardless?

christophfroehlich · 2025-07-15T12:29:07Z

Just thinking if we can move the logic into any helper methods file, instead of duplicating it in every interface class?

Theres a future PR where we merge system, sensor and actuator since theres a lot of overlapping code, that should take care of this imo, or should it be done here regardless?

ok if there are already plans in doing so, i'm also fine with that

MarqRazz

Is there anything special we need to do to make it so that the interface_node and the CM_node do shared memory ros topics because they are part of the same component?

MarqRazz · 2025-07-15T19:32:06Z

hardware_interface/include/hardware_interface/system_interface.hpp

+        "Executor is not available during hardware component initialization for '%s'. Skipping "
+        "node creation!",


How often does this happen? Can we try for the lock more than once?

This doesn't happen as long as the Executor is a valid pointer. We had to add it as we started adding this new API now, so all the tests are calling this without a valid executor.

saikishor · 2025-07-15T19:42:20Z

Is there anything special we need to do to make it so that the interface_node and the CM_node do shared memory ros topics because they are part of the same component?

This can be done through NodeOptions by setting the use_intra_process_comms. We can add an API something like below

ros2_control/controller_interface/include/controller_interface/controller_interface_base.hpp

Lines 215 to 226 in ad47e2a

    
             virtual rclcpp::NodeOptions define_custom_node_options() const 
        
             { 
        
               rclcpp::NodeOptions node_options; 
        
           // \note The versions conditioning is added here to support the source-compatibility with Humble 
        
           #if RCLCPP_VERSION_MAJOR >= 21 
        
               node_options.enable_logger_service(true); 
        
           #else 
        
               node_options.allow_undeclared_parameters(true); 
        
               node_options.automatically_declare_parameters_from_overrides(true); 
        
           #endif 
        
               return node_options; 
        
             }

May be we can do it in a separate PR?

soham2560 added 2 commits June 30, 2025 03:47

added draft for default node

bdf7ad4

added get node method

e7e319b

github-actions bot requested review from anfemosa, aprotyas, christophfroehlich, destogl, fmauch, mhubii and progtologist June 30, 2025 04:53

bmagyar requested changes Jul 9, 2025

View reviewed changes

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

saikishor requested changes Jul 9, 2025

View reviewed changes

soham2560 and others added 9 commits July 11, 2025 09:37

Update hardware_interface/include/hardware_interface/actuator_interfa…

b077541

…ce.hpp Co-authored-by: Sai Kishor Kothakota <[email protected]>

Update hardware_interface/include/hardware_interface/sensor_interface…

adcb985

….hpp Co-authored-by: Sai Kishor Kothakota <[email protected]>

Update hardware_interface/include/hardware_interface/system_interface…

cad5970

….hpp Co-authored-by: Sai Kishor Kothakota <[email protected]>

Update hardware_interface/include/hardware_interface/actuator_interfa…

d56e13f

…ce.hpp Co-authored-by: Sai Kishor Kothakota <[email protected]>

Update hardware_interface/include/hardware_interface/sensor_interface…

c0cc99c

….hpp Co-authored-by: Sai Kishor Kothakota <[email protected]>

Update hardware_interface/include/hardware_interface/system_interface…

fdb8162

….hpp Co-authored-by: Sai Kishor Kothakota <[email protected]>

edited suggestions commits

88b8136

Merge branch 'master' into feat/defaultnode

4e14661

added test for checking is node is added

c1c726d

soham2560 requested review from bmagyar and saikishor July 12, 2025 09:37

soham2560 mentioned this pull request Jul 12, 2025

[Example 17] RRBot with Hardware Component that publishes diagnostics ros-controls/ros2_control_demos#840

Open

saikishor reviewed Jul 12, 2025

View reviewed changes

saikishor previously approved these changes Jul 12, 2025

View reviewed changes

hardware_interface_testing/test/test_resource_manager.cpp Outdated Show resolved Hide resolved

christophfroehlich previously approved these changes Jul 12, 2025

View reviewed changes

Update hardware_interface_testing/test/test_resource_manager.cpp

2d70891

Co-authored-by: Sai Kishor Kothakota <[email protected]>

soham2560 dismissed stale reviews from christophfroehlich and saikishor via 2d70891 July 12, 2025 11:35

soham2560 added 3 commits July 12, 2025 12:34

made node name lower case

7750bd7

ran pre-commit

991d12a

added documentation

83b5631

saikishor requested changes Jul 12, 2025

View reviewed changes

soham2560 and others added 3 commits July 12, 2025 13:43

updated reference syntax

653d8ad

updated to be more general

432b70d

Update hardware_interface/doc/writing_new_hardware_component.rst

93636da

Co-authored-by: Sai Kishor Kothakota <[email protected]>

saikishor requested changes Jul 12, 2025

View reviewed changes

Merge branch 'master' into feat/defaultnode

7ec6ae0

soham2560 and others added 9 commits July 13, 2025 16:20

Update hardware_interface/doc/writing_new_hardware_component.rst

0089ede

Co-authored-by: Sai Kishor Kothakota <[email protected]>

Update hardware_interface/doc/writing_new_hardware_component.rst

d0f48ea

Co-authored-by: Sai Kishor Kothakota <[email protected]>

Update hardware_interface/doc/writing_new_hardware_component.rst

60b9ddd

Co-authored-by: Sai Kishor Kothakota <[email protected]>

Update hardware_interface/doc/writing_new_hardware_component.rst

4e4a248

Co-authored-by: Sai Kishor Kothakota <[email protected]>

updated documentation to correct access logic

afed96d

updated test to match lowercase

a96acbd

Merge branch 'master' into feat/defaultnode

98c58e3

remove reference to example 17

b167a23

updated readme

d43af23

christophfroehlich approved these changes Jul 15, 2025

View reviewed changes

soham2560 requested a review from saikishor July 15, 2025 07:07

MarqRazz reviewed Jul 15, 2025

View reviewed changes

Merge branch 'master' into feat/defaultnode

e1724d4

bmagyar approved these changes Jul 16, 2025

View reviewed changes

saikishor approved these changes Jul 16, 2025

View reviewed changes

bmagyar merged commit d376254 into ros-controls:master Jul 16, 2025
23 of 28 checks passed

		"Executor is not available during hardware component initialization for '%s'. Skipping "
		"node creation!",

Addition of a Default Node for Hardware Component #2348

Addition of a Default Node for Hardware Component #2348

Uh oh!

Conversation

soham2560 commented Jun 30, 2025

Brief

Uh oh!

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

Uh oh!

Codecov Report

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

saikishor Jul 12, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

soham2560 Jul 12, 2025

Choose a reason for hiding this comment

Uh oh!

saikishor left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

christophfroehlich left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

saikishor Jul 12, 2025

Choose a reason for hiding this comment

Uh oh!

soham2560 Jul 12, 2025

Choose a reason for hiding this comment

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!

saikishor commented Jul 12, 2025

Uh oh!

christophfroehlich left a comment

Choose a reason for hiding this comment

Uh oh!

soham2560 commented Jul 15, 2025

Uh oh!

christophfroehlich commented Jul 15, 2025

Uh oh!

MarqRazz left a comment

Choose a reason for hiding this comment

Uh oh!

MarqRazz Jul 15, 2025

Choose a reason for hiding this comment

Uh oh!

saikishor Jul 15, 2025

Choose a reason for hiding this comment

Uh oh!

saikishor commented Jul 15, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

codecov bot commented Jun 30, 2025 •

edited

Loading

saikishor Jul 12, 2025 •

edited

Loading

saikishor commented Jul 15, 2025 •

edited

Loading