rmw icon indicating copy to clipboard operation
rmw copied to clipboard

add: get clients, servers info

Open leeminju531 opened this issue 1 year ago • 9 comments
trafficstars

Add get clients, servers info

Refer to https://github.com/ros2/ros2cli/pull/916

leeminju531 avatar Jun 25 '24 13:06 leeminju531

@wjwwood Mind taking a look at this new rmw API 🧇 ?

sloretz avatar Jul 12 '24 18:07 sloretz

@leeminju531 , any updates from your end?

methylDragon avatar Sep 19 '24 06:09 methylDragon

@methylDragon @fujitatomoya, Updated with underlying implementations first:).

leeminju531 avatar Sep 27 '24 14:09 leeminju531

@leeminju531 thanks for putting the effort on this.

i will do the another review once they are ready, so please let me know.

fujitatomoya avatar Sep 27 '24 20:09 fujitatomoya

@fujitatomoya @wjwwood

Sorry for the late response :upside_down_face:

Based on https://github.com/ros2/ros2cli/pull/916#issuecomment-2372588269, we need service information from rmw. I’d like to confirm the structure for representing service information before diving into the implementation details.

I came up with the following structure. Could you take a look and let me know your thoughts?

typedef struct RMW_PUBLIC_TYPE rmw_service_endpoint_info_s
{
  /// Information about an internally used topic for service communication.
  /// For clients, this field contains a DataReader, while for servers, it holds a DataWriter.
  /// Nullptr for systems that explicitly support services without using topics.
  rmw_topic_endpoint_info_t * internal_topic_info;

  /// The name of the node providing the service.
  const char * node_name;

  /// The namespace of the node providing the service.
  const char * node_namespace;

  /// The type name of the associated service.
  const char * service_type;

  /// A hash representing the service type's description.
  /// Nullptr for systems without explicit service support.
  /// This may be populated in future implementations.
  rosidl_type_hash_t * service_type_hash;

  /// Specifies whether the endpoint is a CLIENT or SERVER.
  rmw_endpoint_type_t endpoint_type;

  /// Globally unique identifier (GID) of the endpoint.
  /// Initialized to zeros (0) for systems without explicit service support.
  /// In future implementations, this may be used to uniquely identify the endpoint.
  uint8_t endpoint_gid[RMW_GID_STORAGE_SIZE];

  /// The QoS profile applied to the service's communication.
  /// This is typically shared for both request and response messages
  /// as they often use the same communication characteristics.
  rmw_qos_profile_t qos_profile;
} rmw_service_endpoint_info_t;

leeminju531 avatar Dec 31 '24 07:12 leeminju531

@leeminju531 glad you came back :smiley: let me have some time, i will bring this up to next PMC meeting.

fujitatomoya avatar Jan 09 '25 20:01 fujitatomoya

@leeminju531 we had a discussion on the information from ros2 service info -v and rmw data structure for this. as in rmw, service information structure should not be dependent implementation details, but generic for all implementations. i think we can collect more information from rmw_zenoh what we can do here. and it is expected that service endpoint should have service type hash (not topic type hashes): this one i am not sure if this is implemented as expected... i thought there are only topic type hash is supported, maybe i was wrong on this.

fujitatomoya avatar Jan 14 '25 17:01 fujitatomoya

@fujitatomoya Thanks for reviewing this!

Here’s the revised rmw_service_endpoint_info_t structure:

typedef struct RMW_PUBLIC_TYPE rmw_service_endpoint_info_s
{
  /// The name of the node providing the service.
  const char * node_name;

  /// The namespace of the node providing the service.
  const char * node_namespace;

  /// The type name of the associated service.
  const char * service_type;

  /// A hash representing the service type's description.
  rosidl_type_hash_t * service_type_hash;

  /// Specifies whether the endpoint is a CLIENT or SERVER.
  rmw_endpoint_type_t endpoint_type;

  /// The Globally Unique Identifier (GID) of the endpoint.
  /// For systems without explicit service support, this retrieves the Writer's GID.
  uint8_t endpoint_gid[RMW_GID_STORAGE_SIZE];

  /// The Quality of Service (QoS) profile applied to the service's communication.
  /// This is typically shared between the request and response messages as they often use the same communication settings.
  rmw_qos_profile_t qos_profile;
} rmw_service_endpoint_info_t;

Modifications:

  • Removed internal_topic_info to ensure the structure remains generic across all implementations.
  • Updated the comment for service_type_hash to clarify that it should be retrieved regardless of the middleware.
  • Changed endpoint_gid handling: Instead of initializing to zero in DDS-based systems (without explicit service support), it implicitly returns the Writer's GID. This aligns better with how service introspection notifies users of each client and server GID.

One thing I’m still unsure about is the qos_profile. Right now, it assumes that the reader and writer share the same QoS profile, but is a single qos_profile sufficient?

PS: I’ll be more available in two months and can dive into the details then.

leeminju531 avatar Feb 09 '25 14:02 leeminju531

Dear @fujitatomoya , @wjwwood

I implemented a quick prototype based on the definition provided above. image

The definition appears suitable for middleware that explicitly support services, such as Zenoh. However, in DDS, several challenges aries :

  1. Service Type Hash Representation : Representing the service type hash is problematic. DDS internally addresses services as topics, making it challenging to collect the service type hash within the graph cache. Currently, it seems feasible to retrieve only the topic type hash, not the service type hash.

  2. GID and QoS Profile Representation: representing the GID and QoS profile is ambiguous. Since DDS uses two GIDs (READER and WRITER) to represent a single service, there is no unique GID for the service itself. Additionally, when using two topics for one service, both aim to write the same QoS profile. However, as far as I know, the middleware will attempt to adapt to the QoS configuration as best it can. Therefore, representing just one QoS profile may not always be accurate.

After further consideration, I revised the structure as follows:

typedef struct RMW_PUBLIC_TYPE rmw_service_endpoint_info_s
{
/// Name of the node
const char * node_name;
/// Namespace of the node
const char * node_namespace;
/// The associated service type's name
const char * service_type;
/// The endpoint type. (CLIENT or SERVER)
rmw_endpoint_type_t endpoint_type;
/// Specifies whether the middleware explicitly supports services (e.g., Zenoh).
/// The `endpoint_count` value is determined as follows:
/// - 1 if the middleware explicitly supports services.
/// - 2 if request/response are represented as separate reader/writer topics.
size_t endpoint_count;
/// Hashed value for service type's description
rosidl_type_hash_t * service_type_hashes;
/// The GID of the endpoint
uint8_t (*endpoint_gids)[RMW_GID_STORAGE_SIZE];
/// QoS profile of the endpoint
rmw_qos_profile_t * qos_profiles;
} rmw_service_endpoint_info_t;

In this revised structure, I introduced the endpoint_count field. For middleware that explicitly supports services, it would be set to 1, while for those that do not (e.g., using separate topics for requests and responses like DDS), it would be 2.

Here’s the output when using Zenoh (an explicit service middleware): image

If the middleware does not support explicit services, the output would look like this:

Node name: minimal_service
Node namespace: /
Service type: example_interfaces/srv/AddTwoInts
Service type hashes: 
 - RIHS01_e118de6bf5eeb66a2491b5bda11202e7b68f198d6f67922cf30364858239c81a
 - RIHS01_e118de6bf5eeb66a2491b5bda11202e7b68f198d6f67922cf30364858239c81b
Endpoint type: SERVER
Endpoint count: 2
GIDs: 
 - c1.e9.6a.12.53.b3.5e.28.c7.25.0b.30.36.53.8a.ad
 - c1.e9.6a.12.53.b3.5e.28.c7.25.0b.30.36.53.8a.ae
QoS profiles:
 - Reliability: RELIABLE
   History (Depth): KEEP_LAST (10)
   Durability: VOLATILE
   Lifespan: Infinite
   Deadline: Infinite
   Liveliness: AUTOMATIC
   Liveliness lease duration: Infinite
 - Reliability: RELIABLE
   History (Depth): KEEP_LAST (10)
   Durability: VOLATILE
   Lifespan: Infinite
   Deadline: Infinite
   Liveliness: AUTOMATIC
   Liveliness lease duration: Infinite

Please let me know if you need further adjustments or improvements. I would greatly appreciate any feedback or suggestions to ensure it is as effective as possible. Thanks!

leeminju531 avatar Mar 29 '25 16:03 leeminju531

@leeminju531 thanks for the update and checking in details. images are broken, especially for zenoh one, could you attach it again?

fujitatomoya avatar Apr 02 '25 05:04 fujitatomoya

DDS internally addresses services as topics, making it challenging to collect the service type hash within the graph cache. Currently, it seems feasible to retrieve only the topic type hash, not the service type hash.

i think service type needs to be collected from service typesupport, and that is what rmw_zenoh does with graph_services_. probably DDS-based rmw implementation also needs to have graph extension for services, service type hash needs to be consistent between all rmw implementations? because topic type hash is not the service type hash generated by rosidl, and that is the wrong information for user.

do you happen to have any thought how to manage service type hash in the graph for DDS rmws?

GID and QoS Profile Representation: representing the GID and QoS profile is ambiguous. Since DDS uses two GIDs (READER and WRITER) to represent a single service, there is no unique GID for the service itself. Additionally, when using two topics for one service, both aim to write the same QoS profile. However, as far as I know, the middleware will attempt to adapt to the QoS configuration as best it can. Therefore, representing just one QoS profile may not always be accurate.

this is true. Requested QoS and actual one can be different, and there is no unique service entity GID... having endpoint_count in rmw_service_endpoint_info_t looks good to me, but maybe it should print which one is reader or writer as output?

fujitatomoya avatar Apr 03 '25 01:04 fujitatomoya

this is true. Requested QoS and actual one can be different, and there is no unique service entity GID... having endpoint_count in rmw_service_endpoint_info_t looks good to me, but maybe it should print which one is reader or writer as output?

To handle READER/WRITER distinction, I propose we implicitly guide the interpretation by defining the order clearly in the comments.

typedef struct RMW_PUBLIC_TYPE rmw_service_endpoint_info_s
{
...
/// GIDs of the endpoints.
/// If `endpoint_count` is 2, ordered as [REQUEST_READER, RESPONSE_WRITER].
uint8_t (*endpoint_gids)[RMW_GID_STORAGE_SIZE];
/// QoS profile of the endpoint
/// If `endpoint_count` is 2, ordered as [REQUEST_READER, RESPONSE_WRITER].
rmw_qos_profile_t * qos_profiles;
...
} rmw_service_endpoint_info_t;

Then in rclpy or rclcpp, we can explicitly print or represent which is READER or WRITER by reading the order. This keeps the RMW layer generic while still providing the necessary info for higher-level tools.

i think service type needs to be collected from service typesupport, and that is what rmw_zenoh does with graph_services_. probably DDS-based rmw implementation also needs to have graph extension for services, service type hash needs to be consistent between all rmw implementations? because topic type hash is not the service type hash generated by rosidl, and that is the wrong information for user.

do you happen to have any thought how to manage service type hash in the graph for DDS rmws?

Yeah, I agree. I’ll look into it, but it’d be super helpful if someone could share some guidance on how this should be handled properly.

leeminju531 avatar Apr 20 '25 15:04 leeminju531

hmmm, yeah this is hard to come up with good solution. i mean we can add service type hash and graph to the DDS-based rmw implementation like rmw_zenoh, but it always comes back to QoS problem...

fujitatomoya avatar Apr 21 '25 17:04 fujitatomoya

@leeminju531 so we talked about this at ROS PMC once again.

  • (we already know) single service type hash needs to be printed with ros2 service info -v on service. that means we need to add service type hash identification from ROS 2 to graph cache in DDS based rmw implementations. (I am not sure what the best way to do this, but let's discuss on that together.)
  • showing 2 QoS (maybe print one QoS if they are identical) and GIDs with endpoint_count is fine, but probably we do not need to print internal topic type hash identifications since those are hidden behind RMW implementation details.
  • we can show 2 QoS and GID with DDS based RMWs, but actually there is no way for user to specify 2 QoS configurations for the service. we only can set QoS for either service client or server, we cannot set the different QoS to request topic and response topic separately. but this does not mean we should not print what we can show as information.

what do you think?

fujitatomoya avatar Apr 22 '25 18:04 fujitatomoya

This pull request has been mentioned on ROS Discourse. There might be relevant details there:

https://discourse.ros.org/t/ros-pmc-minutes-for-april-22-2025/43346/1

ros-discourse avatar Apr 22 '25 18:04 ros-discourse

@fujitatomoya , Thanks for bringing this up again.

As far as I understand, the previous definition is generally acceptable, except for internal topic type hashes. I now understand that showing the topic type hash is part of the implementation details—thanks for clarifying that.

Additionally, we could explore a cleaner or more user-friendly way to display this information in ros2cli at the rclpy layer like this.

Right now, the challenge I’m facing is retrieving the service_type_hash from the graph cache. It's more complicated than I expected. I’m still looking into it and digging deeper. Once I figure it out, I’ll follow up!

leeminju531 avatar May 06 '25 13:05 leeminju531

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 Build Status
  • Linux-aarch64 Build Status
  • Linux-rhel Build Status
  • Windows Build Status

ahcorde avatar Jul 21 '25 11:07 ahcorde

@Mergifyio rebase

fujitatomoya avatar Nov 06 '25 09:11 fujitatomoya

rebase

✅ Branch has been successfully rebased

mergify[bot] avatar Nov 06 '25 09:11 mergify[bot]

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 Build Status
  • Linux-aarch64 Build Status
  • Linux-rhel Build Status
  • Windows Build Status

fujitatomoya avatar Nov 06 '25 11:11 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 Build Status
  • Linux-aarch64 Build Status
  • Linux-rhel Build Status
  • Windows Build Status

fujitatomoya avatar Nov 06 '25 11:11 fujitatomoya

  • Linux Build Status
  • Linux-aarch64 Build Status
  • Linux-rhel Build Status
  • Windows Build Status

ahcorde avatar Nov 07 '25 12:11 ahcorde

@ahcorde @fujitatomoya the CI result above shows test failures for test_rmw_implementation.TestGraphAPI.get_servers_info_by_service_with_bad_arguments. Are any further changes needed?

Yadunund avatar Nov 12 '25 06:11 Yadunund

@Yadunund, Those failures were fixed at https://github.com/ros2/ros2cli/pull/916#issuecomment-3509406823.

leeminju531 avatar Nov 12 '25 07:11 leeminju531

@Yadunund besides that, we are verifying this with full CI on https://github.com/ros2/ros2cli/pull/916#issuecomment-3510999380

fujitatomoya avatar Nov 12 '25 11:11 fujitatomoya