fbarresi / Sharp7

Nuget package for Sharp7
MIT License
208 stars 73 forks source link

Non-negative number required. (Parameter 'length') #19

Closed Schtailian closed 3 years ago

Schtailian commented 3 years ago

Hi, i have a windows service which checks the PLC for data every second. While i do so, i write an integer (can be 0 or 1) in the PLC and sometimes get the following error (max 10 times in 24hours)

E1-1: System.ArgumentOutOfRangeException: Non-negative number required. (Parameter 'length')
   at System.Array.Copy(Array sourceArray, Int32 sourceIndex, Array destinationArray, Int32 destinationIndex, Int32 length, Boolean reliable)
   at Sharp7.S7Client.WriteArea(Int32 Area, Int32 DBNumber, Int32 Start, Int32 Amount, Int32 WordLen, Byte[] Buffer, Int32& BytesWritten)
   at Np300.Service.Utilities.PlcReader.ReadPlcDbBuffer() in C:\Np300.Service\Utilities\PlcReader.cs:line 56
   at Np300.Service.Utilities.PlcReader.ReadPlcDbBuffer() in C:\Np300.Service\Utilities\PlcReader.cs:line 62
   at Np300.Service.Utilities.PlcReader.ReadFromPlc() in C:\Np300.Service\Utilities\PlcReader.cs:line 32
   at Np300.Service.Services.PlcBufferService.WriteEventsFromPlcToBuffer() in C:\Np300.Service\Services\PlcBufferService.cs:line 99

Code

private async Task<byte[]> ReadPlcDbBuffer()
{
    //Irrelevant
    var client = new S7Client();
    client.ConnectTo(_spsOptions.Np300Saw.IpAddress, _spsOptions.Np300Saw.Rack, _spsOptions.Np300Saw.Slot);
    var oldRequestId = readBuffer.GetIntAt(_spsOptions.Np300Saw.RequestId.Position);

    //Relevant
    short requestId = (short)(oldRequestId == 1 ? 0 : 1);
    byte[] writeBuffer = new byte[2];
    writeBuffer.SetIntAt(0, requestId);
    //Error is thrown on the following line
    state = client.DBWrite(_spsOptions.Np300Saw.DbId, _spsOptions.Np300Saw.RequestId.Position, 2, writeBuffer); 

    // Read more from PLC and do stuff...

    client.Disconnect();
}

PLC Variable

Has someone an idea why this happens? Do i forget to dispose something? Should i reuse the client, instead of disconnecting every time?

Thanks Kilian

fbarresi commented 3 years ago

Dear Kilian,

Please apologize my late reply.

I think you have a problem with thread safety. You have to check that only one task is running. I would advise you to use System.Observable and Sharp7Reactive for such a job.

You can try with the following example:


using System;
using System.Reactive.Disposables;
using System.Reactive.Linq;
using System.Reactive.Threading.Tasks;
using System.Threading.Tasks;

public class Program
{
    public async Task Main()
    {
        _spsOptions = new spsOptions();

        using (var subscriptions = new CompositeDisposable())
        {
            var plc = new Sharp7.Rx.Sharp7Plc(_spsOptions.Np300Saw.IpAddress, _spsOptions.Np300Saw.Rack, _spsOptions.Np300Saw.Slot);
            subscriptions.Add(plc);
            await plc.InitializeAsync();

            await plc.ConnectionState.FirstAsync(s => s == Sharp7.Rx.Enums.ConnectionState.Connected).ToTask();

            var subscription = Observable.Interval(TimeSpan.FromSeconds(1))
                    .Select(l => (int) l % 2)
                    .Do(async i =>
                        await plc.SetValue(
                            $"DB{_spsOptions.Np300Saw.DbId}.int{_spsOptions.Np300Saw.RequestId.Position}", i))
                    .Subscribe()
                ;

            subscriptions.Add(subscription);

            Console.ReadLine();
        }
    }
}
fbarresi commented 3 years ago

And yes you also should:

OR (as I told you)

Schtailian commented 3 years ago

Hi Federico, thanks for your time.

I don't think that thread safety is the issue in my case. The program is basically one endless while loop which calls ReadPlcDbBuffer(), i do never create a thread. Some async functions might cause the problem, although i am awaiting every function?

That being said, i do now reuse the client and do not connect and disconnect on every iteration. That seems to have fixed the problem for the past 24h.

Thanks

fbarresi commented 3 years ago

Dear Kilian,

Please apologize (again) my late reply.

Well even if you don't have any thread every task would run in a background thread. Even if you await them all it doesn't means they don't run parallel.

Reuse the same connected client is always a good practice. I now think that was your real problem combined with no waiting for a connection fully established (At the beginning server and client have to handle in a couple of message with wich rate the communication would be).

Sadly Is Sharp7Reactive not so well documented, but also not so difficult to understand. It uses a really common way to describes variables in the plc world.

I will now close this issue, but If you still have some questions feel free to write here or reopen this issue.