peterhinch / micropython-mqtt

A 'resilient' asynchronous MQTT driver. Recovers from WiFi and broker outages.
MIT License
575 stars 126 forks source link

Asynchronous callbacks #8

Closed DanielIbaseta closed 5 years ago

DanielIbaseta commented 5 years ago

Hello, thank you for the effort put in this library. It allows me to subscribe and manage a wide set of topics, but there is something I've been struggling with as a callback for the MQTT_client can't be asynchronous, so that it cannot contain asynchronous methods. I find this quite strange, as the rest of the library is completely async, so I've made a slight change on it to allow async calls inside the callback, just writing an await in mqtt_as.py when calling the callback function, as seen here:

...
447.    msg = await self._as_read(sz)
448.    await self._cb(topic, msg)
449.    if op & 6 == 2:
...

I don't know if this was on purpose or if it was a mistake that would not bee noticeable in simpler callbacks as in the example provided in the README, but it might be nice to have it in the official code.

kevinkk525 commented 5 years ago

This was on purpose because if you await your callback, you prevent the coroutine from cleaning up the workspace. This means that the next received message will mess up a lot of things. Also in subscriptions with qos 1 the client has to send an acknowledge back which you are blocking until your callback is done. This could lead to the server sending the message again.

The best way to call your asynchronous code would be to just have a callback that adds it to the asyncio loop.

peterhinch commented 5 years ago

Agreed. I'd have something along these lines in your callback:

def cb(my_coro, arg1, arg2)
    loop = asyncio.get_event_loop()
    loop.create_task(my_coro(arg1, arg2))  # Immediate return

That avoids putting a delay where it isn't called for.

DanielIbaseta commented 5 years ago

Thank you both, works like a charm.