Fixup detection and commands to work across both Hybrid & AIO models

custom_components/givenergy_local/coordinator.py

@@ -4,7 +4,7 @@
 
 import asyncio
 from dataclasses import dataclass
-from datetime import datetime, timedelta
+from datetime import UTC, datetime, timedelta
 from logging import getLogger
 
 from homeassistant.core import HomeAssistant
@@ -91,9 +91,12 @@ async def _async_update_data(self) -> Plant:
         if not self.client.connected:
             await self.client.connect()
             await self.client.detect_plant()
-            self.require_full_refresh = True
+            self.last_full_refresh = datetime.now(UTC)
+
+            # Detection performs a full refresh - no need to trigger another one now
+            return self.client.plant
 
-        if self.last_full_refresh < (datetime.utcnow() - _FULL_REFRESH_INTERVAL):
+        if self.last_full_refresh < (datetime.now(UTC) - _FULL_REFRESH_INTERVAL):
             self.require_full_refresh = True
 
         # Allow a few attempts to pull back valid data.
@@ -135,7 +138,7 @@ async def _async_update_data(self) -> Plant:
 
             if self.require_full_refresh:
                 self.require_full_refresh = False
-                self.last_full_refresh = datetime.utcnow()
+                self.last_full_refresh = datetime.now(UTC)
             return plant
 
         raise UpdateFailed(

custom_components/givenergy_local/givenergy_modbus/client/client.py

@@ -4,7 +4,9 @@
 from asyncio import Future, Queue, StreamReader, StreamWriter, Task
 from typing import Callable, Dict, List, Optional, Tuple
 
-from custom_components.givenergy_local.givenergy_modbus.client import commands
+from custom_components.givenergy_local.givenergy_modbus.client.commands import (
+    CommandBuilder,
+)
 from custom_components.givenergy_local.givenergy_modbus.exceptions import (
     CommunicationError,
     ExceptionBase,
@@ -31,14 +33,14 @@ class Client:
     framer: Framer
     expected_responses: "Dict[int, Future[TransparentResponse]]" = {}
     plant: Plant
+    command_builder: CommandBuilder
     # refresh_count: int = 0
     # debug_frames: Dict[str, Queue]
     connected = False
     reader: StreamReader
     writer: StreamWriter
     network_consumer_task: Task
     network_producer_task: Task
-    slave_address: int
 
     tx_queue: "Queue[Tuple[bytes, Optional[Future]]]"
 
@@ -48,6 +50,7 @@ def __init__(self, host: str, port: int, connect_timeout: float = 2.0) -> None:
         self.connect_timeout = connect_timeout
         self.framer = ClientFramer()
         self.plant = Plant()
+        self.command_builder = CommandBuilder()
         self.tx_queue = Queue(maxsize=20)
         # self.debug_frames = {
         #     'all': Queue(maxsize=1000),
@@ -75,32 +78,45 @@ async def connect(self) -> None:
         )
         # asyncio.create_task(self._task_dump_queues_to_files(), name='dump_queues_to_files'),
         self.connected = True
-        self.slave_address = 0x32
         _logger.info("Connection established to %s:%d", self.host, self.port)
 
     async def detect_plant(self, timeout: int = 1, retries: int = 3) -> None:
-        """Detect inverter capabilities that influence how subsequent requests are made."""
-        _logger.info("Detectig plant")
+        """Detect inverter capabilities that influence how subsequent requests are made.
+
+        A full refresh of all data is performed in the process."""
 
         # Refresh the core set of registers that work across all inverters
+        _logger.info("Performing model detection")
+        await self.refresh_plant(
+            True, max_batteries=0, timeout=timeout, retries=retries
+        )
+
+        # Different models accept slightly different commands, for example, querying an
+        # AIO for battery information will time out. Now we know the model, refresh
+        # all data again.
+        _logger.info(
+            "Detecting additional capabilities based on model: %s",
+            Model(self.plant.inverter.model).name,
+        )
+        self.command_builder = CommandBuilder(self.plant.inverter.model)
         await self.refresh_plant(True, timeout=timeout, retries=retries)
-        _logger.info("Model detected: %s", Model(self.plant.inverter.model).name)
 
         # Use that to detect the number of batteries
-        # TODO: Reinstate battery detection
-        # self.plant.detect_batteries()
-        # _logger.info("Batteries detected: %d", self.plant.number_batteries)
-
-        # Update the slave address for subsequent requests
-        if self.plant.inverter.model != Model.ALL_IN_ONE:
-            self.slave_address = 0x32
+        self.plant.detect_batteries()
+        _logger.info(
+            "Detected %d external %s connected",
+            self.plant.number_batteries,
+            "battery" if self.plant.number_batteries == 1 else "batteries",
+        )
 
         # Some devices support additional registers
         # When unsupported, devices appear to simply ignore requests
+        # If we had a clear mapping of what's supported across models/generations,
+        # this could be moved to the CommandBuilder
         possible_additional_holding_registers = [300]
         for hr in possible_additional_holding_registers:
             try:
-                reqs = commands.refresh_additional_holding_registers(hr)
+                reqs = self.command_builder.refresh_additional_holding_registers(hr)
                 await self.execute(reqs, timeout=timeout, retries=retries)
                 _logger.info(
                     "Detected additional holding register support (base_register=%d)",
@@ -158,7 +174,7 @@ async def refresh_plant(
         retries: int = 0,
     ) -> Plant:
         """Refresh data about the Plant."""
-        reqs = commands.refresh_plant_data(
+        reqs = self.command_builder.refresh_plant_data(
             full_refresh, self.plant.number_batteries, max_batteries
         )
         await self.execute(reqs, timeout=timeout, retries=retries)
@@ -176,13 +192,14 @@ async def watch_plant(
         """Refresh data about the Plant."""
         await self.connect()
         await self.detect_plant()
-        await self.refresh_plant(True, max_batteries=max_batteries)
         while True:
             if handler:
                 handler()
             await asyncio.sleep(refresh_period)
             if not passive:
-                reqs = commands.refresh_plant_data(False, self.plant.number_batteries)
+                reqs = self.command_builder.refresh_plant_data(
+                    False, self.plant.number_batteries
+                )
                 await self.execute(
                     reqs, timeout=timeout, retries=retries, return_exceptions=True
                 )
@@ -275,11 +292,6 @@ def execute(
         return_exceptions: bool = False,
     ) -> "Future[List[TransparentResponse]]":
         """Helper to perform multiple requests in bulk."""
-        # TODO: Will clobber the slave_address for batteries
-        # Not good for AIO
-        for request in requests:
-            request.slave_address = self.slave_address
-
         return asyncio.gather(
             *[
                 self.send_request_and_await_response(