add verbose in service-info verb

[leeminju531]

trafficstars

Verbose option in service info verb

Expected

Dependencies rmw rmw_implementation rmw_fastrtps rmw_cyclonedds rmw_connextdds rcl rclcpp rclpy

Refer to https://github.com/ros2/ros2cli/issues/877#issue-2091143478

[fujitatomoya]

@leeminju531 thanks for taking care of this! i will try to review all incoming PRs.

[fujitatomoya]

@leeminju531

I think that we need to have discussion on the behavior of this verbose option. Currently suggestion is to display all hidden topics based on the service, like rq, rt internal topics as you demonstrated here. although this is precise information, I am not sure if this is good for ROS 2 user perspective since it exposes internal hidden topics instead of services.

instead, i would suggest that it displays as service endpoint for client and server endpoint. with this, we can conceal the hidden topics for ROS 2 users, and provide the information as service endpoint. besides, when we create either service client or server, we can specify only one QoS as service endpoint but 2 underlying hidden topics. that said, your suggested approach displays redundant information. (exact same QoS for 2 hidden topics)

i am now inclined to support this feature as service endpoint but exposing hidden topics. (rmw implementation needs to be aware of hidden topics but rcl or upper layer do not need to know that hidden topics at all.)

CC: @clalancette @sloretz @wjwwood

[leeminju531]

@fujitatomoya

Sorry for coming back too late.

Displaying the service endpoint makes sense, even though DDS handles them internally as topics. By doing this, other non-DDS systems that explicitly support services can also integrate with it more naturally.

However, I wonder if we could define and populate a ServiceEndpoint in the upper RCL layer?

The TopicEndpoint in the upper layer naturally pulls in the underlying TopicEndpoint like this:

/// A data structure that encapsulates the node name, node namespace,
/// topic_type, gid, and qos_profile of publishers and subscriptions
/// for a topic.
typedef struct RMW_PUBLIC_TYPE rmw_topic_endpoint_info_s
{
  /// Name of the node
  const char * node_name;
  /// Namespace of the node
  const char * node_namespace;
  /// The associated topic type's name
  const char * topic_type;
  /// Hashed value for topic type's description
  rosidl_type_hash_t topic_type_hash;
  /// The endpoint type
  rmw_endpoint_type_t endpoint_type;
  /// The GID of the endpoint
  uint8_t endpoint_gid[RMW_GID_STORAGE_SIZE];
  /// QoS profile of the endpoint
  rmw_qos_profile_t qos_profile;
} rmw_topic_endpoint_info_t;

To represent the ServiceEndpoint, though, it seems like we'd need to drop a few fields like topic_type, topic_type_hash, and endpoint_gid.

$ ros2 service info /add_two_ints -v
Type: example_interfaces/srv/AddTwoInts
Clients count: 0

Services count: 1

Node name: add_two_ints_server
Node namespace: /
Endpoint type: SERVER
QoS profile:
  Reliability: RELIABLE
  History (Depth): UNKNOWN
  Durability: VOLATILE
  Lifespan: Infinite
  Deadline: Infinite
  Liveliness: AUTOMATIC
  Liveliness lease duration: Infinite

Does that make sense?

[fujitatomoya]

@leeminju531 or how about landing in the middle? that means include the topic type, topic hash and GID under the ServiceEndpoint? i think these info are already collected in your implementation, and it does not have to be exclusive? for that, we can have general service information from rmw up to client libraries, and underlying topic information as well?

By doing this, other non-DDS systems that explicitly support services can also integrate with it more naturally.

if rmw implementation does not construct the service based on topics, these TopicEndpointInfo is going to be empty. besides, for having non-DDS support for ServiceEndpointInfo would be updated as it developed, i think. (e.g service endpoint itself has unique global identification but topic's.)

[leeminju531]

@fujitatomoya

Could you kindly review all the requests, including ros2/rmw_dds_common#82 and ros2/rmw_zenoh#679, as they involve essential changes to the graph cache?

ref. Here is the output based on DDS (connextdds, cyclone, fastrtps) :

The output based on zenoh :

Thank you for your patience!

[fujitatomoya]

@leeminju531 thanks for coming back on this, i will allocate some time to go through all PRs including new ones.

[ahcorde]

Pulls: ros2/rmw#371, ros2/rmw_implementation#238, ros2/rcl#1161, ros2/rclcpp#2569, ros2/rclpy#1307, ros2/rmw_zenoh#679, ros2/rmw_fastrtps#771, ros2/rmw_cyclonedds#499, ros2/rmw_connextdds#154, ros2/ros2cli#916 Gist: https://gist.githubusercontent.com/ahcorde/302c3652a6220b1c87e62745e9f9afff/raw/742f65fffc9bfec58f851df6062324b9d1a85a3c/ros2.repos BUILD args: --packages-above-and-dependencies rmw rmw_implementation rcl rclcpp rclpy ros2cli ros2service TEST args: --packages-above rmw rmw_implementation rcl rclcpp rclpy ros2cli ros2service ROS Distro: rolling Job: ci_launcher ci_launcher ran: https://ci.ros2.org/job/ci_launcher/16548

• Linux
• Linux-aarch64
• Linux-rhel
• Windows

[leeminju531]

Dear @ahcorde, https://github.com/ros2/rmw_dds_common/pull/82 seems to be missed in all ci jobs.

[ahcorde]

@fujitatomoya friendly ping

[fujitatomoya]

@leeminju531 @ahcorde ah yeah, i have this in my mind. i will try to get back to this.

[fujitatomoya]

@Mergifyio rebase

[mergify[bot]]

rebase

✅ Branch has been successfully rebased

[fujitatomoya]

@leeminju531

for all related PRs including this one, can you remove "all right reserved"? you can see appendix on https://opensource.org/license/apache-2-0

[fujitatomoya]

Pulls: ros2/ros2cli#916, ros2/rmw#371, ros2/rmw_implementation#238, ros2/rmw_fastrtps#771, ros2/rmw_cyclonedds#499, ros2/rmw_connextdds#154, ros2/rcl#1161, ros2/rclcpp#2569, ros2/rclpy#1307, ros2/rmw_dds_common#82, ros2/rmw_zenoh#679 Gist: https://gist.githubusercontent.com/fujitatomoya/01385d70469b48f205a68e21219fd395/raw/b02c84fd17938124e2b045dbf016f260e6209ae0/ros2.repos BUILD args: --packages-above-and-dependencies rmw_zenoh_cpp rmw_dds_common rclpy rclcpp_lifecycle rclcpp rcl rmw_connextdds rmw_connextdds_common rmw_connextddsmicro rmw_cyclonedds_cpp rmw_fastrtps_cpp rmw_fastrtps_dynamic_cpp rmw_fastrtps_sharedcpp ros2cli ros2service rmw rmw_implementation test_rmw_implementation TEST args: --packages-above rmw_zenoh_cpp rmw_dds_common rclpy rclcpp_lifecycle rclcpp rcl rmw_connextdds rmw_connextdds_common rmw_connextddsmicro rmw_cyclonedds_cpp rmw_fastrtps_cpp rmw_fastrtps_dynamic_cpp rmw_fastrtps_sharedcpp ros2cli ros2service rmw rmw_implementation test_rmw_implementation ROS Distro: rolling Job: ci_launcher ci_launcher ran: https://ci.ros2.org/job/ci_launcher/17407

• Linux
• Linux-aarch64
• Linux-rhel
• Windows

[fujitatomoya]

Pulls: ros2/ros2cli#916, ros2/rmw#371, ros2/rmw_implementation#238, ros2/rmw_fastrtps#771, ros2/rmw_cyclonedds#499, ros2/rmw_connextdds#154, ros2/rcl#1161, ros2/rclcpp#2569, ros2/rclpy#1307, ros2/rmw_dds_common#82, ros2/rmw_zenoh#679 Gist: https://gist.githubusercontent.com/fujitatomoya/3c50929d3b3e884ba6796f6c9f26e92e/raw/b02c84fd17938124e2b045dbf016f260e6209ae0/ros2.repos BUILD args: --packages-above-and-dependencies rmw_zenoh_cpp rmw_dds_common rclpy rclcpp_lifecycle rclcpp rcl rmw_connextdds rmw_connextdds_common rmw_connextddsmicro rmw_cyclonedds_cpp rmw_fastrtps_cpp rmw_fastrtps_dynamic_cpp rmw_fastrtps_shared_cpp ros2cli ros2service rmw rmw_implementation test_rmw_implementation TEST args: --packages-above rmw_zenoh_cpp rmw_dds_common rclpy rclcpp_lifecycle rclcpp rcl rmw_connextdds rmw_connextdds_common rmw_connextddsmicro rmw_cyclonedds_cpp rmw_fastrtps_cpp rmw_fastrtps_dynamic_cpp rmw_fastrtps_shared_cpp ros2cli ros2service rmw rmw_implementation test_rmw_implementation ROS Distro: rolling Job: ci_launcher ci_launcher ran: https://ci.ros2.org/job/ci_launcher/17408

• Linux
• Linux-aarch64
• Linux-rhel
• Windows

[fujitatomoya]

CI:

• Linux
• Linux-aarch64
• Linux-rhel
• Windows

[fujitatomoya]

@ahcorde just FYI, i am trying full CI with https://ci.ros2.org/job/ci_launcher/17411/ above.

[fujitatomoya]

@leeminju531 can you check CI failures if those are related to this PR?

[leeminju531]

@fujitatomoya Sure, I'll take a look !

[leeminju531]

I just updated https://github.com/ros2/rmw_connextdds/pull/154 and https://github.com/ros2/rmw_zenoh/pull/679. @fujitatomoya Could you kindly run CI again?

[fujitatomoya]

Full CI:

• Linux
• Linux-aarch64
• Linux-rhel
• Windows

[ahcorde]

• Linux-aarch64
• Linux-rhel

[leeminju531]

@fujitatomoya @ahcorde It looks like the failure isn't related to this. We may need to rerun the CI.

[ahcorde]

• Linux-rhel

[leeminju531]

@ahcorde In the latest CI, the repos_url seems to be different. Could you take a look ? :wink:

[fujitatomoya]

• Linux-rhel

[ahcorde]

• Linux-rhel