Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

Add record, stop, start_discovery, stop_discovery and is_discovery_stopped services for rosbag2_transport::Recorder #1840

Open
wants to merge 9 commits into
base: rolling
Choose a base branch
from
Prev Previous commit
Next Next commit
Run topic discovery if stopped
Signed-off-by: Sharmin Ramli <sharmin.ramli@enway.ai>
  • Loading branch information
sharminramli committed Oct 17, 2024
commit 56cd53d52c6cb298f00d311c8f9fe10d90675488
4 changes: 2 additions & 2 deletions rosbag2_transport/src/rosbag2_transport/recorder.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -145,7 +145,7 @@ class RecorderImpl

std::mutex start_stop_transition_mutex_;
std::mutex discovery_mutex_;
std::atomic<bool> stop_discovery_ = false;
std::atomic<bool> stop_discovery_ = true;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

stop_discovery_ shall be false by default. Otherwise, we will immediately exit from RecorderImpl::topics_discovery() when running it the first time.

Suggested change
std::atomic<bool> stop_discovery_ = true;
std::atomic<bool> stop_discovery_ = false;

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

stop_discovery_ will be exchanged for false in start_discovery(), and if it was previously true, then topics_discovery will be started. it should not exit because of the exchange.

subsequently stop_discovery() will exchange stop_discovery_ to true and when start_discovery() is called again it can again exchange stop_discovery_ for false and start topics_discovery()

std::atomic_uchar paused_ = 0;
std::atomic<bool> in_recording_ = false;
std::shared_ptr<KeyboardHandler> keyboard_handler_;
Expand Down Expand Up @@ -462,7 +462,7 @@ bool RecorderImpl::is_paused()
void RecorderImpl::start_discovery()
{
std::lock_guard<std::mutex> state_lock(discovery_mutex_);
if (stop_discovery_.exchange(false)) {
if (!stop_discovery_.exchange(false)) {
RCLCPP_DEBUG(node->get_logger(), "Recorder topic discovery is already running.");
} else {
discovery_future_ =
Expand Down