diff --git a/docs/plans/2026-06-06-equipment-namespace-structure-milestone.md.tasks.json b/docs/plans/2026-06-06-equipment-namespace-structure-milestone.md.tasks.json index 62ad8929..11824f39 100644 --- a/docs/plans/2026-06-06-equipment-namespace-structure-milestone.md.tasks.json +++ b/docs/plans/2026-06-06-equipment-namespace-structure-milestone.md.tasks.json @@ -7,7 +7,7 @@ {"id": 1, "nativeTaskId": 87, "subject": "Task 1: Carry equipment signals in the composition + artifact", "status": "completed", "blockedBy": [86]}, {"id": 2, "nativeTaskId": 88, "subject": "Task 2: Materialise equipment signals in the live rebuild", "status": "completed", "blockedBy": [87]}, {"id": 3, "nativeTaskId": 89, "subject": "Task 3: Friendly browse names for UNS folders", "status": "completed", "blockedBy": [87]}, - {"id": 4, "nativeTaskId": 90, "subject": "Task 4: Idempotency + restart-safety", "status": "pending", "blockedBy": [88]}, + {"id": 4, "nativeTaskId": 90, "subject": "Task 4: Idempotency + restart-safety", "status": "completed", "blockedBy": [88]}, {"id": 5, "nativeTaskId": 91, "subject": "Task 5: docker-dev integration verification + tool support", "status": "pending", "blockedBy": [88, 89]} ], "lastUpdated": "2026-06-06" diff --git a/src/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer/Phase7Applier.cs b/src/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer/Phase7Applier.cs index 70ff35d4..9c991847 100644 --- a/src/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer/Phase7Applier.cs +++ b/src/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer/Phase7Applier.cs @@ -70,18 +70,19 @@ public sealed class Phase7Applier var changedCount = plan.ChangedEquipment.Count + plan.ChangedDrivers.Count + plan.ChangedAlarms.Count + - plan.ChangedGalaxyTags.Count; + plan.ChangedGalaxyTags.Count + plan.ChangedEquipmentTags.Count; var addedCount = plan.AddedEquipment.Count + plan.AddedDrivers.Count + plan.AddedAlarms.Count + - plan.AddedGalaxyTags.Count; + plan.AddedGalaxyTags.Count + plan.AddedEquipmentTags.Count; - // Any add/remove of Equipment, ScriptedAlarm, or Galaxy tag topology requires a real - // address-space rebuild. Driver-instance changes don't touch the address-space topology - // directly — they go through DriverHostActor's spawn-plan in Runtime. + // Any add/remove of Equipment, ScriptedAlarm, Galaxy tag, or Equipment tag topology requires + // a real address-space rebuild. Driver-instance changes don't touch the address-space + // topology directly — they go through DriverHostActor's spawn-plan in Runtime. var needsRebuild = plan.AddedEquipment.Count > 0 || plan.RemovedEquipment.Count > 0 || plan.AddedAlarms.Count > 0 || plan.RemovedAlarms.Count > 0 || - plan.AddedGalaxyTags.Count > 0 || plan.RemovedGalaxyTags.Count > 0; + plan.AddedGalaxyTags.Count > 0 || plan.RemovedGalaxyTags.Count > 0 || + plan.AddedEquipmentTags.Count > 0 || plan.RemovedEquipmentTags.Count > 0; if (needsRebuild) { @@ -211,15 +212,19 @@ public sealed class Phase7Applier SafeEnsureFolder(folderNodeId, parentNodeId: tag.EquipmentId, displayName: tag.FolderPath); } - // Variables: NodeId = FullName (the driver-side reference → read/write routing key). Parent - // is the FolderPath sub-folder when set, else the equipment folder directly. Like the Galaxy - // pass, per-variable idempotency relies on the sink's own EnsureVariable idempotency. + // Variables: NodeId is FOLDER-SCOPED ("/"), NOT the raw FullName — a driver + // ref (e.g. a Modbus register) is not unique across identical machines, so FullName-as-NodeId + // would collide in the sink (EnsureVariable keys on NodeId) and drop all but one machine's + // signal. The driver-side FullName lives on EquipmentTagPlan for the later values milestone to + // route by. Parent is the FolderPath sub-folder when set, else the equipment folder directly. + // Like the Galaxy pass, per-variable idempotency relies on the sink's own EnsureVariable. foreach (var tag in composition.EquipmentTags) { var parent = string.IsNullOrWhiteSpace(tag.FolderPath) ? tag.EquipmentId : EquipmentSubFolderNodeId(tag.EquipmentId, tag.FolderPath); - SafeEnsureVariable(tag.FullName, parent, tag.Name, tag.DataType); + var nodeId = $"{parent}/{tag.Name}"; + SafeEnsureVariable(nodeId, parent, tag.Name, tag.DataType); } _logger.LogInformation( diff --git a/src/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer/Phase7Composer.cs b/src/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer/Phase7Composer.cs index d22edc2f..8ea28f82 100644 --- a/src/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer/Phase7Composer.cs +++ b/src/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer/Phase7Composer.cs @@ -81,14 +81,18 @@ public sealed record GalaxyTagPlan( /// /// One Equipment-namespace tag from a row whose -/// is non-null and whose owning driver's namespace is Equipment-kind. Carries the parent -/// folder (already materialised by Phase7Applier.MaterialiseHierarchy) -/// the variable hangs under, the optional sub-folder, the leaf -/// display, the OPC UA , and the driver-side -/// reference (extracted from Tag.TagConfig) used as the variable -/// NodeId + read/write routing key. The equipment-signal analogue of . +/// is non-null and whose owning driver's namespace is Equipment-kind. Carries the stable +/// (diff identity), the parent folder (already +/// materialised by Phase7Applier.MaterialiseHierarchy) the variable hangs under, the +/// optional sub-folder, the leaf display, the OPC UA +/// , and the driver-side reference (extracted from +/// Tag.TagConfig) the later values milestone routes reads/writes by. The variable's NodeId +/// is folder-scoped (parent/Name), NOT , because a raw driver ref +/// (e.g. a Modbus register) is not unique across identical machines. The equipment-signal +/// analogue of . /// public sealed record EquipmentTagPlan( + string TagId, string EquipmentId, string DriverInstanceId, string FolderPath, @@ -218,9 +222,10 @@ public static class Phase7Composer && namespacesById.TryGetValue(di.NamespaceId, out var ns) && ns.Kind == NamespaceKind.Equipment) .OrderBy(t => t.EquipmentId, StringComparer.Ordinal) - .ThenBy(t => t.FolderPath, StringComparer.Ordinal) + .ThenBy(t => t.FolderPath ?? string.Empty, StringComparer.Ordinal) // coalesce so the sort matches the artifact-decode side exactly .ThenBy(t => t.Name, StringComparer.Ordinal) .Select(t => new EquipmentTagPlan( + TagId: t.TagId, EquipmentId: t.EquipmentId!, DriverInstanceId: t.DriverInstanceId, FolderPath: t.FolderPath ?? string.Empty, diff --git a/src/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer/Phase7Plan.cs b/src/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer/Phase7Plan.cs index de2e7c2b..16f09e3b 100644 --- a/src/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer/Phase7Plan.cs +++ b/src/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer/Phase7Plan.cs @@ -26,17 +26,33 @@ public sealed record Phase7Plan( IReadOnlyList RemovedGalaxyTags, IReadOnlyList ChangedGalaxyTags) { + /// + /// Equipment-namespace tag diff sets, keyed by . Added as + /// init-only members (defaulting empty) rather than positional parameters so existing + /// Phase7Plan construction sites compile unchanged — consistent with how + /// was added. Without these, an + /// incremental deploy that changes ONLY equipment tags produced an empty plan and + /// OpcUaPublishActor.HandleRebuild short-circuited before materialising them. + /// + public IReadOnlyList AddedEquipmentTags { get; init; } = Array.Empty(); + /// + public IReadOnlyList RemovedEquipmentTags { get; init; } = Array.Empty(); + /// + public IReadOnlyList ChangedEquipmentTags { get; init; } = Array.Empty(); + /// Gets a value indicating whether the composition plan contains no changes. public bool IsEmpty => AddedEquipment.Count == 0 && RemovedEquipment.Count == 0 && ChangedEquipment.Count == 0 && AddedDrivers.Count == 0 && RemovedDrivers.Count == 0 && ChangedDrivers.Count == 0 && AddedAlarms.Count == 0 && RemovedAlarms.Count == 0 && ChangedAlarms.Count == 0 && - AddedGalaxyTags.Count == 0 && RemovedGalaxyTags.Count == 0 && ChangedGalaxyTags.Count == 0; + AddedGalaxyTags.Count == 0 && RemovedGalaxyTags.Count == 0 && ChangedGalaxyTags.Count == 0 && + AddedEquipmentTags.Count == 0 && RemovedEquipmentTags.Count == 0 && ChangedEquipmentTags.Count == 0; public sealed record EquipmentDelta(EquipmentNode Previous, EquipmentNode Current); public sealed record DriverDelta(DriverInstancePlan Previous, DriverInstancePlan Current); public sealed record AlarmDelta(ScriptedAlarmPlan Previous, ScriptedAlarmPlan Current); public sealed record GalaxyTagDelta(GalaxyTagPlan Previous, GalaxyTagPlan Current); + public sealed record EquipmentTagDelta(EquipmentTagPlan Previous, EquipmentTagPlan Current); } public static class Phase7Planner @@ -74,11 +90,21 @@ public static class Phase7Planner t => t.TagId, (a, b) => new Phase7Plan.GalaxyTagDelta(a, b)); + var (addedEqTags, removedEqTags, changedEqTags) = DiffById( + previous.EquipmentTags, next.EquipmentTags, + t => t.TagId, + (a, b) => new Phase7Plan.EquipmentTagDelta(a, b)); + return new Phase7Plan( addedEq, removedEq, changedEq, addedDrv, removedDrv, changedDrv, addedAlarm, removedAlarm, changedAlarm, - addedGalaxy, removedGalaxy, changedGalaxy); + addedGalaxy, removedGalaxy, changedGalaxy) + { + AddedEquipmentTags = addedEqTags, + RemovedEquipmentTags = removedEqTags, + ChangedEquipmentTags = changedEqTags, + }; } private static (IReadOnlyList Added, IReadOnlyList Removed, IReadOnlyList Changed) diff --git a/src/Server/ZB.MOM.WW.OtOpcUa.Runtime/Drivers/DeploymentArtifact.cs b/src/Server/ZB.MOM.WW.OtOpcUa.Runtime/Drivers/DeploymentArtifact.cs index 38fc5b39..76f6f292 100644 --- a/src/Server/ZB.MOM.WW.OtOpcUa.Runtime/Drivers/DeploymentArtifact.cs +++ b/src/Server/ZB.MOM.WW.OtOpcUa.Runtime/Drivers/DeploymentArtifact.cs @@ -262,6 +262,7 @@ public static class DeploymentArtifact var equipmentId = eqEl.GetString(); if (string.IsNullOrWhiteSpace(equipmentId)) continue; + var tagId = el.TryGetProperty("TagId", out var tidEl) ? tidEl.GetString() : null; var di = el.TryGetProperty("DriverInstanceId", out var diEl) ? diEl.GetString() : null; var name = el.TryGetProperty("Name", out var nmEl) ? nmEl.GetString() : null; var folder = el.TryGetProperty("FolderPath", out var fpEl) && fpEl.ValueKind != JsonValueKind.Null @@ -270,11 +271,12 @@ public static class DeploymentArtifact var tagConfig = el.TryGetProperty("TagConfig", out var tcEl) && tcEl.ValueKind == JsonValueKind.String ? tcEl.GetString() : null; - if (string.IsNullOrWhiteSpace(di) || string.IsNullOrWhiteSpace(name)) continue; + if (string.IsNullOrWhiteSpace(tagId) || string.IsNullOrWhiteSpace(di) || string.IsNullOrWhiteSpace(name)) continue; if (!driverToNamespace.TryGetValue(di!, out var nsId)) continue; if (!equipmentNamespaces.Contains(nsId)) continue; result.Add(new EquipmentTagPlan( + TagId: tagId!, EquipmentId: equipmentId!, DriverInstanceId: di!, FolderPath: folder ?? string.Empty, diff --git a/tests/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer.Tests/Phase7ApplierHierarchyTests.cs b/tests/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer.Tests/Phase7ApplierHierarchyTests.cs index befdcdd7..46d3a95f 100644 --- a/tests/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer.Tests/Phase7ApplierHierarchyTests.cs +++ b/tests/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer.Tests/Phase7ApplierHierarchyTests.cs @@ -110,6 +110,54 @@ public sealed class Phase7ApplierHierarchyTests : IDisposable sdkServer.NodeManager!.FolderCount.ShouldBe(5); } + /// Verifies MaterialiseEquipmentTags is idempotent against a real SDK node manager: + /// applying the same composition twice yields a single Variable node (no duplicates). This is the + /// restart-safety guarantee — HandleRebuild runs on both the apply path and the + /// DriverHostActor.RestoreApplied bootstrap path (same RebuildAddressSpace message), so a node + /// restart re-runs this pass and must not double-materialise. + [Fact] + public async Task MaterialiseEquipmentTags_against_real_SDK_node_manager_is_idempotent() + { + await using var host = new OpcUaApplicationHost( + new OpcUaApplicationHostOptions + { + ApplicationName = "OtOpcUa.EquipmentTags", + ApplicationUri = $"urn:OtOpcUa.EquipmentTags:{Guid.NewGuid():N}", + OpcUaPort = AllocateFreePort(), + PublicHostname = "localhost", + PkiStoreRoot = _pkiRoot, + }, + NullLogger.Instance); + + var sdkServer = new OtOpcUaSdkServer(); + await host.StartAsync(sdkServer, Ct); + sdkServer.NodeManager.ShouldNotBeNull(); + + var sink = new SdkAddressSpaceSink(sdkServer.NodeManager!); + var applier = new Phase7Applier(sink, NullLogger.Instance); + + var composition = new Phase7CompositionResult( + UnsAreas: Array.Empty(), + UnsLines: Array.Empty(), + EquipmentNodes: new[] { new EquipmentNode("eq-1", "filling-eq", UnsLineId: "") }, + DriverInstancePlans: Array.Empty(), + ScriptedAlarmPlans: Array.Empty(), + GalaxyTags: Array.Empty()) + { + EquipmentTags = new[] + { + new EquipmentTagPlan("tag-1", "eq-1", "drv", FolderPath: "", Name: "Speed", DataType: "Float", FullName: "40001"), + }, + }; + + // Equipment folder first (the variable's parent), then the tag pass — applied twice. + applier.MaterialiseHierarchy(composition); + applier.MaterialiseEquipmentTags(composition); + applier.MaterialiseEquipmentTags(composition); + + sdkServer.NodeManager!.VariableCount.ShouldBe(1); // single variable despite the double-apply + } + private static int AllocateFreePort() { using var listener = new System.Net.Sockets.TcpListener(System.Net.IPAddress.Loopback, 0); diff --git a/tests/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer.Tests/Phase7ApplierTests.cs b/tests/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer.Tests/Phase7ApplierTests.cs index c9dabda1..1891680a 100644 --- a/tests/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer.Tests/Phase7ApplierTests.cs +++ b/tests/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer.Tests/Phase7ApplierTests.cs @@ -178,8 +178,9 @@ public sealed class Phase7ApplierTests } /// Verifies MaterialiseEquipmentTags creates one Variable per equipment tag directly - /// under its existing equipment folder (NodeId == FullName, parent == EquipmentId, - /// displayName == Name) and does NOT re-create the equipment folder (decision #4). + /// under its existing equipment folder, with a folder-scoped NodeId (parent/Name — NOT the raw + /// FullName), parent == EquipmentId, displayName == Name, and does NOT re-create the equipment + /// folder (decision #4). [Fact] public void MaterialiseEquipmentTags_creates_variable_under_equipment_folder() { @@ -196,18 +197,19 @@ public sealed class Phase7ApplierTests { EquipmentTags = new[] { - new EquipmentTagPlan("eq-1", "drv", FolderPath: "", Name: "Speed", DataType: "Float", FullName: "40001"), + new EquipmentTagPlan("tag-1", "eq-1", "drv", FolderPath: "", Name: "Speed", DataType: "Float", FullName: "40001"), }, }; applier.MaterialiseEquipmentTags(composition); sink.FolderCalls.ShouldBeEmpty(); // equipment folder already exists; no sub-folder needed - sink.VariableCalls.ShouldHaveSingleItem().ShouldBe(("40001", "eq-1", "Speed", "Float")); + sink.VariableCalls.ShouldHaveSingleItem().ShouldBe(("eq-1/Speed", "eq-1", "Speed", "Float")); } /// Verifies a FolderPath on an equipment tag becomes a sub-folder UNDER the equipment - /// folder (not the namespace root), with the variable parented to that sub-folder. + /// folder (not the namespace root), with the variable parented to that sub-folder and a + /// folder-scoped NodeId. [Fact] public void MaterialiseEquipmentTags_nests_FolderPath_subfolder_under_equipment() { @@ -219,14 +221,64 @@ public sealed class Phase7ApplierTests { EquipmentTags = new[] { - new EquipmentTagPlan("eq-1", "drv", FolderPath: "Diagnostics", Name: "Temp", DataType: "Float", FullName: "40002"), + new EquipmentTagPlan("tag-2", "eq-1", "drv", FolderPath: "Diagnostics", Name: "Temp", DataType: "Float", FullName: "40002"), }, }; applier.MaterialiseEquipmentTags(composition); sink.FolderCalls.ShouldHaveSingleItem().ShouldBe(("eq-1/Diagnostics", "eq-1", "Diagnostics")); - sink.VariableCalls.ShouldHaveSingleItem().ShouldBe(("40002", "eq-1/Diagnostics", "Temp", "Float")); + sink.VariableCalls.ShouldHaveSingleItem().ShouldBe(("eq-1/Diagnostics/Temp", "eq-1/Diagnostics", "Temp", "Float")); + } + + /// Regression for the FullName-as-NodeId collision: two identical machines exposing the + /// SAME driver FullName (e.g. Modbus register 40001) must produce TWO distinct variables — one + /// under each equipment folder — because the NodeId is folder-scoped, not the raw FullName. + [Fact] + public void MaterialiseEquipmentTags_identical_FullName_across_two_equipments_does_not_collide() + { + var sink = new RecordingSink(); + var applier = new Phase7Applier(sink, NullLogger.Instance); + + var composition = new Phase7CompositionResult( + Array.Empty(), Array.Empty(), Array.Empty()) + { + EquipmentTags = new[] + { + new EquipmentTagPlan("tag-a", "eq-1", "drv-1", FolderPath: "", Name: "Speed", DataType: "Float", FullName: "40001"), + new EquipmentTagPlan("tag-b", "eq-2", "drv-2", FolderPath: "", Name: "Speed", DataType: "Float", FullName: "40001"), + }, + }; + + applier.MaterialiseEquipmentTags(composition); + + sink.VariableCalls.Count.ShouldBe(2); + sink.VariableCalls.ShouldContain(("eq-1/Speed", "eq-1", "Speed", "Float")); + sink.VariableCalls.ShouldContain(("eq-2/Speed", "eq-2", "Speed", "Float")); + } + + /// Verifies that added equipment tags in an otherwise-empty plan trigger an + /// address-space rebuild (parity with the Galaxy-tag path — the planner now diffs equipment + /// tags, so a tags-only deploy is no longer a silent no-op). + [Fact] + public void Added_equipment_tags_trigger_rebuild() + { + var sink = new RecordingSink(); + var applier = new Phase7Applier(sink, NullLogger.Instance); + + var plan = EmptyPlan with + { + AddedEquipmentTags = new[] + { + new EquipmentTagPlan("tag-1", "eq-1", "drv", FolderPath: "", Name: "Speed", DataType: "Float", FullName: "40001"), + }, + }; + + var outcome = applier.Apply(plan); + + outcome.RebuildCalled.ShouldBeTrue(); + outcome.AddedNodes.ShouldBe(1); + sink.RebuildCalls.ShouldBe(1); } /// Verifies that added Galaxy tags in an otherwise-empty plan trigger an address-space rebuild. diff --git a/tests/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer.Tests/Phase7PlannerTests.cs b/tests/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer.Tests/Phase7PlannerTests.cs index 126687f1..96c99bc5 100644 --- a/tests/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer.Tests/Phase7PlannerTests.cs +++ b/tests/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer.Tests/Phase7PlannerTests.cs @@ -30,6 +30,31 @@ public sealed class Phase7PlannerTests plan.IsEmpty.ShouldBeTrue(); } + /// Verifies an equipment-tag-only delta (no equipment/driver/alarm/galaxy change) + /// yields a NON-empty plan, so OpcUaPublishActor.HandleRebuild does not short-circuit at the + /// IsEmpty gate before materialising the new equipment variables. + [Fact] + public void Equipment_tag_only_change_yields_non_empty_plan_with_added_tag() + { + var prev = new Phase7CompositionResult( + Array.Empty(), Array.Empty(), Array.Empty()); + var next = new Phase7CompositionResult( + Array.Empty(), Array.Empty(), Array.Empty()) + { + EquipmentTags = new[] + { + new EquipmentTagPlan("tag-1", "eq-1", "drv", FolderPath: "", Name: "Speed", DataType: "Float", FullName: "40001"), + }, + }; + + var plan = Phase7Planner.Compute(prev, next); + + plan.IsEmpty.ShouldBeFalse(); + plan.AddedEquipmentTags.Single().TagId.ShouldBe("tag-1"); + plan.RemovedEquipmentTags.ShouldBeEmpty(); + plan.ChangedEquipmentTags.ShouldBeEmpty(); + } + /// Verifies that new equipment goes to the AddedEquipment list. [Fact] public void New_equipment_goes_to_AddedEquipment() diff --git a/tests/Server/ZB.MOM.WW.OtOpcUa.Runtime.Tests/Drivers/DeploymentArtifactTests.cs b/tests/Server/ZB.MOM.WW.OtOpcUa.Runtime.Tests/Drivers/DeploymentArtifactTests.cs index a6ed7bd9..bd748c34 100644 --- a/tests/Server/ZB.MOM.WW.OtOpcUa.Runtime.Tests/Drivers/DeploymentArtifactTests.cs +++ b/tests/Server/ZB.MOM.WW.OtOpcUa.Runtime.Tests/Drivers/DeploymentArtifactTests.cs @@ -167,6 +167,7 @@ public sealed class DeploymentArtifactTests var c = DeploymentArtifact.ParseComposition(blob); var tag = c.EquipmentTags.ShouldHaveSingleItem(); + tag.TagId.ShouldBe("tag-eq"); tag.EquipmentId.ShouldBe("eq-1"); tag.DriverInstanceId.ShouldBe("drv-modbus"); tag.Name.ShouldBe("Speed");