S7NetPlus / s7netplus

S7.NET+ -- A .NET library to connect to Siemens Step7 devices
MIT License
1.33k stars 588 forks source link

ReadTimeout is always 20s #427

Open jonuriarte opened 2 years ago

jonuriarte commented 2 years ago

Hi,

I connect to a plc S7-1500 or S7-400 without a problem. I can read data from a DB using ReadBytes, ReadVar or ReadStruct. I'm trying to detect when there is a problem with the connection. I would like to set read and write timeouts to something like 1000ms, so that when I unplug the plc network cable (simulating a problem) the reading function does not get waiting and returns an error/exception in 1000ms. I've set a timer that calls ReadBytes function every 1s. I've tried changing the ReadTimeout and WriteTimeout properties:

 plc = new Plc(CpuType.S71500, textBoxIP.Text, 0, 0);
 plc.ReadTimeout = 1000;
 plc.WriteTimeout = 1000;
 plc.Open();

It doesn't matter which value I set, it always timeouts 20s after executing ReadBytes (for example):

var bytes = plc.ReadBytes(DataType.DataBlock, 245, 0, 500);

I'm using v0.13.0.0 from NuGet. I've tried with v0.12.0.0 too. Windows 10 Pro 20H2 Visual Studio 2017

Thank you.

PD: Edited

scamille commented 2 years ago

I suspect that you get a timeout while trying to establishing a connection, not while actually reading/writing data over the established connection, for which those settings apply.

I understand that this might not clearly visible from the outside, but that is how it works.

If you use net5+ you can try using the async functions with a cancdllationtoken that implements a custom timeout. That should work - maybe.

jonuriarte commented 2 years ago

Oh, now that I've read my message I can see that I didn't explain my issue...I'm sorry, I've edited the message.

I can connect to a plc and I can read and write data. The problem is when I want to simulate a network failure by unpluging the plc network cable. I would like the timeout to be around 1000ms (if it could be lower...).

scamille commented 2 years ago

Hmm yeah looking at https://docs.microsoft.com/en-us/dotnet/api/system.net.sockets.tcpclient.receivetimeout?view=net-6.0 I think the Timeouts only apply if the synchronous Read/Write methods are used. Since S7NetPlus switched to wrapping async functions for the sync functions under the hood, we inadvertedly broke those Timeout values.

If you want a fix you can apply yourself, I would definitely recommend using the Async functions (eg. https://github.com/S7NetPlus/s7netplus/blob/b475aee2e768f709e6b8f01084b176e53d43cd8c/S7.Net/PlcAsynchronous.cs#L105) and add your custom timeout. You could use Polly to do that for you, see https://github.com/App-vNext/Polly#timeout or google for it.

jonuriarte commented 2 years ago

Thanks for the answer. So the timeouts are not working as I expected. Do you have any example code using Async functions?

jonuriarte commented 2 years ago

I have a problem using Polly. Can't make the Optimistic Timeout work (the one that should co-operate with the delegated method in cancellation using a cancellation token).I've tried the following code:

This code configures the timeout policy:

private static Polly.Timeout.AsyncTimeoutPolicy GetTimeoutPolicy
                    => Policy.TimeoutAsync(
                            TimeSpan.FromMilliseconds(2000),
                            Polly.Timeout.TimeoutStrategy.Optimistic,
                            (context, timeout, _, exception) =>
                            {
                                Console.WriteLine($"{"Timeout",-10}{timeout,-10:ss\\.fff}: {exception.GetType().Name}");
                                return Task.CompletedTask;
                            });

This code executes the delegated method with Polly:

try
{
    plcA1 = new Plc(CpuType.S71500, "10.1.21.10", 0, 0);
    var strategy = GetTimeoutPolicy;
    await strategy.ExecuteAsync(async (ct) => await SomeMethodAsync(ct), CancellationToken.None);
}
catch (OperationCanceledException)
{
    // everything is good, that is the exception we expect
    Console.WriteLine("Operation was cancelled as expected.");
    return;
}
catch (Exception ex)
{
    Console.WriteLine(ex.Message);
}

This code is the delegated method:

private async Task SomeMethodAsync(CancellationToken ct = default)
{
    Console.WriteLine($"{nameof(SomeMethodAsync)} has been called.");
    //await Task.Delay(15000, ct); //It is aware of the CancellationToken
    await plcA1.OpenAsync(ct);
    Console.WriteLine($"{nameof(SomeMethodAsync)} has finished.");
}

If I comment the OpenAsync line and uncomment the Task.Delay line, everything works as expected. Polly throws a timeout exception 2 seconds after the code is executed. But if I executed the code as is pasted above, there is no timeout and S7NetPlus throws an exception 20s later.

If I configure Polly policy to use a Pessimistic Timeout (executing Task.Delay or OpenAsync), Polly timeouts after 2s BUT the OpenAsync function is not cancelled and throws an exception after 20s.

I've tried opening the connection first and executing await plcA1.ReadBytesAsync(DataType.DataBlock, 245, 0, 500, ct); in the delegated method instead of Task.Delay and OpenAsync, but the result is the same.

Are the async functions in S7NetPlus working with cancellation token? Could you share an example using async functions (OpenAsync, ReadBytesAsync, WriteBytesAsync)?

Thank you.

scamille commented 2 years ago

Ah damn it I got confused a bit between S7NetPlus and Sally7, the latter which got conditional cancellation support for Opening a tcp connection just recently, see https://github.com/mycroes/Sally7/blob/52fa77ea93b4774d84689d148010184a6b2ff3dd/Sally7/S7Connection.cs#L105.

S7NetPlus currently does not support a proper timeout on Opening the connection I fear.

Altought if you try the optimistic timeout on ReadBytesAsync after you already established a connection, it should work. In the end all of this relies on .NET networking functions doing the proper optimistic timeout when cancellation is requested, which I don't know if that always works or not.

gfoidl commented 2 years ago

S7NetPlus currently does not support a proper timeout on Opening the connection I fear.

https://github.com/S7NetPlus/s7netplus/issues/422 enables this via cancellationtoken (which could be created from a CancellationTokenSource that times out).

jonuriarte commented 2 years ago

Altought if you try the optimistic timeout on ReadBytesAsync after you already established a connection, it should work. In the end all of this relies on .NET networking functions doing the proper optimistic timeout when cancellation is requested, which I don't know if that always works or not.

I've already tried that, but with Optimistic timeout the delegated method does not timeout. I tried it using the code I posted above and this function:

private async Task SomeMethodAsync(CancellationToken ct = default)

{
    Console.WriteLine($"{nameof(SomeMethodAsync)} has been called.");
    await plcA1.ReadBytesAsync(DataType.DataBlock, 245, 0, 500, ct);
    Console.WriteLine($"{nameof(SomeMethodAsync)} has finished.");
}

I'm using .Net Framework 4.7.1, don't know if that could be the problem.

mycroes commented 6 months ago

I guess this answer is a bit late, but there's a workaround that seems to work on about every framework, it might lead to different exceptions on different target frameworks though.

In .NET versions where cooperative cancellation is not available you can still Close() the TCP connection to get an immediate finish of Open/OpenAsync. Currently this isn't supported by S7NetPlus, because the Plc.Close() method first checks for TcpClient.Connected before calling close on the actual TcpClient. A quick hack would be to get the private tcpClient field from Plc and directly call Close on it after a timeout.

A quick (untested) sample to achieve this is as follows:

public async Task OpenWithCancellation(Plc plc, CancellationToken cancellationToken)
{
    var tcpClient = (TcpClient) typeof(Plc).GetField(System.Reflection.BindingFlags.Instance | System.Reflection.BindingFlags.NonPublic).GetValue(plc);
    using (cancellationToken.Register(_ => tcpClient.Close(), null)) {
        await plc.OpenAsync();
    }
}

Let me know if this is still an issue you're trying to solve and if the above works for you.

jk987 commented 1 month ago

I have wrapper like this:

        private void Timeout(Action procedure)
        {
            Task task = Task.Factory.StartNew(() => procedure());
            if (!task.Wait(1000))
            {
                throw new TimeoutException("Not finished in time, presume broken!");
            }
        }

...

        Timeout(() => plcConnection.Open());
...
        Timeout(() => plcConnection.ReadMultipleVars(dataItems));
ynioba commented 1 month ago

这是来自”麦冬@广西机械工业研究院”的邮箱自动回复邮件。您好,您的邮件已收到,我查阅后尽快回复您。