Skip to content

Conversation

@saikishor
Copy link
Member

Hello!

I've observed a lot of blocking time of the RT loop due to the calls of the lifecycle state and this PR comes up with a way to cache them locally and use them.

With the current master's version:

RRBot
image
image

TALOS

image

With the proposed changes:

RRBot
image

TALOS
image

@saikishor saikishor self-assigned this Dec 4, 2025
@saikishor saikishor added backport-jazzy Triggers PR backport to ROS 2 jazzy. backport-kilted Triggers PR backport to ROS 2 kilted. labels Dec 4, 2025
@saikishor saikishor force-pushed the fix/lifecycle_state/inconsistent_times branch from 8ae2a75 to 85925f0 Compare December 4, 2025 18:43
@codecov
Copy link

codecov bot commented Dec 4, 2025

Codecov Report

❌ Patch coverage is 84.21053% with 18 lines in your changes missing coverage. Please review.
✅ Project coverage is 89.39%. Comparing base (936d08e) to head (122deae).
⚠️ Report is 4 commits behind head on master.

Files with missing lines Patch % Lines
..._components/test_actuator_exclusive_interfaces.cpp 54.54% 4 Missing and 1 partial ⚠️
hardware_interface/src/hardware_component.cpp 69.23% 0 Missing and 4 partials ⚠️
...chainable_controller/test_chainable_controller.cpp 81.25% 2 Missing and 1 partial ⚠️
...r_manager/test/test_controller/test_controller.cpp 75.00% 2 Missing and 1 partial ⚠️
...rface_testing/test/test_components/test_system.cpp 66.66% 2 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #2884      +/-   ##
==========================================
- Coverage   89.41%   89.39%   -0.03%     
==========================================
  Files         155      155              
  Lines       18144    18209      +65     
  Branches     1478     1483       +5     
==========================================
+ Hits        16224    16278      +54     
- Misses       1329     1337       +8     
- Partials      591      594       +3     
Flag Coverage Δ
unittests 89.39% <84.21%> (-0.03%) ⬇️

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

Files with missing lines Coverage Δ
...controller_interface/controller_interface_base.hpp 91.66% <ø> (ø)
...r_interface/src/chainable_controller_interface.cpp 58.65% <100.00%> (ø)
...roller_interface/src/controller_interface_base.cpp 84.21% <100.00%> (+2.17%) ⬆️
...oller_interface/test/test_controller_interface.cpp 100.00% <ø> (ø)
controller_manager/src/controller_manager.cpp 77.25% <100.00%> (+0.05%) ⬆️
...ager/test/test_controller_manager_urdf_passing.cpp 100.00% <ø> (ø)
.../include/hardware_interface/hardware_component.hpp 100.00% <ø> (ø)
...ardware_interface/hardware_component_interface.hpp 75.08% <100.00%> (+0.08%) ⬆️
hardware_interface/src/resource_manager.cpp 78.10% <100.00%> (-0.06%) ⬇️
...dware_interface/test/test_component_interfaces.cpp 97.88% <ø> (ø)
... and 5 more
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Copy link
Contributor

@MarqRazz MarqRazz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tested locally with the RRBot and the update times are much faster and more consistent 😍

image

@saikishor
Copy link
Member Author

@MarqRazz Thanks a lot for taking time to test this

Copy link
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.

Thanks a lot for the fix, the stats looks great.
However, I have some comments and questions while scanning over your changes.

@saikishor
Copy link
Member Author

Thanks a lot for the fix, the stats looks great.
However, I have some comments and questions while scanning over your changes.

Thanks for the review @christophfroehlich

Copy link
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.

LGTM

@christophfroehlich
Copy link
Member

this is breaking ABI of controllers and hardware base classes, how should we handle the backports?

@saikishor
Copy link
Member Author

This is breaking ABI of controllers and hardware base classes, how should we handle the backports?

I am biased with my opinion, as we have a real need for this on our hardware. Without this, the robot will not be functional for long runs. The jazzy sync has just happened, so if we want, we can add it to the next release

@christophfroehlich christophfroehlich merged commit dca9b57 into ros-controls:master Dec 6, 2025
16 of 18 checks passed
@christophfroehlich christophfroehlich deleted the fix/lifecycle_state/inconsistent_times branch December 6, 2025 20:31
mergify bot pushed a commit that referenced this pull request Dec 6, 2025
(cherry picked from commit dca9b57)

# Conflicts:
#	doc/release_notes.rst
mergify bot pushed a commit that referenced this pull request Dec 6, 2025
(cherry picked from commit dca9b57)

# Conflicts:
#	doc/release_notes.rst
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

None yet

Development

Successfully merging this pull request may close these issues.

3 participants