From 84f4a99e874b89341af4002afc1c6ac4717b3052 Mon Sep 17 00:00:00 2001 From: jan iversen Date: Wed, 7 Jun 2023 13:29:51 +0200 Subject: [PATCH 01/12] transport nullmodem. --- pymodbus/transport/nullmodem.py | 192 ++++++++++++++++++++++++++++++++ pymodbus/transport/transport.py | 8 +- 2 files changed, 196 insertions(+), 4 deletions(-) create mode 100644 pymodbus/transport/nullmodem.py diff --git a/pymodbus/transport/nullmodem.py b/pymodbus/transport/nullmodem.py new file mode 100644 index 000000000..a9190bee5 --- /dev/null +++ b/pymodbus/transport/nullmodem.py @@ -0,0 +1,192 @@ +"""Null modem transport. + +This is a special transport, mostly thought of for testing. + +NullModem interconnect 2 transport objects and transfers calls: + - client/server.connect() + - call client.connection_made() + - call server.connection_made() + - client/server.close() + - call client.connection_lost() + - call server.connection_lost() + - server.close() + - call server.connection_lost() + - call client.connection_lost() + - server/client.send + - call client/server.data_received() + +""" +from __future__ import annotations + +import asyncio + +from pymodbus.logging import Log +from pymodbus.transport.transport import Transport + + +class NullModem(Transport): + """Transport layer. + + Contains pure transport methods needed to connect/listen, send/receive and close connections + for unix socket, tcp, tls and serial communications. + + Contains high level methods like reconnect. + + This class is not available in the pymodbus API, and should not be referenced in Applications + nor in the pymodbus documentation. + + The class is designed to be an object in the message level class. + """ + + + async def transport_connect(self) -> bool: + """Handle generic connect and call on to specific transport connect.""" + Log.debug("Connecting {}", self.comm_params.comm_name) + if not self.loop: + self.loop = asyncio.get_running_loop() + self.transport, self.protocol = None, None + try: + self.transport, self.protocol = await asyncio.wait_for( + self.call_connect_listen(), + timeout=self.comm_params.timeout_connect, + ) + except ( + asyncio.TimeoutError, + OSError, + ) as exc: + Log.warning("Failed to connect {}", exc) + self.close(reconnect=True) + return False + return bool(self.transport) + + async def transport_listen(self): + """Handle generic listen and call on to specific transport listen.""" + Log.debug("Awaiting connections {}", self.comm_params.comm_name) + try: + self.transport = await self.call_connect_listen() + except OSError as exc: + Log.warning("Failed to start server {}", exc) + self.close() + return self.transport + + # ---------------------------------- # + # Transport asyncio standard methods # + # ---------------------------------- # + def connection_made(self, transport: asyncio.BaseTransport): + """Call from asyncio, when a connection is made. + + :param transport: socket etc. representing the connection. + """ + Log.debug("Connected to {}", self.comm_params.comm_name) + if not self.loop: + self.loop = asyncio.get_running_loop() + self.transport = transport + self.reset_delay() + self.cb_connection_made() + + def connection_lost(self, reason: Exception): + """Call from asyncio, when the connection is lost or closed. + + :param reason: None or an exception object + """ + Log.debug("Connection lost {} due to {}", self.comm_params.comm_name, reason) + self.cb_connection_lost(reason) + if self.transport: + self.close() + self.reconnect_task = asyncio.create_task(self.reconnect_connect()) + + def eof_received(self): + """Call when eof received (other end closed connection). + + Handling is moved to connection_lost() + """ + + def data_received(self, data: bytes): + """Call when some data is received. + + :param data: non-empty bytes object with incoming data. + """ + Log.debug("recv: {}", data, ":hex") + self.recv_buffer += data + cut = self.cb_handle_data(self.recv_buffer) + self.recv_buffer = self.recv_buffer[cut:] + + # -------------------------------- # + # Helper methods for child classes # + # -------------------------------- # + async def send(self, data: bytes) -> bool: + """Send request. + + :param data: non-empty bytes object with data to send. + """ + Log.debug("send: {}", data, ":hex") + if self.use_udp: + return self.transport.sendto(data) # type: ignore[union-attr] + return self.transport.write(data) # type: ignore[union-attr] + + def close(self, reconnect: bool = False) -> None: + """Close connection. + + :param reconnect: (default false), try to reconnect + """ + if self.transport: + if hasattr(self.transport, "abort"): + self.transport.abort() + self.transport.close() + self.transport = None + self.protocol = None + if not reconnect and self.reconnect_task: + self.reconnect_task.cancel() + self.reconnect_task = None + self.recv_buffer = b"" + + def reset_delay(self) -> None: + """Reset wait time before next reconnect to minimal period.""" + self.reconnect_delay_current = self.comm_params.reconnect_delay + + def is_active(self) -> bool: + """Return true if connected/listening.""" + return bool(self.transport) + + # ---------------- # + # Internal methods # + # ---------------- # + def handle_listen(self): + """Handle incoming connect.""" + return self + + async def reconnect_connect(self): + """Handle reconnect as a task.""" + try: + self.reconnect_delay_current = self.comm_params.reconnect_delay + while True: + Log.debug( + "Wait {} {} ms before reconnecting.", + self.comm_params.comm_name, + self.reconnect_delay_current * 1000, + ) + await asyncio.sleep(self.reconnect_delay_current) + if await self.transport_connect(): + break + self.reconnect_delay_current = min( + 2 * self.reconnect_delay_current, + self.comm_params.reconnect_delay_max, + ) + except asyncio.CancelledError: + pass + self.reconnect_task = None + + # ----------------- # + # The magic methods # + # ----------------- # + async def __aenter__(self): + """Implement the client with async enter block.""" + return self + + async def __aexit__(self, _class, _value, _traceback) -> None: + """Implement the client with async exit block.""" + self.close() + + def __str__(self) -> str: + """Build a string representation of the connection.""" + return f"{self.__class__.__name__}({self.comm_params.comm_name})" diff --git a/pymodbus/transport/transport.py b/pymodbus/transport/transport.py index fed99d833..107c55ed0 100644 --- a/pymodbus/transport/transport.py +++ b/pymodbus/transport/transport.py @@ -1,4 +1,4 @@ -"""Base for all transport types.""" +"""Transport layer.""" # mypy: disable-error-code="name-defined" # needed because asyncio.Server is not defined (to mypy) in v3.8.16 from __future__ import annotations @@ -346,9 +346,9 @@ def error_received(self, exc): Log.debug("-> error_received {}", exc) raise RuntimeError(str(exc)) - # -------------------------------- # - # Helper methods for child classes # - # -------------------------------- # + # ----------------------------------- # + # Helper methods for external classes # + # ----------------------------------- # async def send(self, data: bytes) -> bool: """Send request. From bfb1e5547cb08697c421506c63c3da97fecc3402 Mon Sep 17 00:00:00 2001 From: jan iversen Date: Sun, 11 Jun 2023 20:01:27 +0200 Subject: [PATCH 02/12] first version. --- pymodbus/transport/nullmodem.py | 152 +++++++------------------------- 1 file changed, 31 insertions(+), 121 deletions(-) diff --git a/pymodbus/transport/nullmodem.py b/pymodbus/transport/nullmodem.py index a9190bee5..2669c7971 100644 --- a/pymodbus/transport/nullmodem.py +++ b/pymodbus/transport/nullmodem.py @@ -3,15 +3,14 @@ This is a special transport, mostly thought of for testing. NullModem interconnect 2 transport objects and transfers calls: - - client/server.connect() + - server.listen() + - dummy + - client.connect() - call client.connection_made() - call server.connection_made() - client/server.close() - call client.connection_lost() - call server.connection_lost() - - server.close() - - call server.connection_lost() - - call client.connection_lost() - server/client.send - call client/server.data_received() @@ -38,78 +37,33 @@ class NullModem(Transport): The class is designed to be an object in the message level class. """ + server: NullModem = None + client: NullModem = None + is_server: bool = False + + class dummy_transport(asyncio.BaseTransport): + """Use in connection_made calls.""" async def transport_connect(self) -> bool: """Handle generic connect and call on to specific transport connect.""" - Log.debug("Connecting {}", self.comm_params.comm_name) + self.is_server = False + self.client = self + Log.debug("NullModem: Simulate connect on {}", self.comm_params.comm_name) if not self.loop: self.loop = asyncio.get_running_loop() self.transport, self.protocol = None, None - try: - self.transport, self.protocol = await asyncio.wait_for( - self.call_connect_listen(), - timeout=self.comm_params.timeout_connect, - ) - except ( - asyncio.TimeoutError, - OSError, - ) as exc: - Log.warning("Failed to connect {}", exc) - self.close(reconnect=True) - return False - return bool(self.transport) + if self.server: + self.server.connection_made(self.dummy_transport()) + self.connection_made(self.dummy_transport()) + return True + return False async def transport_listen(self): """Handle generic listen and call on to specific transport listen.""" - Log.debug("Awaiting connections {}", self.comm_params.comm_name) - try: - self.transport = await self.call_connect_listen() - except OSError as exc: - Log.warning("Failed to start server {}", exc) - self.close() - return self.transport - - # ---------------------------------- # - # Transport asyncio standard methods # - # ---------------------------------- # - def connection_made(self, transport: asyncio.BaseTransport): - """Call from asyncio, when a connection is made. - - :param transport: socket etc. representing the connection. - """ - Log.debug("Connected to {}", self.comm_params.comm_name) - if not self.loop: - self.loop = asyncio.get_running_loop() - self.transport = transport - self.reset_delay() - self.cb_connection_made() - - def connection_lost(self, reason: Exception): - """Call from asyncio, when the connection is lost or closed. - - :param reason: None or an exception object - """ - Log.debug("Connection lost {} due to {}", self.comm_params.comm_name, reason) - self.cb_connection_lost(reason) - if self.transport: - self.close() - self.reconnect_task = asyncio.create_task(self.reconnect_connect()) - - def eof_received(self): - """Call when eof received (other end closed connection). - - Handling is moved to connection_lost() - """ - - def data_received(self, data: bytes): - """Call when some data is received. - - :param data: non-empty bytes object with incoming data. - """ - Log.debug("recv: {}", data, ":hex") - self.recv_buffer += data - cut = self.cb_handle_data(self.recv_buffer) - self.recv_buffer = self.recv_buffer[cut:] + self.is_server = True + self.server = self + Log.debug("NullModem: Simulate listen on {}", self.comm_params.comm_name) + return self # -------------------------------- # # Helper methods for child classes # @@ -119,74 +73,30 @@ async def send(self, data: bytes) -> bool: :param data: non-empty bytes object with data to send. """ - Log.debug("send: {}", data, ":hex") - if self.use_udp: - return self.transport.sendto(data) # type: ignore[union-attr] - return self.transport.write(data) # type: ignore[union-attr] + Log.debug("NullModem: simulate send {}", data, ":hex") + if self.is_server: + self.client.data_received(data) + else: + self.server.data_received(data) + return True def close(self, reconnect: bool = False) -> None: """Close connection. :param reconnect: (default false), try to reconnect """ - if self.transport: - if hasattr(self.transport, "abort"): - self.transport.abort() - self.transport.close() - self.transport = None - self.protocol = None - if not reconnect and self.reconnect_task: - self.reconnect_task.cancel() - self.reconnect_task = None self.recv_buffer = b"" - - def reset_delay(self) -> None: - """Reset wait time before next reconnect to minimal period.""" - self.reconnect_delay_current = self.comm_params.reconnect_delay + if not reconnect: + self.client.cb_connection_lost(None) + self.server.cb_connection_lost(None) def is_active(self) -> bool: """Return true if connected/listening.""" - return bool(self.transport) - - # ---------------- # - # Internal methods # - # ---------------- # - def handle_listen(self): - """Handle incoming connect.""" - return self - - async def reconnect_connect(self): - """Handle reconnect as a task.""" - try: - self.reconnect_delay_current = self.comm_params.reconnect_delay - while True: - Log.debug( - "Wait {} {} ms before reconnecting.", - self.comm_params.comm_name, - self.reconnect_delay_current * 1000, - ) - await asyncio.sleep(self.reconnect_delay_current) - if await self.transport_connect(): - break - self.reconnect_delay_current = min( - 2 * self.reconnect_delay_current, - self.comm_params.reconnect_delay_max, - ) - except asyncio.CancelledError: - pass - self.reconnect_task = None + return True # ----------------- # # The magic methods # # ----------------- # - async def __aenter__(self): - """Implement the client with async enter block.""" - return self - - async def __aexit__(self, _class, _value, _traceback) -> None: - """Implement the client with async exit block.""" - self.close() - def __str__(self) -> str: """Build a string representation of the connection.""" return f"{self.__class__.__name__}({self.comm_params.comm_name})" From c38dd796daf5e6a6afc519631069df1c642a1fce Mon Sep 17 00:00:00 2001 From: jan iversen Date: Sun, 11 Jun 2023 20:17:59 +0200 Subject: [PATCH 03/12] pylint. --- pymodbus/transport/nullmodem.py | 20 ++++++++++++++++---- 1 file changed, 16 insertions(+), 4 deletions(-) diff --git a/pymodbus/transport/nullmodem.py b/pymodbus/transport/nullmodem.py index 2669c7971..f7453e3da 100644 --- a/pymodbus/transport/nullmodem.py +++ b/pymodbus/transport/nullmodem.py @@ -41,9 +41,21 @@ class NullModem(Transport): client: NullModem = None is_server: bool = False - class dummy_transport(asyncio.BaseTransport): + class DummyTransport(asyncio.BaseTransport): """Use in connection_made calls.""" + def close(self): + """Define dummy.""" + + def get_protocol(self): + """Define dummy.""" + + def is_closing(self): + """Define dummy.""" + + def set_protocol(self, _protocol): + """Define dummy.""" + async def transport_connect(self) -> bool: """Handle generic connect and call on to specific transport connect.""" self.is_server = False @@ -53,8 +65,8 @@ async def transport_connect(self) -> bool: self.loop = asyncio.get_running_loop() self.transport, self.protocol = None, None if self.server: - self.server.connection_made(self.dummy_transport()) - self.connection_made(self.dummy_transport()) + self.server.connection_made(self.DummyTransport()) + self.connection_made(self.DummyTransport()) return True return False @@ -63,7 +75,7 @@ async def transport_listen(self): self.is_server = True self.server = self Log.debug("NullModem: Simulate listen on {}", self.comm_params.comm_name) - return self + return self.DummyTransport() # -------------------------------- # # Helper methods for child classes # From 9f92b3d9bb27edb0fea8372570b43dcb1aaf5ee6 Mon Sep 17 00:00:00 2001 From: jan iversen Date: Mon, 12 Jun 2023 12:19:00 +0200 Subject: [PATCH 04/12] first nullmodem test. --- pymodbus/transport/nullmodem.py | 34 ++++++++------- test/sub_transport/conftest.py | 18 ++++++++ test/sub_transport/test_nullmodem.py | 62 ++++++++++++++++++++++++++++ 3 files changed, 99 insertions(+), 15 deletions(-) create mode 100644 test/sub_transport/test_nullmodem.py diff --git a/pymodbus/transport/nullmodem.py b/pymodbus/transport/nullmodem.py index f7453e3da..38efd425d 100644 --- a/pymodbus/transport/nullmodem.py +++ b/pymodbus/transport/nullmodem.py @@ -23,6 +23,25 @@ from pymodbus.transport.transport import Transport +class DummyTransport(asyncio.BaseTransport): + """Use in connection_made calls.""" + + def close(self): + """Define dummy.""" + + def get_protocol(self): + """Define dummy.""" + + def is_closing(self): + """Define dummy.""" + + def set_protocol(self, _protocol): + """Define dummy.""" + + def abort(self): + """Define dummy.""" + + class NullModem(Transport): """Transport layer. @@ -41,21 +60,6 @@ class NullModem(Transport): client: NullModem = None is_server: bool = False - class DummyTransport(asyncio.BaseTransport): - """Use in connection_made calls.""" - - def close(self): - """Define dummy.""" - - def get_protocol(self): - """Define dummy.""" - - def is_closing(self): - """Define dummy.""" - - def set_protocol(self, _protocol): - """Define dummy.""" - async def transport_connect(self) -> bool: """Handle generic connect and call on to specific transport connect.""" self.is_server = False diff --git a/test/sub_transport/conftest.py b/test/sub_transport/conftest.py index 9a3d14184..0cf638179 100644 --- a/test/sub_transport/conftest.py +++ b/test/sub_transport/conftest.py @@ -10,6 +10,7 @@ import pytest import pytest_asyncio +from pymodbus.transport.nullmodem import NullModem from pymodbus.transport.transport import Transport @@ -82,6 +83,23 @@ async def prepare_transport(): return transport +@pytest.fixture(name="nullmodem") +async def prepare_nullmodem(): + """Prepare nullmodem object.""" + transport = NullModem( + BaseParams.comm_name, + BaseParams.reconnect_delay, + BaseParams.reconnect_delay_max, + BaseParams.timeout_connect, + mock.Mock(name="cb_connection_made"), + mock.Mock(name="cb_connection_lost"), + mock.Mock(name="cb_handle_data", return_value=0), + ) + with suppress(RuntimeError): + transport.loop = asyncio.get_running_loop() + return transport + + @pytest_asyncio.fixture(name="transport_server") async def prepare_transport_server(): """Prepare transport object.""" diff --git a/test/sub_transport/test_nullmodem.py b/test/sub_transport/test_nullmodem.py new file mode 100644 index 000000000..a533962a3 --- /dev/null +++ b/test/sub_transport/test_nullmodem.py @@ -0,0 +1,62 @@ +"""Test transport.""" +from unittest import mock + +from pymodbus.transport.nullmodem import DummyTransport + + +class TestNullModemTransport: + """Test null modem module.""" + + async def test_str_magic(self, nullmodem, params): + """Test magic.""" + str(nullmodem) + assert str(nullmodem) == f"NullModem({params.comm_name})" + + def test_DummyTransport(self): + """Test DummyTransport class.""" + socket = DummyTransport() + socket.close() + socket.get_protocol() + socket.is_closing() + socket.set_protocol(None) + socket.abort() + + async def test_transport_connect(self, transport, commparams): + """Test connection_made().""" + transport.loop = None + transport.connection_made(DummyTransport()) + assert transport.transport + assert not transport.recv_buffer + assert not transport.reconnect_task + assert transport.reconnect_delay_current == commparams.reconnect_delay + transport.cb_connection_made.assert_called_once() + transport.cb_connection_lost.assert_not_called() + transport.cb_handle_data.assert_not_called() + transport.close() + + async def test_close(self, transport): + """Test close().""" + + async def test_datagram(self, transport): + """Test datagram_received().""" + transport.data_received = mock.MagicMock() + transport.datagram_received(b"abc", "127.0.0.1") + transport.data_received.assert_called_once() + + async def test_data(self, transport): + """Test data_received.""" + transport.cb_handle_data = mock.MagicMock(return_value=2) + transport.data_received(b"123456") + transport.cb_handle_data.assert_called_once() + assert transport.recv_buffer == b"3456" + transport.data_received(b"789") + assert transport.recv_buffer == b"56789" + + async def test_send(self, transport, params): + """Test send().""" + transport.transport = mock.AsyncMock() + await transport.send(b"abc") + + transport.setup_udp(False, params.host, params.port) + await transport.send(b"abc") + transport.close() From c6dca147939500768b22ecd1b463fd1b2e48f42d Mon Sep 17 00:00:00 2001 From: jan iversen Date: Mon, 12 Jun 2023 12:22:30 +0200 Subject: [PATCH 05/12] Use DummyTransport. --- test/sub_transport/conftest.py | 15 --------------- test/sub_transport/test_basic.py | 12 ++++++++---- 2 files changed, 8 insertions(+), 19 deletions(-) diff --git a/test/sub_transport/conftest.py b/test/sub_transport/conftest.py index 0cf638179..715838ea7 100644 --- a/test/sub_transport/conftest.py +++ b/test/sub_transport/conftest.py @@ -39,21 +39,6 @@ def prepare_baseparams(use_port): return BaseParams -class DummySocket: # pylint: disable=too-few-public-methods - """Socket simulator for test.""" - - def __init__(self): - """Initialize.""" - self.close = mock.Mock() - self.abort = mock.Mock() - - -@pytest.fixture(name="dummy_socket") -def prepare_dummysocket(): - """Prepare dummy_socket class.""" - return DummySocket - - @pytest.fixture(name="commparams") def prepare_testparams(): """Prepare CommParamsClass object.""" diff --git a/test/sub_transport/test_basic.py b/test/sub_transport/test_basic.py index f5261df8f..3535b954e 100644 --- a/test/sub_transport/test_basic.py +++ b/test/sub_transport/test_basic.py @@ -5,6 +5,8 @@ import pytest from serial import SerialException +from pymodbus.transport.nullmodem import DummyTransport + class TestBasicTransport: """Test transport module, base part.""" @@ -45,10 +47,10 @@ async def test_str_magic(self, params, transport): """Test magic.""" assert str(transport) == f"Transport({params.comm_name})" - async def test_connection_made(self, dummy_socket, transport, commparams): + async def test_connection_made(self, transport, commparams): """Test connection_made().""" transport.loop = None - transport.connection_made(dummy_socket()) + transport.connection_made(DummyTransport()) assert transport.transport assert not transport.recv_buffer assert not transport.reconnect_task @@ -76,9 +78,11 @@ async def test_connection_lost(self, transport): transport.close() assert not transport.reconnect_task - async def test_close(self, dummy_socket, transport): + async def test_close(self, transport): """Test close().""" - socket = dummy_socket() + socket = DummyTransport() + socket.abort = mock.Mock() + socket.close = mock.Mock() transport.connection_made(socket) transport.cb_connection_made.reset_mock() transport.cb_connection_lost.reset_mock() From 0a70c603d669ab842c91f79b565cef2b3e23bd07 Mon Sep 17 00:00:00 2001 From: jan iversen Date: Mon, 12 Jun 2023 13:16:31 +0200 Subject: [PATCH 06/12] pylint. --- pymodbus/transport/nullmodem.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/pymodbus/transport/nullmodem.py b/pymodbus/transport/nullmodem.py index 38efd425d..43eb039a2 100644 --- a/pymodbus/transport/nullmodem.py +++ b/pymodbus/transport/nullmodem.py @@ -69,8 +69,8 @@ async def transport_connect(self) -> bool: self.loop = asyncio.get_running_loop() self.transport, self.protocol = None, None if self.server: - self.server.connection_made(self.DummyTransport()) - self.connection_made(self.DummyTransport()) + self.server.connection_made(DummyTransport()) + self.connection_made(DummyTransport()) return True return False @@ -79,7 +79,7 @@ async def transport_listen(self): self.is_server = True self.server = self Log.debug("NullModem: Simulate listen on {}", self.comm_params.comm_name) - return self.DummyTransport() + return DummyTransport() # -------------------------------- # # Helper methods for child classes # From 89977acc6eb5f65be5eea6cf07b86ffd402e08b3 Mon Sep 17 00:00:00 2001 From: jan iversen Date: Mon, 12 Jun 2023 13:53:09 +0200 Subject: [PATCH 07/12] removed self.protocol --- pymodbus/transport/nullmodem.py | 37 +++++++++---------- pymodbus/transport/transport.py | 6 ++-- test/sub_transport/conftest.py | 17 +++++++++ test/sub_transport/test_nullmodem.py | 53 +++++++++++++++++----------- 4 files changed, 68 insertions(+), 45 deletions(-) diff --git a/pymodbus/transport/nullmodem.py b/pymodbus/transport/nullmodem.py index 43eb039a2..f1b5d7c0e 100644 --- a/pymodbus/transport/nullmodem.py +++ b/pymodbus/transport/nullmodem.py @@ -56,28 +56,20 @@ class NullModem(Transport): The class is designed to be an object in the message level class. """ - server: NullModem = None - client: NullModem = None - is_server: bool = False + other_end: NullModem = None async def transport_connect(self) -> bool: """Handle generic connect and call on to specific transport connect.""" - self.is_server = False - self.client = self Log.debug("NullModem: Simulate connect on {}", self.comm_params.comm_name) if not self.loop: self.loop = asyncio.get_running_loop() - self.transport, self.protocol = None, None - if self.server: - self.server.connection_made(DummyTransport()) - self.connection_made(DummyTransport()) + (self.transport,) = None, None + if self.other_end: return True return False async def transport_listen(self): """Handle generic listen and call on to specific transport listen.""" - self.is_server = True - self.server = self Log.debug("NullModem: Simulate listen on {}", self.comm_params.comm_name) return DummyTransport() @@ -90,10 +82,7 @@ async def send(self, data: bytes) -> bool: :param data: non-empty bytes object with data to send. """ Log.debug("NullModem: simulate send {}", data, ":hex") - if self.is_server: - self.client.data_received(data) - else: - self.server.data_received(data) + self.other_end.data_received(data) return True def close(self, reconnect: bool = False) -> None: @@ -103,12 +92,18 @@ def close(self, reconnect: bool = False) -> None: """ self.recv_buffer = b"" if not reconnect: - self.client.cb_connection_lost(None) - self.server.cb_connection_lost(None) - - def is_active(self) -> bool: - """Return true if connected/listening.""" - return True + self.connection_lost(None) + self.other_end.connection_lost.cb_connection_lost(None) + + def connection_made(self, transport: asyncio.BaseTransport): + """Simulate call from asyncio""" + self.other_end = transport + super().connection_made(transport) + + def connection_lost(self, reason: Exception): + """Simulate call from asyncio""" + self.other_end = None + super().connection_lost(reason) # ----------------- # # The magic methods # diff --git a/pymodbus/transport/transport.py b/pymodbus/transport/transport.py index 107c55ed0..89751fd06 100644 --- a/pymodbus/transport/transport.py +++ b/pymodbus/transport/transport.py @@ -94,7 +94,6 @@ def __init__( self.reconnect_delay_current: float = 0.0 self.transport: asyncio.BaseTransport | asyncio.Server = None - self.protocol: asyncio.BaseProtocol = None self.loop: asyncio.AbstractEventLoop = None self.reconnect_task: asyncio.Task = None self.recv_buffer: bytes = b"" @@ -266,9 +265,9 @@ async def transport_connect(self) -> bool: Log.debug("Connecting {}", self.comm_params.comm_name) if not self.loop: self.loop = asyncio.get_running_loop() - self.transport, self.protocol = None, None + self.transport = None try: - self.transport, self.protocol = await asyncio.wait_for( + self.transport, _protocol = await asyncio.wait_for( self.call_connect_listen(), timeout=self.comm_params.timeout_connect, ) @@ -369,7 +368,6 @@ def close(self, reconnect: bool = False) -> None: self.transport.abort() self.transport.close() self.transport = None - self.protocol = None if not reconnect and self.reconnect_task: self.reconnect_task.cancel() self.reconnect_task = None diff --git a/test/sub_transport/conftest.py b/test/sub_transport/conftest.py index 715838ea7..fad804bd1 100644 --- a/test/sub_transport/conftest.py +++ b/test/sub_transport/conftest.py @@ -85,6 +85,23 @@ async def prepare_nullmodem(): return transport +@pytest.fixture(name="nullmodem_server") +async def prepare_nullmodem_server(): + """Prepare nullmodem object.""" + transport = NullModem( + BaseParams.comm_name, + BaseParams.reconnect_delay, + BaseParams.reconnect_delay_max, + BaseParams.timeout_connect, + mock.Mock(name="cb_connection_made"), + mock.Mock(name="cb_connection_lost"), + mock.Mock(name="cb_handle_data", return_value=0), + ) + with suppress(RuntimeError): + transport.loop = asyncio.get_running_loop() + return transport + + @pytest_asyncio.fixture(name="transport_server") async def prepare_transport_server(): """Prepare transport object.""" diff --git a/test/sub_transport/test_nullmodem.py b/test/sub_transport/test_nullmodem.py index a533962a3..64b347f68 100644 --- a/test/sub_transport/test_nullmodem.py +++ b/test/sub_transport/test_nullmodem.py @@ -21,29 +21,42 @@ def test_DummyTransport(self): socket.set_protocol(None) socket.abort() - async def test_transport_connect(self, transport, commparams): + async def xtest_nullmodem_connect(self, nullmodem, nullmodem_server, commparams): """Test connection_made().""" - transport.loop = None - transport.connection_made(DummyTransport()) - assert transport.transport - assert not transport.recv_buffer - assert not transport.reconnect_task - assert transport.reconnect_delay_current == commparams.reconnect_delay - transport.cb_connection_made.assert_called_once() - transport.cb_connection_lost.assert_not_called() - transport.cb_handle_data.assert_not_called() - transport.close() + nullmodem.loop = None + assert not await nullmodem.transport_connect() + assert not nullmodem.other_end + assert nullmodem.loop + nullmodem.cb_connection_made.assert_not_called() + nullmodem.cb_connection_lost.assert_not_called() + nullmodem.cb_handle_data.assert_not_called() - async def test_close(self, transport): - """Test close().""" + nullmodem_server.loop = None + assert await nullmodem_server.transport_listen() + assert nullmodem_server.is_server + assert not nullmodem_server.client + assert nullmodem_server.server + assert nullmodem.loop + nullmodem_server.cb_connection_made.assert_not_called() + nullmodem_server.cb_connection_lost.assert_not_called() + nullmodem_server.cb_handle_data.assert_not_called() - async def test_datagram(self, transport): - """Test datagram_received().""" - transport.data_received = mock.MagicMock() - transport.datagram_received(b"abc", "127.0.0.1") - transport.data_received.assert_called_once() + assert await nullmodem.transport_connect() + assert not nullmodem.is_server + assert nullmodem.client + assert nullmodem.server + assert nullmodem.loop + nullmodem.cb_connection_made.assert_called_once() + nullmodem.cb_connection_lost.assert_not_called() + nullmodem.cb_handle_data.assert_not_called() + nullmodem_server.cb_connection_made.assert_called_once() + nullmodem.cb_connection_lost.assert_not_called() + nullmodem.cb_handle_data.assert_not_called() + + async def xtest_nullmodem_close(self, transport): + """Test close().""" - async def test_data(self, transport): + async def xtest_nullmodem_data(self, transport): """Test data_received.""" transport.cb_handle_data = mock.MagicMock(return_value=2) transport.data_received(b"123456") @@ -52,7 +65,7 @@ async def test_data(self, transport): transport.data_received(b"789") assert transport.recv_buffer == b"56789" - async def test_send(self, transport, params): + async def xtest_nullmodem_send(self, transport, params): """Test send().""" transport.transport = mock.AsyncMock() await transport.send(b"abc") From 64c07b47904cf415b47459f96fecdf7403359125 Mon Sep 17 00:00:00 2001 From: jan iversen Date: Mon, 12 Jun 2023 13:54:48 +0200 Subject: [PATCH 08/12] pylint. --- pymodbus/transport/nullmodem.py | 2 +- test/sub_transport/test_nullmodem.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/pymodbus/transport/nullmodem.py b/pymodbus/transport/nullmodem.py index f1b5d7c0e..bb6e0314e 100644 --- a/pymodbus/transport/nullmodem.py +++ b/pymodbus/transport/nullmodem.py @@ -63,7 +63,7 @@ async def transport_connect(self) -> bool: Log.debug("NullModem: Simulate connect on {}", self.comm_params.comm_name) if not self.loop: self.loop = asyncio.get_running_loop() - (self.transport,) = None, None + self.transport = None if self.other_end: return True return False diff --git a/test/sub_transport/test_nullmodem.py b/test/sub_transport/test_nullmodem.py index 64b347f68..96896d551 100644 --- a/test/sub_transport/test_nullmodem.py +++ b/test/sub_transport/test_nullmodem.py @@ -21,7 +21,7 @@ def test_DummyTransport(self): socket.set_protocol(None) socket.abort() - async def xtest_nullmodem_connect(self, nullmodem, nullmodem_server, commparams): + async def xtest_nullmodem_connect(self, nullmodem, nullmodem_server): """Test connection_made().""" nullmodem.loop = None assert not await nullmodem.transport_connect() From 975362d2f15e98c44a6cd0e916828aad4c08aae7 Mon Sep 17 00:00:00 2001 From: jan iversen Date: Mon, 12 Jun 2023 19:28:21 +0200 Subject: [PATCH 09/12] getting there. --- pymodbus/transport/nullmodem.py | 47 ++++++++++++++-------------- test/sub_transport/test_nullmodem.py | 41 ++++++++++++++++++------ 2 files changed, 54 insertions(+), 34 deletions(-) diff --git a/pymodbus/transport/nullmodem.py b/pymodbus/transport/nullmodem.py index bb6e0314e..40ff94740 100644 --- a/pymodbus/transport/nullmodem.py +++ b/pymodbus/transport/nullmodem.py @@ -45,32 +45,39 @@ def abort(self): class NullModem(Transport): """Transport layer. - Contains pure transport methods needed to connect/listen, send/receive and close connections - for unix socket, tcp, tls and serial communications. - - Contains high level methods like reconnect. - - This class is not available in the pymodbus API, and should not be referenced in Applications - nor in the pymodbus documentation. - - The class is designed to be an object in the message level class. + Contains methods to act as a null modem between 2 objects. + (Allowing tests to be shortcut without actual network calls) """ - other_end: NullModem = None + nullmodem_client: NullModem = None + nullmodem_server: NullModem = None + + def __init__(self, *arg): + """Overwrite init.""" + self.is_server: bool = False + self.other_end: NullModem = None + super().__init__(*arg) async def transport_connect(self) -> bool: """Handle generic connect and call on to specific transport connect.""" Log.debug("NullModem: Simulate connect on {}", self.comm_params.comm_name) if not self.loop: self.loop = asyncio.get_running_loop() - self.transport = None - if self.other_end: + if self.nullmodem_server: + self.__class__.nullmodem_client = self + self.other_end = self.nullmodem_server + self.cb_connection_made() + self.other_end.cb_connection_made() return True return False async def transport_listen(self): """Handle generic listen and call on to specific transport listen.""" Log.debug("NullModem: Simulate listen on {}", self.comm_params.comm_name) + if not self.loop: + self.loop = asyncio.get_running_loop() + self.is_server = True + self.__class__.nullmodem_server = self return DummyTransport() # -------------------------------- # @@ -92,18 +99,10 @@ def close(self, reconnect: bool = False) -> None: """ self.recv_buffer = b"" if not reconnect: - self.connection_lost(None) - self.other_end.connection_lost.cb_connection_lost(None) - - def connection_made(self, transport: asyncio.BaseTransport): - """Simulate call from asyncio""" - self.other_end = transport - super().connection_made(transport) - - def connection_lost(self, reason: Exception): - """Simulate call from asyncio""" - self.other_end = None - super().connection_lost(reason) + self.nullmodem_client.cb_connection_lost(None) + self.nullmodem_server.cb_connection_lost(None) + self.__class__.nullmodem_client = None + self.__class__.nullmodem_server = None # ----------------- # # The magic methods # diff --git a/test/sub_transport/test_nullmodem.py b/test/sub_transport/test_nullmodem.py index 96896d551..a0fe7a57a 100644 --- a/test/sub_transport/test_nullmodem.py +++ b/test/sub_transport/test_nullmodem.py @@ -21,37 +21,58 @@ def test_DummyTransport(self): socket.set_protocol(None) socket.abort() - async def xtest_nullmodem_connect(self, nullmodem, nullmodem_server): + def test_class_variables(self, nullmodem, nullmodem_server): + """Test connection_made().""" + assert not nullmodem.nullmodem_client + assert not nullmodem.nullmodem_server + assert not nullmodem_server.nullmodem_client + assert not nullmodem_server.nullmodem_server + nullmodem.__class__.nullmodem_client = self + nullmodem.is_server = False + nullmodem_server.__class__.nullmodem_server = self + nullmodem_server.is_server = True + + assert nullmodem.nullmodem_client == nullmodem_server.nullmodem_client + assert nullmodem.nullmodem_server == nullmodem_server.nullmodem_server + + async def test_transport_connect(self, nullmodem): """Test connection_made().""" nullmodem.loop = None assert not await nullmodem.transport_connect() - assert not nullmodem.other_end + assert not nullmodem.nullmodem_server + assert not nullmodem.nullmodem_client assert nullmodem.loop nullmodem.cb_connection_made.assert_not_called() nullmodem.cb_connection_lost.assert_not_called() nullmodem.cb_handle_data.assert_not_called() + async def test_transport_listen(self, nullmodem_server): + """Test connection_made().""" nullmodem_server.loop = None assert await nullmodem_server.transport_listen() assert nullmodem_server.is_server - assert not nullmodem_server.client - assert nullmodem_server.server - assert nullmodem.loop + assert nullmodem_server.nullmodem_server + assert not nullmodem_server.nullmodem_client + assert nullmodem_server.loop nullmodem_server.cb_connection_made.assert_not_called() nullmodem_server.cb_connection_lost.assert_not_called() nullmodem_server.cb_handle_data.assert_not_called() + async def test_connected(self, nullmodem, nullmodem_server): + """Test connection is correct.""" + assert await nullmodem_server.transport_listen() assert await nullmodem.transport_connect() - assert not nullmodem.is_server - assert nullmodem.client - assert nullmodem.server + assert nullmodem.nullmodem_client + assert nullmodem.nullmodem_server assert nullmodem.loop + assert not nullmodem.is_server + assert nullmodem_server.is_server nullmodem.cb_connection_made.assert_called_once() nullmodem.cb_connection_lost.assert_not_called() nullmodem.cb_handle_data.assert_not_called() nullmodem_server.cb_connection_made.assert_called_once() - nullmodem.cb_connection_lost.assert_not_called() - nullmodem.cb_handle_data.assert_not_called() + nullmodem_server.cb_connection_lost.assert_not_called() + nullmodem_server.cb_handle_data.assert_not_called() async def xtest_nullmodem_close(self, transport): """Test close().""" From c04f7e8a6ad86f453d4e685c4153f95981990de2 Mon Sep 17 00:00:00 2001 From: jan iversen Date: Mon, 12 Jun 2023 19:37:02 +0200 Subject: [PATCH 10/12] missing data. --- pymodbus/transport/nullmodem.py | 6 ++++-- test/sub_transport/test_nullmodem.py | 27 ++++++++++++++++++++++++++- 2 files changed, 30 insertions(+), 3 deletions(-) diff --git a/pymodbus/transport/nullmodem.py b/pymodbus/transport/nullmodem.py index 40ff94740..f0b46935f 100644 --- a/pymodbus/transport/nullmodem.py +++ b/pymodbus/transport/nullmodem.py @@ -99,8 +99,10 @@ def close(self, reconnect: bool = False) -> None: """ self.recv_buffer = b"" if not reconnect: - self.nullmodem_client.cb_connection_lost(None) - self.nullmodem_server.cb_connection_lost(None) + if self.nullmodem_client: + self.nullmodem_client.cb_connection_lost(None) + if self.nullmodem_server: + self.nullmodem_server.cb_connection_lost(None) self.__class__.nullmodem_client = None self.__class__.nullmodem_server = None diff --git a/test/sub_transport/test_nullmodem.py b/test/sub_transport/test_nullmodem.py index a0fe7a57a..330ce66fb 100644 --- a/test/sub_transport/test_nullmodem.py +++ b/test/sub_transport/test_nullmodem.py @@ -74,8 +74,33 @@ async def test_connected(self, nullmodem, nullmodem_server): nullmodem_server.cb_connection_lost.assert_not_called() nullmodem_server.cb_handle_data.assert_not_called() - async def xtest_nullmodem_close(self, transport): + async def test_client_close(self, nullmodem, nullmodem_server): """Test close().""" + assert await nullmodem_server.transport_listen() + assert await nullmodem.transport_connect() + nullmodem.close() + assert not nullmodem.nullmodem_client + assert not nullmodem.nullmodem_server + nullmodem.cb_connection_made.assert_called_once() + nullmodem.cb_connection_lost.assert_called_once() + nullmodem.cb_handle_data.assert_not_called() + nullmodem_server.cb_connection_made.assert_called_once() + nullmodem_server.cb_connection_lost.assert_called_once() + nullmodem_server.cb_handle_data.assert_not_called() + + async def test_server_close(self, nullmodem, nullmodem_server): + """Test close().""" + assert await nullmodem_server.transport_listen() + assert await nullmodem.transport_connect() + nullmodem_server.close() + assert not nullmodem.nullmodem_client + assert not nullmodem.nullmodem_server + nullmodem.cb_connection_made.assert_called_once() + nullmodem.cb_connection_lost.assert_called_once() + nullmodem.cb_handle_data.assert_not_called() + nullmodem_server.cb_connection_made.assert_called_once() + nullmodem_server.cb_connection_lost.assert_called_once() + nullmodem_server.cb_handle_data.assert_not_called() async def xtest_nullmodem_data(self, transport): """Test data_received.""" From a77b6f380271d41117216320c6fd8aec9e178e72 Mon Sep 17 00:00:00 2001 From: jan iversen Date: Mon, 12 Jun 2023 19:49:16 +0200 Subject: [PATCH 11/12] finally. --- pymodbus/transport/nullmodem.py | 1 + test/sub_transport/test_nullmodem.py | 33 +++++++++++++--------------- 2 files changed, 16 insertions(+), 18 deletions(-) diff --git a/pymodbus/transport/nullmodem.py b/pymodbus/transport/nullmodem.py index f0b46935f..ea8685736 100644 --- a/pymodbus/transport/nullmodem.py +++ b/pymodbus/transport/nullmodem.py @@ -66,6 +66,7 @@ async def transport_connect(self) -> bool: if self.nullmodem_server: self.__class__.nullmodem_client = self self.other_end = self.nullmodem_server + self.nullmodem_server.other_end = self self.cb_connection_made() self.other_end.cb_connection_made() return True diff --git a/test/sub_transport/test_nullmodem.py b/test/sub_transport/test_nullmodem.py index 330ce66fb..3e61765e7 100644 --- a/test/sub_transport/test_nullmodem.py +++ b/test/sub_transport/test_nullmodem.py @@ -1,5 +1,4 @@ """Test transport.""" -from unittest import mock from pymodbus.transport.nullmodem import DummyTransport @@ -102,20 +101,18 @@ async def test_server_close(self, nullmodem, nullmodem_server): nullmodem_server.cb_connection_lost.assert_called_once() nullmodem_server.cb_handle_data.assert_not_called() - async def xtest_nullmodem_data(self, transport): - """Test data_received.""" - transport.cb_handle_data = mock.MagicMock(return_value=2) - transport.data_received(b"123456") - transport.cb_handle_data.assert_called_once() - assert transport.recv_buffer == b"3456" - transport.data_received(b"789") - assert transport.recv_buffer == b"56789" - - async def xtest_nullmodem_send(self, transport, params): - """Test send().""" - transport.transport = mock.AsyncMock() - await transport.send(b"abc") - - transport.setup_udp(False, params.host, params.port) - await transport.send(b"abc") - transport.close() + async def test_data(self, nullmodem, nullmodem_server): + """Test data exchange.""" + data = b"abcd" + assert await nullmodem_server.transport_listen() + assert await nullmodem.transport_connect() + assert await nullmodem.send(data) + assert nullmodem_server.recv_buffer == data + assert not nullmodem.recv_buffer + nullmodem.cb_handle_data.assert_not_called() + nullmodem_server.cb_handle_data.assert_called_once() + assert await nullmodem_server.send(data) + assert nullmodem_server.recv_buffer == data + assert nullmodem.recv_buffer == data + nullmodem.cb_handle_data.assert_called_once() + nullmodem_server.cb_handle_data.assert_called_once() From 2551e0f285d1c1745bde7f85fa4f6eacacde81a9 Mon Sep 17 00:00:00 2001 From: jan iversen Date: Mon, 12 Jun 2023 20:19:34 +0200 Subject: [PATCH 12/12] CI. --- test/sub_transport/conftest.py | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/test/sub_transport/conftest.py b/test/sub_transport/conftest.py index fad804bd1..29bfdbdc7 100644 --- a/test/sub_transport/conftest.py +++ b/test/sub_transport/conftest.py @@ -80,6 +80,8 @@ async def prepare_nullmodem(): mock.Mock(name="cb_connection_lost"), mock.Mock(name="cb_handle_data", return_value=0), ) + transport.__class__.nullmodem_client = None + transport.__class__.nullmodem_server = None with suppress(RuntimeError): transport.loop = asyncio.get_running_loop() return transport @@ -97,6 +99,8 @@ async def prepare_nullmodem_server(): mock.Mock(name="cb_connection_lost"), mock.Mock(name="cb_handle_data", return_value=0), ) + transport.__class__.nullmodem_client = None + transport.__class__.nullmodem_server = None with suppress(RuntimeError): transport.loop = asyncio.get_running_loop() return transport