Repository: ikalchev/HAP-python (language: python)

Issue:
HAP-Python 3.4.0 breaks bridge with multiple accessories
I have a bridge that consists of two accessories:

- Temperature & humidity sensor
- Motion sensor

Everything was working okay with HAP-Python 3.3.0, the behavior described in ikalchev/HAP-python#327 notwithstanding.

However, after upgrading to HAP-Python 3.4.0, my motion sensor stopped working.  I have an `run()` method defined for each accessory, like below, where it outputs something to the log each time the method is invoked.

```python
@Accessory.run_at_interval(5)
async def run(self): ...
```

My `get_bridge()` method looks like this:

```python
def get_bridge(driver):

    '''
    Call this method to get a Bridge instead of a standalone accessory.
    '''

    bridge = Bridge(driver, 'Study HAP-Python Bridge')
    bridge.set_info_service( ... )

    temp_humid_sensor = TemperatureAndHumiditySensor(driver, 'Temperature & Humidity Sensor')
    bridge.add_accessory(temp_humid_sensor)

    motion_sensor = MotionSensor(driver, 'Motion Sensor')
    bridge.add_accessory(motion_sensor)

    return bridge
```

So I should see some log entry from each accessory every 5 seconds or so when `run()` is invoked.  But with the code as-is, I only see my output from the `temp_humid_sensor` accessory.  I still see `DEBUG` log chatter talking about the motion sensor, but nothing from log entry orchestrated in its `run()` method.

If I comment out the two lines with the `temp_humid_sensor` and restart the service, then I will see the output from my motion sensor accessory.

Also, if I uncomment the two lines and downgrade to HAP-Python 3.3.0, then I once again see output from both accessories every 5 seconds.

I will stick with HAP-Python 3.3.0 for now.


Discussion / hints:
There was a missing await that was fixed in 3.4.0.  Is it possible that it's taking longer than 5s to run?
Hi, @bdraco.

Not likely.  After seeing you response, I re-upgraded to 3.4.0, restarted my bridge, and let it run for about 10 minutes.  Nothing from the motion sensor, just the temperature/humidity sensor.

Downgraded again to 3.3.0, restarted my bridge, and I get output from both.
> Hi, @bdraco.
> 
> 
> 
> Not likely.  After seeing you response, I re-upgraded to 3.4.0, restarted my bridge, and let it run for about 10 minutes.  Nothing from the motion sensor, just the temperature/humidity sensor.
> 
> 
> 
> Downgraded again to 3.3.0, restarted my bridge, and I get output from both.

Can post the code for the motion sensor that isn't working?

Fix patch:
diff --git a/pyhap/accessory.py b/pyhap/accessory.py
index 4896c325..b59eb731 100644
--- a/pyhap/accessory.py
+++ b/pyhap/accessory.py
@@ -368,7 +368,7 @@ def get_characteristic(self, aid, iid):
     async def run(self):
         """Schedule tasks for each of the accessories' run method."""
         for acc in self.accessories.values():
-            await self.driver.async_add_job(acc.run)
+            self.driver.async_add_job(acc.run)
 
     async def stop(self):
         """Calls stop() on all contained accessories."""


Test patch:
diff --git a/tests/test_accessory.py b/tests/test_accessory.py
index c4eebe04..7989eeb9 100644
--- a/tests/test_accessory.py
+++ b/tests/test_accessory.py
@@ -1,4 +1,5 @@
 """Tests for pyhap.accessory."""
+import asyncio
 from io import StringIO
 from unittest.mock import patch
 
@@ -6,6 +7,7 @@
 
 from pyhap import accessory
 from pyhap.accessory import Accessory, Bridge
+from pyhap.accessory_driver import AccessoryDriver
 from pyhap.const import (
     CATEGORY_CAMERA,
     CATEGORY_TARGET_CONTROLLER,
@@ -22,6 +24,21 @@
 # #####################
 
 
+class TestAccessory(Accessory):
+    """An accessory that keeps track of if its stopped."""
+
+    def __init__(self, *args, **kwargs):
+        super().__init__(*args, **kwargs)
+        self._stopped = False
+
+    async def stop(self):
+        self._stopped = True
+
+    @property
+    def stopped(self):
+        return self._stopped
+
+
 def test_acc_init(mock_driver):
     Accessory(mock_driver, "Test Accessory")
 
@@ -383,15 +400,27 @@ def test_to_hap(mock_driver):
 
 
 @pytest.mark.asyncio
-async def test_bridge_run_stop(mock_driver):
-    mock_driver.async_add_job = AsyncMock()
-    bridge = Bridge(mock_driver, "Test Bridge")
-    acc = Accessory(mock_driver, "Test Accessory", aid=2)
-    assert acc.available is True
-    bridge.add_accessory(acc)
-    acc2 = Accessory(mock_driver, "Test Accessory 2")
-    bridge.add_accessory(acc2)
+async def test_bridge_run_stop():
+    with patch(
+        "pyhap.accessory_driver.HAPServer.async_stop", new_callable=AsyncMock
+    ), patch(
+        "pyhap.accessory_driver.HAPServer.async_start", new_callable=AsyncMock
+    ), patch(
+        "pyhap.accessory_driver.Zeroconf"
+    ), patch(
+        "pyhap.accessory_driver.AccessoryDriver.persist"
+    ), patch(
+        "pyhap.accessory_driver.AccessoryDriver.load"
+    ):
+        driver = AccessoryDriver(loop=asyncio.get_event_loop())
+        bridge = Bridge(driver, "Test Bridge")
+        acc = TestAccessory(driver, "Test Accessory", aid=2)
+        assert acc.available is True
+        bridge.add_accessory(acc)
+        acc2 = TestAccessory(driver, "Test Accessory 2")
+        bridge.add_accessory(acc2)
 
-    await bridge.run()
-    assert mock_driver.async_add_job.called
-    await bridge.stop()
+        await bridge.run()
+        await bridge.stop()
+    assert acc.stopped is True
+    assert acc2.stopped is True
diff --git a/tests/test_accessory_driver.py b/tests/test_accessory_driver.py
index 39f163ae..34dd9424 100644
--- a/tests/test_accessory_driver.py
+++ b/tests/test_accessory_driver.py
@@ -39,6 +39,38 @@
 }
 
 
+class AsyncIntervalAccessory(Accessory):
+    """An accessory increments a counter at interval."""
+
+    def __init__(self, *args, **kwargs):
+        super().__init__(*args, **kwargs)
+        self._counter = 0
+
+    @Accessory.run_at_interval(0.001)  # Run this method every 0.001 seconds
+    async def run(self):
+        self._counter += 1
+
+    @property
+    def counter(self):
+        return self._counter
+
+
+class SyncIntervalAccessory(Accessory):
+    """An accessory increments a counter at interval."""
+
+    def __init__(self, *args, **kwargs):
+        super().__init__(*args, **kwargs)
+        self._counter = 0
+
+    @Accessory.run_at_interval(0.001)  # Run this method every 0.001 seconds
+    def run(self):  # pylint: disable=invalid-overridden-method
+        self._counter += 1
+
+    @property
+    def counter(self):
+        return self._counter
+
+
 class UnavailableAccessory(Accessory):
     """An accessory that is not available."""
 
@@ -670,3 +702,67 @@ def callback_test():
         await asyncio.sleep(0)
         await asyncio.sleep(0)
         assert called is True
+
+
+@pytest.mark.asyncio
+async def test_bridge_with_multiple_async_run_at_interval_accessories():
+    with patch(
+        "pyhap.accessory_driver.HAPServer.async_stop", new_callable=AsyncMock
+    ), patch(
+        "pyhap.accessory_driver.HAPServer.async_start", new_callable=AsyncMock
+    ), patch(
+        "pyhap.accessory_driver.Zeroconf"
+    ), patch(
+        "pyhap.accessory_driver.AccessoryDriver.persist"
+    ), patch(
+        "pyhap.accessory_driver.AccessoryDriver.load"
+    ):
+        driver = AccessoryDriver(loop=asyncio.get_event_loop())
+        bridge = Bridge(driver, "mybridge")
+        acc = AsyncIntervalAccessory(driver, "TestAcc", aid=2)
+        acc2 = AsyncIntervalAccessory(driver, "TestAcc2", aid=3)
+        acc3 = AsyncIntervalAccessory(driver, "TestAcc3", aid=4)
+        bridge.add_accessory(acc)
+        bridge.add_accessory(acc2)
+        bridge.add_accessory(acc3)
+        driver.add_accessory(bridge)
+        driver.start_service()
+        await asyncio.sleep(0.5)
+        assert not driver.loop.is_closed()
+        await driver.async_stop()
+
+    assert acc.counter > 2
+    assert acc2.counter > 2
+    assert acc3.counter > 2
+
+
+@pytest.mark.asyncio
+async def test_bridge_with_multiple_sync_run_at_interval_accessories():
+    with patch(
+        "pyhap.accessory_driver.HAPServer.async_stop", new_callable=AsyncMock
+    ), patch(
+        "pyhap.accessory_driver.HAPServer.async_start", new_callable=AsyncMock
+    ), patch(
+        "pyhap.accessory_driver.Zeroconf"
+    ), patch(
+        "pyhap.accessory_driver.AccessoryDriver.persist"
+    ), patch(
+        "pyhap.accessory_driver.AccessoryDriver.load"
+    ):
+        driver = AccessoryDriver(loop=asyncio.get_event_loop())
+        bridge = Bridge(driver, "mybridge")
+        acc = SyncIntervalAccessory(driver, "TestAcc", aid=2)
+        acc2 = SyncIntervalAccessory(driver, "TestAcc2", aid=3)
+        acc3 = SyncIntervalAccessory(driver, "TestAcc3", aid=4)
+        bridge.add_accessory(acc)
+        bridge.add_accessory(acc2)
+        bridge.add_accessory(acc3)
+        driver.add_accessory(bridge)
+        driver.start_service()
+        await asyncio.sleep(0.5)
+        assert not driver.loop.is_closed()
+        await driver.async_stop()
+
+    assert acc.counter > 2
+    assert acc2.counter > 2
+    assert acc3.counter > 2
diff --git a/tests/test_hap_server.py b/tests/test_hap_server.py
index 8941878a..062042c4 100644
--- a/tests/test_hap_server.py
+++ b/tests/test_hap_server.py
@@ -43,6 +43,9 @@ async def test_we_can_connect():
     assert server.connections == {}
     _, port = sock.getsockname()
     _, writer = await asyncio.open_connection("127.0.0.1", port)
+    # flush out any call_soon
+    for _ in range(3):
+        await asyncio.sleep(0)
     assert server.connections != {}
     server.async_stop()
     writer.close()
diff --git a/tests/test_tlv.py b/tests/test_tlv.py
index 28260c60..8e9f300c 100644
--- a/tests/test_tlv.py
+++ b/tests/test_tlv.py
@@ -1,6 +1,7 @@
 """Tests for pyhap.tlv."""
 
 import pytest
+
 from pyhap import tlv