Open rytilahti opened 6 years ago
I think this is really good idea. But I wonder about doing asyncio on top of the current implementation (if I understood this correctly). Doing it this way would require running some parts in the executor, missing the best asyncio has to offer - asynchronous, non-blocking sockets and other IO operations.
I think the only sensible solution is going fully asyncio and then add a compatibility layer so it can still be used from non-asyncio applications.
Dropping support for python 3.4 may seem like a hard decision to do, but asyncio in py34 based on decorators and generators is really ugly and hard to read when compared to async/await syntax in py35+. Also Home-Assistant already declared support py34 as deprecated last year and is going to drop it soon.
This would indeed require quite a bit of refactoring, and would also be done while dropping the support for 3.4 (with what I'm okay with). The code-base itself should be converted to be purely asyncio, synchronous interfaces could be added if/where needed, the cli tool could be managed similarly to what's done in https://github.com/rytilahti/python-songpal/blob/master/songpal/main.py .
@rytilahti I've decided to try and port the library to asyncio
. I can say that I already have a working communication based on asyncio
's DatagramProtocol
. I've implemented two DatagramProtocol
subclasses: MIIOProtocol
and MIIODiscoverProtocol
.
The MIIODiscoverProtocol
implements the handshake/discovery logic. Like in the current implementation, it can work either with broadcast and wait for multiple responses (it stores the results in asyncio.Queue
so external asyncio
code can consume the results on the fly, but also a callback callable can be provided which is called on each response) or with a single host and returns the response right away.
The MIIOProtocol
implements the request/response logic with timeouts and retries. It supports parallel requests (for example I can send two get_prop requests without waiting for first one to return before requesting the other set of properties). This class also uses the MIIODiscoveryProtocol
to do the handshake, but I've implemented it so that it doesn't do handshake before every request, but works in a loop with some sleep time (I'm using 60 seconds right now). I'm using an asyncio.Event
instance to make sure that the request can't be made before first successful handshake and then during every next scheduled one (the send coroutine waits on the Event
object to be set).
I haven't modified any other code or class in the library yet. I've been able to successfully reuse the construct schema. But I didn't touch any devices' code yet. I'm testing directly on my new classes.
Also something needs to be done with the zeroconf
library. The one currently in use doesn't support asyncio
. I've found aiozeroconf
which is a direct port of the zeroconf
for asyncio
.
When I'm happy with the code I'll push this as a branch to my fork, but I probably won't create a PR right away. I'll post it here so it can be decided what's next.
It would be nice to have an asyncio-based API on top of the current one for those who prefer avoiding blocking. An open question here would be whether python <3.5 should still be supported after adding this, as
async def
is first available in 3.5 (which is already in debian stable, so it should be a safe target in my opinion).