ros2 / rclcpp

rclcpp (ROS Client Library for C++)
Apache License 2.0
563 stars 426 forks source link

Programmatically loading parameters from yaml files into nodes dynamically #1029

Open ddengster opened 4 years ago

ddengster commented 4 years ago

Feature request

Programmatically loading parameters from yaml files into nodes dynamically

Feature description

A simple c++ function (possibly put into rclcpp::Node?) for loading parameters from yaml files after the node has been started. (ie. not via supplying a yaml file in the ros2 run command)

There were several ros1 repos using this functionality of loading nodes dynamically and supplying yaml file. However, the spec on parameters changed with ros2.

Implementation considerations

coffeebot_gripper_effort_controller: ros__parameters: type: joint_trajectory_controller/JointTrajectoryController joints:

As far as I know it follows the spec given on the ROS2 tutorials

clalancette commented 4 years ago

For what it is worth, it should be possible to do this pretty easily right now. You'd just need a small program that opened up a YAML file (maybe using yaml-cpp), formatted that YAML into a https://github.com/ros2/rcl_interfaces/blob/master/rcl_interfaces/srv/SetParameters.srv (one per node), and then called the set_parameters service on the node(s).

I guess we could potentially make this easier with an API on rclcpp, but I'm not sure.

threeal commented 4 years ago

Actually there is a rcl_parse_yaml_file() under rcl_yaml_param_parser that could be used to parse parameters from yaml config. It returns rcl_params_t though, but the good news, there is also rclcpp::parameter_map_from() that could be used to convert from rcl_params_t to rclcpp::Parameter.

So consider the parsing problem is solved, and it just need to overwrite or create a new parameter from the parsed rclcpp::Parameter.

CursedRock17 commented 1 year ago

Were the multiple load_parameters functions from pull request #1596 which can take from both a parameter_map and a yaml file enough, or can I work on this?

fujitatomoya commented 1 year ago

@CursedRock17 i think we already can do this with, (really easy)

  1. creating node with namespace and node_name.
  2. create rclcpp::AsyncParametersClient with "namespace/node_name"
  3. and then call param_client->load_parameters(parameters_filepath)

we can even do this with remote node.

but there is no such method in NodeParametersInterface, that means we need to create rclcpp::AsyncParametersClient for doing this. i believe that was the original request for this issue, so we would want to add load_parameters to NodeParametersInterface?

CC: @clalancette what do you think?

RobinBaruffa commented 10 months ago

@fujitatomoya Thank you very much for the hints, here is how I got it working for reference :

#include <ament_index_cpp/get_package_share_directory.hpp>

// Read yaml parameter file
const std::string yaml_filepath = ament_index_cpp::get_package_share_directory("package_name") +  "/config/params.yaml";
RCLCPP_INFO_STREAM(node_->get_logger(), "Reading yaml parameter files located at" <<   yaml_filename << std::endl);
auto parameters_client =  std::make_shared<rclcpp::AsyncParametersClient>(node_, "package_name");
parameters_client->load_parameters(yaml_filepath);
CursedRock17 commented 10 months ago

If this something the rclcpp maintainers fine with adding, we could review a PR @RobinBaruffa.

fujitatomoya commented 10 months ago

i believe that having load_parameters in NodeParameterInterface sounds reasonable, so that application can load the parameters with yaml file w/o having AsyncParametersClient.

@clalancette @wjwwood @sloretz @alsora @mjcarroll what do you think?

Note:

wjwwood commented 10 months ago

Sorry I haven't time to read everything here or comment before now, but we discussed this a bit today in our triage meeting and I summarized our thoughts here: https://github.com/ros2/rclcpp/pull/2406#pullrequestreview-1844391537

In short, this seems like a good feature, but we would like to avoid expanding the methods on the Node class and/or the node interfaces.

jmachowinski commented 9 months ago

Hm, I don't get the purpose of the MR for multiple reasons

@clalancette wrote in #2406 Looking at this holistically, I think my other problem with this is that our developer experience around parameters is already poor.

I agree to this statement. But I would like to point out the primary reason on why my colleagues and I think so. The current parameter interface does not support tree like parameters. E.g. lists of parameters and therefore you are restricted to an odd subset of yaml.

RobinBaruffa commented 9 months ago
  • Why do you need to load a yaml file in the first place, can the data not be given through the parameter interface ?

When writing a Gazebo plugin that was connected to ROS, I had to load a yaml directly from the c++ code as the gazebo_ros package acts as the node handler and therefore I couldn't pass parameter through it (at least not to my knowledge).

In addition, gazebo plugins are instantiated by Gazebo and are not always ran using a roslaunch file so in this case I believe it makes sense to load a yaml directly from the code.

  • If you load a yaml file, isn't the configuration state of the node is 'hidden' from external tooling, as it can't be requested via the parameter interface ?

I don't think so, if rclcpp::declare_parameter() is used before loading the yaml file, then the configuration state of the node is public and can be read / modified through ROS.

jmachowinski commented 9 months ago

When writing a Gazebo plugin that was connected to ROS, I had to load a yaml directly from the c++ code as the gazebo_ros package acts as the node handler and therefore I couldn't pass parameter through it (at least not to my knowledge).

In addition, gazebo plugins are instantiated by Gazebo and are not always ran using a roslaunch file so in this case I believe it makes sense to load a yaml directly from the code.

You get passed a sdf::ElementPtr _sdf in the Load function of your plugin. Within the _sdf the parameters for your plugin are contained. You are supposed to put them into the SDF together with 'adding' of the plugin of your robot. So no use case here...

I don't think so, if rclcpp::declare_parameter() is used before loading the yaml file, then the configuration state of the node is public and can be read / modified through ROS.

The proposal does not state the export of the loaded parameters. The suggested implementation seems to do this, even though it is not mentioned in the description of the function.