From dee7f5680cbd794ec695aad2480c0230b3440ed9 Mon Sep 17 00:00:00 2001 From: jan iversen Date: Thu, 28 Aug 2025 17:44:59 +0000 Subject: [PATCH] Test coverage pdu 100%. --- pymodbus/pdu/decoders.py | 3 +- pymodbus/pdu/pdu.py | 45 ------------------------ pymodbus/server/requesthandler.py | 2 +- test/conftest.py | 3 ++ test/pdu/test_bit.py | 13 +++++++ test/pdu/test_decoders.py | 11 ++++++ test/{not_updated => pdu}/test_device.py | 7 ++++ test/{not_updated => pdu}/test_events.py | 0 test/pdu/test_mei_messages.py | 8 +++++ test/pdu/test_register_read_messages.py | 39 +++++++++++--------- test/pdu/test_register_write_messages.py | 14 ++++++-- test/server/test_server_asyncio.py | 29 --------------- 12 files changed, 79 insertions(+), 95 deletions(-) rename test/{not_updated => pdu}/test_device.py (98%) rename test/{not_updated => pdu}/test_events.py (100%) diff --git a/pymodbus/pdu/decoders.py b/pymodbus/pdu/decoders.py index 30be7b725..ecf1cf93e 100644 --- a/pymodbus/pdu/decoders.py +++ b/pymodbus/pdu/decoders.py @@ -6,7 +6,8 @@ from pymodbus.exceptions import MessageRegisterException, ModbusException from pymodbus.logging import Log -from .pdu import ExceptionResponse, ModbusPDU +from .exceptionresponse import ExceptionResponse +from .pdu import ModbusPDU class DecodePDU: diff --git a/pymodbus/pdu/pdu.py b/pymodbus/pdu/pdu.py index 236c4ac3d..038ea5666 100644 --- a/pymodbus/pdu/pdu.py +++ b/pymodbus/pdu/pdu.py @@ -111,51 +111,6 @@ def calculateRtuFrameSize(cls, data: bytes) -> int: ) -class ExceptionResponse(ModbusPDU): - """Base class for a modbus exception PDU.""" - - rtu_frame_size = 5 - - ILLEGAL_FUNCTION = 0x01 - ILLEGAL_ADDRESS = 0x02 - ILLEGAL_VALUE = 0x03 - DEVICE_FAILURE = 0x04 - ACKNOWLEDGE = 0x05 - DEVICE_BUSY = 0x06 - NEGATIVE_ACKNOWLEDGE = 0x07 - MEMORY_PARITY_ERROR = 0x08 - GATEWAY_PATH_UNAVIABLE = 0x0A - GATEWAY_NO_RESPONSE = 0x0B - - def __init__( - self, - function_code: int, - exception_code: int = 0, - device_id: int = 1, - transaction: int = 0) -> None: - """Initialize the modbus exception response.""" - super().__init__(transaction_id=transaction, dev_id=device_id) - self.function_code = function_code | 0x80 - self.exception_code = exception_code - - def __str__(self) -> str: - """Build a representation of an exception response.""" - return ( - f"{self.__class__.__name__}(" - f"dev_id={self.dev_id}, " - f"function_code={self.function_code}, " - f"exception_code={self.exception_code})" - ) - - def encode(self) -> bytes: - """Encode a modbus exception response.""" - return struct.pack(">B", self.exception_code) - - def decode(self, data: bytes) -> None: - """Decode a modbus exception response.""" - self.exception_code = int(data[0]) - - def pack_bitstring(bits: list[bool], align_byte=True) -> bytes: """Create a bytestring out of a list of bits. diff --git a/pymodbus/server/requesthandler.py b/pymodbus/server/requesthandler.py index 5a0ca6fe3..c20560767 100644 --- a/pymodbus/server/requesthandler.py +++ b/pymodbus/server/requesthandler.py @@ -7,7 +7,7 @@ from pymodbus.constants import ExcCodes from pymodbus.exceptions import ModbusIOException, NoSuchIdException from pymodbus.logging import Log -from pymodbus.pdu.pdu import ExceptionResponse +from pymodbus.pdu import ExceptionResponse from pymodbus.transaction import TransactionManager from pymodbus.transport import CommParams diff --git a/test/conftest.py b/test/conftest.py index 3dfdab5a1..25baaed30 100644 --- a/test/conftest.py +++ b/test/conftest.py @@ -8,6 +8,7 @@ import pytest import pytest_asyncio +from pymodbus.constants import ExcCodes from pymodbus.datastore import ModbusBaseDeviceContext from pymodbus.server import ServerAsyncStop from pymodbus.transport import NULLMODEM_HOST, CommParams, CommType @@ -218,6 +219,8 @@ def __init__(self, valid=False, default=True): def getValues(self, _fc, _address, count=0): """Get values.""" + if count > 0x100: + return ExcCodes.ILLEGAL_VALUE return [self.default] * count def setValues(self, _fc, _address, _values): diff --git a/test/pdu/test_bit.py b/test/pdu/test_bit.py index a51738e57..276e08bd5 100644 --- a/test/pdu/test_bit.py +++ b/test/pdu/test_bit.py @@ -4,6 +4,7 @@ import pytest import pymodbus.pdu.bit_message as bit_msg +from pymodbus.constants import ExcCodes class TestModbusBitMessage: @@ -34,6 +35,18 @@ async def test_bit_read_update_datastore_value_errors(self, mock_context): ): await pdu.update_datastore(context) + async def test_bit_datastore_exceptions(self, mock_context): + """Test bit exception response from datastore.""" + context = mock_context() + context.async_getValues = mock.AsyncMock(return_value=ExcCodes.ILLEGAL_VALUE) + for pdu in ( + (bit_msg.ReadCoilsRequest(address=1, count=0x800)), + (bit_msg.ReadDiscreteInputsRequest(address=1, count=0x800)), + (bit_msg.WriteSingleCoilRequest(address=1, bits=[True])), + (bit_msg.WriteMultipleCoilsRequest(address=1, bits=[True] * 5)), + ): + await pdu.update_datastore(context) + async def test_bit_read_update_datastore_address_errors(self, mock_context): """Test bit read request encoding.""" context = mock_context() diff --git a/test/pdu/test_decoders.py b/test/pdu/test_decoders.py index 3bf1664ac..fec67e78e 100644 --- a/test/pdu/test_decoders.py +++ b/test/pdu/test_decoders.py @@ -100,6 +100,17 @@ def test_client_lookup(self, code, frame): if not code & 0x80: assert pdu.function_code == code + def test_client_lookup_no_fc(self): + """Test lookup for responses.""" + data = b'\x01\x70' + pdu = self.client.lookupPduClass(data) + assert not pdu + + def test_list_function_codes(self): + """Test lookup for responses.""" + fc_list = self.client.list_function_codes() + assert fc_list + @pytest.mark.parametrize(("code", "frame"), list(requests)) def test_server_lookup(self, code, frame): """Test lookup for requests.""" diff --git a/test/not_updated/test_device.py b/test/pdu/test_device.py similarity index 98% rename from test/not_updated/test_device.py rename to test/pdu/test_device.py index eea528374..4b4f8e936 100644 --- a/test/not_updated/test_device.py +++ b/test/pdu/test_device.py @@ -362,3 +362,10 @@ def test_modbus_plus_statistics_helpers(self): stats_summary = list(statistics.summary()) assert sorted(summary) == sorted(stats_summary) assert not sum(sum(value[1]) for value in statistics) + + + def test_device_info_name(self): + """Test setting of info_name.""" + name = "Maintainer" + info = ModbusDeviceIdentification(info_name={"VendorName": name}) + assert name == info.VendorName diff --git a/test/not_updated/test_events.py b/test/pdu/test_events.py similarity index 100% rename from test/not_updated/test_events.py rename to test/pdu/test_events.py diff --git a/test/pdu/test_mei_messages.py b/test/pdu/test_mei_messages.py index a16b1e5f7..2256a082d 100644 --- a/test/pdu/test_mei_messages.py +++ b/test/pdu/test_mei_messages.py @@ -81,6 +81,14 @@ def test_read_device_information_calc1(self): assert handle.calculateRtuFrameSize(b"\x0e\x01\x83") == 999 assert handle.calculateRtuFrameSize(b"\x0e\x01\x83\x00\x00\x03\x01\x03") == 998 + + def test_read_device_information_sub_fc(self): + """Test calculateRtuFrameSize, short buffer.""" + handle = ReadDeviceInformationResponse() + assert handle.decode_sub_function_code(b"\x0e\x01\x83") == 0x83 + handle = ReadDeviceInformationRequest() + assert handle.decode_sub_function_code(b"\x0e\x01\x83") == 0x83 + def test_read_device_information_encode(self): """Test that the read fifo queue response can encode.""" message = b"\x0e\x01\x83\x00\x00\x03" diff --git a/test/pdu/test_register_read_messages.py b/test/pdu/test_register_read_messages.py index f7eca47c0..bbc540e3a 100644 --- a/test/pdu/test_register_read_messages.py +++ b/test/pdu/test_register_read_messages.py @@ -17,11 +17,6 @@ TEST_MESSAGE = b"\x06\x00\x0a\x00\x0b\x00\x0c" -# ---------------------------------------------------------------------------# -# Fixture -# ---------------------------------------------------------------------------# - - class TestReadRegisterMessages: """Register Message Test Fixture. @@ -86,14 +81,15 @@ def test_register_read_response_decode_error(self): with pytest.raises(ModbusIOException): reg.decode(b'\x14\x00\x03\x00\x11') - async def test_register_read_requests_count_errors(self): + async def test_register_read_requests_count_errors(self, mock_context): """This tests that the register request messages. will break on counts that are out of range """ + context = mock_context() requests = [ - #ReadHoldingRegistersRequest(address=1, count=0x800), - #ReadInputRegistersRequest(address=1, count=0x800), + ReadHoldingRegistersRequest(address=1, count=0x800), + ReadInputRegistersRequest(address=1, count=0x800), ReadWriteMultipleRegistersRequest( read_address=1, read_count=0x800, write_address=1, write_registers=[5] ), @@ -102,7 +98,7 @@ async def test_register_read_requests_count_errors(self): ), ] for request in requests: - result = await request.update_datastore(None) + result = await request.update_datastore(context) assert result.exception_code == ExcCodes.ILLEGAL_VALUE async def test_register_read_requests_verify_errors(self, mock_context): @@ -114,8 +110,8 @@ async def test_register_read_requests_verify_errors(self, mock_context): requests = [ ReadHoldingRegistersRequest(address=-1, count=5), ReadInputRegistersRequest(address=-1, count=5), - # ReadWriteMultipleRegistersRequest(-1,5,1,5), - # ReadWriteMultipleRegistersRequest(1,5,-1,5), + ReadWriteMultipleRegistersRequest(-1,5,1,[5]), + ReadWriteMultipleRegistersRequest(1,5,-1,[5]), ] for request in requests: await request.update_datastore(context) @@ -147,17 +143,28 @@ async def test_read_write_multiple_registers_verify(self, mock_context): """Test read/write multiple registers.""" context = mock_context() request = ReadWriteMultipleRegistersRequest( - read_address=1, read_count=10, write_address=2, write_registers=[0x00] + read_address=1, read_count=0x200, write_address=2, write_registers=[0x00] ) - await request.update_datastore(context) - #assert response.exception_code == ExcCodes.ILLEGAL_ADDRESS + response = await request.update_datastore(context) + assert response.exception_code == ExcCodes.ILLEGAL_VALUE await request.update_datastore(context) - #assert response.exception_code == ExcCodes.ILLEGAL_ADDRESS + assert response.exception_code == ExcCodes.ILLEGAL_VALUE request.write_byte_count = 0x100 await request.update_datastore(context) - #assert response.exception_code == ExcCodes.ILLEGAL_VALUE + assert response.exception_code == ExcCodes.ILLEGAL_VALUE + + async def test_register_datastore_exceptions(self, mock_context): + """Test exception response from datastore.""" + context = mock_context() + context.async_getValues = mock.AsyncMock(return_value=ExcCodes.ILLEGAL_VALUE) + for pdu in ( + ReadHoldingRegistersRequest(address=-1, count=5), + ReadInputRegistersRequest(address=-1, count=5), + ReadWriteMultipleRegistersRequest(-1, 5, 1, [5], 1), + ): + await pdu.update_datastore(context) def test_serializing_to_string(self): """Test serializing to string.""" diff --git a/test/pdu/test_register_write_messages.py b/test/pdu/test_register_write_messages.py index bb978832a..7c6b9d8cf 100644 --- a/test/pdu/test_register_write_messages.py +++ b/test/pdu/test_register_write_messages.py @@ -110,9 +110,17 @@ async def test_write_multiple_register_request(self, mock_context): result = await request.update_datastore(context) assert result.function_code == request.function_code - # -----------------------------------------------------------------------# - # Mask Write Register Request - # -----------------------------------------------------------------------# + + async def test_register_write_datastore_exceptions(self, mock_context): + """Test exception response from datastore.""" + context = mock_context() + context.async_getValues = mock.AsyncMock(return_value=ExcCodes.ILLEGAL_VALUE) + for pdu in ( + WriteSingleRegisterRequest(address=0x00, registers=[0xF000]), + WriteMultipleRegistersRequest(address=0x00, registers=[0x00] * 10), + MaskWriteRegisterRequest(0x0000, 0x0101, 0x1010), + ): + await pdu.update_datastore(context) def test_mask_write_register_request_encode(self): """Test basic bit message encoding/decoding.""" diff --git a/test/server/test_server_asyncio.py b/test/server/test_server_asyncio.py index 544d7e090..23f82c810 100755 --- a/test/server/test_server_asyncio.py +++ b/test/server/test_server_asyncio.py @@ -9,7 +9,6 @@ import pytest from pymodbus import FramerType, ModbusDeviceIdentification -from pymodbus.client import AsyncModbusTcpClient from pymodbus.datastore import ( ModbusDeviceContext, ModbusSequentialDataBlock, @@ -240,21 +239,6 @@ async def test_async_tcp_server_roundtrip(self): await asyncio.wait_for(BasicClient.done, timeout=0.1) assert BasicClient.received_data, expected_response - @pytest.mark.skip - async def test_async_server_file_descriptors(self): # pragma: no cover - """Test sending and receiving data on tcp socket. - - This test takes a long time (minutes) to run, so should only run when needed. - """ - addr = ("127.0.0.1", 25001) - await self.start_server(serv_addr=addr) - for _ in range(2048): - client = AsyncModbusTcpClient(addr[0], framer=FramerType.SOCKET, port=addr[1]) - await client.connect() - response = await client.read_coils(31, count=1, device_id=1) - assert not response.isError() - client.close() - async def test_async_server_trace_connect_disconnect(self): """Test connect/disconnect trace handler.""" trace_connect = mock.Mock() @@ -392,16 +376,3 @@ async def test_async_udp_server_exception(self): ) await asyncio.wait_for(BasicClient.connected, timeout=0.1) assert not BasicClient.done.done() - - @pytest.mark.skip - async def test_async_tcp_server_exception(self): # pragma: no cover - """Send garbage data on a TCP socket should drop the connection.""" - BasicClient.data = b"\xFF\xFF\xFF\xFF\xFF\xFF\xFF\xFF" - await self.start_server() - with mock.patch( - "pymodbus.framer.FramerSocket.handleFrame", - new_callable=lambda: mock.Mock(side_effect=Exception), - ): - await self.connect_server() - await asyncio.wait_for(BasicClient.eof, timeout=0.1) - # neither of these should timeout if the test is successful