ros2 / design

Design documentation for ROS 2.0 effort
http://design.ros2.org/
Apache License 2.0
224 stars 193 forks source link

Add design document discussing QoS reconfigurability #300

Closed ivanpauno closed 4 years ago

ivanpauno commented 4 years ago

Alternative to https://github.com/ros2/design/pull/296. Based on feedback, the proposal leverages ROS parameters instead of proposing an ad-hoc file format.

ivanpauno commented 4 years ago

@jacobperron this is ready for a first pass.

ivanpauno commented 4 years ago

Another alternative to solve the discussions here, here and here, I was thinking that having a "profile id" that can be independent of the topic name and type may be a good idea.

i.e.: the user provides a construction time an id

node->create_publisher(
  topic_name, qos,
  QosOverridingOptions::Builder()
    .id("my_profile_id")
    .policies({QoSPolicy::Reliability, QoSPolicy::History})
    .callback(callback)
    .build());

and then the file looks like:

/my/node/name:
  ros__parameters:
    qos_profiles:
      publisher:
        my_profile_id:
          reliability: reliable
          history: keep_last

In that way, the node's author can use the same profile name in several entities that must share the qos profile. I would use as the default id the topic name (after expansion and remapping), but allowing the node author's to change it covers all possible use cases without the need of adding more levels of nesting (the type and an optional id).

The profile id could be remappable (using the same remapping rules that services and topic names) or not, I think it doesn't matter because all parameters are "private" to a node. Caveat: topic names can be remapped, so if publisherA uses id1, and publisherB uses its topic name as the id and it gets remapped to id1, then you have a problem. But that can easily be avoided by the user doing systems integration.

jacobperron commented 4 years ago

Using a profile ID as an alternative seems viable and perhaps simpler. Instead of defaulting the ID to the topic name, I would rather make the profile ID a hard requirement if an author wants to make their publisher or subscription QoS configurable. For example:

node->create_publisher(
  topic_name, qos,
  QosOverridingOptions::Builder("my_profile_id")
    .policies({QoSPolicy::Reliability, QoSPolicy::History})
    .callback(callback)
    .build());

This avoids any issues with topic remapping.

We'll probably want tooling to be able to query what pub/subs are associated with what profile IDs (post-remapping).

mjeronimo commented 4 years ago

I agree with @jacobperron that using a profile ID but not defaulting to the topic name would a better way to go. It seems pretty straightforward and less error prone (remapping a topic name resulting in an expected change of QoS settings).

gbiggs commented 4 years ago

If the ID is specified in source code, then it would need to be scoped to the node. Otherwise we could have two nodes from different authors using the same ID and clashing.

ivanpauno commented 4 years ago

If the ID is specified in source code, then it would need to be scoped to the node. Otherwise we could have two nodes from different authors using the same ID and clashing.

Parameter names are all "private" to a node (and not global, like in ROS 1), so that's not an issue.


@jacobperron @mjeronimo My main motivation to use the "resolved" topic name as the default id is that it makes pretty easy to set the QoS when you want is to set the same profile for all nodes for all entities in the same topic:

/**:
  ros__parameters:
    qos_profiles:
      publisher:
        '/my/topic/name':
          reliability: reliable
          history_depth: best_effort

If desired, the qos parameters can be overridden for an specific node. In the weird case that a node wants different qos parameters for the same topic, the you can override the id with something else.

But using the "resolved" topic name as the profile id makes configuration easy for the typical use case (and not impossible for strange use cases).

e.g.: node A publishes to /my_topic, node B publisher to /another_topic. Both are remapped to /new_topic. If the profile id was also modified to /new_topic, then you can configure the same qos easily using node wildcards (like the example above).

jacobperron commented 4 years ago

Defaulting to the topic name sounds okay too. The re-usability argument is convincing. In any case, I think using the (optional) pub/sub ID as a QoS profile ID makes sense.

ivanpauno commented 4 years ago

FYI (to everyone in the thread), I have opened some PRs implementing the proposal:

ivanpauno commented 4 years ago

Merging, thanks @gbiggs @jacobperron @mjeronimo @wjwwood @dejanpan for the feedback in https://github.com/ros2/design/pull/296 and here.

If there's any extra comments I can address them in a follow-up.