Skip to content

Commit d606919

Browse files
committed
Fix TOU V2 switch bitfield edge cases (#10)
Mask the cached old value to 12 bits before diffing/sending as yuanzhi, so a stray high bit never leaks into a control write. Replace the per-CID bitfield merge with a whole-batch merge that only publishes register 43707 on a cold cache once every slot-switch CID has been observed, instead of defaulting unseen bits to off. Exclude 43707 from REGISTER_TO_CID so it can no longer collide with an arbitrary switch mapping's CID. Fixes #10
1 parent 589321a commit d606919

6 files changed

Lines changed: 103 additions & 16 deletions

File tree

‎custom_components/solisconnect/cloud/cloud_controller.py‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -168,7 +168,7 @@ async def _execute_tou_switch_write(self, new_value: int):
168168
new_value &= 0x0FFF
169169
old_value = cache_get(self.hass, self, TOU_SWITCH_REGISTER)
170170
if old_value is not None:
171-
old_value = int(old_value)
171+
old_value = int(old_value) & 0x0FFF
172172
else:
173173
try:
174174
old_value = await self._read_tou_switch_bitfield()

‎custom_components/solisconnect/cloud/cloud_retrieval.py‎

Lines changed: 39 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,8 @@
1616
from custom_components.solisconnect.sensor_data.cloud_mapping import (
1717
CLOUD_CID_MAP,
1818
CLOUD_INPUT_MAP,
19+
TOU_SWITCH_CIDS_BY_BIT,
20+
TOU_SWITCH_REGISTER,
1921
encode_cid_value,
2022
encode_engineering_value,
2123
registers_covered,
@@ -100,13 +102,18 @@ async def _update_cycle(self, now=None):
100102
mapping = CLOUD_CID_MAP.get(cid)
101103
if mapping is None:
102104
continue # the API returns extra related CIDs beyond those requested
105+
if mapping.merge_register_bit is not None:
106+
continue # TOU V2 switch bits are merged together below, not per-CID
103107
words = encode_cid_value(mapping, msg)
104108
if words:
105-
if mapping.merge_register_bit is not None:
106-
words = self._merge_bit_words(words, mapping.merge_register_bit)
107109
self._publish_words(words)
108110
cid_words += len(words)
109111

112+
switch_words = self._merge_switch_bits(batch)
113+
if switch_words:
114+
self._publish_words(switch_words)
115+
cid_words += len(switch_words)
116+
110117
controller.last_cloud_success = datetime.now(UTC)
111118
# INFO so it is visible in `ha core logs` when diagnosing live installs
112119
_LOGGER.info("Cloud poll complete: %s detail registers, %s cid registers", published, cid_words)
@@ -140,16 +147,37 @@ def _publish_words(self, words: dict[int, int]):
140147
cache_save(self.hass, self.controller, register, word)
141148
notify_register_update(self.hass, self.controller, register, word)
142149

143-
def _merge_bit_words(self, words: dict[int, int], bit_position: int) -> dict[int, int]:
144-
"""Merge one CID switch value into the shared TOU V2 bitfield register."""
145-
register, bit_value = next(iter(words.items()))
146-
current = cache_get(self.hass, self.controller, register)
147-
merged = int(current) if current is not None else 0
148-
if int(bit_value):
149-
merged |= 1 << bit_position
150+
def _merge_switch_bits(self, batch: dict[int, str]) -> dict[int, int] | None:
151+
"""Build the merged TOU V2 switch register from this poll's batch reads plus the cache.
152+
153+
A switch CID missing from one poll's batch response must not be treated as "off": on a
154+
cold cache (nothing published yet) we only publish once every switch bit has been observed
155+
at least once, so a transiently-missing CID can never permanently zero a bit it never
156+
actually reported. On a warm cache, a missing CID simply leaves its bit as last observed.
157+
"""
158+
observed: dict[int, int] = {}
159+
for bit, cid in TOU_SWITCH_CIDS_BY_BIT.items():
160+
msg = batch.get(cid)
161+
if msg is None:
162+
continue
163+
words = encode_cid_value(CLOUD_CID_MAP[cid], msg)
164+
if words:
165+
observed[bit] = words[TOU_SWITCH_REGISTER]
166+
167+
current = cache_get(self.hass, self.controller, TOU_SWITCH_REGISTER)
168+
if current is None:
169+
if len(observed) < len(TOU_SWITCH_CIDS_BY_BIT):
170+
return None # incomplete cold-cache read; defer until a cycle observes every bit
171+
merged = 0
150172
else:
151-
merged &= ~(1 << bit_position)
152-
return {register: merged}
173+
merged = int(current)
174+
175+
for bit, value in observed.items():
176+
if value:
177+
merged |= 1 << bit
178+
else:
179+
merged &= ~(1 << bit)
180+
return {TOU_SWITCH_REGISTER: merged}
153181

154182
def _publish_status(self):
155183
"""Pseudo-registers used by diagnostic derived sensors (same as DataRetrieval)."""

‎custom_components/solisconnect/sensor_data/cloud_mapping.py‎

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -348,8 +348,14 @@ def _tou_v2_mappings() -> list[CloudCidMapping]:
348348

349349
CLOUD_CID_MAP: dict[int, CloudCidMapping] = {m.cid: m for m in _CLOUD_CID_MAPPINGS}
350350

351-
# Write routing: which CID a holding-register write lands on
352-
REGISTER_TO_CID: dict[int, CloudCidMapping] = {register: mapping for mapping in CLOUD_CID_MAP.values() for register in mapping.registers}
351+
# Write routing: which CID a holding-register write lands on. TOU V2 switch-slot mappings are
352+
# excluded here: all twelve of them share register 43707, so a plain dict comprehension would
353+
# silently collapse to whichever mapping iterates last. Writes to 43707 are handled by the
354+
# dedicated TOU_SWITCH_REGISTER special-case instead; any future caller that looks up 43707
355+
# here directly should get a clear miss rather than a wrong CID.
356+
REGISTER_TO_CID: dict[int, CloudCidMapping] = {
357+
register: mapping for mapping in CLOUD_CID_MAP.values() if mapping.merge_register_bit is None for register in mapping.registers
358+
}
353359
TOU_SWITCH_REGISTER = 43707
354360
TOU_SWITCH_CIDS_BY_BIT: dict[int, int] = {
355361
mapping.merge_register_bit: mapping.cid for mapping in CLOUD_CID_MAP.values() if mapping.merge_register_bit is not None

‎docs/source/tou.md‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -40,7 +40,7 @@ The newer, richer scheduling mechanism with independent settings per slot.
4040
- `+2`: cut-off voltage (V, editable `number`, 0-300, ×0.1)
4141
- `+3..+6`: start hour, start minute, end hour, end minute (paired into a `time` entity)
4242
- SolisCloud mapping (`sensor_data/cloud_mapping.py`, `_tou_v2_mappings()`):
43-
- Slot enable bits -> CIDs `5916-5927` (charge slots 1-6, then discharge slots 1-6), merged into the shared `43707` bitfield by `CloudDataRetrieval._merge_bit_words`.
43+
- Slot enable bits -> CIDs `5916-5927` (charge slots 1-6, then discharge slots 1-6), merged into the shared `43707` bitfield by `CloudDataRetrieval._merge_switch_bits`. On a cold cache the merged register is only published once all 12 slot-switch CIDs have been observed in a poll cycle, so a CID transiently missing from one `atReadBatch` response is never mistaken for "off".
4444
- Target SOC -> CIDs `5928-5933` (charge), `5965, 5969, 5973, 5977, 5981, 5984` (discharge).
4545
- Battery current -> CIDs `5948-5963` family (charge), `5967-5986` family (discharge).
4646
- Cut-off voltage -> the paired CID in the same current/voltage CID families.

‎tests/test_cloud_integration.py‎

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -48,8 +48,20 @@
4848
636: "33",
4949
157: "52",
5050
158: "15",
51+
# All 12 TOU V2 switch-slot CIDs must be present for the merged 43707 bitfield to publish
52+
# on a cold cache (see CloudDataRetrieval._merge_switch_bits): a partial batch is treated
53+
# as "unknown so far" rather than defaulting the unseen bits to off.
5154
5916: "1",
5255
5917: "0",
56+
5918: "0",
57+
5919: "0",
58+
5920: "0",
59+
5921: "0",
60+
5922: "0",
61+
5923: "0",
62+
5924: "0",
63+
5925: "0",
64+
5926: "0",
5365
5927: "1",
5466
5946: "09:30-10:45",
5567
5948: "12.5",

‎tests/test_cloud_writes.py‎

Lines changed: 42 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,8 +8,9 @@
88

99
from custom_components.solisconnect.cloud.api_client import SolisCloudApiError
1010
from custom_components.solisconnect.cloud.cloud_controller import SolisCloudController
11+
from custom_components.solisconnect.cloud.cloud_retrieval import CloudDataRetrieval
1112
from custom_components.solisconnect.const import DOMAIN, VALUES
12-
from custom_components.solisconnect.sensor_data.cloud_mapping import CLOUD_CID_MAP, cid_msg_from_words
13+
from custom_components.solisconnect.sensor_data.cloud_mapping import CLOUD_CID_MAP, REGISTER_TO_CID, cid_msg_from_words
1314

1415
SERIAL = "CLOUDSN1"
1516

@@ -168,3 +169,43 @@ async def test_verify_tou_switch_mismatch_restores_device_truth(hass, controller
168169

169170
assert controller.api.async_at_read_batch.await_count == 2
170171
assert hass.data[DOMAIN][VALUES][f"{SERIAL}|43707"] == 0b0001
172+
173+
174+
async def test_tou_switch_write_masks_stray_bits_in_cached_old_value(hass, controller):
175+
"""A corrupted/stray high bit in the cached bitfield must not leak into the API's old_value."""
176+
hass.data[DOMAIN][VALUES][f"{SERIAL}|43707"] = 0x1003 # bits 0-1 set, plus a stray bit 12
177+
178+
with patch.object(controller.hass, "async_create_task", side_effect=_close_background_task):
179+
await controller.async_write_holding_register(43707, 0b0101)
180+
181+
assert controller.api.async_control.await_args_list[0].kwargs["old_value"] == "3"
182+
183+
184+
def test_register_to_cid_excludes_tou_switch_register():
185+
"""All 12 TOU V2 switch mappings share register 43707; it must not resolve to any single CID."""
186+
assert 43707 not in REGISTER_TO_CID
187+
188+
189+
class TestMergeSwitchBits:
190+
"""CloudDataRetrieval._merge_switch_bits: cold-cache-incomplete-batch handling for bug #10."""
191+
192+
@pytest.fixture
193+
def retrieval(self, hass, controller):
194+
hass.data[DOMAIN][VALUES] = {}
195+
with patch.object(hass, "async_create_task", side_effect=_close_background_task):
196+
return CloudDataRetrieval(hass=hass, controller=controller)
197+
198+
def test_cold_cache_incomplete_batch_defers_publish(self, retrieval):
199+
"""Only 3 of 12 switch CIDs observed and no prior cache: must not guess the other 9 bits are off."""
200+
batch = {5916: "1", 5917: "0", 5927: "1"}
201+
assert retrieval._merge_switch_bits(batch) is None
202+
203+
def test_cold_cache_complete_batch_publishes(self, retrieval):
204+
batch = {cid: ("1" if cid in (5916, 5927) else "0") for cid in [5916, 5917, 5918, 5919, 5920, 5921, 5922, 5923, 5924, 5925, 5926, 5927]}
205+
assert retrieval._merge_switch_bits(batch) == {43707: (1 << 0) | (1 << 11)}
206+
207+
def test_warm_cache_partial_batch_only_touches_observed_bits(self, hass, retrieval):
208+
"""With a warm cache, a CID missing from this poll's batch must leave its bit untouched."""
209+
hass.data[DOMAIN][VALUES][f"{SERIAL}|43707"] = 0b0110 # bits 1 and 2 set
210+
batch = {5916: "1"} # only bit 0 observed this cycle
211+
assert retrieval._merge_switch_bits(batch) == {43707: 0b0111}

0 commit comments

Comments
 (0)