QOS Profile support

[Roald-Schaum]

Public API Changes

register_advertisement now has a QoSProfile variable named 'qos'
subscribe now has a QoSProfile variable named 'qos'

If the qos variable is not supplied at any point in the pipeline, it defaults to the currently preexisting QOS settings.

Description

Message topics now have support for QOS profiles. This was not a thing in ROS1, but with ROS 2 it is necessary to have compliant QOS profiles between all publishers and subscribers on a node.

#887

[bjsowa]

Thank you for the PR! QoS support has been a long awaited feature in Rosbridge. As this is extending the protocol, it will need more thorough discussion on the changes. I summon @EzraBrooks @MatthijsBurgh @sea-bass .

In the meantime, could you update the Protocol Specification file (ROSBRIDGE_PROTOCOL.md) ?

[Roald-Schaum]

I sincerely apologize for how many commits this has been, I can't actually test any of the code on my own system because of a very abnormal ROS setup that I have, so I've been relying on the github actions to verify if my code actually works.

[Roald-Schaum]

These tests failures are from the tests having incompatible QoS settings between the publishers and subscribers. They all pass if the change is made, but wasn't sure if it is my place to change them.

[MatthijsBurgh]

I have seen this PR. I will try to review as soon as possible. But probably require 1 week.

[Roald-Schaum]

Shall I modify the tests to have compatible QOS profiles between the publishers and subscribers?

[bjsowa]

You are changing the default behavior of rosbridge which tries to adjust QoS for best compatibility. I would like to leave it as it is when custom qos is not provided.

For the protocol, I propose to use a nested JSON object for the qos profile. That way, it's easier to reuse the parsing implementation for different capabilities and we don't pollute the protocol specification that much. Moreover, I would prefer using a string representation of the policies instead of the raw enum numbers and we could add lifespan to the specification as well.

Here's an example of the advertise message in my proposal:

{
  "op": "advertise",
  "topic": "/foo",
  "type": "std_msgs/msg/String",
  "qos": {
    "depth": 10,
    "reliability": "reliable",
    "durability": "volatile",
    "history": "keep_last",
    "lifespan": 1.5
  }
}

Not setting qos field should result in current default behavior of rosbridge. If qos is set but not all options are present, they should fallback to system default. In particular, setting qos to empty object (qos: {}) should result in system default qos profile.

@EzraBrooks @MatthijsBurgh @sea-bass I wonder what you guys think.

[bjsowa]

One other thing to consider: What if two clients subscribe to the same topic but with different qos? Should the subscriber be overwritten or should there be one instance of MultiSubscriber per requested QoS?

[Roald-Schaum]

One other thing to consider: What if two clients subscribe to the same topic but with different qos? Should the subscriber be overwritten or should there be one instance of MultiSubscriber per requested QoS?

This seems to have been an issue discussed previously #582. The already-documented behavior of ROS would be to have one instance per QoS profile as this would ascribe. While it is possible to overwrite QoS settings, I think that would be a much worse solution as it would only lead to some subscribers having to be dropped if new subscribers with different QoS settings came in, which I think should instead be left up to the user.

[bjsowa]

@Roald-Schaum I'm working on rewriting the protocol specification document to make it easier to read and fix some inconsistencies (see #1012) and providing a way to the client for access protocol version (see #1123).

Once that is done, we could proceed with this PR and make it increase the protocol version to v2.1

[Roald-Schaum]

@bjsowa Sounds good. If you need me to do anything just let me know, I'm happy to do it.