Dynamically remap topic names

[fmrico]

Dynamically remap topic names

[jrutgeer]

@fujitatomoya

I was rather thinking about the use case where you'd remap either the publisher, or the subscriber, but not both. E.g. robot moves from 'warehouse_1' to 'warehouse_2' and hence needs to remap its listeners to the publishers of 'warehouse_2' instead of those of 'warehouse_1'. But indeed, add_parameter_event_callback is also a possibility.

[clalancette]

IMO this is good feature so that user can reconnect the topic to the different resource at runtime.

We do have a small section about dynamic remapping in the design document at http://design.ros2.org/articles/static_remapping.html#static-versus-dynamic-remapping . I can't exactly remember now, but I think at the time we didn't implement it to keep the scope small. @sloretz maybe you can comment here?

Regardless, I do think it is a fine feature to have. If we are going to do it, I'd much prefer to have it at the rcl layer, so that both rclcpp and rclpy can more easily get support for it.

I'm not totally convinced that we want to expose a service to allow this remapping to happen. A service is just one way that a user might want to make this change, and services are expensive in terms of discovery. I could imagine a user changing this via a parameter, a service, a topic, or even in reaction to an external event. So my suggestion here is to start off just by defining the API, and worrying about a service or something like that as a totally separate step.

[fujitatomoya]

IMO this is good feature so that user can reconnect the topic to the different resource at runtime.

[fmrico]

Currently, remapping is established either through global arguments (from argc/argv) or through local arguments (node options) and takes value when creating the resource. In rcl_publisher_init, for example, a call to rcl_node_resolve_name calls to rcl_resolve_name with local and global arguments.

Let's center on publishers in this conversation but take into account other resources. One approach could be as follows: The interface to create a publisher could be very similar to rcl_publisher_init, but taking a pre-existing publisher and the new topic name (we can get the current topic name and options from the existing publisher) :

rcl_ret_t
rcl_publisher_remap(
  rcl_publisher_t * publisher,
  const rcl_node_t * node,
  const char * remapped_topic_name,
)

In this function, to avoid changes in rmw, we could replace the current one by one with the remapped name:

1. Finish current publisher via rcl_publisher_fini
2. Add a dynamic remap to the node (see below)
3. Call rcl_publisher_init

What do you think?

Something that worries me is what happens with a subscriber who has received messages that still need to be processed or a client who is waiting for a response. If we destroy the subscriber or the client, messages and responses may be lost.

Add a dynamic remap to the node

To extend rcl_resolve_name with a new parameter dynamic_args:

static
rcl_ret_t
rcl_resolve_name(
  const rcl_arguments_t * local_args,
  const rcl_arguments_t * global_args,
  const rcl_arguments_t * dynamic_args,
  const char * input_topic_name,
  const char * node_name,
[...]

rcl_node_resolve_name would get the dynamic arguments from the node, extending rcl_node_impl_s:

struct rcl_node_impl_s
{
  rcl_node_options_t options;
  rcl_arguments_t dynamic_arguments;
  rmw_node_t * rmw_node_handle;
  rcl_guard_condition_t * graph_guard_condition;
  const char * logger_name;
  const char * fq_name;
};

Many functions that takes local_arguments and global_arguments to resolve a name, need to add a dynamic_arguments, virtually all functions in remap.h: rcl_remap_topic_name, rcl_remap_service_name, rcl_remap_node_name, andrcl_remap_node_namespace. Maybe we also need to add functions to add create rcl_remap_t from C strings, because now they are not parsed from argc/argv.

A good aspect is that it opens the option of having more dynamic arguments, beyond remapping resources.

[fujitatomoya]

@fmrico thanks for sharing idea.

In this function, to avoid changes in rmw, we could replace the current one by one with the remapped name

just checking, so once the topic is remapped, the existing publisher will no longer exist? that will be replaced with new topic name? right?
i think this is what remap means, if existing publisher will stay alive, that sounds more like copy.

Something that worries me is what happens with a subscriber who has received messages that still need to be processed or a client who is waiting for a response.

if we want it to be processed or continue, this feature remap is more like copy the topic? (not finilizing the original publisher.)
I think that is useful too, because the use case for changing topic name at runtime is to connect the topic with different endpoints after they are already launched.
but the above is not remap in ROS? i think original topic should not exist if it is remapped to the another topic name, that aligns to the current static remapping option.

user application should be able to know that there is no publishers anymore since it is remapped?
i think we can detect that matched event that there is no publisher (publisher count is zero) anymore on the topic, but not sure if it can know that is because of remapping.

So my suggestion here is to start off just by defining the API, and worrying about a service or something like that as a totally separate step.

totally agree, currently node has each dedicated service or parameter to manage the specific configuration or options.
this is or will not be cost effective especially for discovery, but it tends to land in this way since service, parameter interfaces are good for user experience.
i believe that it is worth to reconsider those options, external services or parameters provided by node could be optimized.
one way could be, we can have common node service interface to manage options or configuration via single entity with capabilities.

[jrutgeer]

Just as a FYI:

Someone asked for this functionality on the Robotics stack exchange.

I could imagine a user changing this via a parameter,

I proposed as a solution to install a post_set_parameter_callback that resets the current subscriber and creates a new one for the changed topic name.

auto post_set_parameter_callback =
  [this](const std::vector<rclcpp::Parameter> & parameters) {
    for (const auto & param : parameters) {
      if (param.get_name() == "topic_name") {
        std::string new_topic = this->get_parameter("topic_name").as_string();
        // Delete current subscriber
        // Create new subscriber
      }
    }
  }

post_set_parameters_callback_handle_ = this->add_post_set_parameters_callback(post_set_parameter_callback);

I did not test this, but I assume it works.

[jrutgeer]

This issue is probably relevant as well:

ros2/rclcpp#2146

[fujitatomoya]

@jrutgeer

I proposed as a solution to install a post_set_parameter_callback that resets the current subscriber and creates a new one for the changed topic name.

it should work but i would use ParameterEventHandler::add_parameter_event_callback instead. with add_post_set_parameters_callback publisher needs to be responsible to set the parameters on the subscription node each, this could be problem if there are many subscriptions. instead, publisher remaps the topic name and set the specific parameter that indicates that topic name has been changed, then taking advantage of /parameter_event topic and ParameterEventHandler::add_parameter_event_callback will deliver the event to the all concerned subscriptions. i believe that this would be better and scalable for distributed application.