diff --git a/doc/source/client.rst b/doc/source/client.rst index 1ac217a22..57adceaa3 100644 --- a/doc/source/client.rst +++ b/doc/source/client.rst @@ -194,17 +194,19 @@ Client device addressing ------------------------ With **TCP**, **TLS** and **UDP**, the tcp/ip address of the physical device is defined when creating the object. -The logical devices represented by the device is addressed with the :mod:`slave=` parameter. +Logical devices represented by the device is addressed with the :mod:`slave=` parameter. With **Serial**, the comm port is defined when creating the object. The physical devices are addressed with the :mod:`slave=` parameter. :mod:`slave=0` is defined as broadcast in the modbus standard, but pymodbus treats it as a normal device. +please note :mod:`slave=0` can only be used to address devices that truly have id=0 ! Using :mod:`slave=0` to +address a single device with id not 0 is against the protocol. If an application is expecting multiple responses to a broadcast request, it must call :mod:`client.execute` and deal with the responses. If no response is expected to a request, the :mod:`no_response_expected=True` argument can be used -in the normal API calls, this will cause the call to return immediately with :mod:`None`. +in the normal API calls, this will cause the call to return immediately with :mod:`ExceptionResponse(0xff)`. Client response handling diff --git a/pymodbus/transaction/transaction.py b/pymodbus/transaction/transaction.py index 03c5ddd15..e42f84e03 100644 --- a/pymodbus/transaction/transaction.py +++ b/pymodbus/transaction/transaction.py @@ -77,7 +77,7 @@ def dummy_trace_connect(self, connect: bool) -> None: """Do dummy trace.""" _ = connect - def sync_get_response(self) -> ModbusPDU: + def sync_get_response(self, dev_id) -> ModbusPDU: """Receive until PDU is correct or timeout.""" databuffer = b'' while True: @@ -87,6 +87,11 @@ def sync_get_response(self) -> ModbusPDU: used_len, pdu = self.framer.processIncomingFrame(databuffer) databuffer = databuffer[used_len:] if pdu: + if pdu.dev_id != dev_id: + raise ModbusIOException( + f"ERROR: request ask for id={dev_id} but id={pdu.dev_id}, CLOSING CONNECTION." + ) + return pdu def sync_execute(self, no_response_expected: bool, request: ModbusPDU) -> ModbusPDU: @@ -102,10 +107,10 @@ def sync_execute(self, no_response_expected: bool, request: ModbusPDU) -> Modbus count_retries = 0 while count_retries <= self.retries: self.pdu_send(request) - if not request.dev_id or no_response_expected: + if no_response_expected: return ExceptionResponse(0xff) try: - return self.sync_get_response() + return self.sync_get_response(request.dev_id) except asyncio.exceptions.TimeoutError: count_retries += 1 if self.count_until_disconnect < 0: @@ -134,13 +139,17 @@ async def execute(self, no_response_expected: bool, request: ModbusPDU) -> Modbu while count_retries <= self.retries: self.response_future = asyncio.Future() self.pdu_send(request) - if not request.dev_id or no_response_expected: + if no_response_expected: return ExceptionResponse(0xff) try: response = await asyncio.wait_for( self.response_future, timeout=self.comm_params.timeout_connect ) self.count_until_disconnect= self.max_until_disconnect + if response.dev_id != request.dev_id: + raise ModbusIOException( + f"ERROR: request ask for id={request.dev_id} but id={response.dev_id}, CLOSING CONNECTION." + ) return response except asyncio.exceptions.TimeoutError: count_retries += 1 diff --git a/test/not_updated/test_network.py b/test/not_updated/test_network.py deleted file mode 100644 index 97bfcb0a1..000000000 --- a/test/not_updated/test_network.py +++ /dev/null @@ -1,134 +0,0 @@ -"""Test transport.""" -from __future__ import annotations - -import asyncio -from collections.abc import Callable - -import pytest - -from pymodbus.client import AsyncModbusTcpClient -from pymodbus.logging import Log -from pymodbus.transport import NULLMODEM_HOST, CommParams, ModbusProtocol - - -class ModbusProtocolStub(ModbusProtocol): - """Protocol layer including transport.""" - - def __init__( - self, - params: CommParams, - is_server: bool, - handler: Callable[[bytes], bytes] | None = None, - ) -> None: - """Initialize a stub instance.""" - self.stub_handle_data = handler if handler else self.dummy_handler - super().__init__(params, is_server) - - - async def start_run(self): - """Call need functions to start server/client.""" - return await self.listen() - - - def callback_connected(self) -> None: - """Call when connection is succcesfull.""" - - def callback_disconnected(self, exc: Exception | None) -> None: - """Call when connection is lost.""" - Log.debug("callback_disconnected called: {}", exc) - - def callback_data(self, data: bytes, addr: tuple | None = None) -> int: - """Handle received data.""" - if (response := self.stub_handle_data(data)): - self.send(response) - return len(data) - - def callback_new_connection(self) -> ModbusProtocol: - """Call when listener receive new connection request.""" - new_stub = ModbusProtocolStub(self.comm_params, False) - new_stub.stub_handle_data = self.stub_handle_data - return new_stub - - # ---------------- # - # external methods # - # ---------------- # - def dummy_handler(self, data: bytes) -> bytes | None: - """Handle received data.""" - return data - -class TestNetwork: - """Test network problems.""" - - @staticmethod - @pytest.fixture(name="use_port") - def get_port_in_class(base_ports): - """Return next port.""" - base_ports[__class__.__name__] += 1 - return base_ports[__class__.__name__] - - async def test_stub(self, use_port, use_cls): - """Test double packet on network.""" - Log.debug("test_double_packet {}", use_port) - client = AsyncModbusTcpClient(NULLMODEM_HOST, port=use_port) - stub = ModbusProtocolStub(use_cls, True) - assert await stub.start_run() - assert await client.connect() - test_data = b"Data got echoed." - client.ctx.transport.write(test_data) - client.close() - stub.close() - - async def test_parallel_requests(self, use_port, use_cls): - """Test double packet on network.""" - old_data = b'' - client = AsyncModbusTcpClient(NULLMODEM_HOST, port=use_port, retries=0, timeout=30) - - def local_handle_data(data: bytes) -> bytes | None: - """Handle server side for this test case.""" - nonlocal old_data - - addr = int(data[9]) - response = data[0:5] + b'\x05\x00\x03\x02\x00' + (addr*10).to_bytes(1, 'big') - - # 1, 4, 7 return correct data - # 2, 5 return NO data - # 3 return 2 + 3 - # 6 return 6 + 6 (wrong order - # 8 return 7 + half 8 - # 9 return second half 8 + 9 - if addr in {2, 5}: - old_data = response - response = None - elif addr == 3: - response = old_data + response - old_data = b'' - elif addr == 6: - response = response + old_data - old_data = b'' - elif addr == 8: - x = response - response = response[:7] - old_data = x[7:] - elif addr == 9: - response = old_data + response - old_data = b'' - return response - - async def local_call(addr: int) -> bool: - """Call read_holding_register and control.""" - nonlocal client - reply = await client.read_holding_registers(address=addr, count=1) - assert not reply.isError(), f"addr {addr} isError" - assert reply.registers[0] == addr * 10, f"addr {addr} value" - - stub = ModbusProtocolStub(use_cls, True, handler=local_handle_data) - stub.stub_handle_data = local_handle_data - await stub.start_run() - - assert await client.connect() - try: - await asyncio.gather(*[local_call(1) for x in range(1, 10)]) - except Exception as exc: # pylint: disable=broad-exception-caught - pytest.fail(exc) - client.close() - stub.close() diff --git a/test/transaction/test_transaction.py b/test/transaction/test_transaction.py index a3ee3525f..5b984dee3 100755 --- a/test/transaction/test_transaction.py +++ b/test/transaction/test_transaction.py @@ -151,8 +151,8 @@ async def test_transaction_execute(self, use_clc, scenario): transact.trace_pdu = mock.Mock(return_value=request) transact.trace_packet = mock.Mock(return_value=b'123') await transact.execute(True, request) - #transact.trace_pdu.assert_called_once_with(True, request) - #transact.trace_packet.assert_called_once_with(True, b'\x00\x01\x00u\x00\x05\xec\x02') + transact.trace_pdu.assert_called_once_with(True, request) + transact.trace_packet.assert_called_once_with(True, b'\x01\x01\x00u\x00\x05\xed\xd3') elif scenario == 3: # wait receive,timeout, no_responses transact.comm_params.timeout_connect = 0.1 transact.count_no_responses = 10 @@ -212,7 +212,7 @@ async def test_client_protocol_execute_outside(self, use_clc, no_resp): transact.transport.write = mock.Mock() resp = asyncio.create_task(transact.execute(no_resp, request)) await asyncio.sleep(0.2) - data = b"\x00\x00\x12\x34\x00\x06\xff\x01\x01\x02\x00\x04" + data = b"\x00\x00\x12\x34\x00\x06\x01\x01\x01\x02\x00\x04" transact.data_received(data) result = await resp if no_resp: @@ -417,3 +417,76 @@ def test_sync_client_protocol_execute_no_pdu(self, use_clc): transact.sync_client.send = mock.Mock() with pytest.raises(ModbusIOException): transact.sync_execute(False, request) + + def test_transaction_sync_id0(self, use_clc): + """Test id 0 in sync.""" + client = ModbusBaseSyncClient( + FramerType.SOCKET, + 5, + use_clc, + None, + None, + None, + ) + transact = TransactionManager( + use_clc, + FramerRTU(DecodePDU(False)), + 5, + False, + None, + None, + None, + sync_client=client, + ) + transact.sync_client.connect = mock.Mock(return_value=True) + transact.sync_client.send = mock.Mock() + request = ReadCoilsRequest(address=117, count=5, dev_id=0) + response = ReadCoilsResponse(bits=[True, False, True, True, False, False, False, False], dev_id=1) + transact.retries = 0 + transact.transport = 1 + resp_bytes = transact.framer.buildFrame(response) + transact.sync_client.recv = mock.Mock(return_value=resp_bytes) + transact.sync_client.send = mock.Mock() + transact.comm_params.timeout_connect = 0.2 + with pytest.raises(ModbusIOException): + transact.sync_execute(False, request) + response = ReadCoilsResponse(bits=[True, False, True, True, False, False, False, False], dev_id=0) + resp_bytes = transact.framer.buildFrame(response) + transact.sync_client.recv = mock.Mock(return_value=resp_bytes) + resp = transact.sync_execute(False, request) + assert not resp.isError() + + async def test_transaction_id0(self, use_clc): + """Test tracers in disconnect.""" + transact = TransactionManager( + use_clc, + FramerRTU(DecodePDU(False)), + 5, + False, + None, + None, + None, + ) + transact.send = mock.Mock() + request = ReadCoilsRequest(address=117, count=5, dev_id=1) + response = ReadCoilsResponse(bits=[True, False, True, True, False], dev_id=0) + transact.retries = 0 + transact.connection_made(mock.AsyncMock()) + transact.transport.write = mock.Mock() + transact.comm_params.timeout_connect = 0.2 + resp = asyncio.create_task(transact.execute(False, request)) + await asyncio.sleep(0.1) + transact.response_future.set_result(response) + await asyncio.sleep(0.1) + with pytest.raises(ModbusIOException): + await resp + response = ReadCoilsResponse(bits=[True, False, True, True, False], dev_id=1) + transact.retries = 0 + transact.connection_made(mock.AsyncMock()) + transact.transport.write = mock.Mock() + transact.comm_params.timeout_connect = 0.2 + resp = asyncio.create_task(transact.execute(False, request)) + await asyncio.sleep(0.1) + transact.response_future.set_result(response) + await asyncio.sleep(0.1) + assert response == await resp