Fix cloud poll flip-flopping TOU values during the write-verify window (#11)

The regular poll cycle republished every mapped CID register on every
cycle, including ones with an in-flight optimistic write awaiting its
delayed verify. A poll landing inside that window overwrote the
just-written value with stale cloud-side data, and the verify task
could then flip it back again.

SolisCloudController now tracks which registers have a pending write
and CloudDataRetrieval skips republishing them until the verify
completes.

Fixes #11

Author: fboundy

custom_components/solisconnect/cloud/cloud_controller.py

@@ -67,6 +67,10 @@ def __init__(
         # Delayed write-verify tasks; cancelled on close_connection so they cannot
         # touch the cache after the entry is unloaded.
         self._verify_tasks: set[asyncio.Task] = set()
+        # Registers with an optimistic write awaiting verify; CloudDataRetrieval skips
+        # republishing these from its regular poll so a poll landing inside the verify
+        # window can't overwrite the just-written value with stale cloud-side data.
+        self._pending_write_registers: set[int] = set()
 
     @property
     def host(self) -> str:
@@ -89,6 +93,10 @@ def last_modbus_success(self) -> datetime | None:
         """Name kept for parity with ModbusController consumers (pseudo-register 90006)."""
         return self.last_cloud_success
 
+    def is_write_pending(self, register: int) -> bool:
+        """True while an optimistic write to this register is still awaiting its delayed verify."""
+        return register in self._pending_write_registers
+
     def _mapping_for_write(self, registers: list[int]) -> CloudCidMapping:
         mapping = REGISTER_TO_CID.get(registers[0])
         if mapping is None or list(mapping.registers) != registers:
@@ -134,6 +142,7 @@ async def _execute_cloud_write(self, mapping: CloudCidMapping, words: list[int])
             cache_save(self.hass, self, register, word)
             notify_register_update(self.hass, self, register, word)
 
+        self._pending_write_registers.update(mapping.registers)
         self._start_verify_task(self._verify_write(mapping, words))
 
     async def _execute_tou_time_write(self, mapping: CloudCidMapping, write_registers: list[int], write_words: list[int]):
@@ -197,6 +206,7 @@ async def _execute_tou_switch_write(self, new_value: int):
         _LOGGER.info("SolisCloud write: TOU switch register %s = %s (was %s)", TOU_SWITCH_REGISTER, new_value, old_value)
         cache_save(self.hass, self, TOU_SWITCH_REGISTER, new_value)
         notify_register_update(self.hass, self, TOU_SWITCH_REGISTER, new_value)
+        self._pending_write_registers.add(TOU_SWITCH_REGISTER)
         self._start_verify_task(self._verify_tou_switch_write(new_value))
 
     async def _read_tou_switch_bitfield(self) -> int:
@@ -212,54 +222,60 @@ async def _read_tou_switch_bitfield(self) -> int:
 
     async def _verify_write(self, mapping: CloudCidMapping, expected_words: list[int]):
         """Re-read the CID after the cloud catches up; never leave an optimistic lie in the cache."""
-        actual_words = None
-        for _attempt in range(2):
-            await asyncio.sleep(_VERIFY_DELAY_SECONDS)
-            try:
-                msg = await self.api.async_at_read(self.serial_number, mapping.cid)
-            except SolisCloudApiError as error:
-                _LOGGER.warning("Verify read of cid %s failed: %s", mapping.cid, error)
-                continue
-            decoded = encode_cid_value(mapping, msg)
-            actual_words = [decoded[r] for r in mapping.registers] if decoded else None
-            if actual_words == expected_words:
-                _LOGGER.debug("SolisCloud write of cid %s verified", mapping.cid)
-                return
-
-        if actual_words is not None and actual_words != expected_words:
-            _LOGGER.warning(
-                "SolisCloud write of cid %s did not stick: wrote %s, device reports %s — restoring device value",
-                mapping.cid,
-                expected_words,
-                actual_words,
-            )
-            for register, word in zip(mapping.registers, actual_words):
-                cache_save(self.hass, self, register, word)
-                notify_register_update(self.hass, self, register, word)
+        try:
+            actual_words = None
+            for _attempt in range(2):
+                await asyncio.sleep(_VERIFY_DELAY_SECONDS)
+                try:
+                    msg = await self.api.async_at_read(self.serial_number, mapping.cid)
+                except SolisCloudApiError as error:
+                    _LOGGER.warning("Verify read of cid %s failed: %s", mapping.cid, error)
+                    continue
+                decoded = encode_cid_value(mapping, msg)
+                actual_words = [decoded[r] for r in mapping.registers] if decoded else None
+                if actual_words == expected_words:
+                    _LOGGER.debug("SolisCloud write of cid %s verified", mapping.cid)
+                    return
+
+            if actual_words is not None and actual_words != expected_words:
+                _LOGGER.warning(
+                    "SolisCloud write of cid %s did not stick: wrote %s, device reports %s — restoring device value",
+                    mapping.cid,
+                    expected_words,
+                    actual_words,
+                )
+                for register, word in zip(mapping.registers, actual_words):
+                    cache_save(self.hass, self, register, word)
+                    notify_register_update(self.hass, self, register, word)
+        finally:
+            self._pending_write_registers.difference_update(mapping.registers)
 
     async def _verify_tou_switch_write(self, expected_value: int):
         """Verify the composed TOU switch bitfield by reading the twelve switch CIDs."""
-        actual_value = None
-        for _attempt in range(2):
-            await asyncio.sleep(_VERIFY_DELAY_SECONDS)
-            try:
-                actual_value = await self._read_tou_switch_bitfield()
-            except SolisCloudApiError as error:
-                _LOGGER.warning("Verify read of TOU switch CIDs failed: %s", error)
-                continue
-            if actual_value == expected_value:
-                _LOGGER.debug("SolisCloud write of TOU switch register %s verified", TOU_SWITCH_REGISTER)
-                return
-
-        if actual_value is not None and actual_value != expected_value:
-            _LOGGER.warning(
-                "SolisCloud write of TOU switch register %s did not stick: wrote %s, device reports %s — restoring device value",
-                TOU_SWITCH_REGISTER,
-                expected_value,
-                actual_value,
-            )
-            cache_save(self.hass, self, TOU_SWITCH_REGISTER, actual_value)
-            notify_register_update(self.hass, self, TOU_SWITCH_REGISTER, actual_value)
+        try:
+            actual_value = None
+            for _attempt in range(2):
+                await asyncio.sleep(_VERIFY_DELAY_SECONDS)
+                try:
+                    actual_value = await self._read_tou_switch_bitfield()
+                except SolisCloudApiError as error:
+                    _LOGGER.warning("Verify read of TOU switch CIDs failed: %s", error)
+                    continue
+                if actual_value == expected_value:
+                    _LOGGER.debug("SolisCloud write of TOU switch register %s verified", TOU_SWITCH_REGISTER)
+                    return
+
+            if actual_value is not None and actual_value != expected_value:
+                _LOGGER.warning(
+                    "SolisCloud write of TOU switch register %s did not stick: wrote %s, device reports %s — restoring device value",
+                    TOU_SWITCH_REGISTER,
+                    expected_value,
+                    actual_value,
+                )
+                cache_save(self.hass, self, TOU_SWITCH_REGISTER, actual_value)
+                notify_register_update(self.hass, self, TOU_SWITCH_REGISTER, actual_value)
+        finally:
+            self._pending_write_registers.discard(TOU_SWITCH_REGISTER)
 
     async def async_read_holding_register(self, register, count=1):
         """Read from the cloud-backed cache, with a live TOU switch read for cold-cache bit toggles."""

custom_components/solisconnect/cloud/cloud_retrieval.py

@@ -104,12 +104,14 @@ async def _update_cycle(self, now=None):
                     continue  # the API returns extra related CIDs beyond those requested
                 if mapping.merge_register_bit is not None:
                     continue  # TOU V2 switch bits are merged together below, not per-CID
+                if any(controller.is_write_pending(register) for register in mapping.registers):
+                    continue  # an optimistic write is still awaiting its delayed verify; don't stomp on it
                 words = encode_cid_value(mapping, msg)
                 if words:
                     self._publish_words(words)
                     cid_words += len(words)
 
-            switch_words = self._merge_switch_bits(batch)
+            switch_words = None if controller.is_write_pending(TOU_SWITCH_REGISTER) else self._merge_switch_bits(batch)
             if switch_words:
                 self._publish_words(switch_words)
                 cid_words += len(switch_words)

docs/source/protocols.md

@@ -31,7 +31,7 @@ Current cloud write support is limited to verified CIDs and CID families:
 | `5947-5963`, `5966-5986` | `43709-43710`, ... / `43751-43752`, ... | TOU V2 slot current/voltage values |
 | `5946`, `5949`, `5952`, `5955`, `5958`, `5961`, `5964`, `5968`, `5972`, `5976`, `5980`, `5987` | `43711-43714`, ... / `43753-43756`, ... | TOU V2 `HH:MM-HH:MM` time slots |
 
-Cloud writes read the previous CID value, send the control command, update the cache optimistically, and verify after SolisCloud catches up. If the cloud value does not match, SolisConnect restores the value reported by the device.
+Cloud writes read the previous CID value, send the control command, update the cache optimistically, and verify after SolisCloud catches up. If the cloud value does not match, SolisConnect restores the value reported by the device. While a write's verify is pending (up to ~15-30s), the regular poll cycle skips republishing that specific register so it can't overwrite the optimistic value with stale cloud-side data in the meantime.
 
 The TOU V2 timed-slot CID family (`5916-5987`) is mapped into SolisConnect's local `43707-43791` register model. The remaining caveat is feature detection: SolisCloud CID `6798` reports whether TOU V2 is enabled (`43605` / `0xAA55`), but SolisConnect does not yet use that gate to hide or disable the TOU V2 cloud mappings automatically. As a related data point, TOU V2 hardware/firmware shipped from HMI version `FB00` onward (see below); this is not yet used as an alternate detection signal either.
 

tests/test_cloud_writes.py

@@ -94,6 +94,34 @@ async def test_verify_mismatch_restores_device_truth(hass, controller):
     assert hass.data[DOMAIN][VALUES][f"{SERIAL}|43110"] == 35  # device truth restored
 
 
+async def test_write_marks_pending_until_verify_completes(hass, controller):
+    """Bug #11: a register must stay marked pending for the whole verify window, then clear."""
+    with patch.object(controller.hass, "async_create_task", side_effect=_close_background_task):
+        await controller.async_write_holding_register(43110, 33)
+
+    assert controller.is_write_pending(43110) is True  # verify task not run yet (closed instead)
+
+    controller.api.async_at_read = AsyncMock(return_value="33")
+    with patch("custom_components.solisconnect.cloud.cloud_controller.asyncio.sleep", new=AsyncMock()):
+        await controller._verify_write(CLOUD_CID_MAP[636], [33])
+
+    assert controller.is_write_pending(43110) is False
+
+
+async def test_tou_switch_write_marks_pending_until_verify_completes(hass, controller):
+    controller.api.async_at_read_batch = AsyncMock(return_value={5916: "0", 5917: "0"})  # cold-cache bitfield read
+    with patch.object(controller.hass, "async_create_task", side_effect=_close_background_task):
+        await controller.async_write_holding_register(43707, 0b0101)
+
+    assert controller.is_write_pending(43707) is True
+
+    controller.api.async_at_read_batch = AsyncMock(return_value={5916: "1", 5918: "1"})
+    with patch("custom_components.solisconnect.cloud.cloud_controller.asyncio.sleep", new=AsyncMock()):
+        await controller._verify_tou_switch_write(0b0101)
+
+    assert controller.is_write_pending(43707) is False
+
+
 async def test_multi_register_write_without_mapping_raises(controller):
     with pytest.raises(HomeAssistantError, match="no control CID mapping"):
         await controller.async_write_holding_registers(43143, [10, 30])
@@ -209,3 +237,32 @@ def test_warm_cache_partial_batch_only_touches_observed_bits(self, hass, retriev
         hass.data[DOMAIN][VALUES][f"{SERIAL}|43707"] = 0b0110  # bits 1 and 2 set
         batch = {5916: "1"}  # only bit 0 observed this cycle
         assert retrieval._merge_switch_bits(batch) == {43707: 0b0111}
+
+
+async def test_poll_does_not_overwrite_register_with_pending_write(hass, controller):
+    """Bug #11: a poll landing inside the write-verify window must not stomp the optimistic value."""
+    hass.data[DOMAIN][VALUES][f"{SERIAL}|43110"] = 33  # optimistic write already applied
+    controller._pending_write_registers.add(43110)
+    controller.inverter_id = "id1"
+    controller.api.async_inverter_detail = AsyncMock(return_value={})
+    controller.api.async_at_read_batch = AsyncMock(return_value={636: "99"})  # stale cloud-side value
+
+    with patch.object(hass, "async_create_task", side_effect=_close_background_task):
+        retrieval = CloudDataRetrieval(hass=hass, controller=controller)
+    await retrieval._update_cycle()
+
+    assert hass.data[DOMAIN][VALUES][f"{SERIAL}|43110"] == 33
+
+
+async def test_poll_does_not_overwrite_tou_switch_register_with_pending_write(hass, controller):
+    hass.data[DOMAIN][VALUES][f"{SERIAL}|43707"] = 0b0101
+    controller._pending_write_registers.add(43707)
+    controller.inverter_id = "id1"
+    controller.api.async_inverter_detail = AsyncMock(return_value={})
+    controller.api.async_at_read_batch = AsyncMock(return_value={5916: "1", 5917: "1", 5918: "1"})
+
+    with patch.object(hass, "async_create_task", side_effect=_close_background_task):
+        retrieval = CloudDataRetrieval(hass=hass, controller=controller)
+    await retrieval._update_cycle()
+
+    assert hass.data[DOMAIN][VALUES][f"{SERIAL}|43707"] == 0b0101