From 419eda256bda277cf11f10a58771d5e82c1fdc79 Mon Sep 17 00:00:00 2001 From: Joseph Doherty Date: Mon, 18 May 2026 09:43:03 -0400 Subject: [PATCH] feat(server): route OPC UA Part 9 AddComment to ScriptedAlarmEngine MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit RouteScriptedAlarmMethodCalls now handles ConditionType.AddComment alongside Acknowledge/Confirm, dispatching to engine.AddCommentAsync. An empty comment is rejected by the Part 9 state machine and surfaced as BadInvalidArgument. MapCallOperation gates AddComment at the AlarmAcknowledge tier — there is no dedicated AddComment permission bit. Closes phase-7-status.md Gap 1: all Part 9 alarm methods now route to the engine. Adds 3 unit tests. Co-Authored-By: Claude Opus 4.7 (1M context) --- docs/v2/phase-7-status.md | 4 +- looseends.md | 9 ++-- .../OpcUa/DriverNodeManager.cs | 25 ++++++--- .../CallGatingTests.cs | 8 +++ .../Phase7/ScriptedAlarmMethodRoutingTests.cs | 53 +++++++++++++++++++ 5 files changed, 87 insertions(+), 12 deletions(-) diff --git a/docs/v2/phase-7-status.md b/docs/v2/phase-7-status.md index e5e7a46..cb7929d 100644 --- a/docs/v2/phase-7-status.md +++ b/docs/v2/phase-7-status.md @@ -138,9 +138,9 @@ All three are verified closed in the 2026-04-23 exit-gate audit: These are real open items, not issues with the plan reconciliation. -### Gap 1 — OPC UA method-call dispatch for scripted alarm AddComment (Stream G / C.6) +### Gap 1 — OPC UA method-call dispatch for scripted alarm methods (Stream G / C.6) — CLOSED -`Acknowledge` / `Confirm` route to the `ScriptedAlarmEngine` via `DriverNodeManager.RouteScriptedAlarmMethodCalls` (task #24). `OneShotShelve` / `TimedShelve` / `Unshelve` route via the native `AlarmConditionState.OnShelve` / `OnTimedUnshelve` hooks wired in `MarkAsAlarmCondition` (task #24 follow-up); the per-instance shelve method NodeIds are indexed so the Call gate resolves them to `OpcUaOperation.AlarmShelve`. Only `AddComment` is still not wired to the OPC UA method path — the engine has `AddCommentAsync` but no Part 9 `AddComment` method node is dispatched to it. +All Part 9 alarm methods now route to the `ScriptedAlarmEngine`. `Acknowledge` / `Confirm` / `AddComment` route via `DriverNodeManager.RouteScriptedAlarmMethodCalls` (task #24 + follow-up); `AddComment` gates at the `AlarmAcknowledge` tier. `OneShotShelve` / `TimedShelve` / `Unshelve` route via the native `AlarmConditionState.OnShelve` / `OnTimedUnshelve` hooks wired in `MarkAsAlarmCondition`, with the per-instance shelve method NodeIds indexed so the Call gate resolves them to `OpcUaOperation.AlarmShelve`. ### Gap 2 — Admin UI: no `/virtual-tags` tab or form (Stream F.2) diff --git a/looseends.md b/looseends.md index 7cc05ad..3967280 100644 --- a/looseends.md +++ b/looseends.md @@ -28,10 +28,11 @@ remains, plus follow-ups surfaced during the run. `DriverNodeManager.MarkAsAlarmCondition` (scripted alarms get a shelvable `ShelvedStateMachine` subtree created before `alarm.Create`). The three per-instance shelve method NodeIds are indexed so the Call gate resolves - them to `OpcUaOperation.AlarmShelve`. Remaining: address-space - materialisation of the shelve method nodes is best confirmed by a live - OPC UA browse (pairs with the G6 / D.1 rig steps). `AddComment` is still - not wired to an OPC UA method node — see `phase-7-status.md` Gap 1. + them to `OpcUaOperation.AlarmShelve`. `AddComment` also now routes to the + engine (gated at the `AlarmAcknowledge` tier) — `phase-7-status.md` Gap 1 + is fully closed. Remaining: address-space materialisation of the shelve + method nodes is best confirmed by a live OPC UA browse (pairs with the + G6 / D.1 rig steps). - **mxaccessgw alarm epic branch.** The alarm subsystem work (A.2/A.3/A.4 + the two production-gap fixes from #18) lives on the mxaccessgw branch diff --git a/src/Server/ZB.MOM.WW.OtOpcUa.Server/OpcUa/DriverNodeManager.cs b/src/Server/ZB.MOM.WW.OtOpcUa.Server/OpcUa/DriverNodeManager.cs index b3e5c86..801e080 100644 --- a/src/Server/ZB.MOM.WW.OtOpcUa.Server/OpcUa/DriverNodeManager.cs +++ b/src/Server/ZB.MOM.WW.OtOpcUa.Server/OpcUa/DriverNodeManager.cs @@ -652,8 +652,8 @@ public sealed class DriverNodeManager : CustomNodeManager2, IAddressSpaceBuilder } /// - /// Intercepts Part 9 Acknowledge / Confirm slots that - /// target scripted alarm condition nodes and routes them to the + /// Intercepts Part 9 Acknowledge / Confirm / AddComment + /// slots that target scripted alarm condition nodes and routes them to the /// . Slots that are handled have their /// entry set to and are /// not touched by the caller's subsequent base.Call invocation. @@ -663,7 +663,9 @@ public sealed class DriverNodeManager : CustomNodeManager2, IAddressSpaceBuilder /// The OPC UA Part 9 Acknowledge method signature is: /// InputArguments[0] = EventId (ByteString, ignored — scripted alarms identify by /// ConditionId, not EventId), InputArguments[1] = Comment (LocalizedText). - /// Confirm has the same shape. Missing or null comment is treated as empty string. + /// Confirm and AddComment have the same two-argument shape. For Acknowledge / + /// Confirm a missing comment is treated as empty; AddComment requires non-empty + /// comment text and returns otherwise. /// /// /// User identity is extracted from 's @@ -694,10 +696,12 @@ public sealed class DriverNodeManager : CustomNodeManager2, IAddressSpaceBuilder var request = methodsToCall[i]; - // Only handle the two well-known Part 9 method ids. + // Only handle the well-known Part 9 method ids. AddComment is declared on the + // base ConditionType; Acknowledge / Confirm on AcknowledgeableConditionType. var isAcknowledge = request.MethodId == MethodIds.AcknowledgeableConditionType_Acknowledge; var isConfirm = request.MethodId == MethodIds.AcknowledgeableConditionType_Confirm; - if (!isAcknowledge && !isConfirm) continue; + var isAddComment = request.MethodId == MethodIds.ConditionType_AddComment; + if (!isAcknowledge && !isConfirm && !isAddComment) continue; // ObjectId must be a string identifier so we can look it up in the index. if (request.ObjectId.Identifier is not string conditionKey) continue; @@ -717,9 +721,14 @@ public sealed class DriverNodeManager : CustomNodeManager2, IAddressSpaceBuilder if (isAcknowledge) engine.AcknowledgeAsync(alarmId, user, comment, CancellationToken.None) .GetAwaiter().GetResult(); - else + else if (isConfirm) engine.ConfirmAsync(alarmId, user, comment, CancellationToken.None) .GetAwaiter().GetResult(); + else + // AddComment requires comment text — the engine's Part 9 state machine + // rejects null/empty with ArgumentException, surfaced as BadInvalidArgument. + engine.AddCommentAsync(alarmId, user, comment ?? string.Empty, CancellationToken.None) + .GetAwaiter().GetResult(); // Mark the slot as handled so base.Call skips it. A pre-populated Good // result (not null and not Bad) is the signal the base class uses to @@ -904,6 +913,10 @@ public sealed class DriverNodeManager : CustomNodeManager2, IAddressSpaceBuilder return OpcUaOperation.AlarmAcknowledge; if (methodId == MethodIds.AcknowledgeableConditionType_Confirm) return OpcUaOperation.AlarmConfirm; + // AddComment (ConditionType) has no dedicated operation/permission bit — it gates at + // the same operator tier as Acknowledge, the closest existing alarm-action grant. + if (methodId == MethodIds.ConditionType_AddComment) + return OpcUaOperation.AlarmAcknowledge; // Shelve methods live on each alarm's own ShelvedStateMachine subtree, so they're // matched by NodeId membership rather than a constant comparison. if (methodId is not null && shelveMethodIds is not null && shelveMethodIds.Contains(methodId)) diff --git a/tests/Server/ZB.MOM.WW.OtOpcUa.Server.Tests/CallGatingTests.cs b/tests/Server/ZB.MOM.WW.OtOpcUa.Server.Tests/CallGatingTests.cs index d142a98..7735c57 100644 --- a/tests/Server/ZB.MOM.WW.OtOpcUa.Server.Tests/CallGatingTests.cs +++ b/tests/Server/ZB.MOM.WW.OtOpcUa.Server.Tests/CallGatingTests.cs @@ -33,6 +33,14 @@ public sealed class CallGatingTests .ShouldBe(OpcUaOperation.AlarmConfirm); } + [Fact] + public void MapCallOperation_AddComment_maps_to_AlarmAcknowledge() + { + // AddComment has no dedicated permission bit; it gates at the Acknowledge tier. + DriverNodeManager.MapCallOperation(MethodIds.ConditionType_AddComment) + .ShouldBe(OpcUaOperation.AlarmAcknowledge); + } + [Fact] public void MapCallOperation_generic_method_maps_to_Call() { diff --git a/tests/Server/ZB.MOM.WW.OtOpcUa.Server.Tests/Phase7/ScriptedAlarmMethodRoutingTests.cs b/tests/Server/ZB.MOM.WW.OtOpcUa.Server.Tests/Phase7/ScriptedAlarmMethodRoutingTests.cs index 40f46a0..ed69ca0 100644 --- a/tests/Server/ZB.MOM.WW.OtOpcUa.Server.Tests/Phase7/ScriptedAlarmMethodRoutingTests.cs +++ b/tests/Server/ZB.MOM.WW.OtOpcUa.Server.Tests/Phase7/ScriptedAlarmMethodRoutingTests.cs @@ -161,6 +161,18 @@ public sealed class ScriptedAlarmMethodRoutingTests }, }; + private static CallMethodRequest AddCommentRequest(string conditionNodeId, string comment) + => new() + { + ObjectId = new NodeId(conditionNodeId, 2), + MethodId = MethodIds.ConditionType_AddComment, + InputArguments = new VariantCollection + { + new Variant(new byte[] { 1, 2, 3 }), // EventId (ByteString) — ignored + new Variant(new LocalizedText(comment)), + }, + }; + private static CallMethodRequest GenericRequest(string objectNodeId) => new() { @@ -357,6 +369,47 @@ public sealed class ScriptedAlarmMethodRoutingTests engine.GetState("confirm-alarm")!.LastConfirmUser.ShouldBe("ops-user"); } + // ---- AddComment -------------------------------------------------------- + + [Fact] + public void AddComment_appends_comment_to_engine_state() + { + using var engine = BuildEngine("al-1"); + + var calls = new List { AddCommentRequest("al-1.Condition", "checked the line") }; + var results = new List { new CallMethodResult() }; + var errors = new List { null! }; + var index = Index(("al-1.Condition", "al-1")); + + DriverNodeManager.RouteScriptedAlarmMethodCalls( + MakeIdentity("ops-user"), calls, results, errors, engine, index); + + ServiceResult.IsBad(errors[0]).ShouldBeFalse("AddComment handled"); + results[0].StatusCode.ShouldBe((StatusCode)StatusCodes.Good); + var last = engine.GetState("al-1")!.Comments[^1]; + last.Kind.ShouldBe("AddComment"); + last.Text.ShouldBe("checked the line"); + last.User.ShouldBe("ops-user"); + } + + [Fact] + public void AddComment_with_empty_text_returns_BadInvalidArgument() + { + using var engine = BuildEngine("al-1"); + + // The Part 9 state machine rejects an empty comment — surfaced as BadInvalidArgument. + var calls = new List { AddCommentRequest("al-1.Condition", "") }; + var results = new List { new CallMethodResult() }; + var errors = new List { null! }; + var index = Index(("al-1.Condition", "al-1")); + + DriverNodeManager.RouteScriptedAlarmMethodCalls( + MakeIdentity("ops-user"), calls, results, errors, engine, index); + + ServiceResult.IsBad(errors[0]).ShouldBeTrue(); + errors[0].StatusCode.ShouldBe((StatusCode)StatusCodes.BadInvalidArgument); + } + // ---- Mixed batches ----------------------------------------------------- [Fact]