fix(controllers): resolve parameter validation todos

CHANGELOG.md

@@ -32,6 +32,7 @@ Release Versions:
 - feat(controllers)!: add TF listener interface in BaseControllerInterface (#169)
 - feat(controllers)!: add TF broadcaster in BaseControllerInterface (#170)
 - test(controllers): add TF listener and broadcaster tests (#172)
+- fix(controllers)!: remove input validity setter (#209)
 
 ## 5.4.1
 

source/modulo_controllers/extension_descriptions/modulo_controllers_robot_controller_interface.yaml

@@ -28,7 +28,7 @@ parameters:
     default_value: 0.1
     validation:
       range:
-        minimum: 0
+        exclusive_minimum: 0
   - display_name: Command rate limit
     description: The maximum allowable change in command on any interface expressed in command units / second
     parameter_name: command_rate_limit
@@ -37,7 +37,7 @@ parameters:
     optional: true
     validation:
       range:
-        minimum: 0
+        exclusive_minimum: 0
   - display_name: Task-space frame
     description:
       The frame name in the robot model to use for kinematics calculations (defaults to the last frame in the model)

source/modulo_controllers/include/modulo_controllers/BaseControllerInterface.hpp

@@ -269,12 +269,6 @@ class BaseControllerInterface : public controller_interface::ControllerInterface
   template<typename T>
   void add_output(const std::string& name, const std::string& topic_name = "");
 
-  /**
-   * @brief Set the input validity period of input signals
-   * @param input_validity_period The desired input validity period
-   */
-  void set_input_validity_period(double input_validity_period);
-
   /**
    * @brief Read the most recent message of an input
    * @tparam T The expected type of the input data

source/modulo_controllers/include/modulo_controllers/ControllerInterface.hpp

@@ -166,6 +166,12 @@ class ControllerInterface : public BaseControllerInterface {
   */
   void set_command_interface(const std::string& name, const std::string& interface, double value);
 
+  /**
+   * @copydoc modulo_controllers::BaseControllerInterface::on_validate_parameter_callback()
+   */
+  bool
+  on_validate_parameter_callback(const std::shared_ptr<state_representation::ParameterInterface>& parameter) override;
+
   std::string hardware_name_;///< The hardware name provided by a parameter
 
 private:

source/modulo_controllers/src/BaseControllerInterface.cpp

@@ -162,10 +162,9 @@ BaseControllerInterface::on_set_parameters_callback(const std::vector<rclcpp::Pa
 bool BaseControllerInterface::validate_parameter(const std::shared_ptr<ParameterInterface>& parameter) {
   if (parameter->get_name() == "predicate_publishing_rate" || parameter->get_name() == "input_validity_period") {
     auto value = parameter->get_parameter_value<double>();
-    // FIXME: don't allow zero
-    if (value < 0.0 || value > std::numeric_limits<double>::max()) {
+    if (value <= 0.0 || value > std::numeric_limits<double>::max()) {
       RCLCPP_ERROR(
-          get_node()->get_logger(), "Parameter value of parameter '%s' should be a positive finite number",
+          get_node()->get_logger(), "Parameter value of parameter '%s' should be greater than 0",
           parameter->get_name().c_str());
       return false;
     }
@@ -442,10 +441,6 @@ void BaseControllerInterface::add_outputs() {
   }
 }
 
-void BaseControllerInterface::set_input_validity_period(double input_validity_period) {
-  input_validity_period_ = input_validity_period;
-}
-
 bool BaseControllerInterface::check_input_valid(const std::string& name) const {
   if (inputs_.find(name) == inputs_.end()) {
     RCLCPP_WARN_THROTTLE(

source/modulo_controllers/src/ControllerInterface.cpp

@@ -166,7 +166,6 @@ ControllerInterface::on_activate(const rclcpp_lifecycle::State&) {
   }
 
   auto start_time = get_node()->get_clock()->now();
-  // FIXME: validate activation timeout parameter
   auto activation_timeout = rclcpp::Duration::from_seconds(get_parameter_value<double>("activation_timeout"));
   while (read_state_interfaces() != controller_interface::return_type::OK) {
     RCLCPP_DEBUG_THROTTLE(
@@ -283,4 +282,17 @@ void ControllerInterface::set_command_interface(const std::string& name, const s
   }
 }
 
+bool ControllerInterface::on_validate_parameter_callback(const std::shared_ptr<ParameterInterface>& parameter) {
+  if (parameter->get_name() == "activation_timeout") {
+    auto value = parameter->get_parameter_value<double>();
+    if (value < 0.0 || value > std::numeric_limits<double>::max()) {
+      RCLCPP_ERROR(
+          get_node()->get_logger(), "Parameter value of parameter '%s' should be a positive finite number",
+          parameter->get_name().c_str());
+      return false;
+    }
+  }
+  return true;
+}
+
 }// namespace modulo_controllers

source/modulo_controllers/src/RobotControllerInterface.cpp

@@ -370,12 +370,13 @@ void RobotControllerInterface::set_joint_command(const JointState& joint_command
 
 bool RobotControllerInterface::on_validate_parameter_callback(const std::shared_ptr<ParameterInterface>& parameter) {
   if ((parameter->get_name() == "command_half_life" || parameter->get_name() == "command_rate_limit")
-      && parameter->get_parameter_value<double>() < 0.0) {
+      && parameter->get_parameter_value<double>() <= 0.0) {
     RCLCPP_ERROR(
-        get_node()->get_logger(), "Parameter value of '%s' should be greater than 0", parameter->get_name().c_str());
+        get_node()->get_logger(), "Parameter value of parameter '%s' should be greater than 0",
+        parameter->get_name().c_str());
     return false;
   }
-  return true;
+  return ControllerInterface::on_validate_parameter_callback(parameter);
 }
 
 std::string RobotControllerInterface::get_control_type() const {