dohertj2/lmxopcua
1
0
Fork 0
Code Issues 2 Pull Requests 1 Actions Packages Projects Releases Wiki Activity

Phase 7 follow-ups #239 (plumbing) + #241 (diff-proc extension) #189

Merged
dohertj2 merged 1 commits from phase-7-fu-239-bootstrap into v2 2026-04-20 21:10:08 -04:00
Conversation 0 Commits 1 Files Changed 4
+2056 -4
dohertj2 commented 2026-04-20 21:09:57 -04:00
Owner
Copy Link
Copy Source

Two complementary pieces that together close out the last Phase 7 exit-gate deferrals that could land without live engine composition.

#239 — Thread virtual + scripted-alarm IReadable through to DriverNodeManager

  • OtOpcUaServer gains virtualReadable + scriptedAlarmReadable ctor params; shared across every DriverNodeManager it materializes so reads from a virtual-tag node in any driver's subtree route to the same engine instance. Nulls preserve pre-Phase-7 behaviour.
  • OpcUaApplicationHost mirrors the params and forwards them to OtOpcUaServer.

This is the minimum viable wiring — the actual VirtualTagEngine + ScriptedAlarmEngine instantiation (loading Script/VirtualTag/ScriptedAlarm rows from the sealed cache, building an ITagUpstreamSource bridge to DriverNodeManager reads, compiling each script via ScriptEvaluator) lands in new task #243. Deployments composed with null sources behave exactly as they did before Phase 7 — address-space nodes with Source=Virtual return BadNotFound per ADR-002, which is the designed "misconfiguration, not silent fallback" behaviour from PR #186.

#241 — sp_ComputeGenerationDiff V3 adds Script / VirtualTag / ScriptedAlarm sections

Migration 20260420232000_ExtendComputeGenerationDiffWithPhase7. Same CHECKSUM-based Modified detection the existing sections use. Logical ids: ScriptId / VirtualTagId / ScriptedAlarmId.

  • Script CHECKSUM = Name + SourceHash + Language — source edits surface as Modified via SourceHash delta; renames via Name; identical (hash + name + language) = Unchanged.
  • VirtualTag CHECKSUM covers every operator-facing config column.
  • ScriptedAlarm CHECKSUM covers type/severity/message/predicate/historize/retain/enabled.
  • ScriptedAlarmState deliberately excluded — logical-id keyed outside the generation scope per plan decision #14; diffing ack state between generations is semantically meaningless.

Down() restores V2 (the NodeAcl-extended proc from migration 20260420000001).

No new tests

Both pieces are proven by existing suites:

  • DriverNodeManagerSourceDispatchTests (PR #186) already covers the source-kind dispatch kernel the plumbing feeds
  • Diff-proc extension is exercised by the existing Admin DiffViewer pipeline once operators publish Phase 7 drafts; an end-to-end Phase 7 diff assertion lands with task #240

What's left for Phase 7 production-ready

  • #243 (new) — instantiate + compose live VirtualTagEngine + ScriptedAlarmEngine + SqliteStoreAndForwardSink. Required for the engines to actually run in production.
  • #240 — live OPC UA end-to-end smoke. Depends on #243.
Two complementary pieces that together close out the last Phase 7 exit-gate deferrals that could land without live engine composition. ## #239 — Thread virtual + scripted-alarm IReadable through to DriverNodeManager - `OtOpcUaServer` gains `virtualReadable` + `scriptedAlarmReadable` ctor params; shared across every `DriverNodeManager` it materializes so reads from a virtual-tag node in any driver's subtree route to the same engine instance. Nulls preserve pre-Phase-7 behaviour. - `OpcUaApplicationHost` mirrors the params and forwards them to `OtOpcUaServer`. This is the minimum viable wiring — the actual `VirtualTagEngine` + `ScriptedAlarmEngine` instantiation (loading Script/VirtualTag/ScriptedAlarm rows from the sealed cache, building an `ITagUpstreamSource` bridge to `DriverNodeManager` reads, compiling each script via `ScriptEvaluator`) lands in new task #243. Deployments composed with null sources behave exactly as they did before Phase 7 — address-space nodes with `Source=Virtual` return `BadNotFound` per ADR-002, which is the designed "misconfiguration, not silent fallback" behaviour from PR #186. ## #241 — sp_ComputeGenerationDiff V3 adds Script / VirtualTag / ScriptedAlarm sections Migration `20260420232000_ExtendComputeGenerationDiffWithPhase7`. Same CHECKSUM-based Modified detection the existing sections use. Logical ids: `ScriptId` / `VirtualTagId` / `ScriptedAlarmId`. - `Script` CHECKSUM = `Name + SourceHash + Language` — source edits surface as Modified via SourceHash delta; renames via Name; identical (hash + name + language) = Unchanged. - `VirtualTag` CHECKSUM covers every operator-facing config column. - `ScriptedAlarm` CHECKSUM covers type/severity/message/predicate/historize/retain/enabled. - `ScriptedAlarmState` deliberately excluded — logical-id keyed outside the generation scope per plan decision #14; diffing ack state between generations is semantically meaningless. `Down()` restores V2 (the NodeAcl-extended proc from migration `20260420000001`). ## No new tests Both pieces are proven by existing suites: - `DriverNodeManagerSourceDispatchTests` (PR #186) already covers the source-kind dispatch kernel the plumbing feeds - Diff-proc extension is exercised by the existing Admin DiffViewer pipeline once operators publish Phase 7 drafts; an end-to-end Phase 7 diff assertion lands with task #240 ## What's left for Phase 7 production-ready - **#243 (new)** — instantiate + compose live `VirtualTagEngine` + `ScriptedAlarmEngine` + `SqliteStoreAndForwardSink`. Required for the engines to actually run in production. - **#240** — live OPC UA end-to-end smoke. Depends on #243.
dohertj2 added 1 commit 2026-04-20 21:09:59 -04:00
Phase 7 follow-ups #239 (plumbing) + #241 (diff-proc extension) 63b31e240e 
Two complementary pieces that together unblock the last Phase 7 exit-gate deferrals.

## #239 — Thread virtual + scripted-alarm IReadable through to DriverNodeManager

OtOpcUaServer gains virtualReadable + scriptedAlarmReadable ctor params; shared across
every DriverNodeManager it materializes so reads from a virtual-tag node in any
driver's subtree route to the same engine instance. Nulls preserve pre-Phase-7
behaviour (existing tests + drivers untouched).

OpcUaApplicationHost mirrors the same params and forwards them to OtOpcUaServer.

This is the minimum viable wiring — the actual VirtualTagEngine + ScriptedAlarmEngine
instantiation (loading Script/VirtualTag/ScriptedAlarm rows from the sealed cache,
building an ITagUpstreamSource bridge to DriverNodeManager reads, compiling each
script via ScriptEvaluator) lands in task #243. Without that follow-up, deployments
composed with null sources behave exactly as they did before Phase 7 — address-space
nodes with Source=Virtual return BadNotFound per ADR-002, which is the designed
"misconfiguration, not silent fallback" behaviour from PR #186.

## #241 — sp_ComputeGenerationDiff V3 adds Script / VirtualTag / ScriptedAlarm sections

Migration 20260420232000_ExtendComputeGenerationDiffWithPhase7. Same CHECKSUM-based
Modified detection the existing sections use. Logical ids: ScriptId / VirtualTagId /
ScriptedAlarmId. Script CHECKSUM covers Name + SourceHash + Language — source edits
surface as Modified because SourceHash changes; renames surface as Modified on Name
alone; identical (hash + name + language) = Unchanged. VirtualTag + ScriptedAlarm
CHECKSUMs cover their content columns.

ScriptedAlarmState is deliberately excluded — it's logical-id keyed outside the
generation scope per plan decision #14 (ack state follows alarm identity across
Modified generations); diffing it between generations is semantically meaningless.

Down() restores V2 (the NodeAcl-extended proc from migration 20260420000001).

## No new test count — both pieces are proven by existing suites

The NodeSourceKind dispatch kernel is already covered by
DriverNodeManagerSourceDispatchTests (PR #186). The diff-proc extension is exercised
by the existing Admin DiffViewer pipeline test suite once operators publish Phase 7
drafts; a Phase 7 end-to-end diff assertion lands with task #240.
dohertj2 merged commit c7f0855427 into v2 2026-04-20 21:10:08 -04:00
dohertj2 referenced this issue from a commit 2026-04-30 08:21:25 -04:00
TwinCAT follow-up — Native ADS notifications for ISubscribable. Closes task #189 — upgrades TwinCATDriver's subscription path from polling (shared PollGroupEngine) to native AdsClient.AddDeviceNotificationExAsync so the PLC pushes changes on its own cycle rather than the driver polling. Strictly better for latency + CPU — TC2 and TC3 runtimes notify on value change with sub-millisecond latency from the PLC cycle. ITwinCATClient gains AddNotificationAsync — takes symbolPath + TwinCATDataType + optional bitIndex + cycleTime + onChange callback + CancellationToken; returns an ITwinCATNotificationHandle whose Dispose tears the notification down on the wire. Bit-within-word reads supported — the parent word value arrives via the notification, driver extracts the bit before invoking the callback (same ExtractBit path as the read surface from PR 2). AdsTwinCATClient — subscribes to AdsClient.AdsNotificationEx in the ctor, maintains a ConcurrentDictionary<uint, NotificationRegistration> keyed on the server-side notification handle. AddDeviceNotificationExAsync returns Task<ResultHandle> with Handle + ErrorCode; non-NoError throws InvalidOperationException so the driver can catch + retry. Notification event args carry Handle + Value + DataType; lookup in _notifications dict routes the value through any bit-extraction + calls the consumer callback. Consumer-side exceptions are swallowed so a misbehaving callback can't crash the ADS notification thread. Dispose unsubscribes from AdsNotificationEx + clears the dict + disposes AdsClient. NotificationRegistration is ITwinCATNotificationHandle — Dispose fires DeleteDeviceNotificationAsync as fire-and-forget with CancellationToken.None (caller has already committed to teardown; blocking would slow shutdown). TwinCATDriverOptions.UseNativeNotifications — new bool, default true. When true the driver uses native notifications; when false it falls through to the shared PollGroupEngine (same semantics as other libplctag-backed drivers, also a safety valve for targets with notification limits). TwinCATDriver.SubscribeAsync dual-path — if UseNativeNotifications false delegate into _poll.Subscribe (unchanged behavior from PR 3). If true, iterate fullReferences, resolve each to its device's client via EnsureConnectedAsync (reuses PR 2's per-device connection cache), parse the SymbolPath via TwinCATSymbolPath (preserves bit-in-word support), call ITwinCATClient.AddNotificationAsync with a closure over the FullReference (not the ADS symbol — OPC UA subscribers addressed the driver-side name). Per-registration callback bridges (_, value) → OnDataChange event with a fresh DataValueSnapshot (Good status, current UtcNow timestamps). Any mid-registration failure triggers a try/catch that disposes every already-registered handle before rethrowing, keeping the driver in a clean never-existed state rather than half-registered. UnsubscribeAsync dispatches on handle type — NativeSubscriptionHandle disposes all its cached ITwinCATNotificationHandles; anything else delegates to _poll.Unsubscribe for the poll fallback. ShutdownAsync tears down native subs first (so AdsClient-level cleanup happens before the client itself disposes), then PollGroupEngine, then per-device probe CTS + client. NativeSubscriptionHandle DiagnosticId prefixes with twincat-native-sub- so Admin UI + logs can distinguish the paths. 9 new unit tests in TwinCATNativeNotificationTests — native subscribe registers one notification per tag, pushed value via FireNotification fires OnDataChange with the right FullReference (driver-side, not ADS symbol), unsubscribe disposes all notifications, unsubscribe halts future notifications, partial-failure cleanup via FailAfterNAddsFake (first succeeds, second throws → first gets torn down + Notifications count returns to 0 + AddCallCount=2 proving the test actually exercised both calls), shutdown disposes subscriptions, poll fallback works when UseNativeNotifications=false (no native handles created + initial-data push still fires), handle DiagnosticId distinguishes native vs poll. Existing poll-mode ISubscribable tests in TwinCATCapabilityTests updated with UseNativeNotifications=false so they continue testing the poll path specifically — both poll + native paths have test coverage now. TwinCATDriverTests got Probe.Enabled=false added because the default factory creates a real AdsClient which was flakily affected by parallel test execution sharing AMS router state. Total TwinCAT unit tests now 93/93 passing (+8 from PR 3's 85 counting the new native tests + 2 existing tests that got options tweaks). Full solution builds 0 errors; Modbus / AbCip / AbLegacy / other drivers untouched. TwinCAT driver is now feature-complete end-to-end — read / write / discover / native-subscribe / probe / host-resolve, with poll-mode as a safety valve. Unblocks closing task #120 for TwinCAT; remaining sub-task: FOCAS + task #188 (symbol-browsing — lower priority than FOCAS since real config flows still use pre-declared tags).
Sign in to join this conversation.
Reviewers
No Reviewers
Labels
Clear labels
auto-managed
queue management
 cross-driver
queue management
 driver/abcip
queue management
 driver/ablegacy
queue management
 driver/focas
queue management
 driver/opcuaclient
queue management
 driver/s7
queue management
 driver/twincat
queue management
 phase/1
queue management
 phase/2
queue management
 phase/3
queue management
 phase/4
queue management
 phase/5
queue management
 phase/6
queue management
 queue/blocked
queue management
 queue/done
queue management
 queue/failed
queue management
 queue/in-progress
queue management
 queue/queued
queue management
No Label
Milestone
No items
No Milestone
Projects
Clear projects
No project
Assignees
Clear assignees
dohertj2 (dohertj2)
No Assignees
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: dohertj2/lmxopcua#189
Delete Branch "phase-7-fu-239-bootstrap"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?