feat(server): route OPC UA Part 9 AddComment to ScriptedAlarmEngine
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) <noreply@anthropic.com>
This commit is contained in:
@@ -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.
|
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)
|
### Gap 2 — Admin UI: no `/virtual-tags` tab or form (Stream F.2)
|
||||||
|
|
||||||
|
|||||||
@@ -28,10 +28,11 @@ remains, plus follow-ups surfaced during the run.
|
|||||||
`DriverNodeManager.MarkAsAlarmCondition` (scripted alarms get a shelvable
|
`DriverNodeManager.MarkAsAlarmCondition` (scripted alarms get a shelvable
|
||||||
`ShelvedStateMachine` subtree created before `alarm.Create`). The three
|
`ShelvedStateMachine` subtree created before `alarm.Create`). The three
|
||||||
per-instance shelve method NodeIds are indexed so the Call gate resolves
|
per-instance shelve method NodeIds are indexed so the Call gate resolves
|
||||||
them to `OpcUaOperation.AlarmShelve`. Remaining: address-space
|
them to `OpcUaOperation.AlarmShelve`. `AddComment` also now routes to the
|
||||||
materialisation of the shelve method nodes is best confirmed by a live
|
engine (gated at the `AlarmAcknowledge` tier) — `phase-7-status.md` Gap 1
|
||||||
OPC UA browse (pairs with the G6 / D.1 rig steps). `AddComment` is still
|
is fully closed. Remaining: address-space materialisation of the shelve
|
||||||
not wired to an OPC UA method node — see `phase-7-status.md` Gap 1.
|
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
|
- **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
|
+ the two production-gap fixes from #18) lives on the mxaccessgw branch
|
||||||
|
|||||||
@@ -652,8 +652,8 @@ public sealed class DriverNodeManager : CustomNodeManager2, IAddressSpaceBuilder
|
|||||||
}
|
}
|
||||||
|
|
||||||
/// <summary>
|
/// <summary>
|
||||||
/// Intercepts Part 9 Acknowledge / Confirm <see cref="CallMethodRequest"/> slots that
|
/// Intercepts Part 9 Acknowledge / Confirm / AddComment <see cref="CallMethodRequest"/>
|
||||||
/// target scripted alarm condition nodes and routes them to the
|
/// slots that target scripted alarm condition nodes and routes them to the
|
||||||
/// <see cref="ScriptedAlarmEngine"/>. Slots that are handled have their
|
/// <see cref="ScriptedAlarmEngine"/>. Slots that are handled have their
|
||||||
/// <paramref name="errors"/> entry set to <see cref="ServiceResult.Good"/> and are
|
/// <paramref name="errors"/> entry set to <see cref="ServiceResult.Good"/> and are
|
||||||
/// not touched by the caller's subsequent <c>base.Call</c> invocation.
|
/// not touched by the caller's subsequent <c>base.Call</c> invocation.
|
||||||
@@ -663,7 +663,9 @@ public sealed class DriverNodeManager : CustomNodeManager2, IAddressSpaceBuilder
|
|||||||
/// The OPC UA Part 9 Acknowledge method signature is:
|
/// The OPC UA Part 9 Acknowledge method signature is:
|
||||||
/// InputArguments[0] = EventId (ByteString, ignored — scripted alarms identify by
|
/// InputArguments[0] = EventId (ByteString, ignored — scripted alarms identify by
|
||||||
/// ConditionId, not EventId), InputArguments[1] = Comment (LocalizedText).
|
/// 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 <see cref="StatusCodes.BadInvalidArgument"/> otherwise.
|
||||||
/// </para>
|
/// </para>
|
||||||
/// <para>
|
/// <para>
|
||||||
/// User identity is extracted from <paramref name="userIdentity"/>'s
|
/// User identity is extracted from <paramref name="userIdentity"/>'s
|
||||||
@@ -694,10 +696,12 @@ public sealed class DriverNodeManager : CustomNodeManager2, IAddressSpaceBuilder
|
|||||||
|
|
||||||
var request = methodsToCall[i];
|
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 isAcknowledge = request.MethodId == MethodIds.AcknowledgeableConditionType_Acknowledge;
|
||||||
var isConfirm = request.MethodId == MethodIds.AcknowledgeableConditionType_Confirm;
|
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.
|
// ObjectId must be a string identifier so we can look it up in the index.
|
||||||
if (request.ObjectId.Identifier is not string conditionKey) continue;
|
if (request.ObjectId.Identifier is not string conditionKey) continue;
|
||||||
@@ -717,9 +721,14 @@ public sealed class DriverNodeManager : CustomNodeManager2, IAddressSpaceBuilder
|
|||||||
if (isAcknowledge)
|
if (isAcknowledge)
|
||||||
engine.AcknowledgeAsync(alarmId, user, comment, CancellationToken.None)
|
engine.AcknowledgeAsync(alarmId, user, comment, CancellationToken.None)
|
||||||
.GetAwaiter().GetResult();
|
.GetAwaiter().GetResult();
|
||||||
else
|
else if (isConfirm)
|
||||||
engine.ConfirmAsync(alarmId, user, comment, CancellationToken.None)
|
engine.ConfirmAsync(alarmId, user, comment, CancellationToken.None)
|
||||||
.GetAwaiter().GetResult();
|
.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
|
// 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
|
// 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;
|
return OpcUaOperation.AlarmAcknowledge;
|
||||||
if (methodId == MethodIds.AcknowledgeableConditionType_Confirm)
|
if (methodId == MethodIds.AcknowledgeableConditionType_Confirm)
|
||||||
return OpcUaOperation.AlarmConfirm;
|
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
|
// Shelve methods live on each alarm's own ShelvedStateMachine subtree, so they're
|
||||||
// matched by NodeId membership rather than a constant comparison.
|
// matched by NodeId membership rather than a constant comparison.
|
||||||
if (methodId is not null && shelveMethodIds is not null && shelveMethodIds.Contains(methodId))
|
if (methodId is not null && shelveMethodIds is not null && shelveMethodIds.Contains(methodId))
|
||||||
|
|||||||
@@ -33,6 +33,14 @@ public sealed class CallGatingTests
|
|||||||
.ShouldBe(OpcUaOperation.AlarmConfirm);
|
.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]
|
[Fact]
|
||||||
public void MapCallOperation_generic_method_maps_to_Call()
|
public void MapCallOperation_generic_method_maps_to_Call()
|
||||||
{
|
{
|
||||||
|
|||||||
@@ -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)
|
private static CallMethodRequest GenericRequest(string objectNodeId)
|
||||||
=> new()
|
=> new()
|
||||||
{
|
{
|
||||||
@@ -357,6 +369,47 @@ public sealed class ScriptedAlarmMethodRoutingTests
|
|||||||
engine.GetState("confirm-alarm")!.LastConfirmUser.ShouldBe("ops-user");
|
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<CallMethodRequest> { AddCommentRequest("al-1.Condition", "checked the line") };
|
||||||
|
var results = new List<CallMethodResult> { new CallMethodResult() };
|
||||||
|
var errors = new List<ServiceResult> { 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<CallMethodRequest> { AddCommentRequest("al-1.Condition", "") };
|
||||||
|
var results = new List<CallMethodResult> { new CallMethodResult() };
|
||||||
|
var errors = new List<ServiceResult> { 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 -----------------------------------------------------
|
// ---- Mixed batches -----------------------------------------------------
|
||||||
|
|
||||||
[Fact]
|
[Fact]
|
||||||
|
|||||||
Reference in New Issue
Block a user