From 9bd5769bf13b2948ca6d818a2b97462c0663e2e0 Mon Sep 17 00:00:00 2001 From: "Michael X. Grey" Date: Fri, 26 Apr 2024 04:07:22 +0000 Subject: [PATCH 1/2] Comply with rcl_interfaces parameter services documentation Signed-off-by: Michael X. Grey --- rclcpp/src/rclcpp/parameter_service.cpp | 57 ++++++++++++++----------- 1 file changed, 33 insertions(+), 24 deletions(-) diff --git a/rclcpp/src/rclcpp/parameter_service.cpp b/rclcpp/src/rclcpp/parameter_service.cpp index 0923798339..b8fbdc6049 100644 --- a/rclcpp/src/rclcpp/parameter_service.cpp +++ b/rclcpp/src/rclcpp/parameter_service.cpp @@ -41,15 +41,11 @@ ParameterService::ParameterService( const std::shared_ptr request, std::shared_ptr response) { - try { - auto parameters = node_params->get_parameters(request->names); - for (const auto & param : parameters) { - response->values.push_back(param.get_value_message()); - } - } catch (const rclcpp::exceptions::ParameterNotDeclaredException & ex) { - RCLCPP_DEBUG(rclcpp::get_logger("rclcpp"), "Failed to get parameters: %s", ex.what()); - } catch (const rclcpp::exceptions::ParameterUninitializedException & ex) { - RCLCPP_DEBUG(rclcpp::get_logger("rclcpp"), "Failed to get parameters: %s", ex.what()); + response->values.reserve(request->names.size()); + for (const auto & name : request->names) { + rclcpp::Parameter value; + node_params->get_parameter(name, value); + response->values.push_back(value.get_value_message()); } }, qos_profile, nullptr); @@ -62,15 +58,11 @@ ParameterService::ParameterService( const std::shared_ptr request, std::shared_ptr response) { - try { - auto types = node_params->get_parameter_types(request->names); - std::transform( - types.cbegin(), types.cend(), - std::back_inserter(response->types), [](const uint8_t & type) { - return static_cast(type); - }); - } catch (const rclcpp::exceptions::ParameterNotDeclaredException & ex) { - RCLCPP_DEBUG(rclcpp::get_logger("rclcpp"), "Failed to get parameter types: %s", ex.what()); + response->types.reserve(request->names.size()); + for (const auto & name : request->names) { + rclcpp::Parameter value; + node_params->get_parameter(name, value); + response->types.push_back(value.get_type()); } }, qos_profile, nullptr); @@ -93,7 +85,7 @@ ParameterService::ParameterService( } catch (const rclcpp::exceptions::ParameterNotDeclaredException & ex) { RCLCPP_DEBUG(rclcpp::get_logger("rclcpp"), "Failed to set parameter: %s", ex.what()); result.successful = false; - result.reason = ex.what(); + result.reason = "Parameter is not declared by the node, which only allows declared parameters"; } response->results.push_back(result); } @@ -135,11 +127,28 @@ ParameterService::ParameterService( const std::shared_ptr request, std::shared_ptr response) { - try { - auto descriptors = node_params->describe_parameters(request->names); - response->descriptors = descriptors; - } catch (const rclcpp::exceptions::ParameterNotDeclaredException & ex) { - RCLCPP_DEBUG(rclcpp::get_logger("rclcpp"), "Failed to describe parameters: %s", ex.what()); + // TODO(@mxgrey): This could have fewer heap allocations if + // NodeParametersInterface had a singular describe_parameter function or + // if describe_parameters could accept an argument to behave as if + // allow_undeclared_ is true. + for (const auto & name : request->names) { + bool description_found = false; + try { + auto description = node_params->describe_parameters({name}); + if (!description.empty()) { + description_found = true; + response->descriptors.push_back(std::move(description.front())); + } + } catch (const rclcpp::exceptions::ParameterNotDeclaredException & ex) { + // If the parameter was not declared then we will just push_back a + // default-initialized descriptor. + } + + if (!description_found) { + auto default_description = rcl_interfaces::msg::ParameterDescriptor(); + default_description.name = name; + response->descriptors.push_back(default_description); + } } }, qos_profile, nullptr); From 5db9ffad27e3f807fffd50903c147618eed83db0 Mon Sep 17 00:00:00 2001 From: "Michael X. Grey" Date: Mon, 29 Apr 2024 05:14:10 +0000 Subject: [PATCH 2/2] Use a constant value that is set upstream Signed-off-by: Michael X. Grey --- rclcpp/src/rclcpp/parameter_service.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/rclcpp/src/rclcpp/parameter_service.cpp b/rclcpp/src/rclcpp/parameter_service.cpp index b8fbdc6049..519952409c 100644 --- a/rclcpp/src/rclcpp/parameter_service.cpp +++ b/rclcpp/src/rclcpp/parameter_service.cpp @@ -85,7 +85,7 @@ ParameterService::ParameterService( } catch (const rclcpp::exceptions::ParameterNotDeclaredException & ex) { RCLCPP_DEBUG(rclcpp::get_logger("rclcpp"), "Failed to set parameter: %s", ex.what()); result.successful = false; - result.reason = "Parameter is not declared by the node, which only allows declared parameters"; + result.reason = rcl_interfaces::msg::SetParametersResult::REASON_UNDECLARED_NOT_ALLOWED; } response->results.push_back(result); }