Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix nested parameter update #210

Merged
merged 2 commits into from
Aug 16, 2024

Conversation

MarqRazz
Copy link
Contributor

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
Copy link
Collaborator

pac48 commented Jul 16, 2024

This looks right. I'm a bit worried that it touches the same code that was changed in this PR: #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
Copy link
Contributor Author

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
Copy link
Contributor Author

MarqRazz commented Jul 17, 2024

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
Copy link
Contributor Author

MarqRazz commented Aug 7, 2024

Anything else to wrap up here @pac48?

@pac48
Copy link
Collaborator

pac48 commented Aug 7, 2024

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

@pac48 pac48 merged commit e7909a0 into PickNikRobotics:main Aug 16, 2024
7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants