Remove DEFAULT_PUBLISH_WINDOW

system_metrics_collector/src/system_metrics_collector/linux_cpu_measurement_node.hpp

@@ -56,16 +56,15 @@ class LinuxCpuMeasurementNode : public system_metrics_collector::PeriodicMeasure
    * Construct a LinuxCpuMeasurementNode
    *
    * @param name the name of this node
-   * @param measurement_period the period of this node, used to
-   * read measurements
+   * @param measurement_period the period of this node, used to read measurements
    * @param topic the topic name used for publishing
+   * @param publish_period the period at which metrics are published. 0 ms means don't publish
    */
   LinuxCpuMeasurementNode(
     const std::string & name,
     const std::chrono::milliseconds measurement_period,
     const std::string & topic,
-    const std::chrono::milliseconds publish_period =
-    PeriodicMeasurementNode::DEFAULT_PUBLISH_WINDOW);
+    const std::chrono::milliseconds publish_period);
 
   virtual ~LinuxCpuMeasurementNode() = default;
 

system_metrics_collector/src/system_metrics_collector/linux_memory_measurement_node.hpp

@@ -58,16 +58,15 @@ class LinuxMemoryMeasurementNode : public system_metrics_collector::PeriodicMeas
    * Construct a LinuxMemoryMeasurementNode
    *
    * @param name the name of this node
-   * @param measurement_period the period of this node, used to
-   * read measurements
+   * @param measurement_period the period of this node, used to read measurements
    * @param topic the topic name used for publishing
+   * @param publish_period the period at which metrics are published. 0 ms means don't publish
    */
   LinuxMemoryMeasurementNode(
     const std::string & name,
     const std::chrono::milliseconds measurement_period,
     const std::string & topic,
-    const std::chrono::milliseconds publish_period =
-    PeriodicMeasurementNode::DEFAULT_PUBLISH_WINDOW);
+    const std::chrono::milliseconds publish_period);
 
   virtual ~LinuxMemoryMeasurementNode() = default;
 

system_metrics_collector/src/system_metrics_collector/periodic_measurement_node.cpp

@@ -16,6 +16,7 @@
 #include "periodic_measurement_node.hpp"
 
 #include <chrono>
+#include <stdexcept>
 #include <string>
 
 #include "rclcpp/rclcpp.hpp"
@@ -24,7 +25,7 @@ namespace system_metrics_collector
 {
 
 /* static */ constexpr const std::chrono::milliseconds PeriodicMeasurementNode::
-DEFAULT_PUBLISH_WINDOW;
+INVALID_PUBLISH_WINDOW;
 
 PeriodicMeasurementNode::PeriodicMeasurementNode(
   const std::string & name,
@@ -39,7 +40,11 @@ PeriodicMeasurementNode::PeriodicMeasurementNode(
   publish_timer_(nullptr),
   publish_period_(publish_period),
   clear_measurements_on_publish_(clear_measurements_on_publish)
-{}
+{
+  if (publish_period_ < std::chrono::milliseconds(0)) {
+    throw std::invalid_argument("publish period cannot be negative");
+  }
+}
 
 bool PeriodicMeasurementNode::setupStart()
 {
@@ -54,9 +59,7 @@ bool PeriodicMeasurementNode::setupStart()
     RCLCPP_WARN(this->get_logger(), "setupStart: measurement_timer_ already exists!");
   }
 
-  if (!publish_timer_ &&
-    publish_period_ != PeriodicMeasurementNode::DEFAULT_PUBLISH_WINDOW)
-  {
+  if (!publish_timer_ && publish_period_ != INVALID_PUBLISH_WINDOW) {
     RCLCPP_DEBUG(this->get_logger(), "setupStart: creating publish_timer_");
 
     publish_timer_ = this->create_wall_timer(
@@ -88,7 +91,7 @@ std::string PeriodicMeasurementNode::getStatusString() const
     ", measurement_period=" << std::to_string(measurement_period_.count()) << "ms" <<
     ", publishing_topic=" << publishing_topic_ <<
     ", publish_period=" <<
-  (publish_period_ != PeriodicMeasurementNode::DEFAULT_PUBLISH_WINDOW ?
+  (publish_period_ != INVALID_PUBLISH_WINDOW ?
   std::to_string(publish_period_.count()) + "ms" : "None") <<
     ", clear_measurements_on_publish_=" << clear_measurements_on_publish_ <<
     ", " << Collector::getStatusString();

system_metrics_collector/src/system_metrics_collector/periodic_measurement_node.hpp

@@ -32,22 +32,24 @@ namespace system_metrics_collector
 class PeriodicMeasurementNode : public system_metrics_collector::Collector, public rclcpp::Node
 {
 public:
-  static constexpr const std::chrono::milliseconds DEFAULT_PUBLISH_WINDOW =
+  static constexpr const std::chrono::milliseconds INVALID_PUBLISH_WINDOW =
     std::chrono::milliseconds(0);
+
   /**
    * Construct a PeriodicMeasurementNode.
    *
    * @param name the name of this node
    * @param topic the topic for publishing data
    * @param measurement_period
-   * @param publish_period the window of active measurements. If specified all measurements
-   * will be cleared when the window has been exceeded.
+   * @param publish_period the window of active measurements
+   * @param clear_measurements_on_publish whether all measurements should be cleared
+   *        between subsequent publishing windows
    */
   PeriodicMeasurementNode(
     const std::string & name,
     const std::chrono::milliseconds measurement_period,
     const std::string & topic,  // todo @dbbonnie think about a default topic
-    const std::chrono::milliseconds publish_period = DEFAULT_PUBLISH_WINDOW,
+    const std::chrono::milliseconds publish_period,
     const bool clear_measurements_on_publish = true);
 
   virtual ~PeriodicMeasurementNode() = default;

system_metrics_collector/test/system_metrics_collector/test_linux_cpu_measurement.cpp

@@ -41,8 +41,7 @@ class TestLinuxCpuMeasurementNode : public system_metrics_collector::LinuxCpuMea
     const std::string & name,
     const std::chrono::milliseconds measurement_period,
     const std::string & publishing_topic)
-  : LinuxCpuMeasurementNode(name, measurement_period, publishing_topic,
-      PeriodicMeasurementNode::DEFAULT_PUBLISH_WINDOW)
+  : LinuxCpuMeasurementNode(name, measurement_period, publishing_topic, INVALID_PUBLISH_WINDOW)
   {}
   virtual ~TestLinuxCpuMeasurementNode() = default;
 

system_metrics_collector/test/system_metrics_collector/test_linux_memory_measurement.cpp

@@ -90,8 +90,7 @@ class TestLinuxMemoryMeasurementNode : public system_metrics_collector::LinuxMem
     const std::string & name,
     const std::chrono::milliseconds measurement_period,
     const std::string & publishing_topic)
-  : LinuxMemoryMeasurementNode(name, measurement_period, publishing_topic,
-      PeriodicMeasurementNode::DEFAULT_PUBLISH_WINDOW)
+  : LinuxMemoryMeasurementNode(name, measurement_period, publishing_topic, INVALID_PUBLISH_WINDOW)
   {}
   virtual ~TestLinuxMemoryMeasurementNode() = default;
 

system_metrics_collector/test/system_metrics_collector/test_periodic_measurement_node.cpp

@@ -44,8 +44,7 @@ class TestPeriodicMeasurementNode : public ::system_metrics_collector::PeriodicM
     const std::string & name,
     const std::chrono::milliseconds measurement_period,
     const std::string & publishing_topic,
-    const std::chrono::milliseconds publish_period =
-    PeriodicMeasurementNode::DEFAULT_PUBLISH_WINDOW)
+    const std::chrono::milliseconds publish_period)
   : PeriodicMeasurementNode(name, measurement_period, publishing_topic, publish_period)
   {}
   virtual ~TestPeriodicMeasurementNode() = default;
@@ -75,8 +74,11 @@ class PeriodicMeasurementTestFixure : public ::testing::Test
   {
     rclcpp::init(0, nullptr);
 
-    test_periodic_measurer = std::make_shared<TestPeriodicMeasurementNode>("test_periodic_node",
-        TEST_PERIOD, "test_topic");
+    test_periodic_measurer = std::make_shared<TestPeriodicMeasurementNode>(
+      "test_periodic_node",
+      TEST_PERIOD,
+      "test_topic",
+      ::system_metrics_collector::PeriodicMeasurementNode::INVALID_PUBLISH_WINDOW);
 
     ASSERT_FALSE(test_periodic_measurer->isStarted());