Raise LockNotAcquiredException if ADB lock cannot be acquired (#137)

* Raise LockNotAcquiredException if ADB lock cannot be acquired

* Change tab to spaces

* Update HA component code

* Fix Android TV tests

* Import adb-shell exceptions

* Add tests for Home Assistant services

* Remove duplicated variable

* Warn when 'connect()' fails due to LockNotAcquiredException

* Add a test to make sure that 'FakeLock' behaves as expected

* Clean up tests in test_adb_manager.py

androidtv/adb_manager.py

@@ -16,6 +16,7 @@
 from ppadb.client import Client
 
 from .constants import DEFAULT_AUTH_TIMEOUT_S
+from .exceptions import LockNotAcquiredException
 
 _LOGGER = logging.getLogger(__name__)
 
@@ -40,9 +41,16 @@ def _acquire(lock):
     acquired : bool
         Whether or not the lock was acquired
 
+    Raises
+    ------
+    LockNotAcquiredException
+        Raised if the lock was not acquired
+
     """
     try:
         acquired = lock.acquire(**LOCK_KWARGS)
+        if not acquired:
+            raise LockNotAcquiredException
         yield acquired
 
     finally:
@@ -109,8 +117,8 @@ def connect(self, always_log_errors=True, auth_timeout_s=DEFAULT_AUTH_TIMEOUT_S)
             Whether or not the connection was successfully established and the device is available
 
         """
-        with _acquire(self._adb_lock) as acquired:
-            if acquired:
+        try:
+            with _acquire(self._adb_lock):
                 # Catch exceptions
                 try:
                     # Connect with authentication
@@ -159,11 +167,11 @@ def connect(self, always_log_errors=True, auth_timeout_s=DEFAULT_AUTH_TIMEOUT_S)
                     self._available = False
                     return False
 
-        # Lock could not be acquired
-        _LOGGER.warning("Couldn't connect to %s:%d because adb-shell lock not acquired.", self.host, self.port)
-        self.close()
-        self._available = False
-        return False
+        except LockNotAcquiredException:
+            _LOGGER.warning("Couldn't connect to %s:%d because adb-shell lock not acquired.", self.host, self.port)
+            self.close()
+            self._available = False
+            return False
 
     def pull(self, local_path, device_path):
         """Pull a file from the device using the Python ADB implementation.
@@ -180,15 +188,10 @@ def pull(self, local_path, device_path):
             _LOGGER.debug("ADB command not sent to %s:%d because adb-shell connection is not established: pull(%s, %s)", self.host, self.port, local_path, device_path)
             return
 
-        with _acquire(self._adb_lock) as acquired:
-            if acquired:
-                _LOGGER.debug("Sending command to %s:%d via adb-shell: pull(%s, %s)", self.host, self.port, local_path, device_path)
-                self._adb.pull(device_path, local_path)
-                return
-
-        # Lock could not be acquired
-        _LOGGER.warning("ADB command not sent to %s:%d because adb-shell lock not acquired: pull(%s, %s)", self.host, self.port, local_path, device_path)
-        return
+        with _acquire(self._adb_lock):
+            _LOGGER.debug("Sending command to %s:%d via adb-shell: pull(%s, %s)", self.host, self.port, local_path, device_path)
+            self._adb.pull(device_path, local_path)
+            return
 
     def push(self, local_path, device_path):
         """Push a file to the device using the Python ADB implementation.
@@ -205,15 +208,10 @@ def push(self, local_path, device_path):
             _LOGGER.debug("ADB command not sent to %s:%d because adb-shell connection is not established: push(%s, %s)", self.host, self.port, local_path, device_path)
             return
 
-        with _acquire(self._adb_lock) as acquired:
-            if acquired:
-                _LOGGER.debug("Sending command to %s:%d via adb-shell: push(%s, %s)", self.host, self.port, local_path, device_path)
-                self._adb.push(local_path, device_path)
-                return
-
-        # Lock could not be acquired
-        _LOGGER.warning("ADB command not sent to %s:%d because adb-shell lock not acquired: push(%s, %s)", self.host, self.port, local_path, device_path)
-        return
+        with _acquire(self._adb_lock):
+            _LOGGER.debug("Sending command to %s:%d via adb-shell: push(%s, %s)", self.host, self.port, local_path, device_path)
+            self._adb.push(local_path, device_path)
+            return
 
     def shell(self, cmd):
         """Send an ADB command using the Python ADB implementation.
@@ -233,14 +231,9 @@ def shell(self, cmd):
             _LOGGER.debug("ADB command not sent to %s:%d because adb-shell connection is not established: %s", self.host, self.port, cmd)
             return None
 
-        with _acquire(self._adb_lock) as acquired:
-            if acquired:
-                _LOGGER.debug("Sending command to %s:%d via adb-shell: %s", self.host, self.port, cmd)
-                return self._adb.shell(cmd)
-
-        # Lock could not be acquired
-        _LOGGER.warning("ADB command not sent to %s:%d because adb-shell lock not acquired: %s", self.host, self.port, cmd)
-        return None
+        with _acquire(self._adb_lock):
+            _LOGGER.debug("Sending command to %s:%d via adb-shell: %s", self.host, self.port, cmd)
+            return self._adb.shell(cmd)
 
 
 class ADBServer(object):
@@ -343,8 +336,8 @@ def connect(self, always_log_errors=True):
             Whether or not the connection was successfully established and the device is available
 
         """
-        with _acquire(self._adb_lock) as acquired:
-            if acquired:
+        try:
+            with _acquire(self._adb_lock):
                 # Catch exceptions
                 try:
                     self._adb_client = Client(host=self.adb_server_ip, port=self.adb_server_port)
@@ -373,11 +366,11 @@ def connect(self, always_log_errors=True):
                     self._available = False
                     return False
 
-        # Lock could not be acquired
-        _LOGGER.warning("Couldn't connect to %s:%d via ADB server %s:%d because pure-python-adb lock not acquired.", self.host, self.port, self.adb_server_ip, self.adb_server_port)
-        self.close()
-        self._available = False
-        return False
+        except LockNotAcquiredException:
+            _LOGGER.warning("Couldn't connect to %s:%d via ADB server %s:%d because pure-python-adb lock not acquired.", self.host, self.port, self.adb_server_ip, self.adb_server_port)
+            self.close()
+            self._available = False
+            return False
 
     def pull(self, local_path, device_path):
         """Pull a file from the device using an ADB server.
@@ -394,15 +387,10 @@ def pull(self, local_path, device_path):
             _LOGGER.debug("ADB command not sent to %s:%d via ADB server %s:%d because pure-python-adb connection is not established: pull(%s, %s)", self.host, self.port, self.adb_server_ip, self.adb_server_port, local_path, device_path)
             return
 
-        with _acquire(self._adb_lock) as acquired:
-            if acquired:
-                _LOGGER.debug("Sending command to %s:%d via ADB server %s:%d: pull(%s, %s)", self.host, self.port, self.adb_server_ip, self.adb_server_port, local_path, device_path)
-                self._adb_device.pull(device_path, local_path)
-                return
-
-        # Lock could not be acquired
-        _LOGGER.warning("ADB command not sent to %s:%d via ADB server %s:%d: pull(%s, %s)", self.host, self.port, self.adb_server_ip, self.adb_server_port, local_path, device_path)
-        return
+        with _acquire(self._adb_lock):
+            _LOGGER.debug("Sending command to %s:%d via ADB server %s:%d: pull(%s, %s)", self.host, self.port, self.adb_server_ip, self.adb_server_port, local_path, device_path)
+            self._adb_device.pull(device_path, local_path)
+            return
 
     def push(self, local_path, device_path):
         """Push a file to the device using an ADB server.
@@ -419,15 +407,10 @@ def push(self, local_path, device_path):
             _LOGGER.debug("ADB command not sent to %s:%d via ADB server %s:%d because pure-python-adb connection is not established: push(%s, %s)", self.host, self.port, self.adb_server_ip, self.adb_server_port, local_path, device_path)
             return
 
-        with _acquire(self._adb_lock) as acquired:
-            if acquired:
-                _LOGGER.debug("Sending command to %s:%d via ADB server %s:%d: push(%s, %s)", self.host, self.port, self.adb_server_ip, self.adb_server_port, local_path, device_path)
-                self._adb_device.push(local_path, device_path)
-                return
-
-        # Lock could not be acquired
-        _LOGGER.warning("ADB command not sent to %s:%d via ADB server %s:%d: push(%s, %s)", self.host, self.port, self.adb_server_ip, self.adb_server_port, local_path, device_path)
-        return
+        with _acquire(self._adb_lock):
+            _LOGGER.debug("Sending command to %s:%d via ADB server %s:%d: push(%s, %s)", self.host, self.port, self.adb_server_ip, self.adb_server_port, local_path, device_path)
+            self._adb_device.push(local_path, device_path)
+            return
 
     def shell(self, cmd):
         """Send an ADB command using an ADB server.
@@ -447,11 +430,6 @@ def shell(self, cmd):
             _LOGGER.debug("ADB command not sent to %s:%d via ADB server %s:%d because pure-python-adb connection is not established: %s", self.host, self.port, self.adb_server_ip, self.adb_server_port, cmd)
             return None
 
-        with _acquire(self._adb_lock) as acquired:
-            if acquired:
-                _LOGGER.debug("Sending command to %s:%d via ADB server %s:%d: %s", self.host, self.port, self.adb_server_ip, self.adb_server_port, cmd)
-                return self._adb_device.shell(cmd)
-
-        # Lock could not be acquired
-        _LOGGER.warning("ADB command not sent to %s:%d via ADB server %s:%d because pure-python-adb lock not acquired: %s", self.host, self.port, self.adb_server_ip, self.adb_server_port, cmd)
-        return None
+        with _acquire(self._adb_lock):
+            _LOGGER.debug("Sending command to %s:%d via ADB server %s:%d: %s", self.host, self.port, self.adb_server_ip, self.adb_server_port, cmd)
+            return self._adb_device.shell(cmd)

androidtv/exceptions.py

@@ -0,0 +1,7 @@
+"""Exceptions for use throughout the code.
+
+"""
+
+
+class LockNotAcquiredException(Exception):
+    """The ADB lock could not be acquired."""

tests/test_adb_manager.py

@@ -9,7 +9,8 @@
 
 sys.path.insert(0, '..')
 
-from androidtv.adb_manager import ADBPython, ADBServer
+from androidtv.adb_manager import _acquire, ADBPython, ADBServer
+from androidtv.exceptions import LockNotAcquiredException
 from . import patchers
 
 
@@ -54,6 +55,9 @@ def __init__(self, *args, **kwargs):
         self._acquired = True
 
     def acquire(self, *args, **kwargs):
+        if self._acquired:
+            self._acquired = False
+            return True
         return self._acquired
 
     def release(self, *args, **kwargs):
@@ -77,6 +81,19 @@ def setUp(self):
         with patchers.PATCH_ADB_DEVICE_TCP, patchers.patch_connect(True)[self.PATCH_KEY]:
             self.adb = ADBPython('HOST', 5555)
 
+    def test_locked_lock(self):
+        """Test that the ``FakeLock`` class works as expected.
+
+        """
+        with patch.object(self.adb, '_adb_lock', FakeLock()):
+            with _acquire(self.adb._adb_lock):
+                with self.assertRaises(LockNotAcquiredException):
+                    with _acquire(self.adb._adb_lock):
+                        pass
+
+            with _acquire(self.adb._adb_lock) as acquired:
+                self.assertTrue(acquired)
+
     def test_connect_success(self):
         """Test when the connect attempt is successful.
 
@@ -126,8 +143,11 @@ def test_adb_shell_fail(self):
         with patchers.patch_connect(True)[self.PATCH_KEY], patchers.patch_shell("TEST")[self.PATCH_KEY]:
             self.assertTrue(self.adb.connect())
             with patch.object(self.adb, '_adb_lock', LockedLock()):
-                self.assertIsNone(self.adb.shell("TEST"))
-                self.assertIsNone(self.adb.shell("TEST2"))
+                with self.assertRaises(LockNotAcquiredException):
+                    self.adb.shell("TEST")
+
+                with self.assertRaises(LockNotAcquiredException):
+                    self.adb.shell("TEST2")
 
     def test_adb_shell_success(self):
         """Test when an ADB shell command is successfully sent.
@@ -160,7 +180,9 @@ def test_adb_shell_lock_not_acquired_not_released(self):
 
         with patchers.patch_shell("TEST")[self.PATCH_KEY], patch.object(self.adb, '_adb_lock', LockedLock()):
             with patch('{}.LockedLock.release'.format(__name__)) as release:
-                self.assertIsNone(self.adb.shell("TEST"))
+                with self.assertRaises(LockNotAcquiredException):
+                    self.adb.shell("TEST")
+
                 release.assert_not_called()
 
     def test_adb_push_fail(self):
@@ -177,7 +199,9 @@ def test_adb_push_fail(self):
             with patchers.PATCH_PUSH[self.PATCH_KEY] as patch_push:
                 self.assertTrue(self.adb.connect())
                 with patch.object(self.adb, '_adb_lock', LockedLock()):
-                    self.adb.push("TEST_LOCAL_PATH", "TEST_DEVICE_PATH")
+                    with self.assertRaises(LockNotAcquiredException):
+                        self.adb.push("TEST_LOCAL_PATH", "TEST_DEVICE_PATH")
+
                     patch_push.assert_not_called()
 
     def test_adb_push_success(self):
@@ -204,7 +228,8 @@ def test_adb_pull_fail(self):
             with patchers.PATCH_PULL[self.PATCH_KEY] as patch_pull:
                 self.assertTrue(self.adb.connect())
                 with patch.object(self.adb, '_adb_lock', LockedLock()):
-                    self.adb.pull("TEST_LOCAL_PATH", "TEST_DEVICE_PATH")
+                    with self.assertRaises(LockNotAcquiredException):
+                        self.adb.pull("TEST_LOCAL_PATH", "TEST_DEVICE_PATH")
                     patch_pull.assert_not_called()
 
     def test_adb_pull_success(self):
@@ -313,6 +338,7 @@ class TestADBPythonClose(unittest.TestCase):
 
     def test_close(self):
         """Test the `ADBPython.close` method.
+
         """
         with patchers.PATCH_ADB_DEVICE_TCP, patchers.patch_connect(True)[self.PATCH_KEY]:
             self.adb = ADBPython('HOST', 5555)