Merge pull request #36 from mauropasse/mauro/update-events-executor

update APIs

Author: iRobot ROS

rclcpp/include/rclcpp/executors/events_executor_entities_collector.hpp

@@ -50,7 +50,7 @@ class EventsExecutor;
  * in the entities currently used by the executor.
  */
 class EventsExecutorEntitiesCollector final
-: public EventWaitable,
+  : public EventWaitable,
   public std::enable_shared_from_this<EventsExecutorEntitiesCollector>
 {
 public:
@@ -75,7 +75,7 @@ class EventsExecutorEntitiesCollector final
   // If a entity is removed from a node, we should unset its callback
   RCLCPP_PUBLIC
   void
-  execute() override;
+  execute(std::shared_ptr<void> & data) override;
 
   RCLCPP_PUBLIC
   void
@@ -192,6 +192,14 @@ class EventsExecutorEntitiesCollector final
   void
   add_waitable(rclcpp::Waitable::SharedPtr waitable);
 
+  RCLCPP_PUBLIC
+  std::shared_ptr<void>
+  take_data()
+  {
+    // This waitable doesn't handle any data
+    return nullptr;
+  }
+
 private:
   void
   set_callback_group_entities_callbacks(rclcpp::CallbackGroup::SharedPtr group);

rclcpp/include/rclcpp/executors/events_executor_notify_waitable.hpp

@@ -16,6 +16,7 @@
 #define RCLCPP__EXECUTORS__EVENTS_EXECUTOR_NOTIFY_WAITABLE_HPP_
 
 #include <list>
+#include <memory>
 
 #include "rcl/guard_condition.h"
 #include "rclcpp/executors/event_waitable.hpp"
@@ -45,12 +46,14 @@ class EventsExecutorNotifyWaitable final : public EventWaitable
   // The function is a no-op, since we only care of waking up the executor
   RCLCPP_PUBLIC
   void
-  execute() override
-  {}
+  execute(std::shared_ptr<void> & data) override
+  {
+    (void)data;
+  }
 
   RCLCPP_PUBLIC
   void
-  add_guard_condition(rcl_guard_condition_t * guard_condition)
+  add_guard_condition(const rcl_guard_condition_t * guard_condition)
   {
     notify_guard_conditions_.push_back(guard_condition);
   }
@@ -75,8 +78,16 @@ class EventsExecutorNotifyWaitable final : public EventWaitable
     }
   }
 
+  RCLCPP_PUBLIC
+  std::shared_ptr<void>
+  take_data()
+  {
+    // This waitable doesn't handle any data
+    return nullptr;
+  }
+
 private:
-  std::list<rcl_guard_condition_t *> notify_guard_conditions_;
+  std::list<const rcl_guard_condition_t *> notify_guard_conditions_;
 };
 
 }  // namespace executors

rclcpp/include/rclcpp/executors/timers_manager.hpp

@@ -200,8 +200,8 @@ class TimersManager
       bool any_timer_destroyed = false;
 
       auto it = weak_heap_.begin();
-      auto end = weak_heap_.end();
-      while (it != end) {
+
+      while (it != weak_heap_.end()) {
         if (auto timer_shared_ptr = it->lock()) {
           // This timer is valid, add it to the vector
           locked_heap.push_back(std::move(timer_shared_ptr));

rclcpp/src/rclcpp/executors/events_executor.cpp

@@ -33,13 +33,10 @@ EventsExecutor::EventsExecutor(
   entities_collector_ = std::make_shared<EventsExecutorEntitiesCollector>(this);
   entities_collector_->init();
 
-  // This API uses the wait_set only as a token to identify different executors.
-  auto context_interrupt_gc = options.context->get_interrupt_guard_condition(&wait_set_);
-
   // Setup the executor notifier to wake up the executor when some guard conditions are tiggered.
   // The added guard conditions are guaranteed to not go out of scope before the executor itself.
   executor_notifier_ = std::make_shared<EventsExecutorNotifyWaitable>();
-  executor_notifier_->add_guard_condition(context_interrupt_gc);
+  executor_notifier_->add_guard_condition(&shutdown_guard_condition_->get_rcl_guard_condition());
   executor_notifier_->add_guard_condition(&interrupt_guard_condition_);
   executor_notifier_->set_events_executor_callback(this, &EventsExecutor::push_event);
   entities_collector_->add_waitable(executor_notifier_);
@@ -305,7 +302,8 @@ EventsExecutor::execute_event(const rmw_listener_event_t & event)
         auto waitable = entities_collector_->get_waitable(event.entity);
 
         if (waitable) {
-          waitable->execute();
+          auto data = waitable->take_data();
+          waitable->execute(data);
         }
         break;
       }

rclcpp/src/rclcpp/executors/events_executor_entities_collector.cpp

@@ -12,6 +12,7 @@
 // See the License for the specific language governing permissions and
 // limitations under the License.
 
+#include <memory>
 #include <string>
 #include <utility>
 #include <vector>
@@ -141,8 +142,7 @@ EventsExecutorEntitiesCollector::add_callback_group(
     set_guard_condition_callback(node_ptr->get_notify_guard_condition());
 
     // Store node's notify guard condition
-    rclcpp::node_interfaces::NodeBaseInterface::WeakPtr node_weak_ptr(node_ptr);
-    weak_nodes_to_guard_conditions_[node_weak_ptr] = node_ptr->get_notify_guard_condition();
+    weak_nodes_to_guard_conditions_[node_ptr] = node_ptr->get_notify_guard_condition();
   }
 
   // Add callback group to weak_groups_to_node
@@ -161,8 +161,9 @@ EventsExecutorEntitiesCollector::add_callback_group(
 }
 
 void
-EventsExecutorEntitiesCollector::execute()
+EventsExecutorEntitiesCollector::execute(std::shared_ptr<void> & data)
 {
+  (void)data;
   // This function is called when the associated executor is notified that something changed.
   // We do not know if an entity has been added or removed so we have to rebuild everything.
 

rclcpp/test/rclcpp/CMakeLists.txt

@@ -559,7 +559,7 @@ if(TARGET test_executors)
   target_link_libraries(test_executors ${PROJECT_NAME})
 endif()
 
-ament_add_gtest(test_events_executor rclcpp/executors/test_events_executor.cpp
+ament_add_gtest(test_events_executor executors/test_events_executor.cpp
   APPEND_LIBRARY_DIRS "${append_library_dirs}")
 if(TARGET test_events_executor)
   ament_target_dependencies(test_events_executor
@@ -568,7 +568,7 @@ if(TARGET test_events_executor)
   target_link_libraries(test_events_executor ${PROJECT_NAME} mimick)
 endif()
 
-ament_add_gtest(test_events_executor_entities_collector rclcpp/executors/test_events_executor_entities_collector.cpp
+ament_add_gtest(test_events_executor_entities_collector executors/test_events_executor_entities_collector.cpp
   APPEND_LIBRARY_DIRS "${append_library_dirs}")
 if(TARGET test_events_executor_entities_collector)
   ament_target_dependencies(test_events_executor_entities_collector
@@ -602,7 +602,7 @@ if(TARGET test_static_executor_entities_collector)
   target_link_libraries(test_static_executor_entities_collector ${PROJECT_NAME} mimick)
 endif()
 
-ament_add_gtest(test_timers_manager rclcpp/executors/test_timers_manager.cpp
+ament_add_gtest(test_timers_manager executors/test_timers_manager.cpp
   APPEND_LIBRARY_DIRS "${append_library_dirs}")
 if(TARGET test_timers_manager)
   ament_target_dependencies(test_timers_manager