From 09b373e4db5ba7ddda8d2688e94974b47ee14143 Mon Sep 17 00:00:00 2001 From: Joseph Doherty Date: Thu, 12 Nov 2026 23:55:49 -0500 Subject: [PATCH] fix(address-space): no UNS value Variable for a reference to an alarm-bearing tag (#896) A UnsTagReference to a raw tag that carries an "alarm" object produced a UNS value Variable that read BadWaitingForInitialData forever (G6 live gate, TestAlarm002Condition on testmachine-015). BuildUnsReferenceVariables now skips those references; the equipment gets the alarm only as the raw condition wired as an event notifier of its folder, which is unchanged (the reference still feeds RawTagPlan.ReferencingEquipmentPaths). Why condition-only and not "feed the mirror": - The raw side has no value node for such a tag: AddressSpaceApplier materialises an alarm tag as a condition OR a value variable, never both. There is no raw value for the UNS mirror to mirror. - DriverHostActor skips alarm tags when it builds the value fan-out and write routing maps (`continue` before the UNS registration), and the driver leaves them out of its value subscription ("subscribed to 5 refs" with 6 tags in G6). Feeding the mirror would mean subscribing a value the design says the tag does not have. - Docs already state condition-only: ScriptedAlarms.md ("instead of a value variable"), AlarmTracking.md ("only the condition, with no sibling value variable"), the v3 design (native alarm = one condition at the raw tag, surfaced at equipment via notifiers). - The composer tests that asserted the mirror were incidental: f4f3e17e (v3 B4-WP1) put the alarm on the fixture's Speed tag to test raw alarm attachment, and later tests reused Speed for writable / historian / array checks of the mirror. No commit or doc chose a value mirror for alarm tags. - ScadaBridge does not read it: its OtOpcUa alarm path is event subscriptions on the equipment/raw folders (v3 alarm B/C live gate), and a node that never had a value cannot be a working binding. G6 itself authored the alarm's Boolean as a separate value tag (TestAlarm002), which is the supported way to read it. Tests: the composer fixture moves the alarm onto its own HighTemp tag (Speed is now a plain historized value tag), with a new test asserting no UNS Variable for the alarm reference, with positive controls (the raw tag still carries the equipment path; the value references still emit). New real-SDK test Alarm_bearing_reference_surfaces_as_the_condition_only_against_real_SDK: no Variable at the UNS path, the Speed mirror present, one condition, the equipment folder wired as its notifier. Both new tests fail with the skip removed. Docs: Uns.md, ScriptedAlarms.md, G6 evidence note. Claude-Session: https://claude.ai/code/session_01GT67ACmpMR7b9LGRPLijMi --- docs/ScriptedAlarms.md | 4 +- docs/Uns.md | 6 ++ .../evidence/galaxy-g6-e2e-2026-11-12.md | 4 +- .../AddressSpaceComposer.cs | 11 ++- .../AddressSpaceApplierHierarchyTests.cs | 78 +++++++++++++++++++ .../AddressSpaceComposerDualNamespaceTests.cs | 58 +++++++++++--- 6 files changed, 147 insertions(+), 14 deletions(-) diff --git a/docs/ScriptedAlarms.md b/docs/ScriptedAlarms.md index 227c7a594..21c35cc90 100644 --- a/docs/ScriptedAlarms.md +++ b/docs/ScriptedAlarms.md @@ -240,7 +240,9 @@ blob and is parsed byte-parity in both the compose (`AddressSpaceComposer`) and > never one-per-root. Teardown is symmetric (`RemoveNotifier(..., bidirectional:true)` on > rebuild / reference removal) so inverse-notifier entries never leak across redeploys. > Ack/confirm/shelve route on **`ConditionId = RawPath`** (never `SourceNodeId`), and the -> `/alerts` row lists the referencing equipment paths. See +> `/alerts` row lists the referencing equipment paths. The equipment gets the condition +> **only**: no UNS value variable is created for a reference to an alarm-bearing tag (#896), +> matching the raw side, which has no value variable either. See > [AlarmTracking.md](AlarmTracking.md) and the WP4 section of > `docs/plans/2026-07-15-v3-batch4-address-space-plan.md`. diff --git a/docs/Uns.md b/docs/Uns.md index 0676d5142..f136bb1d0 100644 --- a/docs/Uns.md +++ b/docs/Uns.md @@ -169,6 +169,12 @@ source-timestamp — the two NodeIds never drift. - **Native alarms** materialize once at the raw tag (`ConditionId = RawPath`) and fan via SDK notifiers to the raw device folder AND every referencing equipment folder — see [`ScriptedAlarms.md`](ScriptedAlarms.md) / [`AlarmTracking.md`](AlarmTracking.md). + A reference to an **alarm-bearing** raw tag therefore creates **no UNS variable** + (#896): the raw side has only the condition and no value to fan, so the + reference's only effect is to wire the condition as an event notifier of the + equipment folder. A client reads the alarm's state from the condition's fields + (`ActiveState`, `AckedState`, …) at either folder, not from a value variable. The + reference still counts for effective-name uniqueness. Note the two distinct identity strings: the wire **NodeId** is the path `Area/Line/Equipment/EffectiveName`, while the **effective-name uniqueness key** diff --git a/docs/drivers/evidence/galaxy-g6-e2e-2026-11-12.md b/docs/drivers/evidence/galaxy-g6-e2e-2026-11-12.md index 708569d4d..1670fa4a1 100644 --- a/docs/drivers/evidence/galaxy-g6-e2e-2026-11-12.md +++ b/docs/drivers/evidence/galaxy-g6-e2e-2026-11-12.md @@ -237,9 +237,11 @@ cleanly with it. never went active, and its ack would dial a reference the gateway refuses. Three unit tests were added to `GalaxyDriverRawPathKeyingTests`. The main one fails with the fix reverted, which was checked by reverting the fix and re-running it. -- **#896 (open).** The UNS mirror of an alarm-bearing raw tag gets a value Variable that stays +- **#896 (fixed after this run).** The UNS mirror of an alarm-bearing raw tag gets a value Variable that stays `BadWaitingForInitialData` forever, against the documented "condition instead of value variable" contract. It was not fixed here because the composer test fixtures assert that mirror. + Fixed afterwards: the composer no longer emits a UNS Variable for a reference to an alarm-bearing tag, so the + equipment shows only the condition (the `<- #896` Variable in the Step 1 browse is gone). ## Cleanup diff --git a/src/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer/AddressSpaceComposer.cs b/src/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer/AddressSpaceComposer.cs index 84ccc29bc..7a176ce4e 100644 --- a/src/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer/AddressSpaceComposer.cs +++ b/src/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer/AddressSpaceComposer.cs @@ -921,7 +921,8 @@ public static class AddressSpaceComposer /// equipment path resolve, keyed (realm=Uns, s=Area/Line/Equipment/EffectiveName) and carrying /// the backing RawPath (Organizes target + fan-out) plus the inherited DataType / writable. A /// reference whose backing tag is unknown, whose RawPath is unresolvable, or whose UNS path segments - /// are invalid is SKIPPED. Sorted by NodeId ordinal. + /// are invalid is SKIPPED. A reference to an ALARM-BEARING tag is skipped too (#896): it surfaces at the + /// equipment as the raw condition's event notifier, not as a value Variable. Sorted by NodeId ordinal. /// private static IReadOnlyList BuildUnsReferenceVariables( IReadOnlyList? unsTagReferences, @@ -965,6 +966,14 @@ public static class AddressSpaceComposer // doc-comment; 2026-08-15 surgical-deploys review). Parsed from the SAME TagConfigIntent the raw // tag plan uses, so the two sides can never disagree. var intent = TagConfigIntent.Parse(tag.TagConfig); + // #896: an alarm-bearing backing tag gets NO UNS value Variable. Its raw side is a Part 9 condition + // with no sibling value variable (AddressSpaceApplier.MaterialiseRawSubtree), the driver host keeps + // it out of the value fan-out and the value subscription (DriverHostActor skips Alarm tags), so a + // mirror Variable here could never receive a value and read BadWaitingForInitialData forever. The + // reference still reaches the equipment: it puts the equipment's path on the raw tag's + // ReferencingEquipmentPaths, and the applier wires the single condition as an event notifier of that + // equipment folder. Same TagConfigIntent the raw pass reads, so the two sides cannot disagree. + if (intent.Alarm is not null) continue; // Same derivation as BuildRawSubtree's ArrayWritable, off the same row, so a mirror is writable // exactly when its raw node is. var backingArrayWritable = intent.IsArray && arrayWritableFor(tag); diff --git a/tests/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer.Tests/AddressSpaceApplierHierarchyTests.cs b/tests/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer.Tests/AddressSpaceApplierHierarchyTests.cs index 4528812e7..63e98b282 100644 --- a/tests/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer.Tests/AddressSpaceApplierHierarchyTests.cs +++ b/tests/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer.Tests/AddressSpaceApplierHierarchyTests.cs @@ -236,6 +236,84 @@ public sealed class AddressSpaceApplierHierarchyTests : IDisposable sdkServer.NodeManager!.VariableCount.ShouldBe(2); } + /// + /// Gitea #896, against a real SDK node manager: an equipment that references a value tag AND an alarm-bearing + /// tag gets a UNS Variable for the value tag only. The alarm tag is the single Part 9 condition at its RawPath, + /// wired as an event notifier of the equipment folder, and no UNS Variable is created at + /// filling/line-1/station-1/HighTemp (on the G6 live gate that Variable existed and read + /// BadWaitingForInitialData forever, because nothing ever feeds it). The value-tag mirror and the + /// condition's equipment notifier are the positive controls. + /// + [Fact] + public async Task Alarm_bearing_reference_surfaces_as_the_condition_only_against_real_SDK() + { + var applicationUri = $"urn:OtOpcUa.AlarmRefE2e:{Guid.NewGuid():N}"; + var started = await OpcUaHostStart.StartAsync( + port => new OpcUaApplicationHost( + new OpcUaApplicationHostOptions + { + ApplicationName = "OtOpcUa.AlarmRefE2e", + ApplicationUri = applicationUri, + OpcUaPort = port, + PublicHostname = "localhost", + PkiStoreRoot = _pkiRoot, + }, + NullLogger.Instance), + () => new OtOpcUaSdkServer(), Ct); + await using var host = started.Host; + var sdkServer = started.Server; + sdkServer.NodeManager.ShouldNotBeNull(); + var nm = sdkServer.NodeManager!; + + var folder = new RawFolder { RawFolderId = "rf-plant", ClusterId = "c1", Name = "Plant", ParentRawFolderId = null }; + var driver = new DriverInstance { DriverInstanceId = "drv-modbus", ClusterId = "c1", Name = "Modbus", DriverType = "Modbus", DriverConfig = "{}", RawFolderId = "rf-plant" }; + var device = new Device { DeviceId = "dev-1", DriverInstanceId = "drv-modbus", Name = "dev1", DeviceConfig = "{}" }; + var speed = new Tag + { + TagId = "tag-speed", DeviceId = "dev-1", Name = "Speed", DataType = "Float", + AccessLevel = TagAccessLevel.Read, TagConfig = "{}", + }; + var highTemp = new Tag + { + TagId = "tag-hightemp", DeviceId = "dev-1", Name = "HighTemp", DataType = "Boolean", + AccessLevel = TagAccessLevel.Read, + TagConfig = "{\"alarm\":{\"alarmType\":\"OffNormalAlarm\",\"severity\":700}}", + }; + var area = new UnsArea { UnsAreaId = "nw-area-filling", ClusterId = "c1", Name = "filling" }; + var line = new UnsLine { UnsLineId = "nw-line-1", UnsAreaId = "nw-area-filling", Name = "line-1" }; + var equipment = new Equipment { EquipmentId = "eq-1", UnsLineId = "nw-line-1", Name = "station-1", MachineCode = "STATION_001" }; + + var composition = AddressSpaceComposer.Compose( + new[] { area }, new[] { line }, new[] { equipment }, new[] { driver }, + Array.Empty(), + unsTagReferences: new[] + { + new UnsTagReference { UnsTagReferenceId = "ref-speed", EquipmentId = "eq-1", TagId = "tag-speed" }, + new UnsTagReference { UnsTagReferenceId = "ref-hightemp", EquipmentId = "eq-1", TagId = "tag-hightemp" }, + }, + tags: new[] { speed, highTemp }, + rawFolders: new[] { folder }, devices: new[] { device }); + + var applier = new AddressSpaceApplier(new SdkAddressSpaceSink(nm), NullLogger.Instance); + applier.MaterialiseHierarchy(composition); + applier.MaterialiseRawSubtree(composition); + applier.MaterialiseUnsReferences(composition); + + // No UNS Variable for the alarm-bearing reference, and none at the raw RawPath either. + nm.TryGetVariable("filling/line-1/station-1/HighTemp", AddressSpaceRealm.Uns).ShouldBeNull(); + nm.TryGetVariable("Plant/Modbus/dev1/HighTemp", AddressSpaceRealm.Raw).ShouldBeNull(); + + // Positive control 1: the value tag still has both its raw node and its UNS mirror (and nothing else). + nm.TryGetVariable("Plant/Modbus/dev1/Speed", AddressSpaceRealm.Raw).ShouldNotBeNull(); + nm.TryGetVariable("filling/line-1/station-1/Speed", AddressSpaceRealm.Uns).ShouldNotBeNull(); + nm.VariableCount.ShouldBe(2); + + // Positive control 2: the alarm reaches the equipment as the single raw condition wired to its folder. + nm.TryGetAlarmCondition("Plant/Modbus/dev1/HighTemp", AddressSpaceRealm.Raw).ShouldNotBeNull(); + nm.AlarmConditionCount.ShouldBe(1); + nm.FolderNotifierCount("eq-1", AddressSpaceRealm.Uns).ShouldBe(1); + } + /// OpcUaServer-001 — a UNS Area / Line rename-only deploy refreshes the EXISTING folder's /// DisplayName IN PLACE against a real SDK node manager (no rebuild, no node count change). Proves the /// full chain: planner emits a RenamedFolders delta → applier drives the surgical diff --git a/tests/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer.Tests/AddressSpaceComposerDualNamespaceTests.cs b/tests/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer.Tests/AddressSpaceComposerDualNamespaceTests.cs index 0230c7e74..8af7828d3 100644 --- a/tests/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer.Tests/AddressSpaceComposerDualNamespaceTests.cs +++ b/tests/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer.Tests/AddressSpaceComposerDualNamespaceTests.cs @@ -17,7 +17,8 @@ namespace ZB.MOM.WW.OtOpcUa.OpcUaServer.Tests; public sealed class AddressSpaceComposerDualNamespaceTests { // A raw chain: RawFolder "Plant" → Driver "Modbus1" → Device "PLC-A" → Group "Motors" → tags. - // Speed sits under the group; Status directly under the device; Levels is an array tag. + // Speed sits under the group; Status directly under the device; Levels is an array tag; HighTemp is a + // native-alarm tag under the group. Speed, Status and HighTemp are referenced into equipment station-1. private static AddressSpaceComposition ComposeFullFixture() { var folder = new RawFolder { RawFolderId = "raw-plant", ClusterId = "c1", Name = "Plant" }; @@ -33,8 +34,13 @@ public sealed class AddressSpaceComposerDualNamespaceTests { TagId = "t-speed", DeviceId = "dev-1", TagGroupId = "g1", Name = "Speed", DataType = "Float", AccessLevel = TagAccessLevel.ReadWrite, - TagConfig = "{\"isHistorized\":true,\"historianTagname\":\"speed_hist\"," + - "\"alarm\":{\"alarmType\":\"LimitAlarm\",\"severity\":700}}", + TagConfig = "{\"isHistorized\":true,\"historianTagname\":\"speed_hist\"}", + }; + var highTemp = new Tag + { + TagId = "t-hightemp", DeviceId = "dev-1", TagGroupId = "g1", Name = "HighTemp", + DataType = "Boolean", AccessLevel = TagAccessLevel.Read, + TagConfig = "{\"alarm\":{\"alarmType\":\"LimitAlarm\",\"severity\":700}}", }; var status = new Tag { @@ -53,6 +59,7 @@ public sealed class AddressSpaceComposerDualNamespaceTests var equip = new Equipment { EquipmentId = "eq-1", UnsLineId = "line-1", Name = "station-1", MachineCode = "STATION_001" }; var refSpeed = new UnsTagReference { UnsTagReferenceId = "ref-speed", EquipmentId = "eq-1", TagId = "t-speed" }; + var refHighTemp = new UnsTagReference { UnsTagReferenceId = "ref-hightemp", EquipmentId = "eq-1", TagId = "t-hightemp" }; var refStatus = new UnsTagReference { UnsTagReferenceId = "ref-status", EquipmentId = "eq-1", TagId = "t-status", @@ -62,8 +69,8 @@ public sealed class AddressSpaceComposerDualNamespaceTests return AddressSpaceComposer.Compose( new[] { area }, new[] { line }, new[] { equip }, new[] { driver }, Array.Empty(), - unsTagReferences: new[] { refSpeed, refStatus }, - tags: new[] { speed, status, levels }, + unsTagReferences: new[] { refSpeed, refStatus, refHighTemp }, + tags: new[] { speed, status, levels, highTemp }, rawFolders: new[] { folder }, devices: new[] { device }, tagGroups: new[] { group }); @@ -136,11 +143,12 @@ public sealed class AddressSpaceComposerDualNamespaceTests { var c = ComposeFullFixture(); - var speed = c.RawTags.Single(t => t.TagId == "t-speed"); - speed.Alarm.ShouldNotBeNull(); - speed.Alarm!.AlarmType.ShouldBe("LimitAlarm"); - speed.Alarm.Severity.ShouldBe(700); - speed.ReferencingEquipmentPaths.ShouldBe(new[] { "filling/line-1/station-1" }); + var highTemp = c.RawTags.Single(t => t.TagId == "t-hightemp"); + highTemp.NodeId.ShouldBe("Plant/Modbus1/PLC-A/Motors/HighTemp"); // ConditionId = RawPath + highTemp.Alarm.ShouldNotBeNull(); + highTemp.Alarm!.AlarmType.ShouldBe("LimitAlarm"); + highTemp.Alarm.Severity.ShouldBe(700); + highTemp.ReferencingEquipmentPaths.ShouldBe(new[] { "filling/line-1/station-1" }); // A non-alarm tag carries no condition + its own referencing-equipment list. var status = c.RawTags.Single(t => t.TagId == "t-status"); @@ -152,7 +160,35 @@ public sealed class AddressSpaceComposerDualNamespaceTests levels.ReferencingEquipmentPaths.ShouldBeEmpty(); } - /// Each UnsTagReference emits a UNS Variable keyed + /// + /// Gitea #896: a reference to an ALARM-BEARING raw tag emits NO UNS value Variable. The raw side of such a + /// tag is a Part 9 condition with no sibling value variable, and the driver host keeps it out of the value + /// fan-out, so a mirror Variable could never receive a value (it read BadWaitingForInitialData + /// forever on the G6 live gate). The equipment still gets the alarm: the reference puts the equipment's + /// path on the raw tag's ReferencingEquipmentPaths, which is what wires the condition as a notifier + /// of the equipment folder. Positive controls: that path is present, and the value references in the same + /// equipment still emit their Variables. + /// + [Fact] + public void A_reference_to_an_alarm_bearing_tag_emits_no_uns_variable_but_still_reaches_the_equipment() + { + var c = ComposeFullFixture(); + + c.UnsReferenceVariables.ShouldNotContain(v => v.UnsTagReferenceId == "ref-hightemp"); + c.UnsReferenceVariables.ShouldNotContain(v => v.BackingRawPath == "Plant/Modbus1/PLC-A/Motors/HighTemp"); + c.UnsReferenceVariables.ShouldNotContain(v => v.NodeId == "filling/line-1/station-1/HighTemp"); + + // Positive control 1: the condition still reaches station-1 through the raw tag's notifier fan-out. + var highTemp = c.RawTags.Single(t => t.TagId == "t-hightemp"); + highTemp.Alarm.ShouldNotBeNull(); + highTemp.ReferencingEquipmentPaths.ShouldBe(new[] { "filling/line-1/station-1" }); + + // Positive control 2: the value references in the same equipment still get their Variables. + c.UnsReferenceVariables.Select(v => v.UnsTagReferenceId).OrderBy(x => x, StringComparer.Ordinal) + .ShouldBe(new[] { "ref-speed", "ref-status" }); + } + + /// Each value-tag UnsTagReference emits a UNS Variable keyed /// s=<Area>/<Line>/<Equipment>/<EffectiveName> (realm=Uns), carrying its backing /// RawPath (the Organizes target + fan-out) and inheriting DataType + writable from the raw tag. Effective /// name = DisplayNameOverride else the backing raw tag's Name.