search

pymodbus-dev / pymodbus

A full modbus protocol written in python
Other
2.24k stars 915 forks source link

ModbusTcpClient doesn't do reconnects? #2320

Open camtarn opened 1 week ago

camtarn commented 1 week ago

Versions

Pymodbus Specific

Description

On: https://github.com/pymodbus-dev/pymodbus/blob/dcbcfd7f16fb3be82525bccf9699855a06f900c5/pymodbus/client/tcp.py#L129C1-L129C74

there is the comment: Remark: There are no automatic reconnect as with AsyncModbusTcpClient

Is this true? The docs say:

client = ModbusTcpClient('MyDevice.lan')   # Create client object
client.connect()                           # connect to device, reconnect automatically

which seems to contradict this.

This also suggests that this part of the docstring for ModbusTcpClient might be incorrect, if there is no automatic reconnection:

    .. tip::
        **reconnect_delay** doubles automatically with each unsuccessful connect, from
        **reconnect_delay** to **reconnect_delay_max**.
        Set `reconnect_delay=0` to avoid automatic reconnection.

Not a problem if it's not supported, I was just surprised to find this.

janiversen commented 1 week ago

the async client do automatic reconnect, wh6 do you think there are no automatic reconnect?

If you think there is a bug, then please add a debug log so we can see what happens.

janiversen commented 1 week ago

Of course it seems to be a wrong documentation, 8n that case a pull request is welcome.

camtarn commented 1 week ago

This is specifically for the sync client.

Happy to make a pull request if you confirm the sync client also does reconnects.

janiversen commented 1 week ago

The sync client do not do automatic reconnect, as far as I remember.

janiversen commented 1 week ago

sync uses a different approach, it connects if a request is sent and it's not connected.

camtarn commented 6 days ago

Ah, yes, I think I see - https://github.com/pymodbus-dev/pymodbus/blob/aa7e556cb53c4270c4e0b74b771ba430332c62e8/pymodbus/client/base.py#L235 causes the client to try connecting, which returns immediately if it's already connected, or kicks off a new connection.

I'll see if there's a better way to word the documentation.

camtarn commented 6 days ago

I've found a different way to phrase the docs.

However, it seems like it would make sense to remove the reconnect_delay/reconnect_delay_max params from the sync clients, since they don't use them. I'm guessing that removing those would be an API change? For the moment I can flag them as 'not used in the sync client'.