jlusiardi / homekit_python

A python implementation to work as both HomeKit controller and accessory.
Apache License 2.0
221 stars 41 forks source link

Thinking about the events API #151

Closed Jc2k closed 5 years ago

Jc2k commented 5 years ago

I'm looking at how to move Home Assistant to use events with homekit_python. I've just got a prototype working, but its a bit different to how things work now and I wanted to start a conversation about it.

The goals are:

I've been thinking alot about what Apple does. There are a couple of things i've noticed:

What i've been thinking is just - how can this be? AFAICT as soon as you have events - whether you ask for them or not - there is no way to tell which reply belongs to which request. So how can you have request/reply semantics and event stream semantics on the same HTTP connection?

I think there are 2 possibilities here:

Handle HTTP/1.1 vs EVENT/1.0

We need to special case unsolicited HTTP responses, which we don't do right now.

If its HTTP/1.1 then its a reply to a request, and it should be in order, so we can be pretty sure we can match it to an inflight request call. (This is a bit of an assumption based on how HTTP 1.1 pipelining works but im not sure if its explicitly documented). If its an EVENT/1.0 then it needs passing to get_events.

This is a bit messy in our current code base. It would mean that:

This approach is more reasonable when using an async framework like asyncio or Twisted - you'd likely have a single coroutine handling all incoming responses. There could be a single place handling the dispatch, and the API could look exactly like it does now. I think to do that in the sync code we would have to make heavy using of threading and threading.Event, which i think we shouldn't.

Treat HAP like an event bus

I have prototyped this approach, though the API changes need some work.

The idea is that you never expect get_characteristics/put_characteristics to return anything, and let all responses go to the event handlers.

The API would look something like this:

pairing = ...
bus = pairing.get_message_bus()

# In background thread
for event in bus:
    print(event)
    dispatch_event_to_entity_in_hass(event)

# In main thread
bus.subscribe([(1,1), (1, 2)])

bus.subscribe([(1, 3)])
bus.get([(1, 3)])

bus.put([(1, 3, True)])

bus.unsubscribe([(1,1), (1, 2), (1, 3)])

pairing.close()

So what is happening here?

I think this API would work for BLE too. It doesn't have the same request/reply/events problem from what i remember but the process would work. Annoyingly I think we have to run a thread to get events at all. I don't think there is a way to run the mainloop until we get the next event. So in the HASS case it would need 2 threads per BLE device. But right now thats true regardless of the events API.

The existing get_events API would just be implemented something like:

def get_events(characteristics, callback, ...):
    bus = self.get_message_bus()
    bus.subscribe(characteristics)
    for ev in bus:
        callback(ev)

But there would be an explicitly documented warning that you shouldn't use the normal get_characteristics or put_characteristics on this connection while get_events is running.

This API could do a few nice things like automatic reconnections. Automatic reconnections would re-enable events that were active before the disconnect. They would likely trigger a poll in case the state changed whilst the connection was lost. It might be better to leave that to the implementing app, though.

I just tested a slightly hackier version of this with my Hue.

I think i could get the prototype to work with the existing code, but it would be fragile when request/response and events are happening on the same TCP connection.

What do you think? I can tidy up what I have so far and put on a PR if it makes it easier to understand what i mean.

Jc2k commented 5 years ago

Prototype for opion 2 in #152

Jc2k commented 5 years ago

My main concerns about the events API (difficulty in avoiding threading hell and also coping with unsolicited EVENT/1.1 messages finding their way into blocking calls like get_characteristics) are alleviated by #154.

I think that instead of changing the events API the solution is that for my kind of use case you need to be using a natively async API like asyncio. So I will close this for now.