Skip to content

Commit a3a5748

Browse files
jekmanisclaude
andauthored
fix: shared TCP connection never recovers from silent connection loss (#354)
Upstream v1.0.x routes every TCP entry through SharedModbusConnection, whose ensure_connected() trusts pymodbus is_socket_open(). For sync clients that is just `self.socket is not None`, and pymodbus never clears the socket on receive timeouts (connection_lost() is a no-op without an asyncio transport). After a silent drop (gateway reboot, Wi-Fi dropout, idle NAT timeout) every poll reused the dead socket forever; only an HA restart recovered. - SharedModbusConnection.reset(): force-close so the next ensure_connected() performs a real reconnect (with buffer flush) - _fetch_data_shared(): on a failed poll, reset + reconnect + retry once in-poll; reset again if the retry also fails, and on exceptions - hub write_register/write_registers: drop the socket on transport level exceptions so service writes self-heal via ensure_connected() Restores the self-healing the pre-v1.0 per-poll reconnect path had. Co-authored-by: jekmanis <jekmanis@users.noreply.github.com> Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
1 parent 0f5c738 commit a3a5748

2 files changed

Lines changed: 40 additions & 0 deletions

File tree

custom_components/growatt_modbus/coordinator.py

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -951,6 +951,24 @@ def _fetch_data_shared(self) -> GrowattData | None:
951951
self._client.min_read_interval = delay_s
952952

953953
data = self._client.read_all_data()
954+
if data is None:
955+
# The connection may be silently dead: pymodbus sync clients never
956+
# clear their socket on receive timeouts, so is_socket_open() stays
957+
# True and ensure_connected() would reuse the dead socket on every
958+
# poll until HA restarts. Force a real reconnect and retry once.
959+
hub.reset("poll returned no data")
960+
if hub.ensure_connected():
961+
_LOGGER.info(
962+
"Reconnected to %s:%s — retrying poll for slave %s",
963+
self.config.get(CONF_HOST), self.config.get(CONF_PORT),
964+
self.config.get(CONF_SLAVE_ID),
965+
)
966+
data = self._client.read_all_data()
967+
if data is None:
968+
# Still failing — drop the socket so the next scheduled poll
969+
# starts from a clean connect instead of a stale session.
970+
hub.reset("retry poll returned no data")
971+
954972
if data is not None and not self._serial_number:
955973
self._read_device_identification()
956974

@@ -960,6 +978,7 @@ def _fetch_data_shared(self) -> GrowattData | None:
960978
except Exception as err:
961979
_LOGGER.warning("Error during shared data fetch for slave %s: %s",
962980
self.config.get(CONF_SLAVE_ID), err)
981+
hub.reset("exception during poll")
963982
return None
964983

965984
finally:

custom_components/growatt_modbus/growatt_modbus.py

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -454,6 +454,21 @@ def disconnect(self) -> None:
454454
pass
455455
self._connected = False
456456

457+
def reset(self, reason: str = "") -> None:
458+
"""Force-close the socket so the next ensure_connected() does a real reconnect.
459+
460+
pymodbus sync clients never clear their socket after a silent connection loss
461+
(receive timeouts don't trigger close(), and connection_lost() is a no-op for
462+
sync clients), so is_socket_open() keeps returning True and ensure_connected()
463+
would reuse the dead socket forever. Without this, only an HA restart recovers
464+
a wedged connection.
465+
"""
466+
logger.warning(
467+
"[SharedConn %s:%s] Resetting connection%s",
468+
self.host, self.port, f": {reason}" if reason else "",
469+
)
470+
self.disconnect()
471+
457472
def _flush_receive_buffer(self) -> None:
458473
"""Drain stale Modbus responses left in the adapter's TCP buffer after reconnect."""
459474
sock = getattr(self._client, 'socket', None)
@@ -550,6 +565,9 @@ def write_register(self, register: int, value: int, slave_id: int) -> bool:
550565
except Exception as exc:
551566
logger.debug("[SharedConn %s:%s] write_register(%d, %d, slave=%d) error: %s",
552567
self.host, self.port, register, value, slave_id, exc)
568+
# An exception here is transport-level (register-level refusals come back
569+
# as isError() responses) — drop the socket so the next call reconnects.
570+
self.disconnect()
553571
return False
554572

555573
def write_registers(self, register: int, values: list, slave_id: int) -> bool:
@@ -567,6 +585,9 @@ def write_registers(self, register: int, values: list, slave_id: int) -> bool:
567585
except Exception as exc:
568586
logger.debug("[SharedConn %s:%s] write_registers(%d, slave=%d) error: %s",
569587
self.host, self.port, register, slave_id, exc)
588+
# An exception here is transport-level (register-level refusals come back
589+
# as isError() responses) — drop the socket so the next call reconnects.
590+
self.disconnect()
570591
return False
571592

572593

0 commit comments

Comments
 (0)