diff --git a/scripts/e2e/test-galaxy.ps1 b/scripts/e2e/test-galaxy.ps1 index 3a93eb8..b158856 100644 --- a/scripts/e2e/test-galaxy.ps1 +++ b/scripts/e2e/test-galaxy.ps1 @@ -49,16 +49,18 @@ OtOpcUa server endpoint. Default opc.tcp://localhost:4840. .PARAMETER SourceNodeId - NodeId of the driver-sourced Galaxy tag (numeric, writable preferred). - Default matches the Phase 7 seed — `ns=2;s=p7-smoke-tag-source`. + NodeId of the driver-sourced Galaxy tag (numeric, writable preferred). NodeIds + are path-based per OPC UA Part 3 §5.2.2 — the default matches the Phase 7 seed + walking `p7-smoke-galaxy` (DriverInstanceId) → `lab-floor` → `galaxy-line` → + `reactor-1` → `Source` (Tag.Name). .PARAMETER VirtualNodeId - NodeId of the VirtualTag computed as Source × 2 (Phase 7 scripting). - Default matches the Phase 7 seed — `ns=2;s=p7-smoke-vt-derived`. + NodeId of the VirtualTag computed as Source × 2 (Phase 7 scripting). Same + path-based scheme, ending in the VirtualTag.Name (`Doubled`). .PARAMETER AlarmNodeId - NodeId of the scripted-alarm Condition (fires when Source > 50). - Default matches the Phase 7 seed — `ns=2;s=p7-smoke-al-overtemp`. + NodeId of the scripted-alarm Condition (fires when Source > 50). Same + path-based scheme, ending in ScriptedAlarm.Name (`OverTemp`). .PARAMETER AlarmTriggerValue Value written to -SourceNodeId to push it over the alarm threshold. @@ -90,9 +92,9 @@ param( [string]$OpcUaUrl = "opc.tcp://localhost:4840", - [string]$SourceNodeId = "ns=2;s=p7-smoke-tag-source", - [string]$VirtualNodeId = "ns=2;s=p7-smoke-vt-derived", - [string]$AlarmNodeId = "ns=2;s=p7-smoke-al-overtemp", + [string]$SourceNodeId = "ns=2;s=p7-smoke-galaxy/lab-floor/galaxy-line/reactor-1/Source", + [string]$VirtualNodeId = "ns=2;s=p7-smoke-galaxy/lab-floor/galaxy-line/reactor-1/Doubled", + [string]$AlarmNodeId = "ns=2;s=p7-smoke-galaxy/lab-floor/galaxy-line/reactor-1/OverTemp", [string]$AlarmTriggerValue = "75", [int]$ChangeWaitSec = 10, [int]$AlarmWaitSec = 10, diff --git a/src/ZB.MOM.WW.OtOpcUa.Core/OpcUa/EquipmentNodeWalker.cs b/src/ZB.MOM.WW.OtOpcUa.Core/OpcUa/EquipmentNodeWalker.cs index 143ae61..7ed2eea 100644 --- a/src/ZB.MOM.WW.OtOpcUa.Core/OpcUa/EquipmentNodeWalker.cs +++ b/src/ZB.MOM.WW.OtOpcUa.Core/OpcUa/EquipmentNodeWalker.cs @@ -1,3 +1,4 @@ +using System.Text.Json; using ZB.MOM.WW.OtOpcUa.Configuration.Entities; using ZB.MOM.WW.OtOpcUa.Core.Abstractions; @@ -157,7 +158,7 @@ public static class EquipmentNodeWalker private static void AddTagVariable(IAddressSpaceBuilder equipmentBuilder, Tag tag) { var attr = new DriverAttributeInfo( - FullName: tag.TagConfig, + FullName: ExtractFullName(tag.TagConfig), DriverDataType: ParseDriverDataType(tag.DataType), IsArray: false, ArrayDim: null, @@ -166,6 +167,38 @@ public static class EquipmentNodeWalker equipmentBuilder.Variable(tag.Name, tag.Name, attr); } + /// + /// Cross-driver TagConfig convention — the Config DB's CK_Tag_TagConfig_IsJson + /// check constraint requires TagConfig to be a JSON object, and every shipped driver + /// (Galaxy / Modbus / AB CIP / S7 / FOCAS / TwinCAT / ABLegacy) stores the wire-level + /// address in a top-level FullName field. Extracting it here keeps the walker + /// driver-agnostic while giving the driver the plain address string its backend + /// expects at read-time — the raw JSON would otherwise be passed verbatim to + /// IReadable.ReadAsync and the driver would fail to resolve the tag. + /// + /// + /// Falls back to the raw if it doesn't parse as JSON or + /// the FullName field is absent. This preserves the pre-refactor behaviour for + /// any legacy row that slipped past the check constraint or any future driver that + /// wants an opaque non-JSON reference. + /// + internal static string ExtractFullName(string tagConfig) + { + if (string.IsNullOrWhiteSpace(tagConfig)) return tagConfig; + try + { + using var doc = JsonDocument.Parse(tagConfig); + if (doc.RootElement.ValueKind == JsonValueKind.Object + && doc.RootElement.TryGetProperty("FullName", out var fullName) + && fullName.ValueKind == JsonValueKind.String) + { + return fullName.GetString() ?? tagConfig; + } + } + catch (JsonException) { /* fall through */ } + return tagConfig; + } + /// /// Parse (stored as the enum /// name string, decision #138) into the enum value. Unknown names fall back to diff --git a/src/ZB.MOM.WW.OtOpcUa.Server/OpcUa/DriverNodeManager.cs b/src/ZB.MOM.WW.OtOpcUa.Server/OpcUa/DriverNodeManager.cs index 513fa56..3b1874f 100644 --- a/src/ZB.MOM.WW.OtOpcUa.Server/OpcUa/DriverNodeManager.cs +++ b/src/ZB.MOM.WW.OtOpcUa.Server/OpcUa/DriverNodeManager.cs @@ -50,6 +50,15 @@ public sealed class DriverNodeManager : CustomNodeManager2, IAddressSpaceBuilder private FolderState? _driverRoot; private readonly Dictionary _variablesByFullRef = new(StringComparer.OrdinalIgnoreCase); + // NodeId-identifier (string) → driver FullReference. OPC UA Part 3 §5.2.2 requires NodeIds + // to be immutable across a node's lifetime, which precludes minting them from the driver's + // native address (a backend rename would change the NodeId and break every subscribed + // client). NodeIds are therefore path-based (`{driverId}/{folder-path}/{browseName}`) and + // this map recovers the driver-side FullReference for read/write/history dispatch. The + // fallback in lookups preserves the pre-refactor behaviour for any caller that still + // registered a variable via a FullRef-shaped NodeId. + private readonly Dictionary _fullRefByNodeId = new(StringComparer.Ordinal); + // PR 26: SecurityClassification per variable, populated during Variable() registration. // OnWriteValue looks up the classification here to gate the write by the session's roles. // Drivers never enforce authz themselves — the classification is discovery-time metadata @@ -170,12 +179,17 @@ public sealed class DriverNodeManager : CustomNodeManager2, IAddressSpaceBuilder { lock (Lock) { + // Path-based NodeId per OPC UA Part 3 §5.2.2 (NodeIds MUST NOT change across the + // node's lifetime). Shape `{driverId}/{folder-path}/{browseName}` is stable across + // driver-side renames of the underlying FullReference + keeps the identifier + // self-describing against the browse tree. + var nodeKey = $"{_currentFolder.NodeId.Identifier}/{browseName}"; var v = new BaseDataVariableState(_currentFolder) { SymbolicName = browseName, ReferenceTypeId = ReferenceTypeIds.Organizes, TypeDefinitionId = VariableTypeIds.BaseDataVariableType, - NodeId = new NodeId(attributeInfo.FullName, NamespaceIndex), + NodeId = new NodeId(nodeKey, NamespaceIndex), BrowseName = new QualifiedName(browseName, NamespaceIndex), DisplayName = new LocalizedText(displayName), DataType = MapDataType(attributeInfo.DriverDataType), @@ -197,6 +211,7 @@ public sealed class DriverNodeManager : CustomNodeManager2, IAddressSpaceBuilder _securityByFullRef[attributeInfo.FullName] = attributeInfo.SecurityClass; _writeIdempotentByFullRef[attributeInfo.FullName] = attributeInfo.WriteIdempotent; _sourceByFullRef[attributeInfo.FullName] = attributeInfo.Source; + _fullRefByNodeId[nodeKey] = attributeInfo.FullName; v.OnReadValue = OnReadValue; v.OnWriteValue = OnWriteValue; @@ -228,7 +243,7 @@ public sealed class DriverNodeManager : CustomNodeManager2, IAddressSpaceBuilder private ServiceResult OnReadValue(ISystemContext context, NodeState node, NumericRange indexRange, QualifiedName dataEncoding, ref object? value, ref StatusCode statusCode, ref DateTime timestamp) { - var fullRef = node.NodeId.Identifier as string ?? ""; + var fullRef = NodeIdToFullRef(node.NodeId); var source = _sourceByFullRef.TryGetValue(fullRef, out var s) ? s : NodeSourceKind.Driver; var readable = SelectReadable(source, _readable, _virtualReadable, _scriptedAlarmReadable); @@ -717,7 +732,7 @@ public sealed class DriverNodeManager : CustomNodeManager2, IAddressSpaceBuilder private ServiceResult OnWriteValue(ISystemContext context, NodeState node, NumericRange indexRange, QualifiedName dataEncoding, ref object? value, ref StatusCode statusCode, ref DateTime timestamp) { - var fullRef = node.NodeId.Identifier as string; + var fullRef = NodeIdToFullRef(node.NodeId); if (string.IsNullOrEmpty(fullRef)) return StatusCodes.BadNodeIdUnknown; // Per Phase 7 plan decision #6 — virtual tags + scripted alarms reject direct @@ -1102,7 +1117,23 @@ public sealed class DriverNodeManager : CustomNodeManager2, IAddressSpaceBuilder } } - private string? ResolveFullRef(NodeHandle handle) => handle.NodeId?.Identifier as string; + private string? ResolveFullRef(NodeHandle handle) + { + if (handle.NodeId is null) return null; + return NodeIdToFullRef(handle.NodeId); + } + + /// + /// Recover the driver-side FullReference for a given OPC UA . Looks + /// the identifier up in ; when no entry exists (e.g. for + /// legacy test fixtures that still register variables with FullRef-shaped NodeIds) we + /// fall through to the raw identifier string so those code paths keep working. + /// + private string NodeIdToFullRef(NodeId nodeId) + { + if (nodeId?.Identifier is not string key) return string.Empty; + return _fullRefByNodeId.TryGetValue(key, out var fullRef) ? fullRef : key; + } // Both the results list AND the parallel errors list must be populated — MasterNodeManager // merges them and the merged StatusCode is what the client sees. Leaving errors[i] at its diff --git a/tests/ZB.MOM.WW.OtOpcUa.Core.Tests/OpcUa/EquipmentNodeWalkerTests.cs b/tests/ZB.MOM.WW.OtOpcUa.Core.Tests/OpcUa/EquipmentNodeWalkerTests.cs index c656c85..8e9f089 100644 --- a/tests/ZB.MOM.WW.OtOpcUa.Core.Tests/OpcUa/EquipmentNodeWalkerTests.cs +++ b/tests/ZB.MOM.WW.OtOpcUa.Core.Tests/OpcUa/EquipmentNodeWalkerTests.cs @@ -258,6 +258,57 @@ public sealed class EquipmentNodeWalkerTests v.AttributeInfo.ScriptedAlarmId.ShouldBeNull(); } + [Fact] + public void ExtractFullName_unwraps_json_object_with_FullName_field() + { + EquipmentNodeWalker.ExtractFullName( + "{\"FullName\":\"MESReceiver_001.MoveInBatchID\",\"DataType\":\"Int32\"}") + .ShouldBe("MESReceiver_001.MoveInBatchID"); + } + + [Fact] + public void ExtractFullName_handles_S7_style_extra_fields() + { + EquipmentNodeWalker.ExtractFullName( + "{\"FullName\":\"DB1_DBW0\",\"Address\":\"DB1.DBW0\",\"DataType\":\"Int16\"}") + .ShouldBe("DB1_DBW0"); + } + + [Fact] + public void ExtractFullName_returns_raw_when_not_json() + { + // Drivers that opt out of JSON TagConfig still work — fallback preserves the literal + // string so the driver's IReadable sees whatever the row author stored. + EquipmentNodeWalker.ExtractFullName("raw-tag-ref").ShouldBe("raw-tag-ref"); + } + + [Fact] + public void ExtractFullName_returns_raw_when_json_missing_FullName_field() + { + EquipmentNodeWalker.ExtractFullName("{\"Address\":\"DB1.DBW0\"}") + .ShouldBe("{\"Address\":\"DB1.DBW0\"}"); + } + + [Fact] + public void Driver_tag_FullName_passes_through_from_TagConfig_json() + { + // The walker hands the driver the unwrapped FullName string so IReadable.ReadAsync + // sees the plain address, not the raw TagConfig JSON. Verifies the dispatch contract + // the path-based NodeId refactor relies on. + var eq = Eq("eq-1", "line-1", "oven-3"); + var tag = NewTag("t-1", "Temp", "Int32", "plc-01", "eq-1", + tagConfig: "{\"FullName\":\"plc-01/HR200\",\"DataType\":\"Int32\"}"); + var content = new EquipmentNamespaceContent( + [Area("area-1", "warsaw")], [Line("line-1", "area-1", "line-a")], + [eq], [tag]); + + var rec = new RecordingBuilder("root"); + EquipmentNodeWalker.Walk(rec, content); + + var v = rec.Children[0].Children[0].Children[0].Variables.Single(); + v.AttributeInfo.FullName.ShouldBe("plc-01/HR200"); + } + // ----- builders for test seed rows ----- private static UnsArea Area(string id, string name) => new() @@ -282,7 +333,8 @@ public sealed class EquipmentNodeWalkerTests MachineCode = "MC-" + name, }; - private static Tag NewTag(string tagId, string name, string dataType, string address, string? equipmentId) => new() + private static Tag NewTag(string tagId, string name, string dataType, string address, + string? equipmentId, string? tagConfig = null) => new() { TagRowId = Guid.NewGuid(), GenerationId = 1, @@ -292,7 +344,7 @@ public sealed class EquipmentNodeWalkerTests Name = name, DataType = dataType, AccessLevel = ZB.MOM.WW.OtOpcUa.Configuration.Enums.TagAccessLevel.ReadWrite, - TagConfig = address, + TagConfig = tagConfig ?? address, }; // ----- recording IAddressSpaceBuilder -----