Asyncio client for HTTP

[Jc2k]

trafficstars

This is a draft so you can see where I am headed, so far i've got pair-verify and get_characteristics working against an accessoryserver test case.

As discussed in #151/#152/#153: It's going to be really hard to have the synchronous send/receive semantics and spontaneous event semantics mixing with a blocking API without decending into threading hell. Therefore I have been looking into an asyncio which will let us handle that case better.

This will be in addition to the existing API, and we won't break that API when adding asyncio support. The CLI tools will continue to use the synchronous API.

I've used core homekit_python as much as possible. However some of the pieces i wanted to use require adapting to work with asyncio. For now I have copied these into the aio folder, and i'll de-duplicate as seperate small PR's to make it easier to review.

I'm trying to keep the user facing API as similar as possible, only making operations that do network IO awaitable.

Under the hood things are a little bit different but should still be familiar apart from sprinkling await and async keywords everywhere. The most notable differences are:

• An asyncio Protocol calls you when it has received some data. So we don't have to mess around with select and network i/o timeouts. But this means we have push rather than pull based logic for the HTTP parsing.
• Because we are using an asyncio.Protocol with a data_recived we can't directly have a request() method that blocks/awaits on a reply. To make it so we can i create a stack of Future objects. I add one to a stack and return it every time we make a request. These futures are then fired as we parse results in data_received in the order they were added (FIFO, i guess).
• Because the aio API is meant for long term sessions i've tried to work in support for automatic reconnections. This will ultimately restore any subscriptions you hadd, so that get_events will transparently keep working.

I think it's going to be harder to avoid code duplication in the BLE backend, but hopefully will be able to do /something/...

[Jc2k]

@jlusiardi do you have any thoughts about where i'm going with this?

I think there are 2 directions i'd like to go in:

• I'm more than happy to put the asyncio stuff in a seperate library and then try to collaborate on the "model" and serialize/deserialize bits. The AIO bit would be aiohomekit. I'd depend on homekit for the models and serialization stuff. Bits that were common between the two versions i'd try to push upstream, but might have a local copy while i was stabilising things.
• Or we merge this and then iterate on reducing the bits that are basically duplicated (the HTTP response parsing is an example)

Option 1 might be better in the short term so I can iterate quickly on the Home Assistant. But then there is an option 3 where we start with option 1 and then merge back together when things are more battle tested.

[jlusiardi]

so why not option 2? After the merge all the tools / the api should work like before and we have one code base to work on?

[Jc2k]

That's indeed why i prefer option 2 - i just don't want to put pressure on you to review a bunch of code for a framework you aren't familiar with AND a flurry of pull requests with me nagging, and then a bit after that nagging for releases :-) If you are up for that lets crack on with option 2!

[jlusiardi]

ok, so i will try to have a look at the code as fast as possible ;)

[Jc2k]

/me hops up and down excitedly ;)

[jlusiardi]

could we integrate the tests into coverage3 run --branch -m unittest -v somehow?

[Jc2k]

Probably, I used pytest for these because I’m more used to it’s output when something fails and have no idea how to write asyncio tests without it. What are the chances I can persuade you to switch to pytest :) (it can run all the existing tests as they are)

[jlusiardi]

Ok, pytest runs the tests but fails at using cryptography.hazmat.primitives.serialization.Encoding.Raw? So I guess I will have to do some work there ;)

[Jc2k]

That's weird - they pass here on 2 different macs (though skipped the BLE ones). Python 3.7.3. Which use of RAW is failing for you? The new (temporary) one in homekit.aio? Do you have pytest and the pytest-asyncio helper installed?

[Jc2k]

(Have updated CI on this branch to use pytest and its running now)

[Jc2k]

I rebased this on master so the review diff no long contains the accessoryserver.py changes. I'll be able to do the same again when we have merged the other 2 PRs.

[Jc2k]

Thanks for all the merges so far - have rebased this based on the latest round and got rid of the code that i'm now able to share with the synchronous client \o/

[Jc2k]

Have rebased and it now reuses the same pair setup state machines as the sync clients.

[Jc2k]

What can we do to make progress here?

• When you are happy with the basic structure of the new code I want to run this with my home assistant environment for a bit and see how stable it is over 24 hours. I don't want to do this QA if there are large structural changes desired though.
• Again, if there are no big changes to do i'd probably try and at some tests for failure cases etc.
• I have thought about adding asyncio versions of the CLI tools, mostly for development and debugging purposes. I didn't want to clutter this PR further, though.

The current status is:

• No BLE support yet - out of scope for this PR I think.
• I haven't tried to use aiozeroconf yet - there are no issues with Windows and asyncio and UDP that means that home assistant uses the synchronous zeroconf we use. This will be fixed in python 3.8, apparently, so i don't plan to use aiozeroconf any time soon.
• The API is largely similar to the synchronous one, especially for pairings, but using awaitable async defs as needed. The big exception is that discovery returns a "Discovery" object rather than a dict - and this has methods for initiating pairing, doing an identiy, etc.

There are still some bits of logic that could be shared between the sync and sync client - around parsing and serializing data for/from the API calls. But these are less pressing I think. Let me know if you think thats a blocker.

[jlusiardi]

Hey,

• I looked over your PR and do not have any findings. Great work!
• More tests are good, I guess ;)
• I could also try myself on the asyncio implementation of the cli tools to check to get more experience.

For further PRs:

• I would love to have BLE as well on asyncio
• Ok for the aiozeroconf

If you are confident after your QA with HASS.IO I will sure merge it!

[Jc2k]

That's good news! Then:

• I will start doing some QA with my main homeassistant as soon as I can
• I'll throw a few more tests at it
• I'll implement a demo cli to show it polling and listening to events at same time (i need something like this to test/debug the multiplexing of events and request/reply on the same connection) - but i'll let you pick up other CLI stuff as a asyncio practice.

And then i'll let you know!

BLE is definitely on the cards - i have started sketching it out in a branch a bit but wanted to stabilise the HTTP client first. I'm looking at this library - the author is a homeassistant user and has been quite responsive. After i asked they have kindly added descriptor reading support for us already and it seems it can potentially work on windows and mac as well.

[Jc2k]

Hi @jlusiardi

I found a bit of time this month to work on this. It looks like CI might be broken for macs on master though. I have "fixed" it but it installs a python 3.6 instead of a python 3.5. I couldn't get python 3.5 working again.

I wanted to point out homekit/aio/main.py - you should be able to test this branch with the CLI yourself:

echo '{}' > pairing.json
python -m homekit.aio discover_ip
python -m homekit.aio pair_ip -f pairing.json -a testdevice -d DE:VI:CE:ID 
python -m homekit.aio get_accessories -f pairing.json -a testdevice
python -m homekit.aio get_characteristic -f pairing.json -a testdevice -c 1.10
python -m homekit.aio put_characteristic -f pairing.json -a testdevice -c 1.10 true
python -m homekit.aio get_events -f pairing.json -a testdevice -c 1.10
python -m homekit.aio list_pairings -f pairing.json -a testdevice
python -m homekit.aio unpair -f pairing.json -a testdevice

[jlusiardi]

Hi @Jc2k,

I will give it a good look when I’m back home, because all the HomeKit hardware is there.

The CI should be fixable earlier though.

Joachim

[PaulMcMillan]

@jc2k The way the PR currently is, you've got get_characteristics and put_characteristics pluralized, which is reasonable, but not what's in your example comment above and differs from the other client CLI.

[Jc2k]

Good spot. Fixed the argparse definition for those 2.

[PaulMcMillan]

I took the code for a spin with my Honeywell Lyric alarm controller, and can confirm the aio branch works well: https://gist.github.com/PaulMcMillan/bb834a711c7055663888b7e39a193c84

Let me know if there are other tests you'd like to see.

[eugr]

I tested it with my Insignia garage opener and it seems to be working well: https://gist.github.com/eugr/fc454156292c90407afe347b8e782094

[Jc2k]

@PaulMcMillan @eugr thanks for the updates! Really pleased that the basics seem to be working for a range of devices!

@jlusiardi rebased + pushed a few more tests this morning. Mac CI has broken again in a different way this time. Seems to be setting up python without an ssl module now :/ Tried using the homebrew addon to install python based on a closed ticket in their git tracker.

[eugr]

@Jc2k: have you tried Python from MacPorts distribution instead? It’s a completely separate install that does not use system libraries unlike Homebrew.

[rodriguezst]

I have been running this branch of homekit_python with @Jc2k branch of home-assistant for a few weeks without any issues. I have linked a Xiaomi Aqara Hub (HomeKit version) with some door and temperature sensors and everything works great and instantaneously. I can see the sensors updated on real time and also enable and disable the Hub's alarm. No issues at all on Home-Assistant log.

[jlusiardi]

we should investigate why the checks are failing on OS X, i think.

[stoprocent]

Any update on this pull request ?

[jlusiardi]

Hi @stoprocent, the PR sounds promising, but will still introduce a huge amount of code duplication. Because of that I am still a little unsure.

[Jc2k]

Yeah i was worried about that too. There is more that can be done on that front, but its just going to make this PR even bigger. And ultimately there will be duplication, especially if we want a CLI so we can test the async code outside of other projects. But going asyncio native is the only way forward for HomeKit in HomeAssistant. The async <-> sync bridge is a big pain there. I guess it might be easier for me to spin this out into its own project like aiohomekit after all unfortunately.