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 #1307

Open
wants to merge 1 commit into
base: rolling
Choose a base branch
from

Conversation

leeminju531
Copy link
Contributor

Add get clients, servers info

Refer to ros2/ros2cli#916

Signed-off-by: Minju, Lee <dlalswn531@naver.com>
@leeminju531
Copy link
Contributor Author

self,
service_name: str,
no_mangle: bool = False
) -> List[TopicEndpointInfo]:
Copy link
Collaborator

Choose a reason for hiding this comment

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

instead we could return ServiceEndpointInfo?

I think we can introduce EndpointInfo base class then TopicEndpointInfo and ServiceEndpointInfo can inherit EndpointInfo base class. this is more appropriate implementation since TopicEndpointInfo for services does not have topic type, topic type hash. this requires the bit of refactoring for classes, but i think that is the right path we can take here.

@clalancette @sloretz what do you think?

name, otherwise it should be a valid ROS service name. Defaults to ``False``.
:return: A list of TopicEndpointInfo for all the clients on this service.
"""
return self._get_info_by_topic(
Copy link
Collaborator

Choose a reason for hiding this comment

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

how about having EndpointTypeEnum instead of TopicEndpointTypeEnum, and this function can take an argument with EndpointTypeEnum, and callback function together? i think this refactoring would be useful when we support the action endpoints in rclpy.

Suggested change
return self._get_info_by_topic(
return self._get_info_by_endpoint(

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.

2 participants