Skip to content

Can't create ParameterService from within node constructor #409

@dhood

Description

@dhood

Working on ros2/demos#192 I realised that classes that inherit from Node don't have an appropriate constructor they can call (from what I can see) to create a parameter service.

The only ParameterService constructor there is takes a node shared pointer, which can't be used from within a Node constructor because its weak pointer won't have been initialised yet. From a usability standpoint this means the parameter service needs to be created after the node is instantiated, not in its constructor, which is not ideal.

@Karsten1987 mentioned that this was at one point an issue for regular Clients/Services as well but was addressed by f0afcab (see #277 (comment)).

There's a workaround available for parameter clients by using the AsyncParametersClient constructor that takes the interface pointers, as the interfaces will have been initialised by that point. However, the call will look like:

parameters_client_ = std::make_shared<rclcpp::parameter_client::AsyncParametersClient>(
  get_node_base_interface(),
  get_node_topics_interface(),
  get_node_graph_interface(),
  get_node_services_interface());

which is not a great user experience either.


Would it make sense to:

  1. Have a constructor for the ParameterService that takes the relevant interfaces, and the version that takes the node shared pointer will just forward to it (as is done for AsyncParametersClient)? This will allow users to create a parameter service in the constructor, but it will not be pretty.
  2. Make convenience functions in the Node class for creating parameter services and parameter clients, so that the work of getting the node interfaces (shown above) is hidden from users?

Neither of these changes are necessary if users instead do something like:

auto node = std::make_shared<ParameterNode>();
auto parameters_client_ = std::make_shared<rclcpp::parameter_client::AsyncParametersClient>(node);
auto sub = parameters_client_->on_parameter_event(std::bind(&ParameterNode::on_parameter_event, node, std::placeholders::_1));

or if the node has a post-constructor initialisation step. I don't think either of those should be necessary, however the parameter clients/services seem to fundamentally be treated differently to normal services (e.g. that you have to pass a node into the constructor, can anyone point me to why this is?) so perhaps their expected usage is different as well. I think it's actually just something we haven't gotten around to yet, but want to make sure.

Metadata

Metadata

Assignees

Labels

No labels
No labels

Type

No type

Projects

No projects

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions