-
Notifications
You must be signed in to change notification settings - Fork 69
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 rmw count clients, services #334
Conversation
Signed-off-by: leeminju531 <dlalswn531@naver.com>
Signed-off-by: leeminju531 <dlalswn531@naver.com>
F.Y.I
There is yet no reviewers in rmw_connextdds (ros2/rmw_connextdds#93) |
Wait out for CI |
* This function returns the numbers of servers of a given service in the ROS graph, | ||
* as discovered so far by the given node. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we actually support multiple servers on single service name? i believe that we can create the services on the same name, but how it works is really dependent on implementation. If this is supported, i would expect that it can also support load balancing.
IMO, multiple service servers on single topic is not officially supported. in that case, I am not sure what this is for.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@leeminju531 @methylDragon what do you think?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The ROS2 design says
DDS currently does not have a ratified or implemented standard for request-response style RPC which could be used to implement the concept of services in ROS. There is currently an RPC specification being considered for ratification in the OMG DDS working group, and several of the DDS vendors have a draft implementation of the RPC API.
I think It's seem like none of the specification in this situation. I don't know if some supports like load balancing will be added in the future, As long as most vendors allow multiple servers on the same name, it seems like a necessary function even now.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we actually support multiple servers on single service name? i believe that we can create the services on the same name, but how it works is really dependent on implementation. If this is supported, i would expect that it can also support load balancing.
It's not supported, but you can still create more than one.
I think that the default behavior is that all servers reply, no load balancing.
I'm not completely sure though, and I'm not sure if something breaks in that case.
Being able to get the number of service servers still seems okay.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's not supported, but you can still create more than one.
I think that the default behavior is that all servers reply, no load balancing.
it seems that it depends on implementation, i just checked with Fast-DDS, it sometimes both servers reply, sometimes only one server replies. for doing this, i guess we need to consider more details, like how we can match the service type, how we can choose the server, how to load balance and so on. this could be new feature.
Being able to get the number of service servers still seems okay.
yeah, at least we can create multiple servers on the same service path.
Posting this in here for reference too: ros2/rmw_implementation#208 |
this PR is for service counts using it's name like topic.
Related to ros2/ros2cli#771
Signed-off-by: leeminju531 dlalswn531@naver.com