From 400bef47691e2da3cfbc1ac115427082cc64ce8e Mon Sep 17 00:00:00 2001 From: Joseph Doherty Date: Thu, 18 Jun 2026 05:55:58 -0400 Subject: [PATCH] docs(plan): OpcUaClient ReadEventsAsync event-history passthrough design Driver-internal IHistoryProvider.ReadEventsAsync passthrough to the upstream OPC UA server's HistoryReadEvents service. No interface change: the driver builds a fixed canonical 6-clause EventFilter and maps the upstream HistoryEvent onto the existing HistoricalEvent record (the server re-projects only those 6 BaseEventType fields, so richer clauses are discarded anyway). --- ...26-06-18-opcuaclient-read-events-design.md | 154 ++++++++++++++++++ 1 file changed, 154 insertions(+) create mode 100644 docs/plans/2026-06-18-opcuaclient-read-events-design.md diff --git a/docs/plans/2026-06-18-opcuaclient-read-events-design.md b/docs/plans/2026-06-18-opcuaclient-read-events-design.md new file mode 100644 index 00000000..c9cbaf34 --- /dev/null +++ b/docs/plans/2026-06-18-opcuaclient-read-events-design.md @@ -0,0 +1,154 @@ +# OpcUaClient `ReadEventsAsync` event-history passthrough — Design + +**Date:** 2026-06-18 +**Branch:** `feat/opcuaclient-read-events` (off master) +**Backlog item:** `stillpending.md` §A / `project_stillpending_backlog` item 2 — "OpcUaClient `ReadEventsAsync` event-history passthrough." + +## Goal + +Implement the optional `IHistoryProvider.ReadEventsAsync` on `OpcUaClientDriver` so the +driver forwards OPC UA **HistoryReadEvents** to its upstream server — completing the +driver's history-provider surface (Raw / Processed / AtTime already passthrough; Events was +the lone unimplemented method, inheriting the throwing default-interface body). + +## Key finding (scope correction) + +The backlog framed this as "needs an `IHistoryProvider.ReadEventsAsync` filter-spec/EventFilter +param extension." That is **overcautious and not required.** The interface returns a **fixed +6-field `HistoricalEvent`** record (`EventId, SourceName, EventTimeUtc, ReceivedTimeUtc, +Message, Severity`), and the OtOpcUa server's `OtOpcUaNodeManager.HistoryReadEvents` override +**re-projects only those 6 `BaseEventType` fields** (`ProjectEventField`) — a richer +SelectClause set would be discarded server-side. So the driver builds its **own canonical +`EventFilter` internally** and maps the upstream result onto the existing `HistoricalEvent` +record, exactly as `WonderwareHistorianClient.ReadEventsAsync` already does. + +**Decision (user-approved):** driver-internal implementation, **NO** `IHistoryProvider` / +Core.Abstractions / Commons contract change. + +## Architecture + +`OpcUaClientDriver` already implements `IHistoryProvider` and forwards Raw/Processed/AtTime +through a shared wire path (`ExecuteHistoryReadAsync` → `Session.HistoryReadAsync`). Events +needs a sibling path because the request details type differs (`ReadEventDetails` carrying an +`EventFilter`) and the response payload differs (`HistoryEvent` whose `Events` are +`HistoryEventFieldList` arrays, ordered to match the SelectClauses the driver sent) and the +return type differs (`HistoricalEventsResult`, not `HistoryReadResult`). + +The two duplicated history seams stay distinct and unchanged: + +- **`IHistoryProvider`** = a **driver capability** (OpcUaClient → upstream server). This is + what we extend. +- **`IHistorianDataSource`** = the **server-side** HistoryRead backend (single registered + Wonderware/Null source the OtOpcUa node-manager dispatches to). **Untouched.** Note the + driver's `IHistoryProvider` is not currently bridged into the OtOpcUa server's HistoryRead + service — wiring a driver as a server history backend is a separate, larger concern and is + out of scope. This item completes the driver capability surface. + +## Components (all in `src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.OpcUaClient/OpcUaClientDriver.cs`) + +1. **`internal static EventFilter BuildBaseEventFilter()`** — 6 `SimpleAttributeOperand` + SelectClauses on `ObjectTypeIds.BaseEventType` (`AttributeId = Attributes.Value`), in the + fixed canonical order the mapper relies on: + `EventId, SourceName, Time, ReceiveTime, Message, Severity` (via `BrowseNames.*`). + +2. **`internal static IReadOnlyList MapHistoryEvents(HistoryEvent he)`** — + maps each `HistoryEventFieldList.EventFields[i]` by index → `HistoricalEvent`, with + defensive per-field coercion: + - `[0] EventId` — upstream `byte[]` (ByteString) → base64 string (stable/unique; the field + is opaque "driver-specific format" per the record's XML doc). String passthrough if the + upstream already returned a string. + - `[1] SourceName` — string (null-safe). + - `[2] Time` → `EventTimeUtc` (DateTime). + - `[3] ReceiveTime` → `ReceivedTimeUtc` (DateTime). + - `[4] Message` — `LocalizedText` → `.Text` (or string). + - `[5] Severity` → `ushort` (safe convert). + - Field-lists shorter than 6 degrade gracefully (missing fields → null/default), never throw. + - Empty `he.Events` → empty list. + +3. **`public Task ReadEventsAsync(string? sourceName, DateTime startUtc, + DateTime endUtc, int maxEvents, CancellationToken ct)`** (replaces the inherited throwing + default): + - Resolve the upstream notifier NodeId: `sourceName` null/empty → `ObjectIds.Server` + (i=2253, the standard server-wide event notifier); else `TryParseNodeId(session, + sourceName, out nodeId)` — parse-fail → empty result (mirrors the raw path's malformed- + NodeId short-circuit). `sourceName` is the upstream notifier NodeId, mirroring how + `fullReference` is the upstream NodeId for raw reads. + - Build `ReadEventDetails { StartTime = startUtc, EndTime = endUtc, + NumValuesPerNode = maxEvents <= 0 ? 0u : (uint)maxEvents, Filter = BuildBaseEventFilter() }` + (the `maxEvents <= 0` → 0 "no cap" sentinel honored per the interface's documented + convention; OPC UA treats `NumValuesPerNode = 0` as unbounded). + - Call `session.HistoryReadAsync(ReadEventDetails)` under `_gate`; unwrap + `r.HistoryData?.Body is HistoryEvent he` → `MapHistoryEvents(he)`; preserve the upstream + continuation point (`r.ContinuationPoint`). + - Remove the stale "out of scope for this PR" comment at `OpcUaClientDriver.cs:1678-1682`. + +## Data flow + +``` +driver.ReadEventsAsync(sourceName, start, end, maxEvents) + → BuildBaseEventFilter() (canonical 6-clause EventFilter) + → Session.HistoryReadAsync(ReadEventDetails) (upstream HistoryReadEvents) + → upstream HistoryEvent { Events: HistoryEventFieldList[] } + → MapHistoryEvents() (index-mapped → HistoricalEvent[]) + → HistoricalEventsResult(events, continuationPoint) +``` + +## Error handling + +- No session → `InvalidOperationException` (via `RequireSession()`, same as the sibling reads). +- Unparseable `sourceName` → empty result (mirrors raw's malformed-NodeId short-circuit). +- Upstream rejects events (e.g. opc-plc is not a historian) → the upstream Bad status surfaces + through the SDK; empty `Events` returns an empty list, never a throw. Cascading-quality rule: + no status translation. + +## Testing (NO bUnit; xUnit + Shouldly) + +Mirrors the existing history-test split: the **pure cores** are `internal static` and unit- +tested offline (like `MapAggregateToNodeId`); the thin wire glue is covered by the without-init +unit test + the skip-gated integration smoke (the driver exposes no in-process session-injection +seam, so the unwrap glue is integration/live-proven — same as today's Raw/Processed path). + +- **Pure unit (offline, `OpcUaClientHistoryTests`):** + - `BuildBaseEventFilter` → 6 clauses, canonical order, correct `BrowseNames` + `Value` attr + + `BaseEventType` type-definition. + - `MapHistoryEvents` → full 6-field map; EventId base64; short/empty field-list resilience; + empty `Events` → empty list. +- **Contract flip:** replace `ReadEventsAsync_throws_NotSupportedException_as_documented` with + `ReadEventsAsync_without_initialize_throws_InvalidOperationException` (mirrors the sibling + without-init tests). +- **Integration smoke (skip-gated, `OpcUaClientSmokeTests`, opc-plc):** connect + + `ReadEventsAsync` against the Server node returns a `HistoricalEventsResult` gracefully (no + throw), proving the wire request is well-formed and handled. + +The unit + integration test projects already suppress the `UnwrappedCapabilityCallAnalyzer` +rule for direct driver-capability calls (the existing smoke tests call `drv.ReadAsync` directly) +— the new tests follow the same suppression; no new analyzer concern (the driver's own method is +an implementation, not a caller). + +## Live verification (honest) + +Live `/run` drives the skip-gated integration smoke against opc-plc (`--alm`): proves the driver +issues HistoryReadEvents and handles the upstream verdict. A **non-empty** event list is +**infra-gated** — opc-plc generates live alarm conditions for subscriptions but is **not** a +historian, so it does not serve historical events; a non-empty proof needs an upstream OPC UA +server that historizes events. Stated plainly (same class of caveat as the recent +S7-Timer/Counter and TwinCAT-live items). + +## Hard rules honored + +- NO Commons / proto / EF / `IHistoryProvider` contract change. +- NO bUnit. NO `git add .` (stage by explicit path). Never stage `sql_login.txt`, + `src/Server/.../pki/`, `pending.md`, `stillpending.md`, `docker-dev/docker-compose.yml`. +- No force-push, no `--no-verify`. +- Driver-internal only (`Driver.OpcUaClient` + its two test projects + docs). +- Finish = merge to master + push. + +## Slices (for the implementation plan) + +- **T1** — pure cores: `BuildBaseEventFilter` + `MapHistoryEvents` (+ field-coercion helpers) as + `internal static`, with full offline unit tests (TDD). +- **T2** — wire-in: `public ReadEventsAsync` using the pure cores + `Session.HistoryReadAsync`; + remove the stale "out of scope" comment; replace the NotSupported contract test with the + without-init test. +- **T3** — integration smoke (skip-gated) + docs (driver comment region, `docs/Historian.md` + driver-passthrough note, the backlog line) + build + tests + live `/run` + finish (merge+push).