From 391c87ee9639e7589cffc1004c35d529135aa7ab Mon Sep 17 00:00:00 2001 From: Dominic Reber <71256590+domire8@users.noreply.github.com> Date: Thu, 15 May 2025 21:33:16 +0200 Subject: [PATCH 1/3] fix(controllers)!: remove input validity setter (#209) --- CHANGELOG.md | 7 +++++++ .../modulo_controllers/BaseControllerInterface.hpp | 6 ------ .../modulo_controllers/ControllerInterface.hpp | 6 ++++++ .../src/BaseControllerInterface.cpp | 4 ---- .../modulo_controllers/src/ControllerInterface.cpp | 13 +++++++++++++ .../src/RobotControllerInterface.cpp | 2 +- 6 files changed, 27 insertions(+), 11 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index acd0adee..df24c1cc 100644 --- a/CHANGELOG.md +++ b/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 @@ -102,6 +103,12 @@ base class. - feat(components): verify return value of callbacks (#206) - fix(controllers): move input validity period to base (#207) +- feat(controllers)!: remove robot description parameter (#186) +- 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) +- feat(controllers): use parent node for tf listener (#190) +- feat(core): update translators to include new trajectory msgs (#194) ## 5.2.0 diff --git a/source/modulo_controllers/include/modulo_controllers/BaseControllerInterface.hpp b/source/modulo_controllers/include/modulo_controllers/BaseControllerInterface.hpp index a4581df7..b4560dc5 100644 --- a/source/modulo_controllers/include/modulo_controllers/BaseControllerInterface.hpp +++ b/source/modulo_controllers/include/modulo_controllers/BaseControllerInterface.hpp @@ -269,12 +269,6 @@ class BaseControllerInterface : public controller_interface::ControllerInterface template 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 diff --git a/source/modulo_controllers/include/modulo_controllers/ControllerInterface.hpp b/source/modulo_controllers/include/modulo_controllers/ControllerInterface.hpp index 83430c70..aae118ff 100644 --- a/source/modulo_controllers/include/modulo_controllers/ControllerInterface.hpp +++ b/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& parameter) override; + std::string hardware_name_;///< The hardware name provided by a parameter private: diff --git a/source/modulo_controllers/src/BaseControllerInterface.cpp b/source/modulo_controllers/src/BaseControllerInterface.cpp index 830805f8..82214fab 100644 --- a/source/modulo_controllers/src/BaseControllerInterface.cpp +++ b/source/modulo_controllers/src/BaseControllerInterface.cpp @@ -442,10 +442,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( diff --git a/source/modulo_controllers/src/ControllerInterface.cpp b/source/modulo_controllers/src/ControllerInterface.cpp index e88b1ca9..7737919c 100644 --- a/source/modulo_controllers/src/ControllerInterface.cpp +++ b/source/modulo_controllers/src/ControllerInterface.cpp @@ -283,4 +283,17 @@ void ControllerInterface::set_command_interface(const std::string& name, const s } } +bool ControllerInterface::on_validate_parameter_callback(const std::shared_ptr& parameter) { + if (parameter->get_name() == "activation_timeout") { + auto value = parameter->get_parameter_value(); + if (value < 0.0 || value > std::numeric_limits::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 diff --git a/source/modulo_controllers/src/RobotControllerInterface.cpp b/source/modulo_controllers/src/RobotControllerInterface.cpp index 06de4980..7279dabe 100644 --- a/source/modulo_controllers/src/RobotControllerInterface.cpp +++ b/source/modulo_controllers/src/RobotControllerInterface.cpp @@ -375,7 +375,7 @@ bool RobotControllerInterface::on_validate_parameter_callback(const std::shared_ get_node()->get_logger(), "Parameter value of '%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 { From 5e5fa485b85fde6b68c77ede12f74f85890aad85 Mon Sep 17 00:00:00 2001 From: Dominic Reber Date: Tue, 14 Apr 2026 12:16:50 +0200 Subject: [PATCH 2/3] fix: parameter validation --- .../modulo_controllers_robot_controller_interface.yaml | 4 ++-- source/modulo_controllers/src/BaseControllerInterface.cpp | 5 ++--- source/modulo_controllers/src/ControllerInterface.cpp | 1 - source/modulo_controllers/src/RobotControllerInterface.cpp | 5 +++-- 4 files changed, 7 insertions(+), 8 deletions(-) diff --git a/source/modulo_controllers/extension_descriptions/modulo_controllers_robot_controller_interface.yaml b/source/modulo_controllers/extension_descriptions/modulo_controllers_robot_controller_interface.yaml index c8ef0281..109ac7e3 100644 --- a/source/modulo_controllers/extension_descriptions/modulo_controllers_robot_controller_interface.yaml +++ b/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) diff --git a/source/modulo_controllers/src/BaseControllerInterface.cpp b/source/modulo_controllers/src/BaseControllerInterface.cpp index 82214fab..711d5b53 100644 --- a/source/modulo_controllers/src/BaseControllerInterface.cpp +++ b/source/modulo_controllers/src/BaseControllerInterface.cpp @@ -162,10 +162,9 @@ BaseControllerInterface::on_set_parameters_callback(const std::vector& parameter) { if (parameter->get_name() == "predicate_publishing_rate" || parameter->get_name() == "input_validity_period") { auto value = parameter->get_parameter_value(); - // FIXME: don't allow zero - if (value < 0.0 || value > std::numeric_limits::max()) { + if (value <= 0.0 || value > std::numeric_limits::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; } diff --git a/source/modulo_controllers/src/ControllerInterface.cpp b/source/modulo_controllers/src/ControllerInterface.cpp index 7737919c..7bb4d419 100644 --- a/source/modulo_controllers/src/ControllerInterface.cpp +++ b/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("activation_timeout")); while (read_state_interfaces() != controller_interface::return_type::OK) { RCLCPP_DEBUG_THROTTLE( diff --git a/source/modulo_controllers/src/RobotControllerInterface.cpp b/source/modulo_controllers/src/RobotControllerInterface.cpp index 7279dabe..1c065892 100644 --- a/source/modulo_controllers/src/RobotControllerInterface.cpp +++ b/source/modulo_controllers/src/RobotControllerInterface.cpp @@ -370,9 +370,10 @@ void RobotControllerInterface::set_joint_command(const JointState& joint_command bool RobotControllerInterface::on_validate_parameter_callback(const std::shared_ptr& parameter) { if ((parameter->get_name() == "command_half_life" || parameter->get_name() == "command_rate_limit") - && parameter->get_parameter_value() < 0.0) { + && parameter->get_parameter_value() <= 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 ControllerInterface::on_validate_parameter_callback(parameter); From 4d40342a2a940014f90904dfc8279c8dc0f9ea6a Mon Sep 17 00:00:00 2001 From: Dominic Reber <71256590+domire8@users.noreply.github.com> Date: Tue, 14 Apr 2026 13:25:42 +0200 Subject: [PATCH 3/3] fix: changelog Co-authored-by: Dominic Reber <71256590+domire8@users.noreply.github.com> --- CHANGELOG.md | 6 ------ 1 file changed, 6 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index df24c1cc..975d4690 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -103,12 +103,6 @@ base class. - feat(components): verify return value of callbacks (#206) - fix(controllers): move input validity period to base (#207) -- feat(controllers)!: remove robot description parameter (#186) -- 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) -- feat(controllers): use parent node for tf listener (#190) -- feat(core): update translators to include new trajectory msgs (#194) ## 5.2.0