Improvement of exception handling (basnijholt#85)

.gitignore

@@ -67,3 +67,4 @@ target/
 #PyCharm project files
 .idea/
 .test_mac
+.pytest_cache/

.travis.yml

@@ -17,6 +17,8 @@ matrix:
       env: TOXENV=flake8
     - python: "3.6"
       env: TOXENV=pylint
+    - python: "3.6"
+      env: TOXENV=noimport
 
 script: travis_wait tox
 after_success: coveralls

miflora/__init__.py

@@ -14,6 +14,7 @@
 from miflora.backends.bluepy import BluepyBackend  # noqa: E402 # pylint: disable=wrong-import-position
 from miflora.backends.gatttool import GatttoolBackend  # noqa: E402 # pylint: disable=wrong-import-position
 from miflora.backends.pygatt import PygattBackend  # noqa: E402 # pylint: disable=wrong-import-position
+from miflora.backends import BluetoothBackendException  # noqa: F401 E402 # pylint: disable=wrong-import-position
 _ALL_BACKENDS = [BluepyBackend, GatttoolBackend, PygattBackend]
 
 

miflora/backends/__init__.py

@@ -9,23 +9,21 @@ class BluetoothInterface(object):
     """
 
     def __init__(self, backend, adapter='hci0', **kwargs):
-        self.mac = None
-        self.adapter = adapter
-        self.lock = Lock()
-        self.backend = backend(self.adapter, **kwargs)
-        self.backend.check_backend()
+        self._backend = backend(adapter, **kwargs)
+        self._backend.check_backend()
 
     def __del__(self):
         if self.is_connected():
-            self.backend.disconnect()
+            self._backend.disconnect()
 
     def connect(self, mac):
         """Connect to the sensor."""
-        return _BackendConnection(self, mac)
+        return _BackendConnection(self._backend, mac)
 
-    def is_connected(self):
+    @staticmethod
+    def is_connected():
         """Check if we are connected to the sensor."""
-        return self.lock.locked()  # pylint: disable=no-member
+        return _BackendConnection.is_connected()
 
 
 class _BackendConnection(object):  # pylint: disable=too-few-public-methods
@@ -34,18 +32,29 @@ class _BackendConnection(object):  # pylint: disable=too-few-public-methods
     This creates the context for the connection and manages locking.
     """
 
-    def __init__(self, bt_interface, mac):
-        self._bt_interface = bt_interface
-        self.mac = mac
+    _lock = Lock()
+
+    def __init__(self, backend, mac):
+        self._backend = backend
+        self._mac = mac
 
     def __enter__(self):
-        self._bt_interface.lock.acquire()
-        self._bt_interface.backend.connect(self.mac)
-        return self._bt_interface.backend
+        self._lock.acquire()
+        try:
+            self._backend.connect(self._mac)
+        except BluetoothBackendException:
+            self._lock.release()
+            raise
+        return self._backend
 
     def __exit__(self, exc_type, exc_val, exc_tb):
-        self._bt_interface.backend.disconnect()
-        self._bt_interface.lock.release()
+        self._backend.disconnect()
+        self._lock.release()
+
+    @staticmethod
+    def is_connected():
+        """Check if the BackendConnection is connected."""
+        return _BackendConnection._lock.locked()  # pylint: disable=no-member
 
 
 class BluetoothBackendException(Exception):

miflora/backends/bluepy.py

@@ -1,9 +1,37 @@
 """Backend for Miflora using the bluepy library."""
 import re
 import logging
+import time
 from miflora.backends import AbstractBackend, BluetoothBackendException
 
 _LOGGER = logging.getLogger(__name__)
+RETRY_LIMIT = 3
+RETRY_DELAY = 0.1
+
+
+def wrap_exception(func):
+    """Decorator to wrap BTLEExceptions into BluetoothBackendException."""
+    try:
+        # only do the wrapping if bluepy is installed.
+        # otherwise it's pointless anyway
+        from bluepy.btle import BTLEException
+    except ImportError:
+        return func
+
+    def _func_wrapper(*args, **kwargs):
+        error_count = 0
+        last_error = None
+        while error_count < RETRY_LIMIT:
+            try:
+                return func(*args, **kwargs)
+            except BTLEException as exception:
+                error_count += 1
+                last_error = exception
+                time.sleep(RETRY_DELAY)
+                _LOGGER.debug('Call to %s failed, try %d of %d', func, error_count, RETRY_LIMIT)
+        raise BluetoothBackendException() from last_error
+
+    return _func_wrapper
 
 
 class BluepyBackend(AbstractBackend):
@@ -14,21 +42,25 @@ def __init__(self, adapter='hci0'):
         super(BluepyBackend, self).__init__(adapter)
         self._peripheral = None
 
+    @wrap_exception
     def connect(self, mac):
         """Connect to a device."""
         from bluepy.btle import Peripheral
         match_result = re.search(r'hci([\d]+)', self.adapter)
         if match_result is None:
-            raise ValueError('Invalid pattern "{}" for BLuetooth adpater. '
-                             'Expetected something like "hci0".'.format(self.adapter))
+            raise BluetoothBackendException(
+                'Invalid pattern "{}" for BLuetooth adpater. '
+                'Expetected something like "hci0".'.format(self.adapter))
         iface = int(match_result.group(1))
         self._peripheral = Peripheral(mac, iface=iface)
 
+    @wrap_exception
     def disconnect(self):
         """Disconnect from a device."""
         self._peripheral.disconnect()
         self._peripheral = None
 
+    @wrap_exception
     def read_handle(self, handle):
         """Read a handle from the device.
 
@@ -38,6 +70,7 @@ def read_handle(self, handle):
             raise BluetoothBackendException('not connected to backend')
         return self._peripheral.readCharacteristic(handle)
 
+    @wrap_exception
     def write_handle(self, handle, value):
         """Write a handle from the device.
 
@@ -58,6 +91,7 @@ def check_backend():
         return False
 
     @staticmethod
+    @wrap_exception
     def scan_for_devices(timeout):
         """Scan for bluetooth low energy devices.
 

miflora/backends/gatttool.py

@@ -10,11 +10,22 @@
 import re
 import time
 from subprocess import Popen, PIPE, TimeoutExpired, signal, call
-from miflora.backends import AbstractBackend
+from miflora.backends import AbstractBackend, BluetoothBackendException
 
 _LOGGER = logging.getLogger(__name__)
 
 
+def wrap_exception(func):
+    """Wrap all IOErrors to BluetoothBackendException"""
+
+    def _func_wrapper(*args, **kwargs):
+        try:
+            return func(*args, **kwargs)
+        except IOError as exception:
+            raise BluetoothBackendException() from exception
+    return _func_wrapper
+
+
 class GatttoolBackend(AbstractBackend):
     """ Backend using gatttool."""
 
@@ -43,6 +54,7 @@ def is_connected(self):
         """Check if we are connected to the backend."""
         return self._mac is not None
 
+    @wrap_exception
     def write_handle(self, handle, value):
         """Read from a BLE address.
 
@@ -53,7 +65,7 @@ def write_handle(self, handle, value):
         """
 
         if not self.is_connected():
-            raise ValueError('Not connected to any device.')
+            raise BluetoothBackendException('Not connected to any device.')
 
         attempt = 0
         delay = 10
@@ -81,7 +93,7 @@ def write_handle(self, handle, value):
 
             result = result.decode("utf-8").strip(' \n\t')
             if "Write Request failed" in result:
-                raise ValueError('Error writing handls to sensor: {}'.format(result))
+                raise BluetoothBackendException('Error writing handls to sensor: {}'.format(result))
             _LOGGER.debug("Got %s from gatttool", result)
             # Parse the output
             if "successfully" in result:
@@ -95,9 +107,9 @@ def write_handle(self, handle, value):
                 time.sleep(delay)
                 delay *= 2
 
-        _LOGGER.debug("Exit write_ble, no data (%s)", current_thread())
-        return False
+        raise BluetoothBackendException("Exit write_ble, no data ({})".format(current_thread()))
 
+    @wrap_exception
     def read_handle(self, handle):
         """Read from a BLE address.
 
@@ -107,7 +119,7 @@ def read_handle(self, handle):
         """
 
         if not self.is_connected():
-            raise ValueError('Not connected to any device.')
+            raise BluetoothBackendException('Not connected to any device.')
 
         attempt = 0
         delay = 10
@@ -136,7 +148,7 @@ def read_handle(self, handle):
             _LOGGER.debug("Got \"%s\" from gatttool", result)
             # Parse the output
             if "read failed" in result:
-                raise ValueError("Read error from gatttool: {}".format(result))
+                raise BluetoothBackendException("Read error from gatttool: {}".format(result))
 
             res = re.search("( [0-9a-fA-F][0-9a-fA-F])+", result)
             if res:
@@ -150,8 +162,7 @@ def read_handle(self, handle):
                 time.sleep(delay)
                 delay *= 2
 
-        _LOGGER.debug("Exit read_ble, no data (%s)", current_thread())
-        return None
+        raise BluetoothBackendException("Exit read_ble, no data ({})".format(current_thread()))
 
     @staticmethod
     def check_backend():

miflora/backends/pygatt.py

@@ -5,9 +5,31 @@
 from miflora.backends import AbstractBackend, BluetoothBackendException
 
 
+def wrap_exception(func):
+    """Decorator to wrap pygatt exceptions into BluetoothBackendException."""
+    try:
+        # only do the wrapping if pygatt is installed.
+        # otherwise it's pointless anyway
+        from pygatt.backends.bgapi.exceptions import BGAPIError
+        from pygatt.exceptions import NotConnectedError
+    except ImportError:
+        return func
+
+    def _func_wrapper(*args, **kwargs):
+        try:
+            return func(*args, **kwargs)
+        except BGAPIError as exception:
+            raise BluetoothBackendException() from exception
+        except NotConnectedError as exception:
+            raise BluetoothBackendException() from exception
+
+    return _func_wrapper
+
+
 class PygattBackend(AbstractBackend):
     """Bluetooth backend for Blue Giga based bluetooth devices."""
 
+    @wrap_exception
     def __init__(self, adapter=None):
         """Create a new instance.
 
@@ -25,6 +47,7 @@ def __del__(self):
         if self._adapter is not None:
             self._adapter.stop()
 
+    @wrap_exception
     def connect(self, mac):
         """Connect to a device."""
         self._device = self._adapter.connect(mac)
@@ -33,18 +56,21 @@ def is_connected(self):
         """Check if connected to a device."""
         return self._device is not None
 
+    @wrap_exception
     def disconnect(self):
         """Disconnect from a device."""
         if self.is_connected():
             self._device.disconnect()
             self._device = None
 
+    @wrap_exception
     def read_handle(self, handle):
         """Read a handle from the device."""
         if not self.is_connected():
             raise BluetoothBackendException('Not connected to device!')
         return self._device.char_read_handle(handle)
 
+    @wrap_exception
     def write_handle(self, handle, value):
         """Write a handle to the device."""
         if not self.is_connected():

miflora/miflora_poller.py

@@ -6,7 +6,7 @@
 from struct import unpack
 import logging
 from threading import Lock
-from miflora.backends import BluetoothInterface
+from miflora.backends import BluetoothInterface, BluetoothBackendException
 
 _HANDLE_READ_VERSION_BATTERY = 0x38
 _HANDLE_READ_NAME = 0x03
@@ -48,31 +48,34 @@ def __init__(self, mac, backend, cache_timeout=600, retries=3, adapter='hci0'):
     def name(self):
         """Return the name of the sensor."""
         with self._bt_interface.connect(self._mac) as connection:
-            name = connection.read_handle(_HANDLE_READ_NAME)
+            name = connection.read_handle(_HANDLE_READ_NAME)  # pylint: disable=no-member
 
         if not name:
-            raise IOError("Could not read data from Mi Flora sensor %s" % self._mac)
+            raise BluetoothBackendException("Could not read data from Mi Flora sensor %s" % self._mac)
         return ''.join(chr(n) for n in name)
 
     def fill_cache(self):
         """Fill the cache with new data from the sensor."""
         _LOGGER.debug('Filling cache with new sensor data.')
-        firmware_version = self.firmware_version()
-        with self._bt_interface.connect(self._mac) as connection:
-            if not firmware_version:
-                # If a sensor doesn't work, wait 5 minutes before retrying
-                self._last_read = datetime.now() - self._cache_timeout + \
-                    timedelta(seconds=300)
-                return
+        try:
+            firmware_version = self.firmware_version()
+        except BluetoothBackendException:
+            # If a sensor doesn't work, wait 5 minutes before retrying
+            self._last_read = datetime.now() - self._cache_timeout + \
+                timedelta(seconds=300)
+            raise
 
+        with self._bt_interface.connect(self._mac) as connection:
             if firmware_version >= "2.6.6":
                 # for the newer models a magic number must be written before we can read the current data
-                if not connection.write_handle(_HANDLE_WRITE_MODE_CHANGE, _DATA_MODE_CHANGE):
+                try:
+                    connection.write_handle(_HANDLE_WRITE_MODE_CHANGE, _DATA_MODE_CHANGE)   # pylint: disable=no-member
                     # If a sensor doesn't work, wait 5 minutes before retrying
+                except BluetoothBackendException:
                     self._last_read = datetime.now() - self._cache_timeout + \
                         timedelta(seconds=300)
                     return
-            self._cache = connection.read_handle(_HANDLE_READ_SENSOR_DATA)
+            self._cache = connection.read_handle(_HANDLE_READ_SENSOR_DATA)  # pylint: disable=no-member
             _LOGGER.debug('Received result for handle %s: %s',
                           _HANDLE_READ_SENSOR_DATA, self._format_bytes(self._cache))
             self._check_data()
@@ -98,7 +101,7 @@ def firmware_version(self):
                 (datetime.now() - timedelta(hours=24) > self._fw_last_read):
             self._fw_last_read = datetime.now()
             with self._bt_interface.connect(self._mac) as connection:
-                res = connection.read_handle(_HANDLE_READ_VERSION_BATTERY)
+                res = connection.read_handle(_HANDLE_READ_VERSION_BATTERY)  # pylint: disable=no-member
                 _LOGGER.debug('Received result for handle %s: %s',
                               _HANDLE_READ_VERSION_BATTERY, self._format_bytes(res))
             if res is None:
@@ -135,7 +138,7 @@ def parameter_value(self, parameter, read_cached=True):
         if self.cache_available() and (len(self._cache) == 16):
             return self._parse_data()[parameter]
         else:
-            raise IOError("Could not read data from Mi Flora sensor %s" % self._mac)
+            raise BluetoothBackendException("Could not read data from Mi Flora sensor %s" % self._mac)
 
     def _check_data(self):
         """Ensure that the data in the cache is valid.