Remove _header from framers. (#2305)

Author: janiversen

examples/message_parser.py

@@ -80,10 +80,7 @@ def decode(self, message):
             print(f"{decoder.decoder.__class__.__name__}")
             print("-" * 80)
             try:
-                slave = decoder._header.get(  # pylint: disable=protected-access
-                    "uid", 0x00
-                )
-                decoder.processIncomingPacket(message, self.report, slave)
+                decoder.processIncomingPacket(message, self.report, 0)
             except Exception:  # pylint: disable=broad-except
                 self.check_errors(decoder, message)
 

pymodbus/framer/old_framer_ascii.py

@@ -57,7 +57,7 @@ def frameProcessIncomingPacket(self, single, callback, slave, tid=None):
                     return
                 self._buffer = self._buffer[used_len :]
                 continue
-            self._header["uid"] = dev_id
+            self.dev_id = dev_id
             if not self._validate_slave_id(slave, single):
                 Log.error("Not a valid slave id - {}, ignoring!!", dev_id)
                 self.resetFrame()
@@ -67,5 +67,5 @@ def frameProcessIncomingPacket(self, single, callback, slave, tid=None):
                 raise ModbusIOException("Unable to decode response")
             self.populateResult(result)
             self._buffer = self._buffer[used_len :]
-            self._header = {"uid": 0x00}
+            self.dev_id = 0
             callback(result)  # defer this

pymodbus/framer/old_framer_base.py

@@ -3,7 +3,7 @@
 from __future__ import annotations
 
 import time
-from typing import TYPE_CHECKING, Any
+from typing import TYPE_CHECKING
 
 from pymodbus.factory import ClientDecoder, ServerDecoder
 from pymodbus.framer.base import FramerBase
@@ -41,20 +41,10 @@ def __init__(
         """
         self.decoder = decoder
         self.client = client
-        self._header: dict[str, Any]
-        self._reset_header()
         self._buffer = b""
         self.message_handler: FramerBase
-
-    def _reset_header(self) -> None:
-        self._header = {
-            "lrc": "0000",
-            "len": 0,
-            "uid": 0x00,
-            "tid": 0,
-            "pid": 0,
-            "crc": b"\x00\x00",
-        }
+        self.tid = 0
+        self.dev_id = 0
 
     def _validate_slave_id(self, slaves: list, single: bool) -> bool:
         """Validate if the received data is valid for the client.
@@ -69,7 +59,7 @@ def _validate_slave_id(self, slaves: list, single: bool) -> bool:
             # Handle Modbus TCP slave identifier (0x00 0r 0xFF)
             # in asynchronous requests
             return True
-        return self._header["uid"] in slaves
+        return self.dev_id in slaves
 
     def sendPacket(self, message: bytes):
         """Send packets on the bus.
@@ -104,14 +94,8 @@ def resetFrame(self):
             "Resetting frame - Current Frame in buffer - {}", self._buffer, ":hex"
         )
         self._buffer = b""
-        self._header = {
-            "lrc": "0000",
-            "crc": b"\x00\x00",
-            "len": 0,
-            "uid": 0x00,
-            "pid": 0,
-            "tid": 0,
-        }
+        self.dev_id = 0
+        self.tid = 0
 
     def populateResult(self, result):
         """Populate the modbus result header.
@@ -121,9 +105,9 @@ def populateResult(self, result):
 
         :param result: The response packet
         """
-        result.slave_id = self._header.get("uid", 0)
-        result.transaction_id = self._header.get("tid", 0)
-        result.protocol_id = self._header.get("pid", 0)
+        result.slave_id = self.dev_id
+        result.transaction_id = self.tid
+        result.protocol_id = 0
 
     def processIncomingPacket(self, data: bytes, callback, slave, single=False, tid=None):
         """Process new packet pattern.

pymodbus/framer/old_framer_rtu.py

@@ -60,6 +60,7 @@ def __init__(self, decoder, client=None):
         self._hsize = 0x01
         self.function_codes = decoder.lookup.keys() if decoder else {}
         self.message_handler = FramerRTU()
+        self.msg_len = 0
 
     def decode_data(self, data):
         """Decode data."""
@@ -75,20 +76,17 @@ def frameProcessIncomingPacket(self, _single, callback, slave, tid=None):  # noq
 
         def is_frame_ready(self):
             """Check if we should continue decode logic."""
-            size = self._header.get("len", 0)
+            size = self.msg_len
             if not size and len(self._buffer) > self._hsize:
                 try:
-                    self._header["uid"] = int(self._buffer[0])
-                    self._header["tid"] = int(self._buffer[0])
-                    self._header["tid"] = 0  # fix for now
+                    self.dev_id = int(self._buffer[0])
                     func_code = int(self._buffer[1])
                     pdu_class = self.decoder.lookupPduClass(func_code)
                     size = pdu_class.calculateRtuFrameSize(self._buffer)
-                    self._header["len"] = size
+                    self.msg_len = size
 
                     if len(self._buffer) < size:
                         raise IndexError
-                    self._header["crc"] = self._buffer[size - 2 : size]
                 except IndexError:
                     return False
             return len(self._buffer) >= size if size > 0 else False
@@ -116,20 +114,17 @@ def get_frame_start(self, slaves, broadcast, skip_cur_frame):
         def check_frame(self):
             """Check if the next frame is available."""
             try:
-                self._header["uid"] = int(self._buffer[0])
-                self._header["tid"] = int(self._buffer[0])
-                self._header["tid"] = 0  # fix for now
+                self.dev_id = int(self._buffer[0])
                 func_code = int(self._buffer[1])
                 pdu_class = self.decoder.lookupPduClass(func_code)
                 size = pdu_class.calculateRtuFrameSize(self._buffer)
-                self._header["len"] = size
+                self.msg_len = size
 
                 if len(self._buffer) < size:
                     raise IndexError
-                self._header["crc"] = self._buffer[size - 2 : size]
-                frame_size = self._header["len"]
+                frame_size = self.msg_len
                 data = self._buffer[: frame_size - 2]
-                crc = self._header["crc"]
+                crc = self._buffer[size - 2 : size]
                 crc_val = (int(crc[0]) << 8) + int(crc[1])
                 return FramerRTU.check_CRC(data, crc_val)
             except (IndexError, KeyError, struct.error):
@@ -138,7 +133,8 @@ def check_frame(self):
         broadcast = not slave[0]
         skip_cur_frame = False
         while get_frame_start(self, slave, broadcast, skip_cur_frame):
-            self._header = {"uid": 0x00, "len": 0, "crc": b"\x00\x00"}
+            self.dev_id = 0
+            self.msg_len = 0
             if not is_frame_ready(self):
                 Log.debug("Frame - not ready")
                 break
@@ -150,7 +146,7 @@ def check_frame(self):
                 skip_cur_frame = True
                 continue
             start = self._hsize
-            end = self._header["len"] - 2
+            end = self.msg_len - 2
             buffer = self._buffer[start:end]
             if end > 0:
                 Log.debug("Getting Frame - {}", buffer, ":hex")
@@ -159,9 +155,9 @@ def check_frame(self):
                 data = b""
             if (result := self.decoder.decode(data)) is None:
                 raise ModbusIOException("Unable to decode request")
-            result.slave_id = self._header["uid"]
+            result.slave_id = self.dev_id
             result.transaction_id = 0
-            self._buffer = self._buffer[self._header["len"] :]
+            self._buffer = self._buffer[self.msg_len :]
             Log.debug("Frame advanced, resetting header!!")
             callback(result)  # defer or push to a thread?
 

pymodbus/framer/old_framer_socket.py

@@ -47,12 +47,10 @@ def __init__(self, decoder, client=None):
     def decode_data(self, data):
         """Decode data."""
         if len(data) > self._hsize:
-            tid, pid, length, uid, fcode = struct.unpack(
+            _tid, _pid, length, uid, fcode = struct.unpack(
                 SOCKET_FRAME_HEADER, data[0 : self._hsize + 1]
             )
             return {
-                "tid": tid,
-                "pid": pid,
                 "length": length,
                 "slave": uid,
                 "fcode": fcode,
@@ -77,9 +75,8 @@ def frameProcessIncomingPacket(self, single, callback, slave, tid=None):
             used_len, use_tid, dev_id, data = self.message_handler.decode(self._buffer)
             if not data:
                 return
-            self._header["uid"] = dev_id
-            self._header["tid"] = use_tid
-            self._header["pid"] = 0
+            self.dev_id = dev_id
+            self.tid = use_tid
             if not self._validate_slave_id(slave, single):
                 Log.debug("Not a valid slave id - {}, ignoring!!", dev_id)
                 self.resetFrame()
@@ -89,7 +86,6 @@ def frameProcessIncomingPacket(self, single, callback, slave, tid=None):
                 raise ModbusIOException("Unable to decode request")
             self.populateResult(result)
             self._buffer: bytes = self._buffer[used_len:]
-            self._reset_header()
             if tid and tid != result.transaction_id:
                 self.resetFrame()
             else:

pymodbus/framer/old_framer_tls.py

@@ -56,14 +56,12 @@ def frameProcessIncomingPacket(self, _single, callback, _slave, tid=None):
             used_len, use_tid, dev_id, data = self.message_handler.decode(self._buffer)
             if not data:
                 return
-            self._header["uid"] = dev_id
-            self._header["tid"] = use_tid
-            self._header["pid"] = 0
+            self.dev_id = dev_id
+            self.tid = use_tid
 
             if (result := self.decoder.decode(data)) is None:
                 self.resetFrame()
                 raise ModbusIOException("Unable to decode request")
             self.populateResult(result)
             self._buffer: bytes = self._buffer[used_len:]
-            self._reset_header()
             callback(result)  # defer or push to a thread?

test/framers/test_framer.py

@@ -74,17 +74,17 @@ async def test_framer_decode(self, dummy_framer,  data, res_id, res_tid, res_len
         assert res_data == t_data
 
     @pytest.mark.parametrize(
-        ("data", "dev_id", "tid", "res_data"), [
+        ("data", "dev_id", "tr_id", "res_data"), [
         (b'\x01\x02', 5, 6, b'\x05\x06\x01\x02'),
         (b'\x00\x01\x02\x03\x04\x05\x06\x07\x08\x09', 17, 25, b'\x11\x19\x00\x01\x02\x03\x04\x05\x06\x07\x08\x09'),
     ])
-    async def test_framer_encode(self, dummy_framer, data, dev_id, tid, res_data):
+    async def test_framer_encode(self, dummy_framer, data, dev_id, tr_id, res_data):
         """Test decode method in all types."""
-        t_data = dummy_framer.handle.encode(data, dev_id, tid)
+        t_data = dummy_framer.handle.encode(data, dev_id, tr_id)
         assert res_data == t_data
 
     @pytest.mark.parametrize(
-        ("func", "lrc", "expect"),
+        ("func", "test_compare", "expect"),
         [(FramerAscii.check_LRC, 0x1c, True),
          (FramerAscii.check_LRC, 0x0c, False),
          (FramerAscii.compute_LRC, None, 0x1c),
@@ -93,10 +93,10 @@ async def test_framer_encode(self, dummy_framer, data, dev_id, tid, res_data):
          (FramerRTU.compute_CRC, None, 0xE2DB),
         ]
     )
-    def test_LRC_CRC(self, func, lrc, expect):
+    def test_LRC_CRC(self, func, test_compare, expect):
         """Test check_LRC."""
         data = b'\x12\x34\x23\x45\x34\x56\x45\x67'
-        assert expect == func(data, lrc) if lrc else func(data)
+        assert expect == func(data, test_compare) if test_compare else func(data)
 
     def test_roundtrip_LRC(self):
         """Test combined compute/check LRC."""
@@ -226,23 +226,23 @@ class TestFramerType:
         ]
     )
     @pytest.mark.parametrize(
-        ("inx3", "tid"),
+        ("inx3", "tr_id"),
         [
             (0, 0),
             (9, 3077),
         ]
     )
-    def test_encode_type(self, frame, frame_expected, data, dev_id, tid, inx1, inx2, inx3):
+    def test_encode_type(self, frame, frame_expected, data, dev_id, tr_id, inx1, inx2, inx3):
         """Test encode method."""
-        if frame == FramerTLS and dev_id + tid:
+        if frame == FramerTLS and dev_id + tr_id:
             return
         frame_obj = frame()
         expected = frame_expected[inx1 + inx2 + inx3]
-        encoded_data = frame_obj.encode(data, dev_id, tid)
+        encoded_data = frame_obj.encode(data, dev_id, tr_id)
         assert encoded_data == expected
 
     @pytest.mark.parametrize(
-        ("entry", "is_server", "data", "dev_id", "tid", "expected"),
+        ("entry", "is_server", "data", "dev_id", "tr_id", "expected"),
         [
             (FramerType.ASCII, True, b':0003007C00027F\r\n', 0, 0, b"\x03\x00\x7c\x00\x02",),  # Request
             (FramerType.ASCII, False, b':000304008D008EDE\r\n', 0, 0, b"\x03\x04\x00\x8d\x00\x8e",),  # Response
@@ -293,7 +293,7 @@ def test_encode_type(self, frame, frame_expected, data, dev_id, tid, inx1, inx2,
             "single",
         ]
     )
-    async def test_decode_type(self, entry, dummy_framer, data, dev_id, tid, expected, split):
+    async def test_decode_type(self, entry, dummy_framer, data, dev_id, tr_id, expected, split):
         """Test encode method."""
         if entry == FramerType.TLS and split != "no":
             return
@@ -314,7 +314,7 @@ async def test_decode_type(self, entry, dummy_framer, data, dev_id, tid, expecte
                 dummy_framer.callback_request_response.assert_not_called()
             used_len = dummy_framer.callback_data(data)
         assert used_len == len(data)
-        dummy_framer.callback_request_response.assert_called_with(expected, dev_id, tid)
+        dummy_framer.callback_request_response.assert_called_with(expected, dev_id, tr_id)
 
     @pytest.mark.parametrize(
         ("entry", "data", "exp"),