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

AdsSymbolicServer: please check OnWriteRawValue calls #39

Closed beppemarazzi closed 1 year ago

beppemarazzi commented 1 year ago

I'm facing strange problems testing with AdsSymbolicServer (from <PackageReference Include="Beckhoff.TwinCAT.Ads.SymbolicServer" Version="6.0.216" />). I tried to activate immediate notifications changing line https://github.com/Beckhoff/TF6000_ADS_DOTNET_V5_Samples/blob/2325825ba7ad3f55e6c5b9273b9bf88be9d8b76d/Sources/ServerSamples/AdsSymbolicServerSample/Program.cs#L208 in the provided sample with this: IDisposable subscription = SubscribeNotifications(session, symbols, NotificationSettings.ImmediatelyOnChange); and i got strange unexpected behaviours.

So i did some debugging and i realized that the function https://github.com/Beckhoff/TF6000_ADS_DOTNET_V5_Samples/blob/2325825ba7ad3f55e6c5b9273b9bf88be9d8b76d/Sources/ServerSamples/AdsSymbolicServerSample/AdsSymbolicServer.cs#L383-L390 was unexpectedly called during the notification process... Digging into details with the help of decompiled code of the library Beckhoff.TwinCAT.Ads.SymbolicServer i found 3 point where OnWriteRawValue is obviously called by error instead of OnReadRawValue (they both have compatible signatures so the compiler doesn't catch this!)

  1. into FireNotificationAsync function:

        protected async Task<AdsErrorCode> FireNotificationAsync(AmsAddress address, uint handle, CancellationToken cancel)
        {
            if (!base.IsConnected)
            {
                throw new ServerNotConnectedException();
            }
    
            ISymbol symbol = notificationTable!.LookupSymbol(handle);
            if (symbol != null && HasValueChanged(symbol))
            {
                int valueMarshalSize = symbol.GetValueMarshalSize();
                byte[] array = new byte[valueMarshalSize];
                OnWriteRawValue(symbol, array.AsSpan());  //***************** WHY????? IS'NT OnReadRawValue ?????
                NotificationDataSample[] stamps = new NotificationDataSample[1]
                {
                    new NotificationDataSample(handle, array)
                };
                return await OnFireNotificationsAsync(address, stamps, cancel).ConfigureAwait(continueOnCapturedContext: false);
            }
    
            return AdsErrorCode.None;
        }
  2. into createNotificationInformation function:

        private IDictionary<AmsAddress, List<(uint, ISymbol, Memory<byte>, NotificationSettings)>> createNotificationInformation(IEnumerable<ISymbol> symbols)
        {
            bool immediateAccess = true;
            Dictionary<AmsAddress, List<(uint, ISymbol, Memory<byte>, NotificationSettings)>> dictionary = new Dictionary<AmsAddress, List<(uint, ISymbol, Memory<byte>, NotificationSettings)>>();
            foreach (ISymbol symbol in symbols)
            {
                IList<(AmsAddress, uint, NotificationSettings)> notificationInfo = notificationTable!.GetNotificationInfo(symbol, immediateAccess);
                foreach (var item in notificationInfo)
                {
                    if (!dictionary.ContainsKey(item.Item1))
                    {
                        dictionary.Add(item.Item1, new List<(uint, ISymbol, Memory<byte>, NotificationSettings)>());
                    }
    
                    byte[] array = new byte[symbol.GetValueMarshalSize()];
                    OnWriteRawValue(symbol, array.AsSpan()); //***************** WHY????? IS'NT OnReadRawValue ?????
                    dictionary[item.Item1].Add((item.Item2, symbol, array.AsMemory(), item.Item3));
                }
            }
    
            return dictionary;
        }
  3. into OnReadWriteValueByNameAsync function:

    ...
               else if (readLength >= symbol.GetValueMarshalSize())
                {
                    byte[] array3 = new byte[symbol.GetValueMarshalSize()];
                    AdsErrorCode errorCode2 = OnWriteRawValue(symbol, array3.AsSpan());
                    result = ((!errorCode2.Succeeded()) ? ResultReadWriteBytes.CreateError(errorCode2, invokeId) : ResultReadWriteBytes.CreateSuccess(array3, invokeId));
                }
    ...

Please check and fix the AdsSymbolicServer implementation.

Side note: i found many other (probably minor?) glitches and issues with your .NET libraries. (i.e. wrong/inconsistent calculations in padding and packing of user defined marshaled data structures). I was able to workaround many of this issues, with the help of various decompilers... but if the source code of the libraries had been available it would have been easier to find and fix issues... Why don't you consider to open the source and share it on github???? You may also receive a lot of PR to fix and improve your product!!!!

beppemarazzi commented 1 year ago

@RalfHeitmann any chance to get this fixed???

RalfHeitmann commented 1 year ago

Sorry for the delay ... I have produced new 6.0.235 packages that should solve these issues.

beppemarazzi commented 1 year ago

Many thanks