Simplify response rejection. (#2657)

Author: janiversen

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