Some modifications

rqt_image_overlay/src/compositor.cpp

@@ -22,19 +22,22 @@ namespace rqt_image_overlay
 {
 
 Compositor::Compositor(
-  const ImageManager & imageManager, const OverlayManager & overlayManager,
-  float frequency, rclcpp::Duration window)
-: imageManager(imageManager), overlayManager(overlayManager), window_(window)
+  const ImageManager & imageManager, const OverlayManager & overlayManager, float frequency)
+: imageManager(imageManager), overlayManager(overlayManager),
+  window_(std::make_shared<rclcpp::Duration>(0, 300000000))
 {
   startTimer(1000.0 / frequency);
 }
 
-void Compositor::setWindow(rclcpp::Duration window) {
-  window_ = window;
+void Compositor::setWindow(const rclcpp::Duration & window)
+{
+  std::atomic_store(&window_, std::make_shared<rclcpp::Duration>(window));
 }
 
-rclcpp::Duration Compositor::window() const {
-  return window_;
+rclcpp::Duration Compositor::getWindow() const
+{
+  auto window = std::atomic_load(&window_);
+  return *window;
 }
 
 void Compositor::setCallableSetImage(std::function<void(std::shared_ptr<QImage>)> setImage)
@@ -48,7 +51,10 @@ std::shared_ptr<QImage> Compositor::compose()
     return nullptr;
   }
 
-  rclcpp::Time targetTime = systemClock.now() - window_;
+  // Get window_ value through the getWindow() method so that it is thread-safe
+  auto window = getWindow();
+
+  rclcpp::Time targetTime = systemClock.now() - window;
   auto [composition, imageHeaderTime] = imageManager.getClosestImageAndHeaderTime(targetTime);
   OverlayTimeInfo overlayTimeInfo{targetTime, imageHeaderTime};
   overlayManager.overlay(*composition, overlayTimeInfo);

rqt_image_overlay/src/compositor.hpp

@@ -32,12 +32,11 @@ class Compositor : public QObject
 
 public:
   Compositor(
-    const ImageManager & imageManager, const OverlayManager & overlayManager,
-    float frequency, rclcpp::Duration window = rclcpp::Duration{0, 300000000});
+    const ImageManager & imageManager, const OverlayManager & overlayManager, float frequency);
 
-  void setWindow(rclcpp::Duration window);
-
-  rclcpp::Duration window() const;
+  // Thread safe setter/getter for window
+  void setWindow(const rclcpp::Duration & window);
+  rclcpp::Duration getWindow() const;
 
   void setCallableSetImage(std::function<void(std::shared_ptr<QImage>)> setImage);
 
@@ -50,7 +49,11 @@ class Compositor : public QObject
 
   std::function<void(std::shared_ptr<QImage>)> setImage;
 
-  rclcpp::Duration window_;  // Wait window for collecting messages before composing image
+  // Wait window for collecting messages before composing image.
+  // To access this value, use the getWindow() method to ensure thread-safety, even from within
+  // this class.
+  std::shared_ptr<rclcpp::Duration> window_;
+
   rclcpp::Clock systemClock{RCL_SYSTEM_TIME};
 };
 

rqt_image_overlay/src/image_overlay.cpp

@@ -36,7 +36,6 @@ ImageOverlay::~ImageOverlay() = default;
 void ImageOverlay::initPlugin(qt_gui_cpp::PluginContext & context)
 {
   ui = std::make_unique<Ui::ImageOverlay>();
-  ui_configuration_dialog = std::make_unique<Ui::ConfigurationDialog>();
   menu = std::make_unique<QMenu>();
   imageManager = std::make_shared<ImageManager>(node_);
   overlayManager = std::make_shared<OverlayManager>(node_);
@@ -99,8 +98,7 @@ void ImageOverlay::saveSettings(
     instance_settings.setValue("image_topic", QString::fromStdString(imageTopic.topic));
     instance_settings.setValue("image_transport", QString::fromStdString(imageTopic.transport));
   }
-  instance_settings.setValue("window", compositor->window().seconds());
-
+  instance_settings.setValue("compositor_window", compositor->getWindow().seconds());
   overlayManager->saveSettings(instance_settings);
 }
 
@@ -114,16 +112,11 @@ void ImageOverlay::restoreSettings(
     imageManager->addImageTopicExplicitly(ImageTopic{topic, transport});
     ui->image_topics_combo_box->setCurrentIndex(1);
   }
-  auto window_setting = instance_settings.value("window");
-  if (!window_setting.isNull()) {
-    auto window_string = window_setting.toString().toStdString();
-    char * err;
-    double window = std::strtod(window_string.c_str(), &err);
-    if (err == window_string.c_str()) {
-      // double conversion error
-    } else {
-      compositor->setWindow(rclcpp::Duration::from_seconds(window));
-    }
+
+  if (instance_settings.contains("compositor_window")) {
+    auto window_double = instance_settings.value("compositor_window").toDouble();
+    auto duration = rclcpp::Duration::from_seconds(window_double);
+    compositor->setWindow(duration);
   }
   overlayManager->restoreSettings(instance_settings);
 }
@@ -152,22 +145,15 @@ bool ImageOverlay::hasConfiguration() const
 
 void ImageOverlay::triggerConfiguration()
 {
-  if (configuration_dialog) {
-    return;
-  }
-  configuration_dialog = std::make_unique<QDialog>();
-  configuration_dialog->setAttribute(Qt::WA_DeleteOnClose);
+  auto configuration_dialog = std::make_unique<QDialog>();
+  auto ui_configuration_dialog = std::make_unique<Ui::ConfigurationDialog>();
   ui_configuration_dialog->setupUi(configuration_dialog.get());
-  ui_configuration_dialog->window->setValue(compositor->window().seconds());
-  auto connection = connect(
-    configuration_dialog.get(), &QDialog::finished, [this](int result) {
-      if (result == QDialog::Accepted) {
-        auto window_seconds = ui_configuration_dialog->window->value();
-        compositor->setWindow(rclcpp::Duration::from_seconds(window_seconds));
-      }
-      configuration_dialog.reset();
-    });
-  configuration_dialog->open();
+  ui_configuration_dialog->window->setValue(compositor->getWindow().seconds());
+
+  if (configuration_dialog->exec() == QDialog::Accepted) {
+    auto window_seconds = ui_configuration_dialog->window->value();
+    compositor->setWindow(rclcpp::Duration::from_seconds(window_seconds));
+  }
 }
 
 

rqt_image_overlay/src/image_overlay.hpp

@@ -20,7 +20,7 @@
 #include <memory>
 #include "rqt_gui_cpp/plugin.h"
 
-namespace Ui {class ImageOverlay;class ConfigurationDialog;}
+namespace Ui {class ImageOverlay;}
 namespace rqt_image_overlay
 {
 class ImageManager;
@@ -52,12 +52,10 @@ public slots:
   void fillOverlayMenu();
 
   std::unique_ptr<Ui::ImageOverlay> ui;
-  std::unique_ptr<Ui::ConfigurationDialog> ui_configuration_dialog;
   std::unique_ptr<QMenu> menu;
   std::shared_ptr<ImageManager> imageManager;
   std::shared_ptr<OverlayManager> overlayManager;
   std::unique_ptr<Compositor> compositor;
-  std::unique_ptr<QDialog> configuration_dialog;
 };
 
 }  // namespace rqt_image_overlay