Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Thinking about the events API #151

Closed
Jc2k opened this issue Jul 23, 2019 · 2 comments
Closed

Thinking about the events API #151

Jc2k opened this issue Jul 23, 2019 · 2 comments

Comments

@Jc2k
Copy link
Collaborator

Jc2k commented Jul 23, 2019

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:

  • There should be a documented way to build hybrid push/poll applications (e.g. you can get an event for the on characteristic but not for temperature characteristics which you can only poll)
  • The API should work for BLE (Support events for BLE devices #62) and IP accessories
  • It should work with homebridge (currently homebridge works with iOS but not with homekit_python)
  • The API should support dynamically subscribing and unsubscribing

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

  • HomeKit on iOS only has a single connection from a controller to an accessory. Even after turning events on, it continues to use the same TCP connection for any get or put operations.
  • HomeBridge does this thing where it sends unsolicited empty events as keep alive messages. This works on iOS. However this breaks homekit_python when polling - after such an unsolicited event the first get_characteristics returns {}, the second returns the result for the first.

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:

  • every request/reply code path (like get_characteristics, put_characteristics, etc) would need to call _read_response until it got its first HTTP/1.1 response.
  • any EVENT/1.0 responses received in those paths would need to be injected into any ongoing get_events call.
  • get_events would need to handle any HTTP/1.1 messages too, and somehow get them to the call that is currently in progress
  • if this happened on different threads, you'd have synchronization issues where both loops calling _read_response pulled different HTTP/1.1 responses. It's unclear how we'd untangle things if that happened.

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?

  • This is obviously not a replacement for CLI tools where you make a request, get a reply and exit. This API only makes sense for long lived connections e.g. where you have a dispatch method that can take a list of aid/iid state updates and update a home automation systems state.
  • Calling get_message_bus is really about opting into not having request/reply HTTP/1.1 semantics at all. Once you have opted in you are saying - i am turning on events and we can't safely mix events and request/reply any more. We could not do that, and just add _nowait() versions of the method or nowait=True kwargs.
  • None of the methods on bus have request/reply semantics. They mirror their counterparts on the pairing object, but they all return immediately after sending a request.
  • All responses from the device are handled in one place. In hass, this will be on a background thread that dispatches the new values to the right HASS entities.
  • When we poll a characteristic with get() we don't look for a return value. The return value is dispatched as though it was an event.

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.

  • In an app on my phone i turned a Hue light on (via my prod Home Assistant)
  • The Hue notifies over HomeKit
  • My dev home assistant gets notified immediately
  • My web browser is open and there is a websocket so the UI live updates. As i toggle the light on my phone, the HASS UI live updates.
  • Once a minute, bus.get() is called to fetch the pollable characteristics. These are handled by the same code path as the events. So its the same dispatch code for poll and push, which is neat.

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
Copy link
Collaborator Author

Jc2k commented Jul 23, 2019

Prototype for opion 2 in #152

@Jc2k
Copy link
Collaborator Author

Jc2k commented Jul 29, 2019

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.

@Jc2k Jc2k closed this as completed Jul 29, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant