rossmann-engineering / EasyModbusTCP.NET

Modbus TCP, Modbus UDP and Modbus RTU client/server library for .NET implementations
947 stars 411 forks source link

ModbusClient.ReadDiscreteInputs returns a value instead of throwing an exception #18

Closed joshglenn closed 5 years ago

joshglenn commented 5 years ago

result = modbusClient.ReadDiscreteInputs(38 - 1, 1);

When I run this function within 0 to 50 seconds (or so) after connecting, I get an accurate result back. In my case, if the result[0] is true, the machine does one thing, if it is false, it does another.

The problem is, if I let the connection sit idle for 1 minute after the initial connection, and then try it again... I get no errors, just a response of false for that discrete input. This tells the machine: "go ahead, everything is fine"... when, in reality, it is not fine. If I were not debugging this, I would have a major crash causing equipment damage. This function should be throwing an exception instead of returning false (when the PLC input is really true).

I do believe this is a bug.

joshglenn commented 5 years ago

So, I just tested this further... something is timing out at the 60 second mark, causing ReadDiscreteInputs to return false for the given input.

The thing is, it shouldn't return anything. It should throw an exception. This is potentially dangerous for any application of an industrial nature.

Testing EasyModbusTCP ReadDiscreteInputs
Initial result is True.
After waiting 1 seconds, result is True.
After waiting 6 seconds, result is True.
After waiting 11 seconds, result is True.
After waiting 16 seconds, result is True.
After waiting 21 seconds, result is True.
After waiting 26 seconds, result is True.
After waiting 31 seconds, result is True.
After waiting 36 seconds, result is True.
After waiting 41 seconds, result is True.
After waiting 46 seconds, result is True.
After waiting 51 seconds, result is True.
After waiting 56 seconds, result is True.
After waiting 61 seconds, we get the incorrect result, result is False.
After waiting 66 seconds, we get the incorrect result, result is False.
joshglenn commented 5 years ago

I have spoken with the manufacturer of the PLC. They report that their device does not timeout a connection at 60 seconds. If that is true, then wouldn't that mean there's an issue with this function?

rossmann-engineering commented 5 years ago

Hi, I'll check, but I would be surprised if there is a bug. The method has been used hundreds of times by users, and I used it dozens of times. Of course it should throw an exception if the client does not respond, or respond with an error code.

I think you are calling the Method in a loop which is of course your implementation, the library only supports the Method and there is no reason why the Method should work within one minute and then stop working.

Any code of your implementation would be helpful.

joshglenn commented 5 years ago
                modbusClient.Connect();

                bool mainPowerOnInCabinet = modbusClient.ReadDiscreteInputs(16386 - 1, 1)[0];

                Debug.WriteLine("Testing EasyModbusTCP ReadDiscreteInputs");

                Debug.WriteLine("Initial result is " + mainPowerOnInCabinet + ".");
                if (mainPowerOnInCabinet == true) {
                    int waitTime = 1; // seconds to wait
                    while (true)
                    {

                        bool res = modbusClient.ReadDiscreteInputs(16386 - 1, 1)[0];
                        if (res == false)
                        {
                            Debug.WriteLine("After waiting " + waitTime + " seconds, we get the incorrect result, result is " + res + ".");
                        }
                        else {
                            Debug.WriteLine("After waiting " + waitTime + " seconds, result is " + res + ".");
                        }
                        waitTime = waitTime + 5;

                        if (waitTime > 0)
                        {

                            Thread.Sleep(waitTime * 1000);
                        }
                        else {
                            break;
                        }

                    }
                }
rossmann-engineering commented 5 years ago

I checked and it is working as expected. If the connection breaks up we are throwing an exception.

Is it intended that you increase the wait time in every cycle?

joshglenn commented 5 years ago

Hi. Thanks for the response.

Yes, I was gradually increasing the wait time to find out at what point in time the timeout was occuring. After further investigation, I found the source of my 60 second timeout... that is set in the com port settings of my PLC.

However, the issue that I have is still not resolved. So I did some testing.

I started a brand-new Windows forms project, and ran this code:

public EasyModbus.ModbusClient modbusClient;

        public Form1()
        {
            InitializeComponent();

            modbusClient = new ModbusClient("192.168.0.10", 502);
            modbusClient.Connect();

            bool firstResult = modbusClient.ReadDiscreteInputs(16386 - 1, 1)[0];

            Debug.WriteLine("Testing EasyModbusTCP ReadDiscreteInputs");

            Debug.WriteLine("Initial result is " + firstResult + ".");

            Thread.Sleep(61 * 1000); // wait 61 seconds

            bool secondResult = modbusClient.ReadDiscreteInputs(16386 - 1, 1)[0];
            if (secondResult == false)
            {

                Debug.WriteLine("After waiting " + 61 + " seconds, we get the incorrect result, result is " + secondResult + ".");
            }

        }

And these are the results I got:

Testing EasyModbusTCP ReadDiscreteInputs
Initial result is True.
After waiting 61 seconds, we get the incorrect result, result is False.

As far as I know, no exception was thrown at any point, as my application would have presented me with an unhandled exception error. Further, the function returned a value of false, when the condition of the actual input on the PLC was true. So the unsafe condition remains.

Could you try reproducing this problem?