bugfix: avoid request collisions when enabling timed charging (#152)

Author: cdpuk

custom_components/givenergy_local/coordinator.py

@@ -43,6 +43,25 @@
 _RECONNECT_BACKOFF_MAX = 60.0
 
 
+def _dedupe_requests(requests: list[TransparentRequest]) -> list[TransparentRequest]:
+    """Collapse repeat writes to the same register within a single batch.
+
+    Client.execute() gathers requests concurrently, and Client keys in-flight
+    requests by expected-response shape - which covers the register, not the
+    value. Two requests sharing a shape make the second cancel the first in
+    flight, and gather(return_exceptions=False) then aborts the whole batch
+    with the resulting CancelledError. Later entries win, so an intentional
+    final value for a register is preserved.
+    """
+    unique: dict[int, TransparentRequest] = {}
+    for request in requests:
+        key = request.expected_response().shape_hash()
+        if key in unique:
+            _LOGGER.warning("Dropping duplicate request from batch: %s", request)
+        unique[key] = request
+    return list(unique.values())
+
+
 class GivEnergyUpdateCoordinator(DataUpdateCoordinator[Plant]):
     """Update coordinator that fetches data from a GivEnergy inverter."""
 
@@ -216,6 +235,7 @@ async def _async_update_data(self) -> Plant:
 
     async def execute(self, requests: list[TransparentRequest]) -> None:
         """Execute a set of requests and force an update to read any new values."""
+        requests = _dedupe_requests(requests)
         try:
             async with asyncio.timeout(_EXECUTE_TIMEOUT):
                 await self.client.execute(requests, _COMMAND_TIMEOUT, _COMMAND_RETRIES)

custom_components/givenergy_local/manifest.json

@@ -13,5 +13,5 @@
   "requirements": [
     "givenergy-modbus>=2.13.0,<3"
   ],
-  "version": "2.5.2"
+  "version": "2.5.3"
 }

custom_components/givenergy_local/services.py

@@ -12,6 +12,7 @@
 
 from givenergy_modbus.client import commands as ge_commands
 from givenergy_modbus.model import TimeSlot
+from givenergy_modbus.model.slot_map import SINGLE_PHASE_SLOTS
 from givenergy_modbus.pdu.transparent import TransparentRequest
 
 from .const import DOMAIN, LOGGER
@@ -184,12 +185,19 @@ async def _async_enable_timed_charge(hass: HomeAssistant, data: dict[str, Any])
     Note that this isn't a battery mode like "Timed Discharge", "Eco", etc. It operates in
     parallel to those modes.
     """
-    commands = ge_commands.set_enable_charge(True)
+    commands: list[TransparentRequest] = []
 
-    if _ATTR_START_TIME in data and _ATTR_END_TIME in data:
+    if _ATTR_START_TIME in data:
         start_time = datetime.time.fromisoformat(data[_ATTR_START_TIME])
+        commands.extend(
+            ge_commands.set_charge_slot_start(1, start_time, SINGLE_PHASE_SLOTS)
+        )
+
+    if _ATTR_END_TIME in data:
         end_time = datetime.time.fromisoformat(data[_ATTR_END_TIME])
-        commands.extend(ge_commands.set_charge_slot_1(TimeSlot(start_time, end_time)))
+        commands.extend(
+            ge_commands.set_charge_slot_end(1, end_time, SINGLE_PHASE_SLOTS)
+        )
 
     if _ATTR_CHARGE_TARGET in data:
         target_soc = int(data[_ATTR_CHARGE_TARGET])
@@ -198,8 +206,13 @@ async def _async_enable_timed_charge(hass: HomeAssistant, data: dict[str, Any])
         # bounces between 99-100% in a charge/discharge cycle, so avoid this, matching
         # behaviour of GivEnergy logic. set_charge_target_enabled() applies exactly
         # this rule: it enables charging and, for a target of 100%, clears the charge
-        # target rather than setting it.
+        # target rather than setting it. It also enables charging itself, so it must
+        # not be combined with a separate set_enable_charge() call below - the two
+        # would write ENABLE_CHARGE (HR 96) twice in the same batch, and the client
+        # cancels the first of any two same-register writes it sees in flight together.
         commands.extend(ge_commands.set_charge_target_enabled(target_soc))
+    else:
+        commands.extend(ge_commands.set_enable_charge(True))
 
     LOGGER.debug("Activating timed charge mode")
     await _async_service_call(hass, data[ATTR_DEVICE_ID], commands)

requirements_test.txt

@@ -1,2 +1,2 @@
-givenergy-modbus>=2.12.1,<3
+givenergy-modbus>=2.13.0,<3
 pytest-homeassistant-custom-component