Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Add Lock-free Queue #253

Merged

Conversation

saikishor
Copy link
Member

Fixes #14

@codecov-commenter
Copy link

codecov-commenter commented Jan 1, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 71.67%. Comparing base (db891ef) to head (f764bc8).

Additional details and impacted files
@@            Coverage Diff             @@
##           master     #253      +/-   ##
==========================================
+ Coverage   70.12%   71.67%   +1.55%     
==========================================
  Files           8        9       +1     
  Lines         492      519      +27     
  Branches       84       85       +1     
==========================================
+ Hits          345      372      +27     
  Misses         92       92              
  Partials       55       55              
Flag Coverage Δ
unittests 71.67% <100.00%> (+1.55%) ⬆️

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

Files with missing lines Coverage Δ
include/realtime_tools/lock_free_queue.hpp 100.00% <100.00%> (ø)

Copy link
Contributor

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

TBH, I don't fully understand all the C++ specifics here 😬
But the API seems to be clean from a user perspective, looking at the tests.

  • Does this replace the RealtimeBuffer? Or should the RealtimeBuffer be rewritten to use this queue with capacity of 1?
  • What is the practical difference between the RealtimeBox and the queue with capacity of 1? Maybe this is clear for some, but I'd need (and appreciate) a user guide when to use what.
  • If we deprecate something, we can also add migration notes to this repository and link it on control.ros.org

saikishor and others added 2 commits January 2, 2025 21:56
Co-authored-by: Christoph Fröhlich <christophfroehlich@users.noreply.github.com>
Copy link
Contributor

@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 tested that with ros-controls/ros2_controllers#1480, the API works for me!

@saikishor
Copy link
Member Author

I tested that with ros-controls/ros2_controllers#1480, the API works for me!

@christophfroehlich I had to rewrite some parts of tests to reuse rather than copying for every instance. I've add more tests for all constructable types

@bmagyar bmagyar merged commit 1d66fed into ros-controls:master Jan 29, 2025
19 checks passed
@saikishor saikishor deleted the boost/lockfree/realtime/buffer branch January 29, 2025 18:41
@saikishor saikishor added backport-humble This label should be used by maintainers only! Label triggers PR backport to ROS 2 humble. backport-jazzy This label should be used by maintainers only! Label triggers PR backport to ROS 2 humble. labels Jan 29, 2025
mergify bot pushed a commit that referenced this pull request Jan 29, 2025
(cherry picked from commit 1d66fed)
mergify bot pushed a commit that referenced this pull request Jan 29, 2025
(cherry picked from commit 1d66fed)
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
backport-humble This label should be used by maintainers only! Label triggers PR backport to ROS 2 humble. backport-jazzy This label should be used by maintainers only! Label triggers PR backport to ROS 2 humble.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Lock-free buffer implementation
5 participants