diff --git a/src/ZB.MOM.WW.OtOpcUa.Core.Abstractions/DriverAttributeInfo.cs b/src/ZB.MOM.WW.OtOpcUa.Core.Abstractions/DriverAttributeInfo.cs index dc9a7fb..93e1ee8 100644 --- a/src/ZB.MOM.WW.OtOpcUa.Core.Abstractions/DriverAttributeInfo.cs +++ b/src/ZB.MOM.WW.OtOpcUa.Core.Abstractions/DriverAttributeInfo.cs @@ -45,6 +45,13 @@ namespace ZB.MOM.WW.OtOpcUa.Core.Abstractions; /// Set when is — /// stable logical id the ScriptedAlarmEngine addresses by. Null otherwise. /// +/// +/// Human-readable description for this attribute. When non-null + non-empty the generic +/// node-manager surfaces the value as the OPC UA Description attribute on the +/// Variable node so SCADA / engineering clients see the field comment from the source +/// project (Studio 5000 tag descriptions, Galaxy attribute help text, etc.). Defaults to +/// null so drivers that don't carry descriptions are unaffected. +/// public sealed record DriverAttributeInfo( string FullName, DriverDataType DriverDataType, @@ -56,7 +63,8 @@ public sealed record DriverAttributeInfo( bool WriteIdempotent = false, NodeSourceKind Source = NodeSourceKind.Driver, string? VirtualTagId = null, - string? ScriptedAlarmId = null); + string? ScriptedAlarmId = null, + string? Description = null); /// /// Per ADR-002 — discriminates which runtime subsystem owns this node's Read/Write/ diff --git a/src/ZB.MOM.WW.OtOpcUa.Driver.AbCip/AbCipDriver.cs b/src/ZB.MOM.WW.OtOpcUa.Driver.AbCip/AbCipDriver.cs index e1935e6..6287fe0 100644 --- a/src/ZB.MOM.WW.OtOpcUa.Driver.AbCip/AbCipDriver.cs +++ b/src/ZB.MOM.WW.OtOpcUa.Driver.AbCip/AbCipDriver.cs @@ -959,7 +959,8 @@ public sealed class AbCipDriver : IDriver, IReadable, IWritable, ITagDiscovery, : SecurityClassification.ViewOnly, IsHistorized: false, IsAlarm: false, - WriteIdempotent: member.WriteIdempotent)); + WriteIdempotent: member.WriteIdempotent, + Description: member.Description)); } continue; } @@ -1019,7 +1020,8 @@ public sealed class AbCipDriver : IDriver, IReadable, IWritable, ITagDiscovery, : SecurityClassification.ViewOnly, IsHistorized: false, IsAlarm: false, - WriteIdempotent: tag.WriteIdempotent); + WriteIdempotent: tag.WriteIdempotent, + Description: tag.Description); /// Count of registered devices — exposed for diagnostics + tests. internal int DeviceCount => _devices.Count; diff --git a/src/ZB.MOM.WW.OtOpcUa.Driver.AbCip/AbCipDriverOptions.cs b/src/ZB.MOM.WW.OtOpcUa.Driver.AbCip/AbCipDriverOptions.cs index cba983d..deedda2 100644 --- a/src/ZB.MOM.WW.OtOpcUa.Driver.AbCip/AbCipDriverOptions.cs +++ b/src/ZB.MOM.WW.OtOpcUa.Driver.AbCip/AbCipDriverOptions.cs @@ -120,6 +120,10 @@ public sealed record AbCipDeviceOptions( /// and GetString / SetString truncate at the right boundary. null /// keeps libplctag's default 82-byte STRING behaviour for back-compat. Ignored for /// non- types. +/// Tag description carried from the L5K/L5X export (or set explicitly +/// in pre-declared config). Surfaces as the OPC UA Description attribute on the +/// produced Variable node so SCADA / engineering clients see the comment from the source +/// project. null leaves Description unset, matching pre-2.3 behaviour. public sealed record AbCipTagDefinition( string Name, string DeviceHostAddress, @@ -129,7 +133,8 @@ public sealed record AbCipTagDefinition( bool WriteIdempotent = false, IReadOnlyList? Members = null, bool SafetyTag = false, - int? StringLength = null); + int? StringLength = null, + string? Description = null); /// /// One declared member of a UDT tag. Name is the member identifier on the PLC (e.g. Speed, @@ -137,12 +142,18 @@ public sealed record AbCipTagDefinition( /// . Declaration-driven — the real CIP Template Object reader /// (class 0x6C) that would auto-discover member layouts lands as a follow-up PR. /// +/// +/// carries the per-member comment from L5K/L5X UDT definitions so +/// the OPC UA Variable nodes produced for individual members surface their descriptions too, +/// not just the top-level tag. +/// public sealed record AbCipStructureMember( string Name, AbCipDataType DataType, bool Writable = true, bool WriteIdempotent = false, - int? StringLength = null); + int? StringLength = null, + string? Description = null); /// /// One L5K-import entry. Either or must be diff --git a/src/ZB.MOM.WW.OtOpcUa.Driver.AbCip/Import/L5kIngest.cs b/src/ZB.MOM.WW.OtOpcUa.Driver.AbCip/Import/L5kIngest.cs index e9e2ee7..f66e066 100644 --- a/src/ZB.MOM.WW.OtOpcUa.Driver.AbCip/Import/L5kIngest.cs +++ b/src/ZB.MOM.WW.OtOpcUa.Driver.AbCip/Import/L5kIngest.cs @@ -55,7 +55,11 @@ public sealed class L5kIngest var atomic = TryMapAtomic(m.DataType); var memberType = atomic ?? AbCipDataType.Structure; var writable = !IsReadOnly(m.ExternalAccess) && !IsAccessNone(m.ExternalAccess); - members.Add(new AbCipStructureMember(m.Name, memberType, writable)); + members.Add(new AbCipStructureMember( + Name: m.Name, + DataType: memberType, + Writable: writable, + Description: m.Description)); } udtIndex[dt.Name] = members; } @@ -101,7 +105,8 @@ public sealed class L5kIngest TagPath: tagPath, DataType: dataType, Writable: writable, - Members: members)); + Members: members, + Description: t.Description)); } return new L5kIngestResult(tags, skippedAliases, skippedNoAccess); diff --git a/src/ZB.MOM.WW.OtOpcUa.Driver.AbCip/Import/L5kParser.cs b/src/ZB.MOM.WW.OtOpcUa.Driver.AbCip/Import/L5kParser.cs index 3216542..6836688 100644 --- a/src/ZB.MOM.WW.OtOpcUa.Driver.AbCip/Import/L5kParser.cs +++ b/src/ZB.MOM.WW.OtOpcUa.Driver.AbCip/Import/L5kParser.cs @@ -311,7 +311,8 @@ public static class L5kParser if (typePart.Length == 0) return null; var externalAccess = attributes.TryGetValue("ExternalAccess", out var ea) ? ea.Trim() : null; - return new L5kMember(name, typePart, arrayDim, externalAccess); + var description = attributes.TryGetValue("Description", out var d) ? Unquote(d) : null; + return new L5kMember(name, typePart, arrayDim, externalAccess, description); } // ---- helpers ----------------------------------------------------------- @@ -377,4 +378,9 @@ public sealed record L5kTag( public sealed record L5kDataType(string Name, IReadOnlyList Members); /// One member line inside a UDT definition. -public sealed record L5kMember(string Name, string DataType, int? ArrayDim, string? ExternalAccess); +public sealed record L5kMember( + string Name, + string DataType, + int? ArrayDim, + string? ExternalAccess, + string? Description = null); diff --git a/src/ZB.MOM.WW.OtOpcUa.Driver.AbCip/Import/L5xParser.cs b/src/ZB.MOM.WW.OtOpcUa.Driver.AbCip/Import/L5xParser.cs index 6ae6e96..67f5cda 100644 --- a/src/ZB.MOM.WW.OtOpcUa.Driver.AbCip/Import/L5xParser.cs +++ b/src/ZB.MOM.WW.OtOpcUa.Driver.AbCip/Import/L5xParser.cs @@ -163,11 +163,21 @@ public static class L5xParser arrayDim = dim; } + // Description child — same shape as on Tag nodes; sometimes wrapped in CDATA. + string? description = null; + var descNode = memberNode.SelectSingleNode("Description"); + if (descNode is not null) + { + var raw = descNode.Value; + if (!string.IsNullOrEmpty(raw)) description = raw.Trim(); + } + return new L5kMember( Name: name, DataType: dataType, ArrayDim: arrayDim, - ExternalAccess: string.IsNullOrEmpty(externalAccess) ? null : externalAccess); + ExternalAccess: string.IsNullOrEmpty(externalAccess) ? null : externalAccess, + Description: description); } private static L5kDataType? ReadAddOnInstruction(XPathNavigator aoiNode) @@ -200,11 +210,20 @@ public static class L5xParser arrayDim = dim; } + string? paramDescription = null; + var paramDescNode = paramNode.SelectSingleNode("Description"); + if (paramDescNode is not null) + { + var raw = paramDescNode.Value; + if (!string.IsNullOrEmpty(raw)) paramDescription = raw.Trim(); + } + members.Add(new L5kMember( Name: paramName, DataType: dataType, ArrayDim: arrayDim, - ExternalAccess: string.IsNullOrEmpty(externalAccess) ? null : externalAccess)); + ExternalAccess: string.IsNullOrEmpty(externalAccess) ? null : externalAccess, + Description: paramDescription)); } return new L5kDataType(name, members); } diff --git a/src/ZB.MOM.WW.OtOpcUa.Server/OpcUa/DriverNodeManager.cs b/src/ZB.MOM.WW.OtOpcUa.Server/OpcUa/DriverNodeManager.cs index e1331a7..f2220fb 100644 --- a/src/ZB.MOM.WW.OtOpcUa.Server/OpcUa/DriverNodeManager.cs +++ b/src/ZB.MOM.WW.OtOpcUa.Server/OpcUa/DriverNodeManager.cs @@ -178,6 +178,13 @@ public sealed class DriverNodeManager : CustomNodeManager2, IAddressSpaceBuilder NodeId = new NodeId(attributeInfo.FullName, NamespaceIndex), BrowseName = new QualifiedName(browseName, NamespaceIndex), DisplayName = new LocalizedText(displayName), + // Per Task #231 — surface the driver-supplied tag description as the OPC UA + // Description attribute on the Variable node. Drivers that don't carry + // descriptions pass null, leaving Description unset (the stack defaults to + // an empty LocalizedText, matching prior behaviour). + Description = string.IsNullOrEmpty(attributeInfo.Description) + ? null + : new LocalizedText(attributeInfo.Description), DataType = MapDataType(attributeInfo.DriverDataType), ValueRank = attributeInfo.IsArray ? ValueRanks.OneDimension : ValueRanks.Scalar, // Historized attributes get the HistoryRead access bit so the stack dispatches diff --git a/tests/ZB.MOM.WW.OtOpcUa.Driver.AbCip.Tests/AbCipDescriptionThreadingTests.cs b/tests/ZB.MOM.WW.OtOpcUa.Driver.AbCip.Tests/AbCipDescriptionThreadingTests.cs new file mode 100644 index 0000000..4db6566 --- /dev/null +++ b/tests/ZB.MOM.WW.OtOpcUa.Driver.AbCip.Tests/AbCipDescriptionThreadingTests.cs @@ -0,0 +1,179 @@ +using Shouldly; +using Xunit; +using ZB.MOM.WW.OtOpcUa.Core.Abstractions; +using ZB.MOM.WW.OtOpcUa.Driver.AbCip.Import; + +namespace ZB.MOM.WW.OtOpcUa.Driver.AbCip.Tests; + +/// +/// Task #231 — verifies that tag/member descriptions parsed from L5K and L5X exports thread +/// through / +/// + land on +/// on the produced address-space variables, so +/// downstream OPC UA Variable nodes carry the source-project comment as their Description +/// attribute. +/// +[Trait("Category", "Unit")] +public sealed class AbCipDescriptionThreadingTests +{ + private const string DeviceHost = "ab://10.0.0.5/1,0"; + + [Fact] + public void L5kParser_captures_member_description_from_attribute_block() + { + const string body = """ + DATATYPE MyUdt + MEMBER Speed : DINT (Description := "Belt speed in RPM"); + END_DATATYPE + """; + var doc = L5kParser.Parse(new StringL5kSource(body)); + + var member = doc.DataTypes.Single().Members.Single(); + member.Name.ShouldBe("Speed"); + member.Description.ShouldBe("Belt speed in RPM"); + } + + [Fact] + public void L5xParser_captures_member_description_child_node() + { + const string xml = """ + + + + + + + + + + + + + + + """; + var doc = L5xParser.Parse(new StringL5kSource(xml)); + + doc.DataTypes.Single().Members.Single().Description.ShouldBe("Belt speed in RPM"); + } + + [Fact] + public void L5kIngest_threads_tag_and_member_descriptions_into_AbCipTagDefinition() + { + const string body = """ + DATATYPE MotorBlock + MEMBER Speed : DINT (Description := "Setpoint RPM"); + MEMBER Status : DINT; + END_DATATYPE + TAG + Motor1 : MotorBlock (Description := "Conveyor motor 1") := []; + END_TAG + """; + var doc = L5kParser.Parse(new StringL5kSource(body)); + + var result = new L5kIngest { DefaultDeviceHostAddress = DeviceHost }.Ingest(doc); + + var tag = result.Tags.Single(); + tag.Description.ShouldBe("Conveyor motor 1"); + tag.Members.ShouldNotBeNull(); + var members = tag.Members!.ToDictionary(m => m.Name); + members["Speed"].Description.ShouldBe("Setpoint RPM"); + members["Status"].Description.ShouldBeNull(); + } + + [Fact] + public async Task DiscoverAsync_sets_Description_on_DriverAttributeInfo_for_atomic_tag() + { + var builder = new RecordingBuilder(); + var drv = new AbCipDriver(new AbCipDriverOptions + { + Devices = [new AbCipDeviceOptions(DeviceHost)], + Tags = + [ + new AbCipTagDefinition( + Name: "Speed", + DeviceHostAddress: DeviceHost, + TagPath: "Motor1.Speed", + DataType: AbCipDataType.DInt, + Description: "Belt speed in RPM"), + new AbCipTagDefinition( + Name: "NoDescription", + DeviceHostAddress: DeviceHost, + TagPath: "X", + DataType: AbCipDataType.DInt), + ], + }, "drv-1"); + await drv.InitializeAsync("{}", CancellationToken.None); + + await drv.DiscoverAsync(builder, CancellationToken.None); + + builder.Variables.Single(v => v.BrowseName == "Speed").Info.Description + .ShouldBe("Belt speed in RPM"); + // Tags without descriptions leave Info.Description null (back-compat path). + builder.Variables.Single(v => v.BrowseName == "NoDescription").Info.Description + .ShouldBeNull(); + } + + [Fact] + public async Task DiscoverAsync_sets_Description_on_DriverAttributeInfo_for_UDT_members() + { + var builder = new RecordingBuilder(); + var drv = new AbCipDriver(new AbCipDriverOptions + { + Devices = [new AbCipDeviceOptions(DeviceHost)], + Tags = + [ + new AbCipTagDefinition( + Name: "Motor1", + DeviceHostAddress: DeviceHost, + TagPath: "Motor1", + DataType: AbCipDataType.Structure, + Members: + [ + new AbCipStructureMember( + Name: "Speed", + DataType: AbCipDataType.DInt, + Description: "Setpoint RPM"), + new AbCipStructureMember( + Name: "Status", + DataType: AbCipDataType.DInt), + ]), + ], + }, "drv-1"); + await drv.InitializeAsync("{}", CancellationToken.None); + + await drv.DiscoverAsync(builder, CancellationToken.None); + + builder.Variables.Single(v => v.BrowseName == "Speed").Info.Description + .ShouldBe("Setpoint RPM"); + builder.Variables.Single(v => v.BrowseName == "Status").Info.Description + .ShouldBeNull(); + } + + // ---- helpers ---- + + private sealed class RecordingBuilder : IAddressSpaceBuilder + { + public List<(string BrowseName, string DisplayName)> Folders { get; } = new(); + public List<(string BrowseName, DriverAttributeInfo Info)> Variables { get; } = new(); + + public IAddressSpaceBuilder Folder(string browseName, string displayName) + { Folders.Add((browseName, displayName)); return this; } + + public IVariableHandle Variable(string browseName, string displayName, DriverAttributeInfo info) + { Variables.Add((browseName, info)); return new Handle(info.FullName); } + + public void AddProperty(string _, DriverDataType __, object? ___) { } + + private sealed class Handle(string fullRef) : IVariableHandle + { + public string FullReference => fullRef; + public IAlarmConditionSink MarkAsAlarmCondition(AlarmConditionInfo info) => new NullSink(); + } + + private sealed class NullSink : IAlarmConditionSink + { + public void OnTransition(AlarmEventArgs args) { } + } + } +}