Don't close/reopen tcp connection on single modbus message timeout #2346
Conversation
| @@ -86,8 +86,16 @@ def __init__( # pylint: disable=too-many-arguments | |||
| ) | |||
|
|
|||
| def close(self, reconnect: bool = False) -> None: | |||
| """Close connection.""" | |||
There was a problem hiding this comment.
This is wrong, this method is called by the app when it wants to close the connection.
Why would you prevent the app from closing the connection ? Your problem is down in the transport layer.
There was a problem hiding this comment.
Not sure how this can be resolved in the transport layer. The issue is the call to close() in async_execute() in client/base.py after too many retries of a particular request. The remote end will never respond to this request as it addresses an inactive slave. It will however respond to other requests, and closing the connection as a result causes these to fail too.
Maybe a better fix is in async_execute but it feels wrong to have tcp specific code in there. Any suggestions welcome!
There was a problem hiding this comment.
if the issue if the call to close in async_execute, then that would be a better place to fix the problem rather than prohibiting the app to close a connection.
Be aware your PR highlighted some old code, that should have been updated long time ago, it will be so very shortly.
|
dev is updated. |
|
??? I thought you had a problem, that you tried to solve ? Your solution, as I pointed out, had serious side-effects and thus was not viable...but a working solution would be reviewed positively. Please be aware the changes I made to the reconnect= parameter does NOT affect the close in case of timeout. |
|
Apologies trying to redo some changes.
…On Mon, 30 Sept 2024, 21:59 jan iversen, ***@***.***> wrote:
??? I thought you had a problem, that you tried to solve ?
Your solution, as I pointed out, had serious side-effects and thus was not
viable...but a working solution would be reviewed positively.
—
Reply to this email directly, view it on GitHub
<#2346 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/BLWSLS5WWGDNRQMW7SFE3GTZZG3UVAVCNFSM6AAAAABPCYRBAGVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDGOBUGEZTIOBZHE>
.
You are receiving this because you modified the open/close state.Message
ID: ***@***.***>
|
|
Closed in error - trying to reopen |
|
Pull request now updated |
I am connecting to a TCP to RTU modbus gateway which transparently passes on modbus requests to serial slaves on the downstream side. All works fine most of the time.
The application (home assistant) uses the AsyncModbusTCPClient to send periodic requests from a number of different threads to fetch data from different sensors on the different configured slaves.
If one slave malfunctions, or is powered off, a modbus request to it times out (though the gateway ack's the request at the TCP layer). This then causes the connection to be closed, and all the queued pending requests to the other slaves are failed. (Even though they would have succeeded if allowed to proceed).
While the connection quickly re-establishes again, the same thing happens as the polling function repolls the same unresponsive slave, and hence blocks successful activity from other healthy slaves.
I get good behaviour if in the tcp client we do not actually close the connection if the command to close with reconnect==True is sent. This relies on the tcp socket being relied upon to perform an unexpected close if the tcp timers time out.