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.

Sign up for GitHub

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

Add configurable number of connection attempts #46

Closed
wants to merge 8 commits into from
Closed

Add configurable number of connection attempts #46

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
3d8ef81
Add Configurable number of connection attempts
gmoelter Jan 9, 2022
a44c11a
Add configurable number of connection attempts
gmoelter Jan 9, 2022
3f07968
Add property connection_error
gmoelter Jan 15, 2022
1afb63b
Recognize connection_error
gmoelter Jan 15, 2022
f73035d
Update README.md
gmoelter Jan 16, 2022
af880ea
Changed logging levels
gmoelter Feb 20, 2022
be61fe4
Merge branch 'master' of https://github.com/gmoelter/python-eq3bt
gmoelter Feb 20, 2022
603b391
Rename 'connectionattempts' to 'retries' and modify code in eq3cli, e…
gmoelter Feb 21, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 1 addition & 2 deletions README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,8 +37,7 @@ print(thermostat)
<aside class="notice">
Notice: The device in question has to be disconnected from bluetoothd, since BTLE devices can only hold one connection at a time.

The library will try to connect to the device second time in case it wasn't successful in the first time,
which can happen if you are running other applications connecting to the same thermostat.
The library can make several attempts connecting to the device in case it fails on one, which can happen e.g. if you are running other applications connecting to the same thermostat. The number of attempts can be specified via the optional parameter `connection_attempts` to the Thermostat class. By default, it is set to 2.
</aside>

## Fetching schedule
Expand Down
18 changes: 9 additions & 9 deletions eq3bt/connection.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,14 +15,15 @@
class BTLEConnection(btle.DefaultDelegate):
"""Representation of a BTLE Connection."""

def __init__(self, mac, iface):
def __init__(self, mac, iface, retries):
"""Initialize the connection."""
btle.DefaultDelegate.__init__(self)

self._conn = None
self._mac = mac
self._iface = iface
self._callbacks = {}
self._max_conn_attempts = retries+1

def __enter__(self):
"""
Expand All @@ -33,15 +34,15 @@ def __enter__(self):
self._conn = btle.Peripheral()
self._conn.withDelegate(self)
_LOGGER.debug("Trying to connect to %s", self._mac)
try:
self._conn.connect(self._mac, iface=self._iface)
except btle.BTLEException as ex:
_LOGGER.debug("Unable to connect to the device %s, retrying: %s", self._mac, ex)

for conn_attempt in range(1, self._max_conn_attempts+1):
try:
self._conn.connect(self._mac, iface=self._iface)
except Exception as ex2:
_LOGGER.debug("Second connection try to %s failed: %s", self._mac, ex2)
raise
break
except btle.BTLEException as ex:
_LOGGER.warning("%s: Connection attempt #%s(%s) failed", self._mac, conn_attempt, self._max_conn_attempts)
if conn_attempt == self._max_conn_attempts:
raise

_LOGGER.debug("Connected to %s", self._mac)
return self
Expand Down Expand Up @@ -78,4 +79,3 @@ def make_request(self, handle, value, timeout=DEFAULT_TIMEOUT, with_response=Tru
except btle.BTLEException as ex:
_LOGGER.debug("Got exception from bluepy while making a request: %s", ex)
raise

100 changes: 83 additions & 17 deletions eq3bt/eq3btsmart.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@ class TemperatureException(Exception):
class Thermostat:
"""Representation of a EQ3 Bluetooth Smart thermostat."""

def __init__(self, _mac, _iface=None, connection_cls=BTLEConnection):
def __init__(self, _mac, _iface=None, connection_cls=BTLEConnection, retries=1):
"""Initialize the thermostat."""

self._target_temperature = Mode.Unknown
Expand All @@ -94,10 +94,15 @@ def __init__(self, _mac, _iface=None, connection_cls=BTLEConnection):
self._firmware_version = None
self._device_serial = None

self._conn = connection_cls(_mac, _iface)
self._conn = connection_cls(_mac, _iface, retries)
self._conn.set_callback(PROP_NTFY_HANDLE, self.handle_notification)
self._conn_error = False


def __str__(self):
if self._conn_error:
return "[%s] Connection error" % (self._conn.mac)

away_end = "no"
if self.away_end:
away_end = "end: %s" % self._away_end
Expand Down Expand Up @@ -192,7 +197,12 @@ def query_id(self):
"""Query device identification information, e.g. the serial number."""
_LOGGER.debug("Querying id..")
value = struct.pack('B', PROP_ID_QUERY)
self._conn.make_request(PROP_WRITE_HANDLE, value)
try:
self._conn.make_request(PROP_WRITE_HANDLE, value)
except Exception as ex:
_LOGGER.warning(ex)
self._conn_error = True
return

def update(self):
"""Update the data from the thermostat. Always sets the current time."""
Expand All @@ -201,8 +211,12 @@ def update(self):
value = struct.pack('BBBBBBB', PROP_INFO_QUERY,
time.year % 100, time.month, time.day,
time.hour, time.minute, time.second)

self._conn.make_request(PROP_WRITE_HANDLE, value)
try:
self._conn.make_request(PROP_WRITE_HANDLE, value)
except Exception as ex:
Comment on lines +214 to +216
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't currently have a test device at hand to check this, but wouldn't it make more sense to perform the error&retry checking inside make_request or leave the whole re-try machinery for upstreams to handle?

Copy link
Author

@gmoelter gmoelter Jan 11, 2022 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Inside the make_request function (connection.py) possible errors are fetched. But the calling instance (eg. update function) must recognize and handle the error. That's the reason for the stacktrace.

You don't need a real test device, try it with a random mac like 11:22:33:44:55:66.

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okay, I found a test device and did some quick testing.

Now, I think it's better to let the exceptions bubble up instead of having a device global error flag (e.g., if you query multiple different things, some of them may succeed and you can still display that information to the user), and handle the exception at user's end.

In the cli, instead of checking for the the error flag, it would be better to do something like this for the @cli.group to capture all exceptions: https://stackoverflow.com/questions/44344940/python-click-subcommand-unified-error-handling/44347763#44347763

_LOGGER.warning(ex)
self._conn_error = True
return

def query_schedule(self, day):
_LOGGER.debug("Querying schedule..")
Expand All @@ -211,8 +225,12 @@ def query_schedule(self, day):
_LOGGER.error("Invalid day: %s", day)

value = struct.pack('BB', PROP_SCHEDULE_QUERY, day)

self._conn.make_request(PROP_WRITE_HANDLE, value)
try:
self._conn.make_request(PROP_WRITE_HANDLE, value)
except Exception as ex:
_LOGGER.warning(ex)
self._conn_error = True
return

@property
def schedule(self):
Expand All @@ -224,7 +242,12 @@ def schedule(self):
def set_schedule(self, data):
"""Sets the schedule for the given day. """
value = Schedule.build(data)
self._conn.make_request(PROP_WRITE_HANDLE, value)
try:
self._conn.make_request(PROP_WRITE_HANDLE, value)
except Exception as ex:
_LOGGER.warning(ex)
self._conn_error = True
return

@property
def target_temperature(self):
Expand All @@ -241,8 +264,12 @@ def target_temperature(self, temperature):
else:
self._verify_temperature(temperature)
value = struct.pack('BB', PROP_TEMPERATURE_WRITE, dev_temp)

self._conn.make_request(PROP_WRITE_HANDLE, value)
try:
self._conn.make_request(PROP_WRITE_HANDLE, value)
except Exception as ex:
_LOGGER.warning(ex)
self._conn_error = True
return

@property
def mode(self):
Expand Down Expand Up @@ -296,7 +323,12 @@ def set_mode(self, mode, payload=None):
value = struct.pack('BB', PROP_MODE_WRITE, mode)
if payload:
value += payload
self._conn.make_request(PROP_WRITE_HANDLE, value)
try:
self._conn.make_request(PROP_WRITE_HANDLE, value)
except Exception as ex:
_LOGGER.warning(ex)
self._conn_error = True
return

@property
def mode_readable(self):
Expand Down Expand Up @@ -340,7 +372,12 @@ def boost(self, boost):
"""Sets boost mode."""
_LOGGER.debug("Setting boost mode: %s", boost)
value = struct.pack('BB', PROP_BOOST, bool(boost))
self._conn.make_request(PROP_WRITE_HANDLE, value)
try:
self._conn.make_request(PROP_WRITE_HANDLE, value)
except Exception as ex:
_LOGGER.warning(ex)
self._conn_error = True
return

@property
def valve_state(self):
Expand All @@ -363,7 +400,12 @@ def window_open_config(self, temperature, duration):

value = struct.pack('BBB', PROP_WINDOW_OPEN_CONFIG,
int(temperature * 2), int(duration.seconds / 300))
self._conn.make_request(PROP_WRITE_HANDLE, value)
try:
self._conn.make_request(PROP_WRITE_HANDLE, value)
except Exception as ex:
_LOGGER.warning(ex)
self._conn_error = True
return

@property
def window_open_temperature(self):
Expand All @@ -385,7 +427,12 @@ def locked(self, lock):
"""Locks or unlocks the thermostat."""
_LOGGER.debug("Setting the lock: %s", lock)
value = struct.pack('BB', PROP_LOCK, bool(lock))
self._conn.make_request(PROP_WRITE_HANDLE, value)
try:
self._conn.make_request(PROP_WRITE_HANDLE, value)
except Exception as ex:
_LOGGER.warning(ex)
self._conn_error = True
return

@property
def low_battery(self):
Expand All @@ -400,7 +447,12 @@ def temperature_presets(self, comfort, eco):
self._verify_temperature(eco)
value = struct.pack('BBB', PROP_COMFORT_ECO_CONFIG, int(comfort * 2),
int(eco * 2))
self._conn.make_request(PROP_WRITE_HANDLE, value)
try:
self._conn.make_request(PROP_WRITE_HANDLE, value)
except Exception as ex:
_LOGGER.warning(ex)
self._conn_error = True
return

@property
def comfort_temperature(self):
Expand Down Expand Up @@ -433,12 +485,22 @@ def temperature_offset(self, offset):
current += 0.5

value = struct.pack('BB', PROP_OFFSET, values[offset])
self._conn.make_request(PROP_WRITE_HANDLE, value)
try:
self._conn.make_request(PROP_WRITE_HANDLE, value)
except Exception as ex:
_LOGGER.warning(ex)
self._conn_error = True
return

def activate_comfort(self):
"""Activates the comfort temperature."""
value = struct.pack('B', PROP_COMFORT)
self._conn.make_request(PROP_WRITE_HANDLE, value)
try:
self._conn.make_request(PROP_WRITE_HANDLE, value)
except Exception as ex:
_LOGGER.warning(ex)
self._conn_error = True
return

def activate_eco(self):
"""Activates the comfort temperature."""
Expand All @@ -465,3 +527,7 @@ def device_serial(self):
"""Return the device serial number."""
return self._device_serial

@property
def connection_error(self):
"""Return the connection error state"""
return self._conn_error
Loading