From 33c77b752a0195979460d471912191d523ef6991 Mon Sep 17 00:00:00 2001 From: jan iversen Date: Wed, 23 Oct 2024 12:18:59 +0200 Subject: [PATCH 1/3] Correct minor framer/pdu errors. --- examples/client_async.py | 15 +-- examples/client_async_calls.py | 84 +++++++++++----- examples/client_calls.py | 86 +++++++++++------ pymodbus/client/mixin.py | 9 +- pymodbus/framer/base.py | 1 - pymodbus/pdu/__init__.py | 2 + pymodbus/pdu/decoders.py | 2 +- pymodbus/pdu/file_message.py | 95 +++++++++---------- pymodbus/transaction.py | 27 +----- test/framer/test_framer.py | 1 - test/pdu/test_file_message.py | 21 ++-- test/sub_examples/test_client_server_async.py | 11 --- 12 files changed, 183 insertions(+), 171 deletions(-) diff --git a/examples/client_async.py b/examples/client_async.py index c14952dbf..d3301e58b 100755 --- a/examples/client_async.py +++ b/examples/client_async.py @@ -40,7 +40,6 @@ sys.exit(-1) import pymodbus.client as modbusClient -from pymodbus import ModbusException _logger = logging.getLogger(__file__) @@ -124,15 +123,11 @@ async def run_async_client(client, modbus_calls=None): async def run_a_few_calls(client): """Test connection works.""" - try: - rr = await client.read_coils(32, 1, slave=1) - assert len(rr.bits) == 8 - rr = await client.read_holding_registers(4, 2, slave=1) - assert rr.registers[0] == 17 - assert rr.registers[1] == 17 - except ModbusException: - pass - + rr = await client.read_coils(32, 1, slave=1) + assert len(rr.bits) == 8 + rr = await client.read_holding_registers(4, 2, slave=1) + assert rr.registers[0] == 17 + assert rr.registers[1] == 17 async def main(cmdline=None): """Combine setup and run.""" diff --git a/examples/client_async_calls.py b/examples/client_async_calls.py index 3f58c5673..465d925e0 100755 --- a/examples/client_async_calls.py +++ b/examples/client_async_calls.py @@ -33,6 +33,9 @@ import logging import sys +from pymodbus import ModbusException +from pymodbus.pdu import FileRecord + try: import client_async @@ -57,14 +60,14 @@ async def async_template_call(client): """Show complete modbus call, async version.""" try: rr = await client.read_coils(1, 1, slave=SLAVE) - except client_async.ModbusException as exc: + except ModbusException as exc: txt = f"ERROR: exception in pymodbus {exc}" _logger.error(txt) raise exc if rr.isError(): txt = "ERROR: pymodbus returned an error!" _logger.error(txt) - raise client_async.ModbusException(txt) + raise ModbusException(txt) # Validate data txt = f"### Template coils response: {rr.bits!s}" @@ -163,6 +166,24 @@ async def async_handle_input_registers(client): assert len(rr.registers) == 8 +async def async_handle_file_records(client): + """Read/write file records.""" + _logger.info("### Read/write file records") + record = FileRecord(file_number=14, record_number=12, record_length=64) + rr = await client.read_file_record([record, record], slave=SLAVE) + assert not rr.isError() + assert len(rr.records) == 2 + assert rr.records[0].record_data == b'SERVER DUMMY RECORD.' + assert rr.records[1].record_data == b'SERVER DUMMY RECORD.' + record.record_data = b'Pure test ' + record.record_length = len(record.record_data) / 2 + record = FileRecord(file_number=14, record_number=12, record_data=b'Pure test ') + rr = await client.write_file_record([record], slave=1) + assert not rr.isError() + + + + async def async_execute_information_requests(client): """Execute extended information requests.""" _logger.info("### Running information requests.") @@ -172,21 +193,21 @@ async def async_execute_information_requests(client): rr = await client.report_slave_id(slave=SLAVE) assert not rr.isError() # test that call was OK - # assert not rr.status + assert rr.status rr = await client.read_exception_status(slave=SLAVE) assert not rr.isError() # test that call was OK - # assert not rr.status + assert not rr.status rr = await client.diag_get_comm_event_counter(slave=SLAVE) assert not rr.isError() # test that call was OK - # assert rr.status - # assert not rr.count + assert rr.status + assert not rr.count rr = await client.diag_get_comm_event_log(slave=SLAVE) assert not rr.isError() # test that call was OK - # assert rr.status - # assert not (rr.event_count + rr.message_count + len(rr.events)) + assert rr.status + assert not (rr.event_count + rr.message_count + len(rr.events)) async def async_execute_diagnostic_requests(client): @@ -197,23 +218,35 @@ async def async_execute_diagnostic_requests(client): assert not rr.isError() # test that call was OK assert rr.message == message - await client.diag_restart_communication(True, slave=SLAVE) - await client.diag_read_diagnostic_register(slave=SLAVE) - await client.diag_change_ascii_input_delimeter(slave=SLAVE) - - # NOT WORKING: await client.diag_force_listen_only(slave=SLAVE) - - await client.diag_clear_counters() - await client.diag_read_bus_comm_error_count(slave=SLAVE) - await client.diag_read_bus_exception_error_count(slave=SLAVE) - await client.diag_read_slave_message_count(slave=SLAVE) - await client.diag_read_slave_no_response_count(slave=SLAVE) - await client.diag_read_slave_nak_count(slave=SLAVE) - await client.diag_read_slave_busy_count(slave=SLAVE) - await client.diag_read_bus_char_overrun_count(slave=SLAVE) - await client.diag_read_iop_overrun_count(slave=SLAVE) - await client.diag_clear_overrun_counter(slave=SLAVE) - # NOT WORKING await client.diag_getclear_modbus_response(slave=SLAVE) + rr = await client.diag_restart_communication(True, slave=SLAVE) + assert not rr.isError() # test that call was OK + rr = await client.diag_read_diagnostic_register(slave=SLAVE) + assert not rr.isError() # test that call was OK + rr = await client.diag_change_ascii_input_delimeter(slave=SLAVE) + assert not rr.isError() # test that call was OK + rr = await client.diag_clear_counters() + assert not rr.isError() # test that call was OK + rr = await client.diag_read_bus_comm_error_count(slave=SLAVE) + assert not rr.isError() # test that call was OK + rr = await client.diag_read_bus_exception_error_count(slave=SLAVE) + assert not rr.isError() # test that call was OK + rr = await client.diag_read_slave_message_count(slave=SLAVE) + assert not rr.isError() # test that call was OK + rr = await client.diag_read_slave_no_response_count(slave=SLAVE) + assert not rr.isError() # test that call was OK + rr = await client.diag_read_slave_nak_count(slave=SLAVE) + assert not rr.isError() # test that call was OK + rr = await client.diag_read_slave_busy_count(slave=SLAVE) + assert not rr.isError() # test that call was OK + rr = await client.diag_read_bus_char_overrun_count(slave=SLAVE) + assert not rr.isError() # test that call was OK + rr = await client.diag_read_iop_overrun_count(slave=SLAVE) + assert not rr.isError() # test that call was OK + rr = await client.diag_clear_overrun_counter(slave=SLAVE) + assert not rr.isError() # test that call was OK + rr = await client.diag_getclear_modbus_response(slave=SLAVE) + assert not rr.isError() # test that call was OK + assert not await client.diag_force_listen_only(slave=SLAVE, no_response_expected=True) # ------------------------ @@ -226,6 +259,7 @@ async def run_async_calls(client): await async_handle_discrete_input(client) await async_handle_holding_registers(client) await async_handle_input_registers(client) + await async_handle_file_records(client) await async_execute_information_requests(client) await async_execute_diagnostic_requests(client) diff --git a/examples/client_calls.py b/examples/client_calls.py index 3b3971fdf..12998ae71 100755 --- a/examples/client_calls.py +++ b/examples/client_calls.py @@ -159,57 +159,80 @@ def handle_input_registers(client): assert len(rr.registers) == 8 +def handle_file_records(_client): + """Read/write file records.""" + _logger.info("### Read/write file records") + # NOT WORKING: + + def execute_information_requests(client): """Execute extended information requests.""" _logger.info("### Running information requests.") - rr = client.read_device_information(slave=SLAVE) - assert not rr.isError() # test that call was OK - assert rr.information[0] == b"Pymodbus" + # NOT WORKING: ONLY SYNC. + # FAILS WITH framer = RTU + # rr = client.read_device_information(slave=SLAVE) + # assert not rr.isError() # test that call was OK + # assert rr.information[0] == b"Pymodbus" rr = client.report_slave_id(slave=SLAVE) assert not rr.isError() # test that call was OK - # assert not rr.status + assert rr.status rr = client.read_exception_status(slave=SLAVE) assert not rr.isError() # test that call was OK - # assert not rr.status + assert not rr.status rr = client.diag_get_comm_event_counter(slave=SLAVE) assert not rr.isError() # test that call was OK - # assert rr.status - # assert not rr.count + assert rr.status + assert not rr.count rr = client.diag_get_comm_event_log(slave=SLAVE) assert not rr.isError() # test that call was OK - # assert rr.status - # assert not (rr.event_count + rr.message_count + len(rr.events)) + assert rr.status + assert not (rr.event_count + rr.message_count + len(rr.events)) def execute_diagnostic_requests(client): """Execute extended diagnostic requests.""" _logger.info("### Running diagnostic requests.") - message = b"OK" - rr = client.diag_query_data(msg=message, slave=SLAVE) - assert not rr.isError() # test that call was OK - assert rr.message == message - - client.diag_restart_communication(True, slave=SLAVE) - client.diag_read_diagnostic_register(slave=SLAVE) - client.diag_change_ascii_input_delimeter(slave=SLAVE) + # NOT WORKING: ONLY SYNC + # message = b"OK" + # rr = client.diag_query_data(msg=message, slave=SLAVE) + # assert not rr.isError() # test that call was OK + # assert rr.message == message - # NOT WORKING: await client.diag_force_listen_only(slave=SLAVE) + rr = client.diag_restart_communication(True, slave=SLAVE) + assert not rr.isError() # test that call was OK + rr = client.diag_read_diagnostic_register(slave=SLAVE) + assert not rr.isError() # test that call was OK + rr = client.diag_change_ascii_input_delimeter(slave=SLAVE) + assert not rr.isError() # test that call was OK + rr = client.diag_clear_counters() + assert not rr.isError() # test that call was OK + rr = client.diag_read_bus_comm_error_count(slave=SLAVE) + assert not rr.isError() # test that call was OK + rr = client.diag_read_bus_exception_error_count(slave=SLAVE) + assert not rr.isError() # test that call was OK + rr = client.diag_read_slave_message_count(slave=SLAVE) + assert not rr.isError() # test that call was OK + rr = client.diag_read_slave_no_response_count(slave=SLAVE) + assert not rr.isError() # test that call was OK + rr = client.diag_read_slave_nak_count(slave=SLAVE) + assert not rr.isError() # test that call was OK + rr = client.diag_read_slave_busy_count(slave=SLAVE) + assert not rr.isError() # test that call was OK + rr = client.diag_read_bus_char_overrun_count(slave=SLAVE) + assert not rr.isError() # test that call was OK + rr = client.diag_read_iop_overrun_count(slave=SLAVE) + assert not rr.isError() # test that call was OK + rr = client.diag_clear_overrun_counter(slave=SLAVE) + assert not rr.isError() # test that call was OK + # NOT WORKING rr = client.diag_getclear_modbus_response(slave=SLAVE) + assert not rr.isError() # test that call was OK + # NOT WORKING: rr = client.diag_force_listen_only(slave=SLAVE) + assert not rr.isError() # test that call was OK - client.diag_clear_counters() - client.diag_read_bus_comm_error_count(slave=SLAVE) - client.diag_read_bus_exception_error_count(slave=SLAVE) - client.diag_read_slave_message_count(slave=SLAVE) - client.diag_read_slave_no_response_count(slave=SLAVE) - client.diag_read_slave_nak_count(slave=SLAVE) - client.diag_read_slave_busy_count(slave=SLAVE) - client.diag_read_bus_char_overrun_count(slave=SLAVE) - client.diag_read_iop_overrun_count(slave=SLAVE) - client.diag_clear_overrun_counter(slave=SLAVE) - # NOT WORKING client.diag_getclear_modbus_response(slave=SLAVE) # ------------------------ @@ -222,8 +245,9 @@ def run_sync_calls(client): handle_discrete_input(client) handle_holding_registers(client) handle_input_registers(client) - # awaiting fix: execute_information_requests(client) - # awaiting fix: execute_diagnostic_requests(client) + handle_file_records(client) + execute_information_requests(client) + execute_diagnostic_requests(client) def main(cmdline=None): diff --git a/pymodbus/client/mixin.py b/pymodbus/client/mixin.py index bfc071e8a..cf0f92412 100644 --- a/pymodbus/client/mixin.py +++ b/pymodbus/client/mixin.py @@ -367,23 +367,22 @@ def report_slave_id(self, slave: int = 1, no_response_expected: bool = False) -> """ return self.execute(no_response_expected, pdu_other_msg.ReportSlaveIdRequest(slave=slave)) - def read_file_record(self, records: list[tuple], slave: int = 1, no_response_expected: bool = False) -> T: + def read_file_record(self, records: list[pdu_file_msg.FileRecord], slave: int = 1, no_response_expected: bool = False) -> T: """Read file record (code 0x14). - :param records: List of (Reference type, File number, Record Number, Record Length) + :param records: List of FileRecord (Reference type, File number, Record Number) :param slave: device id :param no_response_expected: (optional) The client will not expect a response to the request :raises ModbusException: """ return self.execute(no_response_expected, pdu_file_msg.ReadFileRecordRequest(records, slave=slave)) - def write_file_record(self, records: list[tuple], slave: int = 1, no_response_expected: bool = False) -> T: + def write_file_record(self, records: list[pdu_file_msg.FileRecord], slave: int = 1, no_response_expected: bool = False) -> T: """Write file record (code 0x15). - :param records: List of (Reference type, File number, Record Number, Record Length) + :param records: List of File_record (Reference type, File number, Record Number, Record Length, Record Data) :param slave: (optional) Device id :param no_response_expected: (optional) The client will not expect a response to the request - :param no_response_expected: (optional) The client will not expect a response to the request :raises ModbusException: """ return self.execute(no_response_expected, pdu_file_msg.WriteFileRecordRequest(records,slave=slave)) diff --git a/pymodbus/framer/base.py b/pymodbus/framer/base.py index ffdbf52ca..91ae01dda 100644 --- a/pymodbus/framer/base.py +++ b/pymodbus/framer/base.py @@ -34,7 +34,6 @@ def __init__( ) -> None: """Initialize a ADU (framer) instance.""" self.decoder = decoder - self.databuffer = b"" def decode(self, _data: bytes) -> tuple[int, int, int, bytes]: """Decode ADU. diff --git a/pymodbus/pdu/__init__.py b/pymodbus/pdu/__init__.py index 60ba42e58..d33cae858 100644 --- a/pymodbus/pdu/__init__.py +++ b/pymodbus/pdu/__init__.py @@ -3,9 +3,11 @@ "DecodePDU", "ExceptionResponse", "ExceptionResponse", + "FileRecord", "ModbusExceptions", "ModbusPDU", ] from pymodbus.pdu.decoders import DecodePDU +from pymodbus.pdu.file_message import FileRecord from pymodbus.pdu.pdu import ExceptionResponse, ModbusExceptions, ModbusPDU diff --git a/pymodbus/pdu/decoders.py b/pymodbus/pdu/decoders.py index ca44cc535..f50d037d3 100644 --- a/pymodbus/pdu/decoders.py +++ b/pymodbus/pdu/decoders.py @@ -112,8 +112,8 @@ def decode(self, frame: bytes) -> base.ModbusPDU | None: raise ModbusException(f"Unknown response {function_code}") pdu = pdu_type() pdu.setData(0, 0, False) - Log.debug("decode PDU for {}", function_code) pdu.decode(frame[1:]) + Log.debug("decoded PDU function_code({} sub {}) -> {} ", pdu.function_code, pdu.sub_function_code, str(pdu)) if pdu.sub_function_code >= 0: lookup = self.sub_lookup.get(pdu.function_code, {}) diff --git a/pymodbus/pdu/file_message.py b/pymodbus/pdu/file_message.py index 163375792..619502db2 100644 --- a/pymodbus/pdu/file_message.py +++ b/pymodbus/pdu/file_message.py @@ -7,6 +7,7 @@ # pylint: disable=missing-type-doc import struct +from pymodbus.exceptions import ModbusException from pymodbus.pdu.pdu import ModbusExceptions as merror from pymodbus.pdu.pdu import ModbusPDU @@ -17,29 +18,30 @@ class FileRecord: # pylint: disable=eq-without-hash """Represents a file record and its relevant data.""" - def __init__(self, reference_type=0x06, file_number=0x00, record_number=0x00, record_data=b'', record_length=None, response_length=None): + def __init__(self, file_number=0x00, record_number=0x00, record_data=b'', record_length=0): """Initialize a new instance. - :params reference_type: must be 0x06 :params file_number: Indicates which file number we are reading :params record_number: Indicates which record in the file - :params record_data: The actual data of the record + :params record_data: The actual data of the record OR :params record_length: The length in registers of the record - :params response_length: The length in bytes of the record """ - self.reference_type = reference_type + if record_data: + if record_length: + raise ModbusException("Use either record_data= or record_length=") + if (record_length := len(record_data)) % 2: + raise ModbusException("length of record_data must be a multiple of 2") + record_length //= 2 + self.file_number = file_number self.record_number = record_number self.record_data = record_data - - self.record_length = record_length if record_length else len(self.record_data) // 2 - self.response_length = response_length if response_length else len(self.record_data) + 1 + self.record_length = record_length def __eq__(self, relf): """Compare the left object to the right.""" return ( # pragma: no cover - self.reference_type == relf.reference_type - and self.file_number == relf.file_number + self.file_number == relf.file_number and self.record_number == relf.record_number and self.record_length == relf.record_length and self.record_data == relf.record_data @@ -75,7 +77,7 @@ class ReadFileRecordRequest(ModbusPDU): references within each group must be sequential. Each group is defined in a separate "sub-request" field that contains seven bytes:: - The reference type: 1 byte (must be 0x06) + The reference type: 1 byte The file number: 2 bytes The starting record number within the file: 2 bytes The length of the record to be read: 2 bytes @@ -89,14 +91,14 @@ class ReadFileRecordRequest(ModbusPDU): function_code_name = "read_file_record" _rtu_byte_count_pos = 2 - def __init__(self, records=None, slave=1, transaction=0, skip_encode=False): + def __init__(self, records: list[FileRecord] = [], slave=1, transaction=0, skip_encode=False): # pylint: disable=dangerous-default-value """Initialize a new instance. :param records: The file record requests to be read """ super().__init__() super().setData(slave, transaction, skip_encode) - self.records = records or [] + self.records = records def encode(self): """Encode the request packet. @@ -128,19 +130,17 @@ def decode(self, data): record_number=decoded[2], record_length=decoded[3], ) - if decoded[0] == 0x06: # pragma: no cover - self.records.append(record) + self.records.append(record) - def update_datastore(self, _context): # pragma: no cover + async def update_datastore(self, _context): # pragma: no cover """Run a read exception status request against the store. :returns: The populated response """ - # TODO do some new context operation here # pylint: disable=fixme - # if file number, record number, or address + length - # is too big, return an error. - files: list[FileRecord] = [] - return ReadFileRecordResponse(files) + for record in self.records: + record.record_data = b'SERVER DUMMY RECORD.' + record.record_length = len(record.record_data) // 2 + return ReadFileRecordResponse(self.records) class ReadFileRecordResponse(ModbusPDU): @@ -155,24 +155,24 @@ class ReadFileRecordResponse(ModbusPDU): function_code = 0x14 _rtu_byte_count_pos = 2 - def __init__(self, records=None, slave=1, transaction=0, skip_encode=False): + def __init__(self, records: list[FileRecord] = [], slave=1, transaction=0, skip_encode=False): # pylint: disable=dangerous-default-value """Initialize a new instance. :param records: The requested file records """ super().__init__() super().setData(slave, transaction, skip_encode) - self.records = records or [] + self.records = records def encode(self): """Encode the response. :returns: The byte encoded message """ - total = sum(record.response_length + 1 for record in self.records) + total = sum(len(record.record_data) + 1 + 1 for record in self.records) packet = struct.pack("B", total) for record in self.records: - packet += struct.pack(">BB", record.response_length, 0x06) + packet += struct.pack(">BB", len(record.record_data) + 1, 0x06) packet += record.record_data return packet @@ -184,19 +184,17 @@ def decode(self, data): count, self.records = 1, [] byte_count = int(data[0]) while count < byte_count: - response_length, reference_type = struct.unpack( + calc_length, _ = struct.unpack( ">BB", data[count : count + 2] ) count += 2 - record_length = response_length - 1 # response length includes the type byte + record_length = calc_length - 1 # response length includes the type byte record = FileRecord( - response_length=response_length, record_data=data[count : count + record_length], ) count += record_length - if reference_type == 0x06: # pragma: no cover - self.records.append(record) + self.records.append(record) class WriteFileRecordRequest(ModbusPDU): @@ -212,14 +210,14 @@ class WriteFileRecordRequest(ModbusPDU): function_code_name = "write_file_record" _rtu_byte_count_pos = 2 - def __init__(self, records=None, slave=1, transaction=0, skip_encode=False): + def __init__(self, records: list[FileRecord] = [], slave=1, transaction=0, skip_encode=False): # pylint: disable=dangerous-default-value """Initialize a new instance. :param records: The file record requests to be read """ super().__init__() super().setData(slave, transaction, skip_encode) - self.records = records or [] + self.records = records def encode(self): """Encode the request packet. @@ -249,25 +247,21 @@ def decode(self, data): count, self.records = 1, [] while count < byte_count: decoded = struct.unpack(">BHHH", data[count : count + 7]) - response_length = decoded[3] * 2 - count += response_length + 7 + calc_length = decoded[3] * 2 + count += calc_length + 7 record = FileRecord( - record_length=decoded[3], file_number=decoded[1], record_number=decoded[2], - record_data=data[count - response_length : count], + record_data=data[count - calc_length : count], ) - if decoded[0] == 0x06: # pragma: no cover - self.records.append(record) + record.record_length = decoded[3] + self.records.append(record) - def update_datastore(self, _context): # pragma: no cover + async def update_datastore(self, _context): # pragma: no cover """Run the write file record request against the context. :returns: The populated response """ - # TODO do some new context operation here # pylint: disable=fixme - # if file number, record number, or address + length - # is too big, return an error. return WriteFileRecordResponse(self.records) @@ -277,14 +271,14 @@ class WriteFileRecordResponse(ModbusPDU): function_code = 0x15 _rtu_byte_count_pos = 2 - def __init__(self, records=None, slave=1, transaction=0, skip_encode=False): + def __init__(self, records: list[FileRecord] = [], slave=1, transaction=0, skip_encode=False): # pylint: disable=dangerous-default-value """Initialize a new instance. :param records: The file record requests to be read """ super().__init__() super().setData(slave, transaction, skip_encode) - self.records = records or [] + self.records = records def encode(self): """Encode the response. @@ -313,16 +307,15 @@ def decode(self, data): byte_count = int(data[0]) while count < byte_count: decoded = struct.unpack(">BHHH", data[count : count + 7]) - response_length = decoded[3] * 2 - count += response_length + 7 + calc_length = decoded[3] * 2 + count += calc_length + 7 record = FileRecord( - record_length=decoded[3], file_number=decoded[1], record_number=decoded[2], - record_data=data[count - response_length : count], + record_data=data[count - calc_length : count], ) - if decoded[0] == 0x06: # pragma: no cover - self.records.append(record) + record.record_length = decoded[3] + self.records.append(record) class ReadFifoQueueRequest(ModbusPDU): @@ -367,7 +360,7 @@ def decode(self, data): """ self.address = struct.unpack(">H", data)[0] - def update_datastore(self, _context): # pragma: no cover + async def update_datastore(self, _context): # pragma: no cover """Run a read exception status request against the store. :returns: The populated response diff --git a/pymodbus/transaction.py b/pymodbus/transaction.py index 2e83290cc..dbcc0dda0 100644 --- a/pymodbus/transaction.py +++ b/pymodbus/transaction.py @@ -136,7 +136,6 @@ def __init__(self, client: ModbusBaseSyncClient, retries): self.client: ModbusBaseSyncClient = client self.retries = retries self._transaction_lock = RLock() - self._no_response_devices: list[int] = [] self.databuffer = b'' if client: self._set_adu_size() @@ -185,17 +184,16 @@ def execute(self, no_response_expected: bool, request: ModbusPDU): # noqa: C901 "Current transaction state - {}", ModbusTransactionState.to_string(self.client.state), ) - retries = self.retries if isinstance(self.client.framer, FramerSocket): request.transaction_id = self.getNextTID() else: request.transaction_id = 0 Log.debug("Running transaction {}", request.transaction_id) if _buffer := hexlify_packets( - self.client.framer.databuffer + self.databuffer ): Log.debug("Clearing current Frame: - {}", _buffer) - self.client.framer.databuffer = b'' + self.databuffer = b'' expected_response_length = None if not isinstance(self.client.framer, FramerSocket): response_pdu_size = request.get_response_pdu_size() @@ -205,12 +203,7 @@ def execute(self, no_response_expected: bool, request: ModbusPDU): # noqa: C901 expected_response_length = ( self._calculate_response_length(response_pdu_size) ) - if ( # pylint: disable=simplifiable-if-statement - request.slave_id in self._no_response_devices - ): - full = True - else: - full = False + full = False if self.client.comm_params.comm_type == CommType.UDP: full = True if not expected_response_length: @@ -223,20 +216,6 @@ def execute(self, no_response_expected: bool, request: ModbusPDU): # noqa: C901 ) if no_response_expected: return None - while retries > 0: - if self._validate_response(response): - if ( - request.slave_id in self._no_response_devices - and response - ): - self._no_response_devices.remove(request.slave_id) - Log.debug("Got response!!!") - break - if not response: - if request.slave_id not in self._no_response_devices: - self._no_response_devices.append(request.slave_id) - # No response received and retries not enabled - break self.databuffer += response used_len, pdu = self.client.framer.processIncomingFrame(self.databuffer) self.databuffer = self.databuffer[used_len:] diff --git a/test/framer/test_framer.py b/test/framer/test_framer.py index cce0d98bd..28ab35136 100644 --- a/test/framer/test_framer.py +++ b/test/framer/test_framer.py @@ -415,7 +415,6 @@ def test_processIncomingFrame_roundtrip(self, entry, test_framer, msg, dev_id, t assert result assert result.slave_id == dev_id assert result.transaction_id == tid - assert not test_framer.databuffer expected = test_framer.encode( result.function_code.to_bytes(1,'big') + result.encode(), dev_id, 1) diff --git a/test/pdu/test_file_message.py b/test/pdu/test_file_message.py index fb18e96f3..24ed1aa41 100644 --- a/test/pdu/test_file_message.py +++ b/test/pdu/test_file_message.py @@ -41,27 +41,27 @@ def test_read_fifo_queue_request_decode(self): handle.decode(b"\x12\x34") assert handle.address == 0x1234 - def test_read_fifo_queue_request(self): + async def test_read_fifo_queue_request(self): """Test basic bit message encoding/decoding.""" context = MockContext() handle = ReadFifoQueueRequest(0x1234) - result = handle.update_datastore(context) + result = await handle.update_datastore(context) assert isinstance(result, ReadFifoQueueResponse) handle.address = -1 - result = handle.update_datastore(context) + result = await handle.update_datastore(context) assert ModbusExceptions.IllegalValue == result.exception_code handle.values = [0x00] * 33 - result = handle.update_datastore(context) + result = await handle.update_datastore(context) assert ModbusExceptions.IllegalValue == result.exception_code - def test_read_fifo_queue_request_error(self): + async def test_read_fifo_queue_request_error(self): """Test basic bit message encoding/decoding.""" context = MockContext() handle = ReadFifoQueueRequest(0x1234) handle.values = [0x00] * 32 - result = handle.update_datastore(context) + result = await handle.update_datastore(context) assert result.function_code == 0x98 def test_read_fifo_queue_response_encode(self): @@ -94,7 +94,6 @@ def test_file_record_length(self): file_number=0x01, record_number=0x02, record_data=b"\x00\x01\x02\x04" ) assert record.record_length == 0x02 - assert record.response_length == 0x05 def test_file_record_compare(self): """Test file record comparison operations.""" @@ -148,10 +147,10 @@ def test_read_file_record_request_rtu_frame_size(self): size = handle.calculateRtuFrameSize(request) assert size == 0x0E + 5 - def test_read_file_record_request_update_datastore(self): + async def test_read_file_record_request_update_datastore(self): """Test basic bit message encoding/decoding.""" handle = ReadFileRecordRequest() - result = handle.update_datastore(None) + result = await handle.update_datastore(None) assert isinstance(result, ReadFileRecordResponse) # -----------------------------------------------------------------------# @@ -221,10 +220,10 @@ def test_write_file_record_request_rtu_frame_size(self): size = handle.calculateRtuFrameSize(request) assert size == 0x0D + 5 - def test_write_file_record_request_update_datastore(self): + async def test_write_file_record_request_update_datastore(self): """Test basic bit message encoding/decoding.""" handle = WriteFileRecordRequest() - result = handle.update_datastore(None) + result = await handle.update_datastore(None) assert isinstance(result, WriteFileRecordResponse) # -----------------------------------------------------------------------# diff --git a/test/sub_examples/test_client_server_async.py b/test/sub_examples/test_client_server_async.py index 92446785d..bac88b79b 100755 --- a/test/sub_examples/test_client_server_async.py +++ b/test/sub_examples/test_client_server_async.py @@ -8,7 +8,6 @@ These are basis for most examples and thus tested separately """ import asyncio -from unittest import mock import pytest @@ -18,7 +17,6 @@ run_async_client, setup_async_client, ) -from pymodbus.exceptions import ModbusIOException @pytest.mark.parametrize( @@ -47,15 +45,6 @@ async def test_combinations(self, mock_server, mock_clc): assert mock_server await main(cmdline=mock_clc) - async def test_client_exception(self, mock_server, mock_clc): - """Run async client and server.""" - assert mock_server - test_client = setup_async_client(cmdline=mock_clc) - test_client.read_holding_registers = mock.AsyncMock( - side_effect=ModbusIOException("test") - ) - await run_async_client(test_client, modbus_calls=run_a_few_calls) - async def test_client_no_calls(self, mock_server, mock_clc): """Run async client and server.""" assert mock_server From ffdf01f1c7ba7b1e8bcaf211fc20fff9c2774bcb Mon Sep 17 00:00:00 2001 From: jan iversen Date: Thu, 24 Oct 2024 21:55:14 +0200 Subject: [PATCH 2/3] Step. --- examples/client_calls.py | 16 ++++++++++++++-- 1 file changed, 14 insertions(+), 2 deletions(-) diff --git a/examples/client_calls.py b/examples/client_calls.py index 12998ae71..dc6228eb3 100755 --- a/examples/client_calls.py +++ b/examples/client_calls.py @@ -32,6 +32,8 @@ import logging import sys +from pymodbus.pdu import FileRecord + try: import client_sync @@ -159,10 +161,20 @@ def handle_input_registers(client): assert len(rr.registers) == 8 -def handle_file_records(_client): +def handle_file_records(client): """Read/write file records.""" _logger.info("### Read/write file records") - # NOT WORKING: + record = FileRecord(file_number=14, record_number=12, record_length=64) + rr = client.read_file_record([record, record], slave=SLAVE) + assert not rr.isError() + assert len(rr.records) == 2 + assert rr.records[0].record_data == b'SERVER DUMMY RECORD.' + assert rr.records[1].record_data == b'SERVER DUMMY RECORD.' + record.record_data = b'Pure test ' + record.record_length = len(record.record_data) / 2 + record = FileRecord(file_number=14, record_number=12, record_data=b'Pure test ') + rr = client.write_file_record([record], slave=1) + assert not rr.isError() def execute_information_requests(client): From 948d770d5f7a9bf6dd5574201b0c5614ed625616 Mon Sep 17 00:00:00 2001 From: jan iversen Date: Thu, 24 Oct 2024 22:06:55 +0200 Subject: [PATCH 3/3] FInal. --- pymodbus/pdu/bit_read_message.py | 2 -- pymodbus/pdu/bit_write_message.py | 2 -- pymodbus/pdu/diag_message.py | 1 - pymodbus/pdu/file_message.py | 3 --- pymodbus/pdu/mei_message.py | 1 - pymodbus/pdu/other_message.py | 4 ---- pymodbus/pdu/register_read_message.py | 3 --- pymodbus/pdu/register_write_message.py | 3 --- pymodbus/server/simulator/http_server.py | 4 ++-- 9 files changed, 2 insertions(+), 21 deletions(-) diff --git a/pymodbus/pdu/bit_read_message.py b/pymodbus/pdu/bit_read_message.py index e37a6985e..2873937b8 100644 --- a/pymodbus/pdu/bit_read_message.py +++ b/pymodbus/pdu/bit_read_message.py @@ -132,7 +132,6 @@ class ReadCoilsRequest(ReadBitsRequestBase): """ function_code = 1 - function_code_name = "read_coils" def __init__(self, address=None, count=None, slave=1, transaction=0, skip_encode=False): """Initialize a new instance. @@ -200,7 +199,6 @@ class ReadDiscreteInputsRequest(ReadBitsRequestBase): """ function_code = 2 - function_code_name = "read_discrete_input" def __init__(self, address=None, count=None, slave=1, transaction=0, skip_encode=False): """Initialize a new instance. diff --git a/pymodbus/pdu/bit_write_message.py b/pymodbus/pdu/bit_write_message.py index ad995f9a7..06f4aee30 100644 --- a/pymodbus/pdu/bit_write_message.py +++ b/pymodbus/pdu/bit_write_message.py @@ -39,7 +39,6 @@ class WriteSingleCoilRequest(ModbusPDU): """ function_code = 5 - function_code_name = "write_coil" _rtu_frame_size = 8 @@ -163,7 +162,6 @@ class WriteMultipleCoilsRequest(ModbusPDU): """ function_code = 15 - function_code_name = "write_coils" _rtu_byte_count_pos = 6 def __init__(self, address=0, values=None, slave=None, transaction=0, skip_encode=0): diff --git a/pymodbus/pdu/diag_message.py b/pymodbus/pdu/diag_message.py index 3fa580334..b8a85b1ee 100644 --- a/pymodbus/pdu/diag_message.py +++ b/pymodbus/pdu/diag_message.py @@ -28,7 +28,6 @@ class DiagnosticStatusRequest(ModbusPDU): """This is a base class for all of the diagnostic request functions.""" function_code = 0x08 - function_code_name = "diagnostic_status" sub_function_code = 9999 _rtu_frame_size = 8 diff --git a/pymodbus/pdu/file_message.py b/pymodbus/pdu/file_message.py index 619502db2..ec3594993 100644 --- a/pymodbus/pdu/file_message.py +++ b/pymodbus/pdu/file_message.py @@ -88,7 +88,6 @@ class ReadFileRecordRequest(ModbusPDU): """ function_code = 0x14 - function_code_name = "read_file_record" _rtu_byte_count_pos = 2 def __init__(self, records: list[FileRecord] = [], slave=1, transaction=0, skip_encode=False): # pylint: disable=dangerous-default-value @@ -207,7 +206,6 @@ class WriteFileRecordRequest(ModbusPDU): """ function_code = 0x15 - function_code_name = "write_file_record" _rtu_byte_count_pos = 2 def __init__(self, records: list[FileRecord] = [], slave=1, transaction=0, skip_encode=False): # pylint: disable=dangerous-default-value @@ -333,7 +331,6 @@ class ReadFifoQueueRequest(ModbusPDU): """ function_code = 0x18 - function_code_name = "read_fifo_queue" _rtu_frame_size = 6 def __init__(self, address=0x0000, slave=1, transaction=0, skip_encode=False): diff --git a/pymodbus/pdu/mei_message.py b/pymodbus/pdu/mei_message.py index 0085f23dc..fe2eb9e9a 100644 --- a/pymodbus/pdu/mei_message.py +++ b/pymodbus/pdu/mei_message.py @@ -49,7 +49,6 @@ class ReadDeviceInformationRequest(ModbusPDU): function_code = 0x2B sub_function_code = 0x0E - function_code_name = "read_device_information" _rtu_frame_size = 7 def __init__(self, read_code=None, object_id=0x00, slave=1, transaction=0, skip_encode=False): diff --git a/pymodbus/pdu/other_message.py b/pymodbus/pdu/other_message.py index 597a37bde..c7b28374e 100644 --- a/pymodbus/pdu/other_message.py +++ b/pymodbus/pdu/other_message.py @@ -27,7 +27,6 @@ class ReadExceptionStatusRequest(ModbusPDU): """ function_code = 0x07 - function_code_name = "read_exception_status" _rtu_frame_size = 4 def __init__(self, slave=None, transaction=0, skip_encode=0): @@ -128,7 +127,6 @@ class GetCommEventCounterRequest(ModbusPDU): """ function_code = 0x0B - function_code_name = "get_event_counter" _rtu_frame_size = 4 def __init__(self, slave=1, transaction=0, skip_encode=False): @@ -235,7 +233,6 @@ class GetCommEventLogRequest(ModbusPDU): """ function_code = 0x0C - function_code_name = "get_event_log" _rtu_frame_size = 4 def __init__(self, slave=1, transaction=0, skip_encode=False): @@ -358,7 +355,6 @@ class ReportSlaveIdRequest(ModbusPDU): """ function_code = 0x11 - function_code_name = "report_slave_id" _rtu_frame_size = 4 def __init__(self, slave=1, transaction=0, skip_encode=False): diff --git a/pymodbus/pdu/register_read_message.py b/pymodbus/pdu/register_read_message.py index 524f05091..7d0abe7fb 100644 --- a/pymodbus/pdu/register_read_message.py +++ b/pymodbus/pdu/register_read_message.py @@ -124,7 +124,6 @@ class ReadHoldingRegistersRequest(ReadRegistersRequestBase): """ function_code = 3 - function_code_name = "read_holding_registers" def __init__(self, address=None, count=None, slave=1, transaction=0, skip_encode=0): """Initialize a new instance of the request. @@ -186,7 +185,6 @@ class ReadInputRegistersRequest(ReadRegistersRequestBase): """ function_code = 4 - function_code_name = "read_input_registers" def __init__(self, address=None, count=None, slave=1, transaction=0, skip_encode=0): """Initialize a new instance of the request. @@ -254,7 +252,6 @@ class ReadWriteMultipleRegistersRequest(ModbusPDU): """ function_code = 23 - function_code_name = "read_write_multiple_registers" _rtu_byte_count_pos = 10 def __init__(self, read_address=0x00, read_count=0, write_address=0x00, write_registers=None, slave=1, transaction=0, skip_encode=False): diff --git a/pymodbus/pdu/register_write_message.py b/pymodbus/pdu/register_write_message.py index bb94bad18..89095979a 100644 --- a/pymodbus/pdu/register_write_message.py +++ b/pymodbus/pdu/register_write_message.py @@ -17,7 +17,6 @@ class WriteSingleRegisterRequest(ModbusPDU): """ function_code = 6 - function_code_name = "write_register" _rtu_frame_size = 8 def __init__(self, address=None, value=None, slave=None, transaction=0, skip_encode=0): @@ -150,7 +149,6 @@ class WriteMultipleRegistersRequest(ModbusPDU): """ function_code = 16 - function_code_name = "write_registers" _rtu_byte_count_pos = 6 _pdu_length = 5 # func + adress1 + adress2 + outputQuant1 + outputQuant2 @@ -291,7 +289,6 @@ class MaskWriteRegisterRequest(ModbusPDU): """ function_code = 0x16 - function_code_name = "mask_write_register" _rtu_frame_size = 10 def __init__(self, address=0x0000, and_mask=0xFFFF, or_mask=0x0000, slave=1, transaction=0, skip_encode=False): diff --git a/pymodbus/server/simulator/http_server.py b/pymodbus/server/simulator/http_server.py index aba2f3432..ed6371ef2 100644 --- a/pymodbus/server/simulator/http_server.py +++ b/pymodbus/server/simulator/http_server.py @@ -393,7 +393,7 @@ def build_html_calls(self, params: dict, html: str) -> str: if function.function_code == self.call_monitor.function else "" ) - function_codes += f"" #type: ignore[attr-defined] + function_codes += f"" simulation_action = ( "ACTIVE" if self.call_response.active != RESPONSE_INACTIVE else "" ) @@ -557,7 +557,7 @@ def build_json_calls(self, params: dict) -> dict: for function in self.request_lookup.values(): function_codes.append({ "value": function.function_code, - "text": function.function_code_name, # type: ignore[attr-defined] + "text": "function code name", "selected": function.function_code == self.call_monitor.function })