Codecov Report

Attention: Patch coverage is 96.29630% with 1 line in your changes missing coverage. Please review.

Project coverage is 71.48%. Comparing base (d54140d) to head (689f63c).

@@            Coverage Diff             @@
##           master     #253      +/-   ##
==========================================
+ Coverage   70.12%   71.48%   +1.36%     
==========================================
  Files           8        9       +1     
  Lines         492      519      +27     
  Branches       84       85       +1     
==========================================
+ Hits          345      371      +26     
  Misses         92       92              
- Partials       55       56       +1     

@christophfroehlich First of all, thanks for reviewing it

• Does this replace the RealtimeBuffer? Or should the RealtimeBuffer be rewritten to use this queue with capacity of 1?

I believe yes, we can replace this with the RealtimeBuffer. Ideally, it should serve as a replacement. I wouldn't mix them to add backward compatibility. I think it is better to separate both approaches.

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

IMO, RealtimeBox is something you have to use, if you want to protect the data from concurrent access. For me, it is something to having the same thing protected by the mutex. However, to be frank I'm very skeptical of it being used with the shared_ptrs, because you can still access the contents outside of the RealtimeBox and this is couldn't be safe at all.

Queue of capacity one might not be a good thing to use here, as it is meant to be used for buffer operations. Moreover, a queue of 1 is not practical, as soon as you have an element, you cannot push elements unless you have a consumer. Ofcourse, this can be done with bounded_push.

• If we deprecate something, we can also add migration notes to this repository and link it on control.ros.org

If it's okay for you. I would separate deprecation into a different PR as this is not the main intention of this PR. Once we have it cleanly integrated with ros2_controllers, then we can cleanly deprecate it. What do you think?