Skip to content

Commit

Permalink
Drain queue on exit
Browse files Browse the repository at this point in the history
It seems like publishing a ROS message is not passing the data to the
executor so the executor sends the data during `spin_once`. That said,
it might not be synchronous either as this is
(middleware-)implementation defined. See
[`rmw_publish_loaned_message`][1]. It seems like by default, FastDDS
uses an async mode which uses an internal thread to send the data.
That's fine with this code I think and we can basically treat that
once we call publish, the message is basically sent.

Thus, instead of `spin_once` after stop is requested (due to signals), we
simply drain the queue directly from `Ros2ExecutorThread`. This requires
making `Ros2ExecutorThread` to be a friend of `Ros2Adapter`. There's no
issues with thread (as we are using a SPSC queue for the published
message) because the `TimerCallback` of `Ros2Adapter` is executing as a part
of `Ros2ExecutorThread` anyway. Moving it out of `TimerCallback` and into
`Ros2ExecutorThread` is no different from a threading perspective.

Fixes #104.

[1]: https://docs.ros.org/en/jazzy/p/rmw/generated/function_rmw_8h_1ab01da69d8613952343abd5d65107399a.html
  • Loading branch information
shuhaowu committed Aug 21, 2024
1 parent 86bc1e2 commit 1042897
Show file tree
Hide file tree
Showing 2 changed files with 5 additions and 2 deletions.
4 changes: 4 additions & 0 deletions include/cactus_rt/ros2/ros2_adapter.h
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,11 @@

namespace cactus_rt::ros2 {

class Ros2ExecutorThread;

class Ros2Adapter {
friend class Ros2ExecutorThread;

public:
struct Config {
/**
Expand Down
3 changes: 1 addition & 2 deletions src/cactus_rt/ros2/app.cc
Original file line number Diff line number Diff line change
Expand Up @@ -25,8 +25,7 @@ void Ros2ExecutorThread::Run() {
executor_->spin_once();
}

// Execute one more time to ensure everything is processed.
executor_->spin_once();
ros2_adapter_->DrainQueues();

executor_->remove_node(node_ptr);
}
Expand Down

0 comments on commit 1042897

Please sign in to comment.