Apollo3zehn / FluentModbus

Lightweight and fast client and server implementation of the Modbus protocol (TCP/RTU).
MIT License
189 stars 71 forks source link

Connect-Query-Disconnect vs. persistent connection #44

Closed Adhara3 closed 3 years ago

Adhara3 commented 3 years ago

Hi,

your library is very useful and well written, so thank you.

I need to constantly read from a modbus device, let's say every second or so. The Modus protocol does not provide any mechanism to check server status (a sort of ping/heartbeat) so I was wondering what is the best way to approach this.

  1. Open connection, read registers and close connection each second
  2. Keep the connection open and read data every second, try reconnect if can't read.

I saw that you rely on the TcpCLient.Connected which may be unreliable. What is your suggestion? How would you proceed?

Thanks a lot A

Apollo3zehn commented 3 years ago

Yes, unfortunately the Connected status is unreliable. I think this is because TCP also does not have any heartbeat mechanism. Instead, the connection status is updated either if a packet is received or a timeout occured.

Personally, I would choose option 2. I do not see any reason why to constantly close and reopen the connection and I have often seen and also implemented approaches similar to option 2 instead of option 1.

Here is an example of the SignalR heartbeat implementation:

  1. ping timer https://github.com/dotnet/aspnetcore/blob/f490dd24c924e32acb46d0dee6b838237172d5e8/src/SignalR/common/Http.Connections/src/Internal/HttpConnectionManager.cs#L143-L176

  2. ping callback registration https://github.com/dotnet/aspnetcore/blob/8c8cb4dce235b099d1e419b6a039217eb56a77e6/src/SignalR/server/Core/src/HubConnectionContext.cs#L522

  3. intermediate calls (skipped)

  4. send actual ping message https://github.com/dotnet/aspnetcore/blob/cfc43b38e1d1c205fdac2cf15b994651f9520590/src/SignalR/server/Core/src/HubConnectionContext.cs#L354-L377

Adhara3 commented 3 years ago

Thank you for the reply. I also prefer solution 2. Since Modbus itself does not provide any ping mechanism, I will probably read a single register as a fake ping .

Thanks again! A

Adhara3 commented 3 years ago

Hi,

I found a bizarre behavior with the client. Imagine the following code being called every T seconds.

if (m_client.IsConnected == false)
{
    try
    {
        m_client.Connect(m_ipEndPoint);
    }
    catch (Exception e)
    {
        // Failed to connect
        m_client.Disconnect();
    }
}

The server is down. The first time this code is called, IsConnected is false, tries to connect, fails. The next round when checking for IsConnected 'm_client.IsConnected' threw an exception of type 'System.NullReferenceException'

which is very weird, since the code public bool IsConnected => _tcpClient?.Connected ?? false;

This also means that the client cannot be reused, because if the server comes up, I can-t connect anymore. Which is the right approach here? Is this expected? Should I create a new client when connection fails at least once?

Thanks a lot A

Apollo3zehn commented 3 years ago

I have tried the following, whichs works fine:

var client = new ModbusTcpClient();

while (true)
{
    if (client.IsConnected == false)
    {
        try
        {
            client.Connect();
        }
        catch (Exception e)
        {
            // Failed to connect
            client.Disconnect();
        }
    }

    await Task.Delay(5000);
}

What do I need to do to reproduce your error? :)

Adhara3 commented 3 years ago

Hi,

The problem is only when the library is used in a .NET Framework project. When you call client.Disconnect() you internally close the tcpClient but do not set it to null. This object, once closed, is unusable because the internal clientsockect is null. Thus, this checks fails public bool IsConnected => _tcpClient?.Connected ?? false; because _tcpClient is not null so the connected property is called and internally it does public bool Connected => this.m_ClientSocket.Connected; but m_clientSocket is null so it throws.

In a .NETCore console app this won't happen (and I guess this is why it is working for you) because the System.Socket in this case is behaving in a different (correct) way public bool Connected => _clientSocket?.Connected ?? false;

A

Apollo3zehn commented 3 years ago

I was able to reproduce that error with .NET Framework 4.8 and added a small fix. Hope this helps.