provide specialisation for pointer with deprecation notice

include/realtime_tools/realtime_box.h

@@ -52,9 +52,18 @@ constexpr auto is_ptr_or_smart_ptr = rcpputils::is_pointer<T>::value;
     You can use pointers with this box but the access will be different.
     Only use the get/set methods that take function pointer for accessing the internal value.
 */
-template <class T, typename mutex_type = std::mutex>
-class RealtimeBox
+
+// Provide a base template for the class
+template <class T, typename mutex_type = std::mutex, bool = is_ptr_or_smart_ptr<T>>
+class RealtimeBox;
+
+// Provide a specialisation for non pointer types
+// NOTE: When migrating to a safe access only version just remove the specialisation for pointer
+// and let this be the only version!
+template <class T, typename mutex_type>
+class RealtimeBox<T, mutex_type, false>
 {
+  static_assert(!is_ptr_or_smart_ptr<T>);
   static_assert(
     std::is_same_v<mutex_type, std::mutex> || std::is_same_v<mutex_type, std::recursive_mutex>);
   static_assert(std::is_copy_constructible_v<T>, "Passed type must be copy constructible");
@@ -227,6 +236,194 @@ class RealtimeBox
   T value_;
   mutable mutex_t lock_;
 };
+
+/**
+ * @brief Specialisation for pointer types.
+ * WHY is this specialised. We do not want to break compatibility but show a deprecation note
+ * for get/set etc. methods if used with a pointer type
+ * They are unsafe to use and should therefore be replace with their correspondents that take an std::function for accessing
+ * the value behind the pointer
+*/
+template <class T, typename mutex_type>
+class RealtimeBox<T, mutex_type, true>
+{
+  static_assert(is_ptr_or_smart_ptr<T>);
+  static_assert(
+    std::is_same_v<mutex_type, std::mutex> || std::is_same_v<mutex_type, std::recursive_mutex>);
+  static_assert(std::is_copy_constructible_v<T>, "Passed type must be copy constructible");
+
+public:
+  using mutex_t = mutex_type;
+  using type = T;
+  // Provide various constructors
+  constexpr explicit RealtimeBox(const T & init = T{}) : value_(init) {}
+  constexpr explicit RealtimeBox(const T && init) : value_(std::move(init)) {}
+
+  // Only enabled for types that can be constructed from an initializer list
+  template <typename U = T>
+  constexpr RealtimeBox(
+    const std::initializer_list<U> & init,
+    std::enable_if_t<std::is_constructible_v<U, std::initializer_list>>)
+  : value_(init)
+  {
+  }
+
+  /**
+   * @brief set a new content with best effort
+   * @return false if mutex could not be locked
+   * @note disabled for pointer types
+   */
+  [[deprecated("trySet is not safe for pointer types - use trySet(std::function...) instead")]]
+  bool trySet(const T & value)
+  {
+    std::unique_lock<mutex_t> guard(lock_, std::defer_lock);
+    if (!guard.try_lock()) {
+      return false;
+    }
+    value_ = value;
+    return true;
+  }
+  /**
+   * @brief access the content readable with best effort
+   * @return false if the mutex could not be locked
+   * @note only safe way to access pointer type content (rw)
+   */
+  bool trySet(const std::function<void(T &)> & func)
+  {
+    std::unique_lock<mutex_t> guard(lock_, std::defer_lock);
+    if (!guard.try_lock()) {
+      return false;
+    }
+
+    func(value_);
+    return true;
+  }
+  /**
+   * @brief get the content with best effort
+   * @return std::nullopt if content could not be access, otherwise the content is returned
+   */
+  [[deprecated(
+    "tryGet is not safe for pointer types - use tryGet(std::function...) "
+    "instead")]] [[nodiscard]] std::optional<T>
+  tryGet() const
+  {
+    std::unique_lock<mutex_t> guard(lock_, std::defer_lock);
+    if (!guard.try_lock()) {
+      return std::nullopt;
+    }
+    return value_;
+  }
+  /**
+   * @brief access the content (r) with best effort
+   * @return false if the mutex could not be locked
+   * @note only safe way to access pointer type content (r)
+   */
+  bool tryGet(const std::function<void(const T &)> & func)
+  {
+    std::unique_lock<mutex_t> guard(lock_, std::defer_lock);
+    if (!guard.try_lock()) {
+      return false;
+    }
+
+    func(value_);
+    return true;
+  }
+
+  /**
+   * @brief set the content and wait until the mutex could be locked (RealtimeBox behavior)
+   * @return true
+   */
+  [[deprecated("set is not safe for pointer types - use set(std::function...) instead")]]
+  void set(const T & value)
+  {
+    std::lock_guard<mutex_t> guard(lock_);
+    // cppcheck-suppress missingReturn
+    value_ = value;
+  }
+  /**
+   * @brief access the content (rw) and wait until the mutex could locked
+   */
+  void set(const std::function<void(T &)> & func)
+  {
+    std::lock_guard<mutex_t> guard(lock_);
+    func(value_);
+  }
+
+  /**
+   * @brief get the content and wait until the mutex could be locked (RealtimeBox behaviour)
+   * @return copy of the value
+   */
+  [[deprecated(
+    "get is not safe for pointer types - use get(std::function...) instead")]] [[nodiscard]] T
+  get() const
+  {
+    std::lock_guard<mutex_t> guard(lock_);
+    return value_;
+  }
+  /**
+   * @brief get the content and wait until the mutex could be locked
+   * @note same signature as in the existing RealtimeBox<T>
+   */
+  [[deprecated("get is not safe for pointer types - use get(std::function...) instead")]]
+  void get(T & in) const
+  {
+    std::lock_guard<mutex_t> guard(lock_);
+    // cppcheck-suppress missingReturn
+    in = value_;
+  }
+  /**
+   * @brief access the content (r) and wait until the mutex could be locked
+   * @note only safe way to access pointer type content (r)
+   * @note same signature as in the existing RealtimeBox<T>
+   */
+  void get(const std::function<void(const T &)> & func)
+  {
+    std::lock_guard<mutex_t> guard(lock_);
+    func(value_);
+  }
+
+  /**
+   * @brief provide a custom assignment operator for easier usage
+   * @note only to be used from non-RT!
+   */
+  template <typename U = T>
+  typename std::enable_if_t<!is_ptr_or_smart_ptr<U>, void> operator=(const T & value)
+  {
+    set(value);
+  }
+
+  /**
+   * @brief provide a custom conversion operator
+   * @note Can only be used from non-RT!
+   */
+  template <typename U = T, typename = typename std::enable_if_t<!is_ptr_or_smart_ptr<U>>>
+  [[nodiscard]] operator T() const
+  {
+    // Only makes sense with the getNonRT method otherwise we would return an std::optional
+    return get();
+  }
+  /**
+   * @brief provide a custom conversion operator
+   * @note Can be used from non-RT and RT contexts
+   */
+  template <typename U = T, typename = typename std::enable_if_t<!is_ptr_or_smart_ptr<U>>>
+  [[nodiscard]] operator std::optional<T>() const
+  {
+    return tryGet();
+  }
+
+  // In case one wants to actually use a pointer
+  // in this implementation we allow accessing the lock directly.
+  // Note: Be careful with lock.unlock().
+  // It may only be called from the thread that locked the mutex!
+  [[nodiscard]] const mutex_t & getMutex() const { return lock_; }
+  [[nodiscard]] mutex_t & getMutex() { return lock_; }
+
+private:
+  T value_;
+  mutable mutex_t lock_;
+};
+
 }  // namespace realtime_tools
 
 #endif  // REALTIME_TOOLS__REALTIME_BOX_H_

test/realtime_box_tests.cpp

@@ -122,7 +122,7 @@ TEST(RealtimeBox, assignment_operator)
 }
 TEST(RealtimeBox, typecast_operator)
 {
-  RealtimeBox box(DefaultConstructable{.a = 100, .str = ""});
+  RealtimeBox<DefaultConstructable> box(DefaultConstructable{.a = 100, .str = ""});
 
   // Use non RT access
   DefaultConstructable data = box;
@@ -142,7 +142,7 @@ TEST(RealtimeBox, pointer_type)
   int a = 100;
   int * ptr = &a;
 
-  RealtimeBox box(ptr);
+  RealtimeBox<int *> box(ptr);
   // This does not and should not compile!
   // auto value = box.get();
 
@@ -161,7 +161,7 @@ TEST(RealtimeBox, smart_ptr_type)
 {
   std::shared_ptr<int> ptr = std::make_shared<int>(100);
 
-  RealtimeBox box(ptr);
+  RealtimeBox<std::shared_ptr<int>> box(ptr);
   // This does not and should not compile!
   // auto value = box.get();
 
@@ -178,6 +178,26 @@ TEST(RealtimeBox, smart_ptr_type)
   box.tryGet([](const auto & p) { EXPECT_EQ(*p, 10); });
 }
 
+TEST(RealtimeBox, deprecated_note)
+{
+  int a = 100;
+  int * ptr = &a;
+
+  RealtimeBox<int *> box(ptr);
+
+  int * res;
+  box.get(res);
+  EXPECT_EQ(*res, 100);
+
+  EXPECT_EQ(*box.get(), 100);
+
+  std::shared_ptr<int> sptr = std::make_shared<int>(10);
+
+  RealtimeBox<std::shared_ptr<int>> sbox(sptr);
+
+  EXPECT_EQ(*sbox.get(), 10);
+}
+
 // These are the tests from the old RealtimeBox implementation
 // They are therefore suffixed with _existing