From 727478d93e34395c71a0c1125772eee0fdc60821 Mon Sep 17 00:00:00 2001 From: aiqubits Date: Sat, 29 Aug 2026 23:08:30 +0800 Subject: [PATCH] Reject incompatible parameter range constraints Signed-off-by: aiqubits --- .../node_interfaces/node_parameters.cpp | 67 ++++++++++++++++++- rclcpp/test/rclcpp/test_node.cpp | 33 +++++++++ 2 files changed, 97 insertions(+), 3 deletions(-) diff --git a/rclcpp/src/rclcpp/node_interfaces/node_parameters.cpp b/rclcpp/src/rclcpp/node_interfaces/node_parameters.cpp index fb4ddaec35..da55046e06 100644 --- a/rclcpp/src/rclcpp/node_interfaces/node_parameters.cpp +++ b/rclcpp/src/rclcpp/node_interfaces/node_parameters.cpp @@ -195,6 +195,54 @@ format_range_reason(const std::string & name, const char * range_type) return ss.str(); } +static +std::string +format_range_type_reason( + const std::string & name, + const char * range_type, + rclcpp::ParameterType parameter_type) +{ + std::ostringstream ss; + ss << "Parameter {" << name << "} of type {" << rclcpp::to_string(parameter_type) << + "} cannot use " << range_type << " range constraints."; + return ss.str(); +} + +RCLCPP_LOCAL +rcl_interfaces::msg::SetParametersResult +__check_parameter_type_supports_range( + const rcl_interfaces::msg::ParameterDescriptor & descriptor, + rclcpp::ParameterType parameter_type) +{ + rcl_interfaces::msg::SetParametersResult result; + result.successful = true; + + if (parameter_type == rclcpp::PARAMETER_NOT_SET) { + return result; + } + + if ( + !descriptor.integer_range.empty() && + parameter_type != rclcpp::PARAMETER_INTEGER && + parameter_type != rclcpp::PARAMETER_INTEGER_ARRAY) + { + result.successful = false; + result.reason = format_range_type_reason(descriptor.name, "integer", parameter_type); + return result; + } + + if ( + !descriptor.floating_point_range.empty() && + parameter_type != rclcpp::PARAMETER_DOUBLE && + parameter_type != rclcpp::PARAMETER_DOUBLE_ARRAY) + { + result.successful = false; + result.reason = format_range_type_reason(descriptor.name, "floating point", parameter_type); + } + + return result; +} + RCLCPP_LOCAL rcl_interfaces::msg::SetParametersResult __check_integer_range( @@ -260,8 +308,11 @@ __check_parameter_value_in_range( const rcl_interfaces::msg::ParameterDescriptor & descriptor, const rclcpp::ParameterValue & value) { - rcl_interfaces::msg::SetParametersResult result; - result.successful = true; + auto result = __check_parameter_type_supports_range(descriptor, value.get_type()); + if (!result.successful) { + return result; + } + if (!descriptor.integer_range.empty() && value.get_type() == rclcpp::PARAMETER_INTEGER) { result = __check_integer_range(descriptor, value.get()); return result; @@ -508,6 +559,17 @@ __declare_parameter_common( // If there is no initial value, then skip initialization if (initial_value->get_type() == rclcpp::PARAMETER_NOT_SET) { + rcl_interfaces::msg::SetParametersResult result; + if (!parameter_descriptor.dynamic_typing) { + auto descriptor = parameter_descriptor; + descriptor.name = name; + result = __check_parameter_type_supports_range( + descriptor, static_cast(descriptor.type)); + if (!result.successful) { + return result; + } + } + // Add declared parameters to storage (without a value) parameter_infos[name].descriptor.name = name; if (parameter_descriptor.dynamic_typing) { @@ -516,7 +578,6 @@ __declare_parameter_common( parameter_infos[name].descriptor.type = parameter_descriptor.type; } parameters_out[name] = parameter_infos.at(name); - rcl_interfaces::msg::SetParametersResult result; result.successful = true; return result; } diff --git a/rclcpp/test/rclcpp/test_node.cpp b/rclcpp/test/rclcpp/test_node.cpp index cf9f6c6b02..c94a6a543c 100644 --- a/rclcpp/test/rclcpp/test_node.cpp +++ b/rclcpp/test/rclcpp/test_node.cpp @@ -559,6 +559,39 @@ TEST_F(TestNode, declare_parameter_with_no_initial_values) { } } +TEST_F(TestNode, parameter_range_constraints_reject_incompatible_types) { + auto node = std::make_shared("test_parameter_range_types"_unq); + + rcl_interfaces::msg::ParameterDescriptor integer_range_descriptor; + integer_range_descriptor.integer_range.resize(1); + integer_range_descriptor.integer_range[0].from_value = 0; + integer_range_descriptor.integer_range[0].to_value = 10; + EXPECT_THROW( + node->declare_parameter("bool_with_integer_range", false, integer_range_descriptor), + rclcpp::exceptions::InvalidParameterValueException); + EXPECT_THROW( + node->declare_parameter( + "uninitialized_bool_with_integer_range", + rclcpp::PARAMETER_BOOL, + integer_range_descriptor), + rclcpp::exceptions::InvalidParameterValueException); + + rcl_interfaces::msg::ParameterDescriptor floating_point_range_descriptor; + floating_point_range_descriptor.floating_point_range.resize(1); + floating_point_range_descriptor.floating_point_range[0].from_value = 0.0; + floating_point_range_descriptor.floating_point_range[0].to_value = 10.0; + EXPECT_THROW( + node->declare_parameter( + "integer_with_floating_point_range", 1, floating_point_range_descriptor), + rclcpp::exceptions::InvalidParameterValueException); + + integer_range_descriptor.dynamic_typing = true; + node->declare_parameter("dynamic_integer_range", 1, integer_range_descriptor); + auto result = node->set_parameter(rclcpp::Parameter("dynamic_integer_range", false)); + EXPECT_FALSE(result.successful); + EXPECT_NE(result.reason.find("cannot use integer range constraints"), std::string::npos); +} + TEST_F(TestNode, declare_parameter_with_allow_undeclared_parameters) { // test cases without initial values auto node = std::make_shared(