ModbusTcpClient doesn't do reconnects?

Versions

Python: N/A
OS: N/A
Pymodbus: 3.7.2
Modbus Hardware (if used): N/A

Pymodbus Specific

Client: tcp - sync

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.

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.

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

This is specifically for the sync client.

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

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

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

Ah, yes, I think I see -

pymodbus/pymodbus/client/base.py

Line 235 in aa7e556

if not self.connect():

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.

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'.