Enhanced ros2 topic info to display node name, node namespace, topic type and qos profile of the publishers and subscribers. #385

jaisontj · 2019-10-24T22:50:02Z

NOTE: DO NOT MERGE until rmw #186, rmw_implementation #72, rcl #511 and rclpy #454 are merged.

This PR makes the necessary changes to implement this feature request. Once all the dependent PRs are merged, the ros2cli layer can now use the node.get_publishers_info_by_topic and node.get_subscriptions_info_by_topic functions to enhance the output of ros2 topic info <topic_name> as follows:

$ ros2 topic info rosout

Topic              : rosout

Publisher count    : 2

Node Name: _NODE_NAME_UNKNOWN_
Node Namespace: _NODE_NAMESPACE_UNKNOWN_
Topic Type: rcl_interfaces::msg::dds_::Log_
QoS Profile:
  Reliability: RMW_QOS_POLICY_RELIABILITY_RELIABLE
  Durability: RMW_QOS_POLICY_DURABILITY_VOLATILE
  Lifespan: 2147483651294967295 nanoseconds
  Deadline: 2147483651294967295 nanoseconds
  Liveliness: RMW_QOS_POLICY_LIVELINESS_AUTOMATIC
  Liveliness Lease Duration: 2147483651294967295 nanoseconds

Node Name: listener
Node Namespace: /
Topic Type: rcl_interfaces::msg::dds_::Log_
QoS Profile:
  Reliability: RMW_QOS_POLICY_RELIABILITY_RELIABLE
  Durability: RMW_QOS_POLICY_DURABILITY_VOLATILE
  Lifespan: 2147483651294967295 nanoseconds
  Deadline: 2147483651294967295 nanoseconds
  Liveliness: RMW_QOS_POLICY_LIVELINESS_AUTOMATIC
  Liveliness Lease Duration: 2147483651294967295 nanoseconds

Subscription count : 0

Related to issues in aws-roadmap #86

jaisontj · 2019-10-29T21:21:27Z

@ivanpauno

dabonnie

LGTM

ivanpauno

LGTM!

ros2topic/ros2topic/verb/info.py

ivanpauno

LGTM!

hidmic

I think this is the first time we expose the RMW message type name. I can see value in providing such information, but this is not how we usually refer to topic types i.e. we use std_msgs/msg/String, not std_msgs::msg::dds_::String_. @dirk-thomas @jacobperron do you recall if we provide an API somewhere to carry out the conversion?

hidmic · 2019-12-17T21:06:46Z

ros2topic/ros2topic/verb/info.py

@@ -25,6 +51,10 @@ def add_arguments(self, parser, cli_name):
        arg = parser.add_argument(
            'topic_name',
            help="Name of the ROS topic to get info (e.g. '/chatter')")
+        parser.add_argument(
+            '--verbose', '-v', action='store_true',
+            help='Prints detailed information like the Node Name, Node Namespace, Topic Type, '


@jaisontj @mm318 consider:

Suggested change

help='Prints detailed information like the Node Name, Node Namespace, Topic Type, '

help='Prints detailed information like the node name, node namespace, topic type, '

instead

hidmic · 2019-12-17T21:07:13Z

ros2topic/ros2topic/verb/info.py

+    except NotImplementedError as e:
+        logger.warning(str(e))
+    except Exception as e:
+        logger.error(str(e))


@jaisontj @mm318 consider:

Not catching all but only the ones you'd expect.

Not using the logging module but returning the error string from InfoVerb.main instead.

hidmic · 2019-12-17T21:07:49Z

ros2topic/ros2topic/verb/info.py

+            if (args.verbose):
+                print_topic_info(topic_name, node.get_publishers_info_by_topic)
+            print('\nSubscription count : %d' % node.count_subscribers(topic_name))
+            if (args.verbose):


@jaisontj @mm318 parenthesis are not needed.

Removed the parenthesis.

hidmic · 2019-12-17T21:07:52Z

ros2topic/ros2topic/verb/info.py

-            print('Publisher count: %d' % node.count_publishers(topic_name))
-            print('Subscriber count: %d' % node.count_subscribers(topic_name))
+            print('\nPublisher count    : %d' % node.count_publishers(topic_name))
+            if (args.verbose):


@jaisontj @mm318 parenthesis are not needed.

Removed the parenthesis.

jacobperron · 2019-12-17T21:35:57Z

ros2topic/ros2topic/verb/info.py

@@ -40,5 +70,10 @@ def main(self, *, args):
                return "Unknown topic '%s'" % topic_name
            type_str = topic_types[0] if len(topic_types) == 1 else topic_types
            print('Type: %s' % type_str)
-            print('Publisher count: %d' % node.count_publishers(topic_name))
-            print('Subscriber count: %d' % node.count_subscribers(topic_name))
+            print('\nPublisher count    : %d' % node.count_publishers(topic_name))


nitpick: it looks odd to me with the added whitespace before the colon, especially when Type: does not have the same alignment. IMO, it looks fine without the added whitespace.

Removed the whitespace.

jacobperron · 2019-12-17T21:40:14Z

do you recall if we provide an API somewhere to carry out the conversion?

Not off the top of my head. I wonder if the displayed type in verbose mode will confuse users, since it is language-specific and rmw-implementation specific. We're also displaying the ROS type (e.g. std_msgs/msg/String). Maybe we should change the label of the verbose type to something like "RMW type"?

ivanpauno · 2019-12-18T12:55:05Z

I think this is the first time we expose the RMW message type name. I can see value in providing such information, but this is not how we usually refer to topic types i.e. we use std_msgs/msg/String, not std_msgs::msg::dds_::String_.

If std_msgs::msg::dds_::String_ is appearing, it's because something is wrong in the rmw_fastrtps PR (more specifically, the type wasn't demangled).

hidmic

Also, I've just realized this has no associated test.

mm318 · 2019-12-19T08:49:18Z

The mangled type name is being discussed and addressed in ros2/rmw_fastrtps#336.

I'll add some tests to this pull request.

Thanks.

mm318 · 2019-12-31T03:52:39Z

Hi @hidmic, I have fixed and added some tests regarding the verbose mode of ros2 topic info.

hidmic

LGTM!

ros2topic/ros2topic/verb/info.py

mm318 · 2020-02-14T21:34:43Z

Is it okay to merge this as well now that ros2/rclcpp#960 and ros2/rclpy#454 are merged, @hidmic, @ivanpauno, @wjwwood?

wjwwood · 2020-02-14T21:43:06Z

Let me re-review it, and it will need CI after that I think.

mm318 · 2020-02-14T22:03:46Z

Oh okay, I was under the impression that the CI run at ros2/rclpy#454 (comment) already included ros2cli.

wjwwood · 2020-02-14T22:09:38Z

That's possible, though I didn't assume that. Any ways it would be good to double check after I review. Could be --packages-up-to ros2topic, so it would be somewhat quick.

wjwwood

lgtm

ros2topic/ros2topic/verb/info.py

Signed-off-by: Miaofei <miaofei@amazon.com>

wjwwood · 2020-02-14T23:11:10Z

Here's Linux CI, since CI is really backed up right now on Windows:

wjwwood · 2020-02-15T00:16:12Z

Not sure what the issue was CI failed, and the tests ran for like an hour before failing...

mm318 · 2020-02-15T17:38:28Z

I find that the ros2topic unit tests are flaky and take a long time to run. Can you run CI again to see if the same failures occur?

ivanpauno · 2020-02-17T13:51:58Z

I see a bunch of:

[ros2topic-cli-80] Failed to get information by topic for publishers: function not supported by RMW_IMPLEMENTATION
[ERROR] [ros2topic-cli-80]: process has died [pid 10299, exit code 1, cmd 'ros2 topic info -v /chatter'].

That's because the function is not implemented in rmw_connext yet.
It has already been merged in rmw_cyclonedds, so I would rather implement it for connext before merging this. @mm318 Can you add that implementation?

mm318 · 2020-02-17T17:21:54Z

I don't understand why it's an issue causing the test to fail for rmw_connext_cpp but not for rmw_opensplice_cpp?

ivanpauno · 2020-02-17T17:29:15Z

I don't understand why it's an issue causing the test to fail for rmw_connext_cpp but not for rmw_opensplice_cpp?

Because that CI job didn't include opensplice.
Also, support for OpenSplice was recently removed (https://discourse.ros.org/t/opensplice-move-to-eclipse-cyclone-dds-in-ros-2/12370/7, ros2/ros2#865, ros2/ci#380), so I wouldn't worry about implementing the feature there.

mm318 · 2020-02-18T00:50:12Z

Does this comment still apply for the rmw_connext_cpp implementation?

ivanpauno · 2020-02-18T12:52:12Z

Does this comment still apply for the rmw_connext_cpp implementation?

I think that not having an implementation in rmw_connext blocks this PR.
My comment was because implementing ros2/rmw_fastrtps#312 (comment) in rmw_connext will solve the issue, as all the graph logic was moved to a common package. Though that will be done before Foxy release, it's still waiting.
Considering that not having an implementation in rmw_connext blocks this PR and that all the other supported rmw implementations had already implemented the feature, I suggest doing rmw_connext implementation now rather than wait to the other refactor.

mm318 · 2020-02-21T21:49:33Z

Hi @ivanpauno, the rmw_connext_cpp implementation is ready for review here: ros2/rmw_connext#391

The full set of relevant pull requests is now:

Implement the rmw_get_publishers/subscriptions_info_by_topic() methods rmw_connext#391
Enable test_get_publishers_subscriptions_info_by_topic() unit test for more rmw_implementations rclpy#511
Enhanced ros2 topic info to display node name, node namespace, topic type and qos profile of the publishers and subscribers. #385

Thanks

zmichaels11 · 2020-02-24T18:03:47Z

@ros2/aws-oncall - please run this CI job
Gist: https://gist.github.com/prajakta-gokhale/3bfd17646ef9b816edbab230f73df9d2
BUILD args: --packages-up-to ros2topic
TEST args: --packages-select rmw_connext_shared_cpp rclpy ros2topic
Job: ci_launcher

dirk-thomas · 2020-02-24T18:19:05Z

--packages-select ros2cli

This won't test ros2topic at all.

zmichaels11 · 2020-02-24T18:23:13Z

@mm318 please fill in @ros2/aws-oncall with what build args and test args should be used.
I'll update my comment to match afterwards.

prajakta-gokhale · 2020-02-24T18:35:30Z

Gist: https://gist.githubusercontent.com/prajakta-gokhale/3bfd17646ef9b816edbab230f73df9d2/raw/30b621cdf64821ec62dc94d4853aafce08e5e7af/ros2.repos
Build args: --packages-up-to ros2topic
Test args: --packages-select rmw_connext_shared_cpp rclpy ros2topic

Linux
Linux-aarch64
macOS
Windows
Windows-container

Rebuilding Windows:

Windows
Windows-container

ivanpauno · 2020-02-26T17:04:19Z

Also including rcl rmw_connext_cpp tests:

macOS
Windows

ivanpauno · 2020-02-26T18:30:54Z

@mm318 Can you also enable tests here https://github.com/ros2/rcl/blob/25f39c3a4867018f1140bec3ab904bd074a49849/rcl/test/rcl/test_info_by_topic.cpp#L57? Thanks!

mm318 · 2020-02-26T21:51:21Z

@mm318 Can you also enable tests here https://github.com/ros2/rcl/blob/25f39c3a4867018f1140bec3ab904bd074a49849/rcl/test/rcl/test_info_by_topic.cpp#L57? Thanks!

Done, @ivanpauno.

The full set of relevant pull requests is now:

Implement the rmw_get_publishers/subscriptions_info_by_topic() methods rmw_connext#391
Enable TestInfoByTopicFixture unit tests for more rmw_implementations rcl#583
Enable test_get_publishers_subscriptions_info_by_topic() unit test for more rmw_implementations rclpy#511
Enhanced ros2 topic info to display node name, node namespace, topic type and qos profile of the publishers and subscribers. #385

Thanks!

ivanpauno · 2020-02-27T14:18:36Z

Another run, including the enabled rcl tests:

Linux

@mm318 I see that ros2topic tests are failing in macOS and Windows, though that's happening in CI too.
I will try to investigate what's the problem before merging.

ivanpauno · 2020-02-27T17:27:24Z

@mm318 I see that ros2topic tests are failing in macOS and Windows, though that's happening in CI too.

The faileres are definetly unrelated, I'm merging!

jaisontj force-pushed the jaisontj/enhance_ros2_topic_info branch from 76a106f to dde5dea Compare October 29, 2019 20:28

dabonnie approved these changes Oct 30, 2019

View reviewed changes

jaisontj marked this pull request as ready for review November 6, 2019 18:01

jaisontj mentioned this pull request Nov 6, 2019

Adding required structs and methods to get a list of publishers or subscribers with their respective qos ros2/rmw#186

Merged

jaisontj force-pushed the jaisontj/enhance_ros2_topic_info branch from 5745b57 to 9b262af Compare November 14, 2019 20:04

jaisontj requested a review from dabonnie November 14, 2019 20:05

ivanpauno approved these changes Nov 21, 2019

View reviewed changes

jaisontj requested a review from ivanpauno November 23, 2019 22:02

ivanpauno reviewed Nov 25, 2019

View reviewed changes

ros2topic/ros2topic/verb/info.py Outdated Show resolved Hide resolved

jaisontj requested a review from ivanpauno November 27, 2019 02:04

ivanpauno approved these changes Nov 27, 2019

View reviewed changes

Jconn mentioned this pull request Dec 4, 2019

Use QoS for subscription, recording, and playback ros2/rosbag2#125

Closed

21 tasks

hidmic reviewed Dec 17, 2019

View reviewed changes

jacobperron reviewed Dec 17, 2019

View reviewed changes

ivanpauno mentioned this pull request Dec 18, 2019

Implement functions to get publisher and subcription informations like QoS policies from topic name ros2/rmw_fastrtps#336

Merged

hidmic reviewed Dec 18, 2019

View reviewed changes

mm318 force-pushed the jaisontj/enhance_ros2_topic_info branch from f035b9f to ebdbb10 Compare December 31, 2019 03:51

hidmic approved these changes Jan 2, 2020

View reviewed changes

mm318 mentioned this pull request Jan 14, 2020

Implement functions to get publisher and subcription informations like QoS policies from topic name ros2/rclcpp#960

Merged

wjwwood reviewed Jan 22, 2020

View reviewed changes

ros2topic/ros2topic/verb/info.py Outdated Show resolved Hide resolved

ros2topic/ros2topic/verb/info.py Outdated Show resolved Hide resolved

ros2topic/ros2topic/verb/info.py Show resolved Hide resolved

wjwwood approved these changes Feb 14, 2020

View reviewed changes

ros2topic/ros2topic/verb/info.py Show resolved Hide resolved

ros2topic/ros2topic/verb/info.py Outdated Show resolved Hide resolved

mm318 added 5 commits February 14, 2020 14:53

fix pre-existing unit test failures

73caa5c

Signed-off-by: Miaofei <miaofei@amazon.com>

add test_info_topic_verbose()

4522188

Signed-off-by: Miaofei <miaofei@amazon.com>

rename *topic_info* to *topic_endpoint_info*

3906022

Signed-off-by: Miaofei <miaofei@amazon.com>

make use of TopicEndpointInfo objects

2efa973

Signed-off-by: Miaofei <miaofei@amazon.com>

rename variable to from lineend to line_end

4c6eaec

Signed-off-by: Miaofei <miaofei@amazon.com>

mm318 force-pushed the jaisontj/enhance_ros2_topic_info branch from baed2dc to 4c6eaec Compare February 14, 2020 22:56

wjwwood approved these changes Feb 14, 2020

View reviewed changes

mm318 mentioned this pull request Feb 24, 2020

Implement the rmw_get_publishers/subscriptions_info_by_topic() methods ros2/rmw_connext#391

Merged

mm318 mentioned this pull request Feb 26, 2020

Enable TestInfoByTopicFixture unit tests for more rmw_implementations ros2/rcl#583

Merged

wjwwood approved these changes Feb 26, 2020

View reviewed changes

ivanpauno merged commit c3bf21d into ros2:master Feb 27, 2020

thomas-moulard mentioned this pull request Mar 4, 2020

ROS 2 Foxy Fitzroy ros2/ros2#830

Closed

3 tasks

	help='Prints detailed information like the Node Name, Node Namespace, Topic Type, '
	help='Prints detailed information like the node name, node namespace, topic type, '

Enhanced ros2 topic info to display node name, node namespace, topic type and qos profile of the publishers and subscribers. #385

Enhanced ros2 topic info to display node name, node namespace, topic type and qos profile of the publishers and subscribers. #385

Conversation

jaisontj commented Oct 24, 2019 • edited Loading

jaisontj commented Oct 29, 2019

dabonnie left a comment

Choose a reason for hiding this comment

ivanpauno left a comment

Choose a reason for hiding this comment

ivanpauno left a comment

Choose a reason for hiding this comment

hidmic left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jacobperron commented Dec 17, 2019

ivanpauno commented Dec 18, 2019

hidmic left a comment

Choose a reason for hiding this comment

mm318 commented Dec 19, 2019

mm318 commented Dec 31, 2019

hidmic left a comment

Choose a reason for hiding this comment

mm318 commented Feb 14, 2020

wjwwood commented Feb 14, 2020

mm318 commented Feb 14, 2020 • edited Loading

wjwwood commented Feb 14, 2020

wjwwood left a comment

Choose a reason for hiding this comment

wjwwood commented Feb 14, 2020

wjwwood commented Feb 15, 2020

mm318 commented Feb 15, 2020

ivanpauno commented Feb 17, 2020

mm318 commented Feb 17, 2020

ivanpauno commented Feb 17, 2020 • edited Loading

mm318 commented Feb 18, 2020

ivanpauno commented Feb 18, 2020

mm318 commented Feb 21, 2020

zmichaels11 commented Feb 24, 2020 • edited Loading

dirk-thomas commented Feb 24, 2020

zmichaels11 commented Feb 24, 2020

prajakta-gokhale commented Feb 24, 2020 • edited Loading

ivanpauno commented Feb 26, 2020 • edited Loading

ivanpauno commented Feb 26, 2020

mm318 commented Feb 26, 2020

ivanpauno commented Feb 27, 2020 • edited Loading

ivanpauno commented Feb 27, 2020

jaisontj commented Oct 24, 2019 •

edited

Loading

mm318 commented Feb 14, 2020 •

edited

Loading

ivanpauno commented Feb 17, 2020 •

edited

Loading

zmichaels11 commented Feb 24, 2020 •

edited

Loading

prajakta-gokhale commented Feb 24, 2020 •

edited

Loading

ivanpauno commented Feb 26, 2020 •

edited

Loading

ivanpauno commented Feb 27, 2020 •

edited

Loading