MrYsLab / pymata-aio

This is the second generation PyMata client.
https://github.com/MrYsLab/pymata-aio/wiki
GNU Affero General Public License v3.0
155 stars 51 forks source link

Revisit calls to generate reports and other sections that use asyncio.ensure_future #74

Closed MrYsLab closed 6 years ago

MrYsLab commented 6 years ago

The following user provided analysis is correct and the code should be modified to make sure that the methods are not called twice.

In pymata_core.py, starting at line 306 here: https://github.com/MrYsLab/pymata-aio/blob/master/pymata_aio/pymata_core.py

        # get an analog pin map

        asyncio.ensure_future(self.get_analog_map())

        # try to get an analog report. if it comes back as none - shutdown

        report = self.loop.run_until_complete(self.get_analog_map())

It seems like this approach calls get_analog_map() twice. The first time just calls it in passing without waiting for it to complete, whereas the second time actually waits for it to complete. This could be remedied in one of two ways:

        # get an analog pin map

        get_analog_map_task = asyncio.ensure_future(self.get_analog_map())

        # try to get an analog report. if it comes back as none - shutdown

        report = self.loop.run_until_complete(get_analog_map_task)

or more simply (because run_until_complete() wraps a coroutine by ensure_future(), and there is no obvious need to save a reference to the task itself:

        # get an analog pin map. try to get an analog report. if it comes back as none - shutdown

        report = self.loop.run_until_complete(self.get_analog_map())

For a simple example of how the current implementation calls get_analog_map() twice, you can execute this in a python interpreter, which matches the existing call structure:

import asyncio

async def long_operation():

    print('long_operation started')

    await asyncio.sleep(3)

    print('long_operation finished')

    return “done”

loop = asyncio.get_event_loop()

asyncio.ensure_future(long_operation())

report = loop.run_until_complete(long_operation())

Whereas this calls it only once:

import asyncio

async def long_operation():

    print('long_operation started')

    await asyncio.sleep(3)

    print('long_operation finished')

    return “done”

loop = asyncio.get_event_loop()

report = loop.run_until_complete(long_operation())