diff --git a/examples/message_parser.py b/examples/message_parser.py index 312a27cc3..6fe3b1b42 100755 --- a/examples/message_parser.py +++ b/examples/message_parser.py @@ -80,7 +80,7 @@ def decode(self, message): print(f"{decoder.decoder.__class__.__name__}") print("-" * 80) try: - _, pdu = decoder.processIncomingFrame(message) + _, pdu = decoder.handleFrame(message, 0, 0) self.report(pdu) except Exception: # pylint: disable=broad-except self.check_errors(decoder, message) diff --git a/pymodbus/client/serial.py b/pymodbus/client/serial.py index d5cc73c37..30bfa69ce 100644 --- a/pymodbus/client/serial.py +++ b/pymodbus/client/serial.py @@ -270,7 +270,7 @@ def send(self, request: bytes, addr: tuple | None = None) -> int: if waitingbytes := self._in_waiting(): result = self.socket.read(waitingbytes) Log.warning("Cleanup recv buffer before send: {}", result, ":hex") - if (size := self.socket.write(request)) is None: + if (size := self.socket.write(request)) is None: # pragma: no cover size = 0 return size return 0 diff --git a/pymodbus/client/tcp.py b/pymodbus/client/tcp.py index fb08076bd..dcf71f1a0 100644 --- a/pymodbus/client/tcp.py +++ b/pymodbus/client/tcp.py @@ -259,7 +259,7 @@ def recv(self, size: int | None) -> bytes: return self._handle_abrupt_socket_close( size, data, time.time() - time_ ) - except SSLWantReadError: + except SSLWantReadError: # pragma: no cover continue data.append(recv_data) data_length += len(recv_data) diff --git a/pymodbus/constants.py b/pymodbus/constants.py index 472c70e4c..bd025adbb 100644 --- a/pymodbus/constants.py +++ b/pymodbus/constants.py @@ -112,10 +112,6 @@ class DeviceInformation(int, enum.Enum): EXTENDED = 0x03 SPECIFIC = 0x04 - def __str__(self): - """Override to force int representation for enum members.""" - return str(int(self)) - class MoreData(int, enum.Enum): """Represents the more follows condition. diff --git a/pymodbus/exceptions.py b/pymodbus/exceptions.py index 89e219d02..8d8392d00 100644 --- a/pymodbus/exceptions.py +++ b/pymodbus/exceptions.py @@ -5,7 +5,6 @@ __all__ = [ "ConnectionException", - "InvalidMessageReceivedException", "MessageRegisterException", "ModbusIOException", "NoSuchIdException", @@ -95,18 +94,6 @@ def __init__(self, string=""): ModbusException.__init__(self, message) -class InvalidMessageReceivedException(ModbusException): - """Error resulting from invalid response received or decoded.""" - - def __init__(self, string=""): - """Initialize the exception. - - :param string: The message to append to the error - """ - message = f"[Invalid Message] {string}" - ModbusException.__init__(self, message) - - class MessageRegisterException(ModbusException): """Error resulting from failing to register a custom message request/response.""" diff --git a/pymodbus/framer/base.py b/pymodbus/framer/base.py index ddad1a11b..e09025985 100644 --- a/pymodbus/framer/base.py +++ b/pymodbus/framer/base.py @@ -63,40 +63,29 @@ def buildFrame(self, message: ModbusPDU) -> bytes: frame = self.encode(data, message.dev_id, message.transaction_id) return frame - def processIncomingFrame(self, data: bytes) -> tuple[int, ModbusPDU | None]: - """Process new packet pattern. - - This takes in a new request packet, adds it to the current - packet stream, and performs framing on it. That is, checks - for complete messages, and once found, will process all that - exist. - """ + def handleFrame(self, data: bytes, exp_devid: int, exp_tid: int) -> tuple[int, ModbusPDU | None]: + """Process incoming data.""" used_len = 0 while True: - data_len, pdu = self._processIncomingFrame(data[used_len:]) + if used_len >= len(data): + return used_len, None + Log.debug("Processing: {}", data, ":hex") + data_len, dev_id, tid, frame_data = self.decode(data) used_len += data_len - if not data_len: + if not data_len or not frame_data: return used_len, None - if pdu: - return used_len, pdu - - def _processIncomingFrame(self, data: bytes) -> tuple[int, ModbusPDU | None]: - """Process new packet pattern. - - This takes in a new request packet, adds it to the current - packet stream, and performs framing on it. That is, checks - for complete messages, and once found, will process all that - exist. - """ - Log.debug("Processing: {}", data, ":hex") - if not data: - return 0, None - used_len, dev_id, tid, frame_data = self.decode(data) - if not frame_data: - return used_len, None - if (result := self.decoder.decode(frame_data)) is None: - raise ModbusIOException("Unable to decode request") - result.dev_id = dev_id - result.transaction_id = tid - Log.debug("Frame advanced, resetting header!!") - return used_len, result + if exp_devid and dev_id != exp_devid: + Log.error( + f"ERROR: request ask for id={exp_devid} but got id={dev_id}, Skipping." + ) + continue + if exp_tid and tid and tid != exp_tid: + Log.error( + f"ERROR: request ask for transaction_id={exp_tid} but got id={tid}, Skipping." + ) + continue + if (pdu := self.decoder.decode(frame_data)) is None: + raise ModbusIOException("Unable to decode request") + pdu.dev_id = dev_id + pdu.transaction_id = tid + return used_len, pdu diff --git a/pymodbus/logging.py b/pymodbus/logging.py index 764eab215..aee5e19be 100644 --- a/pymodbus/logging.py +++ b/pymodbus/logging.py @@ -83,7 +83,7 @@ def build_msg(cls, txt, *args): string_args.append(hexlify_packets(args[i])) elif args[i + 1] == ":str": string_args.append(str(args[i])) - elif args[i + 1] == ":b2a": + else: # args[i + 1] == ":b2a": string_args.append(b2a_hex(args[i])) skip = True else: @@ -117,5 +117,4 @@ def error(cls, txt, *args): @classmethod def critical(cls, txt, *args): """Log critical messages.""" - if cls._logger.isEnabledFor(logging.CRITICAL): - cls._logger.critical(cls.build_msg(txt, *args), stacklevel=2) + cls._logger.critical(cls.build_msg(txt, *args), stacklevel=2) diff --git a/pymodbus/pdu/bit_message.py b/pymodbus/pdu/bit_message.py index 80f1d7ed7..6579fbe23 100644 --- a/pymodbus/pdu/bit_message.py +++ b/pymodbus/pdu/bit_message.py @@ -38,8 +38,6 @@ async def update_datastore(self, context: ModbusDeviceContext) -> ModbusPDU: values = await context.async_getValues( self.function_code, self.address, self.count ) - if isinstance(values, int): - return ExceptionResponse(self.function_code, values) response_class = (ReadCoilsResponse if self.function_code == 1 else ReadDiscreteInputsResponse) return response_class(dev_id=self.dev_id, transaction_id=self.transaction_id, bits=cast(list[bool], values)) @@ -98,9 +96,6 @@ async def update_datastore(self, context: ModbusDeviceContext) -> ModbusPDU: if (rc := await context.async_setValues(self.function_code, self.address, self.bits)): return ExceptionResponse(self.function_code, rc) values = await context.async_getValues(self.function_code, self.address, 1) - if isinstance(values, int): - return ExceptionResponse(self.function_code, values) - return WriteSingleCoilResponse(address=self.address, bits=cast(list[bool], values), dev_id=self.dev_id, transaction_id=self.transaction_id) def get_response_pdu_size(self) -> int: diff --git a/pymodbus/pdu/device.py b/pymodbus/pdu/device.py index 2db85e485..9303dfcc7 100644 --- a/pymodbus/pdu/device.py +++ b/pymodbus/pdu/device.py @@ -18,7 +18,7 @@ # pylint: disable=missing-type-doc from collections import OrderedDict -from pymodbus.constants import INTERNAL_ERROR, DeviceInformation +from pymodbus.constants import DeviceInformation from pymodbus.pdu.events import ModbusEvent from pymodbus.utilities import dict_property @@ -293,11 +293,6 @@ def __gets(cls, identity, object_ids): # pylint: disable=unused-private-member """ return {oid: identity[oid] for oid in object_ids if identity[oid]} - def __init__(self): - """Prohibit objects.""" - raise RuntimeError(INTERNAL_ERROR) - - # ---------------------------------------------------------------------------# # Counters Handler # ---------------------------------------------------------------------------# @@ -562,8 +557,7 @@ def setDiagnostic(self, mapping): :param mapping: Dictionary of key:value pairs to set """ for entry in iter(mapping.items()): - if entry[0] >= 0 and entry[0] < len(self._diagnostic): - self._diagnostic[entry[0]] = bool(entry[1]) + self._diagnostic[entry[0]] = bool(entry[1]) def getDiagnostic(self, bit): """Get the value in the diagnostic register. diff --git a/pymodbus/pdu/register_message.py b/pymodbus/pdu/register_message.py index 3b67ee266..aa199acca 100644 --- a/pymodbus/pdu/register_message.py +++ b/pymodbus/pdu/register_message.py @@ -2,6 +2,7 @@ from __future__ import annotations import struct +from collections.abc import Sequence from typing import cast from pymodbus.datastore import ModbusDeviceContext @@ -37,8 +38,6 @@ async def update_datastore(self, context: ModbusDeviceContext) -> ModbusPDU: values = await context.async_getValues( self.function_code, self.address, self.count ) - if isinstance(values, int): - return ExceptionResponse(self.function_code, values) response_class = (ReadHoldingRegistersResponse if self.function_code == 3 else ReadInputRegistersResponse) return response_class(registers=cast(list[int], values), dev_id=self.dev_id, transaction_id=self.transaction_id) @@ -147,8 +146,6 @@ async def update_datastore(self, context: ModbusDeviceContext) -> ModbusPDU: registers = await context.async_getValues( self.function_code, self.read_address, self.read_count ) - if isinstance(registers, int): - return ExceptionResponse(self.function_code, registers) return ReadWriteMultipleRegistersResponse(registers=cast(list[int], registers), dev_id=self.dev_id, transaction_id=self.transaction_id) def get_response_pdu_size(self) -> int: @@ -194,8 +191,6 @@ async def update_datastore(self, context: ModbusDeviceContext) -> ModbusPDU: if rc: return ExceptionResponse(self.function_code, rc) values = await context.async_getValues(self.function_code, self.address, 1) - if isinstance(values, int): - return ExceptionResponse(self.function_code, values) return WriteSingleRegisterResponse(address=self.address, registers=cast(list[int], values)) def get_response_pdu_size(self) -> int: @@ -288,9 +283,7 @@ async def update_datastore(self, context: ModbusDeviceContext) -> ModbusPDU: if not 0x0000 <= self.or_mask <= 0xFFFF: return ExceptionResponse(self.function_code, ExceptionResponse.ILLEGAL_VALUE) values = await context.async_getValues(self.function_code, self.address, 1) - if isinstance(values, int): - return ExceptionResponse(self.function_code, values) - values = (values[0] & self.and_mask) | (self.or_mask & ~self.and_mask) + values = (cast(Sequence[int | bool], values)[0] & self.and_mask) | (self.or_mask & ~self.and_mask) rc = await context.async_setValues( self.function_code, self.address, cast(list[int], [values]) ) diff --git a/pymodbus/transaction/transaction.py b/pymodbus/transaction/transaction.py index 007d0c1ac..9a2ea5778 100644 --- a/pymodbus/transaction/transaction.py +++ b/pymodbus/transaction/transaction.py @@ -79,7 +79,7 @@ def dummy_trace_connect(self, connect: bool) -> None: """Do dummy trace.""" _ = connect - def sync_get_response(self, dev_id) -> ModbusPDU: + def sync_get_response(self, dev_id, tid) -> ModbusPDU: """Receive until PDU is correct or timeout.""" databuffer = b'' while True: @@ -108,13 +108,9 @@ def sync_get_response(self, dev_id) -> ModbusPDU: continue databuffer += data - used_len, pdu = self.framer.processIncomingFrame(self.trace_packet(False, databuffer)) + used_len, pdu = self.framer.handleFrame(self.trace_packet(False, databuffer), dev_id, tid) databuffer = databuffer[used_len:] if pdu: - if pdu.dev_id != dev_id: - raise ModbusIOException( - f"ERROR: request ask for id={dev_id} but id={pdu.dev_id}, CLOSING CONNECTION." - ) return self.trace_pdu(False, pdu) def sync_execute(self, no_response_expected: bool, request: ModbusPDU) -> ModbusPDU: @@ -133,7 +129,7 @@ def sync_execute(self, no_response_expected: bool, request: ModbusPDU) -> Modbus if no_response_expected: return ExceptionResponse(0xff) try: - return self.sync_get_response(request.dev_id) + return self.sync_get_response(request.dev_id, request.transaction_id) except asyncio.exceptions.TimeoutError: count_retries += 1 if self.count_until_disconnect < 0: @@ -191,8 +187,9 @@ async def execute(self, no_response_expected: bool, request: ModbusPDU) -> Modbu def pdu_send(self, pdu: ModbusPDU, addr: tuple | None = None) -> None: """Build byte stream and send.""" - self.request_dev_id = pdu.dev_id - self.request_transaction_id = pdu.transaction_id + if not self.is_server: + self.request_dev_id = pdu.dev_id + self.request_transaction_id = pdu.transaction_id packet = self.framer.buildFrame(self.trace_pdu(True, pdu)) if self.is_sync and self.comm_params.handle_local_echo: self.sent_buffer = packet @@ -214,16 +211,12 @@ def callback_disconnected(self, exc: Exception | None) -> None: def callback_data(self, data: bytes, addr: tuple | None = None) -> int: """Handle received data.""" self.last_pdu = self.last_addr = None - used_len, pdu = self.framer.processIncomingFrame(self.trace_packet(False, data)) + used_len, pdu = self.framer.handleFrame(self.trace_packet(False, data), self.request_dev_id, self.request_transaction_id) if pdu: self.last_pdu = self.trace_pdu(False, pdu) self.last_addr = addr if not self.is_server: - if pdu.dev_id != self.request_dev_id: - Log.warning(f"ERROR: expected id {self.request_dev_id} but got {pdu.dev_id}, IGNORING.") - elif pdu.transaction_id != self.request_transaction_id: - Log.warning(f"ERROR: expected transaction {self.request_transaction_id} but got {pdu.transaction_id}, IGNORING.") - elif self.response_future.done(): + if self.response_future.done(): Log.warning("ERROR: received pdu without a corresponding request, IGNORING") else: self.response_future.set_result(self.last_pdu) diff --git a/test/client/test_client.py b/test/client/test_client.py index 8c99a684c..84b942e8b 100755 --- a/test/client/test_client.py +++ b/test/client/test_client.py @@ -136,6 +136,12 @@ def fake_execute(_self, _no_response_expected, request): [0xC009, 0x21FB, 0x5444, 0x2D11], None, ), + ( + ModbusClientMixin.DATATYPE.BITS, + [True], + [1], # 0x00 0x01 + None, + ), ( ModbusClientMixin.DATATYPE.BITS, [True] + [False] * 15, @@ -197,6 +203,9 @@ def test_client_mixin_convert(self, datatype, word_order, registers, value, stri regs = ModbusClientMixin.convert_to_registers(value, datatype, **kwargs) assert regs == registers result = ModbusClientMixin.convert_from_registers(registers, datatype, **kwargs) + if datatype == ModbusClientMixin.DATATYPE.BITS: + if (missing := len(value) % 16): + value = value + [False] * (16 - missing) if datatype == ModbusClientMixin.DATATYPE.FLOAT32: result = round(result, 6) assert result == value diff --git a/test/client/test_client_faulty_response.py b/test/client/test_client_faulty_response.py index dec879baf..0e7e2c090 100644 --- a/test/client/test_client_faulty_response.py +++ b/test/client/test_client_faulty_response.py @@ -19,7 +19,7 @@ def fixture_framer(self): def test_ok_frame(self, framer): """Test ok frame.""" - used_len, pdu = framer.processIncomingFrame(self.good_frame) + used_len, pdu = framer.handleFrame(self.good_frame, 0, 0) assert pdu assert used_len == len(self.good_frame) @@ -27,7 +27,7 @@ def test_1917_frame(self): """Test invalid frame in issue 1917.""" recv = b"\x01\x86\x02\x00\x01" framer = FramerRTU(DecodePDU(False)) - used_len, pdu = framer.processIncomingFrame(recv) + used_len, pdu = framer.handleFrame(recv, 0, 0) assert not pdu assert not used_len @@ -35,7 +35,7 @@ def test_faulty_frame1(self, framer): """Test ok frame.""" faulty_frame = b"\x00\x04\x00\x00\x00\x05\x00\x03\x0a\x00\x04" with pytest.raises(ModbusIOException): - framer.processIncomingFrame(faulty_frame) - used_len, pdu = framer.processIncomingFrame(self.good_frame) + framer.handleFrame(faulty_frame, 0, 0) + used_len, pdu = framer.handleFrame(self.good_frame, 0, 0) assert pdu assert used_len == len(self.good_frame) diff --git a/test/client/test_client_sync.py b/test/client/test_client_sync.py index 0f88d29af..24a1526e7 100755 --- a/test/client/test_client_sync.py +++ b/test/client/test_client_sync.py @@ -12,7 +12,7 @@ ModbusTlsClient, ModbusUdpClient, ) -from pymodbus.exceptions import ConnectionException +from pymodbus.exceptions import ConnectionException, ModbusIOException from pymodbus.framer import ( FramerAscii, FramerRTU, @@ -82,8 +82,8 @@ def test_udp_client_recv_duplicate(self): client.socket.mock_prepare_receive(test_msg) reply_ok = client.read_input_registers(0x820, count=1, device_id=1) assert not reply_ok.isError() - reply_ok = client.read_input_registers(0x40, count=10, device_id=1) - assert not reply_ok.isError() + with pytest.raises(ModbusIOException): + client.read_input_registers(0x40, count=10, device_id=1) client.close() def test_udp_client_repr(self): diff --git a/test/framer/test_extras.py b/test/framer/test_extras.py index 3f0ddc704..8ce2e3842 100755 --- a/test/framer/test_extras.py +++ b/test/framer/test_extras.py @@ -41,10 +41,10 @@ def test_tcp_framer_transaction_half2(self): """Test a half completed tcp frame transaction.""" msg1 = b"\x00\x01\x12\x34\x00\x06\xff" msg2 = b"\x02\x01\x02\x00\x08" - used_len, pdu = self._tcp.processIncomingFrame(msg1) + used_len, pdu = self._tcp.handleFrame(msg1, 0, 0) assert not pdu assert not used_len - used_len, pdu = self._tcp.processIncomingFrame(msg1+msg2) + used_len, pdu = self._tcp.handleFrame(msg1+msg2, 0, 0) assert pdu assert used_len == len(msg1) + len(msg2) assert pdu.function_code.to_bytes(1,'big') + pdu.encode() == msg2 @@ -53,10 +53,10 @@ def test_tcp_framer_transaction_half3(self): """Test a half completed tcp frame transaction.""" msg1 = b"\x00\x01\x12\x34\x00\x06\xff\x02\x01\x02\x00" msg2 = b"\x08" - used_len, pdu = self._tcp.processIncomingFrame(msg1) + used_len, pdu = self._tcp.handleFrame(msg1, 0, 0) assert not pdu assert not used_len - used_len, pdu = self._tcp.processIncomingFrame(msg1+msg2) + used_len, pdu = self._tcp.handleFrame(msg1+msg2, 0, 0) assert pdu assert used_len == len(msg1) + len(msg2) assert pdu.function_code.to_bytes(1,'big') + pdu.encode() == msg1[7:] + msg2 @@ -65,10 +65,10 @@ def test_tcp_framer_transaction_short(self): """Test that we can get back on track after an invalid message.""" msg1 = b'' msg2 = b"\x00\x01\x12\x34\x00\x06\xff\x02\x01\x02\x00\x08" - used_len, pdu = self._tcp.processIncomingFrame(msg1) + used_len, pdu = self._tcp.handleFrame(msg1, 0, 0) assert not pdu assert not used_len - used_len, pdu = self._tcp.processIncomingFrame(msg1+msg2) + used_len, pdu = self._tcp.handleFrame(msg1+msg2, 0, 0) assert pdu assert used_len == len(msg1) + len(msg2) assert pdu.function_code.to_bytes(1,'big') + pdu.encode() == msg2[7:] @@ -76,23 +76,23 @@ def test_tcp_framer_transaction_short(self): def test_tls_incoming_packet(self): """Framer tls incoming packet.""" msg = b"\x00\x01\x12\x34\x00\x06\xff\x02\x01\x02\x00\x08" - _, pdu = self._tls.processIncomingFrame(msg) + _, pdu = self._tls.handleFrame(msg, 0, 0) assert pdu def test_rtu_process_incoming_packets(self): """Test rtu process incoming packets.""" msg = b"\x00\x01\x00\x00\x00\x01\xfc\x1b" - _, pdu = self._rtu.processIncomingFrame(msg) + _, pdu = self._rtu.handleFrame(msg, 0, 0) assert pdu def test_ascii_process_incoming_packets(self): """Test ascii process incoming packet.""" msg = b":F7031389000A60\r\n" - _, pdu = self._ascii.processIncomingFrame(msg) + _, pdu = self._ascii.handleFrame(msg, 0, 0) assert pdu def test_rtu_decode_exception(self): """Test that the RTU framer can decode errors.""" msg = b"\x00\x90\x02\x9c\x01" - _, pdu = self._rtu.processIncomingFrame(msg) + _, pdu = self._rtu.handleFrame(msg, 0, 0) assert pdu diff --git a/test/framer/test_framer.py b/test/framer/test_framer.py index ba002fa6e..b2bf29d78 100644 --- a/test/framer/test_framer.py +++ b/test/framer/test_framer.py @@ -388,20 +388,20 @@ def test_framer_decode(self, test_framer): assert not res_data @pytest.mark.parametrize(("is_server"), [False]) - async def test_processIncomingFrame_no(self, test_framer): - """Test processIncomingFrame.""" + async def test_handleFrame_no(self, test_framer): + """Test handleFrame.""" msg = b"\x00\x01\x00\x00\x00\x01\xfc\x1b" - with mock.patch.object(test_framer, "_processIncomingFrame") as mock_process: - mock_process.side_effect = [(5, None), (0, None)] - used_len, pdu = test_framer.processIncomingFrame(msg) + with mock.patch.object(test_framer, "decode") as mock_process: + mock_process.side_effect = [(5, 0, 0, None), (0, 0, 0, None)] + used_len, pdu = test_framer.handleFrame(msg, 0, 0) assert used_len == 5 assert not pdu @pytest.mark.parametrize(("is_server"), [True]) - async def test_processIncomingFrame1(self, test_framer): - """Test processIncomingFrame.""" + async def test_handleFrame1(self, test_framer): + """Test handleFrame.""" msg = b"\x00\x01\x00\x00\x00\x01\xfc\x1b" - _, pdu = test_framer.processIncomingFrame(msg) + _, pdu = test_framer.handleFrame(msg, 0, 0) assert pdu @pytest.mark.parametrize(("is_server"), [True]) @@ -411,9 +411,9 @@ async def test_processIncomingFrame1(self, test_framer): (FramerType.RTU, b"\x00\x01\x00\x00\x00\x01\xfc\x1b"), (FramerType.ASCII, b":F7031389000A60\r\n"), ]) - def test_processIncomingFrame2(self, test_framer, msg): + def test_handleFrame2(self, test_framer, msg): """Test a tcp frame transaction.""" - used_len, pdu = test_framer.processIncomingFrame(msg) + used_len, pdu = test_framer.handleFrame(msg, 0, 0) assert pdu assert used_len == len(msg) @@ -425,16 +425,16 @@ def test_processIncomingFrame2(self, test_framer, msg): (FramerType.RTU, b"\x00\x01\x00\x00\x00\x01\xfc\x1b", 0, 0), (FramerType.ASCII, b":F7031389000A60\r\n", 0xf7, 0), ]) - def test_processIncomingFrame_roundtrip(self, entry, test_framer, msg, dev_id, tid, half): + def test_handleFrame_roundtrip(self, entry, test_framer, msg, dev_id, tid, half): """Test a tcp frame transaction.""" if half and entry != FramerType.TLS: data_len = int(len(msg) / 2) - used_len, pdu = test_framer.processIncomingFrame(msg[:data_len]) + used_len, pdu = test_framer.handleFrame(msg[:data_len], 0, 0) assert not pdu assert not used_len - used_len, result = test_framer.processIncomingFrame(msg) + used_len, result = test_framer.handleFrame(msg, 0, 0) else: - used_len, result = test_framer.processIncomingFrame(msg) + used_len, result = test_framer.handleFrame(msg, 0, 0) assert used_len == len(msg) assert result assert result.dev_id == dev_id diff --git a/test/framer/test_multidrop.py b/test/framer/test_multidrop.py index 0bf11a995..49411ff2e 100644 --- a/test/framer/test_multidrop.py +++ b/test/framer/test_multidrop.py @@ -20,24 +20,24 @@ def fixture_framer(self): def test_ok_frame(self, framer): """Test ok frame.""" serial_event = self.good_frame - used_len, pdu = framer.processIncomingFrame(serial_event) + used_len, pdu = framer.handleFrame(serial_event, 0, 0) assert pdu assert used_len == len(serial_event) def test_ok_2frame(self, framer): """Test ok frame.""" serial_event = self.good_frame + self.good_frame - used_len, pdu = framer.processIncomingFrame(serial_event) + used_len, pdu = framer.handleFrame(serial_event, 0, 0) assert pdu assert used_len == len(self.good_frame) - used_len, pdu = framer.processIncomingFrame(serial_event[used_len:]) + used_len, pdu = framer.handleFrame(serial_event[used_len:], 0, 0) assert pdu assert used_len == len(self.good_frame) def test_bad_crc(self, framer): """Test bad crc.""" serial_event = b"\x02\x03\x00\x01\x00}\xd4\x19" # Manually mangled crc - _, pdu = framer.processIncomingFrame(serial_event) + _, pdu = framer.handleFrame(serial_event, 0, 0) assert not pdu def test_big_split_response_frame_from_other_id(self, framer): @@ -62,19 +62,19 @@ def test_big_split_response_frame_from_other_id(self, framer): data = b'' for serial_event in serial_events: data += serial_event - used_len, pdu = framer.processIncomingFrame(data) + used_len, pdu = framer.handleFrame(data, 0, 0) assert not pdu assert not used_len - used_len, pdu = framer.processIncomingFrame(data + final) + used_len, pdu = framer.handleFrame(data + final, 0, 0) assert pdu assert used_len == len(data + final) def test_split_frame(self, framer): """Test split frame.""" - used_len, pdu = framer.processIncomingFrame(self.good_frame[:5]) + used_len, pdu = framer.handleFrame(self.good_frame[:5], 0, 0) assert not pdu assert not used_len - used_len, pdu = framer.processIncomingFrame(self.good_frame) + used_len, pdu = framer.handleFrame(self.good_frame, 0, 0) assert pdu assert used_len == len(self.good_frame) @@ -82,7 +82,7 @@ def test_complete_frame_trailing_data_without_id(self, framer): """Test trailing data.""" garbage = b"\x05\x04\x03" # without id serial_event = garbage + self.good_frame - used_len, pdu = framer.processIncomingFrame(serial_event) + used_len, pdu = framer.handleFrame(serial_event, 0, 0) assert pdu assert used_len == len(serial_event) @@ -90,7 +90,7 @@ def test_complete_frame_trailing_data_with_id(self, framer): """Test trailing data.""" garbage = b"\x05\x04\x03\x02\x01\x00" # with id serial_event = garbage + self.good_frame - used_len, pdu = framer.processIncomingFrame(serial_event) + used_len, pdu = framer.handleFrame(serial_event, 0, 0) assert pdu assert used_len == len(serial_event) @@ -98,10 +98,10 @@ def test_split_frame_trailing_data_with_id(self, framer): """Test split frame.""" garbage = b"ABCDEF" serial_events = garbage + self.good_frame - used_len, pdu = framer.processIncomingFrame(serial_events[:11]) + used_len, pdu = framer.handleFrame(serial_events[:11], 0, 0) assert not pdu serial_events = serial_events[used_len:] - used_len, pdu = framer.processIncomingFrame(serial_events) + used_len, pdu = framer.handleFrame(serial_events, 0, 0) assert pdu assert used_len == len(serial_events) @@ -114,10 +114,10 @@ def test_split_frame_trailing_data_with_id(self, framer): def test_coincidental(self, garbage, framer): """Test conincidental.""" serial_events = garbage + self.good_frame - used_len, pdu = framer.processIncomingFrame(serial_events[:5]) + used_len, pdu = framer.handleFrame(serial_events[:5], 0, 0) assert not pdu serial_events = serial_events[used_len:] - used_len, pdu = framer.processIncomingFrame(serial_events) + used_len, pdu = framer.handleFrame(serial_events, 0, 0) assert pdu assert used_len == len(serial_events) @@ -129,7 +129,7 @@ def test_wrapped_frame(self, framer): # i.e. this probably represents a case where a command came for us, but we didn't get # to the serial buffer in time (some other co-routine or perhaps a block on the USB bus) # and the master moved on and queried another device - _, pdu = framer.processIncomingFrame(serial_event) + _, pdu = framer.handleFrame(serial_event, 0, 0) assert pdu def test_frame_with_trailing_data(self, framer): @@ -137,7 +137,7 @@ def test_frame_with_trailing_data(self, framer): garbage = b"\x05\x04\x03\x02\x01\x00" serial_event = self.good_frame + garbage # We should not respond in this case for identical reasons as test_wrapped_frame - _, pdu = framer.processIncomingFrame(serial_event) + _, pdu = framer.handleFrame(serial_event, 0, 0) assert pdu def test_wrong_class(self): @@ -150,19 +150,19 @@ def return_none(_data): framer = FramerAscii(DecodePDU(True)) framer.decoder.decode = return_none with pytest.raises(ModbusIOException): - framer.processIncomingFrame(b':1103007C00026E\r\n') + framer.handleFrame(b':1103007C00026E\r\n', 0, 0) def test_getFrameStart(self, framer): """Test getFrameStart.""" framer_ok = b"\x02\x03\x00\x01\x00\x7d\xd4\x18" - _, pdu = framer.processIncomingFrame(framer_ok) + _, pdu = framer.handleFrame(framer_ok, 0, 0) assert framer_ok[1:-2] == pdu.function_code.to_bytes(1,'big')+pdu.encode() framer_2ok = framer_ok + framer_ok - used_len, pdu = framer.processIncomingFrame(framer_2ok) + used_len, pdu = framer.handleFrame(framer_2ok, 0, 0) assert pdu framer_2ok = framer_2ok[used_len:] - used_len, pdu = framer.processIncomingFrame(framer_2ok) + used_len, pdu = framer.handleFrame(framer_2ok, 0, 0) assert pdu assert used_len == len(framer_2ok) @@ -174,12 +174,12 @@ def test_rtu_split_frame(self, framer): b'\xaa\xbb\xcc\xdd\xee\xff\x00\x11\x22\x33\x44\x55\x66\x77\x88\x99' b'\xaa\xbb\xcc\xdd\xee\xff\x00\x11\x22\x33\x44\x55\x66\x77\x88\x99' b'\xaa\xbb\xcc\xdd\xee\xff\xe7\x65') - used_len, pdu = framer.processIncomingFrame(msg1+msg2) + used_len, pdu = framer.handleFrame(msg1+msg2, 0, 0) assert pdu assert used_len == len(msg1 + msg2) - used_len, pdu = framer.processIncomingFrame(msg1) + used_len, pdu = framer.handleFrame(msg1, 0, 0) assert not pdu assert not used_len - used_len, pdu = framer.processIncomingFrame(msg1+msg2) + used_len, pdu = framer.handleFrame(msg1+msg2, 0, 0) assert pdu assert used_len == len(msg1 + msg2) diff --git a/test/not_updated/test_exceptions.py b/test/not_updated/test_exceptions.py index d80e0d71d..029a79220 100644 --- a/test/not_updated/test_exceptions.py +++ b/test/not_updated/test_exceptions.py @@ -26,3 +26,7 @@ def test_exceptions(self): for exc in self.exceptions: with pytest.raises(ModbusException, match="Modbus Error:"): raise exc + + def test_is_error(self): + """Test is_error().""" + assert self.exceptions[0].isError() diff --git a/test/not_updated/test_logging.py b/test/not_updated/test_logging.py index 3ce498133..1de923fbc 100644 --- a/test/not_updated/test_logging.py +++ b/test/not_updated/test_logging.py @@ -48,9 +48,17 @@ def test_apply_logging(self): pymodbus_apply_logging_config("debug", "pymodbus.log") pymodbus_apply_logging_config("info") Log.info("test") + pymodbus_apply_logging_config(logging.NOTSET) + Log.warning("test") pymodbus_apply_logging_config("warning") Log.warning("test") + pymodbus_apply_logging_config(logging.NOTSET) + Log.critical("test") pymodbus_apply_logging_config("critical") Log.critical("test") + pymodbus_apply_logging_config(logging.NOTSET) + Log.error("test") pymodbus_apply_logging_config("error") Log.error("test") + pymodbus_apply_logging_config(logging.NOTSET) + Log.critical("test") diff --git a/test/pdu/test_bit.py b/test/pdu/test_bit.py index 172571454..a51738e57 100644 --- a/test/pdu/test_bit.py +++ b/test/pdu/test_bit.py @@ -1,4 +1,7 @@ """Bit Message Test Fixture.""" +from unittest import mock + +import pytest import pymodbus.pdu.bit_message as bit_msg @@ -94,7 +97,6 @@ async def test_write_single_coil_update_datastore(self, mock_context): context = mock_context(False, default=True) request = bit_msg.WriteSingleCoilRequest(address=2, bits=[True]) result = await request.update_datastore(context) - # assert result.exception_code == ExceptionResponse.ILLEGAL_ADDRESS context.valid = True result = await request.update_datastore(context) @@ -122,6 +124,19 @@ async def test_write_multiple_coils_update_datastore(self, mock_context): result = await request.update_datastore(context) assert result.encode() == b"\x00\x02\x00\x04" + @pytest.mark.parametrize(("request_pdu"), + [ + bit_msg.WriteSingleCoilRequest(address=2, bits=[True]), + bit_msg.WriteMultipleCoilsRequest(address=2, bits=[]), + ] + ) + async def test_write_coil_exception(self, request_pdu, mock_context): + """Test write single coil.""" + context = mock_context(True, default=True) + context.async_setValues = mock.AsyncMock(return_value=1) + result = await request_pdu.update_datastore(context) + assert result.exception_code == 1 + def test_write_multiple_coils_response(self): """Test write multiple coils.""" response = bit_msg.WriteMultipleCoilsResponse() diff --git a/test/pdu/test_pdu.py b/test/pdu/test_pdu.py index 243fd9300..194cf1bd5 100644 --- a/test/pdu/test_pdu.py +++ b/test/pdu/test_pdu.py @@ -306,3 +306,7 @@ def test_bit_packing8(self, bytestream, bitlist): def test_bit_unpacking(self, bytestream, bitlist): """Test all string <=> bit packing functions.""" assert unpack_bitstring(bytestream) == bitlist + + def test_ExceptionResponse_str(self): + """Test string conversion of ExceptionResponse.""" + assert str(self.exception) != "" diff --git a/test/pdu/test_register_read_messages.py b/test/pdu/test_register_read_messages.py index f2d2955c2..b8064ab44 100644 --- a/test/pdu/test_register_read_messages.py +++ b/test/pdu/test_register_read_messages.py @@ -1,4 +1,6 @@ """Test register read messages.""" +from unittest import mock + import pytest from pymodbus.exceptions import ModbusIOException @@ -163,3 +165,17 @@ def test_serializing_to_string(self): assert str(request) for request in iter(self.response_read.keys()): assert str(request) + + @pytest.mark.parametrize(("request_pdu"), + [ + ReadWriteMultipleRegistersRequest( + read_address=1, read_count=5, write_address=1, write_registers=[5] + ), + ] + ) + async def test_register_read_exception(self, request_pdu, mock_context): + """Test write single coil.""" + context = mock_context(True, default=True) + context.async_setValues = mock.AsyncMock(return_value=1) + result = await request_pdu.update_datastore(context) + assert result.exception_code == 1 diff --git a/test/pdu/test_register_write_messages.py b/test/pdu/test_register_write_messages.py index 02877d22c..23b4317e1 100644 --- a/test/pdu/test_register_write_messages.py +++ b/test/pdu/test_register_write_messages.py @@ -1,4 +1,8 @@ """Test register write messages.""" +from unittest import mock + +import pytest + from pymodbus.pdu import ExceptionResponse from pymodbus.pdu.register_message import ( MaskWriteRegisterRequest, @@ -171,3 +175,18 @@ def test_mask_write_register_response_decode(self): assert handle.address == 0x0004 assert handle.and_mask == 0x00F2 assert handle.or_mask == 0x0025 + + + @pytest.mark.parametrize(("request_pdu"), + [ + WriteSingleRegisterRequest(address=0x00, registers=[5]), + WriteMultipleRegistersRequest(address=0x00, registers=[0x00] * 10), + MaskWriteRegisterRequest(0x0000, 0x01, 0x1010), + ] + ) + async def test_register_write_exception(self, request_pdu, mock_context): + """Test write single coil.""" + context = mock_context(True, default=True) + context.async_setValues = mock.AsyncMock(return_value=1) + result = await request_pdu.update_datastore(context) + assert result.exception_code == 1 diff --git a/test/server/test_server_asyncio.py b/test/server/test_server_asyncio.py index bbc83d454..860c059d4 100755 --- a/test/server/test_server_asyncio.py +++ b/test/server/test_server_asyncio.py @@ -56,14 +56,14 @@ def data_received(self, data): """Get Data received.""" _logger.debug("TEST Client data received") BasicClient.received_data = data - if BasicClient.done is not None: + if BasicClient.done is not None: # pragma: no cover BasicClient.done.set_result(True) def datagram_received(self, data, _addr): """Get Datagram received.""" _logger.debug("TEST Client datagram received") BasicClient.received_data = data - if BasicClient.done is not None: + if BasicClient.done is not None: # pragma: no cover BasicClient.done.set_result(True) self.transport.close() @@ -129,7 +129,7 @@ async def _setup_teardown(self): self.server = None if self.task is not None: await asyncio.sleep(0.1) - if not self.task.cancelled(): + if not self.task.cancelled(): # pragma: no cover self.task.cancel() with suppress(CancelledError): await self.task @@ -224,7 +224,7 @@ async def test_async_tcp_server_receive_data(self): BasicClient.data = b"\x01\x00\x00\x00\x00\x06\x01\x03\x00\x00\x00\x19" await self.start_server() with mock.patch( - "pymodbus.framer.FramerSocket.processIncomingFrame", + "pymodbus.framer.FramerSocket.handleFrame", new_callable=mock.Mock, ) as process: await self.connect_server() @@ -241,7 +241,7 @@ async def test_async_tcp_server_roundtrip(self): assert BasicClient.received_data, expected_response @pytest.mark.skip - async def test_async_server_file_descriptors(self): + 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. @@ -369,7 +369,7 @@ async def test_async_udp_server_exception(self): BasicClient.done = asyncio.Future() await self.start_server(do_udp=True) with mock.patch( - "pymodbus.framer.FramerSocket.processIncomingFrame", + "pymodbus.framer.FramerSocket.handleFrame", new_callable=lambda: mock.Mock(side_effect=Exception), ): # get the random server port pylint: disable=protected-access @@ -381,12 +381,12 @@ async def test_async_udp_server_exception(self): assert not BasicClient.done.done() @pytest.mark.skip - async def test_async_tcp_server_exception(self): + 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.processIncomingFrame", + "pymodbus.framer.FramerSocket.handleFrame", new_callable=lambda: mock.Mock(side_effect=Exception), ): await self.connect_server() diff --git a/test/transaction/test_transaction.py b/test/transaction/test_transaction.py index fb9997cd6..8a39ae071 100755 --- a/test/transaction/test_transaction.py +++ b/test/transaction/test_transaction.py @@ -121,19 +121,19 @@ async def test_transaction_data(self, use_clc, test, is_server): None, ) transact.is_server = is_server - transact.framer.processIncomingFrame = mock.Mock(return_value=(0, None)) + transact.framer.handleFrame = mock.Mock(return_value=(0, None)) transact.callback_data(packet) assert not transact.response_future.done() if test: transact.trace_packet = mock.Mock(return_value=packet) - transact.framer.processIncomingFrame.return_value = (1, pdu) + transact.framer.handleFrame.return_value = (1, pdu) transact.callback_data(packet) transact.trace_packet.assert_called_once_with(False, packet) else: transact.trace_packet = mock.Mock(return_value=packet) transact.trace_pdu = mock.Mock(return_value=pdu) - transact.framer.processIncomingFrame.return_value = (1, pdu) + transact.framer.handleFrame.return_value = (1, pdu) transact.callback_data(packet) transact.trace_packet.assert_called_with(False, packet) transact.trace_pdu.assert_called_once_with(False, pdu) @@ -153,9 +153,9 @@ async def test_transaction_data_2(self, use_clc, test): None, None, ) - transact.framer.processIncomingFrame = mock.Mock() + transact.framer.handleFrame = mock.Mock() transact.trace_packet = mock.Mock(return_value=packet) - transact.framer.processIncomingFrame.return_value = (1, pdu) + transact.framer.handleFrame.return_value = (1, pdu) if test: pdu.dev_id = 17 else: @@ -351,7 +351,7 @@ async def test_delayed_response(self, use_clc, framer, scenario): transact.callback_data(cb_response1, None) result = await resp assert result.bits == response1.bits - elif scenario == 1: # timeout + new request + double response + else: # if scenario == 1: # timeout + new request + double response resp = asyncio.create_task(transact.execute(False, request1)) await asyncio.sleep(0.25) with pytest.raises(ModbusIOException): @@ -591,17 +591,18 @@ def test_transaction_sync_id0(self, use_clc): ) transact.sync_client.connect = mock.Mock(return_value=True) transact.sync_client.send = mock.Mock() - request = ReadCoilsRequest(address=117, count=5, dev_id=0) - response = ReadCoilsResponse(bits=[True, False, True, True, False, False, False, False], dev_id=1) transact.retries = 0 transact.transport = 1 + response = ReadCoilsResponse(bits=[True, False, True, True, False, False, False, False], dev_id=2) resp_bytes = transact.framer.buildFrame(response) - transact.sync_client.recv = mock.Mock(return_value=resp_bytes) + transact.sync_client.recv = mock.Mock() + transact.sync_client.recv.side_effect = [resp_bytes, None] transact.sync_client.send = mock.Mock() transact.comm_params.timeout_connect = 0.2 + request = ReadCoilsRequest(address=117, count=5, dev_id=1) with pytest.raises(ModbusIOException): transact.sync_execute(False, request) - response = ReadCoilsResponse(bits=[True, False, True, True, False, False, False, False], dev_id=0) + response = ReadCoilsResponse(bits=[True, False, True, True, False, False, False, False], dev_id=1) resp_bytes = transact.framer.buildFrame(response) transact.sync_client.recv = mock.Mock(return_value=resp_bytes) resp = transact.sync_execute(False, request) @@ -632,13 +633,13 @@ def test_transaction_sync_get_response(self, use_clc): response = transact.framer.buildFrame(ReadCoilsResponse(bits=[True*8], dev_id=1)) transact.sent_buffer = request client.recv.side_effect = [request, response] - pdu = transact.sync_get_response(1) + pdu = transact.sync_get_response(1, 0) assert isinstance(pdu, ReadCoilsResponse) transact.sent_buffer = request client.recv.side_effect = [request[:3], request[3:], response] - pdu = transact.sync_get_response(1) + pdu = transact.sync_get_response(1, 0) assert isinstance(pdu, ReadCoilsResponse) transact.sent_buffer = request client.recv.side_effect = [response] - pdu = transact.sync_get_response(1) + pdu = transact.sync_get_response(1, 0) assert isinstance(pdu, ReadCoilsResponse)