From fa7efb3b62292db5812bb41c845529b6d60712be Mon Sep 17 00:00:00 2001 From: Joseph Doherty Date: Thu, 8 Oct 2026 00:57:06 -0400 Subject: [PATCH] fix(client-cli): call Part 9 ack/confirm/enable/shelve methods on the condition node itself The client appended .Condition / .ShelvingState to the condition id; neither child exists on the OtOpcUa server (conditions are their own SourceNode, ShelvingState is an SDK numeric ns0 node), so every CLI ack/confirm/shelve/enable/disable returned BadNodeIdUnknown. Claude-Session: https://claude.ai/code/session_01Ef2ozxS7QPanGT8uoATbTe --- docs/Client.CLI.md | 2 +- .../OpcUaClientService.cs | 33 ++++++-------- .../OpcUaClientServiceTests.cs | 43 ++++++------------- 3 files changed, 29 insertions(+), 49 deletions(-) diff --git a/docs/Client.CLI.md b/docs/Client.CLI.md index 870f55ac..1b3a7dd6 100644 --- a/docs/Client.CLI.md +++ b/docs/Client.CLI.md @@ -286,7 +286,7 @@ Confirm successful: ns=1;s=TestMachine_001 ### shelve -Shelves or unshelves an active alarm condition via the OPC UA Part 9 `ShelvedStateMachine` (`OneShotShelve` / `TimedShelve` / `Unshelve`). Gated on the `AlarmAck` role. `--duration` is given in **seconds** (converted to milliseconds for the OPC UA call) and is required only for `--kind Timed`. +Shelves or unshelves an active alarm condition via the OPC UA Part 9 `ShelvedStateMachine` (`OneShotShelve` / `TimedShelve` / `Unshelve`). Gated on the `AlarmAck` role. `--duration` is given in **seconds** (converted to milliseconds for the OPC UA call) and is required only for `--kind Timed`. The `--node` is the condition node itself (the `SourceNode`/condition id an `alarms` notification reports): `ack`, `confirm`, `shelve`, `enable` and `disable` call the Part 9 method ids directly on that node — no `.Condition` / `.ShelvingState` suffix is appended (those child nodes do not exist on the OtOpcUa server). ```bash # One-shot shelve until the next clear diff --git a/src/Client/ZB.MOM.WW.OtOpcUa.Client.Shared/OpcUaClientService.cs b/src/Client/ZB.MOM.WW.OtOpcUa.Client.Shared/OpcUaClientService.cs index 7408431c..595d15b5 100644 --- a/src/Client/ZB.MOM.WW.OtOpcUa.Client.Shared/OpcUaClientService.cs +++ b/src/Client/ZB.MOM.WW.OtOpcUa.Client.Shared/OpcUaClientService.cs @@ -370,10 +370,9 @@ public sealed class OpcUaClientService : IOpcUaClientService ThrowIfDisposed(); ThrowIfNotConnected(); - // The Acknowledge method lives on the .Condition child node, not the source node itself - var conditionObjId = conditionNodeId.EndsWith(".Condition") - ? NodeId.Parse(conditionNodeId) - : NodeId.Parse(conditionNodeId + ".Condition"); + // OtOpcUa conditions are their own event source (SourceNode == ConditionId), so the Part 9 methods are + // invoked on that node directly. There is no ".Condition" child to append. + var conditionObjId = NodeId.Parse(conditionNodeId); var acknowledgeMethodId = MethodIds.AcknowledgeableConditionType_Acknowledge; // CallMethodAsync throws ServiceResultException on a bad call result; @@ -406,10 +405,8 @@ public sealed class OpcUaClientService : IOpcUaClientService ThrowIfDisposed(); ThrowIfNotConnected(); - // Confirm lives on the same .Condition child node as Acknowledge - var conditionObjId = conditionNodeId.EndsWith(".Condition") - ? NodeId.Parse(conditionNodeId) - : NodeId.Parse(conditionNodeId + ".Condition"); + // Confirm is invoked on the condition node itself, like Acknowledge. + var conditionObjId = NodeId.Parse(conditionNodeId); var confirmMethodId = MethodIds.AcknowledgeableConditionType_Confirm; try @@ -438,11 +435,11 @@ public sealed class OpcUaClientService : IOpcUaClientService ThrowIfDisposed(); ThrowIfNotConnected(); - // The shelve methods live on the ShelvingState child of the condition node. - // conditionNodeId is the alarm/condition node; append .ShelvingState to get the state machine node. - var shelvingStateNodeId = conditionNodeId.EndsWith(".ShelvingState") - ? NodeId.Parse(conditionNodeId) - : NodeId.Parse(conditionNodeId + ".ShelvingState"); + // The server's ShelvingState child is SDK-created with a numeric namespace-0 id, so a synthesized + // ".ShelvingState" node id never exists. The shelve methods are called on the condition + // object itself with the AlarmConditionType_ShelvingState_* method ids, which the server dispatches + // to the condition's ShelvingState methods. + var shelvingStateNodeId = NodeId.Parse(conditionNodeId); NodeId methodId; object[] inputArgs; @@ -495,8 +492,8 @@ public sealed class OpcUaClientService : IOpcUaClientService /// /// Shared body for the H4 Enable/Disable client path. Invokes the OPC UA Part 9 ConditionType /// Enable/Disable method (no input arguments) on the condition object node, mirroring - /// the convention: the methods live on the .Condition - /// child node, and a bad call result surfaces as the returned rather than + /// the convention: the methods are invoked on the condition + /// node itself, and a bad call result surfaces as the returned rather than /// escaping as an uncaught . /// /// The condition node associated with the alarm. @@ -507,10 +504,8 @@ public sealed class OpcUaClientService : IOpcUaClientService ThrowIfDisposed(); ThrowIfNotConnected(); - // Enable/Disable live on the same .Condition child node as Acknowledge/Confirm. - var conditionObjId = conditionNodeId.EndsWith(".Condition") - ? NodeId.Parse(conditionNodeId) - : NodeId.Parse(conditionNodeId + ".Condition"); + // Enable/Disable are invoked on the condition node itself, like Acknowledge/Confirm. + var conditionObjId = NodeId.Parse(conditionNodeId); var methodId = enabling ? MethodIds.ConditionType_Enable : MethodIds.ConditionType_Disable; try diff --git a/tests/Client/ZB.MOM.WW.OtOpcUa.Client.Shared.Tests/OpcUaClientServiceTests.cs b/tests/Client/ZB.MOM.WW.OtOpcUa.Client.Shared.Tests/OpcUaClientServiceTests.cs index 84b1fbd5..d74a1bfe 100644 --- a/tests/Client/ZB.MOM.WW.OtOpcUa.Client.Shared.Tests/OpcUaClientServiceTests.cs +++ b/tests/Client/ZB.MOM.WW.OtOpcUa.Client.Shared.Tests/OpcUaClientServiceTests.cs @@ -1040,21 +1040,22 @@ public class OpcUaClientServiceTests : IDisposable } /// - /// Verifies the ".Condition" suffix is appended when the caller supplies the - /// source node, but left alone when the caller already passes the condition node — - /// matches the documented contract. + /// Regression: OtOpcUa conditions are their own event source, so Acknowledge/Confirm must be invoked on + /// the supplied condition node itself — no ".Condition" child exists on the server to append. /// [Fact] - public async Task AcknowledgeAlarmAsync_LeavesConditionSuffixAlone() + public async Task AcknowledgeAndConfirm_InvokeMethodsOnTheConditionNodeItself() { var session = new FakeSessionAdapter(); _sessionFactory.EnqueueSession(session); await _service.ConnectAsync(ValidSettings()); - await _service.AcknowledgeAlarmAsync("ns=2;s=Cond.Condition", new byte[] { 1, 2 }, "acked"); + await _service.AcknowledgeAlarmAsync("ns=2;s=Plant/Alarm1", new byte[] { 1, 2 }, "acked"); + await _service.ConfirmAlarmAsync("ns=2;s=Plant/Alarm1", new byte[] { 1, 2 }, "confirmed"); - // Both call shapes reach the adapter once. - session.CallMethodCount.ShouldBe(1); + session.CallMethodObjectIds.ShouldAllBe(id => id == NodeId.Parse("ns=2;s=Plant/Alarm1")); + session.CallMethodMethodIds[0].ShouldBe(MethodIds.AcknowledgeableConditionType_Acknowledge); + session.CallMethodMethodIds[1].ShouldBe(MethodIds.AcknowledgeableConditionType_Confirm); } // --- ConfirmAlarm tests --- @@ -1113,6 +1114,8 @@ public class OpcUaClientServiceTests : IDisposable result.ShouldBe(StatusCodes.Good); session.CallMethodCount.ShouldBe(1); + session.CallMethodObjectIds[0].ShouldBe(NodeId.Parse("ns=2;s=Cond")); + session.CallMethodMethodIds[0].ShouldBe(MethodIds.AlarmConditionType_ShelvingState_OneShotShelve); session.CallMethodInputArgs[0].ShouldBeEmpty(); } @@ -1150,6 +1153,8 @@ public class OpcUaClientServiceTests : IDisposable result.ShouldBe(StatusCodes.Good); session.CallMethodCount.ShouldBe(1); + session.CallMethodObjectIds[0].ShouldBe(NodeId.Parse("ns=2;s=Cond")); + session.CallMethodMethodIds[0].ShouldBe(MethodIds.AlarmConditionType_ShelvingState_Unshelve); session.CallMethodInputArgs[0].ShouldBeEmpty(); } @@ -1191,7 +1196,7 @@ public class OpcUaClientServiceTests : IDisposable result.ShouldBe(StatusCodes.Good); session.CallMethodCount.ShouldBe(1); - session.CallMethodObjectIds[0].ShouldBe(NodeId.Parse("ns=2;s=Cond.Condition")); + session.CallMethodObjectIds[0].ShouldBe(NodeId.Parse("ns=2;s=Cond")); session.CallMethodMethodIds[0].ShouldBe(MethodIds.ConditionType_Enable); session.CallMethodInputArgs[0].ShouldBeEmpty(); } @@ -1211,31 +1216,11 @@ public class OpcUaClientServiceTests : IDisposable result.ShouldBe(StatusCodes.Good); session.CallMethodCount.ShouldBe(1); - session.CallMethodObjectIds[0].ShouldBe(NodeId.Parse("ns=2;s=Cond.Condition")); + session.CallMethodObjectIds[0].ShouldBe(NodeId.Parse("ns=2;s=Cond")); session.CallMethodMethodIds[0].ShouldBe(MethodIds.ConditionType_Disable); session.CallMethodInputArgs[0].ShouldBeEmpty(); } - /// - /// Verifies the ".Condition" suffix is appended when the caller supplies the source node, - /// but left alone when the caller already passes the condition node — mirrors - /// for the Enable path so the - /// object-node derivation in CallEnableDisableAsync is regression-covered. - /// - [Fact] - public async Task EnableAsync_WhenConditionSuffixAlreadyPresent_DoesNotDoubleSuffix() - { - var session = new FakeSessionAdapter(); - _sessionFactory.EnqueueSession(session); - await _service.ConnectAsync(ValidSettings()); - - var result = await _service.EnableAsync("ns=2;s=Cond.Condition"); - - result.ShouldBe(StatusCodes.Good); - session.CallMethodCount.ShouldBe(1); - session.CallMethodObjectIds[0].ShouldBe(NodeId.Parse("ns=2;s=Cond.Condition")); - } - /// /// Verifies that a from the session is captured and /// returned as a bad rather than propagating to the caller.