Skip to content

Commit ef5cf5b

Browse files
0xAHAclaude
andcommitted
fix: do not write a control that already holds the requested value (#384)
These registers live in EEPROM, which has finite write endurance. Nothing in the integration writes on its own - a write happens only when async_set_native_value is called - but there was no guard against a write that could not change anything. An automation re-applying the same value on a schedule burned a cycle every run. Raised by a reporter whose contact repairs inverters and has seen four SPF6000ES units with failed EEPROMs in a year. That is not attributable to anything here, and the integration was already innocent of the thing he was worried about, but there is no argument for spending write cycles on writes that are no-ops. Gated on having a current reading. When the register has not been read yet the comparison is meaningless and the write proceeds - a skipped write that should have happened is a worse failure than a redundant one. Tests assert the guard exists, that it requires a known current value, and - the reassuring half of the answer given to the reporter - that neither coordinator.py nor __init__.py performs any Modbus write at all. If polling ever starts writing, that promise is void and the test says so. 1008 tests pass. Co-Authored-By: Claude <noreply@anthropic.com>
1 parent 0ecc4b4 commit ef5cf5b

3 files changed

Lines changed: 68 additions & 0 deletions

File tree

RELEASENOTES.md

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,10 @@
88

99
Merged to `main`, not yet in any release.
1010

11+
- **Setting a control to the value it already has no longer writes to the inverter.** These
12+
registers are held in EEPROM, which has a finite number of write cycles. Nothing polls or
13+
writes on its own, but an automation re-applying the same value on a schedule used to burn
14+
a cycle every run for no effect. Raised by @dinkalin-ux. (#384)
1115
- **A wrong protocol variant can now be corrected without deleting the integration.** Ten
1216
inverter families exist as two register maps, chosen by auto-detection at setup. When that
1317
choice was wrong there was no way back - the profile list shows one name for both, and

custom_components/growatt_modbus/number.py

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -319,6 +319,28 @@ async def async_set_native_value(self, value: float) -> None:
319319

320320
# Write to Modbus register with read-back verification
321321
register = self._control_config['register']
322+
323+
# Skip a write that would change nothing (#384).
324+
#
325+
# These registers are held in EEPROM, which has a finite write endurance. Nothing in
326+
# the integration writes on its own - a write happens only when this method is
327+
# called - but an automation that re-applies the same value on a schedule would
328+
# burn a write cycle every time it ran, for no effect. A repairer working on this
329+
# inverter family reported four SPF6000ES units with failed EEPROMs in a year, and
330+
# while that is not attributable to anything here, there is no reason to spend
331+
# cycles on writes that cannot change the value.
332+
#
333+
# Gated on having a current reading: when the register has not been read yet the
334+
# comparison is meaningless and the write goes ahead. A skipped write that should
335+
# have happened is worse than a redundant one.
336+
current_raw = getattr(self.coordinator.data, self._control_name, None) \
337+
if self.coordinator.data is not None else None
338+
if current_raw is not None and int(current_raw) == raw_value:
339+
_LOGGER.debug(
340+
"%s already reads %s (raw %d) — skipping write to register %d",
341+
self._control_name, value, raw_value, register,
342+
)
343+
return
322344
try:
323345
write_ok, verified = await self.hass.async_add_executor_job(
324346
self.coordinator.modbus_client.write_register_verified,

tests/test_spf_max_charge_current.py

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -196,3 +196,45 @@ def test_the_block_read_covers_35_and_36():
196196
/ "growatt_modbus.py").read_text(encoding="utf-8")
197197
assert "data.bulk_charge_voltage = int(battery_ctrl_regs[1])" in source
198198
assert "data.float_charge_voltage = int(battery_ctrl_regs[2])" in source
199+
200+
201+
# ---------------------------------------------------------------------------
202+
# EEPROM write avoidance (#384)
203+
#
204+
# These registers live in EEPROM, which has finite write endurance. Nothing in the
205+
# integration writes on its own - a write happens only when a user or an automation sets a
206+
# value - but an automation re-applying the same value on a schedule would burn a cycle every
207+
# run for no effect. Raised by a reporter whose contact repairs inverters and saw four
208+
# SPF6000ES units with failed EEPROMs in a year.
209+
# ---------------------------------------------------------------------------
210+
211+
def test_nothing_writes_outside_an_explicit_set():
212+
"""The reassuring half of the answer: polling never writes. If this ever changes, the
213+
EEPROM guarantee given to users on #384 is void."""
214+
for name in ("coordinator.py", "__init__.py"):
215+
source = (Path(__file__).parent.parent / "custom_components" / "growatt_modbus"
216+
/ name).read_text(encoding="utf-8")
217+
assert "write_register(" not in source, f"{name} performs a Modbus write"
218+
assert "write_registers(" not in source, f"{name} performs a Modbus write"
219+
220+
221+
def test_setting_the_current_value_does_not_write():
222+
"""A redundant write costs an EEPROM cycle and cannot change anything."""
223+
source = (Path(__file__).parent.parent / "custom_components" / "growatt_modbus"
224+
/ "number.py").read_text(encoding="utf-8")
225+
guard = source[source.index("async def async_set_native_value"):]
226+
guard = guard[:guard.index("write_register_verified")]
227+
assert "int(current_raw) == raw_value" in guard, (
228+
"a write is issued even when the register already holds the requested value"
229+
)
230+
assert "return" in guard
231+
232+
233+
def test_the_guard_needs_a_current_reading():
234+
"""A skipped write that should have happened is worse than a redundant one, so an
235+
unread register must fall through to the write."""
236+
source = (Path(__file__).parent.parent / "custom_components" / "growatt_modbus"
237+
/ "number.py").read_text(encoding="utf-8")
238+
assert "current_raw is not None and" in source, (
239+
"the no-op guard fires without a known current value"
240+
)

0 commit comments

Comments
 (0)