Skip to content

Commit 65969f0

Browse files
0xAHAclaude
andcommitted
Bump version to v1.3.4 - a missing 32-bit pair partner no longer decodes as zero (#367)
Reported by tdalejandro: pv1_power published as 65,536,000 W and pv2_power as 109,544,683 W, with the same values recurring byte-for-byte 14 hours apart. The repetition identified the cause. Decoded as 32-bit pairs, two of the three impossible values had a low word of exactly zero. _get_register_value() substituted 0 for a pair register missing from the read cache, so a truncated block capturing the high word but not the low word decoded as (high << 16) - a high word of 10000 becoming 65,536,000 W. Those values went into HA long-term statistics as real measurements. The protocol is unambiguous: UINT32/INT32 are defined as high word first, low word last, and every 32-bit register table entry declares a length of 2. A 32-bit value always occupies both registers, so a missing partner cannot mean zero - it means the read did not complete. The decoder now returns None. Protocol-level rather than profile-specific, so it applies to every 32-bit register in every profile. The only behaviour change is that a value which was previously garbage now reads as no value; a complete pair whose high word is genuinely 0 still decodes normally, which is covered by test. Three regression tests added. 191 pass, including every pre-existing decode test. NOT fixed: five call sites in the WIT battery-power path use the same substitution, but there pair_addr may legitimately be None - the profile genuinely having no high word - which is a different case from a register that exists but was not read. Separating those safely needs a WIT owner to verify given that path's history in #247 and #323. Co-Authored-By: Claude <noreply@anthropic.com>
1 parent d2acc16 commit 65969f0

5 files changed

Lines changed: 109 additions & 4 deletions

File tree

README.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@
33
# Growatt Modbus Integration for Home Assistant ☀️
44

55
![HACS Badge](https://img.shields.io/badge/HACS-Custom-orange.svg)
6-
![Version](https://img.shields.io/badge/Version-1.3.3-blue.svg)
6+
![Version](https://img.shields.io/badge/Version-1.3.4-blue.svg)
77
[![GitHub Issues](https://img.shields.io/github/issues/0xAHA/Growatt_ModbusTCP.svg)](https://github.com/0xAHA/Growatt_ModbusTCP/issues)
88
[![GitHub Stars](https://img.shields.io/github/stars/0xAHA/Growatt_ModbusTCP.svg?style=social)](https://github.com/0xAHA/Growatt_ModbusTCP)
99

RELEASENOTES.md

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,45 @@
44

55
---
66

7+
## v1.3.4
8+
9+
Issues: #367
10+
11+
- **Fix: a truncated read could fabricate physically impossible values and write them into
12+
long-term statistics.**
13+
Reported by @tdalejandro with unusually good evidence — `pv1_power` published as
14+
**65,536,000 W**, `pv2_power` as **109,544,683 W**, and the same values recurring
15+
byte-for-byte 14 hours apart. That repetition is what identified the cause.
16+
17+
Decoded as 32-bit pairs, two of the three impossible values had a low word of *exactly
18+
zero*. The decoder was substituting `0` for a pair register missing from the read cache,
19+
so a truncated block that captured the high word and not the low word decoded as
20+
`high << 16` — a high word of 10000 becoming 65,536,000 W.
21+
22+
The protocol leaves no ambiguity here: `UINT32`/`INT32` are defined as "high word first,
23+
low word last", and every 32-bit entry in the register table declares a length of 2. A
24+
32-bit value always occupies both registers, so a missing partner cannot mean zero — it
25+
means the read did not complete. The decoder now returns no value in that case.
26+
27+
This is protocol-level rather than profile-specific, so it applies to every 32-bit
28+
register across every profile.
29+
30+
**What changes for you:** nothing, unless your gateway is truncating responses. Values
31+
that previously appeared as millions of watts will now read 0 for that poll instead.
32+
Still not ideal, but it no longer corrupts Energy Dashboard history — which is
33+
permanent, and has to be repaired by hand.
34+
35+
- **Note:** if you already have corrupted hourly statistics, Developer Tools → Statistics
36+
can correct the affected means without touching the database.
37+
38+
- **Known remaining:** five call sites in the WIT battery-power path use the same
39+
substitution. They are not fixed here because in that code `pair_addr` may legitimately
40+
be `None` — meaning the profile genuinely has no high word — which is a different case
41+
from "the register exists but wasn't read". Separating those safely needs a WIT owner to
42+
verify, given that path's history in #247 and #323.
43+
44+
---
45+
746
## v1.3.3
847

948
Issues: #361

custom_components/growatt_modbus/growatt_modbus.py

Lines changed: 22 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1229,16 +1229,36 @@ def _get_register_value(self, address: int) -> Optional[float]:
12291229
scale = reg_info.get('scale', 1)
12301230
return raw_value * scale
12311231

1232+
# A 32-bit value ALWAYS occupies both registers — the protocol defines
1233+
# UINT32/INT32 as "high word first, low word last", and every 32-bit entry in
1234+
# the register table declares a length of 2. So a partner missing from the
1235+
# cache never means "zero"; it means the read did not complete.
1236+
#
1237+
# Substituting 0 here fabricated values out of data that never arrived. A
1238+
# truncated block that captured the high word but not the low word decoded as
1239+
# (high << 16): a high word of 10000 was published as 65,536,000 W of PV power,
1240+
# and went straight into Home Assistant's long-term statistics (Issue #367).
1241+
#
1242+
# Returning None instead lets the caller treat the value as unread.
1243+
partner_value = self._register_cache.get(pair_addr)
1244+
if partner_value is None:
1245+
logger.debug(
1246+
"Register %d is a 32-bit pair with %d, but %d was not read — "
1247+
"returning no value rather than assuming 0",
1248+
address, pair_addr, pair_addr,
1249+
)
1250+
return None
1251+
12321252
# Check which register is HIGH and which is LOW
12331253
if address < pair_addr:
12341254
# Current address is HIGH, pair is LOW
12351255
high_value = raw_value
1236-
low_value = self._register_cache.get(pair_addr, 0)
1256+
low_value = partner_value
12371257
combined_scale = pair_info.get('combined_scale', 1)
12381258
else:
12391259
# Current address is LOW, pair is HIGH
12401260
low_value = raw_value
1241-
high_value = self._register_cache.get(pair_addr, 0)
1261+
high_value = partner_value
12421262
combined_scale = reg_info.get('combined_scale', 1)
12431263

12441264
# Combine 32-bit value

custom_components/growatt_modbus/manifest.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,5 +12,5 @@
1212
"pymodbus>=3.0.0",
1313
"pyserial>=3.4"
1414
],
15-
"version": "1.3.3"
15+
"version": "1.3.4"
1616
}

tests/test_register_decoding.py

Lines changed: 46 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -130,6 +130,52 @@ def test_register_absent_from_cache_returns_none_not_zero():
130130
assert client._get_register_value(400) is None
131131

132132

133+
def test_pair_with_missing_partner_returns_none_not_a_fabricated_value():
134+
"""Regression guard for Issue #367.
135+
136+
A truncated block read can capture a 32-bit value's high word and not its low word.
137+
The decoder used to substitute 0 for the missing half, producing (high << 16) — a
138+
high word of 10000 was published as 65,536,000 W of PV power and written into Home
139+
Assistant's long-term statistics as if it were a real measurement.
140+
141+
The protocol defines UINT32/INT32 as "high word first, low word last" and every
142+
32-bit register table entry declares a length of 2, so a missing partner can only
143+
mean the read failed. It must decode to nothing.
144+
"""
145+
regs = {
146+
5: {"name": "pv1_power_high", "scale": 1, "pair": 6},
147+
6: {"name": "pv1_power_low", "scale": 1, "pair": 5, "combined_scale": 0.1},
148+
}
149+
# High word arrived, low word did not — exactly the reported failure.
150+
client = _client(regs, {5: 10000})
151+
assert client._get_register_value(5) is None
152+
153+
# And the same when only the low word arrived.
154+
assert _client(regs, {6: 1234})._get_register_value(6) is None
155+
156+
157+
def test_the_exact_reported_value_can_no_longer_be_produced():
158+
"""65,536,000 W from Issue #367 was (10000 << 16) * 0.1 with a missing low word."""
159+
regs = {
160+
5: {"name": "pv1_power_high", "scale": 1, "pair": 6},
161+
6: {"name": "pv1_power_low", "scale": 1, "pair": 5, "combined_scale": 0.1},
162+
}
163+
assert _client(regs, {5: 10000})._get_register_value(5) != pytest.approx(65536000.0)
164+
165+
166+
def test_complete_pair_still_decodes_when_partner_is_zero():
167+
"""A partner genuinely READ as 0 is valid and must still decode.
168+
169+
The fix keys on absence from the cache, not on the value being falsy — a high word
170+
of 0 is the normal case for any value below 65536.
171+
"""
172+
regs = {
173+
5: {"name": "power_high", "scale": 1, "pair": 6},
174+
6: {"name": "power_low", "scale": 1, "pair": 5, "combined_scale": 0.1},
175+
}
176+
assert _client(regs, {5: 0, 6: 2500})._get_register_value(6) == pytest.approx(250.0)
177+
178+
133179
def test_register_not_in_profile_returns_none():
134180
client = _client({400: {"name": "pv1_voltage", "scale": 0.1}}, {999: 123})
135181
assert client._get_register_value(999) is None

0 commit comments

Comments
 (0)