From 6c5b20291035ee59348d10a2ba405b27414ad8cf Mon Sep 17 00:00:00 2001 From: Joseph Doherty Date: Sun, 19 Apr 2026 18:49:48 -0400 Subject: [PATCH] =?UTF-8?q?TwinCAT=20follow-up=20=E2=80=94=20Native=20ADS?= =?UTF-8?q?=20notifications=20for=20ISubscribable.=20Closes=20task=20#189?= =?UTF-8?q?=20=E2=80=94=20upgrades=20TwinCATDriver's=20subscription=20path?= =?UTF-8?q?=20from=20polling=20(shared=20PollGroupEngine)=20to=20native=20?= =?UTF-8?q?AdsClient.AddDeviceNotificationExAsync=20so=20the=20PLC=20pushe?= =?UTF-8?q?s=20changes=20on=20its=20own=20cycle=20rather=20than=20the=20dr?= =?UTF-8?q?iver=20polling.=20Strictly=20better=20for=20latency=20+=20CPU?= =?UTF-8?q?=20=E2=80=94=20TC2=20and=20TC3=20runtimes=20notify=20on=20value?= =?UTF-8?q?=20change=20with=20sub-millisecond=20latency=20from=20the=20PLC?= =?UTF-8?q?=20cycle.=20ITwinCATClient=20gains=20AddNotificationAsync=20?= =?UTF-8?q?=E2=80=94=20takes=20symbolPath=20+=20TwinCATDataType=20+=20opti?= =?UTF-8?q?onal=20bitIndex=20+=20cycleTime=20+=20onChange=20callback=20+?= =?UTF-8?q?=20CancellationToken;=20returns=20an=20ITwinCATNotificationHand?= =?UTF-8?q?le=20whose=20Dispose=20tears=20the=20notification=20down=20on?= =?UTF-8?q?=20the=20wire.=20Bit-within-word=20reads=20supported=20?= =?UTF-8?q?=E2=80=94=20the=20parent=20word=20value=20arrives=20via=20the?= =?UTF-8?q?=20notification,=20driver=20extracts=20the=20bit=20before=20inv?= =?UTF-8?q?oking=20the=20callback=20(same=20ExtractBit=20path=20as=20the?= =?UTF-8?q?=20read=20surface=20from=20PR=202).=20AdsTwinCATClient=20?= =?UTF-8?q?=E2=80=94=20subscribes=20to=20AdsClient.AdsNotificationEx=20in?= =?UTF-8?q?=20the=20ctor,=20maintains=20a=20ConcurrentDictionary=20keyed=20on=20the=20server-side=20not?= =?UTF-8?q?ification=20handle.=20AddDeviceNotificationExAsync=20returns=20?= =?UTF-8?q?Task=20with=20Handle=20+=20ErrorCode;=20non-NoErr?= =?UTF-8?q?or=20throws=20InvalidOperationException=20so=20the=20driver=20c?= =?UTF-8?q?an=20catch=20+=20retry.=20Notification=20event=20args=20carry?= =?UTF-8?q?=20Handle=20+=20Value=20+=20DataType;=20lookup=20in=20=5Fnotifi?= =?UTF-8?q?cations=20dict=20routes=20the=20value=20through=20any=20bit-ext?= =?UTF-8?q?raction=20+=20calls=20the=20consumer=20callback.=20Consumer-sid?= =?UTF-8?q?e=20exceptions=20are=20swallowed=20so=20a=20misbehaving=20callb?= =?UTF-8?q?ack=20can't=20crash=20the=20ADS=20notification=20thread.=20Disp?= =?UTF-8?q?ose=20unsubscribes=20from=20AdsNotificationEx=20+=20clears=20th?= =?UTF-8?q?e=20dict=20+=20disposes=20AdsClient.=20NotificationRegistration?= =?UTF-8?q?=20is=20ITwinCATNotificationHandle=20=E2=80=94=20Dispose=20fire?= =?UTF-8?q?s=20DeleteDeviceNotificationAsync=20as=20fire-and-forget=20with?= =?UTF-8?q?=20CancellationToken.None=20(caller=20has=20already=20committed?= =?UTF-8?q?=20to=20teardown;=20blocking=20would=20slow=20shutdown).=20Twin?= =?UTF-8?q?CATDriverOptions.UseNativeNotifications=20=E2=80=94=20new=20boo?= =?UTF-8?q?l,=20default=20true.=20When=20true=20the=20driver=20uses=20nati?= =?UTF-8?q?ve=20notifications;=20when=20false=20it=20falls=20through=20to?= =?UTF-8?q?=20the=20shared=20PollGroupEngine=20(same=20semantics=20as=20ot?= =?UTF-8?q?her=20libplctag-backed=20drivers,=20also=20a=20safety=20valve?= =?UTF-8?q?=20for=20targets=20with=20notification=20limits).=20TwinCATDriv?= =?UTF-8?q?er.SubscribeAsync=20dual-path=20=E2=80=94=20if=20UseNativeNotif?= =?UTF-8?q?ications=20false=20delegate=20into=20=5Fpoll.Subscribe=20(uncha?= =?UTF-8?q?nged=20behavior=20from=20PR=203).=20If=20true,=20iterate=20full?= =?UTF-8?q?References,=20resolve=20each=20to=20its=20device's=20client=20v?= =?UTF-8?q?ia=20EnsureConnectedAsync=20(reuses=20PR=202's=20per-device=20c?= =?UTF-8?q?onnection=20cache),=20parse=20the=20SymbolPath=20via=20TwinCATS?= =?UTF-8?q?ymbolPath=20(preserves=20bit-in-word=20support),=20call=20ITwin?= =?UTF-8?q?CATClient.AddNotificationAsync=20with=20a=20closure=20over=20th?= =?UTF-8?q?e=20FullReference=20(not=20the=20ADS=20symbol=20=E2=80=94=20OPC?= =?UTF-8?q?=20UA=20subscribers=20addressed=20the=20driver-side=20name).=20?= =?UTF-8?q?Per-registration=20callback=20bridges=20(=5F,=20value)=20?= =?UTF-8?q?=E2=86=92=20OnDataChange=20event=20with=20a=20fresh=20DataValue?= =?UTF-8?q?Snapshot=20(Good=20status,=20current=20UtcNow=20timestamps).=20?= =?UTF-8?q?Any=20mid-registration=20failure=20triggers=20a=20try/catch=20t?= =?UTF-8?q?hat=20disposes=20every=20already-registered=20handle=20before?= =?UTF-8?q?=20rethrowing,=20keeping=20the=20driver=20in=20a=20clean=20neve?= =?UTF-8?q?r-existed=20state=20rather=20than=20half-registered.=20Unsubscr?= =?UTF-8?q?ibeAsync=20dispatches=20on=20handle=20type=20=E2=80=94=20Native?= =?UTF-8?q?SubscriptionHandle=20disposes=20all=20its=20cached=20ITwinCATNo?= =?UTF-8?q?tificationHandles;=20anything=20else=20delegates=20to=20=5Fpoll?= =?UTF-8?q?.Unsubscribe=20for=20the=20poll=20fallback.=20ShutdownAsync=20t?= =?UTF-8?q?ears=20down=20native=20subs=20first=20(so=20AdsClient-level=20c?= =?UTF-8?q?leanup=20happens=20before=20the=20client=20itself=20disposes),?= =?UTF-8?q?=20then=20PollGroupEngine,=20then=20per-device=20probe=20CTS=20?= =?UTF-8?q?+=20client.=20NativeSubscriptionHandle=20DiagnosticId=20prefixe?= =?UTF-8?q?s=20with=20twincat-native-sub-=20so=20Admin=20UI=20+=20logs=20c?= =?UTF-8?q?an=20distinguish=20the=20paths.=209=20new=20unit=20tests=20in?= =?UTF-8?q?=20TwinCATNativeNotificationTests=20=E2=80=94=20native=20subscr?= =?UTF-8?q?ibe=20registers=20one=20notification=20per=20tag,=20pushed=20va?= =?UTF-8?q?lue=20via=20FireNotification=20fires=20OnDataChange=20with=20th?= =?UTF-8?q?e=20right=20FullReference=20(driver-side,=20not=20ADS=20symbol)?= =?UTF-8?q?,=20unsubscribe=20disposes=20all=20notifications,=20unsubscribe?= =?UTF-8?q?=20halts=20future=20notifications,=20partial-failure=20cleanup?= =?UTF-8?q?=20via=20FailAfterNAddsFake=20(first=20succeeds,=20second=20thr?= =?UTF-8?q?ows=20=E2=86=92=20first=20gets=20torn=20down=20+=20Notification?= =?UTF-8?q?s=20count=20returns=20to=200=20+=20AddCallCount=3D2=20proving?= =?UTF-8?q?=20the=20test=20actually=20exercised=20both=20calls),=20shutdow?= =?UTF-8?q?n=20disposes=20subscriptions,=20poll=20fallback=20works=20when?= =?UTF-8?q?=20UseNativeNotifications=3Dfalse=20(no=20native=20handles=20cr?= =?UTF-8?q?eated=20+=20initial-data=20push=20still=20fires),=20handle=20Di?= =?UTF-8?q?agnosticId=20distinguishes=20native=20vs=20poll.=20Existing=20p?= =?UTF-8?q?oll-mode=20ISubscribable=20tests=20in=20TwinCATCapabilityTests?= =?UTF-8?q?=20updated=20with=20UseNativeNotifications=3Dfalse=20so=20they?= =?UTF-8?q?=20continue=20testing=20the=20poll=20path=20specifically=20?= =?UTF-8?q?=E2=80=94=20both=20poll=20+=20native=20paths=20have=20test=20co?= =?UTF-8?q?verage=20now.=20TwinCATDriverTests=20got=20Probe.Enabled=3Dfals?= =?UTF-8?q?e=20added=20because=20the=20default=20factory=20creates=20a=20r?= =?UTF-8?q?eal=20AdsClient=20which=20was=20flakily=20affected=20by=20paral?= =?UTF-8?q?lel=20test=20execution=20sharing=20AMS=20router=20state.=20Tota?= =?UTF-8?q?l=20TwinCAT=20unit=20tests=20now=2093/93=20passing=20(+8=20from?= =?UTF-8?q?=20PR=203's=2085=20counting=20the=20new=20native=20tests=20+=20?= =?UTF-8?q?2=20existing=20tests=20that=20got=20options=20tweaks).=20Full?= =?UTF-8?q?=20solution=20builds=200=20errors;=20Modbus=20/=20AbCip=20/=20A?= =?UTF-8?q?bLegacy=20/=20other=20drivers=20untouched.=20TwinCAT=20driver?= =?UTF-8?q?=20is=20now=20feature-complete=20end-to-end=20=E2=80=94=20read?= =?UTF-8?q?=20/=20write=20/=20discover=20/=20native-subscribe=20/=20probe?= =?UTF-8?q?=20/=20host-resolve,=20with=20poll-mode=20as=20a=20safety=20val?= =?UTF-8?q?ve.=20Unblocks=20closing=20task=20#120=20for=20TwinCAT;=20remai?= =?UTF-8?q?ning=20sub-task:=20FOCAS=20+=20task=20#188=20(symbol-browsing?= =?UTF-8?q?=20=E2=80=94=20lower=20priority=20than=20FOCAS=20since=20real?= =?UTF-8?q?=20config=20flows=20still=20use=20pre-declared=20tags).?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-Authored-By: Claude Opus 4.7 (1M context) --- .../AdsTwinCATClient.cs | 81 ++++++- .../ITwinCATClient.cs | 23 ++ .../TwinCATDriver.cs | 80 ++++++- .../TwinCATDriverOptions.cs | 12 + .../FakeTwinCATClient.cs | 42 ++++ .../TwinCATCapabilityTests.cs | 2 + .../TwinCATDriverTests.cs | 2 + .../TwinCATNativeNotificationTests.cs | 221 ++++++++++++++++++ 8 files changed, 458 insertions(+), 5 deletions(-) create mode 100644 tests/ZB.MOM.WW.OtOpcUa.Driver.TwinCAT.Tests/TwinCATNativeNotificationTests.cs diff --git a/src/ZB.MOM.WW.OtOpcUa.Driver.TwinCAT/AdsTwinCATClient.cs b/src/ZB.MOM.WW.OtOpcUa.Driver.TwinCAT/AdsTwinCATClient.cs index 8336d20..bbf86c6 100644 --- a/src/ZB.MOM.WW.OtOpcUa.Driver.TwinCAT/AdsTwinCATClient.cs +++ b/src/ZB.MOM.WW.OtOpcUa.Driver.TwinCAT/AdsTwinCATClient.cs @@ -1,3 +1,4 @@ +using System.Collections.Concurrent; using TwinCAT.Ads; namespace ZB.MOM.WW.OtOpcUa.Driver.TwinCAT; @@ -17,6 +18,12 @@ namespace ZB.MOM.WW.OtOpcUa.Driver.TwinCAT; internal sealed class AdsTwinCATClient : ITwinCATClient { private readonly AdsClient _client = new(); + private readonly ConcurrentDictionary _notifications = new(); + + public AdsTwinCATClient() + { + _client.AdsNotificationEx += OnAdsNotificationEx; + } public bool IsConnected => _client.IsConnected; @@ -95,7 +102,79 @@ internal sealed class AdsTwinCATClient : ITwinCATClient } } - public void Dispose() => _client.Dispose(); + public async Task AddNotificationAsync( + string symbolPath, + TwinCATDataType type, + int? bitIndex, + TimeSpan cycleTime, + Action onChange, + CancellationToken cancellationToken) + { + var clrType = MapToClrType(type); + // NotificationSettings takes cycle + max-delay in 100ns units. AdsTransMode.OnChange + // fires when the value differs; OnCycle fires every cycle. OnChange is the right default + // for OPC UA data-change semantics — the PLC already has the best view of "has this + // changed" so we let it decide. + var cycleTicks = (uint)Math.Max(1, cycleTime.Ticks / TimeSpan.TicksPerMillisecond * 10_000); + var settings = new NotificationSettings(AdsTransMode.OnChange, (int)cycleTicks, 0); + + // AddDeviceNotificationExAsync returns Task; AdsNotificationEx fires + // with the handle as part of the event args so we use the handle as the correlation + // key into _notifications. + var result = await _client.AddDeviceNotificationExAsync( + symbolPath, settings, userData: null, clrType, args: null, cancellationToken) + .ConfigureAwait(false); + if (result.ErrorCode != AdsErrorCode.NoError) + throw new InvalidOperationException( + $"AddDeviceNotificationExAsync failed with ADS error {result.ErrorCode} for {symbolPath}"); + + var reg = new NotificationRegistration(symbolPath, type, bitIndex, onChange, this, result.Handle); + _notifications[result.Handle] = reg; + return reg; + } + + private void OnAdsNotificationEx(object? sender, AdsNotificationExEventArgs args) + { + if (!_notifications.TryGetValue(args.Handle, out var reg)) return; + var value = args.Value; + if (reg.BitIndex is int bit && reg.Type == TwinCATDataType.Bool && value is not bool) + value = ExtractBit(value, bit); + try { reg.OnChange(reg.SymbolPath, value); } catch { /* consumer-side errors don't crash the ADS thread */ } + } + + internal async Task DeleteNotificationAsync(uint handle, CancellationToken cancellationToken) + { + _notifications.TryRemove(handle, out _); + try { await _client.DeleteDeviceNotificationAsync(handle, cancellationToken).ConfigureAwait(false); } + catch { /* best-effort tear-down; target may already be gone */ } + } + + public void Dispose() + { + _client.AdsNotificationEx -= OnAdsNotificationEx; + _notifications.Clear(); + _client.Dispose(); + } + + private sealed class NotificationRegistration( + string symbolPath, + TwinCATDataType type, + int? bitIndex, + Action onChange, + AdsTwinCATClient owner, + uint handle) : ITwinCATNotificationHandle + { + public string SymbolPath { get; } = symbolPath; + public TwinCATDataType Type { get; } = type; + public int? BitIndex { get; } = bitIndex; + public Action OnChange { get; } = onChange; + + public void Dispose() + { + // Fire-and-forget AMS call — caller has already committed to the tear-down. + _ = owner.DeleteNotificationAsync(handle, CancellationToken.None); + } + } private static Type MapToClrType(TwinCATDataType type) => type switch { diff --git a/src/ZB.MOM.WW.OtOpcUa.Driver.TwinCAT/ITwinCATClient.cs b/src/ZB.MOM.WW.OtOpcUa.Driver.TwinCAT/ITwinCATClient.cs index cc6a086..66e19c0 100644 --- a/src/ZB.MOM.WW.OtOpcUa.Driver.TwinCAT/ITwinCATClient.cs +++ b/src/ZB.MOM.WW.OtOpcUa.Driver.TwinCAT/ITwinCATClient.cs @@ -46,8 +46,31 @@ public interface ITwinCATClient : IDisposable /// Used by 's probe loop. /// Task ProbeAsync(CancellationToken cancellationToken); + + /// + /// Register a cyclic / on-change ADS notification for a symbol. Returns a handle whose + /// tears the notification down. Callback fires on the + /// thread libplctag / AdsClient uses for notifications — consumers should marshal to + /// their own scheduler before doing work of any size. + /// + /// ADS symbol path (e.g. MAIN.bStart). + /// Declared type; drives the native layout + callback value boxing. + /// For BOOL-within-word tags — the bit to extract from the parent word. + /// Minimum interval between change notifications (native-floor depends on target). + /// Invoked with (symbolPath, boxedValue) per notification. + /// Cancels the initial registration; does not tear down an established notification. + Task AddNotificationAsync( + string symbolPath, + TwinCATDataType type, + int? bitIndex, + TimeSpan cycleTime, + Action onChange, + CancellationToken cancellationToken); } +/// Opaque handle for a registered ADS notification. tears it down. +public interface ITwinCATNotificationHandle : IDisposable { } + /// Factory for s. One client per device. public interface ITwinCATClientFactory { diff --git a/src/ZB.MOM.WW.OtOpcUa.Driver.TwinCAT/TwinCATDriver.cs b/src/ZB.MOM.WW.OtOpcUa.Driver.TwinCAT/TwinCATDriver.cs index 1768a6e..145b575 100644 --- a/src/ZB.MOM.WW.OtOpcUa.Driver.TwinCAT/TwinCATDriver.cs +++ b/src/ZB.MOM.WW.OtOpcUa.Driver.TwinCAT/TwinCATDriver.cs @@ -1,3 +1,4 @@ +using System.Collections.Concurrent; using ZB.MOM.WW.OtOpcUa.Core.Abstractions; namespace ZB.MOM.WW.OtOpcUa.Driver.TwinCAT; @@ -78,6 +79,12 @@ public sealed class TwinCATDriver : IDriver, IReadable, IWritable, ITagDiscovery public async Task ShutdownAsync(CancellationToken cancellationToken) { + // Native subs first — disposing the handles is cheap + lets the client close its + // notifications before the AdsClient itself goes away. + foreach (var sub in _nativeSubs.Values) + foreach (var r in sub.Registrations) { try { r.Dispose(); } catch { } } + _nativeSubs.Clear(); + await _poll.DisposeAsync().ConfigureAwait(false); foreach (var state in _devices.Values) { @@ -238,18 +245,83 @@ public sealed class TwinCATDriver : IDriver, IReadable, IWritable, ITagDiscovery return Task.CompletedTask; } - // ---- ISubscribable (polling overlay via shared engine) ---- + // ---- ISubscribable (native ADS notifications with poll fallback) ---- - public Task SubscribeAsync( - IReadOnlyList fullReferences, TimeSpan publishingInterval, CancellationToken cancellationToken) => - Task.FromResult(_poll.Subscribe(fullReferences, publishingInterval)); + private readonly ConcurrentDictionary _nativeSubs = new(); + private long _nextNativeSubId; + + /// + /// Subscribe via native ADS notifications when + /// is true, otherwise fall through to the shared . + /// Native path registers one per tag against the + /// target's PLC runtime — the PLC pushes changes on its own cycle so we skip the poll + /// loop entirely. Unsub path disposes the handles. + /// + public async Task SubscribeAsync( + IReadOnlyList fullReferences, TimeSpan publishingInterval, CancellationToken cancellationToken) + { + if (!_options.UseNativeNotifications) + return _poll.Subscribe(fullReferences, publishingInterval); + + var id = Interlocked.Increment(ref _nextNativeSubId); + var handle = new NativeSubscriptionHandle(id); + var registrations = new List(fullReferences.Count); + var now = DateTime.UtcNow; + + try + { + foreach (var reference in fullReferences) + { + if (!_tagsByName.TryGetValue(reference, out var def)) continue; + if (!_devices.TryGetValue(def.DeviceHostAddress, out var device)) continue; + + var client = await EnsureConnectedAsync(device, cancellationToken).ConfigureAwait(false); + var parsed = TwinCATSymbolPath.TryParse(def.SymbolPath); + var symbolName = parsed?.ToAdsSymbolName() ?? def.SymbolPath; + var bitIndex = parsed?.BitIndex; + + var reg = await client.AddNotificationAsync( + symbolName, def.DataType, bitIndex, publishingInterval, + (_, value) => OnDataChange?.Invoke(this, + new DataChangeEventArgs(handle, reference, new DataValueSnapshot( + value, TwinCATStatusMapper.Good, DateTime.UtcNow, DateTime.UtcNow))), + cancellationToken).ConfigureAwait(false); + registrations.Add(reg); + } + } + catch + { + // On any registration failure, tear down everything we got so far + rethrow. Leaves + // the subscription in a clean "never existed" state rather than a half-registered + // state the caller has to clean up. + foreach (var r in registrations) { try { r.Dispose(); } catch { } } + throw; + } + + _nativeSubs[id] = new NativeSubscription(handle, registrations); + return handle; + } public Task UnsubscribeAsync(ISubscriptionHandle handle, CancellationToken cancellationToken) { + if (handle is NativeSubscriptionHandle native && _nativeSubs.TryRemove(native.Id, out var sub)) + { + foreach (var r in sub.Registrations) { try { r.Dispose(); } catch { } } + return Task.CompletedTask; + } _poll.Unsubscribe(handle); return Task.CompletedTask; } + private sealed record NativeSubscriptionHandle(long Id) : ISubscriptionHandle + { + public string DiagnosticId => $"twincat-native-sub-{Id}"; + } + + private sealed record NativeSubscription( + NativeSubscriptionHandle Handle, + IReadOnlyList Registrations); + // ---- IHostConnectivityProbe ---- public IReadOnlyList GetHostStatuses() => diff --git a/src/ZB.MOM.WW.OtOpcUa.Driver.TwinCAT/TwinCATDriverOptions.cs b/src/ZB.MOM.WW.OtOpcUa.Driver.TwinCAT/TwinCATDriverOptions.cs index 4a0a8f4..be0671e 100644 --- a/src/ZB.MOM.WW.OtOpcUa.Driver.TwinCAT/TwinCATDriverOptions.cs +++ b/src/ZB.MOM.WW.OtOpcUa.Driver.TwinCAT/TwinCATDriverOptions.cs @@ -11,6 +11,18 @@ public sealed class TwinCATDriverOptions public IReadOnlyList Tags { get; init; } = []; public TwinCATProbeOptions Probe { get; init; } = new(); public TimeSpan Timeout { get; init; } = TimeSpan.FromSeconds(2); + + /// + /// When true (default), SubscribeAsync registers native ADS notifications + /// via AddDeviceNotificationExAsync — the PLC pushes changes on its own cycle + /// rather than the driver polling. Strictly better for latency + CPU when the target + /// supports it (TC2 + TC3 PLC runtimes always do; some soft-PLC / third-party ADS + /// implementations may not). When false, the driver falls through to the shared + /// — same semantics as the other + /// libplctag-backed drivers. Set false for deployments where the AMS router has + /// notification limits you can't raise. + /// + public bool UseNativeNotifications { get; init; } = true; } /// diff --git a/tests/ZB.MOM.WW.OtOpcUa.Driver.TwinCAT.Tests/FakeTwinCATClient.cs b/tests/ZB.MOM.WW.OtOpcUa.Driver.TwinCAT.Tests/FakeTwinCATClient.cs index 51598ef..f3bec53 100644 --- a/tests/ZB.MOM.WW.OtOpcUa.Driver.TwinCAT.Tests/FakeTwinCATClient.cs +++ b/tests/ZB.MOM.WW.OtOpcUa.Driver.TwinCAT.Tests/FakeTwinCATClient.cs @@ -56,6 +56,48 @@ internal class FakeTwinCATClient : ITwinCATClient DisposeCount++; IsConnected = false; } + + // ---- notification fake ---- + + public List Notifications { get; } = new(); + public bool ThrowOnAddNotification { get; set; } + + public virtual Task AddNotificationAsync( + string symbolPath, TwinCATDataType type, int? bitIndex, TimeSpan cycleTime, + Action onChange, CancellationToken cancellationToken) + { + if (ThrowOnAddNotification) + throw Exception ?? new InvalidOperationException("fake AddNotification failure"); + + var reg = new FakeNotification(symbolPath, type, bitIndex, onChange, this); + Notifications.Add(reg); + return Task.FromResult(reg); + } + + /// Fire a change event through the registered callback for . + public void FireNotification(string symbolPath, object? value) + { + foreach (var n in Notifications) + if (!n.Disposed && string.Equals(n.SymbolPath, symbolPath, StringComparison.OrdinalIgnoreCase)) + n.OnChange(symbolPath, value); + } + + public sealed class FakeNotification( + string symbolPath, TwinCATDataType type, int? bitIndex, + Action onChange, FakeTwinCATClient owner) : ITwinCATNotificationHandle + { + public string SymbolPath { get; } = symbolPath; + public TwinCATDataType Type { get; } = type; + public int? BitIndex { get; } = bitIndex; + public Action OnChange { get; } = onChange; + public bool Disposed { get; private set; } + + public void Dispose() + { + Disposed = true; + owner.Notifications.Remove(this); + } + } } internal sealed class FakeTwinCATClientFactory : ITwinCATClientFactory diff --git a/tests/ZB.MOM.WW.OtOpcUa.Driver.TwinCAT.Tests/TwinCATCapabilityTests.cs b/tests/ZB.MOM.WW.OtOpcUa.Driver.TwinCAT.Tests/TwinCATCapabilityTests.cs index d38f349..21d482a 100644 --- a/tests/ZB.MOM.WW.OtOpcUa.Driver.TwinCAT.Tests/TwinCATCapabilityTests.cs +++ b/tests/ZB.MOM.WW.OtOpcUa.Driver.TwinCAT.Tests/TwinCATCapabilityTests.cs @@ -49,6 +49,7 @@ public sealed class TwinCATCapabilityTests Devices = [new TwinCATDeviceOptions("ads://5.23.91.23.1.1:851")], Tags = [new TwinCATTagDefinition("X", "ads://5.23.91.23.1.1:851", "MAIN.X", TwinCATDataType.DInt)], Probe = new TwinCATProbeOptions { Enabled = false }, + UseNativeNotifications = false, // poll-mode test }, "drv-1", factory); await drv.InitializeAsync("{}", CancellationToken.None); @@ -74,6 +75,7 @@ public sealed class TwinCATCapabilityTests Devices = [new TwinCATDeviceOptions("ads://5.23.91.23.1.1:851")], Tags = [new TwinCATTagDefinition("X", "ads://5.23.91.23.1.1:851", "MAIN.X", TwinCATDataType.DInt)], Probe = new TwinCATProbeOptions { Enabled = false }, + UseNativeNotifications = false, // poll-mode test }, "drv-1", factory); await drv.InitializeAsync("{}", CancellationToken.None); diff --git a/tests/ZB.MOM.WW.OtOpcUa.Driver.TwinCAT.Tests/TwinCATDriverTests.cs b/tests/ZB.MOM.WW.OtOpcUa.Driver.TwinCAT.Tests/TwinCATDriverTests.cs index fd0e6bf..0b264fc 100644 --- a/tests/ZB.MOM.WW.OtOpcUa.Driver.TwinCAT.Tests/TwinCATDriverTests.cs +++ b/tests/ZB.MOM.WW.OtOpcUa.Driver.TwinCAT.Tests/TwinCATDriverTests.cs @@ -54,6 +54,7 @@ public sealed class TwinCATDriverTests var drv = new TwinCATDriver(new TwinCATDriverOptions { Devices = [new TwinCATDeviceOptions("ads://5.23.91.23.1.1:851")], + Probe = new TwinCATProbeOptions { Enabled = false }, }, "drv-1"); await drv.InitializeAsync("{}", CancellationToken.None); @@ -68,6 +69,7 @@ public sealed class TwinCATDriverTests var drv = new TwinCATDriver(new TwinCATDriverOptions { Devices = [new TwinCATDeviceOptions("ads://5.23.91.23.1.1:851")], + Probe = new TwinCATProbeOptions { Enabled = false }, }, "drv-1"); await drv.InitializeAsync("{}", CancellationToken.None); await drv.ReinitializeAsync("{}", CancellationToken.None); diff --git a/tests/ZB.MOM.WW.OtOpcUa.Driver.TwinCAT.Tests/TwinCATNativeNotificationTests.cs b/tests/ZB.MOM.WW.OtOpcUa.Driver.TwinCAT.Tests/TwinCATNativeNotificationTests.cs new file mode 100644 index 0000000..119fd5b --- /dev/null +++ b/tests/ZB.MOM.WW.OtOpcUa.Driver.TwinCAT.Tests/TwinCATNativeNotificationTests.cs @@ -0,0 +1,221 @@ +using System.Collections.Concurrent; +using Shouldly; +using Xunit; +using ZB.MOM.WW.OtOpcUa.Core.Abstractions; +using ZB.MOM.WW.OtOpcUa.Driver.TwinCAT; + +namespace ZB.MOM.WW.OtOpcUa.Driver.TwinCAT.Tests; + +[Trait("Category", "Unit")] +public sealed class TwinCATNativeNotificationTests +{ + private static (TwinCATDriver drv, FakeTwinCATClientFactory factory) NewNativeDriver(params TwinCATTagDefinition[] tags) + { + var factory = new FakeTwinCATClientFactory(); + var drv = new TwinCATDriver(new TwinCATDriverOptions + { + Devices = [new TwinCATDeviceOptions("ads://5.23.91.23.1.1:851")], + Tags = tags, + Probe = new TwinCATProbeOptions { Enabled = false }, + UseNativeNotifications = true, + }, "drv-1", factory); + return (drv, factory); + } + + [Fact] + public async Task Native_subscribe_registers_one_notification_per_tag() + { + var (drv, factory) = NewNativeDriver( + new TwinCATTagDefinition("A", "ads://5.23.91.23.1.1:851", "MAIN.A", TwinCATDataType.DInt), + new TwinCATTagDefinition("B", "ads://5.23.91.23.1.1:851", "MAIN.B", TwinCATDataType.Real)); + await drv.InitializeAsync("{}", CancellationToken.None); + + var handle = await drv.SubscribeAsync(["A", "B"], TimeSpan.FromMilliseconds(100), CancellationToken.None); + handle.DiagnosticId.ShouldStartWith("twincat-native-sub-"); + + factory.Clients[0].Notifications.Count.ShouldBe(2); + factory.Clients[0].Notifications.Select(n => n.SymbolPath).ShouldBe(["MAIN.A", "MAIN.B"], ignoreOrder: true); + } + + [Fact] + public async Task Native_notification_fires_OnDataChange_with_pushed_value() + { + var (drv, factory) = NewNativeDriver( + new TwinCATTagDefinition("Speed", "ads://5.23.91.23.1.1:851", "MAIN.Speed", TwinCATDataType.DInt)); + await drv.InitializeAsync("{}", CancellationToken.None); + + var events = new ConcurrentQueue(); + drv.OnDataChange += (_, e) => events.Enqueue(e); + + _ = await drv.SubscribeAsync(["Speed"], TimeSpan.FromMilliseconds(100), CancellationToken.None); + + factory.Clients[0].FireNotification("MAIN.Speed", 4200); + factory.Clients[0].FireNotification("MAIN.Speed", 4201); + + events.Count.ShouldBe(2); + events.Last().Snapshot.Value.ShouldBe(4201); + events.Last().FullReference.ShouldBe("Speed"); // driver-side reference, not ADS symbol + } + + [Fact] + public async Task Native_unsubscribe_disposes_all_notifications() + { + var (drv, factory) = NewNativeDriver( + new TwinCATTagDefinition("A", "ads://5.23.91.23.1.1:851", "MAIN.A", TwinCATDataType.DInt), + new TwinCATTagDefinition("B", "ads://5.23.91.23.1.1:851", "MAIN.B", TwinCATDataType.DInt)); + await drv.InitializeAsync("{}", CancellationToken.None); + + var handle = await drv.SubscribeAsync(["A", "B"], TimeSpan.FromMilliseconds(100), CancellationToken.None); + factory.Clients[0].Notifications.Count.ShouldBe(2); + + await drv.UnsubscribeAsync(handle, CancellationToken.None); + factory.Clients[0].Notifications.ShouldBeEmpty(); + } + + [Fact] + public async Task Native_unsubscribe_halts_future_notifications() + { + var (drv, factory) = NewNativeDriver( + new TwinCATTagDefinition("X", "ads://5.23.91.23.1.1:851", "MAIN.X", TwinCATDataType.DInt)); + await drv.InitializeAsync("{}", CancellationToken.None); + + var events = new ConcurrentQueue(); + drv.OnDataChange += (_, e) => events.Enqueue(e); + + var handle = await drv.SubscribeAsync(["X"], TimeSpan.FromMilliseconds(100), CancellationToken.None); + factory.Clients[0].FireNotification("MAIN.X", 1); + var snapshotFake = factory.Clients[0]; + await drv.UnsubscribeAsync(handle, CancellationToken.None); + + var afterUnsub = events.Count; + // After unsubscribe the fake's Notifications list is empty so FireNotification finds nothing + // to invoke. This mirrors the production contract — disposed handles no longer deliver. + snapshotFake.FireNotification("MAIN.X", 999); + events.Count.ShouldBe(afterUnsub); + } + + [Fact] + public async Task Native_subscribe_failure_mid_registration_cleans_up_partial_state() + { + // Fail-on-second-call fake — first AddNotificationAsync succeeds, second throws. + // Subscribe's catch block must tear the first one down before rethrowing so no zombie + // notification lingers. + var fake = new FailAfterNAddsFake(new AbTagParamsIrrelevant(), succeedBefore: 1); + var factory = new FakeTwinCATClientFactory { Customise = () => fake }; + var drv = new TwinCATDriver(new TwinCATDriverOptions + { + Devices = [new TwinCATDeviceOptions("ads://5.23.91.23.1.1:851")], + Tags = + [ + new TwinCATTagDefinition("A", "ads://5.23.91.23.1.1:851", "MAIN.A", TwinCATDataType.DInt), + new TwinCATTagDefinition("B", "ads://5.23.91.23.1.1:851", "MAIN.B", TwinCATDataType.DInt), + ], + Probe = new TwinCATProbeOptions { Enabled = false }, + UseNativeNotifications = true, + }, "drv-1", factory); + await drv.InitializeAsync("{}", CancellationToken.None); + + await Should.ThrowAsync(() => + drv.SubscribeAsync(["A", "B"], TimeSpan.FromMilliseconds(100), CancellationToken.None)); + + // First registration succeeded then got torn down by the catch; second threw. + fake.AddCallCount.ShouldBe(2); + fake.Notifications.Count.ShouldBe(0); // partial handle cleaned up + } + + private sealed class AbTagParamsIrrelevant { } + + private sealed class FailAfterNAddsFake : FakeTwinCATClient + { + private readonly int _succeedBefore; + public int AddCallCount { get; private set; } + + public FailAfterNAddsFake(AbTagParamsIrrelevant _, int succeedBefore) : base() + { + _succeedBefore = succeedBefore; + } + + public override Task AddNotificationAsync( + string symbolPath, TwinCATDataType type, int? bitIndex, TimeSpan cycleTime, + Action onChange, CancellationToken cancellationToken) + { + AddCallCount++; + if (AddCallCount > _succeedBefore) + throw new InvalidOperationException($"fake fail on call #{AddCallCount}"); + return base.AddNotificationAsync(symbolPath, type, bitIndex, cycleTime, onChange, cancellationToken); + } + } + + [Fact] + public async Task Native_shutdown_disposes_subscriptions() + { + var (drv, factory) = NewNativeDriver( + new TwinCATTagDefinition("X", "ads://5.23.91.23.1.1:851", "MAIN.X", TwinCATDataType.DInt)); + await drv.InitializeAsync("{}", CancellationToken.None); + + _ = await drv.SubscribeAsync(["X"], TimeSpan.FromMilliseconds(100), CancellationToken.None); + factory.Clients[0].Notifications.Count.ShouldBe(1); + + await drv.ShutdownAsync(CancellationToken.None); + factory.Clients[0].Notifications.ShouldBeEmpty(); + } + + [Fact] + public async Task Poll_path_still_works_when_UseNativeNotifications_false() + { + var factory = new FakeTwinCATClientFactory + { + Customise = () => new FakeTwinCATClient { Values = { ["MAIN.X"] = 7 } }, + }; + var drv = new TwinCATDriver(new TwinCATDriverOptions + { + Devices = [new TwinCATDeviceOptions("ads://5.23.91.23.1.1:851")], + Tags = [new TwinCATTagDefinition("X", "ads://5.23.91.23.1.1:851", "MAIN.X", TwinCATDataType.DInt)], + Probe = new TwinCATProbeOptions { Enabled = false }, + UseNativeNotifications = false, + }, "drv-1", factory); + await drv.InitializeAsync("{}", CancellationToken.None); + + var events = new ConcurrentQueue(); + drv.OnDataChange += (_, e) => events.Enqueue(e); + + var handle = await drv.SubscribeAsync(["X"], TimeSpan.FromMilliseconds(150), CancellationToken.None); + await WaitForAsync(() => events.Count >= 1, TimeSpan.FromSeconds(2)); + + events.First().Snapshot.Value.ShouldBe(7); + factory.Clients[0].Notifications.ShouldBeEmpty(); // no native notifications on poll path + await drv.UnsubscribeAsync(handle, CancellationToken.None); + } + + [Fact] + public async Task Subscribe_handle_DiagnosticId_indicates_native_vs_poll() + { + var (drvNative, _) = NewNativeDriver( + new TwinCATTagDefinition("X", "ads://5.23.91.23.1.1:851", "MAIN.X", TwinCATDataType.DInt)); + await drvNative.InitializeAsync("{}", CancellationToken.None); + var nativeHandle = await drvNative.SubscribeAsync(["X"], TimeSpan.FromMilliseconds(100), CancellationToken.None); + nativeHandle.DiagnosticId.ShouldContain("native"); + + var factoryPoll = new FakeTwinCATClientFactory + { + Customise = () => new FakeTwinCATClient { Values = { ["MAIN.X"] = 1 } }, + }; + var drvPoll = new TwinCATDriver(new TwinCATDriverOptions + { + Devices = [new TwinCATDeviceOptions("ads://5.23.91.23.1.1:851")], + Tags = [new TwinCATTagDefinition("X", "ads://5.23.91.23.1.1:851", "MAIN.X", TwinCATDataType.DInt)], + Probe = new TwinCATProbeOptions { Enabled = false }, + UseNativeNotifications = false, + }, "drv-1", factoryPoll); + await drvPoll.InitializeAsync("{}", CancellationToken.None); + var pollHandle = await drvPoll.SubscribeAsync(["X"], TimeSpan.FromMilliseconds(100), CancellationToken.None); + pollHandle.DiagnosticId.ShouldNotContain("native"); + } + + private static async Task WaitForAsync(Func condition, TimeSpan timeout) + { + var deadline = DateTime.UtcNow + timeout; + while (!condition() && DateTime.UtcNow < deadline) + await Task.Delay(20); + } +} -- 2.49.1