Skip to content
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

Resubmit: Don't close/reopen tcp connection on single modbus message timeout #2350

Merged
merged 4 commits into from
Oct 19, 2024
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
Binary file modified doc/source/_static/examples.tgz
Binary file not shown.
Binary file modified doc/source/_static/examples.zip
Binary file not shown.
8 changes: 8 additions & 0 deletions doc/source/client.rst
Original file line number Diff line number Diff line change
Expand Up @@ -166,6 +166,14 @@ The line :mod:`result = await client.read_coils(2, 3, slave=1)` is an example of

The last line :mod:`client.close()` closes the connection and render the object inactive.

Retry logic for async clients
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

If no response is received to a request (call), it is retried (parameter retries) times, if not successful
an exception response is returned, BUT the connection is not touched.

If 3 consequitve requests (calls) do not receive a response, the connection is terminated.

Development notes
^^^^^^^^^^^^^^^^^

Expand Down
19 changes: 13 additions & 6 deletions pymodbus/client/base.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@
from pymodbus.exceptions import ConnectionException, ModbusIOException
from pymodbus.framer import FRAMER_NAME_TO_CLASS, FramerBase, FramerType
from pymodbus.logging import Log
from pymodbus.pdu import DecodePDU, ModbusPDU
from pymodbus.pdu import DecodePDU, ExceptionResponse, ModbusPDU
from pymodbus.transaction import SyncModbusTransactionManager
from pymodbus.transport import CommParams
from pymodbus.utilities import ModbusTransactionState
Expand Down Expand Up @@ -50,6 +50,8 @@ def __init__(
self.last_frame_end: float | None = 0
self.silent_interval: float = 0
self._lock = asyncio.Lock()
self.accept_no_response_limit = 3
self.count_no_responses = 0

@property
def connected(self) -> bool:
Expand Down Expand Up @@ -115,11 +117,16 @@ async def async_execute(self, request) -> ModbusPDU:
except asyncio.exceptions.TimeoutError:
count += 1
if count > self.retries:
self.ctx.connection_lost(asyncio.TimeoutError("Server not responding"))
raise ModbusIOException(
f"ERROR: No response received after {self.retries} retries"
)

if self.count_no_responses >= self.accept_no_response_limit:
self.ctx.connection_lost(asyncio.TimeoutError("Server not responding"))
raise ModbusIOException(
f"ERROR: No response received of the last {self.accept_no_response_limit} request, CLOSING CONNECTION."
)
self.count_no_responses += 1
Log.error(f"No response received after {self.retries} retries, continue with next request")
return ExceptionResponse(request.function_code)

self.count_no_responses = 0
return resp

def build_response(self, request: ModbusPDU):
Expand Down
7 changes: 2 additions & 5 deletions pymodbus/pdu/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,13 +2,10 @@
__all__ = [
"DecodePDU",
"ExceptionResponse",
"ExceptionResponse",
"ModbusExceptions",
"ModbusPDU",
]

from pymodbus.pdu.decoders import DecodePDU
from pymodbus.pdu.pdu import (
ExceptionResponse,
ModbusExceptions,
ModbusPDU,
)
from pymodbus.pdu.pdu import ExceptionResponse, ModbusExceptions, ModbusPDU
13 changes: 7 additions & 6 deletions test/sub_client/test_client.py
Original file line number Diff line number Diff line change
Expand Up @@ -20,8 +20,8 @@
from pymodbus.client.mixin import ModbusClientMixin
from pymodbus.datastore import ModbusSlaveContext
from pymodbus.datastore.store import ModbusSequentialDataBlock
from pymodbus.exceptions import ConnectionException, ModbusException, ModbusIOException
from pymodbus.pdu import ModbusPDU
from pymodbus.exceptions import ConnectionException, ModbusException
from pymodbus.pdu import ExceptionResponse, ModbusPDU
from pymodbus.transport import CommParams, CommType


Expand Down Expand Up @@ -436,8 +436,9 @@ async def test_client_execute_broadcast():
transport = MockTransport(base, request)
base.ctx.connection_made(transport=transport)

with pytest.raises(ModbusIOException):
assert not await base.async_execute(request)
# with pytest.raises(ModbusIOException):
# assert not await base.async_execute(request)
assert await base.async_execute(request)

async def test_client_protocol_retry():
"""Test the client protocol execute method with retries."""
Expand Down Expand Up @@ -477,8 +478,8 @@ async def test_client_protocol_timeout():
transport = MockTransport(base, request, retries=4)
base.ctx.connection_made(transport=transport)

with pytest.raises(ModbusIOException):
await base.async_execute(request)
pdu = await base.async_execute(request)
assert isinstance(pdu, ExceptionResponse)
assert transport.retries == 1


Expand Down