Beckhoff / TF6000_ADS_DOTNET_V5_Samples

Sample code for the Version 6.X series of the TwinCAT ADS .NET Packages
https://infosys.beckhoff.com/content/1033/tc3_ads.net/9407515403.html?id=6770980177009971601
BSD Zero Clause License
37 stars 15 forks source link

"NoError" returned when "WriteAsync" to disconnected server #53

Closed jonev-akva closed 7 months ago

jonev-akva commented 8 months ago

Hi, I am using the AmsTcpIpRouter, AdsServer and AdsClient in my project and i am experiencing some strange behavior. I have created an xunit project to reproduce the behavior;

This is my csproj;

<Project Sdk="Microsoft.NET.Sdk">

  <PropertyGroup>
    <TargetFramework>net7.0</TargetFramework>
    <ImplicitUsings>enable</ImplicitUsings>
    <Nullable>enable</Nullable>

    <IsPackable>false</IsPackable>
  </PropertyGroup>

  <ItemGroup>
    <PackageReference Include="Beckhoff.TwinCAT.Ads" Version="6.1.154" />
    <PackageReference Include="Beckhoff.TwinCAT.Ads.Server" Version="6.1.154" />
    <PackageReference Include="Beckhoff.TwinCAT.Ads.TcpRouter" Version="6.1.154" />
    <PackageReference Include="Divergic.Logging.Xunit" Version="4.3.0" />

    <PackageReference Include="Microsoft.NET.Test.Sdk" Version="17.3.2" />
    <PackageReference Include="xunit" Version="2.4.2" />
    <PackageReference Include="xunit.runner.visualstudio" Version="2.4.5">
      <IncludeAssets>runtime; build; native; contentfiles; analyzers; buildtransitive</IncludeAssets>
      <PrivateAssets>all</PrivateAssets>
    </PackageReference>
    <PackageReference Include="coverlet.collector" Version="3.1.2">
      <IncludeAssets>runtime; build; native; contentfiles; analyzers; buildtransitive</IncludeAssets>
      <PrivateAssets>all</PrivateAssets>
    </PackageReference>
  </ItemGroup>

</Project>

A minimal implementation of the AdsServer;

public class TwincatAdsTestServer : AdsServer
{
    public TwincatAdsTestServer(ushort portNumber, string portName)
        : base(portNumber, portName)
    {
    }

    public string? LastReceivedData { get; private set; }

    public string? LastReceivedExternalSensorData { get; private set; }

    public void ClearReceivedData()
    {
        this.LastReceivedData = null;
        this.LastReceivedExternalSensorData = null;
    }

    public async Task WaitForDataAsync(TimeSpan timeout)
    {
        var start = DateTime.Now;
        while (this.LastReceivedData == null)
        {
            if (start.Add(timeout) < DateTime.Now)
                throw new TimeoutException($"Failed to await receiving data in {nameof(TwincatAdsTestServer)}");

            await Task.Delay(10).ConfigureAwait(false);
        }
    }

    public async Task ConnectServerWithRetryAsync(TimeSpan timeout, CancellationToken cancellationToken)
    {
        var start = DateTime.Now;
        while (!this.IsConnected && !cancellationToken.IsCancellationRequested)
        {
            try
            {
                var amsServerPort = this.ConnectServer();
            }
            catch (Exception e)
            {
                if (start.Add(timeout) < DateTime.Now)
                    throw new TimeoutException("Failed to await connect server", e);

                await Task.Delay(50, cancellationToken).ConfigureAwait(false);
            }
        }
    }

    protected override Task<ResultWrite> OnWriteAsync(
        AmsAddress target,
        uint invokeId,
        uint indexGroup,
        uint indexOffset,
        ReadOnlyMemory<byte> writeData,
        CancellationToken cancel)
    {
        this.LastReceivedData = Encoding.Default.GetString(writeData.Span.ToArray()).Trim('\0');

        if (indexOffset == 3)
        {
            this.LastReceivedExternalSensorData = this.LastReceivedData;
        }

        return Task.FromResult(ResultWrite.CreateError(AdsErrorCode.NoError, invokeId));
    }
}

And the code reproducing the behavior;

var cts = new CancellationTokenSource(TimeSpan.FromMinutes(1));
// Setting up router
using var routerLogger = _output.BuildLoggerFor<AmsTcpIpRouter>();
var router = new AmsTcpIpRouter(new AmsNetId("10.10.10.10.1.1"));
var routerTask = router.StartAsync(cts.Token);
await Task.Delay(1000);

try
{
    // Starting server and connecting to router
    var adsServer = new TwincatAdsTestServer(
        45086, "TestServer");
    await adsServer.ConnectServerWithRetryAsync(TimeSpan.FromSeconds(5), default);

    // Initializing client
    using var adsClientLogger = _output.BuildLoggerFor<AmsTcpIpRouter>();
    using var adsClient = new AdsClient(adsClientLogger);
    adsClient.Connect(new AmsAddress("10.10.10.10.1.1", 45086));

    // 1. Sending message to server OK
    _output.WriteLine("----- First message -----");
    var payload = "First message";
    var writeResult = await adsClient.WriteAsync(
            0,
            1,
            Encoding.Default.GetBytes(payload + "\0"),
            cts.Token)
        .ConfigureAwait(false);
    Assert.True(writeResult.Succeeded);
    Assert.False(writeResult.Failed);
    await adsServer.WaitForDataAsync(TimeSpan.FromSeconds(5));
    Assert.Equal(payload, adsServer.LastReceivedData);
    _output.WriteLine("----------");

    // 2. Simulating server goes down
    _output.WriteLine("----- Ads server disconnect -----");
    var disconnected = adsServer.Disconnect();
    _output.WriteLine($"----Disconnected; {disconnected}------");

    // 3. Sending message to server
    _output.WriteLine("----- Second message -----");
    payload = "Second message";
    writeResult = await adsClient.WriteAsync(
            0,
            1,
            Encoding.Default.GetBytes(payload + "\0"),
            cts.Token)
        .ConfigureAwait(false);
    _output.WriteLine($"Write status; {writeResult.ErrorCode}");
    Assert.False(writeResult.Succeeded);
    Assert.True(writeResult.Failed);
    Assert.Equal(AdsErrorCode.TargetPortNotFound, writeResult.ErrorCode);
    _output.WriteLine("----------");
}
finally
{
    router.Stop();
    await routerTask;
}

The code executes the following scenario

In most executions the last sending returns "NoError" but in some it returns "TargetPortNotFound" and failed. I guess "TargetPortNotFound" is correct? Or should it return "NoError"?

Any help is appreciated.

jonev-akva commented 7 months ago

Full project and unit test to reproduce this can be found at this repository.

RalfHeitmann commented 7 months ago

Thx for sending the UnitTest Code and sorry about the long response time. But we are quite busy here to release TwinCAT 4026 at the moment ... With your Tests, the problem returning "NoError" instead of "TargetPortNotFound" error could be located and fixed easily. See the upcoming (hopefully a few hours) next version 6.1.197 or 6.0.356.

RalfHeitmann commented 7 months ago

In addition, I can see a further problem causing problems that comes from the AdsClient configuration in your UnitTest Example. TargetPortNotFound is what we call a 'tripping error' that indicates communication problems to an ADS Server. The AdsClient has a mechanism that detects these errors and limits communication to targets indicating these sort of errors for client application robustness by default - what means, that the AdsClient blocks communication (with always the same error) for the next 21 Seconds. You have to switch this feature of, if you want to proceed immediatly in your UnitTests (see also https://infosys.beckhoff.com/content/1033/tc3_ads.net/9407915787.html?id=7609512520970210). You should initialize your AdsClient with switched of FailFastHandler and a higher timeout like this:

            using var adsClient = new AdsClient(null,AdsClientSettings.FastWriteThrough, adsClientLogger);
            adsClient.Timeout = 60000;

to remove the not wanted side-effects.

jonev-akva commented 7 months ago

Hi @RalfHeitmann, thank you, the new version and the settings you recommends fixed our problem! I can see from the release notes on version 6.1.197 that dependencies are replaced with .NET 8 versions. But from the requirements in the readme it does not look like you are supporting .NET 8 yet? Due to upcoming end of support on .NET 6 (LTS - May 2024) we would like bump our project to .NET 8, do you have any plan to official support this version soon?

RalfHeitmann commented 7 months ago

We simply missed to update the Readmes (we support .NET 8!) .NET7 is out-of-support soon (on the Microsoft side, not .NET 6 as you have stated) and so in the ADS 6.1.XX versions we replaced the Dependency from .NET7 to .NET 8. For users of the Packages, this shouldn't change something significantly. The new and the old packages are functionally equal, it doesn't matter which .NET Framework you are using. Even in case you are still using .NET7 in your application, the package Version 6.1.197 dependency will fall back to the .NET 6 version - instead of using .NET 7 dependency packages in older variants - and with no changes of functionality or behaviour. I don't see any issue here.