diff --git a/.claude/rules/drivers.md b/.claude/rules/drivers.md index 8586e567e..88aba9960 100644 --- a/.claude/rules/drivers.md +++ b/.claude/rules/drivers.md @@ -268,6 +268,10 @@ host logger factory (wired in `DriverFactoryBootstrap`; the FOCAS and TwinCAT fa `DriverHealthChanged.Diagnostics` / proto `has_diagnostics`+`diagnostics`; its digest is in the publish fingerprint). - **TwinCAT (#637):** TIME/TOD publish Float64 ms, DATE/DT a UTC DateTime; a tag authored `UDInt` keeps the raw counter (and keeps historizing — DateTime is not historized). +- **TwinCAT Test Connect (#863):** the probe opens no socket of its own — `AdsClient.Connect` + `ReadStateAsync` + through the LOCAL AMS router, exactly the driver's path. Never derive a host from an AMS Net ID (it is not an IP; + the retired pre-check dialled its first four octets on the AMS port — a public stranger — and always timed out). + No router ⇒ red "No local AMS router" (no degrade-to-green). `docs/drivers/TestConnectProbes.md`. - **TwinCAT (#645, #310/#314):** opt-in ADS sum-read (`useSumRead`, `sumReadMaxItems` default 200 / ceiling 500, 32 KiB payload budget). It reads raw bytes and decodes as the declared type, so only a scalar passing the strict width+kind check (`CheckNotification`) rides a batch; value-converted pairs (DInt over INT, Real over LREAL/DINT) read singly. Only overall diff --git a/docs/drivers/TestConnectProbes.md b/docs/drivers/TestConnectProbes.md index 946303481..71e8f73d7 100644 --- a/docs/drivers/TestConnectProbes.md +++ b/docs/drivers/TestConnectProbes.md @@ -46,7 +46,7 @@ with a human-readable explanation rather than a false-green TCP-open tick. | **S7** | `Plc.OpenAsync` (COTP CR/CC + S7 setup-communication), check `IsConnected`, then `Close`. Wrong rack/slot or a non-S7 server causes `OpenAsync` to throw → handshake fail. | `"S7 connected (CPU …)"` | `10.100.0.35:1102` (python-snap7 sim) | | **AbCip** | `libplctag` Tag `InitializeAsync` (EIP session + CIP Forward Open). A CIP-level error such as tag-not-found still proves the controller answered CIP → `Ok`. A session/ForwardOpen/connect error → handshake fail. | `"CIP session OK"` | `10.100.0.35:44818` (CIP sim) | | **AbLegacy** | Same `libplctag` `InitializeAsync` handshake as AbCip, PCCC protocol family. | `"CIP session OK"` (PCCC family) | Deferred — no PLC5/SLC sim | -| **TwinCAT** | `AdsClient.Connect` + `ReadStateAsync`. See [degrade semantics](#twincat-degrade) below. | `"ADS state: {state}"` | Deferred — no ADS target | +| **TwinCAT** | `AdsClient.Connect` + `ReadStateAsync` through the **local AMS router** — no TCP pre-check. See [TwinCAT: no TCP pre-check](#twincat-no-tcp-pre-check) below. | `"ADS state: {state}"` | Z35474 (production TC3 press, read-only, via an in-process TcpRouter) | | **FOCAS** | `cnc_allclibhndl3` via a direct `DllImport("fwlib32")` in the probe. See [degrade semantics](#focas-degrade) below. | `"FOCAS handle OK"` | Deferred — no CNC + FWLIB | | **Galaxy** | gRPC unary call to `GalaxyRepository.TestConnection` on the configured mxaccessgw endpoint. See [auth-rejection rule](#galaxy-auth-rejection) below. | `"gateway gRPC OK"` | `http://10.100.0.48:5120` (mxaccessgw) | | **Heidenhain** | LSV2 `A_LG INSPECT` (password resolved from its `secret:`/`env:` reference) + `R_VR`, through the same read-only allowlist as the driver; a refusal or malformed version record → fail. No logout — the socket close ends the session. | `"LSV2 INSPECT session OK at host:port: NC "` | Deferred — no LSV2 control; simulator-proven | @@ -61,29 +61,36 @@ backend is now the external HistorianGateway (a gRPC client package, not a probe ## Degrade semantics -Three drivers have environmental constraints that can prevent the handshake +Some drivers have environmental constraints that can prevent the handshake from running on certain hosts. The **degradation principle** is: the probe must never produce a result *worse* than today's TCP-only probe. A genuine protocol rejection from a reachable device is a correct RED; an inability to -*run* the handshake at all (no FWLIB, no managed router) degrades to the -existing TCP-reachability message — still a green tick but annotated. +*run* the handshake at all (no FWLIB) degrades to the +existing TCP-reachability message — still a green tick but annotated. TwinCAT +no longer degrades (see below). -### TwinCAT degrade +### TwinCAT: no TCP pre-check -Where the handshake is available: +TwinCAT is the exception to the "TCP connect fails" row above: it opens **no socket of its own** +(#863). The driver has no IP field — an AMS Net ID is a Beckhoff identifier the AMS router maps to a +host through its route table, and the AMS port (801/851) is not a TCP port. The probe therefore takes +exactly the driver's path (`AdsTwinCATClient.ConnectAsync`): `AdsClient.Connect(netId, port)` through +the local AMS router (TwinCAT XAR, or `Beckhoff.TwinCAT.Ads.TcpRouter` hosted in-process), then one +read-only `ReadStateAsync`, all inside the probe budget. Until #863 a first-phase check dialled the Net +ID's first four octets as an IPv4 address on the AMS port — for Z35474 (`5.118.56.93.1.1`) an +unrelated public host — so Test Connect timed out against every real runtime. -- `AdsClient.Connect(netId, port)` + `ReadStateAsync` → `Ok=true`, - `"ADS state: {state}"` (Run / Config / Stop). -- An ADS **route-table rejection** from a reachable ADS router is a **true RED**: - `"Reachable at {host}:{port} but ADS handshake failed: {detail} — check the - target's ADS route table authorizes this host"`. This is the correct result: - the driver would also be unable to function without an authorized route. +| Outcome | `Ok` | Message | +|---------|------|---------| +| ReadState succeeds | `true` | `"ADS state: {state}"` (Run / Config / Stop) | +| No local AMS router (Beckhoff "Check for a running TwinCAT router instance!", `ClientPortNotOpen`, router not active, ADS client cannot load) | `false` | `"No local AMS router on this host — start TwinCAT XAR or host the Beckhoff TcpRouter; the target was not contacted (…)"` | +| Router answered but cannot reach the target (`TargetMachineNotFound`, `TargetPortNotFound`, port/access refused, host unreachable) | `false` | `"No route to {netId}:{port} or target unreachable: {code} — check the local AMS router has a route …"` | +| Sync/device timeout code, or the budget expired | `false` | `"Probe timed out after {n}s waiting for ADS state from {netId}:{port} …"` | +| Any other ADS error | `false` | `"ADS handshake with {netId}:{port} failed: {code}"` | -Where the handshake is unavailable (headless server, no TwinCAT runtime, the -managed AMS router cannot start): - -- Probe degrades to TCP-reachability: `Ok=true`, - `"(ADS handshake unavailable on this host — TCP reachability only)"`. +There is **no degrade-to-green** any more: with no TCP check, a host that cannot run the handshake +has contacted nothing, so it reports the missing router rather than a green tick. A stock admin node +has no AMS router, so TwinCAT Test Connect is red there by design — the driver needs the same router. ### FOCAS degrade diff --git a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.TwinCAT/TwinCATDriverProbe.cs b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.TwinCAT/TwinCATDriverProbe.cs index 8300a2b46..f1e2933e8 100644 --- a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.TwinCAT/TwinCATDriverProbe.cs +++ b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.TwinCAT/TwinCATDriverProbe.cs @@ -1,5 +1,4 @@ using System.Diagnostics; -using System.Net.Sockets; using System.Text.Json; using System.Text.Json.Serialization; using TwinCAT.Ads; @@ -8,59 +7,40 @@ using ZB.MOM.WW.OtOpcUa.Core.Abstractions; namespace ZB.MOM.WW.OtOpcUa.Driver.TwinCAT; /// -/// Two-phase, degrade-guarded Test-Connect probe for the -shaped -/// driver config. Phase 1: a bare TCP connect to the first device's AMS router host (first four -/// octets of the AMS Net ID) on the AMS port — fast rejection of unreachable targets. Phase 2: -/// a real ADS handshake — AdsClient.Connect(netId, port) + ReadStateAsync — to -/// confirm the endpoint speaks ADS and report the controller's run-state, not just that a TCP -/// socket opened. +/// Test-Connect probe for the -shaped driver config. It takes +/// exactly the path the driver takes (): +/// AdsClient.Connect(netId, port) through the local AMS router (TwinCAT XAR, or +/// Beckhoff.TwinCAT.Ads.TcpRouter hosted in-process), followed by one read-only +/// ReadStateAsync, all inside the caller's time budget. /// -/// Outcome classification — three cases: -/// -/// -/// ADS connected + ReadState OK → Ok=true, message "ADS state: {AdsState}" -/// (e.g. "Run" / "Config" / "Stop"), with latency. -/// -/// -/// Route/auth rejection from a reachable router — an -/// (or a non-success ReadStateAsync result) whose means the -/// router answered but won't let us in (e.g. , -/// , , -/// ) → Ok=false, message -/// "Reachable at {host}:{port} but ADS handshake failed: {code} — check the target's ADS -/// route table authorizes this host". This is a TRUE red: the driver itself also needs -/// the route, so a green tick here would be a false positive. -/// -/// -/// Handshake could not be ATTEMPTED on this host — the managed AMS router cannot run -/// headless (Beckhoff's AdsClient.Connect throws a server exception -/// "Check for a running TwinCAT router instance!"), or a -/// / / / -/// surfaces, or ReadStateAsync reports a client-side -/// port-not-open status → DEGRADE: Ok=true, message -/// "Reachable at {host}:{port} (ADS handshake unavailable on this host — TCP reachability -/// only)", with latency. The probe NEVER produces a result worse than the old TCP-only -/// probe. -/// +/// No TCP pre-check (#863). The driver has no IP field: an AMS Net ID is a Beckhoff +/// identifier that the router maps to a host through its route table, and the AMS port +/// (801/851/…) is not a TCP port. The probe therefore never opens a socket of its own, and never +/// dials anything derived from the AMS Net ID. The former first-phase check dialled the first four +/// Net ID octets as an IPv4 address on the AMS port — a host unrelated to the target (often a +/// public internet address) on a port ADS does not use — so Test Connect timed out against every +/// real runtime. +/// +/// +/// Outcome classification — Ok=true only when the target answered: +/// +/// ReadState OK → Ok=true, "ADS state: {AdsState}", with latency. +/// No local AMS router — AdsClient.Connect cannot reach a router on this +/// host (Beckhoff's AdsServerException "Check for a running TwinCAT router +/// instance!"), the client port never opened, or the ADS client cannot load here → +/// Ok=false, "No local AMS router on this host …". The target was not contacted. +/// No route / target unreachable — the router answered but has no route to the Net +/// ID, the target refused the route or port, or the router's own connection to the target +/// failed → Ok=false, "No route to … or target unreachable: {code} …". +/// Timeout — a sync/device timeout code, or the caller's budget expired → +/// Ok=false, "Probe timed out after {n}s …". +/// Any other ADS error → Ok=false, "ADS handshake with … failed: {code}". /// /// /// /// -/// The line between case 2 (device-rejected → RED) and case 3 (can't-attempt → DEGRADE) is the -/// crux. Classification rests on the exception's identity: only an -/// (or a result ) carrying a route/target-port code is a RED — that is the -/// ADS router answering and refusing the route. Beckhoff's TwinCAT.Ads.Server.AdsServerException -/// ("running TwinCAT router instance!") derives from plain , NOT from -/// , so it is correctly classified as "can't attempt → DEGRADE". -/// When genuinely ambiguous, the probe DEGRADES (Ok=true, TCP-only note) rather than emit a false RED. -/// -/// AMS Net ID format is six dot-separated octets (e.g. 192.168.1.10.1.1); the first four -/// are typically the host IPv4 address by Beckhoff convention. The AMS router resolves the real IP -/// route server-side; the probe uses the first-four-octet heuristic for the TCP preflight target, -/// which is reliable for the overwhelming majority of production deployments. -/// -/// The probe is read-only — ReadStateAsync never mutates PLC state — and always disposes the -/// . +/// The probe is read-only — ReadStateAsync never mutates PLC state, and the probe sends no +/// AddRoute — and always disposes the . /// public sealed class TwinCATDriverProbe : IDriverProbe { @@ -71,22 +51,57 @@ public sealed class TwinCATDriverProbe : IDriverProbe Converters = { new JsonStringEnumConverter() }, }; - /// - /// AMS error codes that mean "the router answered but refused the route / target port" — a - /// genuine RED. The driver itself would also be denied, so a green tick would be a false - /// positive. Everything NOT in this set (client-side port/connection errors, sync timeouts, - /// router-not-initialised, etc.) is treated as "couldn't attempt the handshake" → DEGRADE. - /// - private static readonly HashSet _routeRejectCodes = + /// Codes meaning the router on this host is absent, inactive or never gave us a port. + private static readonly HashSet _noRouterCodes = + [ + AdsErrorCode.ClientPortNotOpen, + AdsErrorCode.ClientNoAmsAddr, + AdsErrorCode.RouterNotInitialized, + AdsErrorCode.RouterNotActive, + ]; + + /// + /// Codes meaning the router answered but cannot get us to the target: no route, the route or + /// port was refused, or the router's connection to the target host failed. + /// + private static readonly HashSet _noRouteCodes = [ - AdsErrorCode.TargetPortNotFound, AdsErrorCode.TargetMachineNotFound, + AdsErrorCode.TargetPortNotFound, AdsErrorCode.PortNotConnected, AdsErrorCode.PortDisabled, AdsErrorCode.AccessDenied, AdsErrorCode.DeviceAccessDenied, + AdsErrorCode.HostUnreachable, + AdsErrorCode.WSA_NetworkUnreachable, + AdsErrorCode.WSA_HostDown, + AdsErrorCode.WSA_HostUnreachable, + AdsErrorCode.WSA_HostNotFound, ]; + /// Codes meaning a route exists but the target did not answer in time. + private static readonly HashSet _timeoutCodes = + [ + AdsErrorCode.ClientSyncTimeOut, + AdsErrorCode.SyncTimeOut, + AdsErrorCode.DeviceTimeOut, + AdsErrorCode.WSA_TimedOut, + AdsErrorCode.RouterFragmentTimeout, + ]; + + /// + /// The ADS handshake: connect through the local AMS router and read the target's state. + /// Throws for a non-success ReadState result. Swappable for tests. + /// + private readonly Func> _readState; + + /// Initializes the probe with the real ADS handshake through the local AMS router. + public TwinCATDriverProbe() : this(ReadStateViaLocalRouterAsync) { } + + /// Test seam — substitutes the ADS handshake. + internal TwinCATDriverProbe(Func> readState) + => _readState = readState; + /// public string DriverType => "TwinCAT"; @@ -98,110 +113,90 @@ public sealed class TwinCATDriverProbe : IDriverProbe catch (Exception ex) { return new(false, $"Config JSON is invalid: {ex.Message}", null); } if (opts is null) return new(false, "Config JSON deserialized to null.", null); - var (host, port, parsed) = ExtractTarget(opts); - if (parsed is null || string.IsNullOrWhiteSpace(host) || port <= 0) - return new(false, "Config has no host/port to probe.", null); + var target = TwinCATAmsAddress.TryParse(opts.Devices.FirstOrDefault()?.HostAddress); + if (target is null) + return new(false, "Config has no host/port to probe (expected a device hostAddress ads://{netId}:{port}).", null); - // Phase 1: bare TCP preflight — fast rejection for unreachable hosts. Messages here are - // UNCHANGED from the original TCP-only probe. + var label = $"{target.NetId}:{target.Port}"; + var timedOut = $"Probe timed out after {timeout.TotalSeconds:F0}s waiting for ADS state from {label}"; var sw = Stopwatch.StartNew(); try { - using var socket = new Socket(AddressFamily.InterNetwork, SocketType.Stream, ProtocolType.Tcp); - await socket.ConnectAsync(host, port, ct); - } - catch (SocketException ex) - { - return new(false, $"Connect failed: {ex.SocketErrorCode}", null); - } - catch (OperationCanceledException) when (ct.IsCancellationRequested) - { - return new(false, $"Probe timed out after {timeout.TotalSeconds:F0}s.", null); - } - catch (Exception ex) - { - return new(false, ex.Message, null); - } - - // Phase 2: real ADS handshake. Connect + ReadStateAsync. The crux is the three-way - // classification of how this can fail — see the class-doc and ClassifyHandshakeFailure. - var degradeNote = $"Reachable at {host}:{port} (ADS handshake unavailable on this host — TCP reachability only)"; - try - { - using var client = new AdsClient(); - // Bound the ADS round-trip by the caller's timeout (clamped >=1s, mirrors AdsTwinCATClient). - client.Timeout = (int)Math.Max(1_000, timeout.TotalMilliseconds); - - // Connect can throw a server exception ("running TwinCAT router instance!") on a headless - // host with no AMS router — that is a can't-attempt DEGRADE, classified below. - client.Connect(AmsNetId.Parse(parsed.NetId), parsed.Port); - - var state = await client.ReadStateAsync(ct).ConfigureAwait(false); + // Task.Run + WaitAsync keep the whole handshake — including the synchronous + // AdsClient.Connect — inside the caller's budget. + var state = await Task.Run(() => _readState(target, timeout, ct), ct) + .WaitAsync(timeout, ct).ConfigureAwait(false); sw.Stop(); - - if (state.Succeeded) - return new(true, $"ADS state: {state.State.AdsState}", sw.Elapsed); - - // Non-throwing failure carried in the result's error code. - return state.ErrorCode == AdsErrorCode.ClientPortNotOpen - ? new(true, degradeNote, sw.Elapsed) // client never opened — DEGRADE - : ClassifyHandshakeFailure(state.ErrorCode, host, port, sw, degradeNote); + return new(true, $"ADS state: {state}", sw.Elapsed); + } + catch (TimeoutException) + { + return new(false, $"{timedOut} — the target did not answer.", null); } catch (OperationCanceledException) when (ct.IsCancellationRequested) { - // Caller timeout — keep the original timed-out message. - return new(false, $"Probe timed out after {timeout.TotalSeconds:F0}s.", null); + return new(false, $"{timedOut}.", null); } catch (AdsErrorException ex) { - // The router answered with an ADS-level error. Route/auth rejection → RED; anything - // else (sync timeout, client port issues, …) → DEGRADE. - return ClassifyHandshakeFailure(ex.ErrorCode, host, port, sw, degradeNote); + return Classify(ex.ErrorCode, label, timedOut); } - catch (Exception) + catch (Exception ex) when (IsNoRouter(ex)) { - // Everything else — TwinCAT.Ads.Server.AdsServerException ("running TwinCAT router - // instance!"), PlatformNotSupportedException, TypeInitializationException, - // DllNotFoundException, NotSupportedException, etc. — means the handshake could not be - // ATTEMPTED on this host. DEGRADE: never worse than the TCP-only probe. - sw.Stop(); - return new(true, degradeNote, sw.Elapsed); + return new(false, NoRouterMessage(ex.Message), null); + } + catch (Exception ex) + { + return new(false, $"ADS handshake with {label} failed: {ex.Message}", null); } } + /// Default handshake — the same calls makes. + private static async Task ReadStateViaLocalRouterAsync( + TwinCATAmsAddress target, TimeSpan timeout, CancellationToken ct) + { + using var client = new AdsClient(); + client.Timeout = (int)Math.Max(1_000, timeout.TotalMilliseconds); + client.Connect(AmsNetId.Parse(target.NetId), target.Port); + + var state = await client.ReadStateAsync(ct).ConfigureAwait(false); + if (!state.Succeeded) + throw new AdsErrorException($"ReadState failed: {state.ErrorCode}", state.ErrorCode); + return state.State.AdsState; + } + + /// Maps an ADS error code to one of the probe's failure classes. + internal static DriverProbeResult Classify(AdsErrorCode code, string label, string timedOut) + { + if (_noRouterCodes.Contains(code)) + return new(false, NoRouterMessage(code.ToString()), null); + if (_noRouteCodes.Contains(code)) + return new(false, + $"No route to {label} or target unreachable: {code} — check the local AMS router has a route to this AMS Net ID and the target's route table authorizes this host.", + null); + if (_timeoutCodes.Contains(code)) + return new(false, $"{timedOut} ({code}) — the router has a route but the target did not answer.", null); + return new(false, $"ADS handshake with {label} failed: {code}", null); + } + + private static string NoRouterMessage(string detail) => + $"No local AMS router on this host — start TwinCAT XAR or host the Beckhoff TcpRouter; the target was not contacted ({detail})."; + /// - /// Classifies an ADS-level failure (from an or a non-success - /// ReadStateAsync result). A route/target-port/access code means the router answered - /// but refused the route → RED. Any other code is treated as "couldn't attempt" → DEGRADE, - /// so the probe never under-reports a host with no usable ADS runtime. + /// True when the ADS client could not reach a router on this host, or cannot load here at all. + /// Beckhoff's AdsServerException / AmsServerException ("Check for a running TwinCAT + /// router instance!") derive from plain , so they are matched by type name. /// - private static DriverProbeResult ClassifyHandshakeFailure( - AdsErrorCode code, string host, int port, Stopwatch sw, string degradeNote) + private static bool IsNoRouter(Exception ex) { - if (_routeRejectCodes.Contains(code)) - return new(false, - $"Reachable at {host}:{port} but ADS handshake failed: {code} — check the target's ADS route table authorizes this host", - null); - - sw.Stop(); - return new(true, degradeNote, sw.Elapsed); - } - - private static (string host, int port, TwinCATAmsAddress? parsed) ExtractTarget(TwinCATDriverOptions opts) - { - // Parse the first device's ads:// address. AMS Net ID is six-octet; by Beckhoff - // convention the first four octets are the host IPv4. Extract those as the TCP target. - var firstDevice = opts.Devices.FirstOrDefault(); - if (firstDevice is null) return (string.Empty, 0, null); - - var parsed = TwinCATAmsAddress.TryParse(firstDevice.HostAddress); - if (parsed is null) return (string.Empty, 0, null); - - // NetId = "a.b.c.d.e.f" — take the first 4 octets as the host IP. - var parts = parsed.NetId.Split('.'); - if (parts.Length < 4) return (string.Empty, 0, null); - - var hostIp = string.Join('.', parts[0], parts[1], parts[2], parts[3]); - return (hostIp, parsed.Port, parsed); + for (var e = ex; e is not null; e = e.InnerException) + { + if (e is PlatformNotSupportedException or TypeInitializationException or DllNotFoundException) + return true; + var name = e.GetType().FullName; + if (name is "TwinCAT.Ads.Server.AdsServerException" or "TwinCAT.Ams.AmsServerException") + return true; + } + return false; } } diff --git a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.TwinCAT.Tests/TwinCATDriverProbeTests.cs b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.TwinCAT.Tests/TwinCATDriverProbeTests.cs index ec0ca8628..01bdf68ae 100644 --- a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.TwinCAT.Tests/TwinCATDriverProbeTests.cs +++ b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.TwinCAT.Tests/TwinCATDriverProbeTests.cs @@ -1,22 +1,17 @@ -// Unit tests for TwinCATDriverProbe — the degrade-guarded ADS ReadState Test-Connect probe. +// Unit tests for TwinCATDriverProbe — the ADS ReadState Test-Connect probe (#863). // -// Coverage here is the set of outcomes that are *deterministic on a CI/dev box with no TwinCAT -// runtime*: invalid JSON, missing host/port, an unreachable TCP target, and — the crux — the -// DEGRADE path. On this box there is no AMS router, so once the TCP preflight succeeds against a -// reachable listener the managed ADS client cannot initialise (it throws a "Check for a running -// TwinCAT router instance!" server exception, NOT an ADS device rejection). The probe MUST treat -// that as "TCP reachability only" (Ok=true) so it never regresses below the old TCP-only probe. -// -// The two paths that REQUIRE a live ADS target are LIVE-VERIFY DEFERRED (no TwinCAT runtime on the -// rig): (a) the happy "ADS state: Run/Config/Stop" path (Ok=true with the AdsState name), and -// (b) the route/auth-reject RED path (a reachable router that refuses the route, surfaced as an -// AdsErrorException carrying a TargetPortNotFound / route AdsErrorCode → Ok=false with the -// "check the target's ADS route table" message). Those are exercised against a real TwinCAT XAR / -// remote runtime, out of scope for this unit suite. +// The probe takes exactly the driver's path: AdsClient.Connect(netId, port) through the LOCAL AMS +// router, then one ReadStateAsync. It must never open a socket of its own, and never to a host derived +// from the AMS Net ID (the old first-phase check dialled the Net ID's first four octets on the AMS +// port — an unrelated host, often a public one, on a port ADS does not use — so Test Connect timed out +// against every real runtime). The classification and budget tests drive the handshake through the +// internal seam; the no-dial and no-router tests use the real AdsClient on a box with no AMS router. +using System.Diagnostics; using System.Net; using System.Net.Sockets; using Shouldly; +using TwinCAT.Ads; using Xunit; namespace ZB.MOM.WW.OtOpcUa.Driver.TwinCAT.Tests; @@ -24,199 +19,162 @@ namespace ZB.MOM.WW.OtOpcUa.Driver.TwinCAT.Tests; [Trait("Category", "Unit")] public sealed class TwinCATDriverProbeTests { - private static readonly TwinCATDriverProbe Probe = new(); + private const string Z35474Like = "{\"devices\":[{\"hostAddress\":\"ads://5.118.56.93.1.1:851\"}]}"; - // ------------------------------------------------------------------------- - // 1. Invalid JSON - // ------------------------------------------------------------------------- + private static TwinCATDriverProbe ProbeWith(Func> handshake) + => new(handshake); + + // ---- config validation ---------------------------------------------------------------- - /// Invalid JSON returns Ok=false with a message containing "invalid". [Fact] public async Task InvalidJson_Returns_OkFalse_WithInvalidMessage() { - var result = await Probe.ProbeAsync( - "not-valid-json{{{", - TimeSpan.FromSeconds(3), - TestContext.Current.CancellationToken); + var result = await new TwinCATDriverProbe().ProbeAsync( + "not-valid-json{{{", TimeSpan.FromSeconds(3), TestContext.Current.CancellationToken); result.Ok.ShouldBeFalse(); - result.Message.ShouldNotBeNull(); result.Message!.ToLowerInvariant().ShouldContain("invalid"); result.Latency.ShouldBeNull(); } - // ------------------------------------------------------------------------- - // 2. Config with no device / no host - // ------------------------------------------------------------------------- - - /// An empty Devices list returns Ok=false with a "no host/port" message. - [Fact] - public async Task NoDevices_Returns_OkFalse_WithNoHostPortMessage() + [Theory] + [InlineData("{\"devices\":[]}")] + [InlineData("{\"devices\":[{\"hostAddress\":\"not-an-ads-url\"}]}")] + public async Task NoUsableDevice_Returns_OkFalse_WithNoHostPortMessage(string json) { - var result = await Probe.ProbeAsync( - "{\"devices\":[]}", - TimeSpan.FromSeconds(3), - TestContext.Current.CancellationToken); + var called = false; + var result = await ProbeWith((_, _, _) => { called = true; return Task.FromResult(AdsState.Run); }) + .ProbeAsync(json, TimeSpan.FromSeconds(3), TestContext.Current.CancellationToken); result.Ok.ShouldBeFalse(); - result.Message.ShouldNotBeNull(); result.Message!.ShouldContain("no host/port"); - result.Latency.ShouldBeNull(); + called.ShouldBeFalse(); } - /// - /// A first device with a malformed (non-ads://) host address returns Ok=false with a - /// "no host/port" message because returns null. - /// - [Fact] - public async Task MalformedHostAddress_Returns_OkFalse_WithNoHostPortMessage() - { - var result = await Probe.ProbeAsync( - "{\"devices\":[{\"hostAddress\":\"not-an-ads-url\"}]}", - TimeSpan.FromSeconds(3), - TestContext.Current.CancellationToken); - - result.Ok.ShouldBeFalse(); - result.Message.ShouldNotBeNull(); - result.Message!.ShouldContain("no host/port"); - result.Latency.ShouldBeNull(); - } - - // ------------------------------------------------------------------------- - // 3. Unreachable target — TCP preflight fails first - // ------------------------------------------------------------------------- + // ---- #863: never dial a host derived from the AMS Net ID -------------------------------- /// - /// A closed port causes the TCP preflight ConnectAsync to throw - /// ; the probe returns Ok=false with "Connect failed". - /// The port is reserved then released so the OS refuses the connection immediately. + /// Regression for #863. A listener sits on exactly the endpoint the old probe dialled + /// (Net ID octets 1–4 as IPv4, AMS port as TCP port). The real probe must never connect to + /// it — the old TCP pre-check connected here first, so this fails on the old behaviour. /// [Fact] - public async Task ClosedPort_Returns_OkFalse_WithConnectFailedMessage() + public async Task Probe_NeverConnects_ToAmsNetIdDerivedEndpoint() { - // Reserve an ephemeral port then release it so the port is definitely closed. - var reserved = new TcpListener(IPAddress.Loopback, 0); - reserved.Start(); - var port = ((IPEndPoint)reserved.LocalEndpoint).Port; - reserved.Stop(); - - // 127.0.0.1 as the first four AMS-Net-ID octets → the probe's TCP target is 127.0.0.1. - var configJson = $"{{\"devices\":[{{\"hostAddress\":\"ads://127.0.0.1.1.1:{port}\"}}]}}"; - - using var cts = CancellationTokenSource.CreateLinkedTokenSource( - TestContext.Current.CancellationToken); - cts.CancelAfter(TimeSpan.FromSeconds(5)); - - var result = await Probe.ProbeAsync( - configJson, - TimeSpan.FromSeconds(3), - cts.Token); - - result.Ok.ShouldBeFalse(); - result.Message.ShouldNotBeNull(); - // Either a fast SocketException ("Connect failed") or, if the OS stalls, a timeout. - var lower = result.Message!.ToLowerInvariant(); - (lower.Contains("connect failed") || lower.Contains("timed out")).ShouldBeTrue( - $"Expected 'Connect failed' or 'timed out' but got: {result.Message}"); - result.Latency.ShouldBeNull(); - } - - /// - /// A target on a non-routable IP (192.0.2.1 — TEST-NET-1, RFC 5737) with a short - /// cancellation window times out (or fails fast on EHOSTUNREACH) at the TCP preflight. - /// - [Fact] - public async Task BlackHoleTarget_Returns_OkFalse_WithTimedOutOrConnectFailed() - { - using var cts = CancellationTokenSource.CreateLinkedTokenSource( - TestContext.Current.CancellationToken); - cts.CancelAfter(TimeSpan.FromSeconds(1)); - - var configJson = "{\"devices\":[{\"hostAddress\":\"ads://192.0.2.1.1.1:851\"}]}"; - var result = await Probe.ProbeAsync( - configJson, - TimeSpan.FromSeconds(1), - cts.Token); - - result.Ok.ShouldBeFalse(); - result.Message.ShouldNotBeNull(); - var lower = result.Message!.ToLowerInvariant(); - (lower.Contains("timed out") || lower.Contains("connect failed")).ShouldBeTrue( - $"Expected 'timed out' or 'Connect failed' but got: {result.Message}"); - result.Latency.ShouldBeNull(); - } - - // ------------------------------------------------------------------------- - // 4. DEGRADE path — TCP reachable, but the ADS handshake can't be attempted here - // ------------------------------------------------------------------------- - - /// - /// The crux. A reachable TCP listener satisfies the preflight, but on this box (no TwinCAT - /// AMS router) the managed ADS client cannot establish a session — AdsClient.Connect - /// throws an environment/router exception (NOT an ADS device rejection). The probe MUST - /// DEGRADE: Ok=true with the "TCP reachability only" note, never a false RED. This - /// guarantees the new probe never reports worse than the old TCP-only probe on a host with - /// no ADS runtime. - /// - [Fact] - public async Task ReachableTcp_NoAdsRuntime_Degrades_To_OkTrue_WithTcpOnlyNote() - { - // Stand up a real TCP listener and accept (then drop) connections so the preflight - // ConnectAsync succeeds. We never speak ADS — the ADS handshake is what must degrade. var listener = new TcpListener(IPAddress.Loopback, 0); listener.Start(); var port = ((IPEndPoint)listener.LocalEndpoint).Port; - - using var acceptCts = CancellationTokenSource.CreateLinkedTokenSource( - TestContext.Current.CancellationToken); - var acceptLoop = Task.Run(async () => - { - try - { - while (true) - { - var client = await listener.AcceptTcpClientAsync(acceptCts.Token); - // Hold the socket briefly so the preflight sees a clean accept, then close. - _ = Task.Run(async () => - { - await Task.Delay(200); - client.Dispose(); - }); - } - } - catch (OperationCanceledException) { /* accept cancelled — expected on teardown */ } - catch (ObjectDisposedException) { /* listener stopped — expected on teardown */ } - catch (SocketException) { /* listener stopped — expected on teardown */ } - }, TestContext.Current.CancellationToken); - try { - var configJson = $"{{\"devices\":[{{\"hostAddress\":\"ads://127.0.0.1.1.1:{port}\"}}]}}"; - - using var cts = CancellationTokenSource.CreateLinkedTokenSource( - TestContext.Current.CancellationToken); + using var cts = CancellationTokenSource.CreateLinkedTokenSource(TestContext.Current.CancellationToken); cts.CancelAfter(TimeSpan.FromSeconds(10)); + await new TwinCATDriverProbe().ProbeAsync( + $"{{\"devices\":[{{\"hostAddress\":\"ads://127.0.0.1.1.1:{port}\"}}]}}", + TimeSpan.FromSeconds(3), cts.Token); - var result = await Probe.ProbeAsync( - configJson, - TimeSpan.FromSeconds(3), - cts.Token); - - // DEGRADE guard: must be Ok=true (never worse than TCP-only), with the explicit note - // and a latency. If on some box AdsClient instead threw synchronously at construction, - // the probe still degrades the same way — this assertion holds either way. - result.Ok.ShouldBeTrue( - $"Degrade guard violated — probe returned a worse-than-TCP result: {result.Message}"); - result.Message.ShouldNotBeNull(); - result.Message!.ShouldContain("ADS handshake unavailable on this host"); - result.Message!.ShouldContain("TCP reachability only"); - result.Latency.ShouldNotBeNull(); + listener.Pending().ShouldBeFalse( + "the probe opened a TCP connection to the AmsNetId-derived address — it must only use the local AMS router"); } finally { - await acceptCts.CancelAsync(); listener.Stop(); - try { await acceptLoop; } catch { /* ignore teardown races */ } } } + + /// The handshake receives the AMS address unchanged — the probe resolves no host itself. + [Fact] + public async Task Handshake_IsHandedTheAmsAddress_Unchanged() + { + TwinCATAmsAddress? seen = null; + var result = await ProbeWith((t, _, _) => { seen = t; return Task.FromResult(AdsState.Run); }) + .ProbeAsync(Z35474Like, TimeSpan.FromSeconds(3), TestContext.Current.CancellationToken); + + result.Ok.ShouldBeTrue(); + seen.ShouldBe(new TwinCATAmsAddress("5.118.56.93.1.1", 851)); + } + + // ---- outcome classification ------------------------------------------------------------- + + [Fact] + public async Task ReadStateOk_Returns_OkTrue_WithStateAndLatency() + { + var result = await ProbeWith((_, _, _) => Task.FromResult(AdsState.Run)) + .ProbeAsync(Z35474Like, TimeSpan.FromSeconds(3), TestContext.Current.CancellationToken); + + result.Ok.ShouldBeTrue(); + result.Message.ShouldBe("ADS state: Run"); + result.Latency.ShouldNotBeNull(); + } + + /// + /// On a box with no AMS router the real AdsClient cannot connect. That is a RED naming the + /// missing router — no longer a green "TCP reachability only", since nothing was contacted. + /// + [Fact] + public async Task NoLocalRouter_RealClient_Returns_OkFalse_NamingTheRouter() + { + var result = await new TwinCATDriverProbe().ProbeAsync( + Z35474Like, TimeSpan.FromSeconds(3), TestContext.Current.CancellationToken); + + result.Ok.ShouldBeFalse(); + result.Message!.ShouldContain("No local AMS router"); + result.Message!.ShouldContain("not contacted"); + result.Latency.ShouldBeNull(); + } + + [Fact] + public async Task AmsServerException_Classified_AsNoLocalRouter() + { + var result = await ProbeWith((_, _, _) => throw new global::TwinCAT.Ams.AmsServerException("Connect to Server failed!")) + .ProbeAsync(Z35474Like, TimeSpan.FromSeconds(3), TestContext.Current.CancellationToken); + + result.Ok.ShouldBeFalse(); + result.Message!.ShouldContain("No local AMS router"); + } + + [Theory] + [InlineData(AdsErrorCode.ClientPortNotOpen, "No local AMS router")] + [InlineData(AdsErrorCode.TargetMachineNotFound, "No route to 5.118.56.93.1.1:851 or target unreachable")] + [InlineData(AdsErrorCode.TargetPortNotFound, "No route to 5.118.56.93.1.1:851 or target unreachable")] + [InlineData(AdsErrorCode.WSA_HostUnreachable, "No route to 5.118.56.93.1.1:851 or target unreachable")] + [InlineData(AdsErrorCode.ClientSyncTimeOut, "timed out")] + [InlineData(AdsErrorCode.DeviceInvalidGroup, "ADS handshake with 5.118.56.93.1.1:851 failed")] + public async Task AdsErrorCodes_AreClassified(AdsErrorCode code, string expected) + { + var result = await ProbeWith((_, _, _) => throw new AdsErrorException("x", code)) + .ProbeAsync(Z35474Like, TimeSpan.FromSeconds(3), TestContext.Current.CancellationToken); + + result.Ok.ShouldBeFalse(); + result.Message!.ShouldContain(expected); + result.Message!.ShouldContain(code.ToString()); + result.Latency.ShouldBeNull(); + } + + // ---- time budget -------------------------------------------------------------------------- + + /// A handshake that never returns (and ignores its token) is cut off at the budget. + [Fact] + public async Task HangingHandshake_IsBoundedByTimeout() + { + var never = new TaskCompletionSource(); + var sw = Stopwatch.StartNew(); + var result = await ProbeWith((_, _, _) => never.Task) + .ProbeAsync(Z35474Like, TimeSpan.FromSeconds(1), TestContext.Current.CancellationToken); + sw.Stop(); + + result.Ok.ShouldBeFalse(); + result.Message!.ShouldContain("timed out after 1s"); + sw.Elapsed.ShouldBeLessThan(TimeSpan.FromSeconds(5)); + } + + [Fact] + public async Task CallerCancellation_Returns_TimedOut() + { + using var cts = new CancellationTokenSource(TimeSpan.FromMilliseconds(200)); + var result = await ProbeWith(async (_, _, ct) => { await Task.Delay(Timeout.Infinite, ct); return AdsState.Run; }) + .ProbeAsync(Z35474Like, TimeSpan.FromSeconds(10), cts.Token); + + result.Ok.ShouldBeFalse(); + result.Message!.ShouldContain("Probe timed out after 10s"); + } }