locusrobotics / aiorospy

asyncio wrapper for rospy
MIT License
40 stars 8 forks source link

Use functools.partial in service.send #22

Closed brean closed 4 years ago

brean commented 5 years ago

Hi,

I am using aiorospy to communicate with rosplan. To update my ROSPlan knowledge base I need to send a KnowledgeItem with some parameters. So I created my own send-function that allows parameters on the service call:

import rospy
from std_srvs.srv import Empty
from diagnostic_msgs.msg import KeyValue
from rosplan_dispatch_msgs.msg import ActionDispatch, ActionFeedback
from rosplan_dispatch_msgs.srv import DispatchService
from rosplan_knowledge_msgs.msg import KnowledgeItem
from rosplan_knowledge_msgs.srv import KnowledgeUpdateService
from rosplan_knowledge_msgs.srv import KnowledgeUpdateServiceRequest
import functools
import aiorospy
from aiorospy.helpers import log_during

KB_UPDATE = '/rosplan_knowledge_base/update'

class MyAsyncServiceProxy(aiorospy.AsyncServiceProxy):
    async def send(self, log_period=None, *args, **kwargs):
        """ Send a request to a ROS service. """
        return await log_during(
            self._loop.run_in_executor(
                None, functools.partial(self._srv_proxy.call, *args, **kwargs)),
            f"Trying to call service...", log_period)

class KnowledgeBase(object):
    def __init__(self):
        self.knowledge_service = MyAsyncServiceProxy(
            KB_UPDATE, KnowledgeUpdateService)

    async def update_knowledge(
            self, type, update_type, attribute_name='', values=None,
            instance_type='', instance_name=''):
        if values:
            if isinstance(values, dict):
                values = values.items()
            values = [KeyValue(key=f'?{k}', value=v) for k, v in values]
        else:
            values = []
        msg = KnowledgeItem(
            knowledge_type=type,
            instance_type=instance_type,
            instance_name=instance_name,
            attribute_name=attribute_name,
            values=values
        )
        response = await self.knowledge_service.ensure(
            update_type=update_type, knowledge=msg)
        return response.success

Based on https://github.com/locusrobotics/aiorospy/blob/master/aiorospy/src/aiorospy/service.py#L33

I think it was intended to call the parameters using functools.partial on the service as I am doing it in MyAsyncServiceProxy but I am not providing a PR because I am not sure if there is a case where you need to call these parameter on run_in_executor?

paulbovbel commented 5 years ago

I think it was intended to call the parameters using functools.partial on the service as I am doing it in MyAsyncServiceProxy

Turns out run_in_executor takes *args, but not **kwargs (https://docs.python.org/3/library/asyncio-eventloop.html#asyncio.loop.run_in_executor). That's really unfortunate, and I wish I had caught this in a test. A PR to wrap the send args/kwargs in partial would be very appreciated!