Writes are not recovered from a dropped socket, while reads are — first write after an idle drop fails silently to the user

[0xAHA]

Summary

SharedModbusConnection recovers reads from a dropped socket and does not recover writes. On any gateway or datalogger that reaps idle TCP connections, the first read after a drop retries transparently and succeeds; the first write after a drop fails and returns False.

The user sees a control that did not take effect, with an ERROR in the log and no retry. Anyone driving the integration from an external controller with its own retry layer will not notice; anyone using the entities directly will.

Observed and correctly diagnosed by @alanmk while field-testing the #358 fix (comment) — a burst of Failed atomic FC16 write errors with [Errno 32] Broken pipe on a datalogger that drops long-lived sockets. Filed here rather than left on that thread, since it is unrelated to the reversion detector and affects a different set of users.

The asymmetry, in the code

Reads — growatt_modbus.py, read_input_registers and read_holding_registers:

for attempt in (0, 1):
    try:
        resp = self._client.read_input_registers(...)
    except Exception as exc:
        if attempt == 0 and self._begin_recovery():
            self.reset("transport error during block read")
            if self.ensure_connected():
                continue          # <- retried inside the same call
        return None

Writes — write_register and write_registers, same class:

except Exception as exc:
    # An exception here is transport-level (register-level refusals come back
    # as isError() responses) — drop the socket so the next call reconnects.
    self.disconnect()
    return False                  # <- current write already lost

There is no attempt loop, no _begin_recovery(), no reconnect-and-retry. The comment states the intent plainly: the next call reconnects. Whichever write triggered the drop is the one that fails.

Why this is a regression rather than long-standing

Before SharedModbusConnection, the write path checked the socket before each write. Moving to a shared, long-lived connection gave reads an explicit reset-and-retry and left writes with only the disconnect. The asymmetry arrived with the refactor.

Proposed fix

Give writes the same reset-and-retry-once the reads have, using the existing _begin_recovery() budget so a genuinely dead gateway still cannot turn one poll into a chain of reconnects.

This is safe here because every write in this integration is idempotent — each one sets a register to an absolute value, and there are no increments or toggles-relative-to-current. Replaying a write whose transport attempt failed cannot compound, and the failure mode it prevents (a silently ineffective control) is worse than the failure mode it risks (writing the same value twice).

The retry must be bounded to a transport-level exception only. A register-level refusal comes back as an isError() response rather than an exception, and must keep failing immediately — retrying an illegal-address write would be pointless noise, and on the registers we know reject writes (#371) it would double the log volume.

Verification

• Unit test asserting both write methods consult _begin_recovery() and retry, mirroring the existing read-path tests in tests/test_connection_recovery.py
• Test asserting a retry is not attempted for an isError() response
• Field confirmation ideally from a setup that reproduces the socket drop

Environment where observed

Integration	v1.1.10
Inverter	Growatt SPH 3600, sph_3000_6000_v201
Transport	Modbus TCP via a datalogger that drops long-lived sockets
Symptom	Failed atomic FC16 write (15 ERROR lines), [Errno 32] Broken pipe on almost every poll at DEBUG
Mitigation in place	Predbat's own write retries recovered every case

[0xAHA]

Fixed on main in 9fa9a6e. Not released yet — it will roll into the next release rather than get one of its own, and is recorded under an Unreleased heading in RELEASENOTES.md so it does not get lost in between.

What changed

Both write methods now use the same attempt loop as the reads, drawing on the same per-poll _begin_recovery() budget — so a genuinely dead gateway still cannot turn one write into a chain of reconnects.

Only transport-level exceptions retry. A register-level refusal comes back as an isError() response, not an exception, and still fails on the first attempt.

On testing it properly

tests/test_connection_recovery.py already had a scriptable fake client that exercises the real SharedModbusConnection, so the fake gained write methods sharing the same script rather than a new source-parsing test. That matters here: this is a behavioural change, and a test that reads the source for the string _begin_recovery would pass whether or not the retry actually works.

Five of the new cases fail against the previous write path:

test_write_transport_error_resets_and_retries_once[write_register]
test_write_transport_error_resets_and_retries_once[write_registers]
test_write_transport_error_on_retry_gives_up[write_register]
test_write_transport_error_on_retry_gives_up[write_registers]
test_write_recovery_shares_the_poll_budget

The two test_write_protocol_error_does_not_retry cases pass both before and after, which is deliberate — they pin behaviour that was already correct, so the change cannot quietly turn an illegal-address refusal into a retry loop on the registers known to reject writes (#371).

Credit

@alanmk diagnosed this from log behaviour alone while field-testing #358 — reads recovering silently, writes failing their first attempt — and correctly identified that the pre-SharedModbusConnection code checked the socket before each write. The code matched the inference exactly. Filed and fixed without needing a reproduction.