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

WIP: Prototype for #151 #152

Closed
wants to merge 7 commits into from
Closed

Conversation

Jc2k
Copy link
Collaborator

@Jc2k Jc2k commented Jul 23, 2019

This is a quick prototype for #151 option 2. This provides a mechanism to allow HAP events on the same TCP stream as calls that have request/reply semantics like get_characteristics (which is what Apple does) by treating the replies as events.

This:

  • Avoids races where the event stream reads our response for a request/response method like get_characteristics or get_characteristic reads an event instead of its result
  • Fixes compatibility with HomeBridge which has an exacerbated form of the above (unsolicited events without even subscribing).
  • Allows us to implement controllers that are as rich as the Apple ones without needing the accessories to handle double the number of TCP streams small underpowered devices have to support
  • Incidentally provides a nice way for my HASS code to share code paths for pull and push.

The final PR should contain documentation that warns not to use the main API when subscribing to events.

I don't think i can safely implement option 1 in #151 without porting to asyncio or similar.

This is added as a new API to keep the old API and keep this as a non breaking change.

The code needs some cleanups - and there are some opportunities to reduce code duplication - but i wanted to form some consensus around the approach first.

The TL;DR for the changes are:

  • Make some _nowait() variants of the low level HTTP API. These make the HTTP requests but do not wait for a return.
  • Add a new get_message_bus(). This returns an object that can be iterated over to receive the events. It also has get_characteristics, put_characteristics, subscribe and unsubscribe methods that use the new _nowait() variants under the hood
  • Reimplements get_event on top of this API.

If you like the direction this is going in I will polish it - get rid of code duplication, add tests, fix docstrings, add warnings etc.

@Jc2k
Copy link
Collaborator Author

Jc2k commented Jul 23, 2019

I'm hoping to replace #115 with this, too, perhaps by having a stop() or close() on the messagebus object.

@Jc2k
Copy link
Collaborator Author

Jc2k commented Jul 23, 2019

Last commit adds the new interface for BLE too, though it is completely untested!

@Jc2k
Copy link
Collaborator Author

Jc2k commented Jul 24, 2019

Tried to have a quick look this morning but getting an error when trying to pair on Debian Stretch.

org.freedesktop.DBus.Error.UnknownObject: Method "Get" with signature "ss" on interface "org.freedesktop.DBus.Properties" doesn't exist

2019-07-24 07:54:56,750 pair_ble.py:0075 DEBUG org.freedesktop.DBus.Error.UnknownObject: Method "Get" with signature "ss" on interface "org.freedesktop.DBus.Properties" doesn't exist
Traceback (most recent call last):
  File "/home/john/homekit_python/homekit/controller/ble_impl/device.py", line 54, in __init__
    self.name = self._properties.Get('org.bluez.Device1', 'Alias')
  File "/home/john/homekit_python/venv/lib/python3.5/site-packages/dbus/proxies.py", line 70, in __call__
    return self._proxy_method(*args, **keywords)
  File "/home/john/homekit_python/venv/lib/python3.5/site-packages/dbus/proxies.py", line 145, in __call__
    **keywords)
  File "/home/john/homekit_python/venv/lib/python3.5/site-packages/dbus/connection.py", line 651, in call_blocking
    message, timeout)
dbus.exceptions.DBusException: org.freedesktop.DBus.Error.UnknownObject: Method "Get" with signature "ss" on interface "org.freedesktop.DBus.Properties" doesn't exist

@Jc2k Jc2k mentioned this pull request Jul 29, 2019
@Jc2k
Copy link
Collaborator Author

Jc2k commented Jul 29, 2019

Closing in favor of #154

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

Successfully merging this pull request may close these issues.

1 participant