Skip to content

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

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Feb 10, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 4 additions & 2 deletions doc/source/client.rst
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
17 changes: 13 additions & 4 deletions pymodbus/transaction/transaction.py
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand All @@ -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:
Expand All @@ -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:
Expand Down Expand Up @@ -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
Expand Down
134 changes: 0 additions & 134 deletions test/not_updated/test_network.py

This file was deleted.

79 changes: 76 additions & 3 deletions test/transaction/test_transaction.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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:
Expand Down Expand Up @@ -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