mqtt_as: Added Async Callback functionality.

[TrevisGordan]

_Hello Peter, I have two additions to mqttas.py. I don't know how you like your pull request, so I made the logical changes separately. And would then in case you like them, make a third one - combining them.

Added Async Callback functionality.

I added the functionality to call an Async Callback function. I actually recycled your code from your "aswitch" library so you should be familiar with the launch functionality.

On top of that, I have a problem, which I will address separately in an issue.

[peterhinch]

This can be done without altering the library with a callback function that launches a coro.

def callback():
    loop = asyncio.get_event_loop()
    loop.create_task(my_coro())

[TrevisGordan]

Sure, but this would be more kind of a workaround.

In some use cases, the callback function acts like a callback_handler. Since you can subscribe and receive messages on multiple different topics: ("if Topic A do this / if Topic B do that"). Since all subscribed topics share one callback function. (Sure, I thought about giving every subscribed topic its own callback but a single callback as a handler is cleaner.)

• You would not be able to call other async functions (with await) directly, nor work with Events or other asyn functionalities. - Even publishing a message from inside the callback would not be possible.

• Especially: If you somehow receive too many callbacks, it would create a loop task every time -possibly- eventually leading to an IndexError: queue overflow. (if these tasks are never completed, the handler will always be completed)

The change would come with no changes to the API, since the launch function works automatically.

[kevinkk525]

You proposal isn't any better then your 2nd bullet point.

The synchronous callback makes more sense here as launching a new async task for every new message could easily cause a queue overflow without the possibility to prevent it other than making those async callbacks exit quickly (but then you could have gone for the synchronous callback). Well that's your 2nd bullet point.

With the sync callback you have your message handler you have the control over how many async tasks you want to create, flow control or store new messages until previous ones are processed. It is a lot cleaner and the better way to do it. Launching a new coroutine for connected_coro is already a big enough compromise in my opinion as that coro is usually used for subscribing topics again after a reconnect. This takes time and if a reconnect happens within the resubscribing process, then you can easily get multiple coros if you are in a bad wifi.

[peterhinch]

Quite. @TrevisGordan if you look at the code for launch it is logically identical to my callback suggestion above. Both suffer exactly the same potential drawbacks identified by @kevinkk525.

I think the code should stand: it encourages a synchronous approach while permitting launching a coro if a user is determined.

Incidentally I wouldn't describe

def foo():
    loop = asyncio.get_event_loop()
    loop.create_task(my_coro())

as a workround. It is the standard way to launch a coroutine from synchronous code.

[TrevisGordan]

Yes, sure. I absolutely see the drawback, since it is the same described by my second bullet point.

But since the launch function dynamically alters the way the Callback is called, it gives more control & flexibility to the user. If in a use case a user, for example, @kevinkk525 wants to call a synchronous function since is cleaner for him, he can just declare it as a normal one and now loop.task will ever be created. If the user needs an asyn. one he can just do so.

that's what I love about aswitch functionality.

[kevinkk525]

It is true that it adds flexibility, however I consider it bad practice in a library like this to add unneccesary complexity that requires the user to know about the drawbacks. With a sync callback he doesn't need to know anything besides that it should exit quickly as it will block mqtt. This makes it more user friendly. And considering that many users that just started to use uasyncio are switching to this library, it would be best to keep it as simple as possible.

At least that's my view on this.