Allow id=0 and check if response.id == request.id. (#2572)

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

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

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