Refactor PDU diag. (#2421)

Author: janiversen

doc/source/library/pymodbus.rst

@@ -76,12 +76,7 @@ PDU classes
     :show-inheritance:
     :noindex:
 
-.. automodule:: pymodbus.pdu.register_read_message
-    :members:
-    :undoc-members:
-    :show-inheritance:
-
-.. automodule:: pymodbus.pdu.register_write_message
+.. automodule:: pymodbus.pdu.register_message
     :members:
     :undoc-members:
     :show-inheritance:

examples/client_custom_msg.py

@@ -38,8 +38,7 @@ class CustomModbusPDU(ModbusPDU):
 
     def __init__(self, values=None, slave=1, transaction=0):
         """Initialize."""
-        super().__init__()
-        super().setBaseData(slave, transaction)
+        super().__init__(slave_id=slave, transaction_id=transaction)
         self.values = values or []
 
     def encode(self):
@@ -71,8 +70,7 @@ class CustomRequest(ModbusPDU):
 
     def __init__(self, address=None, slave=1, transaction=0):
         """Initialize."""
-        super().__init__()
-        super().setBaseData(slave, transaction)
+        super().__init__(slave_id=slave, transaction_id=transaction)
         self.address = address
         self.count = 16
 
@@ -107,8 +105,7 @@ def __init__(self, address, slave=1, transaction=0):
 
         :param address: The address to start reading from
         """
-        super().__init__()
-        self.setData(address, 16, slave, transaction)
+        super().__init__(address=address, count=16, slave_id=slave, transaction_id=transaction)
 
 
 # --------------------------------------------------------------------------- #

pymodbus/client/mixin.py

@@ -16,18 +16,6 @@ class ReadCoilsRequest(ModbusPDU):
     rtu_frame_size = 8
     function_code = 1
 
-    def __init__(self) -> None:
-        """Initialize the read request."""
-        super().__init__()
-        self.address: int = 0
-        self.count: int = 0
-
-    def setData(self, address: int, count: int, slave_id: int, transaction_id: int) -> None:
-        """Set data."""
-        super().setBaseData(slave_id, transaction_id)
-        self.address = address
-        self.count = count
-
     def encode(self) -> bytes:
         """Encode a request pdu."""
         return struct.pack(">HH", self.address, self.count)
@@ -54,15 +42,10 @@ async def update_datastore(self, context: ModbusSlaveContext) -> ModbusPDU:
             self.function_code, self.address, self.count
         ))
         response = (ReadCoilsResponse if self.function_code == 1 else ReadDiscreteInputsResponse)()
-        response.setData(values, self.slave_id, self.transaction_id)
+        response.bits = values
         return response
 
 
-    def __str__(self) -> str:
-        """Return a string representation of the instance."""
-        return f"{self.__class__.__name__}({self.address},{self.count})"
-
-
 class ReadDiscreteInputsRequest(ReadCoilsRequest):
     """ReadDiscreteInputsRequest."""
 
@@ -75,11 +58,6 @@ class ReadCoilsResponse(ModbusPDU):
     function_code = 1
     rtu_byte_count_pos = 2
 
-    def setData(self, values: list[bool], slave_id: int, transaction_id: int) -> None:
-        """Set data."""
-        super().setBaseData(slave_id, transaction_id)
-        self.bits = values
-
     def encode(self) -> bytes:
         """Encode response pdu."""
         result = pack_bitstring(self.bits)
@@ -90,10 +68,6 @@ def decode(self, data):
         """Decode response pdu."""
         self.bits = unpack_bitstring(data[1:])
 
-    def __str__(self):
-        """Return a string representation of the instance."""
-        return f"{self.__class__.__name__}({len(self.bits)})"
-
 
 class ReadDiscreteInputsResponse(ReadCoilsResponse):
     """ReadDiscreteInputsResponse."""
@@ -107,31 +81,15 @@ class WriteSingleCoilResponse(ModbusPDU):
     function_code = 5
     rtu_frame_size = 8
 
-    def __init__(self) -> None:
-        """Instancitate object."""
-        super().__init__()
-        self.address: int = 0
-        self.value: bool = False
-
-    def setData(self, address: int, value: bool, slave_id: int, transaction_id: int) -> None:
-        """Set data."""
-        super().setBaseData(slave_id, transaction_id)
-        self.address = address
-        self.value = value
-
     def encode(self) -> bytes:
         """Encode write coil request."""
-        val = ModbusStatus.ON if self.value else ModbusStatus.OFF
+        val = ModbusStatus.ON if self.bits[0] else ModbusStatus.OFF
         return struct.pack(">HH", self.address, val)
 
     def decode(self, data: bytes) -> None:
         """Decode a write coil request."""
         self.address, value = struct.unpack(">HH", data)
-        self.value = value == ModbusStatus.ON
-
-    def __str__(self) -> str:
-        """Return a string representation of the instance."""
-        return f"{self.__class__.__name__}({self.address}) => {self.value}"
+        self.bits = [value == ModbusStatus.ON]
 
 
 class WriteSingleCoilRequest(WriteSingleCoilResponse):
@@ -142,11 +100,9 @@ async def update_datastore(self, context: ModbusSlaveContext) -> ModbusPDU:
         if not context.validate(self.function_code, self.address, 1):
             return self.doException(merror.IllegalAddress)
 
-        await context.async_setValues(self.function_code, self.address, [self.value])
+        await context.async_setValues(self.function_code, self.address, self.bits)
         values = cast(list[bool], await context.async_getValues(self.function_code, self.address, 1))
-        pdu = WriteSingleCoilResponse()
-        pdu.setData(self.address, values[0], self.slave_id, self.transaction_id)
-        return pdu
+        return WriteSingleCoilResponse(address=self.address, bits=values, slave_id=self.slave_id, transaction_id=self.transaction_id)
 
     def get_response_pdu_size(self) -> int:
         """Get response pdu size.
@@ -162,50 +118,32 @@ class WriteMultipleCoilsRequest(ModbusPDU):
     function_code = 15
     rtu_byte_count_pos = 6
 
-    def __init__(self) -> None:
-        """Initialize a new instance."""
-        super().__init__()
-        self.address: int = 0
-        self.values: list[bool] = []
-
-    def setData(self, address: int, values: list[bool], slave_id: int, transaction_id: int) -> None:
-        """Set data."""
-        super().setBaseData(slave_id, transaction_id)
-        self.address = address
-        self.values = values
-
     def encode(self) -> bytes:
         """Encode write coils request."""
-        count = len(self.values)
+        count = len(self.bits)
         byte_count = (count + 7) // 8
         packet = struct.pack(">HHB", self.address, count, byte_count)
-        packet += pack_bitstring(self.values)
+        packet += pack_bitstring(self.bits)
         return packet
 
     def decode(self, data: bytes) -> None:
         """Decode a write coils request."""
         self.address, count, _ = struct.unpack(">HHB", data[0:5])
         values = unpack_bitstring(data[5:])
-        self.values = values[:count]
+        self.bits = values[:count]
 
     async def update_datastore(self, context: ModbusSlaveContext) -> ModbusPDU:
         """Run a request against a datastore."""
-        count = len(self.values)
+        count = len(self.bits)
         if not 1 <= count <= 0x07B0:
             return self.doException(merror.IllegalValue)
         if not context.validate(self.function_code, self.address, count):
             return self.doException(merror.IllegalAddress)
 
         await context.async_setValues(
-            self.function_code, self.address, self.values
+            self.function_code, self.address, self.bits
         )
-        pdu = WriteMultipleCoilsResponse()
-        pdu.setData(self.address, count, self.slave_id, self.transaction_id)
-        return pdu
-
-    def __str__(self) -> str:
-        """Return a string representation of the instance."""
-        return f"{self.__class__.__name__}({self.address}) => {len(self.values)}"
+        return WriteMultipleCoilsResponse(address=self.address, count=count, slave_id=self.slave_id, transaction_id=self.transaction_id)
 
     def get_response_pdu_size(self) -> int:
         """Get response pdu size.
@@ -222,26 +160,10 @@ class WriteMultipleCoilsResponse(ModbusPDU):
     function_code = 15
     rtu_frame_size = 8
 
-    def __init__(self) -> None:
-        """Initialize a new instance."""
-        super().__init__()
-        self.address: int = 0
-        self.count: int = 0
-
-    def setData(self, address: int, count: int, slave_id: int, transaction_id: int) -> None:
-        """Set data."""
-        super().setBaseData(slave_id, transaction_id)
-        self.address = address
-        self.count = count
-
     def encode(self) -> bytes:
         """Encode write coils response."""
         return struct.pack(">HH", self.address, self.count)
 
     def decode(self, data: bytes) -> None:
         """Decode a write coils response."""
         self.address, self.count = struct.unpack(">HH", data)
-
-    def __str__(self) -> str:
-        """Return a string representation of the instance."""
-        return f"{self.__class__.__name__}({self.address}, {self.count})"

pymodbus/pdu/bit_message.py

@@ -7,8 +7,7 @@
 import pymodbus.pdu.mei_message as mei_msg
 import pymodbus.pdu.other_message as o_msg
 import pymodbus.pdu.pdu as base
-import pymodbus.pdu.register_read_message as reg_r_msg
-import pymodbus.pdu.register_write_message as reg_w_msg
+import pymodbus.pdu.register_message as reg_msg
 from pymodbus.exceptions import MessageRegisterException, ModbusException
 from pymodbus.logging import Log
 
@@ -17,23 +16,23 @@ class DecodePDU:
     """Decode pdu requests/responses (server/client)."""
 
     _pdu_class_table: set[tuple[type[base.ModbusPDU], type[base.ModbusPDU]]] = {
-        (reg_r_msg.ReadHoldingRegistersRequest, reg_r_msg.ReadHoldingRegistersResponse),
+        (reg_msg.ReadHoldingRegistersRequest, reg_msg.ReadHoldingRegistersResponse),
         (bit_msg.ReadDiscreteInputsRequest, bit_msg.ReadDiscreteInputsResponse),
-        (reg_r_msg.ReadInputRegistersRequest, reg_r_msg.ReadInputRegistersResponse),
+        (reg_msg.ReadInputRegistersRequest, reg_msg.ReadInputRegistersResponse),
         (bit_msg.ReadCoilsRequest, bit_msg.ReadCoilsResponse),
         (bit_msg.WriteMultipleCoilsRequest, bit_msg.WriteMultipleCoilsResponse),
-        (reg_w_msg.WriteMultipleRegistersRequest, reg_w_msg.WriteMultipleRegistersResponse),
-        (reg_w_msg.WriteSingleRegisterRequest, reg_w_msg.WriteSingleRegisterResponse),
+        (reg_msg.WriteMultipleRegistersRequest, reg_msg.WriteMultipleRegistersResponse),
+        (reg_msg.WriteSingleRegisterRequest, reg_msg.WriteSingleRegisterResponse),
         (bit_msg.WriteSingleCoilRequest, bit_msg.WriteSingleCoilResponse),
-        (reg_r_msg.ReadWriteMultipleRegistersRequest, reg_r_msg.ReadWriteMultipleRegistersResponse),
+        (reg_msg.ReadWriteMultipleRegistersRequest, reg_msg.ReadWriteMultipleRegistersResponse),
         (diag_msg.DiagnosticStatusRequest, diag_msg.DiagnosticStatusResponse),
         (o_msg.ReadExceptionStatusRequest, o_msg.ReadExceptionStatusResponse),
         (o_msg.GetCommEventCounterRequest, o_msg.GetCommEventCounterResponse),
         (o_msg.GetCommEventLogRequest, o_msg.GetCommEventLogResponse),
         (o_msg.ReportSlaveIdRequest, o_msg.ReportSlaveIdResponse),
         (file_msg.ReadFileRecordRequest, file_msg.ReadFileRecordResponse),
         (file_msg.WriteFileRecordRequest, file_msg.WriteFileRecordResponse),
-        (reg_w_msg.MaskWriteRegisterRequest, reg_w_msg.MaskWriteRegisterResponse),
+        (reg_msg.MaskWriteRegisterRequest, reg_msg.MaskWriteRegisterResponse),
         (file_msg.ReadFifoQueueRequest, file_msg.ReadFifoQueueResponse),
         (mei_msg.ReadDeviceInformationRequest, mei_msg.ReadDeviceInformationResponse),
     }