Run the sync tcp client on the async client (draft) by sankalpsthakur · Pull Request #3088 · pymodbus-dev/pymodbus

sankalpsthakur · 2026-11-12T17:49:51Z

Draft, to discuss the approach from #3076 on real code. Only the tcp client is converted.

ModbusTcpClient now creates an AsyncModbusTcpClient on a private event loop that runs in a daemon thread. connect() starts the loop and connects, execute() submits the request with asyncio.run_coroutine_threadsafe() and waits for the result, close() closes the async client and stops the loop. The async client gets reconnect_delay=0, because the sync client connects when a request needs it.

The mechanism is in ModbusBaseSyncClient (_LoopThread, _connect_async, _close_async), so the other clients only need _new_async_client() and _run_async = True. The TLS client keeps its sync code for now, UDP and serial are unchanged.

Open points:

5 tests in test_client_sync.py and test_client.py mock socket.create_connection/socket.send for the tcp client and fail now. I'd rewrite them against a server, like the new test_client_sync_loop.py, once the approach is agreed.
trace_packet/trace_pdu/trace_connect are now called on the loop thread, not the caller's thread.
Benchmark numbers are in Discard the late response of a cancelled request #3076.

Validation

pytest test/client/test_client_sync_loop.py: 6 passed (real server: read, connect on request, with block, failed connect leaves no thread, call from a thread that runs a loop, set_max_no_responses).
pytest test/client: 242 passed, 8 failed (the 5 above, and 3 tls tests that cannot find the certificates in my worktree, also on dev).
ruff check, ruff format --check, codespell and pylint (10.00/10) are clean; mypy reports the same findings as on dev.

Related to #3076.

AI disclosure: written with an AI coding agent and tested as described above.

ModbusTcpClient now runs an AsyncModbusTcpClient on a private event loop in a daemon thread. Each request is submitted with asyncio.run_coroutine_threadsafe(), so the sync client uses the async transaction code and also works when the calling thread runs a loop. The loop starts on connect and stops on close. The TLS, UDP and serial clients keep their sync code for now.

janiversen · 2026-11-12T19:21:51Z

Please check that an exception raised in the async part goes back to the app....I remember having that problem in the server.

janiversen

Looks good ! I have added a small number of questions/comments.

janiversen · 2026-11-12T19:24:33Z

        self.use_udp = False
+        self._loop_thread: _LoopThread | None = None
+        self._async_client: ModbusBaseClient | None = None
+        self._custom_responses: list[type[ModbusPDU]] = []


Why do we need to store the custom responses, once registred they are kept in the pdu module.

They are kept per decoder: DecodePDU.register() copies pdu_table into the instance, so the async client created on connect did not know them. I removed the list, the async client now uses the decoder of the sync client, so registered responses are known to both.

janiversen · 2026-11-12T19:32:18Z

+
+        :meta private:
+        """
+        return AsyncModbusTcpClient(**self._async_args, reconnect_delay=0)  # type: ignore[arg-type]


it would be nicer with functools.partial

Done, the tcp client sets self._async_client_factory = functools.partial(AsyncModbusTcpClient, ...).

janiversen · 2026-11-12T19:35:14Z

+from pymodbus.simulator import DataType, SimData, SimDevice
+
+
+def free_port() -> int:


please see the other tests, we use a standard fixture use_port

Done, the test class uses use_port with a new entry in BASE_PORTS.

janiversen · 2026-11-12T19:35:34Z

+        return sock.getsockname()[1]
+
+
+class ServerThread:  # pylint: disable=too-few-public-methods


There is a standard fixture for this.

Done, the tests use mock_server. The server runs on the test loop, so the sync calls go through asyncio.to_thread().

The tcp client creates its async counterpart with functools.partial. The async client shares the decoder of the sync client, so the list of registered custom responses is no longer needed. The tests use the standard use_port and mock_server fixtures, run with _run_async True and False, and check that an exception raised in the async client reaches the caller.

sankalpsthakur · 2026-11-13T13:24:24Z

Checked: run_coroutine_threadsafe().result() raises the exception of the coroutine in the calling thread. test_exception makes the async execute() raise a ModbusIOException and checks that the app gets the same exception, and that the client still works afterwards. test_connect_fails covers the ConnectionException.

sankalpsthakur · 2026-11-13T17:12:57Z

Kept the 5 tests that mock the socket on the old sync code path, so they test what they did before.

janiversen

LGTM, with one expection test_sync_client only run the old tests, please confirm that the new test file contains the same tests and possible more.

It is important to show that any old app will still work, basically ensuring that test_sync_client works in both implementations. Of course it is perfectly ok, if you state the tests are all in the new file.

janiversen · 2026-11-14T16:07:48Z

I suggest to make a PR (later) for each type of client to make bisecting easier.

sankalpsthakur · 2026-11-14T22:03:05Z

not all of them, so i added a commit. the tcp tests in test_client_sync.py call send(), recv() and the socket directly, that is the old sync code, and the new path does not use it. test_client_sync_loop.py tests the app api against a real server instead: read, connect on request, with, failed connect, exception back to the app, call from a thread with a running loop, set_max_no_responses and the shared decoder, with test_read on both implementations

to show that old apps still work, 490d06d runs the tcp combinations of TestSyncExamples (client_calls.py with all calls, and simple_sync_client.py) and TestClientServerSyncExamples with _run_async True and False

agree on one PR per client type, i will do udp, tls and serial separately once this one is in

janiversen reviewed Nov 12, 2026

View reviewed changes

Adapt the tcp tests that mock the socket

739b81b

janiversen reviewed Nov 14, 2026

View reviewed changes

Run the sync tcp example tests on both implementations

490d06d

sankalpsthakur marked this pull request as ready for review November 14, 2026 22:03

		from pymodbus.simulator import DataType, SimData, SimDevice


		def free_port() -> int:

		return sock.getsockname()[1]


		class ServerThread: # pylint: disable=too-few-public-methods

Conversation

sankalpsthakur commented Nov 12, 2026

Uh oh!

janiversen commented Nov 12, 2026

Uh oh!

janiversen left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

sankalpsthakur commented Nov 13, 2026

Uh oh!

sankalpsthakur commented Nov 13, 2026

Uh oh!

janiversen left a comment

Choose a reason for hiding this comment

Uh oh!

janiversen commented Nov 14, 2026

Uh oh!

sankalpsthakur commented Nov 14, 2026

Uh oh!

This branch has not been deployed

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants