Skip to content

Conversation

@saikishor
Copy link
Member

No description provided.

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 for working on this. But this is a (maybe legit) behavior change, not only a fix for the tests, right?

@codecov
Copy link

codecov bot commented Nov 16, 2025

Codecov Report

❌ Patch coverage is 83.33333% with 3 lines in your changes missing coverage. Please review.
✅ Project coverage is 89.61%. Comparing base (30416ba) to head (ac70c1b).
⚠️ Report is 1 commits behind head on master.

Files with missing lines Patch % Lines
...roller_interface/src/controller_interface_base.cpp 78.57% 1 Missing and 2 partials ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #2828      +/-   ##
==========================================
- Coverage   89.62%   89.61%   -0.01%     
==========================================
  Files         152      152              
  Lines       17817    17831      +14     
  Branches     1455     1457       +2     
==========================================
+ Hits        15968    15980      +12     
- Misses       1263     1264       +1     
- Partials      586      587       +1     
Flag Coverage Δ
unittests 89.61% <83.33%> (-0.01%) ⬇️

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

Files with missing lines Coverage Δ
...ntroller_interface/controller_interface_params.hpp 100.00% <ø> (ø)
...oller_interface/test/test_controller_interface.cpp 100.00% <100.00%> (ø)
controller_manager/src/controller_manager.cpp 77.09% <100.00%> (+0.06%) ⬆️
...ontroller_manager/test/test_controller_manager.cpp 96.22% <100.00%> (ø)
...roller_interface/src/controller_interface_base.cpp 82.03% <78.57%> (-0.55%) ⬇️
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@saikishor
Copy link
Member Author

Thanks for working on this. But this is a (maybe legit) behavior change, not only a fix for the tests, right?

Yup, I think it is a good fix in general and can be backported too IMHO. Because, if you internally use the update_rate inside your code and have a weird value as the update rate, then your logic might be messed up with what's happening in reality

@saikishor saikishor added backport-jazzy Triggers PR backport to ROS 2 jazzy. backport-kilted Triggers PR backport to ROS 2 kilted. labels Nov 16, 2025
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.

I like. Maybe worth a release note?

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.

Thx

@christophfroehlich christophfroehlich changed the title Fix the flaky controller update rate tests Calculate the achievable update rate of controllers if no common divisor with cm update rate Nov 17, 2025
@saikishor saikishor marked this pull request as draft November 21, 2025 11:02
@saikishor saikishor marked this pull request as ready for review November 21, 2025 11:34
@christophfroehlich christophfroehlich changed the title Calculate the achievable update rate of controllers if no common divisor with cm update rate Calculate achievable update rate of controllers Nov 21, 2025
@christophfroehlich christophfroehlich merged commit cde86ae into ros-controls:master Nov 21, 2025
17 of 21 checks passed
@christophfroehlich christophfroehlich deleted the fix/flaky/cm_update_rate_tests branch November 21, 2025 14:08
mergify bot pushed a commit that referenced this pull request Nov 21, 2025
mergify bot pushed a commit that referenced this pull request Nov 21, 2025
saikishor added a commit that referenced this pull request Nov 21, 2025
(cherry picked from commit cde86ae)

Co-authored-by: Sai Kishor Kothakota <sai.kishor@pal-robotics.com>
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