Updated Naive implementation with Dependency Injection

.github/workflows/naive_tests.yaml

@@ -0,0 +1,39 @@
+name: naive_tests
+
+on:
+  workflow_dispatch:
+  pull_request:
+  push:
+
+jobs:
+  naive_tests:
+    runs-on: ubuntu-latest
+    container:
+      image: osrf/ros:humble-desktop
+    defaults:
+      run:
+        shell: bash
+
+    steps:
+    # - name: Setup ROS 2 Environment
+    #   uses: ros-tooling/[email protected]
+    #   with:
+    #     required-ros-distributions: humble
+
+    - name: Checkout repository
+      run: |
+        mkdir -p src
+        cd src
+        git clone --single-branch --branch bgill92/update-naive-example https://github.com/PickNikRobotics/ros_testing_templates.git
+        cd ..
+
+    - name: Build the workspace
+      run: |
+        source /opt/ros/humble/setup.bash
+        colcon build --packages-select naive naive_superclass
+
+    - name: Run tests
+      run: |
+        source /opt/ros/humble/setup.bash
+        colcon test --return-code-on-test-failure --packages-select naive naive_superclass
+        colcon test-result

naive/CMakeLists.txt

@@ -10,7 +10,7 @@ find_package(ament_cmake REQUIRED)
 find_package(rclcpp REQUIRED)
 find_package(std_msgs REQUIRED)
 
-add_executable(naive src/node.cpp src/incrementer.cpp)
+add_executable(naive src/node.cpp src/incrementer.cpp src/ros_middleware.cpp)
 target_include_directories(naive PRIVATE include)
 ament_target_dependencies(naive rclcpp std_msgs)
 
@@ -19,4 +19,11 @@ install(TARGETS
   DESTINATION lib/${PROJECT_NAME}
 )
 
+if(BUILD_TESTING)
+  find_package(ament_cmake_gmock REQUIRED)
+  ament_add_gmock(incrementer_test test/incrementer_test.cpp src/incrementer.cpp src/ros_middleware.cpp TIMEOUT 5)
+  target_include_directories(incrementer_test PRIVATE include)
+  ament_target_dependencies(incrementer_test rclcpp std_msgs)
+endif()
+
 ament_package()

naive/include/naive/incrementer.hpp

@@ -42,18 +42,24 @@
 
 class Incrementer {
  public:
+
+  /**
+   * @brief      An injectable Middleware Object
+   */
+  struct MiddlewareHandle {
+    using Callback = std::function<void(const std_msgs::msg::Int64::SharedPtr)>;
+    virtual void registerCallback(Callback cb) = 0;
+    virtual void publish(std_msgs::msg::Int64 msg) = 0;
+    virtual ~MiddlewareHandle() = default;
+  };
+
   /**
    * @brief      Incrementer constructor
    *
-   * @param[in]  node       Shared pointer to a ROS node
-   * @param[in]  in_topic   ROS topic to the Incrementer subscribes to
-   * @param[in]  out_topic  ROS topic that the Incrementer publishes to
+   * @param[in]  mw       Unique pointer to a MiddlewareHandle object
    */
-  Incrementer(std::shared_ptr<rclcpp::Node> node, std::string const& in_topic,
-              std::string const& out_topic);
+  Incrementer(std::unique_ptr<MiddlewareHandle> mw);
 
  private:
-  std::shared_ptr<rclcpp::Node> node_;
-  std::shared_ptr<rclcpp::Publisher<std_msgs::msg::Int64>> pub_;
-  std::shared_ptr<rclcpp::Subscription<std_msgs::msg::Int64>> sub_;
+  std::unique_ptr<MiddlewareHandle> mw_;
 };

naive/include/naive/ros_middleware.hpp

@@ -0,0 +1,56 @@
+/*********************************************************************
+ * Software License Agreement (BSD License)
+ *
+ *  Copyright (c) 2022, PickNik, LLC.
+ *  All rights reserved.
+ *
+ *  Redistribution and use in source and binary forms, with or without
+ *  modification, are permitted provided that the following conditions
+ *  are met:
+ *
+ *   * Redistributions of source code must retain the above copyright
+ *     notice, this list of conditions and the following disclaimer.
+ *   * Redistributions in binary form must reproduce the above
+ *     copyright notice, this list of conditions and the following
+ *     disclaimer in the documentation and/or other materials provided
+ *     with the distribution.
+ *   * Neither the name of PickNik nor the names of its
+ *     contributors may be used to endorse or promote products derived
+ *     from this software without specific prior written permission.
+ *
+ *  THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
+ *  "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
+ *  LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS
+ *  FOR A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE
+ *  COPYRIGHT OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT,
+ *  INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING,
+ *  BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES;
+ *  LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER
+ *  CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT
+ *  LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN
+ *  ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE
+ *  POSSIBILITY OF SUCH DAMAGE.
+ *********************************************************************/
+#pragma once
+
+#include <string>
+
+#include <rclcpp/rclcpp.hpp>
+#include <naive/incrementer.hpp>
+
+#include <std_msgs/msg/int64.hpp>
+
+class RosMiddleware : public Incrementer::MiddlewareHandle {
+public: 
+  RosMiddleware(const rclcpp::Node::SharedPtr node_handle, std::string in_topic,
+                const std::string& out_topic);
+  void registerCallback(Callback cb) override;
+  void publish(std_msgs::msg::Int64 msg) override;
+
+private:
+  static constexpr int QUEUE_SIZE = 10;
+  rclcpp::Node::SharedPtr node_handle_;
+  std::string in_topic_;
+  rclcpp::Subscription<std_msgs::msg::Int64>::SharedPtr subscriber_;
+  rclcpp::Publisher<std_msgs::msg::Int64>::SharedPtr publisher_;
+};

naive/src/incrementer.cpp

@@ -1,25 +1,18 @@
 #include "naive/incrementer.hpp"
+#include "naive/ros_middleware.hpp"
 
 #include <memory>
 
 #include <rclcpp/rclcpp.hpp>
 
 #include <std_msgs/msg/int64.hpp>
 
-namespace {
-constexpr auto queue_size = 10;
-}
-
-Incrementer::Incrementer(std::shared_ptr<rclcpp::Node> node,
-                         std::string const& in_topic,
-                         std::string const& out_topic)
-    : node_{std::move(node)},
-      pub_{
-          node_->create_publisher<std_msgs::msg::Int64>(out_topic, queue_size)},
-      sub_{node_->create_subscription<std_msgs::msg::Int64>(
-          in_topic, queue_size,
-          [this](std_msgs::msg::Int64::UniquePtr const msg) {
+Incrementer::Incrementer(std::unique_ptr<MiddlewareHandle> mw)
+    : mw_{std::move(mw)}
+      {
+        mw_->registerCallback([this](const std_msgs::msg::Int64::SharedPtr msg) {
             std_msgs::msg::Int64 incremented;
             incremented.data = msg->data + 1;
-            pub_->publish(incremented);
-          })} {}
+            mw_->publish(incremented);
+          });
+      }

naive/src/node.cpp

@@ -1,11 +1,16 @@
 #include "naive/incrementer.hpp"
+#include "naive/ros_middleware.hpp"
 
 #include <rclcpp/rclcpp.hpp>
 
+#include <memory>
+
 int main(int argc, char **argv) {
   rclcpp::init(argc, argv);
   auto const node = std::make_shared<rclcpp::Node>("incrementer");
-  Incrementer incrementer{node, "/in", "/out"};
+  Incrementer incrementer{std::make_unique<RosMiddleware>(node, "/in", "/out")};
   rclcpp::spin(node);
   rclcpp::shutdown();
+
+  return 0;
 }

naive/src/ros_middleware.cpp

@@ -0,0 +1,18 @@
+#include <string>
+
+#include <rclcpp/rclcpp.hpp>
+#include <std_msgs/msg/int64.hpp>
+
+#include <naive/ros_middleware.hpp>
+
+RosMiddleware::RosMiddleware(const rclcpp::Node::SharedPtr node_handle, std::string in_topic,
+                const std::string& out_topic)
+	: node_handle_{node_handle}, 
+	  in_topic_{std::move(in_topic)}, 
+	  publisher_{node_handle_->create_publisher<std_msgs::msg::Int64>(out_topic, QUEUE_SIZE)} {}
+
+void RosMiddleware::registerCallback(Callback cb) {
+	subscriber_ = node_handle_->create_subscription<std_msgs::msg::Int64>(in_topic_, QUEUE_SIZE, cb);
+}
+
+void RosMiddleware::publish(std_msgs::msg::Int64 msg) { publisher_->publish(msg); }

naive/test/incrementer_test.cpp

@@ -0,0 +1,67 @@
+// C++ Standard Library
+#include <memory>
+
+// Gtest
+#include <gmock/gmock.h>
+#include <gtest/gtest.h>
+
+// ROS
+#include <std_msgs//msg/int64.hpp>
+
+// Code being tested
+#include "naive/incrementer.hpp"
+
+using ::testing::_;
+using ::testing::Invoke;
+
+/**
+ \brief Mock middleware handle, allowing us to define test behavior.
+ */
+struct MockMiddleware : public Incrementer::MiddlewareHandle {
+  MOCK_METHOD1(registerCallback, void(Callback cb));
+  MOCK_METHOD1(publish, void(std_msgs::msg::Int64 msg));
+};
+
+/** \brief Callback is given to subscriber on construction. */
+TEST(IncrementerTests, RegisterSubscriber) {
+  // GIVEN a middleware handle
+  auto mw = std::make_unique<MockMiddleware>();
+  // THEN a callback should be registered for topic subscription.
+  EXPECT_CALL(*mw, registerCallback(_)).Times(1);
+  // WHEN the incrementer is created with that handle
+  Incrementer incrementer{std::move(mw)};
+}
+
+/** \brief Incremented number is published when a message is received. */
+TEST(IncrementerTests, PublishIncrement) {
+  // GIVEN a middleware handle
+  auto mw = std::make_unique<MockMiddleware>();
+  /*
+  Configure the mock handle to capture the incrementer callback so we can
+  publish a message to it as the test trigger event. This is analogous to ROS
+  publishing a message to a topic if the ROS middleware handle was used.
+  We know that registerCallback will be called from a previous test so we
+  do not need to test that again, only define the behavior.
+  */
+  Incrementer::MiddlewareHandle::Callback callback;
+  ON_CALL(*mw, registerCallback(_))
+      .WillByDefault(Invoke(
+          [&](Incrementer::MiddlewareHandle::Callback cb) { callback = cb; }));
+
+  // THEN the incrementer should publish an incremented message.
+  std_msgs::msg::Int64 expected;
+  expected.data = 8;
+  EXPECT_CALL(*mw, publish(expected)).Times(1);
+  /* Create the System Under Test (SUT)*/
+  Incrementer incrementer{std::move(mw)};
+
+  // WHEN the middleware publishes a message to the callback
+  std_msgs::msg::Int64::SharedPtr msg = std::make_shared<std_msgs::msg::Int64>();
+  msg->data = 7;
+  callback(msg);
+}
+
+int main(int argc, char **argv) {
+  ::testing::InitGoogleTest(&argc, argv);
+  return RUN_ALL_TESTS();
+}

naive_superclass/CMakeLists.txt

@@ -0,0 +1,29 @@
+cmake_minimum_required(VERSION 3.16)
+project(naive_superclass)
+
+if(CMAKE_COMPILER_IS_GNUCXX OR CMAKE_CXX_COMPILER_ID MATCHES "Clang")
+  add_compile_options(-Wall -Wextra -Wpedantic -Werror)
+endif()
+
+# find dependencies
+find_package(ament_cmake REQUIRED)
+find_package(rclcpp REQUIRED)
+find_package(std_msgs REQUIRED)
+
+add_executable(naive_superclass src/node.cpp src/incrementer.cpp)
+target_include_directories(naive_superclass PRIVATE include)
+ament_target_dependencies(naive_superclass rclcpp std_msgs)
+
+install(TARGETS
+  naive_superclass
+  DESTINATION lib/${PROJECT_NAME}
+)
+
+if(BUILD_TESTING)
+  find_package(ament_cmake_gtest REQUIRED)
+  ament_add_gtest(incrementer_test test/incrementer_test.cpp src/incrementer.cpp TIMEOUT 5)
+  target_include_directories(incrementer_test PRIVATE include)
+  ament_target_dependencies(incrementer_test rclcpp std_msgs)
+endif()
+
+ament_package()

naive_superclass/include/naive_superclass/incrementer.hpp

@@ -0,0 +1,60 @@
+/*********************************************************************
+ * Software License Agreement (BSD License)
+ *
+ *  Copyright (c) 2022, PickNik, LLC.
+ *  All rights reserved.
+ *
+ *  Redistribution and use in source and binary forms, with or without
+ *  modification, are permitted provided that the following conditions
+ *  are met:
+ *
+ *   * Redistributions of source code must retain the above copyright
+ *     notice, this list of conditions and the following disclaimer.
+ *   * Redistributions in binary form must reproduce the above
+ *     copyright notice, this list of conditions and the following
+ *     disclaimer in the documentation and/or other materials provided
+ *     with the distribution.
+ *   * Neither the name of PickNik nor the names of its
+ *     contributors may be used to endorse or promote products derived
+ *     from this software without specific prior written permission.
+ *
+ *  THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
+ *  "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
+ *  LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS
+ *  FOR A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE
+ *  COPYRIGHT OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT,
+ *  INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING,
+ *  BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES;
+ *  LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER
+ *  CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT
+ *  LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN
+ *  ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE
+ *  POSSIBILITY OF SUCH DAMAGE.
+ *********************************************************************/
+#pragma once
+
+#include <memory>
+#include <string>
+
+#include <rclcpp/rclcpp.hpp>
+
+#include <std_msgs/msg/int64.hpp>
+
+class Incrementer : public rclcpp::Node {
+ public:
+  /**
+   * @brief      Incrementer constructor
+   *
+   * @param[in]  mw       Unique pointer to a MiddlewareHandle object
+   */
+  Incrementer(const std::string node_name, const std::string in_topic, const std::string out_topic);
+
+  void callback(const std_msgs::msg::Int64::SharedPtr msg);
+
+  int getQueueSize(){return QUEUE_SIZE;}
+
+ private:
+  static constexpr int QUEUE_SIZE = 10;
+  rclcpp::Publisher<std_msgs::msg::Int64>::SharedPtr publisher_;
+  rclcpp::Subscription<std_msgs::msg::Int64>::SharedPtr subscriber_;
+};