Refactor ComponentInterface to use node interfaces (#33)

* Move ComponentInterface constructor to source file

Remove template class NodeT

Change ComponentInterface constructor arguments to get all the node interfaces

* Pass the publisher type as argument to create_output

* Fix logging with logging interface

* Fix parameters with parameters interface

* Update creation of subscriptions, services and TF helpers

* Update ComponentInterface tests

* Rebase fixes

* 2.2.10 -> 2.2.11

* Update CHANGELOG

CHANGELOG.md

@@ -8,6 +8,7 @@ Release Versions:
 
 ## Upcoming changes (in development)
 
+- Refactor ComponentInterface to use node interfaces (#33)
 - Refactor component predicates with a single Predicate publisher (#26)
 - Add and install component descriptions (#31)
 - Apply AICA C++ style guide (#30)

VERSION

@@ -1 +1 @@
-2.2.10
+2.2.11

doxygen/doxygen.conf

@@ -38,7 +38,7 @@ PROJECT_NAME           = "Modulo"
 # could be handy for archiving the generated documentation or if some version
 # control system is used.
 
-PROJECT_NUMBER = 2.2.10
+PROJECT_NUMBER = 2.2.11
 
 # Using the PROJECT_BRIEF tag one can provide an optional one line description
 # for a project that appears at the top of each page and should give viewer a

source/modulo_component_interfaces/package.xml

@@ -2,7 +2,7 @@
 <?xml-model href="http://download.ros.org/schema/package_format3.xsd" schematypens="http://www.w3.org/2001/XMLSchema"?>
 <package format="3">
   <name>modulo_component_interfaces</name>
-  <version>2.2.10</version>
+  <version>2.2.11</version>
   <description>Interface package for communicating with modulo components through the ROS framework</description>
   <maintainer email="[email protected]">Enrico Eberhard</maintainer>
   <license>GPLv3</license>

source/modulo_components/CMakeLists.txt

@@ -24,6 +24,7 @@ ament_auto_find_build_dependencies()
 include_directories(include)
 
 ament_auto_add_library(${PROJECT_NAME} SHARED
+    ${PROJECT_SOURCE_DIR}/src/ComponentInterface.cpp
     ${PROJECT_SOURCE_DIR}/src/Component.cpp
     ${PROJECT_SOURCE_DIR}/src/LifecycleComponent.cpp)
 

source/modulo_components/package.xml

@@ -2,7 +2,7 @@
 <?xml-model href="http://download.ros.org/schema/package_format3.xsd" schematypens="http://www.w3.org/2001/XMLSchema"?>
 <package format="3">
   <name>modulo_components</name>
-  <version>2.2.10</version>
+  <version>2.2.11</version>
   <description>Modulo base classes that wrap ROS2 Nodes as modular components for the AICA application framework</description>
   <maintainer email="[email protected]">Baptiste Busch</maintainer>
   <maintainer email="[email protected]">Enrico Eberhard</maintainer>

source/modulo_components/src/ComponentInterface.cpp

@@ -0,0 +1,33 @@
+#include "modulo_components/ComponentInterface.hpp"
+
+using namespace modulo_core::communication;
+
+namespace modulo_components {
+
+ComponentInterface::ComponentInterface(
+    const std::shared_ptr<rclcpp::node_interfaces::NodeInterfaces<ALL_RCLCPP_NODE_INTERFACES>>& interfaces
+) :
+    node_base_(interfaces->get_node_base_interface()),
+    node_clock_(interfaces->get_node_clock_interface()),
+    node_logging_(interfaces->get_node_logging_interface()),
+    node_parameters_(interfaces->get_node_parameters_interface()),
+    node_services_(interfaces->get_node_services_interface()),
+    node_timers_(interfaces->get_node_timers_interface()),
+    node_topics_(interfaces->get_node_topics_interface()) {
+  this->cb_group_ = node_base_->create_callback_group(rclcpp::CallbackGroupType::Reentrant);
+  // register the parameter change callback handler
+  this->parameter_cb_handle_ = this->node_parameters_->add_on_set_parameters_callback(
+      [this](const std::vector<rclcpp::Parameter>& parameters) -> rcl_interfaces::msg::SetParametersResult {
+        return this->on_set_parameters_callback(parameters);
+      });
+  this->add_parameter("period", 0.1, "The time interval in seconds for all periodic callbacks", true);
+  this->predicate_publisher_ =
+      rclcpp::create_publisher<modulo_component_interfaces::msg::Predicate>(*this, "/predicates", this->qos_);
+
+  this->add_predicate("in_error_state", false);
+
+  this->step_timer_ = rclcpp::create_wall_timer(
+      std::chrono::nanoseconds(static_cast<int64_t>(this->get_parameter_value<double>("period") * 1e9)),
+      [this] { this->step(); }, this->cb_group_, this->node_base_.get(), this->node_timers_.get());
+}
+}// namespace modulo_components

source/modulo_components/test/cpp/include/test_modulo_components/component_public_interfaces.hpp

@@ -8,65 +8,72 @@ using namespace state_representation;
 
 namespace modulo_components {
 
-template<class NodeT>
-class ComponentInterfacePublicInterface : public ComponentInterface<NodeT> {
+class ComponentInterfacePublicInterface : public ComponentInterface {
 public:
-  explicit ComponentInterfacePublicInterface(
-      const rclcpp::NodeOptions& node_options, modulo_core::communication::PublisherType publisher_type,
-      const std::string& fallback_name = "ComponentInterfacePublicInterface"
-  ) : ComponentInterface<NodeT>(node_options, publisher_type, fallback_name) {}
-  using ComponentInterface<NodeT>::add_parameter;
-  using ComponentInterface<NodeT>::get_parameter;
-  using ComponentInterface<NodeT>::get_parameter_value;
-  using ComponentInterface<NodeT>::set_parameter_value;
-  using ComponentInterface<NodeT>::parameter_map_;
-  using ComponentInterface<NodeT>::add_predicate;
-  using ComponentInterface<NodeT>::get_predicate;
-  using ComponentInterface<NodeT>::set_predicate;
-  using ComponentInterface<NodeT>::predicates_;
-  using ComponentInterface<NodeT>::add_trigger;
-  using ComponentInterface<NodeT>::trigger;
-  using ComponentInterface<NodeT>::triggers_;
-  using ComponentInterface<NodeT>::declare_input;
-  using ComponentInterface<NodeT>::declare_output;
-  using ComponentInterface<NodeT>::remove_input;
-  using ComponentInterface<NodeT>::add_input;
-  using ComponentInterface<NodeT>::add_service;
-  using ComponentInterface<NodeT>::inputs_;
-  using ComponentInterface<NodeT>::create_output;
-  using ComponentInterface<NodeT>::outputs_;
-  using ComponentInterface<NodeT>::periodic_outputs_;
-  using ComponentInterface<NodeT>::empty_services_;
-  using ComponentInterface<NodeT>::string_services_;
-  using ComponentInterface<NodeT>::add_tf_broadcaster;
-  using ComponentInterface<NodeT>::add_static_tf_broadcaster;
-  using ComponentInterface<NodeT>::add_tf_listener;
-  using ComponentInterface<NodeT>::publish_output;
-  using ComponentInterface<NodeT>::send_transform;
-  using ComponentInterface<NodeT>::send_transforms;
-  using ComponentInterface<NodeT>::send_static_transform;
-  using ComponentInterface<NodeT>::send_static_transforms;
-  using ComponentInterface<NodeT>::lookup_transform;
-  using ComponentInterface<NodeT>::raise_error;
-  using ComponentInterface<NodeT>::get_qos;
-  using ComponentInterface<NodeT>::set_qos;
-  using ComponentInterface<NodeT>::get_clock;
+  template<typename NodeT>
+  explicit ComponentInterfacePublicInterface(const std::shared_ptr<NodeT>& node) : ComponentInterface(
+      std::make_shared<rclcpp::node_interfaces::NodeInterfaces<ALL_RCLCPP_NODE_INTERFACES>>(
+          node->get_node_base_interface(), node->get_node_clock_interface(), node->get_node_graph_interface(),
+          node->get_node_logging_interface(), node->get_node_parameters_interface(),
+          node->get_node_services_interface(), node->get_node_time_source_interface(),
+          node->get_node_timers_interface(), node->get_node_topics_interface(),
+          node->get_node_waitables_interface())) {}
+  using ComponentInterface::node_base_;
+  using ComponentInterface::node_clock_;
+  using ComponentInterface::node_logging_;
+  using ComponentInterface::node_parameters_;
+  using ComponentInterface::add_parameter;
+  using ComponentInterface::get_parameter;
+  using ComponentInterface::get_parameter_value;
+  using ComponentInterface::set_parameter_value;
+  using ComponentInterface::parameter_map_;
+  using ComponentInterface::add_predicate;
+  using ComponentInterface::get_predicate;
+  using ComponentInterface::set_predicate;
+  using ComponentInterface::predicates_;
+  using ComponentInterface::add_trigger;
+  using ComponentInterface::trigger;
+  using ComponentInterface::triggers_;
+  using ComponentInterface::declare_input;
+  using ComponentInterface::declare_output;
+  using ComponentInterface::remove_input;
+  using ComponentInterface::add_input;
+  using ComponentInterface::add_service;
+  using ComponentInterface::inputs_;
+  using ComponentInterface::create_output;
+  using ComponentInterface::outputs_;
+  using ComponentInterface::periodic_outputs_;
+  using ComponentInterface::empty_services_;
+  using ComponentInterface::string_services_;
+  using ComponentInterface::add_tf_broadcaster;
+  using ComponentInterface::add_static_tf_broadcaster;
+  using ComponentInterface::add_tf_listener;
+  using ComponentInterface::publish_output;
+  using ComponentInterface::send_transform;
+  using ComponentInterface::send_transforms;
+  using ComponentInterface::send_static_transform;
+  using ComponentInterface::send_static_transforms;
+  using ComponentInterface::lookup_transform;
+  using ComponentInterface::raise_error;
+  using ComponentInterface::get_qos;
+  using ComponentInterface::set_qos;
 
   bool on_validate_parameter_callback(const std::shared_ptr<state_representation::ParameterInterface>&) override {
     validate_parameter_was_called = true;
     return validate_parameter_return_value;
   }
 
   rclcpp::Parameter get_ros_parameter(const std::string& name) {
-    return NodeT::get_parameter(name);
+    return this->node_parameters_->get_parameter(name);
   }
 
   rcl_interfaces::msg::SetParametersResult set_ros_parameter(const rclcpp::Parameter& parameter) {
-    return NodeT::set_parameter(parameter);
+    return this->node_parameters_->set_parameters({parameter}).at(
+        0);
   }
 
   std::string get_parameter_description(const std::string& name) {
-    return NodeT::describe_parameter(name).description;
+    return this->node_parameters_->describe_parameters({name}).at(0).description;
   }
 
   bool validate_parameter_was_called = false;

source/modulo_components/test/cpp/test_component_interface.cpp

@@ -24,18 +24,20 @@ class ComponentInterfaceTest : public ::testing::Test {
 
   void SetUp() override {
     exec_ = std::make_shared<rclcpp::executors::SingleThreadedExecutor>();
+    this->node_ = std::make_shared<NodeT>("ComponentInterfacePublicInterface", rclcpp::NodeOptions());
     if (std::is_same<NodeT, rclcpp::Node>::value) {
-      this->component_ = std::make_shared<ComponentInterfacePublicInterface<NodeT>>(
-          rclcpp::NodeOptions(), modulo_core::communication::PublisherType::PUBLISHER);
+      this->pub_type_ = modulo_core::communication::PublisherType::PUBLISHER;
+      this->component_ = std::make_shared<ComponentInterfacePublicInterface>(this->node_);
     } else if (std::is_same<NodeT, rclcpp_lifecycle::LifecycleNode>::value) {
-      this->component_ = std::make_shared<ComponentInterfacePublicInterface<NodeT>>(
-          rclcpp::NodeOptions(), modulo_core::communication::PublisherType::LIFECYCLE_PUBLISHER);
+      this->pub_type_ = modulo_core::communication::PublisherType::LIFECYCLE_PUBLISHER;
+      this->component_ = std::make_shared<ComponentInterfacePublicInterface>(this->node_);
     }
   }
 
   std::shared_ptr<rclcpp::executors::SingleThreadedExecutor> exec_;
-  std::shared_ptr<ComponentInterfacePublicInterface<NodeT>> component_;
+  std::shared_ptr<ComponentInterfacePublicInterface> component_;
   std::shared_ptr<NodeT> node_;
+  modulo_core::communication::PublisherType pub_type_;
 };
 
 using NodeTypes = ::testing::Types<rclcpp::Node, rclcpp_lifecycle::LifecycleNode>;
@@ -126,7 +128,8 @@ TYPED_TEST(ComponentInterfaceTest, AddInputWithUserCallback) {
       this->component_->inputs_.at("state")->template get_handler<modulo_core::EncodedState>()->get_callback();
   state_representation::CartesianState new_state("B");
   auto message = std::make_shared<modulo_core::EncodedState>();
-  modulo_core::translators::write_message(*message, new_state, this->component_->get_clock()->now());
+  modulo_core::translators::write_message(
+      *message, new_state, this->component_->node_clock_->get_clock()->now());
   EXPECT_STREQ(state_data->get_name().c_str(), "A");
   EXPECT_NO_THROW(callback(message));
   EXPECT_TRUE(callback_triggered);
@@ -188,8 +191,8 @@ TYPED_TEST(ComponentInterfaceTest, CallEmptyService) {
   EXPECT_NO_THROW(this->component_->add_service("empty", empty_callback));
 
   auto client = std::make_shared<ServiceClient<modulo_component_interfaces::srv::EmptyTrigger>>(
-      rclcpp::NodeOptions(), "/" + std::string(this->component_->get_name()) + "/empty");
-  this->exec_->add_node(this->component_->get_node_base_interface());
+      rclcpp::NodeOptions(), "/" + std::string(this->component_->node_base_->get_name()) + "/empty");
+  this->exec_->add_node(this->component_->node_base_);
   this->exec_->add_node(client);
   auto request = std::make_shared<modulo_component_interfaces::srv::EmptyTrigger::Request>();
   auto future = client->call_async(request);
@@ -208,8 +211,8 @@ TYPED_TEST(ComponentInterfaceTest, CallStringService) {
   EXPECT_NO_THROW(this->component_->add_service("string", string_callback));
 
   auto client = std::make_shared<ServiceClient<modulo_component_interfaces::srv::StringTrigger>>(
-      rclcpp::NodeOptions(), "/" + std::string(this->component_->get_name()) + "/string");
-  this->exec_->add_node(this->component_->get_node_base_interface());
+      rclcpp::NodeOptions(), "/" + std::string(this->component_->node_base_->get_name()) + "/string");
+  this->exec_->add_node(this->component_->node_base_);
   this->exec_->add_node(client);
   auto request = std::make_shared<modulo_component_interfaces::srv::StringTrigger::Request>();
   request->payload = "payload";
@@ -223,7 +226,7 @@ TYPED_TEST(ComponentInterfaceTest, CallStringService) {
 
 TYPED_TEST(ComponentInterfaceTest, CreateOutput) {
   auto data = std::make_shared<bool>(true);
-  EXPECT_NO_THROW(this->component_->create_output("test", data, "/topic", true, true));
+  EXPECT_NO_THROW(this->component_->create_output(this->pub_type_, "test", data, "/topic", true, true));
   EXPECT_FALSE(this->component_->outputs_.find("test") == this->component_->outputs_.end());
   EXPECT_EQ(this->component_->template get_parameter_value<std::string>("test_topic"), "/topic");
   EXPECT_TRUE(this->component_->periodic_outputs_.at("test"));
@@ -237,7 +240,7 @@ TYPED_TEST(ComponentInterfaceTest, CreateOutput) {
   EXPECT_EQ(pub_interface->get_message_pair()->get_type(), modulo_core::communication::MessageType::BOOL);
   EXPECT_THROW(pub_interface->publish(), modulo_core::exceptions::CoreException);
 
-  EXPECT_NO_THROW(this->component_->create_output("_tEsT_#1@3", data, "", true, false));
+  EXPECT_NO_THROW(this->component_->create_output(this->pub_type_, "_tEsT_#1@3", data, "", true, false));
   EXPECT_FALSE(this->component_->periodic_outputs_.at("test_13"));
   EXPECT_NO_THROW(this->component_->publish_output("_tEsT_#1@3"));
   EXPECT_NO_THROW(this->component_->publish_output("test_13"));