Skip to content

Commit 81daa24

Browse files
0xAHAclaude
andcommitted
fix: release the serial port between polls (#384)
v1.7.0 gave serial entries a shared connection and, with it, a connection held open for the lifetime of the entry. That is right for TCP and wrong for serial: a serial port is exclusive, so holding it denies it to everything else on the machine. A reporter with two SPF inverters went from working on v1.6.6 to one inverter permanently offline, with pyserial reporting 'Could not exclusively lock port /dev/ttyUSB3' on every poll. Before v1.7.0 the port was opened and closed per poll, so two entries naming the same adapter differently alternated and mostly worked; holding it turned that into a hard lockout for whichever lost the race. end_poll() gives the port back after each poll for serial and leaves TCP alone. Reopening costs ~2 ms against a poll that reads 98 registers. Entries that do share a hub are still serialized by the lock. Also: a failed serial open now explains itself instead of surfacing as a bare 'Failed to connect', and the docs cover choosing a stable path - CH340 adapters (vendor 1a86) have no serial number, so by-id cannot distinguish two of them and ttyUSBn numbering swaps on reboot, which is how two entries end up on one adapter without anyone noticing. Verified red without the fix. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
1 parent c364186 commit 81daa24

5 files changed

Lines changed: 121 additions & 2 deletions

File tree

RELEASENOTES.md

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

55
---
66

7+
## v1.7.3 (pre-release)
8+
9+
Issues: #384
10+
11+
> **Pre-release for testing.** v1.6.2 remains the stable release.
12+
>
13+
> **Fixes a serial regression introduced in v1.7.0.** If you are on v1.7.0-v1.7.2 with a
14+
> USB-RS485 adapter, update.
15+
16+
- **Serial ports are released between polls again.** v1.7.0 held the port open for the
17+
lifetime of the entry. A serial port is exclusive, so on some setups the second config
18+
entry could never open it and reported `Could not exclusively lock port` on every poll,
19+
taking that inverter permanently offline. Reopening costs about 2 ms. Reported by
20+
@dinkalin-ux. (#384)
21+
- **A serial port that cannot be opened now says why.** Previously this surfaced only as
22+
`Failed to connect`, with the real reason buried in a pymodbus line above it. The warning
23+
now names the likely cause and the command that confirms it.
24+
- **Documentation: choosing a stable serial path.** CH340 adapters — the most common cheap
25+
USB-RS485 type — have no serial number, so `/dev/serial/by-id/` cannot tell two of them
26+
apart and `/dev/ttyUSBn` numbering swaps between reboots. `by-path` is the right choice
27+
for those. This makes it easy to configure two entries that unknowingly point at the same
28+
adapter.
29+
30+
---
31+
732
## v1.7.2 (pre-release)
833

934
Issues: #384

custom_components/growatt_modbus/growatt_modbus.py

Lines changed: 44 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -512,6 +512,28 @@ def begin_poll(self) -> None:
512512
"""Reset the per-poll recovery budget. Call once per poll, before any reads."""
513513
self._recoveries_this_poll = 0
514514

515+
def end_poll(self) -> None:
516+
"""Give an exclusive serial port back between polls.
517+
518+
A TCP socket is cheap to hold and reconnecting costs a round trip, so TCP keeps its
519+
connection open across polls. A serial port is different: it is **exclusive**, and
520+
holding it denies it to every other process on the machine — including a second
521+
config entry that names the same adapter by a different path.
522+
523+
Before v1.7.0 the port was opened and closed per poll, so two such entries alternated
524+
and mostly worked, with occasional collisions. Holding it open turned that into a
525+
permanent lockout for whichever entry lost the race, with pyserial reporting
526+
"Could not exclusively lock port" on every poll thereafter (#384).
527+
528+
Reopening a serial port costs about 2 ms, which is nothing next to a poll that reads
529+
98 registers. The lock still serializes entries that do share a hub.
530+
"""
531+
if not self.is_serial:
532+
return
533+
with self._lock:
534+
self.disconnect()
535+
self._client = None
536+
515537
def _begin_recovery(self) -> bool:
516538
"""True if a reset+retry is still within this poll's recovery budget."""
517539
if self._recoveries_this_poll >= self._max_recoveries_per_poll:
@@ -567,6 +589,25 @@ def ensure_connected(self) -> bool:
567589
if result:
568590
self._connected = True
569591
self._flush_receive_buffer()
592+
elif self.is_serial:
593+
# pyserial logs "[Errno 11] Could not exclusively lock port ..." and pymodbus
594+
# turns it into a bare "Failed to connect", which tells the user nothing about
595+
# the one cause that actually matters: somebody else already has this port.
596+
#
597+
# The usual somebody else is a second config entry naming the same adapter by a
598+
# different path. That is easy to do by accident with cheap CH340 adapters
599+
# (USB vendor 1a86), which ship without a serial number — so two of them produce
600+
# by-id names that do not distinguish them, and the /dev/ttyUSBn numbering swaps
601+
# on reboot. by-path is stable per physical socket and is the right choice there.
602+
logger.warning(
603+
"[SharedConn %s] Could not open the serial port. If the log above shows "
604+
"'Could not exclusively lock port', another process or another Growatt "
605+
"config entry already has it open. Check whether two entries point at the "
606+
"same adapter under different names (/dev/ttyUSBn vs /dev/serial/by-id/... "
607+
"vs /dev/serial/by-path/...) — run: ls -l /dev/serial/by-id/ "
608+
"/dev/serial/by-path/",
609+
self.connection_id,
610+
)
570611
return result
571612

572613
def disconnect(self) -> None:
@@ -1098,7 +1139,9 @@ def _flush_receive_buffer(self) -> None:
10981139
def disconnect(self):
10991140
"""Close connection and release resources (critical for preventing file descriptor leaks)"""
11001141
if self._shared_conn is not None:
1101-
# Connection lifetime is managed by the hub.
1142+
# Connection lifetime is managed by the hub — except for serial, where the hub
1143+
# gives the port back between polls. See SharedModbusConnection.end_poll().
1144+
self._shared_conn.end_poll()
11021145
return
11031146
if self.client:
11041147
try:

custom_components/growatt_modbus/manifest.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,5 +14,5 @@
1414
"pymodbus>=3.0.0",
1515
"pyserial>=3.4"
1616
],
17-
"version": "1.7.2"
17+
"version": "1.7.3"
1818
}

docs/troubleshooting/rs485-gateways.md

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -165,6 +165,29 @@ The integration handles this for you: every entry pointing at the same device pa
165165

166166
**Serial setups need v1.7.0 or later for this.** Before that, only TCP entries were coordinated — each serial entry opened its own client on the same adapter and paced only itself, which produced random read failures on *all* the inverters sharing that bus. If you have two entries on one adapter and see unexplained dropouts, this is the first thing to rule out.
167167

168+
### Two adapters: check they are actually two
169+
170+
The most common cheap USB-RS485 adapters use the **CH340** chip (USB vendor `1a86`), and CH340s ship **without a serial number**. That has two consequences if you own two of them:
171+
172+
- Their `/dev/serial/by-id/` names do not distinguish them — you may see one entry, or two that differ only by an index that can move.
173+
- Their `/dev/ttyUSBn` numbers are assigned in enumeration order and **swap between reboots**.
174+
175+
So it is entirely possible to configure two entries that both point at the *same* adapter under different names, leaving the second adapter unused. The symptom is one entry working and the other failing with:
176+
177+
```
178+
[Errno 11] Could not exclusively lock port /dev/ttyUSB3: Resource temporarily unavailable
179+
```
180+
181+
A serial port can only be held by one owner. If you see that, run:
182+
183+
```bash
184+
ls -l /dev/serial/by-id/ /dev/serial/by-path/
185+
```
186+
187+
Both listings show what each name resolves to. If two entries land on the same `ttyUSBn`, that is the problem.
188+
189+
**For adapters without a serial number, prefer `/dev/serial/by-path/`.** It identifies the physical USB socket rather than the device, so it stays stable across reboots *and* distinguishes two identical adapters — which `by-id` cannot. Use `by-id` when the adapter does have a serial number (FTDI adapters usually do); it survives being moved to a different socket.
190+
168191
Two consequences worth knowing:
169192

170193
- **Polls queue, they do not overlap.** With several inverters the effective cycle is the sum of their polls. If a single poll takes 8 s, three inverters need an interval comfortably above 24 s.

tests/test_serial_shared_connection.py

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -127,6 +127,34 @@ def test_the_serial_buffer_is_drained_with_pyserials_own_method(serial_hub):
127127
assert fake.reset_calls >= 1, "stale bytes were never drained on a serial connection"
128128

129129

130+
def test_a_serial_port_is_released_between_polls(serial_hub):
131+
"""A serial port is exclusive. Holding it open across polls denies it to every other
132+
process, including a second config entry naming the same adapter by a different path —
133+
which turned an intermittent collision into a permanent 'Could not exclusively lock
134+
port' for one of them (#384)."""
135+
serial_hub.ensure_connected()
136+
assert serial_hub._client is not None
137+
138+
serial_hub.end_poll()
139+
assert serial_hub._client is None, "the serial port is still held after the poll ended"
140+
141+
# ...and the next poll must be able to open it again.
142+
assert serial_hub.ensure_connected() is True
143+
144+
145+
def test_a_tcp_socket_is_kept_open_between_polls():
146+
"""The opposite rule for TCP: a socket costs nothing to hold and reconnecting costs a
147+
round trip, so end_poll() must leave it alone."""
148+
hub = SharedModbusConnection(host="10.0.0.1", port=502)
149+
hub._client = object()
150+
hub.end_poll()
151+
assert hub._client is not None, "TCP connections should persist across polls"
152+
153+
154+
def test_ending_a_poll_is_safe_before_anything_connected(serial_hub):
155+
serial_hub.end_poll() # must not raise
156+
157+
130158
def test_flushing_never_propagates_an_error(serial_hub):
131159
"""A failed flush is non-critical and must not take down the poll."""
132160
serial_hub.ensure_connected()

0 commit comments

Comments
 (0)