Use one (shared) device client for all devices (#17)

* Make one connection, multiple subscriptions

* Add tests for multiple subs one connection

* Update deviceclient functions to take device serial

 - Do not require a device serial for deviceclient init
 - Update deviceclient device functions to require a device serial
 - Send received messages to the correct callback (+ test)

Author: jpelgrom

.vscode/settings.json

@@ -0,0 +1,7 @@
+{
+    "python.testing.pytestArgs": [
+        "tests"
+    ],
+    "python.testing.unittestEnabled": false,
+    "python.testing.pytestEnabled": true
+}

README.md

@@ -30,12 +30,13 @@ async def main():
         devices = await client.get_devices()
         print(devices)
 
-        device_client = LetPotDeviceClient(auth, devices[0].serial_number)
-        await device_client.subscribe(lambda status: print(status))
-        await device_client.request_status_update()
+        device_client = LetPotDeviceClient(auth)
+        device_serial = devices[0].serial_number
+        await device_client.subscribe(device_serial, lambda status: print(status))
+        await device_client.request_status_update(device_serial)
 
         # do work, and finally
-        device_client.disconnect()
+        device_client.disconnect(device_serial)
 
 
 asyncio.run(main())

letpot/deviceclient.py

@@ -54,6 +54,16 @@ class LetPotDevice:
     is_remote: bool | None
 
 
+@dataclass
+class LetPotDeviceInfo:
+    """Information about a device model, based on the serial number."""
+
+    model: str
+    model_name: str | None
+    model_code: str | None
+    features: DeviceFeature
+
+
 @dataclass
 class LetPotDeviceErrors:
     """Device errors model. Errors not supported by the device will be None."""

letpot/models.py

@@ -26,11 +26,15 @@ ruff = "0.12.3"
 pytest = "8.4.1"
 pytest-cov = "6.2.1"
 mypy = "1.17.0"
+pytest-asyncio = "1.0.0"
 
 [tool.poetry.urls]
 Changelog = "https://github.com/jpelgrom/python-letpot/releases"
 Issues = "https://github.com/jpelgrom/python-letpot/issues"
 
+[tool.pytest.ini_options]
+asyncio_mode = "auto"
+
 [build-system]
 requires = ["poetry-core"]
 build-backend = "poetry.core.masonry.api"

poetry.lock

@@ -1 +1,12 @@
 """Tests for Python client for LetPot hydroponic gardens."""
+
+from letpot.models import AuthenticationInfo
+
+AUTHENTICATION = AuthenticationInfo(
+    access_token="access_token",
+    access_token_expires=1738368000,  # 2025-02-01 00:00:00 GMT
+    refresh_token="refresh_token",
+    refresh_token_expires=1740441600,  # 2025-02-25 00:00:00 GMT
+    user_id="a1b2c3d4e5f6a1b2c3d4e5f6",
+    email="[email protected]",
+)

pyproject.toml

@@ -0,0 +1,130 @@
+"""Tests for the device client."""
+
+from collections.abc import AsyncGenerator
+from unittest.mock import MagicMock, patch
+
+from aiomqtt import Client, Message
+
+import pytest_asyncio
+import asyncio
+
+from . import AUTHENTICATION
+
+from letpot.deviceclient import LetPotDeviceClient
+
+
+class MockMessagesIterator:
+    """A simple iterator which waits for messages in a queue."""
+
+    def __init__(self, queue=None):
+        self.queue = queue or asyncio.Queue()
+        self.next_call_count = 0
+
+    def __aiter__(self):
+        return self
+
+    async def __anext__(self):
+        self.next_call_count += 1
+        item = await self.queue.get()
+        if item is StopAsyncIteration:
+            raise StopAsyncIteration
+        return item
+
+
+@pytest_asyncio.fixture()
+async def mock_aiomqtt() -> AsyncGenerator[MagicMock]:
+    """Mock a aiomqtt.Client."""
+
+    with patch("letpot.deviceclient.aiomqtt.Client") as mock_client_class:
+        client = MagicMock(spec=Client)
+        client.messages = MockMessagesIterator()
+
+        mock_client_class.return_value.__aenter__.return_value = client
+
+        yield mock_client_class
+
+
+async def test_subscribe_setup_shutdown(mock_aiomqtt: MagicMock) -> None:
+    """Test subscribing/unsubscribing creates a client and shuts it down."""
+    device_client = LetPotDeviceClient(AUTHENTICATION)
+    topic = "LPH21ABCD/status"
+
+    # Test subscribing sets up a client + subscription
+    await device_client.subscribe(topic, lambda _: None)
+    assert device_client._client is not None
+    assert (
+        device_client._connected is not None
+        and device_client._connected.result() is True
+    )
+
+    # Test unsubscribing cancels the subscription + shuts down client
+    await device_client.unsubscribe(topic)
+    assert device_client._client is None
+    assert device_client._client_task.cancelled()
+
+
+async def test_subscribe_multiple(mock_aiomqtt: MagicMock) -> None:
+    """Test multiple subscriptions use one client and shuts down only when all are done."""
+    device_client = LetPotDeviceClient(AUTHENTICATION)
+    device1 = "LPH21ABCD"
+    device2 = "LPH21DEFG"
+
+    await device_client.subscribe(device1, lambda _: None)
+    await device_client.subscribe(device2, lambda _: None)
+    assert device_client._client is not None
+    assert device_client._client.subscribe.call_count == 2  # type: ignore[attr-defined]
+    # Check number of calls on message queue. Nothing is sent so 1 call = 1 client.
+    assert device_client._client.messages.next_call_count == 1  # type: ignore[attr-defined]
+
+    await device_client.unsubscribe(device1)
+    assert device_client._client.unsubscribe.call_count == 1  # type: ignore[attr-defined]
+    assert device_client._client is not None
+
+    await device_client.unsubscribe(device2)
+    assert device_client._client is None
+    assert device_client._client_task.cancelled()
+
+
+async def test_subscribe_callback(mock_aiomqtt: MagicMock) -> None:
+    """Test subscription receiving a status update passing it to the callback."""
+    device_client = LetPotDeviceClient(AUTHENTICATION)
+    device1 = "LPH21ABCD"
+    device2 = "LPH21DEFG"
+    callback1 = MagicMock()
+    callback2 = MagicMock()
+
+    await device_client.subscribe(device1, callback1)
+    await device_client.subscribe(device2, callback2)
+
+    assert device_client._client is not None
+    device_client._handle_message(
+        Message(
+            topic=f"{device1}/data",
+            payload=b"4d0001126201000101010100000f000f1e01f4000000",
+            qos=0,
+            retain=False,
+            mid=1,
+            properties=None,
+        )
+    )
+    # Only device1 should be called
+    assert callback1.call_count == 1
+    assert not callback2.called
+
+    device_client._handle_message(
+        Message(
+            topic=f"{device2}/data",
+            payload=b"4d0001126201000101010100000f000f1e01f4000000",
+            qos=0,
+            retain=False,
+            mid=1,
+            properties=None,
+        )
+    )
+    # Only device2 should be called, device1 should be same as before
+    assert callback1.call_count == 1
+    assert callback2.call_count == 1
+
+    # Shutdown gracefully
+    await device_client.unsubscribe(device1)
+    await device_client.unsubscribe(device2)

tests/__init__.py

@@ -3,22 +3,13 @@
 import dataclasses
 from datetime import datetime, timedelta
 
-from letpot.models import AuthenticationInfo
-
-INFO = AuthenticationInfo(
-    access_token="abcdef",
-    access_token_expires=0,
-    refresh_token="123456",
-    refresh_token_expires=0,
-    user_id="a1b2c3d4e5f6a1b2c3d4e5f6",
-    email="[email protected]",
-)
+from . import AUTHENTICATION
 
 
 def test_valid_info() -> None:
     """Test auth with access token expiring in the future is valid."""
     auth_info = dataclasses.replace(
-        INFO,
+        AUTHENTICATION,
         access_token_expires=int((datetime.now() + timedelta(days=7)).timestamp()),
         refresh_token_expires=int((datetime.now() + timedelta(days=30)).timestamp()),
     )
@@ -28,7 +19,7 @@ def test_valid_info() -> None:
 def test_expired_info() -> None:
     """Test auth with expired access token is considered invalid."""
     auth_info = dataclasses.replace(
-        INFO,
+        AUTHENTICATION,
         access_token_expires=int((datetime.now() - timedelta(days=7)).timestamp()),
         refresh_token_expires=int((datetime.now() + timedelta(days=14)).timestamp()),
     )