Simplify response rejection.

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/framer/base.py

@@ -63,40 +63,24 @@ 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 frame_data
+                    or (exp_devid and dev_id != exp_devid)
+                    or (exp_tid and tid != exp_tid)
+            ):
+                if data_len:
+                    continue
                 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 (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/transaction/transaction.py

@@ -108,7 +108,7 @@ 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), 0, 0)
             databuffer = databuffer[used_len:]
             if pdu:
                 if pdu.dev_id != dev_id:
@@ -214,7 +214,7 @@ 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), 0, 0)
         if pdu:
             self.last_pdu = self.trace_pdu(False, pdu)
             self.last_addr = addr

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/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,34 +65,34 @@ 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:]
 
     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

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

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,46 +62,46 @@ 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)
 
     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)
 
     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)
 
     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,15 +129,15 @@ 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):
         """Test trailing data."""
         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)