Skip to content

Conversation

@saikishor
Copy link
Member

Right now, the image looks like below instead of the expected_total_time. inorder to fix it, we need to move the statistics publishing to the write callback part, and the other one is good at its place
image

@saikishor saikishor added backport-jazzy Triggers PR backport to ROS 2 jazzy. backport-kilted Triggers PR backport to ROS 2 kilted. labels Nov 27, 2025
@saikishor saikishor changed the title Fix the instance of CM statistics publication Fix the CM statistics async publish placement Nov 27, 2025
@codecov
Copy link

codecov bot commented Nov 28, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 89.57%. Comparing base (380622f) to head (f1f1cc9).
⚠️ Report is 1 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #2865      +/-   ##
==========================================
+ Coverage   89.56%   89.57%   +0.01%     
==========================================
  Files         155      155              
  Lines       18105    18104       -1     
  Branches     1474     1474              
==========================================
+ Hits        16215    16217       +2     
+ Misses       1302     1300       -2     
+ Partials      588      587       -1     
Flag Coverage Δ
unittests 89.57% <ø> (+0.01%) ⬆️

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

Files with missing lines Coverage Δ
controller_manager/src/controller_manager.cpp 77.29% <ø> (+0.13%) ⬆️
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

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.

To understand that: We want to publish the data once per cm cycle, which is read-update-write-wait. And earlier this was updated in the update method, which means the data of the write were one cycle old?

@Juliaj
Copy link
Contributor

Juliaj commented Nov 28, 2025

To understand that: We want to publish the data once per cm cycle, which is read-update-write-wait. And earlier this was updated in the update method, which means the data of the write were one cycle old?

My interpretation is that we want to publish the data(_total_time) right after it is updated.

Copy link
Contributor

@Juliaj Juliaj left a comment

Choose a reason for hiding this comment

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

LGTM.

@saikishor
Copy link
Member Author

To understand that: We want to publish the data once per cm cycle, which is read-update-write-wait. And earlier this was updated in the update method, which means the data of the write were one cycle old?

Yes, it was one cycle old, and that's why the visualization didn't add up

@christophfroehlich christophfroehlich merged commit f0f003f into ros-controls:master Nov 29, 2025
17 checks passed
mergify bot pushed a commit that referenced this pull request Nov 29, 2025
mergify bot pushed a commit that referenced this pull request Nov 29, 2025
saikishor added a commit that referenced this pull request Nov 29, 2025
(cherry picked from commit f0f003f)

Co-authored-by: Sai Kishor Kothakota <[email protected]>
saikishor added a commit that referenced this pull request Nov 29, 2025
(cherry picked from commit f0f003f)

Co-authored-by: Sai Kishor Kothakota <[email protected]>
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