Add input argument checking to PeriodicMeasurementNode

system_metrics_collector/src/system_metrics_collector/periodic_measurement_node.cpp

@@ -38,8 +38,23 @@ PeriodicMeasurementNode::PeriodicMeasurementNode(
   publish_timer_(nullptr),
   publish_period_(publish_period)
 {
-  if (publish_period_ <= std::chrono::milliseconds{0}) {
-    throw std::invalid_argument("publish period cannot be negative");
+  // rclcpp::Node throws if name is empty
+
+  if (measurement_period <= std::chrono::milliseconds{0}) {
+    throw std::invalid_argument{"measurement_period cannot be negative"};
+  }
+
+  if (publishing_topic.empty()) {
+    throw std::invalid_argument{"publishing_topic cannot be empty"};
+  }
+
+  if (publish_period <= std::chrono::milliseconds{0}) {
+    throw std::invalid_argument{"publish_period cannot be negative"};
+  }
+
+  if (publish_period <= measurement_period) {
+    throw std::invalid_argument{
+            "publish_period cannot be less than or equal to the measurement_period"};
   }
 }
 

system_metrics_collector/src/system_metrics_collector/periodic_measurement_node.hpp

@@ -39,12 +39,12 @@ class PeriodicMeasurementNode : public system_metrics_collector::Collector,
   /**
    * 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
-   * @param clear_measurements_on_publish whether all measurements should be cleared
-   *        between subsequent publishing windows
+   * @param name the name of this node. This must be non-empty.
+   * @param topic the topic for publishing data. This must be non-empty.
+   * @param measurement_period. This must be non-negative and strictly less than publish_period.
+   * @param publish_period the window of active measurements. This must be non-negative and
+   * strictly greater than measurement_period.
+   * @throws std::invalid_argument for any invalid input
    */
   PeriodicMeasurementNode(
     const std::string & name,

system_metrics_collector/test/system_metrics_collector/test_periodic_measurement_node.cpp

@@ -18,9 +18,10 @@
 #include <atomic>
 #include <chrono>
 #include <iostream>
-#include <string>
 #include <memory>
 #include <mutex>
+#include <stdexcept>
+#include <string>
 
 #include "../../src/system_metrics_collector/collector.hpp"
 #include "../../src/system_metrics_collector/periodic_measurement_node.hpp"
@@ -169,6 +170,37 @@ TEST_F(PeriodicMeasurementTestFixure, TestStartAndStop) {
     test_constants::kTestDuration.count() / kDontPublishDuringTest.count(), times_published);
 }
 
+TEST_F(PeriodicMeasurementTestFixure, TestConstructorInputValidation) {
+  ASSERT_THROW(TestPeriodicMeasurementNode("throw",
+    std::chrono::milliseconds{-1},
+    "",
+    kDontPublishDuringTest), std::invalid_argument);
+
+  // bad measurement period
+  ASSERT_THROW(TestPeriodicMeasurementNode("throw",
+    std::chrono::milliseconds{-1},
+    "",
+    test_constants::kPublishPeriod), std::invalid_argument);
+
+  // bad node name
+  ASSERT_THROW(TestPeriodicMeasurementNode("throw",
+    test_constants::kMeasurePeriod,
+    kTestTopic,
+    std::chrono::milliseconds{-1}), std::invalid_argument);
+
+  // invalid publish period
+  ASSERT_THROW(TestPeriodicMeasurementNode("throw",
+    std::chrono::milliseconds{2},
+    kTestTopic,
+    std::chrono::milliseconds{1}), std::invalid_argument);
+
+  // invalid node name
+  ASSERT_THROW(TestPeriodicMeasurementNode("",
+    std::chrono::milliseconds{2},
+    kTestTopic,
+    std::chrono::milliseconds{1}), std::invalid_argument);
+}
+
 int main(int argc, char ** argv)
 {
   ::testing::InitGoogleTest(&argc, argv);