Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

print history and depth via topic info -v #849

Merged
merged 2 commits into from
Dec 22, 2021
Merged

print history and depth via topic info -v #849

merged 2 commits into from
Dec 22, 2021

Conversation

fujitatomoya
Copy link
Collaborator

address ros2/ros2cli#667

Signed-off-by: Tomoya.Fujita Tomoya.Fujita@sony.com

@fujitatomoya
Copy link
Collaborator Author

How it displays.

root@a6cfc30c581b:~/docker_ws/ros2_colcon# ros2 run demo_nodes_cpp talker
[INFO] [1636831476.771070892] [talker]: Publishing: 'Hello World: 1'

root@a6cfc30c581b:~/docker_ws/ros2_colcon# ros2 topic info -v /chatter
Type: std_msgs/msg/String

Publisher count: 1

Node name: talker
Node namespace: /
Topic type: std_msgs/msg/String
Endpoint type: PUBLISHER
GID: 01.10.5a.ea.44.3c.29.8e.ee.fa.4b.9a.00.00.15.03.00.00.00.00.00.00.00.00
QoS profile:
  Reliability: RELIABLE
  History (Depth): KEEP_LAST (7)  !!! HERE !!!
  Durability: VOLATILE
  Lifespan: Infinite
  Deadline: Infinite
  Liveliness: AUTOMATIC
  Liveliness lease duration: Infinite

Subscription count: 0

root@a6cfc30c581b:~/docker_ws/ros2_colcon# ros2 run demo_nodes_cpp listener
^C[INFO] [1636831516.595457482] [rclcpp]: signal_handler(signum=2)

root@a6cfc30c581b:~/docker_ws/ros2_colcon# ros2 topic info -v /chatter
Type: std_msgs/msg/String

Publisher count: 0

Subscription count: 1

Node name: listener
Node namespace: /
Topic type: std_msgs/msg/String
Endpoint type: SUBSCRIPTION
GID: 01.10.d3.2b.f9.d3.d8.b5.32.38.05.68.00.00.15.04.00.00.00.00.00.00.00.00
QoS profile:
  Reliability: RELIABLE
  History (Depth): KEEP_LAST (10) !!! HERE !!!
  Durability: VOLATILE
  Lifespan: Infinite
  Deadline: Infinite
  Liveliness: AUTOMATIC
  Liveliness lease duration: Infinite

@fujitatomoya fujitatomoya mentioned this pull request Nov 13, 2021
Open
@fujitatomoya
Copy link
Collaborator Author

@clalancette @ivanpauno minor extension to print topic info verbose. could you help review when you got time?

aprotyas
aprotyas reviewed Nov 13, 2021
View reviewed changes
Copy link
Member

@aprotyas aprotyas left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM but looks like you'll have to add the history information to the expected test output in test_topic_endpoint_info.

matthews-jca
matthews-jca approved these changes Nov 15, 2021
View reviewed changes
Copy link

@matthews-jca matthews-jca left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is exactly what I was picturing. Agreed with other comment on checking test output. LGTM

ivanpauno
ivanpauno requested changes Nov 15, 2021
View reviewed changes
rclpy/rclpy/topic_endpoint_info.py Outdated Show resolved Hide resolved
@fujitatomoya
Copy link
Collaborator Author

I did not know #849 (comment), and it is clearly described that in rmw interface,

https://github.com/ros2/rmw/blob/35fc6ab8fad4db90eb55db9d1ecf50dc1aa3638d/rmw/include/rmw/get_topic_endpoint_info.h#L39-L40

 * From the current QoS settings available, the only ones not shared by DDS based
 * implementations are `history` and `history_depth`.

thinking about using different rmw implementation, i am not inclined to print history and history_depth at this moment to avoid the confusion...

i'd like to close this PR if there is no denial or suggestions.

@ivanpauno
Copy link
Member

i'd like to close this PR if there is no denial or suggestions.

My suggestion is to fix rmw implementations so they return history = UNKNOWN when they cannot provide that information, instead of returning sth that's wrong. rmw API docs actually require this.

With that solved, history and history depth could be printed conditionally (if history != UNKNOWN: ...).

@matthews-jca
Copy link

My suggestion is to fix rmw implementations so they return history = UNKNOWN when they cannot provide that information, instead of returning sth that's wrong. rmw API docs actually require this.

With that solved, history and history depth could be printed conditionally (if history != UNKNOWN: ...).

This makes sense to me. Development of large systems with cyclone has been a real pain without being able to inspect the current state of the system. Our original drive from FastDDS to Cyclone under foxy was actually just to get diagnostic output when we had QOS mismatches

@ivanpauno
Copy link
Member

@fujitatomoya @matthews-jca are you planning to contribute the missing pieces in the rmw implementations (see above comment) to make this PR possible?

If there's no plan to do that in a near future, we can close this PR and reopen it when the underlying issue is solved.

@ivanpauno ivanpauno added the help wanted Extra attention is needed label Dec 3, 2021
@ivanpauno ivanpauno removed their assignment Dec 3, 2021
@fujitatomoya
Copy link
Collaborator Author

@ivanpauno i will try to address in next week! thanks for the ping 👍

@fujitatomoya
print history and depth via topic info -v
c72b859 
Signed-off-by: Tomoya.Fujita <Tomoya.Fujita@sony.com>
@fujitatomoya
print QoS history and depth only if available.
cddbecd 
Signed-off-by: Tomoya Fujita <Tomoya.Fujita@sony.com>
@fujitatomoya
Copy link
Collaborator Author

Verification note,

  • Cyclonedds

rmw_cyclonedds_cpp sets history and depth accordingly if available.

https://github.com/ros2/rmw_cyclonedds/blob/287e781937f42570d4c7a14ed24e201c528c20af/rmw_cyclonedds_cpp/src/rmw_node.cpp#L626-L636

# RMW_IMPLEMENTATION=rmw_cyclonedds_cpp ros2 run demo_nodes_cpp talker
...

# RMW_IMPLEMENTATION=rmw_cyclonedds_cpp ros2 topic info -v /chatter --no-daemon
Type: std_msgs/msg/String

Publisher count: 1

Node name: talker
Node namespace: /
Topic type: std_msgs/msg/String
Endpoint type: PUBLISHER
GID: 01.10.2c.a6.c6.2e.e4.6e.be.dc.75.d6.00.00.15.03.00.00.00.00.00.00.00.00
QoS profile:
  Reliability: RELIABLE
  History (Depth): KEEP_LAST (7)
  Durability: VOLATILE
  Lifespan: Infinite
  Deadline: Infinite
  Liveliness: AUTOMATIC
  Liveliness lease duration: Infinite

Subscription count: 0
  • Fast-DDS Publisher

it does not set QoS history nor depth during endpoint discovery.
rmw_qos_profile_unknown is used for initialization so it will be QoSHistoryPolicy.UNKNOWN.

https://github.com/ros2/rmw_fastrtps/blob/c455bc3498d587b973a5bd64c11d3c78cde2ec67/rmw_fastrtps_shared_cpp/include/rmw_fastrtps_shared_cpp/custom_participant_info.hpp#L149-L162

# RMW_IMPLEMENTATION=rmw_fastrtps_cpp ros2 run demo_nodes_cpp talker
...

# RMW_IMPLEMENTATION=rmw_fastrtps_cpp ros2 topic info -v /chatter --no-daemon
Type: std_msgs/msg/String

Publisher count: 1

Node name: talker
Node namespace: /
Topic type: std_msgs/msg/String
Endpoint type: PUBLISHER
GID: 01.0f.64.e2.c3.73.a8.65.01.00.00.00.00.00.12.03.00.00.00.00.00.00.00.00
QoS profile:
  Reliability: RELIABLE
  History (Depth): UNKNOWN
  Durability: VOLATILE
  Lifespan: Infinite
  Deadline: Infinite
  Liveliness: AUTOMATIC
  Liveliness lease duration: Infinite

Subscription count: 0
  • RTI Connextdds

it tries to set history nor depth if available, but it seems that history is unavailable.

https://github.com/ros2/rmw_connextdds/blob/2e328403e27a81532e74a8f3ec73e1e71c0b00c8/rmw_connextdds_common/src/common/rmw_graph.cpp#L745-L760

# RMW_IMPLEMENTATION=rmw_connextdds ros2 run demo_nodes_cpp talker
...

# RMW_IMPLEMENTATION=rmw_connextdds ros2 topic info -v /chatter --no-daemon
RTI Data Distribution Service Non-commercial license is for academic, research, evaluation and personal use only. USE FOR COMMERCIAL PURPOSES IS PROHIBITED. See RTI_LICENSE.TXT for terms. Download free tools at rti.com/ncl. License issued to Non-Commercial User license@rti.com For non-production use only.
Expires on 00-jan-00 See www.rti.com for more information.
Type: std_msgs/msg/String

Publisher count: 1

Node name: talker
Node namespace: /
Topic type: std_msgs/msg/String
Endpoint type: PUBLISHER
GID: 01.01.9e.b8.ec.96.a9.9f.5f.a3.c2.8b.80.01.29.03.00.00.00.00.00.00.00.00
QoS profile:
  Reliability: RELIABLE
  History (Depth): UNKNOWN
  Durability: VOLATILE
  Lifespan: Infinite
  Deadline: Infinite
  Liveliness: AUTOMATIC
  Liveliness lease duration: Infinite

Subscription count: 0

@fujitatomoya fujitatomoya mentioned this pull request Dec 8, 2021
Merged
@fujitatomoya
Copy link
Collaborator Author

CI:

  • Linux Build Status
  • Linux-aarch64 Build Status
  • macOS Build Status
  • Windows Build Status

audrow
audrow approved these changes Dec 22, 2021
View reviewed changes
Copy link
Member

@audrow audrow left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me

@fujitatomoya fujitatomoya merged commit f2eb216 into ros2:master Dec 22, 2021
@fujitatomoya fujitatomoya mentioned this pull request Dec 27, 2021
Merged
@fujitatomoya fujitatomoya mentioned this pull request Apr 7, 2023
Open
@clalancette clalancette mentioned this pull request Jul 4, 2023
Open
@jelly-FF jelly-FF mentioned this pull request Jul 7, 2023
Closed
1 task
@fujitatomoya fujitatomoya mentioned this pull request Oct 6, 2023
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@aprotyas aprotyas aprotyas left review comments

@ivanpauno ivanpauno ivanpauno requested changes

@matthews-jca matthews-jca matthews-jca approved these changes

@audrow audrow audrow approved these changes

Assignees
No one assigned
Labels
help wanted Extra attention is needed
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@fujitatomoya @ivanpauno @matthews-jca @audrow @aprotyas