fix(client): alarm filter selected ActiveState/Id as ONE QualifiedName — every event read inactive
EventFilter.AddSelectClause(type, "ActiveState/Id") does not split the string; it selects a single QualifiedName named "ActiveState/Id", which no event carries. AckedState/Id, ActiveState/Id and EnabledState/Id therefore always arrived null and Client.CLI 'alarms' (and Client.UI) reported every alarm inactive and unacknowledged while a direct Read said Active. Pre-existing since a2883b82; server-side event delivery was correct (proven with two-element select clauses, incl. after #629). Select them with explicit two-element browse paths. Over-the-wire test drives OpcUaClientService.SubscribeAlarmsAsync against the in-process server (native + scripted); ServerEventRig now also selects ActiveState/Id, Severity and Retain. Claude-Session: https://claude.ai/code/session_01Ef2ozxS7QPanGT8uoATbTe
This commit is contained in:
@@ -248,6 +248,12 @@ otopcua-cli alarms -u opc.tcp://localhost:4840/OtOpcUa \
|
|||||||
| `-i` / `--interval` | Publishing interval in milliseconds (default: 1000) |
|
| `-i` / `--interval` | Publishing interval in milliseconds (default: 1000) |
|
||||||
| `--refresh` | Request a `ConditionRefresh` after subscribing to get current retained alarm states |
|
| `--refresh` | Request a `ConditionRefresh` after subscribing to get current retained alarm states |
|
||||||
|
|
||||||
|
The active/acknowledged columns come from the standard `ActiveState/Id`, `AckedState/Id` and
|
||||||
|
`EnabledState/Id` event fields. Before the 2026-10 fix the client's filter selected them with a single
|
||||||
|
QualifiedName named `ActiveState/Id` (a `/`-joined string passed to `EventFilter.AddSelectClause` is **not**
|
||||||
|
split into a browse path), which matched nothing — so every event printed inactive and unacknowledged
|
||||||
|
whatever the condition's real state. A direct Read of the condition was always correct.
|
||||||
|
|
||||||
### ack
|
### ack
|
||||||
|
|
||||||
Acknowledges an active alarm condition via the OPC UA Part 9 `Acknowledge` method. The server gates this on the **`AlarmAck`** role (fail-closed — a session whose API-key principal holds no `AlarmAck`-conferring `NodeAcl` grant gets `BadUserAccessDenied`; the data plane does not use LDAP; see [ScriptedAlarms.md](ScriptedAlarms.md) §"AlarmAck gate"). The `--event-id` is the hex-encoded `EventId` taken from a prior `alarms` subscription notification for the same condition.
|
Acknowledges an active alarm condition via the OPC UA Part 9 `Acknowledge` method. The server gates this on the **`AlarmAck`** role (fail-closed — a session whose API-key principal holds no `AlarmAck`-conferring `NodeAcl` grant gets `BadUserAccessDenied`; the data plane does not use LDAP; see [ScriptedAlarms.md](ScriptedAlarms.md) §"AlarmAck gate"). The `--event-id` is the hex-encoded `EventId` taken from a prior `alarms` subscription notification for the same condition.
|
||||||
|
|||||||
@@ -867,7 +867,7 @@ public sealed class OpcUaClientService : IOpcUaClientService
|
|||||||
catch { return false; }
|
catch { return false; }
|
||||||
}
|
}
|
||||||
|
|
||||||
private static EventFilter CreateAlarmEventFilter()
|
internal static EventFilter CreateAlarmEventFilter()
|
||||||
{
|
{
|
||||||
var filter = new EventFilter();
|
var filter = new EventFilter();
|
||||||
// 0: EventId
|
// 0: EventId
|
||||||
@@ -886,12 +886,18 @@ public sealed class OpcUaClientService : IOpcUaClientService
|
|||||||
filter.AddSelectClause(ObjectTypeIds.ConditionType, BrowseNames.ConditionName);
|
filter.AddSelectClause(ObjectTypeIds.ConditionType, BrowseNames.ConditionName);
|
||||||
// 7: Retain
|
// 7: Retain
|
||||||
filter.AddSelectClause(ObjectTypeIds.ConditionType, BrowseNames.Retain);
|
filter.AddSelectClause(ObjectTypeIds.ConditionType, BrowseNames.Retain);
|
||||||
|
// 8-10: the two-state /Id booleans. Each browse path is TWO QualifiedNames. Passing "AckedState/Id" as a
|
||||||
|
// string to AddSelectClause does not split it — it selects one element NAMED "AckedState/Id", which no
|
||||||
|
// event carries, so all three arrived null and every alarm read inactive + unacknowledged.
|
||||||
// 8: AckedState/Id
|
// 8: AckedState/Id
|
||||||
filter.AddSelectClause(ObjectTypeIds.AcknowledgeableConditionType, "AckedState/Id");
|
filter.SelectClauses.Add(new SimpleAttributeOperand(
|
||||||
|
ObjectTypeIds.AcknowledgeableConditionType, [BrowseNames.AckedState, BrowseNames.Id]));
|
||||||
// 9: ActiveState/Id
|
// 9: ActiveState/Id
|
||||||
filter.AddSelectClause(ObjectTypeIds.AlarmConditionType, "ActiveState/Id");
|
filter.SelectClauses.Add(new SimpleAttributeOperand(
|
||||||
|
ObjectTypeIds.AlarmConditionType, [BrowseNames.ActiveState, BrowseNames.Id]));
|
||||||
// 10: EnabledState/Id
|
// 10: EnabledState/Id
|
||||||
filter.AddSelectClause(ObjectTypeIds.AlarmConditionType, "EnabledState/Id");
|
filter.SelectClauses.Add(new SimpleAttributeOperand(
|
||||||
|
ObjectTypeIds.ConditionType, [BrowseNames.EnabledState, BrowseNames.Id]));
|
||||||
// 11: SuppressedOrShelved
|
// 11: SuppressedOrShelved
|
||||||
filter.AddSelectClause(ObjectTypeIds.AlarmConditionType, "SuppressedOrShelved");
|
filter.AddSelectClause(ObjectTypeIds.AlarmConditionType, "SuppressedOrShelved");
|
||||||
// 12: SourceNode (ConditionId for acknowledgment)
|
// 12: SourceNode (ConditionId for acknowledgment)
|
||||||
|
|||||||
@@ -0,0 +1,38 @@
|
|||||||
|
using Opc.Ua;
|
||||||
|
using Shouldly;
|
||||||
|
using Xunit;
|
||||||
|
|
||||||
|
namespace ZB.MOM.WW.OtOpcUa.Client.Shared.Tests;
|
||||||
|
|
||||||
|
/// <summary>
|
||||||
|
/// Pins the browse paths of the alarm subscription's select clauses. <c>EventFilter.AddSelectClause(type,
|
||||||
|
/// "ActiveState/Id")</c> does NOT split the string — it yields ONE QualifiedName named <c>ActiveState/Id</c>,
|
||||||
|
/// which matches nothing — so the client reported every alarm inactive and unacknowledged. The over-the-wire
|
||||||
|
/// proof is <c>ClientAlarmMethodTests.The_client_alarm_subscription_reports_active_acked_and_exact_severity</c>.
|
||||||
|
/// </summary>
|
||||||
|
public sealed class AlarmEventFilterTests
|
||||||
|
{
|
||||||
|
/// <summary>No select clause carries a QualifiedName with a path separator in it.</summary>
|
||||||
|
[Fact]
|
||||||
|
public void No_browse_path_element_contains_a_separator()
|
||||||
|
{
|
||||||
|
var filter = OpcUaClientService.CreateAlarmEventFilter();
|
||||||
|
|
||||||
|
filter.SelectClauses.SelectMany(c => c.BrowsePath).ShouldAllBe(q => !q.Name.Contains('/'));
|
||||||
|
}
|
||||||
|
|
||||||
|
/// <summary>The two-state ids are selected by their two-element browse paths, at the indices the parser reads.</summary>
|
||||||
|
[Fact]
|
||||||
|
public void Two_state_ids_are_two_element_browse_paths()
|
||||||
|
{
|
||||||
|
var clauses = OpcUaClientService.CreateAlarmEventFilter().SelectClauses;
|
||||||
|
|
||||||
|
string Path(int i) => string.Join("/", clauses[i].BrowsePath.Select(q => q.Name));
|
||||||
|
Path(8).ShouldBe("AckedState/Id");
|
||||||
|
clauses[8].BrowsePath.Count.ShouldBe(2);
|
||||||
|
Path(9).ShouldBe("ActiveState/Id");
|
||||||
|
clauses[9].BrowsePath.Count.ShouldBe(2);
|
||||||
|
Path(10).ShouldBe("EnabledState/Id");
|
||||||
|
clauses[10].BrowsePath.Count.ShouldBe(2);
|
||||||
|
}
|
||||||
|
}
|
||||||
@@ -3,6 +3,7 @@ using Shouldly;
|
|||||||
using Xunit;
|
using Xunit;
|
||||||
using ZB.MOM.WW.OtOpcUa.Client.Shared;
|
using ZB.MOM.WW.OtOpcUa.Client.Shared;
|
||||||
using ZB.MOM.WW.OtOpcUa.Client.Shared.Models;
|
using ZB.MOM.WW.OtOpcUa.Client.Shared.Models;
|
||||||
|
using ZB.MOM.WW.OtOpcUa.Commons.OpcUa;
|
||||||
using ZB.MOM.WW.OtOpcUa.Configuration.Enums;
|
using ZB.MOM.WW.OtOpcUa.Configuration.Enums;
|
||||||
using static ZB.MOM.WW.OtOpcUa.OpcUaServer.IntegrationTests.ServerEventRig;
|
using static ZB.MOM.WW.OtOpcUa.OpcUaServer.IntegrationTests.ServerEventRig;
|
||||||
|
|
||||||
@@ -135,6 +136,70 @@ public sealed class ClientAlarmMethodTests
|
|||||||
finally { await rig.DisposeAsync(); }
|
finally { await rig.DisposeAsync(); }
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/// <summary>
|
||||||
|
/// The client's alarm SUBSCRIPTION (Client.CLI <c>alarms</c>) reads the condition's two-state fields off a
|
||||||
|
/// real event. Its filter used to pass <c>"ActiveState/Id"</c> to <c>EventFilter.AddSelectClause</c>, which
|
||||||
|
/// does NOT split the string: the operand's browse path was ONE QualifiedName named <c>ActiveState/Id</c>,
|
||||||
|
/// which matches nothing, so every event arrived with Active/Acked/Enabled null and the client reported
|
||||||
|
/// them all inactive and unacknowledged — while a direct Read of the same condition said Active. The
|
||||||
|
/// fake-adapter unit tests feed field values straight in and so never exercise the filter.
|
||||||
|
/// </summary>
|
||||||
|
/// <param name="native">Drive the native (raw device folder) or the scripted (equipment folder) condition.</param>
|
||||||
|
[Theory]
|
||||||
|
[InlineData(true)]
|
||||||
|
[InlineData(false)]
|
||||||
|
public async Task The_client_alarm_subscription_reports_active_acked_and_exact_severity(bool native)
|
||||||
|
{
|
||||||
|
var ct = TestContext.Current.CancellationToken;
|
||||||
|
var rig = await StartAsync($"ClientAlarmSubscribe{native}", encrypted: false, ct);
|
||||||
|
try
|
||||||
|
{
|
||||||
|
rig.Enforce(false, [Key()], ClusterGrant(KeyPrincipal, "acl-key", All));
|
||||||
|
if (native) rig.MaterialiseNative(); else rig.MaterialiseScripted();
|
||||||
|
|
||||||
|
using var client = await ConnectClientAsync(rig, ct);
|
||||||
|
using var observer = await rig.ConnectAsync(anonymous: false, ct);
|
||||||
|
var source = native
|
||||||
|
? new NodeId(RawDeviceFolder, (ushort)observer.NamespaceUris.GetIndex(V3NodeIds.RawNamespaceUri))
|
||||||
|
: UnsNodeId(observer, EqA);
|
||||||
|
|
||||||
|
var seen = new List<AlarmEventArgs>();
|
||||||
|
client.AlarmEvent += (_, e) => { lock (seen) seen.Add(e); };
|
||||||
|
await client.SubscribeAlarmsAsync(source, 100, ct);
|
||||||
|
|
||||||
|
AlarmEventArgs? Latest(string message)
|
||||||
|
{
|
||||||
|
lock (seen) return seen.LastOrDefault(e => e.Message == message);
|
||||||
|
}
|
||||||
|
|
||||||
|
// 800 is not one of the SDK's five EventSeverity values (900 is): the node must carry it verbatim.
|
||||||
|
rig.Project(native, new AlarmConditionSnapshot(Active: true, Acknowledged: false, Confirmed: true,
|
||||||
|
Enabled: true, AlarmShelvingKind.Unshelved, 800, "raised"));
|
||||||
|
await RecordedEvents.WaitUntilAsync(() => Latest("raised") is not null);
|
||||||
|
var raised = Latest("raised")!;
|
||||||
|
raised.ActiveState.ShouldBeTrue("the condition is active");
|
||||||
|
raised.AckedState.ShouldBeFalse();
|
||||||
|
raised.Severity.ShouldBe((ushort)800);
|
||||||
|
raised.Retain.ShouldBeTrue();
|
||||||
|
|
||||||
|
rig.Project(native, new AlarmConditionSnapshot(Active: true, Acknowledged: true, Confirmed: true,
|
||||||
|
Enabled: true, AlarmShelvingKind.Unshelved, 800, "acked"));
|
||||||
|
await RecordedEvents.WaitUntilAsync(() => Latest("acked") is not null);
|
||||||
|
Latest("acked")!.AckedState.ShouldBeTrue("the positive control for the Acked field");
|
||||||
|
Latest("acked")!.ActiveState.ShouldBeTrue();
|
||||||
|
|
||||||
|
rig.Project(native, new AlarmConditionSnapshot(Active: false, Acknowledged: true, Confirmed: true,
|
||||||
|
Enabled: true, AlarmShelvingKind.Unshelved, 800, "cleared"));
|
||||||
|
await RecordedEvents.WaitUntilAsync(() => Latest("cleared") is not null);
|
||||||
|
var cleared = Latest("cleared")!;
|
||||||
|
cleared.ActiveState.ShouldBeFalse();
|
||||||
|
cleared.Retain.ShouldBeFalse("an inactive, acknowledged condition is not retained");
|
||||||
|
|
||||||
|
await client.UnsubscribeAlarmsAsync(ct);
|
||||||
|
}
|
||||||
|
finally { await rig.DisposeAsync(); }
|
||||||
|
}
|
||||||
|
|
||||||
private static async Task<OpcUaClientService> ConnectClientAsync(ServerEventRig rig, CancellationToken ct)
|
private static async Task<OpcUaClientService> ConnectClientAsync(ServerEventRig rig, CancellationToken ct)
|
||||||
{
|
{
|
||||||
var client = new OpcUaClientService();
|
var client = new OpcUaClientService();
|
||||||
|
|||||||
@@ -315,6 +315,9 @@ internal sealed class RecordedEvents
|
|||||||
public const int ChangesField = 8;
|
public const int ChangesField = 8;
|
||||||
public const int SourceNodeField = 9;
|
public const int SourceNodeField = 9;
|
||||||
public const int UnshelveTimeField = 10;
|
public const int UnshelveTimeField = 10;
|
||||||
|
public const int ActiveField = 11;
|
||||||
|
public const int SeverityField = 12;
|
||||||
|
public const int RetainField = 13;
|
||||||
|
|
||||||
private readonly object _gate = new();
|
private readonly object _gate = new();
|
||||||
private readonly List<(uint Handle, VariantCollection Fields)> _events = [];
|
private readonly List<(uint Handle, VariantCollection Fields)> _events = [];
|
||||||
@@ -374,6 +377,10 @@ internal sealed class RecordedEvents
|
|||||||
filter.AddSelectClause(ObjectTypes.BaseEventType, BrowseNames.SourceNode);
|
filter.AddSelectClause(ObjectTypes.BaseEventType, BrowseNames.SourceNode);
|
||||||
filter.SelectClauses.Add(new SimpleAttributeOperand(
|
filter.SelectClauses.Add(new SimpleAttributeOperand(
|
||||||
ObjectTypeIds.AlarmConditionType, [BrowseNames.ShelvingState, BrowseNames.UnshelveTime]));
|
ObjectTypeIds.AlarmConditionType, [BrowseNames.ShelvingState, BrowseNames.UnshelveTime]));
|
||||||
|
filter.SelectClauses.Add(new SimpleAttributeOperand(
|
||||||
|
ObjectTypeIds.AlarmConditionType, [BrowseNames.ActiveState, BrowseNames.Id]));
|
||||||
|
filter.AddSelectClause(ObjectTypes.BaseEventType, BrowseNames.Severity);
|
||||||
|
filter.AddSelectClause(ObjectTypeIds.ConditionType, BrowseNames.Retain);
|
||||||
|
|
||||||
var item = new MonitoredItem(subscription.DefaultItem)
|
var item = new MonitoredItem(subscription.DefaultItem)
|
||||||
{
|
{
|
||||||
|
|||||||
Reference in New Issue
Block a user