Simplify response rejection.

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)

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

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)

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.

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."""
 

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

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)

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:

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.

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])
         )

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)

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

test/client/test_client_faulty_response.py

@@ -19,23 +19,23 @@ 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)
 
     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
 
     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)

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):