PickNikRobotics / generate_parameter_library

Declarative ROS 2 Parameters
BSD 3-Clause "New" or "Revised" License
240 stars 45 forks source link

Fix nested parameter update #210

Closed MarqRazz closed 2 months ago

MarqRazz commented 3 months ago

When calling param_listener.update(override_parameters) nested parameters will fail to load because of the missing namespace separator .

This PR fixes the template that creates update() function for the parameters with mapped namespaces so that they match & update properly.

pac48 commented 3 months ago

This looks right. I'm a bit worried that it touches the same code that was changed in this PR: https://github.com/PickNikRobotics/generate_parameter_library/issues/182 We just have to be careful not to break existing configs again. This was the config that caused issues before:

  __map_joints:
    weight: {
      type: double,
      default_value: 1.0,
      description: "Joint weight",
      validation: {
        gt<>: [ 0.0 ]
      }
    }
MarqRazz commented 3 months ago

Okay so I just tested with a mapped parameter as you suggested.

supported_things: {
    type: string_array,
    default_value: ["thing1", "thing2"],
    read_only: true,
    description: "Things we support",
    validation: {
      subset_of<>: [ [ "thing1", "thing2"] ],
    }
  }
__map_supported_things:
      super_cool_double_array: {
        type: double_array,
        default_value: [0.0, 0.0, 0.0],
        read_only: true,
        description: "The dimensions of our thing",
        validation: {
          fixed_size<>: 3,
        }
      }

and it generates valid C++ code

for (const auto & value_1 : updated_params.supported_things) {

          auto& entry = updated_params.supported_things_map[value_1];
          std::string value = fmt::format("{}", value_1);

          auto param_name = fmt::format("{}{}.{}", prefix_, value, "super_cool_double_array");
          if (!parameters_interface_->has_parameter(param_name)) {
              rcl_interfaces::msg::ParameterDescriptor descriptor;
              descriptor.description = "The dimensions of our thing";
              descriptor.read_only = true;
              auto parameter = rclcpp::ParameterValue(entry.super_cool_double_array);
              parameters_interface_->declare_parameter(param_name, parameter, descriptor);
          }
          param = parameters_interface_->get_parameter(param_name);
          RCLCPP_DEBUG_STREAM(logger_, param.get_name() << ": " << param.get_type_name() << " = " << param.value_to_string());
          if(auto validation_result = fixed_size<double>(param, 3);
            !validation_result) {
              throw rclcpp::exceptions::InvalidParameterValueException(fmt::format("Invalid value set during initialization for parameter '__map_supported_things.super_cool_double_array': {}", validation_result.error()));
          }
          entry.super_cool_double_array = param.as_double_array();}
MarqRazz commented 3 months ago

In #185 there were two . removed from the param_name This PR only adds one of them back (and only on the Python side). image

MarqRazz commented 2 months ago

Anything else to wrap up here @pac48?

pac48 commented 2 months ago

@MarqRazz Can you rebase on main now that CI is passing?