Skip to content

Conversation

saikishor
Copy link
Member

@saikishor saikishor commented Aug 11, 2025

Hello!

This PR implements a way to switch controllers (activation or deactivation) in the non-RT loop, this is the default behavior of ros_control, and I have observed that activating some broadcasters and some controllers is causing some overruns in our system and this should bride the gap as well work without any issues. I've also added an option to switch between the behaviors.

I could find it, thanks to the changes from #2449

TIAGo Pro running at 1 kHz

With the realtime switching on (The results are in microseconds)

image

WIth the non-realtime switching

image

TALOS Running at 2 kHz

With the realtime switching on (The results are in microseconds)

image

WIth the non-realtime switching

image

Copy link

codecov bot commented Aug 11, 2025

Codecov Report

❌ Patch coverage is 95.83333% with 11 lines in your changes missing coverage. Please review.
✅ Project coverage is 89.28%. Comparing base (e433836) to head (1c5ced4).
⚠️ Report is 1 commits behind head on master.

Files with missing lines Patch % Lines
controller_manager/src/controller_manager.cpp 91.26% 9 Missing ⚠️
...er_manager/test/controller_manager_test_common.hpp 95.23% 0 Missing and 1 partial ⚠️
...ller_manager/test/test_controller_manager_srvs.cpp 99.20% 0 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #2452      +/-   ##
==========================================
+ Coverage   89.21%   89.28%   +0.06%     
==========================================
  Files         144      144              
  Lines       16334    16499     +165     
  Branches     1393     1394       +1     
==========================================
+ Hits        14573    14731     +158     
- Misses       1225     1228       +3     
- Partials      536      540       +4     
Flag Coverage Δ
unittests 89.28% <95.83%> (+0.06%) ⬆️

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

Files with missing lines Coverage Δ
controller_manager/controller_manager/spawner.py 71.15% <100.00%> (+0.37%) ⬆️
.../include/controller_manager/controller_manager.hpp 97.36% <100.00%> (+0.49%) ⬆️
...r_manager/test/test_controller/test_controller.cpp 87.01% <100.00%> (+0.71%) ⬆️
...r_manager/test/test_controller/test_controller.hpp 100.00% <ø> (ø)
...e_interface/include/hardware_interface/helpers.hpp 96.55% <100.00%> (ø)
...er_manager/test/controller_manager_test_common.hpp 89.18% <95.23%> (-0.65%) ⬇️
...ller_manager/test/test_controller_manager_srvs.cpp 99.31% <99.20%> (-0.02%) ⬇️
controller_manager/src/controller_manager.cpp 74.50% <91.26%> (+0.07%) ⬆️

... and 1 file with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@saikishor saikishor added the backport-jazzy Triggers PR backport to ROS 2 jazzy. label Aug 11, 2025
@@ -141,6 +141,13 @@ def main(args=None):
action="store_true",
required=False,
)
parser.add_argument(
Copy link
Contributor

Choose a reason for hiding this comment

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

this means that you change the default behavior, right?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, for the rolling that's the idea. In ros_control that's the default behavior. I'm also fine to change it back to true.

Copy link
Member Author

Choose a reason for hiding this comment

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

If you activate-as-group a large number of controllers, you will definitely see a difference that's why I set it as False by default

Copy link
Contributor

Choose a reason for hiding this comment

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

ok, so let's add this explicitly in the release notes (and migration notes maybe?)

Copy link
Member Author

Choose a reason for hiding this comment

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

Sure thing 🫡

@saikishor saikishor force-pushed the non-realtime/switching/controllers branch from 758246f to 2c5cad4 Compare August 15, 2025 06:55
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.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants