Composition and Parameters: Best Practice Suggestions

Next Generation ROS
1

I’m playing around with the manual_composition example code and I want to insert a parameter service for the nodes.

I wasn’t sure what the intended implementation path is for that.
I implemented it this way:

// Add some nodes to the executor which provide work for the executor during its "spin" function.
// An example of available work is executing a subscription callback, or a timer callback.
auto talker = std::make_shared<composition::Talker>();
auto tparameter_service = std::make_shared<rclcpp::parameter_service::ParameterService>(talker);
auto tparameter_client = std::make_shared<rclcpp::parameter_client::SyncParametersClient>(talker);
exec.add_node(talker);
auto listener = std::make_shared<composition::Listener>();
auto lparameter_service = std::make_shared<rclcpp::parameter_service::ParameterService>(listener);
auto lparameter_client = std::make_shared<rclcpp::parameter_client::AsyncParametersClient>(listener);
exec.add_node(listener);

// spin will block until work comes in, execute work as it becomes available, and keep blocking.
// It will only be interrupted by Ctrl-C.
exec.spin();

By adding the two service/client objects for each node, I am successfully able to send and get params for each individual node using the ros2param tool.

However, I run into issues if I try to connect an on_parameter_event() callback.

auto parameter_event_sub = tparameter_client->on_parameter_event(std::bind(&composition::Talker::event_callback, talker, std::placeholders::_1));

void Talker::event_callback(const rcl_interfaces::msg::ParameterEvent::SharedPtr event)
{
    std::cout << "Parameter event:" << std::endl << " new parameters:" << std::endl;
    for (auto & new_parameter : event->new_parameters) {
        std::cout << "  " << new_parameter.name << std::endl;
    }
    std::cout << " changed parameters:" << std::endl;
    for (auto & changed_parameter : event->changed_parameters) {
        std::cout << "  " << changed_parameter.name << std::endl;
    }
    std::cout << " deleted parameters:" << std::endl;
    for (auto & deleted_parameter : event->deleted_parameters) {
        std::cout << "  " << deleted_parameter.name << std::endl;
    }
}

I added one to the talker client tparameter_client, but when I set variables to the listener, it still triggers the on_parameter_event. Interestingly, it doesn’t add the parameter to the talker, but it does fire its on_parameter_event callback.
So, I’m not sure if that’s a bug, or if I’m using the API incorrectly.

I’ve seen some comments where the parameter service/client stuff may be launched automatically inside the node in the future. So should I be putting it in the Talker class constructer instead? Will that be the intended usage of the parameter service, especially when working with components and composition?
Or have I discovered a bug and should add it to the GitHub Issues?

Thanks.

2

The way you extended the example to include parameter client and service will work, but that’s not the way I think you should do it. The idea of the manual_composition file is that you take existing nodes, instantiate them, add them to an executor, and run them, but nothing else. If I were you I would have added the parameter service and client to the node itself. (also I think the node automatically has a parameter service, so you should only need a client)

As for the parameter_event callback, that is a global event topic, meaning any change to any node will generate a message there. It was designed for tools to be able to notice any change to any parameter on the whole graph. We’ve talked about different ways of breaking that up, but for now it is an aggregate topic only. @rohbotics already opened an issue that these events cannot be associated to the node in question because the same topic is used by every node:

https://github.com/ros2/rclcpp/issues/243

We haven’t addressed that, I think, because we still had not gotten to parameter addressing and how namespacing will work with DDS. We covered the namespacing part, but we still haven’t gotten to the parameter address scheme.

However, if all you want to know is when a parameter has changed on the current node, you can use the Node::register_param_change_callback() method, see:

https://github.com/ros2/rclcpp/pull/244

3

To add on to @wjwwood

ParameterService allows other nodes to set/get/list parameters in this node.
ParameterClient provides API to set/get/list parameters on other nodes.

Neither of these are the API for interacting with the parameters for this node, which should be done through Node.

A caveat on Node::register_param_change_callback(), you can only register one callback per node (currently), if you register multiple times, only the last one will be called.

4

I initially tried sending params to the talker without adding any code, and it failed. So that puts this statement into question.

Thanks for the insight on the proper usage.

I guess I never knew this. What is the API for setting/getting params in Node? I hadn’t seen anything in the past. Maybe things have changed and I’m just oblivious. :blush:

The way I’ve been working with parameters is to create a Service and Client for each node. Then I register an event callback for each node. And inside the param event callback for each node, I condition on the name of the parameter being modified/created/deleted, and assign the value to a local variable used for computation, etc.

Sounds like that approach is off-the-mark. I’ll look into the Node::register_param_change_callback() method and modify how I’m working with parameters.
Usually my use case is to create/setup my node, wait for parameters to be set to a good value (typically by an external entity), then proceed to do work.

1 Like
5

Yeah, definitely oblivious. I see it there, just the same function calls of get_parameters and set_parameters.

6

Yup, they’re in the Node class: http://docs.ros2.org/beta1/api/rclcpp/classrclcpp_1_1node_1_1_node.html#a8eb74228f477cd6426060575dab5f25c

7

Hmm, that might be a regression on our part then. If you could confirm that a basic node cannot have parameters set externally that would be nice to know.

8

Ok.

Ran the talker app directly (assuming this comes from https://github.com/ros2/tutorials/blob/master/rclcpp_tutorials/src/topics/talker.cpp], and can see the Hello World output.
On another terminal, run ros2param set talker/foo 1, I get the response Failed to set parameter.
This is running beta1 with FastRTPS as the only rmw, on a Windows machine.

9

I opened an issue here
https://github.com/ros2/rclcpp/issues/295

We should move this discussion there.

10

I saw that this was moved, but I believe I am running into an issue that is independent of the ParameterService starting by default.

I haven’t seen an example of doing ros params not involving the “manual_composition” yet. All the tutorials for parameters still appear to use clients/services.

Looking over the comments for the register_param_change_callback, it appears that the callback should be doing set_parameters_atomically, unless I am misreading that.

This is what I have:

auto node = rclcpp::node::Node::make_shared(path_control_auditor::NODE_NAME);
auto paramServer = make_shared<rclcpp::parameter_service::ParameterService>(node);
node->set_parameters({ rcllcpp::parameter::ParameterVariant("someVar", ""), });
node->register_param_change_callback(
[node](const std::vector<rclcpp::parameter::ParameterVariant> params) {
    for (auto param : params)
    {
        std::cout << "Parameter name: " << param.get_name() << std::endl;
        std::cout << "Parameter value (" << param.get_type_name() << "): " <<
            param.value_to_string() << std::endl;
    }
    rcl_interfaces::msg::SetParametersResult result;
    result = node->set_parameters_atomically(params);
    return result;
}
);

Then when I use ros2params to set the parameter. Callback occurs, but as soon as “set_paremeters_atomically” occurs, I get memory violations. If I fail to call that I see no problems.

So am I reading the comments correctly? Should I be calling “set_parameters_atomically”

FYI, I am using Alpha 8 on Windows.

Thanks,

11

This is the only place I know that is using the node interface, but there might be others. turtlebot2_demo/turtlebot2_drivers/src/kobuki_node.cpp at master · ros2/turtlebot2_demo · GitHub

As for register_param_change_callback, I can see how the comments could be confusing. To clarify, the function is supposed to act atomically in sense that any changes to state in your code are only expected to occur if all the parameters are valid. In other words if changing the parameters modifies internal variables, it is expected to change nothing if it fails.

This restriction/expectation is there because otherwise the atomic parameter API could guarantee nothing.

Another note, it is expect that you set the result manually in the callback, by doing result.successful = true after you verify that the parameters are valid for your node.

The parameter callback is actually called by set_parameters_atomically. By calling set_parameters_atomically inside your callback you are actually creating a cycle, creating the memory errors you are seeing (probably some kind of stackoverflow).

Hope that helps.

1 Like
12

Thanks, that clarity is what I was looking for.

13

Hi All,
I am also trying to use manual composition and ros2 parameter feature with the demo example only.
I tried both ways.

  1. add parameter client to the node itself.

    rclcpp::SyncParametersClient::SharedPtr parameters_client = std::make_sharedrclcpp::SyncParametersClient(this);

With this approch, I get error as no matchin function as “rclcpp::SyncParametersClient::SyncParametersClient”. Because this is referrence to talker node which is of type composition.

2)add parameter client before adding to the executor:
auto talker = std::make_sharedcomposition::Talker();
auto parameter_client = std::make_sharedrclcpp::parameter_client::SyncParametersClient(talker);
exec.add_node(talker);
With this approach, not able to set parameters, as parameter_client is define here and set parameter action is getting done in talker_component.cpp.

I am testing this on crystal.
Thanks