support connect/disconnect, support multiple callbacks #39
Conversation
support keep connection alive support multiple callbacks
eq3bt/connection.py
Outdated
| if adapter is None: | ||
| continue | ||
| iface_list.append(re.search(r'\d+$', path)[0]) | ||
| return iface_list |
There was a problem hiding this comment.
This would introduce hard dependency for dbus (which may or may not be available on all environments), so it's a no go. Could you please create a separate PR for adding optional support for multiple interfaces?
Also, are you sure bluepy does not provide an interface for enumerating over existing adapters? That would be the preferred way over manually scraping bluez responses.
There was a problem hiding this comment.
Unfortunately bluepy doesn't provide such a functionality, that was the first place I searched for. I could find this one from bluez which is the functionality I implemented:
https://github.com/pauloborges/bluez/blob/master/test/test-adapter
Another possible solution would be to remove this one from eq3bt and add an additional parameter "iface".
There was a problem hiding this comment.
Hmm, I think the best solution is to refactor the connection handling (so that connect can be done separately outside the __enter__) and expose iface parameter for it as you propose. The eq3bt cli tool could also optionally expose --interface option, and the downstream users can use whatever they want (like dbus) to locate available interfaces.
There was a problem hiding this comment.
Just to be sure that we are on the same page:
- add
connect(iface)method - add
ifaceparameter toThermostat - remove dbus and get adapters completely
There was a problem hiding this comment.
Sounds good to me!
connect(iface)will be useful for your use case, where you want to fall back to other interfaces if the connection fails- Adding optional
ifacetoThermostatallows forcing to use specific controller (e.g., via cli) during the initialization phase, in case someone wants to do that.
The real fix for obtaining interfaces should be done inside bluepy, but I think the library is not very actively maintained, notwithstanding the problem with dbus availability (iirc there was also some problems getting that library easily pip-installable due to some compiling needed during the setup process). Last time I did some dbus interfacing, I used python-dbus-next which is pure python only library.
eq3bt/connection.py
Outdated
| raise | ||
| conn_state = self._conn.getState() | ||
| except: | ||
| self._conn = None |
There was a problem hiding this comment.
This will lose the reference to the potentially already existing _conn object, is there something else that should be done in cases where getState errors on already existing connection?
Anycase, please don't use bare excepts but catch only exceptions you are expecting.
There was a problem hiding this comment.
Ok, I can change that to catch only the errors which indicate that the connection was already lost and therefore a disconnect() call isn't needed.
eq3bt/connection.py
Outdated
| _LOGGER.debug("Second connection try to %s using ifaces %s failed: %s", self._mac, self._ifaces[self._iface_idx], ex2) | ||
| if self.next_iface() is False: | ||
| # tried all ifaces, raise exception | ||
| raise |
There was a problem hiding this comment.
This structure feels very error-prone, I think the correct way would be refactoring the connection code away from __enter__ (it's not really exposed anywhere), and create separate connect() method that does the connection using the given iface.
There was a problem hiding this comment.
That makes sense. I'll refactor and add a connect() method with iface as parameter.
add connect(iface) for keep_connection usage
|
Hi, |
fix keep_connected
fhempy
changed the title
support multiple hci interfaces:
Tries to connect via all available interfaces and sticks to one if the connection works.
support keep connection alive:
Allows the user to keep the connection alive and re-establishes the connection if it dropped inbetween.
support multiple callbacks:
Allow to register multiple callbacks.