Remove some more "empty" implementations.

clalancette · clalancette · commit 0194fb774f69 · 2024-07-31T21:31:24.000Z
In particular, these are implementations in the Waitable
class that only throw exceptions.  Rather than do this,
make these APIs pure virtual so all downstream classes
have to implement them.  All of the current users (except
for tests) do anyway, and it makes the API much cleaner.

Signed-off-by: Chris Lalancette &lt;clalancette@gmail.com&gt;
diff --git a/rclcpp/include/rclcpp/waitable.hpp b/rclcpp/include/rclcpp/waitable.hpp
@@ -176,7 +176,7 @@ class Waitable
   RCLCPP_PUBLIC
   virtual
   std::shared_ptr<void>
-  take_data_by_entity_id(size_t id);
+  take_data_by_entity_id(size_t id) = 0;
 
   /// Execute data that is passed in.
   /**
@@ -246,7 +246,7 @@ class Waitable
   RCLCPP_PUBLIC
   virtual
   void
-  set_on_ready_callback(std::function<void(size_t, int)> callback);
+  set_on_ready_callback(std::function<void(size_t, int)> callback) = 0;
 
   /// Unset any callback registered via set_on_ready_callback.
   /**
@@ -256,7 +256,7 @@ class Waitable
   RCLCPP_PUBLIC
   virtual
   void
-  clear_on_ready_callback();
+  clear_on_ready_callback() = 0;
 
 private:
   std::atomic<bool> in_use_by_wait_set_{false};
diff --git a/rclcpp/src/rclcpp/waitable.cpp b/rclcpp/src/rclcpp/waitable.cpp
@@ -54,35 +54,8 @@ Waitable::get_number_of_ready_guard_conditions()
   return 0u;
 }
 
-std::shared_ptr<void>
-Waitable::take_data_by_entity_id(size_t id)
-{
-  (void)id;
-  throw std::runtime_error(
-          "Custom waitables should override take_data_by_entity_id "
-          "if they want to use it.");
-}
-
 bool
 Waitable::exchange_in_use_by_wait_set_state(bool in_use_state)
 {
   return in_use_by_wait_set_.exchange(in_use_state);
 }
-
-void
-Waitable::set_on_ready_callback(std::function<void(size_t, int)> callback)
-{
-  (void)callback;
-
-  throw std::runtime_error(
-          "Custom waitables should override set_on_ready_callback "
-          "if they want to use it.");
-}
-
-void
-Waitable::clear_on_ready_callback()
-{
-  throw std::runtime_error(
-          "Custom waitables should override clear_on_ready_callback if they "
-          "want to use it and make sure to call it on the waitable destructor.");
-}
diff --git a/rclcpp/test/rclcpp/node_interfaces/test_node_waitables.cpp b/rclcpp/test/rclcpp/node_interfaces/test_node_waitables.cpp
@@ -31,13 +31,13 @@ class TestWaitable : public rclcpp::Waitable
   void add_to_wait_set(rcl_wait_set_t &) override {}
   bool is_ready(const rcl_wait_set_t &) override {return false;}
 
-  std::shared_ptr<void>
-  take_data() override
-  {
-    return nullptr;
-  }
-
+  std::shared_ptr<void> take_data() override {return nullptr;}
   void execute(const std::shared_ptr<void> &) override {}
+
+  void set_on_ready_callback(std::function<void(size_t, int)>) override {}
+  void clear_on_ready_callback() override {}
+
+  std::shared_ptr<void> take_data_by_entity_id(size_t) override {return nullptr;}
 };
 
 class TestNodeWaitables : public ::testing::Test
diff --git a/rclcpp/test/rclcpp/strategies/test_allocator_memory_strategy.cpp b/rclcpp/test/rclcpp/strategies/test_allocator_memory_strategy.cpp
@@ -51,13 +51,13 @@ class TestWaitable : public rclcpp::Waitable
     return test_waitable_result;
   }
 
-  std::shared_ptr<void>
-  take_data() override
-  {
-    return nullptr;
-  }
-
+  std::shared_ptr<void> take_data() override {return nullptr;}
   void execute(const std::shared_ptr<void> &) override {}
+
+  void set_on_ready_callback(std::function<void(size_t, int)>) override {}
+  void clear_on_ready_callback() override {}
+
+  std::shared_ptr<void> take_data_by_entity_id(size_t) override {return nullptr;}
 };
 
 struct RclWaitSetSizes
diff --git a/rclcpp/test/rclcpp/test_memory_strategy.cpp b/rclcpp/test/rclcpp/test_memory_strategy.cpp
@@ -40,6 +40,11 @@ class TestWaitable : public rclcpp::Waitable
 
   std::shared_ptr<void> take_data() override {return nullptr;}
   void execute(const std::shared_ptr<void> &) override {}
+
+  void set_on_ready_callback(std::function<void(size_t, int)>) override {}
+  void clear_on_ready_callback() override {}
+
+  std::shared_ptr<void> take_data_by_entity_id(size_t) override {return nullptr;}
 };
 
 class TestMemoryStrategy : public ::testing::Test
diff --git a/rclcpp/test/rclcpp/wait_set_policies/test_dynamic_storage.cpp b/rclcpp/test/rclcpp/wait_set_policies/test_dynamic_storage.cpp
@@ -52,16 +52,18 @@ class TestWaitable : public rclcpp::Waitable
   : is_ready_(false) {}
 
   void add_to_wait_set(rcl_wait_set_t &) override {}
-
   bool is_ready(const rcl_wait_set_t &) override {return is_ready_;}
 
   std::shared_ptr<void> take_data() override {return nullptr;}
-
-  void
-  execute(const std::shared_ptr<void> &) override {}
+  void execute(const std::shared_ptr<void> &) override {}
 
   void set_is_ready(bool value) {is_ready_ = value;}
 
+  void set_on_ready_callback(std::function<void(size_t, int)>) override {}
+  void clear_on_ready_callback() override {}
+
+  std::shared_ptr<void> take_data_by_entity_id(size_t) override {return nullptr;}
+
 private:
   bool is_ready_;
 };
diff --git a/rclcpp/test/rclcpp/wait_set_policies/test_static_storage.cpp b/rclcpp/test/rclcpp/wait_set_policies/test_static_storage.cpp
@@ -52,16 +52,18 @@ class TestWaitable : public rclcpp::Waitable
   : is_ready_(false) {}
 
   void add_to_wait_set(rcl_wait_set_t &) override {}
-
   bool is_ready(const rcl_wait_set_t &) override {return is_ready_;}
 
   std::shared_ptr<void> take_data() override {return nullptr;}
-
-  void
-  execute(const std::shared_ptr<void> &) override {}
+  void execute(const std::shared_ptr<void> &) override {}
 
   void set_is_ready(bool value) {is_ready_ = value;}
 
+  void set_on_ready_callback(std::function<void(size_t, int)>) override {}
+  void clear_on_ready_callback() override {}
+
+  std::shared_ptr<void> take_data_by_entity_id(size_t) override {return nullptr;}
+
 private:
   bool is_ready_;
 };
diff --git a/rclcpp/test/rclcpp/wait_set_policies/test_storage_policy_common.cpp b/rclcpp/test/rclcpp/wait_set_policies/test_storage_policy_common.cpp
@@ -61,14 +61,17 @@ class TestWaitable : public rclcpp::Waitable
   bool is_ready(const rcl_wait_set_t &) override {return is_ready_;}
 
   std::shared_ptr<void> take_data() override {return nullptr;}
-
-  void
-  execute(const std::shared_ptr<void> & data) override {(void)data;}
+  void execute(const std::shared_ptr<void> &) override {}
 
   void set_is_ready(bool value) {is_ready_ = value;}
 
   void set_add_to_wait_set(bool value) {add_to_wait_set_ = value;}
 
+  void set_on_ready_callback(std::function<void(size_t, int)>) override {}
+  void clear_on_ready_callback() override {}
+
+  std::shared_ptr<void> take_data_by_entity_id(size_t) override {return nullptr;}
+
 private:
   bool is_ready_;
   bool add_to_wait_set_;
diff --git a/rclcpp/test/rclcpp/wait_set_policies/test_thread_safe_synchronization.cpp b/rclcpp/test/rclcpp/wait_set_policies/test_thread_safe_synchronization.cpp
@@ -52,16 +52,18 @@ class TestWaitable : public rclcpp::Waitable
   : is_ready_(false) {}
 
   void add_to_wait_set(rcl_wait_set_t &) override {}
-
   bool is_ready(const rcl_wait_set_t &) override {return is_ready_;}
 
   std::shared_ptr<void> take_data() override {return nullptr;}
-
-  void
-  execute(const std::shared_ptr<void> &) override {}
+  void execute(const std::shared_ptr<void> &) override {}
 
   void set_is_ready(bool value) {is_ready_ = value;}
 
+  void set_on_ready_callback(std::function<void(size_t, int)>) override {}
+  void clear_on_ready_callback() override {}
+
+  std::shared_ptr<void> take_data_by_entity_id(size_t) override {return nullptr;}
+
 private:
   bool is_ready_;
 };
