Getting QoS policies of all the publishers publishing to and subscribers subscribing to a topic

[jaisontj]

Feature request

Feature description

As a part of implementing this feature for the ros2cli, some form of map of topic to publisher and subscriber qos policies need to be maintained.

Implementation considerations

Similar to how rmw_count_publishers and rmw_count_subscribers works; custom_participant_info.hpp has two variables; reader_topic_cache and writer_topic_cache which can be used to get a list of subscribers and publishers to a topic represented by their GUID.

Question 1: Is there a way to get Publisher and Subscriber attributes from their respective GUIDs?

Assuming that the answer to the above question is "No", I was thinking that a new qos_cache.hpp could be created which stores a map of GUID to rmw_qos_profile_t and this map could be populated inside the process_discovery_info method found inside custom_participant_info.hpp in a manner similar to how topic_cache.add/removeTopic works, i.e get the Writer/ReaderQos from their corresponding Writer/ReaderProxyData.

This leaves us with figuring out a way to convert Writer/ReaderQos to rmw_qos_profile_t; There is a function dds_qos_to_rmw_qos inside qos.hpp. However, this method expects data of type SubscriberAttributes or PublisherAttributes; this is because rmw_qos_profile stores history and depth data which comes from TopicAttributes.

Question 2: How do we go about getting the TopicAttributes in this case?

Question 3: Should the dds_qos_to_rmw_qos function be using two functions inside: one to populate data from TopicAttributes and another to populate data from Writer/ReaderQos (this code can now be reused for our use-case)?

Question 4: Is there a better way to do what I'm trying to do?

[ivanpauno]

Question 1: Is there a way to get Publisher and Subscriber attributes from their respective GUIDs?

Assuming that the answer to the above question is "No", I was thinking that a new qos_cache.hpp could be created which stores a map of GUID to rmw_qos_profile_t and this map could be populated inside the process_discovery_info method found inside custom_participant_info.hpp in a manner similar to how topic_cache.add/removeTopic works, i.e get the Writer/ReaderQos from their corresponding Writer/ReaderProxyData.

AFAIK, you're analysis is correct. There is no way to access the data if you don't store it at discovery.

Question 2: How do we go about getting the TopicAttributes in this case?

Haven't check thoroughly, but if it's not part of the discovery data I don't see a way of recover it.

Question 3: Should the dds_qos_to_rmw_qos function be using two functions inside: one to populate data from TopicAttributes and another to populate data from Writer/ReaderQos (this code can now be reused for our use-case)?

Sure! any change that improves code reuse is ok.

Question 4: Is there a better way to do what I'm trying to do?

I would have done the same, so I don't have a better way.

[ivanpauno]

this is because rmw_qos_profile stores history and depth data which comes from TopicAttributes.

@richiprosima Is this somehow available from the publisher/subscriber discovery information?

[MiguelCompany]

@ivanpauno Yes, the discovery info structure has an XXXProxyData info field, that is the one processed here. This structure has an XXXQos m_qos field with the QoS parameters of the discovered endpoint. See here and here

[ivanpauno]

@richiprosima Sorry for the unclear question.
I see that WritterQoS is available in WritterProxyData, though some of our qos settings like history_kind and history_depth don't seem to be part of the WritterQoS, but of HistoryQos which is part of the TopicAttributes that are available in PublisherAttributes (and not directly in the WritterQoS). HistoryQoS nor TopicAttributes seems to be available in WriterProxyData.
I wonder if HistoryQoS policy is available somehow or not.

[jaisontj]

@ivanpauno I want to display the participant name along with their QoS Profile. For this I plan on leveraging the discovered_names map, but I've observed that some participants do not have names and therefore do not have an entry in the discovered_names map.

Question 1: Why is it that some participants do not have names?

Question 2: My plan is to display their name as NAME_NOT_FOUND and display their QoS Policy anyway. Does it make sense, will this have some unintended side effects?

Question 3: Is there another way to retrieve participant name that I may be missing?

Moreover, on further thought, I think it makes sense to modify the ParticipantTopicMap to

typedef std::unordered_map<std::string, std::vector<std::string>> TopicToTypes;
typedef std::map<GUID_t, std::pair<rmw_qos_profile_t, TopicToTypes>> ParticipantTopicMap;

As opposed to introducing a new map inside qos_cache.hpp (original plan). A new map will add complexities like:

• we will have to ensure thread safety of this new map
• ensure that additions and removals to this map are atomic with TopicCache

Question 4: Have I missed something with the above change in approach?

[ivanpauno]

Question 1: Why is it that some participants do not have names?

All nodes have names. All participants associated with a node have names.
Some DDS vendors provide a participant name (IIRC, only Fast-RTPS).
If not, we use the user data qos policy to communicate the node name.

Still, the discovered_names map should have all the node names.
Participants created using non-ros code may not have a name (nor use our user data convention).

Question 2: My plan is to display their name as NAME_NOT_FOUND and display their QoS Policy anyway. Does it make sense, will this have some unintended side effects?

IMO, If the participant doesn't have an entry in discovered_names, we could left the node name empty.
It should be understanded as a publisher/subscription created by non-ros code and still following ROS topic name conventions (or ROS code using non-demangle option).

Question 4: Have I missed something with the above change in approach?

Using the same map sounds reasonable.
It should be something like:
typedef std::unordered_map<std::string, TopicData> TopicToTypes;

And TopicData should be an struct with the types and the qos policies.
Remember that you may find more than one Publisher/Subscription created by the same node and using the same topic name, so you may have many qos policies items in each TopicData (not only one).

[jaisontj]

Question 1: Why is it that some participants do not have names?

All nodes have names. All participants associated with a node have names.
Some DDS vendors provide a participant name (IIRC, only Fast-RTPS).
If not, we use the user data qos policy to communicate the node name.

Still, the discovered_names map should have all the node names.
Participants created using non-ros code may not have a name (nor use our user data convention).

Alright!

Question 2: My plan is to display their name as NAME_NOT_FOUND and display their QoS Policy anyway. Does it make sense, will this have some unintended side effects?

IMO, If the participant doesn't have an entry in discovered_names, we could left the node name empty.
It should be understanded as a publisher/subscription created by non-ros code and still following ROS topic name conventions (or ROS code using non-demangle option).

If this is obvious then leaving it empty makes sense, but if not, leaving something that might help the user identify why some publishers/subscribers do not have names might be helpful IMO.

Question 4: Have I missed something with the above change in approach?

Using the same map sounds reasonable.
It should be something like:
typedef std::unordered_map<std::string, TopicData> TopicToTypes;

And TopicData should be an struct with the types and the qos policies.
Remember that you may find more than one Publisher/Subscription created by the same node and using the same topic name, so you may have many qos policies items in each TopicData (not only one).

Hmm, interesting. The reason why I went with including the QoS Policy in the ParticipantTopicMap was because

• we are associating QoS Policies with each publisher's/subscriber's GUID_t
• avoid duplication of the QoS Policy since publishers and subscribers can be associated with multiple topics.
• In case we need to get the QoS Policy for a specific GUID_t in the future, as a separate feature.

Is there a reason why you prefer adding the QoS Policy to TopicToTypes instead, apart from the advantage of making retrieval of data easy?

Moreover, just to make sure that I've understood correctly, the TopicData structure would have a list of topic types and another list of a struct that encapsulates a Participant's name (gotten from discovered_names) and its respective QoS Profile (rmw_qos_profile_t) (like rmw_participant_qos_profile_t created in this PR). Is that correct or should we use a new struct encapsulating topic_type, node_name and rmw_qos_profile_t and then have a list of that inside TopicData?

Question: What exactly does topic_type represent?
Does every publisher/subscriber to a topic have a topic_type?
Why it the count of topic_type used to count the number of publishers/subscribers to a topic instead of counting all GUID_t associated with a topic?

[ivanpauno]

If this is obvious then leaving it empty makes sense, but if not, leaving something that might help the user identify why some publishers/subscribers do not have names might be helpful IMO.

Leaving a message is fine too.
Please, left something that is not a valid node name (see https://github.com/ros2/rmw/blob/ca572aa2de3082ace9b8572011c393630745e010/rmw/include/rmw/validate_node_name.h#L34-L40 and https://github.com/ros2/rmw/blob/ca572aa2de3082ace9b8572011c393630745e010/rmw/include/rmw/validate_namespace.h#L40-L42).

we are associating QoS Policies with each publisher's/subscriber's GUID_t

Watch out, it's the participant GUID, not the publisher/subscriber GUID (see

rmw_fastrtps/rmw_fastrtps_shared_cpp/include/rmw_fastrtps_shared_cpp/custom_participant_info.hpp

Lines 172 to 173 in 455d214

	trigger = topic_cache().addTopic(proxyData.RTPSParticipantKey(),
	proxyData.topicName().to_string(), proxyData.typeName().to_string());

).
So, each entry to the map is directly associated with a node (well, with a participant).

[jaisontj]

Watch out, it's the participant GUID, not the publisher/subscriber GUID (see

rmw_fastrtps/rmw_fastrtps_shared_cpp/include/rmw_fastrtps_shared_cpp/custom_participant_info.hpp

Lines 172 to 173 in 455d214

	trigger = topic_cache().addTopic(proxyData.RTPSParticipantKey(),
	proxyData.topicName().to_string(), proxyData.typeName().to_string());

).
So, each entry to the map is directly associated with a node (well, with a participant).

This is a little confusing because this line resides in a function which is called from onSubscriberDiscovery and onPublisherDiscovery. Therefore, I assumed that the GUIDs would correspond to Subscriber and Publisher GUIDs respectively.

Ok, so to summarize on the design:

• Create a new struct TopicData

struct TopicData {
  vector<string> types;
  vector<rmw_qos_profile_t> qos_profiles;
}

Here, types and qos_profiles have the same indexing.

• update typedef std::unordered_map<std::string, TopicData> TopicToTypes;
  Question 1: Should ParticipantToTopicMap also be updated to have TopicData?

• update TopicCache.addTopic to

bool addTopic(
    const eprosima::fastrtps::rtps::InstanceHandle_t & rtpsParticipantKey,
    const std::string & topic_name,
    const std::string & type_name,
    const WriterQos & dds_qos) {..}

Question 2: Should addTopic function be overloaded for WriterQos and ReaderQos or just use a template variable?

- Question 3: how will removeTopic work. Are topic_types (or at least the combination of topic_type, topic_name and qos_profile) unique ? Otherwise I am not sure how I can identify the right QoS Policy to remove.

If we can finalize on removeTopic; I think this should work. Let me know what you think.

[ivanpauno]

Question 1: Should ParticipantToTopicMap also be updated to have TopicData?

If we only need the "by topic" info, and we don't provide an equivalent "by node" introspection ability, ParticipantToTopicMap doesn't need to be updated.

Question 2: Should addTopic function be overloaded for WriterQos and ReaderQos or just use a template variable?

Both a template or taking directly rmw_qos_profile_t sounds reasonable.

Question 3: how will removeTopic work. Are topic_types (or at least the combination of topic_type, topic_name and qos_profile) unique ? Otherwise I am not sure how I can identify the right QoS Policy to remove.

Good question.
What if we add the publisher/subscription GUID to the TopicData struct?
That will provide an unique way of identifying each publisher/subscription.

[jaisontj]

Thank you for answering these @ivanpauno

Question 3: how will removeTopic work. Are topic_types (or at least the combination of topic_type, topic_name and qos_profile) unique ? Otherwise I am not sure how I can identify the right QoS Policy to remove.

Good question.
What if we add the publisher/subscription GUID to the TopicData struct?
That will provide an unique way of identifying each publisher/subscription.

Yeah, that seems to be the only way. This is why I initially thought that modifying the `ParticipantQosMap

So here is what I am thinking about the struct:

struct TopicData {
  vector<GUID_t> guids;
  vector<string> types;
  vector<rmw_qos_profile_t> qos_profiles;
}

where all three have the same indexing.

Alternatively, we could also have them be a vector<tuple<guid_t, string, rmw_qos_profile_t>>

Is there a reason to go for separate arrays with same indexing as opposed to encapsulating it into one type?

Let me know what you think?

[ivanpauno]

Is there a reason to go for separate arrays with same indexing as opposed to encapsulating it into one type?

Encapsulating in only one type sounds reasonable, whatever you prefer.

[emersonknapp]

@dirk-thomas this one is also complete