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.

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

add: get clients, servers info #371

Open
wants to merge 2 commits into
base: rolling
Choose a base branch
from

Conversation

leeminju531
Copy link
Contributor

Add get clients, servers info

Refer to ros2/ros2cli#916

@leeminju531
Copy link
Contributor Author

Copy link
Collaborator

@fujitatomoya fujitatomoya left a comment

Choose a reason for hiding this comment

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

I think in rmw service endpoints are handled as topics internally, precisely client is DataReader and server is DataWriter to fill in the topic endpoint information. so all the field of rmw_topic_endpoint_info_t can be filled in with calling the graph cache.

IMO i think in rmw we can use rmw_topic_endpoint_info_t as it is now, but i would add comment that says this structure is used for service client and server as well.

rmw/include/rmw/get_topic_endpoint_info.h Outdated Show resolved Hide resolved
rmw/include/rmw/get_topic_endpoint_info.h Outdated Show resolved Hide resolved
rmw/include/rmw/get_topic_endpoint_info.h Outdated Show resolved Hide resolved
rmw/include/rmw/get_topic_endpoint_info.h Outdated Show resolved Hide resolved
@sloretz
Copy link
Contributor

sloretz commented Jul 12, 2024

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

@methylDragon
Copy link
Contributor

@leeminju531 , any updates from your end?

Signed-off-by: Minju, Lee <dlalswn531@naver.com>
Signed-off-by: Minju, Lee <dlalswn531@naver.com>
@leeminju531 leeminju531 force-pushed the add-clients-servers-info branch from 2c52767 to 3ac5f99 Compare September 27, 2024 14:47
@leeminju531
Copy link
Contributor Author

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

@fujitatomoya
Copy link
Collaborator

@leeminju531 thanks for putting the effort on this.

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

@leeminju531
Copy link
Contributor Author

@fujitatomoya @wjwwood

Sorry for the late response 🙃

Based on ros2/ros2cli#916 (comment), 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;

@fujitatomoya
Copy link
Collaborator

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

@fujitatomoya
Copy link
Collaborator

@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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants