search

rytilahti / python-eq3bt

Python library and command-line tool for eQ-3 Smart Bluetooth thermostats
MIT License
114 stars 36 forks source link

Add bleak backend and make it default #53

Closed rytilahti closed 2 years ago

rytilahti commented 2 years ago

This PR adds a new bleak (https://github.com/hbldh/bleak) backend and makes it default. This should make it easier for downstreams (by avoiding bluepy's helper compilation), and will also add support for windows and mac platforms.

The implementation is a very basic sync-wrapping for bleak's asyncio calls, I'm not completely sure if this is the proper way to have it but it seems to work based on some preliminary testing.

The bleak does not seem to allow selecting an interface, so that setting is currently ignored. Ping @TechHummel do you think this is a deal breaker, considering that you added that feature in #44?

A more proper way would be either 1) adding asyncio interfaces to this library to avoid blocking altogether or 2) converting the whole code base to asyncio.

Fixes #50

TechHummel commented 2 years ago

I'd be ok with no interface selection—but maybe the issue I linked helps! Using hci0, hci1, ... for interface selection has drawbacks as they can change after reboot. I like how Ble Monitor allows you to choose the MAC address, but they are using yet another library.

rytilahti commented 2 years ago

Using hci0, hci1, ... for interface selection has drawbacks as they can change after reboot.

Ahh, I get it, but I had thought that systemd solved this problem as it did for network adapter names (enpX instead of ethX etc.)... Tbh, I'm not even sure which adapter my setup using and what would be a proper input for the interface field -- I have no /dev/hcX on my system, and while the debug logs from bleak hint towards hci0 (DEBUG:bleak.backends.bluezdbus.scanner:cached devices: {'/org/bluez/hci0/dev_00_XXXXXXX .. I'm not seeing any related device nodes :-)

Anyway, I'll adapt the PR to allow defining the interface, adding a separate dependency for this will be out of question though.

TechHummel commented 2 years ago

Awesome, thank you!

Siffup commented 2 years ago

@rytilahti will the integration as an entity work in the future, so we can use the climate card in UI again (as this was perfect)? Or makes a downgrad to an older release of ha more sense? Thank you

rytilahti commented 2 years ago

@Siffup I'm not sure what you mean by that? Was it working prior the most recent homeassistant release, if yes, then it should work again with https://github.com/home-assistant/core/pull/75145 - or at least it is working on my test setup with a single thermostat just fine.

Siffup commented 2 years ago

@rytilahti Thank you for your quick reply. I usually added this lines to my .yaml

climate:

and saw my thermostat as an entity. By now I´m using eq3cli and bind the thermostat and controll it that way, but there is no climate entity to use it for example with a scheduler card.

rytilahti commented 2 years ago

Ah, for that you need to wait for that linked PR to get merged, and a new homeassistant release if you don't want to manually modify your installation.

So the simplest option at the moment is to downgrade homeassistant, if you can.

Siffup commented 2 years ago

ok, thank you. Keep it up, your doing a great job...