feat(modbus): let the connection be kept between polls, off by default

Dialling a fresh connection for every read is the unusual choice for modbus
TCP; one connection held open is the normal one. datalogger.persistent_
connection does that, and is off by default because it is a trade rather than
an improvement: this datalogger accepts one connection at a time, so holding
ours keeps everything else -- Solis Cloud included -- off the stick, where
hanging up leaves a gap in every poll cycle where something else can get in.

The client is now the app's rather than the poll's. ensure_connected is the
only thing that dials, drop_connection the only thing that throws one away,
and release_connection at the end of a good poll is the single place the
setting is read. That also means a read that raised redials here instead of
leaving pymodbus to do it out of sight, and a poll that could not read the
whole span drops the connection so the next one starts fresh -- which is what
every poll did before, so turning the setting on cannot make a bad morning
worse than it already was.

ensure_connected checks whether the client still holds a socket before
reusing it. pymodbus closes its own socket when the peer hangs up mid-read
(_handle_abrupt_socket_close) and its next request dials again silently, which
is precisely the case being measured here: it would have counted as zero
reconnects, on a socket carrying none of the keepalive set below.

Keepalive is set on every connection: SO_KEEPALIVE, 30 seconds idle, then
three probes five seconds apart, so a datalogger that vanished while the app
slept is declared dead by the kernel inside 45 seconds instead of by the next
read timing out CHUNK_ATTEMPTS times over. The option names are Linux's and
the container is Alpine, but the tests run on macOS, so each is applied only
where the platform has it.

Every connection logs one numbered line saying why the previous one ended, at
info when the setting is on and debug when it is off -- 2880 lines a day
otherwise, saying only that the app is working as configured. That line is the
point of shipping the setting at all: these sticks are widely reported to hang
up on an idle connection after a minute or two, and at a 30 second poll
interval that would mean reconnecting nearly every poll and gaining nothing.
It cannot be settled from here, only measured. Turn it on, leave it a day,
count the lines. Then the setting and the losing branch both go, and that
removal is the breaking change worth a 4.0.0.

The online transition needed no change: #181 already moved it off the
handshake and onto the first chunk of registers that arrives, which is the
only thing a reused connection could have said it from anyway. A poll whose
reads are all refused still counts against the retry budget, kept connection
or not, and a test pins it.

The stub datalogger and the query/polls fixtures moved from test_modbus_read
to conftest so the new connection lifetime tests poll the same stub.

Closes #172

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01BizgAhMgYcmMXjHFX7pbbs

Author: ahinko

CLAUDE.md

@@ -53,6 +53,22 @@ read that table, so a new read type is an entry plus a decoder, and never a bran
 either of them. `app/sensors.py` decides which names `sensors.yaml` may use, and
 `tests/test_decode.py` fails if the two lists drift apart.
 
+### The modbus connection
+
+The client belongs to the app (`self.client`), not to a poll. `ensure_connected()` is
+the only thing that dials one and `drop_connection(reason)` the only thing that throws
+one away, which is what lets a read that raised redial mid-poll instead of writing
+into a dead socket. `release_connection()` at the end of a good poll is the single
+place the `datalogger.persistent_connection` setting is read.
+
+That setting is off by default and expected to be temporary. The stick accepts one
+connection at a time, so holding ours keeps everything else — Solis Cloud included —
+off it. Whether holding it even helps depends on whether this stick hangs up on an
+idle connection, which can only be measured: every connection logs one numbered line
+saying why the previous one ended, at info when the setting is on and debug when it is
+off. Once there are enough of those lines to decide, the setting and the losing branch
+both go, and that removal is the breaking change worth a 4.0.0.
+
 `VERSION` in `app/app.py` is hand-maintained and ships in the discovery payload as
 the device's `sw_version`. Bump it with a release.
 

app/app.py

@@ -20,6 +20,17 @@ class DataLoggerConfig(Schema):
     poll_interval_if_off = fields.Int(required=False, load_default=600)
     poll_retries = fields.Int(required=False, load_default=10)
     register_chunks = fields.Int(required=False, load_default=80)
+    # Keep the modbus connection open between polls instead of dialling a new one
+    # every time. Off by default, and deliberately so: this datalogger accepts one
+    # connection at a time, so holding it means nothing else -- Solis Cloud included
+    # -- can talk to the stick, where closing it leaves a gap in every poll cycle.
+    #
+    # Expected to be temporary. These sticks are widely reported to close an idle
+    # connection themselves after a minute or two, which at a 30 second poll interval
+    # would mean reconnecting nearly every poll and gaining nothing. The reconnect
+    # counts this logs are how that gets measured; once it is measured, one of the two
+    # behaviours goes and this setting goes with it.
+    persistent_connection = fields.Bool(required=False, load_default=False)
     http = fields.Nested(HttpConfig(), required=False)
 
 

app/config.py

@@ -8,6 +8,11 @@ datalogger:
   poll_interval_if_off: 600
   poll_retries: 10
   register_chunks: 20
+  # Keep the modbus connection open between polls instead of dialling a new one every
+  # poll. The datalogger accepts one connection at a time, so with this on nothing
+  # else can reach it -- including Solis Cloud. Leave it off unless you are measuring
+  # what it does; the app logs a line for every connection it opens.
+  persistent_connection: False
   http:
     enabled: True
     user: admin

config.example.yaml

@@ -41,7 +41,7 @@ Because it works! If I for some reason would stop using this myself then I will
 
 ### Does Solis Cloud still work?
 
-Most other integrations that I found all stated that Solis Cloud would not work when polling the data logger over MODBUS. In my brief and short testing Solid Cloud still works, but not perfectly. My guess is that most other integrations keeps the connection with the data logger alive while this project closes the connection when its done and then reconnects when it's time for the next update. By "not perfectly" I mean that I can see that Solis Cloud is not always updated every 5 minutes. Sometimes it takes 15-20 minutes between updates but I've always seen that the data logger sooner or later are able to send data to Solis Cloud. And if I stop the script/Docker container I always seen an update in Solid cloud within 10 minutes without having to restart the data logger.
+Most other integrations that I found all stated that Solis Cloud would not work when polling the data logger over MODBUS. In my brief and short testing Solid Cloud still works, but not perfectly. My guess is that most other integrations keeps the connection with the data logger alive while this project closes the connection when its done and then reconnects when it's time for the next update. That is still the default, and `datalogger.persistent_connection` is the setting that changes it — see [Keeping the connection open](#keeping-the-connection-open). By "not perfectly" I mean that I can see that Solis Cloud is not always updated every 5 minutes. Sometimes it takes 15-20 minutes between updates but I've always seen that the data logger sooner or later are able to send data to Solis Cloud. And if I stop the script/Docker container I always seen an update in Solid cloud within 10 minutes without having to restart the data logger.
 
 ### Is it possible to control the inverter?
 
@@ -62,6 +62,28 @@ Maybe, I have no plans for it at the moment. My main goal is to get the data int
 ## Getting started
 Prepare a config file. Use `config.example.yaml` and modify it to your needs and save it as `config.yaml`. Most values should be self-explanatory. `register_chunks` is set to 20 by default but during my testing I've been able to query my datalogger for more than 80 registers at the same time.
 
+### Keeping the connection open
+By default the app dials a connection to the data logger, reads, and hangs up again on every poll. For MODBUS TCP that's the unusual choice — one connection held open is the normal one — and `datalogger.persistent_connection: True` does that instead:
+
+```yaml
+datalogger:
+  persistent_connection: True
+```
+
+It's off by default because it's a trade, not an improvement. The data logger accepts **one connection at a time**, so while this app holds it, nothing else can talk to the stick — Solis Cloud included. Hanging up after each poll leaves a gap in every cycle where something else can get in.
+
+Whether it's worth the trade is a question about your stick, not about this app: these WiFi sticks are widely reported to close an idle connection themselves after a minute or two, and at a 30 second poll interval that would mean reconnecting nearly every poll and gaining nothing. So the app logs a line for every connection it opens, numbered and saying why the previous one ended:
+
+```
+Connected to datalogger, connection 12 since startup, previous one ended because a read raised OSError: Connection reset by peer
+```
+
+Turn the setting on, leave it a day, and count those lines. One or two means the connection is surviving. One per poll means the stick is hanging up and the setting is buying nothing.
+
+Either way the app sets TCP keepalive on the socket, so a data logger that vanishes while the app is sleeping is noticed by the kernel within about 45 seconds instead of by the next read timing out three times over.
+
+Expect this setting to go away once there are enough measurements to pick a winner. Whichever behaviour loses will be removed along with it.
+
 ### Upgrading to 3.0
 **`inverter.max_power_kw` is now required and the container won't start without it.** Set it to the nameplate rating of your inverter, in kW:
 

readme.md

@@ -38,6 +38,90 @@ def total_power(sensors_config):
     return next(s for s in sensors_config if s["name"] == "total_power")
 
 
+# The registers sensors.yaml actually asks for: 3004 to 3077 inclusive.
+SPAN = 74
+FIRST, LAST = 3004, 3077
+
+
+class Response:
+    def __init__(self, registers=None, error=False):
+        self.registers = registers or []
+        self.error = error
+
+    def isError(self):
+        return self.error
+
+
+class FakeSocket:
+    """Records the socket options the app sets, and can refuse them."""
+
+    def __init__(self, refuse=False):
+        self.options = []
+        self.refuse = refuse
+
+    def setsockopt(self, level, option, value):
+        if self.refuse:
+            raise OSError("Protocol not available")
+
+        self.options.append((level, option, value))
+
+
+class StubClient:
+    """A datalogger that answers with whatever the test queued up.
+
+    One object stands in for every client the app builds, so that dialling again
+    shows up as another connect() rather than as another object.
+    """
+
+    def __init__(
+        self, *answers, refuse_socket_options=False, hangs_up_after_read=False
+    ):
+        self.answers = list(answers)
+        self.reads = []
+        self.closes = 0
+        self.connects = 0
+        self.connected = False
+        self.socket = None
+        # Every socket this client has had, since close() drops the current one and a
+        # test still wants to see what was set on it.
+        self.sockets = []
+        self.refuse_socket_options = refuse_socket_options
+        # The datalogger hanging up on its own: pymodbus closes its socket when the
+        # peer goes away mid-read and says nothing if the answer arrived first.
+        self.hangs_up_after_read = hangs_up_after_read
+
+    def connect(self):
+        self.connects += 1
+        self.connected = True
+        self.socket = FakeSocket(refuse=self.refuse_socket_options)
+        self.sockets.append(self.socket)
+        return True
+
+    def close(self):
+        self.closes += 1
+        self.connected = False
+        self.socket = None
+
+    def read_input_registers(self, device_id, address, count):
+        self.reads.append((address, count))
+        answer = self.answers.pop(0) if self.answers else Response(error=True)
+
+        if isinstance(answer, Exception):
+            raise answer
+
+        if self.hangs_up_after_read:
+            self.connected = False
+            self.socket = None
+
+        return answer
+
+
+def live_response(address=FIRST, count=SPAN):
+    # Register 3041 is the inverter temperature, the only thing that has to be non
+    # zero for the response not to count as dead.
+    return Response([250 if address + i == 3041 else 0 for i in range(count)])
+
+
 class Clock:
     """Stand in for time.monotonic so tests can span days in an instant."""
 
@@ -64,7 +148,13 @@ def make_app(sensors_config):
     assertions look at.
     """
 
-    def _make(day="2026-07-28", max_power_kw=15, poll_retries=3, register_chunks=80):
+    def _make(
+        day="2026-07-28",
+        max_power_kw=15,
+        poll_retries=3,
+        register_chunks=80,
+        persistent_connection=False,
+    ):
         app = App.__new__(App)
         app.config = {
             "mqtt": {"enabled": False, "topic_prefix": "tcpsolis2mqtt"},
@@ -77,6 +167,7 @@ def _make(day="2026-07-28", max_power_kw=15, poll_retries=3, register_chunks=80)
                 "poll_interval_if_off": 600,
                 "poll_retries": poll_retries,
                 "register_chunks": register_chunks,
+                "persistent_connection": persistent_connection,
                 "http": {"enabled": False},
             },
         }
@@ -95,6 +186,10 @@ def _make(day="2026-07-28", max_power_kw=15, poll_retries=3, register_chunks=80)
         app.register_span_start = 0
         app.register_span_end = 0
 
+        app.client = None
+        app.connections_opened = 0
+        app.connection_closed_reason = "nothing has been connected yet"
+
         app.day = day
         app.local_date = lambda: app.day
 
@@ -106,3 +201,46 @@ def _make(day="2026-07-28", max_power_kw=15, poll_retries=3, register_chunks=80)
         return app
 
     return _make
+
+
+@pytest.fixture
+def query(make_app, clock, monkeypatch):
+    """Run one poll against a stub datalogger and return what it produced."""
+
+    def _query(*answers, client=None, **config):
+        client = client or StubClient(*answers)
+        app = make_app(**config)
+        app.get_register_interval()
+
+        def build(*args, **kwargs):
+            app.client_kwargs = kwargs
+            return client
+
+        monkeypatch.setattr(app_module, "ModbusTcpClient", build)
+        return app, client, app.query_modbus()
+
+    return _query
+
+
+@pytest.fixture
+def polls(make_app, clock, monkeypatch):
+    """One App polled repeatedly, so state that carries between polls is visible."""
+
+    def _polls(*rounds, client=None, **config):
+        app = make_app(**config)
+        app.get_register_interval()
+        client = client or StubClient()
+        app.stub_client = client
+
+        def build(*args, **kwargs):
+            return client
+
+        monkeypatch.setattr(app_module, "ModbusTcpClient", build)
+
+        for answers in rounds:
+            client.answers = list(answers)
+            app.query_modbus()
+
+        return app
+
+    return _polls

tests/conftest.py

@@ -55,6 +55,28 @@ def test_a_fractional_rating_is_kept():
     )
 
 
+def test_the_connection_is_not_kept_unless_it_is_asked_for():
+    # The datalogger accepts one connection at a time, so holding ours keeps
+    # everything else -- Solis Cloud included -- off the stick. Anyone who wants that
+    # trade has to say so.
+    assert AppConfig().load(config())["datalogger"]["persistent_connection"] is False
+
+
+def test_the_connection_can_be_kept():
+    kept = config()
+    kept["datalogger"]["persistent_connection"] = True
+
+    assert AppConfig().load(kept)["datalogger"]["persistent_connection"] is True
+
+
+def test_the_shipped_example_leaves_the_connection_per_poll_behaviour_alone():
+    import yaml
+
+    example = yaml.safe_load(open("config.example.yaml"))
+
+    assert not example["datalogger"].get("persistent_connection")
+
+
 def test_the_shipped_example_names_a_rating():
     import yaml
 

tests/test_config.py

@@ -9,98 +9,12 @@
 register count was incremented once per attempt rather than once per chunk.
 """
 
-import pytest
-
 import app as app_module
 
-# The registers sensors.yaml actually asks for: 3004 to 3077 inclusive. This used to
-# be 80, the chunk size, because whole chunks were read whatever the span was.
-SPAN = 74
-FIRST, LAST = 3004, 3077
-
-
-class Response:
-    def __init__(self, registers=None, error=False):
-        self.registers = registers or []
-        self.error = error
-
-    def isError(self):
-        return self.error
-
-
-class StubClient:
-    """A datalogger that answers with whatever the test queued up."""
-
-    def __init__(self, *answers):
-        self.answers = list(answers)
-        self.reads = []
-        self.closes = 0
-        self.connected = True
-
-    def connect(self):
-        self.connected = True
-        return True
-
-    def close(self):
-        self.closes += 1
-        self.connected = False
-
-    def read_input_registers(self, device_id, address, count):
-        self.reads.append((address, count))
-        answer = self.answers.pop(0) if self.answers else Response(error=True)
-
-        if isinstance(answer, Exception):
-            raise answer
-
-        return answer
-
-
-def live_response(address=FIRST, count=SPAN):
-    # Register 3041 is the inverter temperature, the only thing that has to be non
-    # zero for the response not to count as dead.
-    return Response([250 if address + i == 3041 else 0 for i in range(count)])
-
-
-@pytest.fixture
-def query(make_app, clock, monkeypatch):
-    """Run one poll against a stub datalogger and return what it produced."""
-
-    def _query(*answers):
-        client = StubClient(*answers)
-        app = make_app()
-        app.get_register_interval()
-
-        def build(*args, **kwargs):
-            app.client_kwargs = kwargs
-            return client
-
-        monkeypatch.setattr(app_module, "ModbusTcpClient", build)
-        return app, client, app.query_modbus()
-
-    return _query
-
-
-@pytest.fixture
-def polls(make_app, clock, monkeypatch):
-    """One App polled repeatedly, so state that carries between polls is visible."""
-
-    def _polls(*rounds):
-        app = make_app()
-        app.get_register_interval()
-        client = StubClient()
-
-        def build(*args, **kwargs):
-            return client
-
-        monkeypatch.setattr(app_module, "ModbusTcpClient", build)
-
-        for answers in rounds:
-            client.answers = list(answers)
-            app.query_modbus()
-
-        return app
-
-    return _polls
+# The stub datalogger, the register span and the query/polls fixtures live in
+# conftest.py, because the connection lifetime tests in test_persistent_connection.py
+# poll the same stub.
+from conftest import FIRST, LAST, SPAN, Response, StubClient, live_response
 
 
 def test_a_chunk_is_read_once_when_it_answers(query):