Skip to content

Conversation

@destogl
Copy link
Member

@destogl destogl commented Dec 22, 2025

In the async_handler_callback function, I think we should first call write than read. The reason for this is that in that case we would at least have a chance of actually running the {read » update » write » sleep} cycle, where now it is basically always [update » read » write » sleep]. Exactly, it would probably be from HW perspective {write » read » sleep » (probably) update} (assuming the same frequency). Of course we don't know, but the chance that the values are updated between read and write is very low. So in the current way, we are guaranteed to be at least one cycle behind; in the opposite case, we would have a chance to be more "in the cycle" with the most recent read data.

Why is this important? When we think about "SLAVE" mode, then read is usually blocking from the HW, so there we want to “sleep” and wait for the newest data and finish the execution of the loop. Hopefully update gets called, and then we can write the values. Nevertheless, I don't see we can do anything better, as we should not be slow and waiting for update as it might break the connection to the robot.

Removed timing and return value checks from read and write operations.
@codecov
Copy link

codecov bot commented Dec 22, 2025

Codecov Report

❌ Patch coverage is 77.77778% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 89.51%. Comparing base (b424f14) to head (3d49765).

Files with missing lines Patch % Lines
...ardware_interface/hardware_component_interface.hpp 77.77% 1 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #2926      +/-   ##
==========================================
- Coverage   89.53%   89.51%   -0.02%     
==========================================
  Files         156      156              
  Lines       18624    18624              
  Branches     1493     1493              
==========================================
- Hits        16675    16672       -3     
- Misses       1347     1349       +2     
- Partials      602      603       +1     
Flag Coverage Δ
unittests 89.51% <77.77%> (-0.02%) ⬇️

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

Files with missing lines Coverage Δ
...ardware_interface/hardware_component_interface.hpp 75.00% <77.77%> (-0.09%) ⬇️

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

@mergify
Copy link
Contributor

mergify bot commented Dec 29, 2025

This pull request is in conflict. Could you fix it @destogl?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants