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 service node #34

Closed
rctoris opened this issue Apr 4, 2013 · 18 comments
Closed

Add service node #34

rctoris opened this issue Apr 4, 2013 · 18 comments

Comments

@rctoris
Copy link
Contributor

rctoris commented Apr 4, 2013

Original discussion started at RobotWebTools/roslibjs#28

Discussion on having rosbridge protocol support advertising services started by @daveconger

Current thoughts on that thread but moved to here to open up more discussion.

@fmessmer
Copy link
Contributor

fmessmer commented Apr 5, 2013

+1 for this issue

For us, the assumption to have only one client advertising a service is absolutely sufficient (for the time being ;-)

Let us know, how we can help?

@dabertram
Copy link
Contributor

I was thinking about this issue..

I'm not sure if i really understand why it shouldn't be possible for the rosbridge server to know to which of it's clients a service request should be sent.
rosbridge clients already get a client id when connecting to rosbridge... so rosbridge server could be just a 'man in the middle' that presents the advertised service to ros and just pass requests to that particular service to the client that is offering it while 'keeping the connection open' and waiting for it's client to send the response..
maybe we would need to use a request id that rosbridge server could add to the request and the service provider would need to send this id back to rosbridge server so the recipient of the service response can be identified by rosbridge...

please correct me if I misunderstood the problem..

@dabertram
Copy link
Contributor

another option should be to implement a service provider that has it's 'own' dedicated connection to the non ros system and just passes requests to that system as well as just passing responses back to ros by answering the service request as if no connection to 'outside' of ros had been made.. this, of course, would not even need to use rosbridge at all.. ;-)

@rctoris
Copy link
Contributor Author

rctoris commented Jun 7, 2013

The issue does not stem from the possibility of having the client advertise a service, but from the implications of allowing them to do so. In particular, if you think of rosbridge as being a server, then multiple clients can be connected at any point (a common use case). If this is true, then if client A advertised /add_two_ints and client two advertises /add_two_ints, it is now unclear where a service request to /add_two_ints should go. Currently, the rosbridge protocol does not send any kind of feedback or error message back to the client to let them know the cannot advertise the service so it would lead to unexpected behavior.

The more I thought about this over the past several weeks the more I agree that this should be brought into rosbridge; however, it should come with a big old warning about the issue above! 😄

@rctoris
Copy link
Contributor Author

rctoris commented Jun 7, 2013

I'll also add that if we want to move down this direction, we can start making suggestions to how the protocol should look in this thread and then merge it into ROSBRIDGE_PROTOCOL.md

@dabertram
Copy link
Contributor

okay..

I just checked what happens when I try to register a service twice from within ros..
I got following output:
"shutdown request: new node registered with same name"

what about using a 'fixed' service that comes with rosbridge to register external services?
this way we would have the possibility to give feedback to clients trying to register a service..

@dabertram
Copy link
Contributor

what about the status messages(3.2)? that would be way better than abusing a service for that functionality..
is currently anyone working on that?

they are marked as experimental..

@rctoris
Copy link
Contributor Author

rctoris commented Jun 7, 2013

I think that would be the correct way to handle it, yeah -- and nope, no one is currently working on that! 👍

@dabertram
Copy link
Contributor

another thing.. I checked again how ros itself behaves when a service is registered several times ..
(forget about "shutdown request: new node registered with same name".. that was because of the same node name ;) )

:$ rosrun beginner_tutorials add_two_ints_server.py &
[1] 2726
:
$ Ready to add two ints. server1
:$ rosrun beginner_tutorials add_two_ints_client.py 1 2
Requesting 1+2
Returning [1 + 2 = 3] server1
1 + 2 = 3
:
$ rosrun beginner_tutorials add_two_ints_server2.py &
[2] 2808
:$ Ready to add two ints. server2
:
$ rosrun beginner_tutorials add_two_ints_client.py 1 3
Requesting 1+3
Returning [1 + 3 = 4] server2
1 + 3 = 4
:$ rosrun beginner_tutorials add_two_ints_server3.py &
[3] 2839
:
$ Ready to add two ints. server3
rosrun beginner_tutorials add_two_ints_client.py 1 4
Requesting 1+4
Returning [1 + 4 = 5] server3
1 + 4 = 5

@dabertram
Copy link
Contributor

I should add that this shows that not even ros itself is giving any feedback about registering an already advertised service.. (above output is from a fresh ros-fuerte installation, not sure if a newer ros version behaves differently)

@fmessmer
Copy link
Contributor

fmessmer commented Jun 8, 2013

Here are some remarks from my side:

  • At least for our application, we can guarantee that there is only one client advertising a service under a certain name. no other client will advertise a service on the same name

ROS directs a service request always to the node (service server) that advertised the service the last.
The new node advertising on the same name does not get notified that the service has already been advertised on the same name by another node
Previous ROS nodes that advertised a service under the same name before do not get notified nor is there any other Warning/Notification in the ROS system.

So, if we want to behave in the same way as ROS does, there is also only one node (rosbridge_client) at a time being the service server for a service. This could be handled by mapping a service name to (the currently responsible) node - i.e. the rosbridgeserver client represented by his client-id.
In that case, we just need to update this mapping in case a new node (rosbridge_client) advertises a service that has already been advertised before. (-> dictionary)
The question is then:
Do we want to notify the involved nodes (previous advertiser, new advertise, rosbridge_server, ros-system, service-clients)? And how should we do this?

If we want to allow only one service server to advertise under a name, we could achieve this by putting the service names in a unique namespace (e.g. /{client-id}/{service-name}). in that way a "add-two-ints"-service that is advertised by two different clients would appear as "/client-1/add-two-ints" and "/client-2/add-two-ints".

To sum up, for our scenario, the "ROS"-way would be sufficient and we also do not need notification as we guarantee to have unique service servers ourselves.

What do you think?

@fmessmer
Copy link
Contributor

fmessmer commented Jun 8, 2013

And then I am wondering whether we would need a new opmode for advertising a service or whether we could use "advertise" that is used to advertise a topic.

It's probably easier to use a new opmode (e.g. "advertise_service") to not mess up with the topic advertisement and to cleanly separate it.

@fmessmer
Copy link
Contributor

fmessmer commented Jun 8, 2013

I also just found that - at least in the ROSBRIDGE_PROTOCOL.md - many-to-many communication with topics is also not supported:
"If the topic already exists with the same type, a warning status message is sent and this message is dropped."
Why is that?

@rctoris
Copy link
Contributor Author

rctoris commented Jun 8, 2013

I like where this is going 👍

I think we should go with what you said/ROS is doing. In the long term, once status messages are implemented, an error/warning should be sent to the client through that, if anything.

As for the OP code, you are correct. We should have a completely new op code instead of overloading the topic one.

Also, I think you found a typo in the ROSBRIDGE_PROTOCOL.md or else I will have to check what the code is actually doing (I was not the original author of ROSBRIDGE_PROTOCOL or the implementation of rosbridge_suite). I think, it should be that if a topic exists with a different type, it will be dropped since topics can not have many types. many-to-many with topics should be supported

@jihoonl
Copy link
Member

jihoonl commented Jun 9, 2013

+1 to @ipa-fxm

And I also think it is a typo in the protocol. It should say that it cannot have multiple type for a single topic. many to many topics should be supported.

@dabertram
Copy link
Contributor

just to let you know:

I wrote some basic classes that allow advertising and calling of services which are provided by "non-ros" clients, connected to ros via rosbridge.
.. still needs some testing and enhancements, but first service calls receive correct answers :-)

will update on this issue

@dabertram
Copy link
Contributor

feel free to follow my code via:
git@github.com:ipa-fxm-db/rosbridge_suite.git

https://github.com/ipa-fxm-db/rosbridge_suite

fmessmer pushed a commit to fmessmer/rosbridge_suite that referenced this issue Aug 19, 2013
service provider does not need to provide module and type within responses
@rctoris
Copy link
Contributor Author

rctoris commented Aug 27, 2013

#54

@rctoris rctoris closed this as completed Aug 27, 2013
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

No branches or pull requests

4 participants