ros2_control icon indicating copy to clipboard operation
ros2_control copied to clipboard

Working async controllers and components [not synchronized]

Open VX792 opened this issue 1 year ago • 3 comments

I haven't found a good way to synchronize the controllers / components with the ros2 control node's thread without changing a lot of stuff inside the handles (and controllers), and since they are getting reworked anyway, I thought I'll wait until then.

Before that, this solution assumes that all writes to the command interfaces' double values are atomic operations.

VX792 avatar May 29 '23 13:05 VX792

Codecov Report

Attention: Patch coverage is 40.00000% with 36 lines in your changes are missing coverage. Please review.

Project coverage is 31.67%. Comparing base (35bb5f7) to head (ad1510a). Report is 20 commits behind head on master.

:exclamation: Current head ad1510a differs from pull request most recent head 9469f8b. Consider uploading reports for the commit 9469f8b to get more accurate results

Additional details and impacted files
@@             Coverage Diff             @@
##           master    #1041       +/-   ##
===========================================
- Coverage   75.51%   31.67%   -43.84%     
===========================================
  Files          41       93       +52     
  Lines        3328    10854     +7526     
  Branches     1921     7428     +5507     
===========================================
+ Hits         2513     3438      +925     
- Misses        421      793      +372     
- Partials      394     6623     +6229     
Flag Coverage Δ
unittests 31.67% <40.00%> (-43.84%) :arrow_down:

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

Files Coverage Δ
.../include/controller_manager/controller_manager.hpp 60.00% <ø> (+27.85%) :arrow_up:
controller_manager/src/ros2_control_node.cpp 0.00% <0.00%> (ø)
...lude/hardware_interface/loaned_state_interface.hpp 76.92% <66.66%> (+10.25%) :arrow_up:
...de/hardware_interface/loaned_command_interface.hpp 81.81% <50.00%> (-18.19%) :arrow_down:
...re_interface/include/hardware_interface/handle.hpp 80.00% <71.42%> (-20.00%) :arrow_down:
...ce/include/hardware_interface/async_components.hpp 0.00% <0.00%> (ø)
controller_manager/src/controller_manager.cpp 38.13% <0.00%> (-29.93%) :arrow_down:
hardware_interface/src/resource_manager.cpp 46.66% <0.00%> (-30.19%) :arrow_down:
hardware_interface/test/test_handle.cpp 46.00% <50.00%> (ø)

... and 77 files with indirect coverage changes

codecov-commenter avatar May 29 '23 13:05 codecov-commenter

I'll create the final version after the handle rework is merged, as it will be easier to support atomic types after that.

Do we wish to backport this to humble though? Because in that case, I think I can finish this PR by adding async systems / sensors / actuators, and if I'm not wrong it could be done with the current structure without breaking the API.

VX792 avatar Dec 03 '23 21:12 VX792

After some internal discussion with @saikishor and @jordan-palacios, we are open to accept this PR.

Even though we still have concerns about data coherency and random sampling, maybe the community can benefit anyway of @VX792's effort.

atzaros avatar Mar 13 '24 16:03 atzaros

@mergifyio backport humble iron

destogl avatar May 08 '24 17:05 destogl

backport humble iron

✅ Backports have been created

mergify[bot] avatar May 08 '24 17:05 mergify[bot]