diff --git a/docs/Driver.AbCip.Cli.md b/docs/Driver.AbCip.Cli.md index d2a05819..3a82de36 100644 --- a/docs/Driver.AbCip.Cli.md +++ b/docs/Driver.AbCip.Cli.md @@ -33,7 +33,12 @@ Family ↔ CIP-path cheat sheet: ### `probe` — is the PLC up? ```powershell -# ControlLogix — read the canonical libplctag system tag +# ControlLogix — read a real tag +otopcua-abcip-cli probe -g ab://10.0.0.5/1,0 -t TestDINT --type DInt + +# …or any name at all: `@raw_cpu_type` is NOT a libplctag special tag (only `@raw`, `@tags` +# and `@udt/` are), so it goes out as an ordinary symbolic name that no controller defines. +# The "tag does not exist" answer is itself proof the CIP session came up. otopcua-abcip-cli probe -g ab://10.0.0.5/1,0 -t @raw_cpu_type --type DInt # Micro800 — point at a user-supplied global diff --git a/docs/plans/2026-07-15-universal-discovery-browser-implementation.md b/docs/plans/2026-07-15-universal-discovery-browser-implementation.md index a0fd089a..470e93bb 100644 --- a/docs/plans/2026-07-15-universal-discovery-browser-implementation.md +++ b/docs/plans/2026-07-15-universal-discovery-browser-implementation.md @@ -1132,12 +1132,24 @@ neither agreed with a controller. Fixed, with the layout now pinned by a capture ### Follow-ups discovered during the second gate -1. **Browse-commit discards the discovered data type** — a browsed REAL commits with - `"dataType":"DInt"` in its `TagConfig`, and the driver decodes off that field rather than the - row's `DataType`. Filed as **Gitea #536**; affects AbCip and (by inspection) TwinCAT. -2. **Test Connect fails against this fixture regardless of the device** — the probe falls back to - `@raw_cpu_type` when no tags are authored, which `ab_server` does not implement. Fixture - limitation, documented in the fixture README; browse is unaffected. +1. ~~**Browse-commit discards the discovered data type**~~ — **CLOSED 2026-08-01 (Gitea #536).** A + browsed REAL committed with `"dataType":"DInt"`, and AbCip/TwinCAT/**FOCAS** decode off that field + rather than the raw row's `DataType`, so the tag published a plausible wrong number with no error + raised. FOCAS was missed when the issue was filed. Fixed by carrying the driver's own type name on + `DriverAttributeInfo.NativeDataType` → `AttributeInfo.AddressFields["dataType"]` → + `RawBrowseCommitMapper`. **It could not be fixed by reverse-mapping the leaf's driver-agnostic + `DriverDataType`**: `SInt`/`Int`/`DInt`/`USInt`/`UInt` all collapse into `Int32`, so that "fix" + would still bind a 16-bit INT as a 32-bit DINT while looking correct on a REAL. +2. ~~**Test Connect fails against this fixture regardless of the device**~~ — **CLOSED 2026-08-01, and + the stated cause was wrong.** `@raw_cpu_type` is not a libplctag special tag at all (only `@raw`, + `@tags`, `@udt/` are), so it is sent as an ordinary symbolic name and the controller's "tag does not + exist" IS the successful answer. Two real defects hid behind the "fixture limitation" label: the sim + answered CIP **0x03** where a controller answers **0x04** (fixed in vendored patch 0002), and + `AbCipDriverProbe.IsReachableException` matched the native C status spellings (`STATUS_NOT_FOUND`) + that libplctag.NET never emits — it throws with the managed enum name — so the reachable + classification could not fire on the exception path, which is the path every no-authored-tags probe + takes. **Test Connect was therefore failing on real hardware too, not just against the sim.** + Pinned by `AbCip_WithNoAuthoredTags_ReportsTheControllerReachable` plus offline classification tests. ### Follow-ups discovered during the first gate diff --git a/src/Core/ZB.MOM.WW.OtOpcUa.Commons/Browsing/CapturedTreeBrowseSession.cs b/src/Core/ZB.MOM.WW.OtOpcUa.Commons/Browsing/CapturedTreeBrowseSession.cs index 47ac6fdd..0ace043a 100644 --- a/src/Core/ZB.MOM.WW.OtOpcUa.Commons/Browsing/CapturedTreeBrowseSession.cs +++ b/src/Core/ZB.MOM.WW.OtOpcUa.Commons/Browsing/CapturedTreeBrowseSession.cs @@ -50,7 +50,7 @@ public sealed class CapturedTreeBrowseSession(CapturedTree tree) : IBrowseSessio var attrs = new List(); if (n.Attribute is { } a) attrs.Add(new AttributeInfo(n.Name, a.DriverDataType.ToString(), a.IsArray, - a.SecurityClass.ToString(), a.IsAlarm || n.IsAlarmMarked)); + a.SecurityClass.ToString(), a.IsAlarm || n.IsAlarmMarked, NativeTypeFields(a))); attrs.AddRange(n.Properties.Select(p => new AttributeInfo(p.Name, p.Type.ToString(), false, "", false))); return Task.FromResult>(attrs); @@ -65,6 +65,29 @@ public sealed class CapturedTreeBrowseSession(CapturedTree tree) : IBrowseSessio private CapturedTree Tree() => _tree ?? throw new ObjectDisposedException(nameof(CapturedTreeBrowseSession)); + /// + /// Surfaces a leaf's driver-native type name as the dataType address field, so a + /// browse-commit can stamp the driver-typed TagConfig with the type the controller + /// actually reported (Gitea #536). + /// + /// The captured attribute. + /// The address fields, or null when the driver reported no native type. + /// + /// It rides rather than + /// because that field is the driver-AGNOSTIC + /// type and several consumers already read it as such. The commit mapper cannot recover the + /// native type from it: SInt/Int/DInt all arrive as Int32, so a + /// reverse map would bind a 16-bit INT as a 32-bit DINT and read four bytes for two. + /// + /// Null when the driver stamped nothing — the mapper then leaves the model's own default in + /// place, which is exactly the pre-#536 behaviour rather than a guess. + /// + /// + private static IReadOnlyDictionary? NativeTypeFields(DriverAttributeInfo a) + => string.IsNullOrWhiteSpace(a.NativeDataType) + ? null + : new Dictionary(StringComparer.Ordinal) { ["dataType"] = a.NativeDataType }; + private static BrowseNode Map(CapturedNode n) => n.IsLeaf ? new BrowseNode(n.Id, n.DisplayName, BrowseNodeKind.Leaf, false) : new BrowseNode(n.Id, n.DisplayName, BrowseNodeKind.Folder, n.Children.Count > 0); diff --git a/src/Core/ZB.MOM.WW.OtOpcUa.Core.Abstractions/DriverAttributeInfo.cs b/src/Core/ZB.MOM.WW.OtOpcUa.Core.Abstractions/DriverAttributeInfo.cs index c3b2085f..350a9037 100644 --- a/src/Core/ZB.MOM.WW.OtOpcUa.Core.Abstractions/DriverAttributeInfo.cs +++ b/src/Core/ZB.MOM.WW.OtOpcUa.Core.Abstractions/DriverAttributeInfo.cs @@ -45,6 +45,21 @@ namespace ZB.MOM.WW.OtOpcUa.Core.Abstractions; /// Set when is — /// stable logical id the ScriptedAlarmEngine addresses by. Null otherwise. /// +/// +/// The driver's OWN type name for this attribute (e.g. "Real" for an +/// AbCipDataType, "Int16" for a TwinCATDataType) when the driver binds +/// by a driver-specific type enum in its TagConfig; null for a driver whose tag config +/// carries no type (OPC UA Client, Galaxy) or which takes the type from the wire (MQTT +/// Sparkplug births, MTConnect probe inference). +/// +/// It exists because is lossy in exactly the direction a +/// browse-commit needs (Gitea #536): AbCipDataType.SInt, .Int and .DInt +/// all map to , so a commit that reverse-mapped the +/// driver-agnostic type would bind a 16-bit INT as a 32-bit DINT. Nothing downstream of the +/// address-space build reads this — it is authoring metadata carried alongside the type the +/// node manager actually uses. +/// +/// public sealed record DriverAttributeInfo( string FullName, DriverDataType DriverDataType, @@ -56,7 +71,8 @@ public sealed record DriverAttributeInfo( bool WriteIdempotent = false, NodeSourceKind Source = NodeSourceKind.Driver, string? VirtualTagId = null, - string? ScriptedAlarmId = null); + string? ScriptedAlarmId = null, + string? NativeDataType = null); /// /// Discriminates which runtime subsystem owns this node's Read/Write/ diff --git a/src/Drivers/Cli/ZB.MOM.WW.OtOpcUa.Driver.AbCip.Cli/Commands/ProbeCommand.cs b/src/Drivers/Cli/ZB.MOM.WW.OtOpcUa.Driver.AbCip.Cli/Commands/ProbeCommand.cs index a0cf284f..804ff1bc 100644 --- a/src/Drivers/Cli/ZB.MOM.WW.OtOpcUa.Driver.AbCip.Cli/Commands/ProbeCommand.cs +++ b/src/Drivers/Cli/ZB.MOM.WW.OtOpcUa.Driver.AbCip.Cli/Commands/ProbeCommand.cs @@ -14,8 +14,10 @@ public sealed class ProbeCommand : AbCipCommandBase { /// Gets the tag path to probe. [CommandOption("tag", 't', Description = - "Tag path to probe. ControlLogix default is '@raw_cpu_type' (the canonical libplctag " + - "system tag); Micro800 takes a user-supplied global (e.g. '_SYSVA_CLOCK_HOUR').", + "Tag path to probe — a real tag on the controller. Any name works to prove reachability: " + + "libplctag knows only @raw, @tags and @udt/ as special names, so anything else (including " + + "'@raw_cpu_type', which no controller defines) is sent as an ordinary symbolic name and the " + + "controller's 'tag does not exist' answer already proves the CIP session is live.", IsRequired = true)] public string TagPath { get; init; } = default!; diff --git a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip.Contracts/AbCipDriverOptions.cs b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip.Contracts/AbCipDriverOptions.cs index 7c949862..24c9a74b 100644 --- a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip.Contracts/AbCipDriverOptions.cs +++ b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip.Contracts/AbCipDriverOptions.cs @@ -213,8 +213,11 @@ public sealed class AbCipProbeOptions /// Tag path used for the probe. When is true but this is /// null/blank, the driver logs a warning and runs no probe loops; /// GetHostStatuses() will then report every device as Unknown. A family-default - /// system-tag fallback (e.g. @raw_cpu_type on ControlLogix) is a deferred follow-up; - /// today an operator opting into the probe must supply a tag path explicitly. + /// fallback is a deferred follow-up; today an operator opting into the probe must supply a tag + /// path explicitly. (This used to name @raw_cpu_type as the ControlLogix system tag such + /// a default would read. It is not one — libplctag's CIP special-tag dispatch knows only + /// @raw, @tags and @udt/ — so any default here would be a name the + /// controller rejects, which proves reachability but never returns a value.) /// public string? ProbeTagPath { get; init; } } diff --git a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip/AbCipDriver.cs b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip/AbCipDriver.cs index 703579d6..2cd918ae 100644 --- a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip/AbCipDriver.cs +++ b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip/AbCipDriver.cs @@ -1068,7 +1068,8 @@ public sealed class AbCipDriver : IDriver, IReadable, IWritable, ITagDiscovery, : SecurityClassification.ViewOnly, IsHistorized: false, IsAlarm: false, - WriteIdempotent: member.WriteIdempotent)); + WriteIdempotent: member.WriteIdempotent, + NativeDataType: member.DataType.ToString())); } continue; } @@ -1145,7 +1146,8 @@ public sealed class AbCipDriver : IDriver, IReadable, IWritable, ITagDiscovery, : SecurityClassification.Operate, IsHistorized: false, IsAlarm: false, - WriteIdempotent: false)); + WriteIdempotent: false, + NativeDataType: discovered.DataType.ToString())); } } } @@ -1248,7 +1250,8 @@ public sealed class AbCipDriver : IDriver, IReadable, IWritable, ITagDiscovery, : SecurityClassification.Operate, IsHistorized: false, IsAlarm: false, - WriteIdempotent: false)); + WriteIdempotent: false, + NativeDataType: member.DataType.ToString())); anyEmitted = true; } @@ -1277,7 +1280,8 @@ public sealed class AbCipDriver : IDriver, IReadable, IWritable, ITagDiscovery, : SecurityClassification.ViewOnly, IsHistorized: false, IsAlarm: false, - WriteIdempotent: tag.WriteIdempotent); + WriteIdempotent: tag.WriteIdempotent, + NativeDataType: tag.DataType.ToString()); /// Count of registered devices — exposed for diagnostics + tests. internal int DeviceCount => _devices.Count; diff --git a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip/AbCipDriverProbe.cs b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip/AbCipDriverProbe.cs index 58e8a0ec..9ad6abae 100644 --- a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip/AbCipDriverProbe.cs +++ b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip/AbCipDriverProbe.cs @@ -91,9 +91,16 @@ public sealed class AbCipDriverProbe : IDriverProbe // Prefer the first authored tag path as the probe tag name (it must exist on the PLC). v3: the // tags arrive as RawTagEntry blobs — map each through the pure factory to recover its TagPath + - // device routing key. Fall back to a benign system attribute present on all ControlLogix / - // CompactLogix controllers (@raw_cpu_type); still returns ErrorNotFound on Micro800 / MicroLogix, - // which is sufficient to confirm CIP reachability. + // device routing key. + // + // The fallback is NOT a system attribute the controller can read, despite its name and despite + // what this comment used to claim: libplctag's CIP special-tag dispatch knows only @raw, @tags + // and @udt/, so "@raw_cpu_type" is encoded and sent as an ORDINARY symbolic name. That is what + // makes it a usable probe — no controller defines it, so the controller answers CIP 0x04 ("tag + // does not exist") and the answer itself proves the session is live. IsReachableStatus is what + // turns that into a green tick, so a device with no authored tags reports reachable, not failed. + // Do not "fix" this to @tags: libplctag builds a tag-listing tag WITHOUT any network I/O, so it + // reports OK against any TCP listener — measured green against a Mosquitto broker. var mappedTags = opts.RawTags .Select(e => AbCipTagDefinitionFactory.FromTagConfig(e.TagConfig, e.RawPath, out var d) ? (Ok: true, Device: e.DeviceName, d.TagPath) @@ -167,22 +174,35 @@ public sealed class AbCipDriverProbe : IDriverProbe /// but the probe tag name was not found or did not match — sufficient to confirm CIP /// reachability. Returns true for (tag was found). /// - private static bool IsReachableStatus(Status s) => s is + internal static bool IsReachableStatus(Status s) => s is Status.Ok or Status.ErrorNotFound or Status.ErrorNoMatch or Status.ErrorBadDevice; /// - /// Inspects a message to decide whether the error is a - /// "controller answered CIP but tag not found" condition (reachable) versus a session / - /// transport failure (unreachable). Falls back to checking the exception message for - /// known tag-not-found phrases used by the libplctag native library. + /// Decides whether a is a "controller answered CIP but the + /// probe tag did not resolve" condition (reachable) or a session / transport failure + /// (unreachable), by recovering the the wrapper threw on and deferring + /// to — one classification, not two that can drift. /// - private static bool IsReachableException(LibPlcTagException ex) + /// The exception libplctag threw out of InitializeAsync. + /// true when the controller answered; false for a transport/session failure. + /// + /// The status arrives as the managed enum NAME. NativeTagWrapper.ThrowIfStatusNotOk + /// constructs the exception with Status.ToString(), so the message is exactly + /// "ErrorNotFound" — never the native C spelling "STATUS_NOT_FOUND" this method + /// used to look for. Those substrings could not match, so the reachable classification was + /// unreachable on this path — and this IS the path a probe takes whenever its tag does not + /// resolve, which is every Test Connect against a device with no authored tags. The native + /// spellings are kept as a secondary match in case a future wrapper reports them. + /// + internal static bool IsReachableException(LibPlcTagException ex) { - // libplctag wraps the Status in the message as "STATUS_NOT_FOUND", "STATUS_NO_MATCH", etc. var msg = ex.Message ?? string.Empty; + if (Enum.TryParse(msg.Trim(), ignoreCase: true, out var status) && Enum.IsDefined(status)) + return IsReachableStatus(status); + return msg.Contains("NOT_FOUND", StringComparison.OrdinalIgnoreCase) || msg.Contains("NO_MATCH", StringComparison.OrdinalIgnoreCase) || msg.Contains("BAD_DEVICE", StringComparison.OrdinalIgnoreCase); diff --git a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.FOCAS/FocasDriver.cs b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.FOCAS/FocasDriver.cs index ebd68e38..aea048ff 100644 --- a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.FOCAS/FocasDriver.cs +++ b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.FOCAS/FocasDriver.cs @@ -613,7 +613,8 @@ public sealed class FocasDriver : IDriver, IReadable, IWritable, ITagDiscovery, SecurityClass: SecurityClassification.ViewOnly, IsHistorized: false, IsAlarm: false, - WriteIdempotent: tag.WriteIdempotent)); + WriteIdempotent: tag.WriteIdempotent, + NativeDataType: tag.DataType.ToString())); } } return Task.CompletedTask; @@ -638,7 +639,8 @@ public sealed class FocasDriver : IDriver, IReadable, IWritable, ITagDiscovery, SecurityClass: SecurityClassification.ViewOnly, IsHistorized: false, IsAlarm: false, - WriteIdempotent: false)); + WriteIdempotent: false, + NativeDataType: NativeName(type))); } private static void EmitAxisVariable( @@ -653,9 +655,24 @@ public sealed class FocasDriver : IDriver, IReadable, IWritable, ITagDiscovery, SecurityClass: SecurityClassification.ViewOnly, IsHistorized: false, IsAlarm: false, - WriteIdempotent: false)); + WriteIdempotent: false, + NativeDataType: nameof(FocasDataType.Float64))); } + /// + /// The name a fixed-tree node binds by, for the browse-commit + /// path (Gitea #536). The fixed tree only ever emits these three, and each has an + /// identically-named member — the driver-agnostic + /// cannot be reversed (Byte/Int16/Int32 all collapse to + /// ), so the native name is carried rather than derived. + /// + private static string NativeName(FocasDriverDataType type) => type switch + { + FocasDriverDataType.Int32 => nameof(FocasDataType.Int32), + FocasDriverDataType.Float64 => nameof(FocasDataType.Float64), + _ => nameof(FocasDataType.String), + }; + /// /// Emit a variable under a named fixed-tree folder (Program, OperationMode, /// …). Full-reference shape is {deviceHost}/{folderPath}/{field}. @@ -673,7 +690,13 @@ public sealed class FocasDriver : IDriver, IReadable, IWritable, ITagDiscovery, SecurityClass: SecurityClassification.ViewOnly, IsHistorized: false, IsAlarm: false, - WriteIdempotent: false)); + WriteIdempotent: false, + NativeDataType: NativeName(type switch + { + DriverDataType.Int32 => FocasDriverDataType.Int32, + DriverDataType.Float64 => FocasDriverDataType.Float64, + _ => FocasDriverDataType.String, + }))); } /// diff --git a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.TwinCAT/TwinCATDriver.cs b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.TwinCAT/TwinCATDriver.cs index e041066f..a2aaaf34 100644 --- a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.TwinCAT/TwinCATDriver.cs +++ b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.TwinCAT/TwinCATDriver.cs @@ -421,7 +421,8 @@ public sealed class TwinCATDriver : IDriver, IReadable, IWritable, ITagDiscovery : SecurityClassification.ViewOnly, IsHistorized: false, IsAlarm: false, - WriteIdempotent: tag.WriteIdempotent)); + WriteIdempotent: tag.WriteIdempotent, + NativeDataType: tag.DataType.ToString())); } // Controller-side symbol browse — opt-in. Falls back to pre-declared-only on any @@ -452,7 +453,8 @@ public sealed class TwinCATDriver : IDriver, IReadable, IWritable, ITagDiscovery : SecurityClassification.Operate, IsHistorized: false, IsAlarm: false, - WriteIdempotent: false)); + WriteIdempotent: false, + NativeDataType: dt.ToString())); } } catch (OperationCanceledException) { throw; } diff --git a/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Uns/RawBrowseCommitMapper.cs b/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Uns/RawBrowseCommitMapper.cs index a76d6295..1a912093 100644 --- a/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Uns/RawBrowseCommitMapper.cs +++ b/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Uns/RawBrowseCommitMapper.cs @@ -3,7 +3,10 @@ using ZB.MOM.WW.OtOpcUa.AdminUI.Uns.TagEditors; using ZB.MOM.WW.OtOpcUa.Commons.Types; using ZB.MOM.WW.OtOpcUa.Configuration.Enums; using ZB.MOM.WW.OtOpcUa.Core.Abstractions; +using ZB.MOM.WW.OtOpcUa.Driver.AbCip; +using ZB.MOM.WW.OtOpcUa.Driver.FOCAS; using ZB.MOM.WW.OtOpcUa.Driver.Sql.Contracts; +using ZB.MOM.WW.OtOpcUa.Driver.TwinCAT; using ZB.MOM.WW.OtOpcUa.Driver.Mqtt; namespace ZB.MOM.WW.OtOpcUa.AdminUI.Uns; @@ -192,11 +195,19 @@ public static class RawBrowseCommitMapper } /// - /// Builds the driver-typed TagConfig JSON for a browse-committed tag, setting ONLY the driver's - /// address field to and leaving every other field at its model default. Reuses - /// the <Driver>TagConfigModel for driver types that have a typed editor; the model-less Galaxy + /// Builds the driver-typed TagConfig JSON for a browse-committed tag: the driver's address field + /// from , plus — for a driver that binds by its OWN data-type enum — the + /// native type the browse leaf stated (see ). Every other field stays at + /// its model default, for the operator to author afterwards. Reuses the + /// <Driver>TagConfigModel for driver types that have a typed editor; the model-less Galaxy /// driver gets the canonical camelCase attributeRef key directly. /// + /// The data type is not optional decoration for AB CIP / TwinCAT / FOCAS — their tag-definition + /// factories decode the wire bytes by this field, not by the raw tag row's OPC UA DataType. A + /// committed leaf left at the model default therefore reads real data through the wrong decoder and + /// publishes a plausible wrong number rather than an error (Gitea #536). + /// + /// /// MQTT is the one driver whose address is not a single string — a Sparkplug tag binds by a /// (group, edgeNode, device?, metric) tuple — so it is built from the browse session's stated /// instead. See . @@ -218,13 +229,25 @@ public static class RawBrowseCommitMapper if (Is(driverType, DriverTypeNames.OpcUaClient)) return new OpcUaClientTagConfigModel { NodeId = address }.ToJson(); if (Is(driverType, DriverTypeNames.AbCip)) - return new AbCipTagConfigModel { TagPath = address }.ToJson(); + return new AbCipTagConfigModel + { + TagPath = address, + DataType = NativeType(addressFields, AbCipDataType.DInt), + }.ToJson(); if (Is(driverType, DriverTypeNames.AbLegacy)) return new AbLegacyTagConfigModel { Address = address }.ToJson(); if (Is(driverType, DriverTypeNames.TwinCAT)) - return new TwinCATTagConfigModel { SymbolPath = address }.ToJson(); + return new TwinCATTagConfigModel + { + SymbolPath = address, + DataType = NativeType(addressFields, TwinCATDataType.DInt), + }.ToJson(); if (Is(driverType, DriverTypeNames.FOCAS)) - return new FocasTagConfigModel { Address = address }.ToJson(); + return new FocasTagConfigModel + { + Address = address, + DataType = NativeType(addressFields, FocasDataType.Int32), + }.ToJson(); if (Is(driverType, DriverTypeNames.S7)) return new S7TagConfigModel { Address = address }.ToJson(); // MTConnect: the DataItem id. The driver also accepts "address"/"dataItemId", but the typed @@ -338,6 +361,46 @@ public static class RawBrowseCommitMapper + "the tag manually."; } + /// + /// Reads the browse leaf's driver-native data type out of the stated address fields, falling + /// back to — the driver model's own default — when the browser + /// stated none or stated one this driver's enum does not name. + /// + /// The driver's own data-type enum. + /// The leaf's stated address fields. + /// The model default to keep when no usable native type was stated. + /// The parsed native type, or . + /// + /// The type is READ, never derived (Gitea #536). A commit used to leave this at the + /// model default, so every browsed AB CIP leaf — a REAL, a BOOL, a STRING — was written as + /// DInt and the driver decoded four bytes of a float as an integer: a plausible + /// number, no error, wrong value. Deriving it from the leaf's driver-AGNOSTIC + /// DriverDataType instead would fix the REAL and still get the integers wrong — + /// SInt, Int and DInt all collapse into Int32 on the way out, so + /// a 16-bit INT would come back as a 32-bit DINT. The producing driver knows its own type + /// and states it; see CapturedTreeBrowseSession.NativeTypeFields. + /// + /// Case-insensitive because the key travels as an enum NAME, and a driver is free to spell + /// it however its own enum does. An unparseable value keeps the default rather than failing + /// the commit: the address — the part that cannot be re-derived later — is still correct, + /// and the operator can fix the type in the typed editor. + /// + /// + /// A numeric string is refused before parsing. Enum.TryParse accepts "9" + /// for any enum and hands back whatever member sits at ordinal 9 — which Enum.IsDefined + /// then happily confirms — so a driver that stamped a number would bind a tag to a type nobody + /// named. Same trap MTConnectTagConfigModel documents for its own dataType. + /// + /// + private static TEnum NativeType(IReadOnlyDictionary? addressFields, TEnum fallback) + where TEnum : struct, Enum + => TryGetField(addressFields, "dataType") is { Length: > 0 } raw + && !char.IsAsciiDigit(raw[0]) && raw[0] is not ('-' or '+') + && Enum.TryParse(raw, ignoreCase: true, out var parsed) + && Enum.IsDefined(parsed) + ? parsed + : fallback; + /// Reads one non-blank address field, or null when it is absent or blank. /// The stated address fields, or null. /// The field to read. diff --git a/tests/Core/ZB.MOM.WW.OtOpcUa.Commons.Tests/Browsing/CapturedTreeBrowseSessionTests.cs b/tests/Core/ZB.MOM.WW.OtOpcUa.Commons.Tests/Browsing/CapturedTreeBrowseSessionTests.cs index 9aa3a433..76cbdfb2 100644 --- a/tests/Core/ZB.MOM.WW.OtOpcUa.Commons.Tests/Browsing/CapturedTreeBrowseSessionTests.cs +++ b/tests/Core/ZB.MOM.WW.OtOpcUa.Commons.Tests/Browsing/CapturedTreeBrowseSessionTests.cs @@ -70,6 +70,43 @@ public class CapturedTreeBrowseSessionTests alarmAttrs.Single().IsAlarm.ShouldBeTrue(); } + /// + /// A driver's OWN type name reaches the picker as the dataType address field, so the + /// browse-commit can stamp the driver-typed TagConfig with the type the device reported rather + /// than the tag model's default (Gitea #536). It cannot ride + /// : that is the driver-agnostic type, and several + /// native types share one agnostic value. + /// + [Fact] + public async Task AttributesCarryTheDriverNativeTypeForTheCommitPath() + { + var b = new CapturingAddressSpaceBuilder(); + b.Variable("Speed", "Speed", new DriverAttributeInfo( + "Pump_1_Speed", DriverDataType.Float32, false, null, + SecurityClassification.Operate, false, NativeDataType: "Real")); + await using var s = new CapturedTreeBrowseSession(b.Build()); + + var a = (await s.AttributesAsync("Pump_1_Speed", CancellationToken.None)).Single(); + + a.AddressFields.ShouldNotBeNull()["dataType"].ShouldBe("Real"); + a.DriverDataType.ShouldBe(DriverDataType.Float32.ToString()); // the agnostic type is unchanged + } + + /// + /// A driver that states no native type states NOTHING — the commit then keeps its model default + /// instead of a guess. An empty-but-present dictionary would be indistinguishable from "the + /// driver stamped a blank type", so the absence has to be represented as absence. + /// + [Fact] + public async Task AttributesCarryNoAddressFieldsWhenTheDriverStatedNoNativeType() + { + await using var s = new CapturedTreeBrowseSession(SampleTree()); + + var a = (await s.AttributesAsync("Plc1.Speed", CancellationToken.None)).Single(); + + a.AddressFields.ShouldBeNull(); + } + [Fact] public async Task TruncatedTreeAppendsMarkerFolderAtRoot() { diff --git a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip.IntegrationTests/Docker/README.md b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip.IntegrationTests/Docker/README.md index a4478384..1c9b3844 100644 --- a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip.IntegrationTests/Docker/README.md +++ b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip.IntegrationTests/Docker/README.md @@ -12,10 +12,12 @@ clone needs Docker Desktop and nothing else. |---|---| | [`Dockerfile`](Dockerfile) | Multi-stage: build from libplctag at pinned tag → copy binary into `debian:bookworm-slim` runtime image | | [`docker-compose.yml`](docker-compose.yml) | One service per family (`controllogix` / `compactlogix` / `micro800` / `guardlogix`); all bind `:44818` | -| [`patches/`](patches/) | Applied to the libplctag checkout before the build — see [Local patches](#local-patches) | +| [`patches/`](patches/) | Applied to the libplctag checkout before the build, in filename order — see [Local patches](#local-patches) | ## Local patches +### 0001 — serve the Symbol Object listing + `patches/0001-ab_server-serve-cip-symbol-object-listing.patch` teaches `ab_server` to answer **CIP service 0x55 (Get Instance Attribute List) on class 0x6B**, the Symbol Object listing a client reads through the `@tags` pseudo-tag to enumerate @@ -49,7 +51,28 @@ Partial-transfer paging is exercised the same way by starting `ab_server` with a few hundred tags: all of them should come back, with no duplicates and no gaps at the page boundaries. -**Bumping `LIBPLCTAG_TAG` means re-checking the patch against the new tag.** It +### 0002 — answer "tag does not exist" the way a controller does + +`patches/0002-ab_server-answer-tag-not-found-with-cip-0x04.patch` changes the reply +to an unresolvable tag path from CIP **0x03** (invalid parameter) to CIP **0x04** +(*IOI could not be deciphered or tag does not exist*), which is what a Logix +controller returns. libplctag translates the two very differently — `ErrorBadParam` +versus `ErrorNotFound` — and every client that classifies by status code reads the +first as "your request was malformed" and the second as "the controller answered, +that tag isn't there." + +The distinction is load-bearing here: the driver's Test Connect probe falls back to a +name no controller defines, so **"tag not found" is the successful answer** when a +device has no authored tags yet. Before this patch the fixture reported a perfectly +healthy sim as a failed handshake, and — worse — the discrepancy was written down as +a fixture limitation, which hid a real driver defect behind it for as long as it +stood (`AbCipDriverProbe.IsReachableException` matched the native C spellings, which +the .NET wrapper never produces, so the reachable classification could not fire at +all). `AbCip_WithNoAuthoredTags_ReportsTheControllerReachable` in +`Host.IntegrationTests` is the pin, and it needs the fixture **rebuilt** from this +patch, not merely running. + +**Bumping `LIBPLCTAG_TAG` means re-checking both patches against the new tag.** It is a concrete tag rather than the `release` branch precisely so that this stays a deliberate act — the branch moves, and the Dockerfile previously called it "pinned" while tracking it. @@ -117,11 +140,6 @@ place means updating both. unit tests with golden byte buffers. The Symbol Object patch above lists symbols, including their types; it does not add Template Object support, so a browsed UDT still cannot be expanded into members here. -- **No `@raw_cpu_type`** — the driver's Test Connect probe falls back to that - pseudo-tag when a device has no authored tags, and `ab_server` answers - `ErrorBadParam`. So **Test Connect reports failure against this fixture even - when the device is fine**, and browse works regardless. Author one tag and the - probe uses it instead. - **Controller-scope listing only** — a program-scope listing arrives with a leading symbolic segment naming the program, and `ab_server` has no notion of programs. `Program:`-scoped tags cannot be browsed here. diff --git a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip.IntegrationTests/Docker/patches/0002-ab_server-answer-tag-not-found-with-cip-0x04.patch b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip.IntegrationTests/Docker/patches/0002-ab_server-answer-tag-not-found-with-cip-0x04.patch new file mode 100644 index 00000000..3e7635e6 --- /dev/null +++ b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip.IntegrationTests/Docker/patches/0002-ab_server-answer-tag-not-found-with-cip-0x04.patch @@ -0,0 +1,30 @@ +diff --git a/src/tools/ab_server/cip.c b/src/tools/ab_server/cip.c +index 68a5794..f286504 100644 +--- a/src/tools/ab_server/cip.c ++++ b/src/tools/ab_server/cip.c +@@ -746,7 +746,11 @@ slice_s handle_read_request(uint8_t cip_service, slice_s cip_service_path, slice + if(!parse_tag_path(cip_service_path, plc, &tag, &num_indexes, &(indexes[0]))) { + log_info("Unable to parse tag path:"); + log_info_slice(cip_service_path); +- return make_cip_log_error(output, cip_service, CIP_ERR_INVALID_PARAM, false, 0); ++ /* A Logix controller answers an unresolvable tag path with 0x04 ("IOI could not be ++ * deciphered or tag does not exist"), which clients read as "tag not found" and ++ * distinguish from a malformed request. Answering 0x03 instead reported a live ++ * controller as a failed handshake to any client that classifies by status code. */ ++ return make_cip_log_error(output, cip_service, CIP_ERR_PATH_SEGMENT, false, 0); + } + + /* Record request arrival time for latency tracking */ +@@ -916,7 +920,11 @@ slice_s handle_write_request(uint8_t cip_service, slice_s cip_service_path, slic + if(!parse_tag_path(cip_service_path, plc, &tag, &num_indexes, &(indexes[0]))) { + log_info("Unable to parse tag path:"); + log_info_slice(cip_service_path); +- return make_cip_log_error(output, cip_service, CIP_ERR_INVALID_PARAM, false, 0); ++ /* A Logix controller answers an unresolvable tag path with 0x04 ("IOI could not be ++ * deciphered or tag does not exist"), which clients read as "tag not found" and ++ * distinguish from a malformed request. Answering 0x03 instead reported a live ++ * controller as a failed handshake to any client that classifies by status code. */ ++ return make_cip_log_error(output, cip_service, CIP_ERR_PATH_SEGMENT, false, 0); + } + + /* Record request arrival time for latency tracking */ diff --git a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip.Tests/AbCipDataTypeExtensionsTests.cs b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip.Tests/AbCipDataTypeExtensionsTests.cs new file mode 100644 index 00000000..79777a1a --- /dev/null +++ b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip.Tests/AbCipDataTypeExtensionsTests.cs @@ -0,0 +1,44 @@ +using Shouldly; +using Xunit; +using ZB.MOM.WW.OtOpcUa.Core.Abstractions; + +namespace ZB.MOM.WW.OtOpcUa.Driver.AbCip.Tests; + +/// +/// Pins the one property of that another +/// subsystem depends on: it is LOSSY, so it cannot be run backwards. +/// +[Trait("Category", "Unit")] +public sealed class AbCipDataTypeExtensionsTests +{ + /// + /// Four distinct Logix integer widths leave the driver as one driver-agnostic type. This is why a + /// browse-commit carries the native type instead of deriving it from the leaf's + /// (Gitea #536) — a reverse map would have to pick one width for all + /// four, and picking wrong reads the wrong number of bytes with no error raised anywhere. + /// + [Fact] + public void The_integer_widths_collapse_which_is_why_the_native_type_is_carried() + { + var widths = new[] + { + AbCipDataType.SInt, AbCipDataType.Int, AbCipDataType.DInt, + AbCipDataType.USInt, AbCipDataType.UInt, + }; + + widths.Select(t => t.ToDriverDataType()).Distinct().ShouldHaveSingleItem() + .ShouldBe(DriverDataType.Int32); + } + + /// + /// The falsifying half: an unsigned 32-bit tag does NOT collapse with the rest, so a test that only + /// showed "several types share an output" would pass against a map that collapsed everything. + /// + [Fact] + public void The_wider_unsigned_types_stay_distinct() + { + AbCipDataType.UDInt.ToDriverDataType().ShouldBe(DriverDataType.UInt32); + AbCipDataType.ULInt.ToDriverDataType().ShouldBe(DriverDataType.UInt64); + AbCipDataType.Real.ToDriverDataType().ShouldBe(DriverDataType.Float32); + } +} diff --git a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip.Tests/AbCipDriverDiscoveryTests.cs b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip.Tests/AbCipDriverDiscoveryTests.cs index 5be7525d..439a5bda 100644 --- a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip.Tests/AbCipDriverDiscoveryTests.cs +++ b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip.Tests/AbCipDriverDiscoveryTests.cs @@ -34,6 +34,36 @@ public sealed class AbCipDriverDiscoveryTests builder.Variables.Single(v => v.BrowseName == "Temperature").Info.SecurityClass.ShouldBe(SecurityClassification.ViewOnly); } + /// + /// Every emitted variable states its Logix type, so a browse-commit binds the tag with the type + /// the controller has rather than the tag model's DInt default (Gitea #536). The + /// driver-agnostic cannot serve — SInt, + /// Int and DInt all arrive there as . + /// + [Fact] + public async Task Emitted_variables_state_their_Logix_type_for_the_commit_path() + { + var builder = new RecordingBuilder(); + var drv = new AbCipDriver(new AbCipDriverOptions + { + Devices = [new AbCipDeviceOptions("ab://10.0.0.5/1,0")], + RawTags = AbCipRawTags.From( + new AbCipTagDefinition("Speed", "ab://10.0.0.5/1,0", "Motor1.Speed", AbCipDataType.Real), + new AbCipTagDefinition("Count", "ab://10.0.0.5/1,0", "C", AbCipDataType.Int)), + }, "drv-1"); + await drv.InitializeAsync("{}", CancellationToken.None); + + await drv.DiscoverAsync(builder, CancellationToken.None); + + builder.Variables.Single(v => v.BrowseName == "Speed").Info.NativeDataType.ShouldBe("Real"); + + // Int and DInt are indistinguishable downstream — this pair is the whole reason the native + // name is carried, so assert both halves rather than only the one that differs by kind. + var count = builder.Variables.Single(v => v.BrowseName == "Count").Info; + count.NativeDataType.ShouldBe("Int"); + count.DriverDataType.ShouldBe(DriverDataType.Int32); + } + /// Verifies that device folder display name falls back to host address when not provided. [Fact] public async Task Device_folder_displayname_falls_back_to_host_address() diff --git a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip.Tests/AbCipDriverProbeTests.cs b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip.Tests/AbCipDriverProbeTests.cs index c4dd326a..ba406b9f 100644 --- a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip.Tests/AbCipDriverProbeTests.cs +++ b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip.Tests/AbCipDriverProbeTests.cs @@ -1,5 +1,6 @@ using System.Net; using System.Net.Sockets; +using libplctag; using Shouldly; using Xunit; @@ -144,4 +145,45 @@ public sealed class AbCipDriverProbeTests $"Expected 'timed out' or 'Connect failed' but got: {result.Message}"); result.Latency.ShouldBeNull(); } + + // ------------------------------------------------------------------------- + // 5. Reachable-vs-failed classification (offline — no controller needed) + // ------------------------------------------------------------------------- + + /// + /// The exception path classifies by the SAME rule as the status path. libplctag.NET builds + /// with Status.ToString(), so its message is + /// "ErrorNotFound" — this method used to look for the native C spelling "STATUS_NOT_FOUND", + /// which that message can never contain, so a controller that answered "tag does not exist" + /// was reported as a failed handshake. That is the answer every Test Connect gets on a device + /// with no authored tags, i.e. every device at the moment an operator first presses the button. + /// + /// The status libplctag threw on. + /// Whether the controller should be reported reachable. + [Theory] + [InlineData(Status.ErrorNotFound, true)] + [InlineData(Status.ErrorNoMatch, true)] + [InlineData(Status.ErrorBadDevice, true)] + [InlineData(Status.ErrorTimeout, false)] + [InlineData(Status.ErrorBadConnection, false)] + [InlineData(Status.ErrorBadGateway, false)] + [InlineData(Status.ErrorBadParam, false)] + public void ThrownStatus_IsClassifiedTheSameWayAsAReturnedStatus(Status status, bool expectedReachable) + { + // Exactly how NativeTagWrapper.ThrowIfStatusNotOk constructs it. + var ex = new LibPlcTagException(status.ToString()); + + AbCipDriverProbe.IsReachableException(ex).ShouldBe(expectedReachable); + AbCipDriverProbe.IsReachableStatus(status).ShouldBe(expectedReachable); + } + + /// + /// A message that names no status at all stays UNreachable rather than defaulting green — the + /// conservative half, and the one an over-eager "treat anything we cannot parse as reachable" + /// fix would break. + /// + [Fact] + public void AnUnrecognisedExceptionMessageIsNotTreatedAsReachable() + => AbCipDriverProbe.IsReachableException( + new LibPlcTagException("something the wrapper has never said")).ShouldBeFalse(); } diff --git a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.FOCAS.Tests/FocasCapabilityTests.cs b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.FOCAS.Tests/FocasCapabilityTests.cs index 54c6243d..3bbad4e8 100644 --- a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.FOCAS.Tests/FocasCapabilityTests.cs +++ b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.FOCAS.Tests/FocasCapabilityTests.cs @@ -36,6 +36,13 @@ public sealed class FocasCapabilityTests // Writable field, because WireFocasClient.WriteAsync always returns BadNotWritable. builder.Variables.Single(v => v.BrowseName == "Run").Info.SecurityClass.ShouldBe(SecurityClassification.ViewOnly); builder.Variables.Single(v => v.BrowseName == "Alarm").Info.SecurityClass.ShouldBe(SecurityClassification.ViewOnly); + + // Each variable states its FOCAS type for the browse-commit path (Gitea #536). Byte is the + // case that matters: it reaches Info.DriverDataType as Int32, exactly like Int16 and Int32, + // so a commit deriving the type from there would read four PMC bytes for a one-byte tag. + var run = builder.Variables.Single(v => v.BrowseName == "Run").Info; + run.NativeDataType.ShouldBe("Byte"); + run.DriverDataType.ShouldBe(DriverDataType.Int32); } // ---- ISubscribable ---- diff --git a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.TwinCAT.Tests/TwinCATCapabilityTests.cs b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.TwinCAT.Tests/TwinCATCapabilityTests.cs index a3fa2752..ff43a6a4 100644 --- a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.TwinCAT.Tests/TwinCATCapabilityTests.cs +++ b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.TwinCAT.Tests/TwinCATCapabilityTests.cs @@ -34,6 +34,12 @@ public sealed class TwinCATCapabilityTests builder.Folders.ShouldContain(f => f.BrowseName == "ads://5.23.91.23.1.1:851" && f.DisplayName == "Mach1"); builder.Variables.Single(v => v.BrowseName == "Speed").Info.SecurityClass.ShouldBe(SecurityClassification.Operate); builder.Variables.Single(v => v.BrowseName == "Status").Info.SecurityClass.ShouldBe(SecurityClassification.ViewOnly); + + // Each variable states its IEC type, so a browse-commit binds the tag with the type the PLC + // declares rather than TwinCATTagConfigModel's DInt default (Gitea #536). It cannot be + // recovered from Info.DriverDataType — Byte/Int16/Int32 all land there as Int32. + builder.Variables.Single(v => v.BrowseName == "Status").Info.NativeDataType.ShouldBe("Bool"); + builder.Variables.Single(v => v.BrowseName == "Speed").Info.NativeDataType.ShouldBe("DInt"); } // ---- ISubscribable ---- diff --git a/tests/Server/ZB.MOM.WW.OtOpcUa.AdminUI.Tests/Uns/RawBrowseCommitMapperTests.cs b/tests/Server/ZB.MOM.WW.OtOpcUa.AdminUI.Tests/Uns/RawBrowseCommitMapperTests.cs index 832496c1..d4cb90b9 100644 --- a/tests/Server/ZB.MOM.WW.OtOpcUa.AdminUI.Tests/Uns/RawBrowseCommitMapperTests.cs +++ b/tests/Server/ZB.MOM.WW.OtOpcUa.AdminUI.Tests/Uns/RawBrowseCommitMapperTests.cs @@ -6,6 +6,7 @@ using ZB.MOM.WW.OtOpcUa.AdminUI.Uns; using ZB.MOM.WW.OtOpcUa.Commons.Types; using ZB.MOM.WW.OtOpcUa.Configuration.Enums; using ZB.MOM.WW.OtOpcUa.Core.Abstractions; +using ZB.MOM.WW.OtOpcUa.Driver.AbCip; using ZB.MOM.WW.OtOpcUa.Driver.Mqtt; namespace ZB.MOM.WW.OtOpcUa.AdminUI.Tests.Uns; @@ -98,6 +99,126 @@ public sealed class RawBrowseCommitMapperTests JsonNode.Parse(json)!.AsObject()["address"]!.GetValue().ShouldBe("40001"); } + // ---- BuildTagConfig: the browsed data type (Gitea #536) -------------------------------------- + + /// The address fields a captured browse leaf states for its driver-native type. + /// The driver's own data-type enum name, as the driver stamped it. + /// The stated address fields. + private static Dictionary NativeTypeFields(string nativeType) => + new() { ["dataType"] = nativeType }; + + /// + /// The committed dataType is the type the CONTROLLER reported, not the model default. + /// AB CIP / TwinCAT / FOCAS decode the wire bytes by this field, so a browsed REAL left at the + /// DInt default reads four bytes of a float as an integer and publishes a plausible + /// wrong number with no error anywhere. + /// + /// The owning device's driver type. + /// The native type the browse leaf stated. + /// The leaf's driver-side reference. + [Theory] + [InlineData("AbCip", "Real", "Pump_1_Speed")] + [InlineData("AbCip", "Bool", "Motor_Run")] + [InlineData("AbCip", "String", "Recipe_Name")] + [InlineData("TwinCAT", "Bool", "MAIN.bStart")] + [InlineData("FOCAS", "Float64", "MACRO100")] + public void BuildTagConfig_stamps_the_browsed_data_type( + string driverType, string nativeType, string reference) + { + var json = RawBrowseCommitMapper.BuildTagConfig(driverType, reference, NativeTypeFields(nativeType)); + + JsonNode.Parse(json)!.AsObject()["dataType"]!.GetValue().ShouldBe(nativeType); + } + + /// + /// The narrow integer widths are the reason the native type is CARRIED rather than derived from + /// the leaf's driver-agnostic DriverDataType: SInt, Int and DInt all + /// leave the driver as Int32, so a reverse map would read a 16-bit INT through a 32-bit + /// decoder — two bytes of the next tag's memory included — and still look like a fix. + /// + /// The Logix integer width the controller reported. + [Theory] + [InlineData("SInt")] + [InlineData("Int")] + [InlineData("DInt")] + [InlineData("UDInt")] + public void BuildTagConfig_keeps_integer_widths_that_the_agnostic_type_collapses(string nativeType) + { + // That these widths really do collapse on the way out of the driver is pinned next to the map + // itself — AbCipDataTypeExtensionsTests.The_integer_widths_collapse_which_is_why_the_native_type_is_carried. + var json = RawBrowseCommitMapper.BuildTagConfig("AbCip", "Line1_Count", NativeTypeFields(nativeType)); + + JsonNode.Parse(json)!.AsObject()["dataType"]!.GetValue().ShouldBe(nativeType); + } + + /// + /// A leaf that states no type — a browser that reports none, or a session captured before the + /// driver stamped it — keeps the model default rather than guessing. That is the pre-#536 + /// behaviour, deliberately retained as the fallback. + /// + /// The owning device's driver type. + /// The model default the commit must fall back to. + [Theory] + [InlineData("AbCip", "DInt")] + [InlineData("TwinCAT", "DInt")] + [InlineData("FOCAS", "Int32")] + public void BuildTagConfig_falls_back_to_the_model_default_when_no_type_is_stated( + string driverType, string expectedDefault) + { + var noFields = RawBrowseCommitMapper.BuildTagConfig(driverType, "X"); + var unparseable = RawBrowseCommitMapper.BuildTagConfig( + driverType, "X", NativeTypeFields("NotATypeThisDriverKnows")); + + JsonNode.Parse(noFields)!.AsObject()["dataType"]!.GetValue().ShouldBe(expectedDefault); + JsonNode.Parse(unparseable)!.AsObject()["dataType"]!.GetValue().ShouldBe(expectedDefault); + } + + /// + /// A numeric string is NOT a type name. Enum.TryParse accepts "3" for any enum and yields + /// whatever member happens to sit at 3, so an unguarded parse would silently bind a tag to a type + /// nobody named — the same trap MTConnectTagConfigModel documents for its own dataType. + /// + [Fact] + public void BuildTagConfig_rejects_a_numeric_type_rather_than_indexing_the_enum() + { + // AbCipDataType member 9 is Real; a numeric parse would hand back exactly that. + ((AbCipDataType)9).ShouldBe(AbCipDataType.Real); + + var json = RawBrowseCommitMapper.BuildTagConfig("AbCip", "X", NativeTypeFields("9")); + + JsonNode.Parse(json)!.AsObject()["dataType"]!.GetValue().ShouldBe("DInt"); + } + + /// + /// The stated type never displaces the address — the two travel in different fields and a commit + /// that got the type right by losing the binding would be no better than the defect. + /// + [Fact] + public void BuildTagConfig_stamps_the_type_without_disturbing_the_address() + { + var json = RawBrowseCommitMapper.BuildTagConfig( + "AbCip", "Program:Main.Motor.Speed", NativeTypeFields("Real")); + + var o = JsonNode.Parse(json)!.AsObject(); + o["tagPath"]!.GetValue().ShouldBe("Program:Main.Motor.Speed"); + o["dataType"]!.GetValue().ShouldBe("Real"); + } + + /// + /// A driver whose tag config carries no data type is unaffected — the key must not appear, or a + /// typed editor would round-trip a field its model does not own. + /// + /// The owning device's driver type. + [Theory] + [InlineData("OpcUaClient")] + [InlineData("GalaxyMxGateway")] + public void BuildTagConfig_does_not_invent_a_data_type_for_a_driver_that_has_none(string driverType) + { + var json = RawBrowseCommitMapper.BuildTagConfig(driverType, "X", NativeTypeFields("Real")); + + JsonNode.Parse(json)!.AsObject().ContainsKey("dataType").ShouldBeFalse(); + } + // ---- BuildTagConfig: MQTT (the address is a DESCRIPTOR, not a single reference string) -------- /// The Sparkplug address a browse session states for a metric under a device. diff --git a/tests/Server/ZB.MOM.WW.OtOpcUa.Host.IntegrationTests/DriverProbeHandshakeE2eTests.cs b/tests/Server/ZB.MOM.WW.OtOpcUa.Host.IntegrationTests/DriverProbeHandshakeE2eTests.cs index 25de396a..850b6553 100644 --- a/tests/Server/ZB.MOM.WW.OtOpcUa.Host.IntegrationTests/DriverProbeHandshakeE2eTests.cs +++ b/tests/Server/ZB.MOM.WW.OtOpcUa.Host.IntegrationTests/DriverProbeHandshakeE2eTests.cs @@ -173,11 +173,7 @@ public sealed class DriverProbeHandshakeE2eTests { SkipUnless(DockerHost, AbCipPort); // Probe an explicit tag that the ab_server ControlLogix sim actually defines - // (`TestDINT:DINT[1]`). The no-tags fallback (`@raw_cpu_type`) is NOT exercised here: - // ab_server answers an unknown/unsupported tag with libplctag ErrorBadParam (a REAL - // ControlLogix instead returns ErrorNotFound, which the probe classifies as - // reachable). Whether the `@raw_cpu_type` system-tag fallback is valid on a real - // ControlLogix is a hardware-gated follow-up (AbCipDriverOptions.cs flags it deferred). + // (`TestDINT:DINT[1]`). The no-tags fallback is exercised separately, below. // ⚠️ The config must use the v3 `RawTags` shape — `[{RawPath, TagConfig, WriteIdempotent, // DeviceName}]`, where the Logix tag path lives inside the TagConfig blob as `tagPath`. This // test passed the retired v2 `Tags: [{DeviceHostAddress, TagPath}]` shape until 2026-08-01, so @@ -200,4 +196,34 @@ public sealed class DriverProbeHandshakeE2eTests // ever resolved. Reading the authored tag is the entire point of this test. result.Message.ShouldBe("CIP session OK"); } + + /// + /// Test Connect on a device with NO authored tags — which is every AB CIP device at the moment + /// an operator first presses the button — must report the controller as reachable. + /// + /// + /// The probe falls back to a name no controller defines, so the green answer arrives as a CIP + /// "tag does not exist" (0x04 → libplctag ErrorNotFound), which + /// AbCipDriverProbe.IsReachableStatus classifies as reachable. Until 2026-08-01 that + /// classification had never been exercised against any backend: ab_server answered an + /// unresolvable tag path with 0x03 (ErrorBadParam) where a Logix controller answers 0x04, + /// so the fixture reported a healthy sim as a failed handshake and the discrepancy was written + /// down as a fixture limitation rather than fixed. It is now fixed in the vendored patch — + /// Docker/patches/0002-ab_server-answer-tag-not-found-with-cip-0x04.patch — so this test + /// needs the fixture rebuilt from that patch, not merely running. + /// + [Fact] + public async Task AbCip_WithNoAuthoredTags_ReportsTheControllerReachable() + { + SkipUnless(DockerHost, AbCipPort); + var result = await new AbCipDriverProbe().ProbeAsync( + $"{{\"Devices\":[{{\"HostAddress\":\"ab://{DockerHost}:{AbCipPort}/1,0\"}}],\"RawTags\":[]}}", + Timeout, Ct); + + result.Ok.ShouldBeTrue($"Probe message: {result.Message}"); + + // EXACT for the same reason as the test above, and here it carries the extra weight of proving + // WHICH green this is: the fallback's "probe tag not found", not a tag that was actually read. + result.Message.ShouldBe("CIP session OK (controller reachable; probe tag not found)"); + } }