search

jlusiardi / homekit_python

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

Bluetooth LE Accessories #56

Closed der-punkt closed 5 years ago

der-punkt commented 5 years ago

Hey,

what is needed to implement the support for Bluetooth LE Accessories?

I don't have much free time, but I'm motivated to contribute here.

Best regards

Jc2k commented 5 years ago

Hey i saw you pushed some more stuff! Looking good! I'm hoping to try and use this branch from hass as soon as I get some time. I was thinking we could do with something like replacing:

if not self.session:
    self.session = BleSession(self.pairing_data)

with:

if not self.session or not self.session.is_valid():
    self.session = BleSession(self.pairing_data)

where that method would be:

class BleSession(...):
    ...
    def is_valid(self):
        if not self.device.is_connected():
            return False
        if not self.device.is_services_resolved():
            return False
        return False

Then the pairing methods like put_characteristics have a chance of survivng some of the more common ways a secure session ending will manifest.

And we should take care to call self.device.disconnect() if the a request fails too.

Jc2k commented 5 years ago

Here's my homeassistant branch to get this working in homeassistant:

https://github.com/home-assistant/home-assistant/compare/dev...Jc2k:homekit-bluetooth

Thought it might be a good case study for how the branch is shaping up from an API use point of view.

The big change is that we can't (or shouldn't) use list_accessories_and_characteristics as much in BLE land, as its a lot heavier and slower. I imagine it wouldn't be good for battery powered sensors. I'm sorting this on the homeassistant side by using get_characteristics on subset of fields, rather than trying to get them all. The stuff in update() is a bit awkward right now. I think it should be safe to pregenerate the list of characteristics to poll when the Entity object is created. I don't currently think this needs any changes on the hk_python side, its just inherently a bit messy mapping between the 2 systems.

I'd like to do something different with discover, i don't think the end user should have to filter out non-HK devices, and 'Manufacturer Specific' is an awkward key name. Incidentally, is there a limitation that stops us doing discovery through bluez/gatt?

One problem outstanding is concurrent access. We could drop a lock around BleSession.request() but i think its safe to document that concurrent access is not supported, and i can sort it on the homeassistant side (AIUI, it already has threading helpers so I assume it has mechanisms that can help with this).

To try out this branch you need to add this to your configuration.yaml:

homekit_controller:

and run it as root so that discovery can work.

Jc2k commented 5 years ago

I've got discovery working without any command line tools and without root. Not ready for a PR yet but wanted to give you a heads up that its coming.

Jc2k commented 5 years ago

Just started failing here, turns out the neighbours must have new BLE devices without manufacturer data. Have pushed a fix to my PR.

Have we got a list anywhere of whats left to do on this branch? Off the top of my head the things i'm thinking about looking at are:

I imagine there is much more on yours!

jlusiardi commented 5 years ago

hey I would propose to handle the missing topics by separate issues labeled with "BLE Transport". Is that ok? This one will be a little to much...

E.g. https://github.com/jlusiardi/homekit_python/issues/61

I'm a little busy at the moment. perhaps some more tommorow.

Thanks for your work, i hope it does not demotivate you, if i am a little slow at the moment.

Jc2k commented 5 years ago

Hey no worries, we’ve all been there, just happy to be making progress.

jlusiardi commented 5 years ago

I'll close this one as we should handle bugs / enhancements in separate issues.