Merge pull request '[abcip] AbCip — Descriptions to OPC UA Description' (#348) from auto/abcip/2.3 into auto/driver-gaps

src/ZB.MOM.WW.OtOpcUa.Core.Abstractions/DriverAttributeInfo.cs

@@ -45,6 +45,13 @@ namespace ZB.MOM.WW.OtOpcUa.Core.Abstractions;
 ///     Set when <paramref name="Source"/> is <see cref="NodeSourceKind.ScriptedAlarm"/> —
 ///     stable logical id the ScriptedAlarmEngine addresses by. Null otherwise.
 /// </param>
+/// <param name="Description">
+///     Human-readable description for this attribute. When non-null + non-empty the generic
+///     node-manager surfaces the value as the OPC UA <c>Description</c> 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.
+/// </param>
 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);
 
 /// <summary>
 ///     Per ADR-002 — discriminates which runtime subsystem owns this node's Read/Write/

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);
 
     /// <summary>Count of registered devices — exposed for diagnostics + tests.</summary>
     internal int DeviceCount => _devices.Count;

src/ZB.MOM.WW.OtOpcUa.Driver.AbCip/AbCipDriverOptions.cs

@@ -120,6 +120,10 @@ public sealed record AbCipDeviceOptions(
 ///     and <c>GetString</c> / <c>SetString</c> truncate at the right boundary. <c>null</c>
 ///     keeps libplctag's default 82-byte STRING behaviour for back-compat. Ignored for
 ///     non-<see cref="AbCipDataType.String"/> types.</param>
+/// <param name="Description">Tag description carried from the L5K/L5X export (or set explicitly
+///     in pre-declared config). Surfaces as the OPC UA <c>Description</c> attribute on the
+///     produced Variable node so SCADA / engineering clients see the comment from the source
+///     project. <c>null</c> leaves Description unset, matching pre-2.3 behaviour.</param>
 public sealed record AbCipTagDefinition(
     string Name,
     string DeviceHostAddress,
@@ -129,7 +133,8 @@ public sealed record AbCipTagDefinition(
     bool WriteIdempotent = false,
     IReadOnlyList<AbCipStructureMember>? Members = null,
     bool SafetyTag = false,
-    int? StringLength = null);
+    int? StringLength = null,
+    string? Description = null);
 
 /// <summary>
 ///     One declared member of a UDT tag. Name is the member identifier on the PLC (e.g. <c>Speed</c>,
@@ -137,12 +142,18 @@ public sealed record AbCipTagDefinition(
 ///     <see cref="AbCipTagDefinition"/>. Declaration-driven — the real CIP Template Object reader
 ///     (class 0x6C) that would auto-discover member layouts lands as a follow-up PR.
 /// </summary>
+/// <remarks>
+///     <see cref="Description"/> 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.
+/// </remarks>
 public sealed record AbCipStructureMember(
     string Name,
     AbCipDataType DataType,
     bool Writable = true,
     bool WriteIdempotent = false,
-    int? StringLength = null);
+    int? StringLength = null,
+    string? Description = null);
 
 /// <summary>
 ///     One L5K-import entry. Either <see cref="FilePath"/> or <see cref="InlineText"/> must be

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);

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<L5kMember> Members);
 
 /// <summary>One member line inside a UDT definition.</summary>
-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);

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);
     }

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

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;
+
+/// <summary>
+///     Task #231 — verifies that tag/member descriptions parsed from L5K and L5X exports thread
+///     through <see cref="AbCipTagDefinition.Description"/> /
+///     <see cref="AbCipStructureMember.Description"/> + land on
+///     <see cref="DriverAttributeInfo.Description"/> on the produced address-space variables, so
+///     downstream OPC UA Variable nodes carry the source-project comment as their Description
+///     attribute.
+/// </summary>
+[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 = """
+            <?xml version="1.0" encoding="UTF-8"?>
+            <RSLogix5000Content>
+              <Controller>
+                <DataTypes>
+                  <DataType Name="MyUdt">
+                    <Members>
+                      <Member Name="Speed" DataType="DINT" Dimension="0" ExternalAccess="Read/Write">
+                        <Description><![CDATA[Belt speed in RPM]]></Description>
+                      </Member>
+                    </Members>
+                  </DataType>
+                </DataTypes>
+              </Controller>
+            </RSLogix5000Content>
+            """;
+        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) { }
+        }
+    }
+}