gramaziokohler / roslibpy

Python ROS Bridge library
MIT License
277 stars 57 forks source link

roslibpy service call hangs when executing another service call in callback #68

Closed brean closed 3 years ago

brean commented 3 years ago

Hi,

I am trying to build a bridge between a ros instance in the cloud and a second ros instance on a robot. I want to call a service from the robot that is announced by roslibpy, so roslibpy can call the service on the cloud instance and send its result back to the robot. On both ros instance I run the rosbridge_websocket but on different ports. To make things simple both instances have access to the custom_service-message types and services, so the robot can use the CustomMessage.msg and CustomService.srv-files from the service without me having to create any type conversions.

Expected Result

I should get the result from the service on the robot. This works if I execute "service.call" outside of the handler-function that is the callback of the service. So this would work, but it calls the service directly and does not wait for the request from the robot:

        cloud_service = roslibpy.Service(cloud, name, service_type)
        robot_service = roslibpy.Service(robot, name, service_type)
        request = roslibpy.ServiceRequest()
        result = cloud_service.call(request).data
        def handler(request, response):
            logger.info(f'Result from service: {result}')
            for key, value in result.items():
                response[key] = value
            return True
        robot_service.advertise(handler)

Actual Result

The service hangs when I call the service inside the handler:

        cloud_service = roslibpy.Service(cloud, name, service_type)
        robot_service = roslibpy.Service(robot, name, service_type)
        def handler(request, response):
            request = roslibpy.ServiceRequest()
            # the next logger.info never gets called and the code just hangs here.
            result = cloud_service.call(request).data
            logger.info(f'Result from service: {result}')
            for key, value in result.items():
                response[key] = value
            return True
        robot_service.advertise(handler)

I would expect this to also return the reponse message from the service, for example:

rosservice_1  | [INFO] [1606834419.250682]: service received request: 
robot_1       | 2020-12-01 14:53:39+0000 [-] [INFO] [1606834419.436973]: [Client 0] Advertised service /my_service.
robot_1       | [INFO] [1606834419.591722]: service ready, creating proxy!
bridge_1      | [INFO] ros_bridge: Result from service: {'msgs': [{'name': 'a', 'number': 12.34000015258789}, {'name': 'b', 'number': 3.1414999961853027}]}
robot_1       | [INFO] [1606834419.612157]: service response: msgs: 
robot_1       |   - 
robot_1       |     name: "a"
robot_1       |     number: 12.3400001526
robot_1       |   - 
robot_1       |     name: "b"
robot_1       |     number: 3.14149999619
robot_1       | [INFO] [1606834419.616236]: received data from service: {'a': 12.34000015258789, 'b': 3.1414999961853027}

Reproduction Steps

Run the docker-container at https://github.com/brean/roslibpy_bridge_test/ using

docker-compose build && docker-compose up

System Information

Using ROS melodic and Python 3 (see https://github.com/brean/roslibpy_bridge_test/blob/main/docker-compose.yml)

gonzalocasas commented 3 years ago

@brean i guess this is blocking twisted's event loop. As a quick option, how about trying the async service calls instead: https://roslibpy.readthedocs.io/en/latest/reference/index.html#roslibpy.Service.call? ie pass a callback and errback to cloud_service.call(...). The tricky part will be to resolve how to give back control from the handler, while being able to get it back to return the async result of the cloud service. We might need to add something, like, the ability to create a service handler that returns a generator instead of a one-off return value.

brean commented 3 years ago

The tricky part will be to resolve how to give back control from the handler, while being able to get it back to return the async result of the cloud service. We might need to add something, like, the ability to create a service handler that returns a generator instead of a one-off return value.

When I use a callback for the call it gets executed after the response has been send to the robot. So the robot gets an empty response. Like you wrote, we need something that handles this case and waits with the return from the handler. I am currently looking into the twisted documentation...

brean commented 3 years ago

It is NOT the twisted event loop that is blocking!

The problem is that in twisted.internet.thread the blockingCallFromThread-function executes a queue.get() which blocks. It should block until there is data in the queue but the callback-function (_callFromThread) that gets handed to reactor.callFromThread never gets executed. I don't know twisted well enough to say why the callback function never executes, but when I change the queue.get to a non-blocking call and just return an empty result the code runs... except that I want the result from the service instead of my empty of course, so any ideas?

brean commented 3 years ago

I found a workaround, I split the _service_response_handler into 2 functions, so after self._service_callback gets executed I exit the function and in my handler I have a callback-function for my service call in which I execute this new after_service_response. This is ugly because:

  1. I need to call the function from within my callback manually
  2. It breaks the current API.

But it is working...

class AsyncServiceWorkaround(roslibpy.Service):
    def _service_response_handler(self, request):
        response = roslibpy.ServiceResponse()
        def done(success):
            call = roslibpy.Message({'op': 'service_response',
                            'service': self.name,
                            'values': dict(response),
                            'result': success
                            })

            if 'id' in request:
                call['id'] = request['id']

            self.ros.send_on_ready(call)
        self._service_callback(request['args'], response, done)

[...]

    cloud_service = roslibpy.Service(cloud, name, service_type)
    robot_service = AsyncServiceWorkaround(robot, name, service_type)

    def handler(request, response, done):
        cloud_request = roslibpy.ServiceRequest()
        def cloud_cb(result):
            for key, value in result.items():
                response[key] = value
            done(True)
        cloud_service.call(cloud_request, callback=cloud_cb)

    robot_service.advertise(handler)
beverlylytle commented 3 years ago

In the latest version of roslibpy there is a bug fix which I think addresses this issue. I was able to run the non-workaround version (after modifying the ros/Dockerfile with suggestions from https://answers.ros.org/question/325039/apt-update-fails-cannot-install-pkgs-key-not-working/) and got the expected results.

rcywongaa commented 1 year ago

I don't think this issue has been fixed. Using @brean 's example and the updated apt keys from @beverlylytle , the same problem persists. The workaround is still required to have the example working.

I'd open a new issue, but there's no new information I can provide besides that provided here...

Ref: The bug fix that should have fixed this issue is https://github.com/gramaziokohler/roslibpy/compare/v1.2.0...v1.2.1