Two bugs caught by live verification against the mxaccessgw at 10.100.0.48:5120:
- MaxAttempts=1 produced an invalid Polly RetryStrategyOptions -> the probe failed
on every real gateway. Removed the Retry override (matches GalaxyDriver); fail-fast
is already guaranteed by the TCP preflight + the per-call deadline.
- A rejected key surfaces as a typed MxGatewayAuthenticationException, not a raw
RpcException, so 'auth-rejection = reachable' was bypassed. Catch the typed auth/
authorization exceptions -> Ok=true.
Adds DriverProbeHandshakeE2eTests: direct-probe, skip-gated cross-protocol green/red
discrimination (Modbus, OpcUaClient, Galaxy + a local real OPC UA server).
Replaces the bare-TCP AbLegacyDriverProbe with a two-phase probe:
Phase 1 is the existing TCP preflight; Phase 2 initialises a
LibplctagLegacyTagRuntime (Protocol.ab_eip + per-family PlcType) to
open a real PCCC-over-EIP session, using AbLegacyProbeOptions.ProbeAddress
("S:0") as the probe tag. Status-code discrimination mirrors the AbCip
probe: ErrorNotFound/ErrorNoMatch/ErrorBadDevice → Ok=true "controller
reachable"; transport errors → Ok=false "handshake failed".
Adds AbLegacyDriverProbeTests (5 unit tests, all green, 168 total).
Replace the bare-TCP-only AbCipDriverProbe with a two-phase check:
Phase 1 keeps the existing TCP preflight; Phase 2 initialises a
LibplctagTagRuntime against the first device to open a real EIP session
and CIP Forward Open, so a live-but-rejecting CIP endpoint reads red
instead of a false-positive green.
Status mapping: ErrorNotFound / ErrorNoMatch / ErrorBadDevice → reachable
(controller answered CIP, probe tag absent); ErrorTimeout / ErrorBadConnection
/ ErrorBadGateway / ErrorWinsock / ErrorOpen / ErrorClose / ErrorRead /
ErrorWrite / ErrorBadReply / ErrorRemoteErr / ErrorPartial / ErrorAbort →
handshake failed. LibPlcTagException message text is used as a secondary
signal for the reachable-exception path. All other statuses default to
handshake-failed (conservative).
Add AbCipDriverProbeTests: invalid JSON, no devices, malformed host address,
closed-port TCP rejection, and black-hole timeout — all offline-determinable.
Happy path + CIP-error path covered live against the CIP sim.
Replace the bare TCP-connect return in OpcUaClientDriverProbe with a real
OPC UA GetEndpoints discovery handshake (mirroring SelectMatchingEndpointAsync
in the driver). TCP preflight still fast-fails closed ports; the handshake
confirms the remote is actually an OPC UA server, so a live-but-rejecting
non-OPC-UA process now reads RED instead of a false-healthy green.
Replace bare TCP-connect with a two-phase probe: Phase 1 keeps the
existing SocketException / timeout / generic preflight paths unchanged;
Phase 2 runs Plc.OpenAsync (COTP CR/CC + S7 setup-communication) so a
device that accepts TCP but is not an S7 PLC reads red instead of green.
A linked CTS distinguishes caller cancellation ("timed out") from the
S7netplus internal read-timeout OCE ("handshake failed: timed out").
Replace the bare TCP-connect probe in ModbusDriverProbe with a two-phase
check: TCP connect via ModbusTcpTransport (keeps the same SocketException /
timeout / generic error paths and messages), then a one-shot FC03 Read
Holding Registers (qty 1 @ addr 0). A normal response → Ok=true "Modbus
FC03 OK"; a Modbus exception PDU → Ok=true "Modbus FC03 OK (device
returned exception PDU)"; any other failure after TCP succeeds → Ok=false
"Reachable at host:port but Modbus FC03 handshake failed: …".
Add ModbusDriverProbeTests (6 tests) covering invalid JSON, missing
host/port, closed port, TCP-accept-then-close, canned MBAP happy path,
and Modbus exception PDU path. All 277 Modbus tests green.
Add IFocasClientFactory.EnsureUsable() — a config-time probe called by
FocasDriver.InitializeAsync before any background loops start. The
UnimplementedFocasClientFactory throws NotSupportedException immediately
(faulting the driver at init), eliminating the footgun where a driver on
the 'unimplemented' backend appeared Healthy then failed every read/write/
subscribe silently. WireFocasClientFactory and FakeFocasClientFactory are
no-ops. Backstop Create() throw remains in place.
Make MapDataType internal, split the combined Int64/UInt64 arm to return
DriverDataType.Int64 and DriverDataType.UInt64 respectively, and remove
the now-stale Driver.Modbus-007 caveat doc block and inline comment.
Add a Theory covering both cases; full suite 271/271 green.
The owning DriverInstanceActor re-subscribes alarms on every Connected
entry (DetachAlarmSource nulls its cached handle on Connected->Reconnecting
without calling UnsubscribeAlarmsAsync), and the driver object + its alarm
projection are reused across every in-place reconnect. Each SubscribeAsync
started a fresh, never-cancelled Task.Run poll loop and added it to _subs,
so N reconnects leaked N concurrent loops all polling the device and all
firing the same raise/clear transitions => duplicate alarm events + CPU/mem
growth.
Mirrors the Galaxy #399 fix (Clear-before-Add) but for live poll loops the
collapse must also CANCEL the superseded loops, not just drop references.
SubscribeAsync now snapshots existing subs under _subsLock, clears _subs,
adds the new sub, starts its loop, then retires each stale sub out-of-band
(RetireAsync: Cancel + await loop + Dispose CTS, fire-and-forget so the new
subscription's return isn't blocked on a poll interval). Snapshot+clear under
the same lock DisposeAsync uses guarantees no double-own / double-dispose.
There is exactly one consumer per driver instance (factory-per-actor), so
retiring all prior subscriptions before starting the new one is faithful.
Regression tests (TDD, fail->pass): subscribe twice then drive one device
raise; assert OnAlarmEvent fires exactly once (was twice with two leaked
loops).
GalaxyDriver's StreamAlarms feed is session-less and survives an in-place
reconnect, so DriverInstanceActor re-subscribed on every Connected re-entry
(after dropping its own cached handle without an Unsubscribe — sync teardown).
The re-subscribe was additive: _alarmSubscriptions.Add grew the list by one
untracked handle per reconnect cycle — a slow unbounded leak. Functionally
harmless (the gate is Count>0 and OnAlarmFeedTransition only reads [0], firing
once regardless), but it accumulated forever.
Fix: SubscribeAlarmsAsync clears the set before adding, collapsing to a single
live handle (under the existing _alarmHandlersLock, atomic w.r.t. the fan-out
reader). There is exactly one consumer per driver instance (factory-per-actor
lifecycle), so replacing the set with the latest handle is faithful. Chosen
over making the actor's sync DetachAlarmSource call UnsubscribeAlarmsAsync
async/fire-and-forget — disproportionate for a minor leak.
Regression test Re_subscribe_collapses_to_a_single_handle_no_accumulation
(TDD-verified: FAILS without the Clear — releasing the latest handle leaves
the feed open because stale handles remain; PASSES with the fix). Galaxy tests
263 pass / 3 skip; Runtime native-alarm 24 pass. Code-reviewed (approved).
HandleRestartDriver stopped + respawned the child within one synchronous
message handler, reusing the base actor name drv-<id>. Context.Stop is async
(the child processes its own stop on its own mailbox), so the old child was
ALWAYS still registered when the respawn ran — Context.ActorOf threw
InvalidActorNameException deterministically on every AdminUI Restart press,
crashing + restarting the host.
Fix: a monotonic _childSpawnGeneration counter (single-threaded actor) feeds a
-g<gen> suffix on every spawned child name, so a respawn can never collide with
the still-terminating predecessor. Children are tracked by the _children dict
(by IActorRef), never by actor path, so the suffix is invisible to callers.
This also closes the same-shaped latent race in the reconcile path (a removed-
then-readded instance, and a driver-type-change ToStop+ToSpawn in one plan).
Regression test RestartDriver_respawns_the_child_without_an_actor_name_collision
(verified: FAILS on the old code with the exact InvalidActorNameException,
PASSES with the fix). Runtime.Tests 238/238 green. Code-reviewed (approved).
C1 (critical): a boundary tie cluster larger than NumValuesPerNode could
silently truncate a resumed read to GoodNoData, permanently dropping the
un-emitted ties — the (timestamp, skip) cursor cannot advance past a single
timestamp the fixed-(start,end,cap) backend keeps re-returning. Now detected
and failed LOUDLY per node with BadHistoryOperationUnsupported + a log naming
the tag/timestamp/cap; documented in Historian.md with the larger-cap remedy.
Regression test Raw_tie_cluster_larger_than_page_fails_loudly_not_silently.
I3: build HistoryData before Save() so a projection failure can never orphan a
stored continuation cursor.
N1 (YAGNI): drop the never-produced HistoryReadKind enum + Processed-only
Aggregate/IntervalTicks fields from HistoryContinuationState — only Raw pages.
N3: ComputeResumeCursor guards its documented non-empty precondition.
I1: document InMemoryHistoryContinuationStore's eventual-consistency (test double).
Build clean, 182/182 OpcUaServer tests pass.
The Wonderware historian backend is single-shot — it returns up to
NumValuesPerNode samples with a null continuation point — so paging is
synthesised server-side, time-based, for the only count-capped arm (Raw):
- A full page (count == NumValuesPerNode, NumValuesPerNode > 0) emits an
opaque 16-byte continuation point and stores a resume cursor; a short page
(or NumValuesPerNode == 0 "all values") emits none.
- A resume read takes the stored cursor, reads the next page from the boundary
forward, and emits a fresh CP only if that page is also full.
- The resume cursor is tie-safe (HistoryPaging.ComputeResumeCursor /
TrimBoundaryDuplicates): the next page resumes from the boundary timestamp
INCLUSIVE and drops the head ties already returned, so samples sharing the
boundary SourceTimestamp are neither duplicated nor skipped.
Continuation points are bound to the OPC UA session via the SDK's
ISession.SaveHistoryContinuationPoint / RestoreHistoryContinuationPoint store
(SessionHistoryContinuationStore) — capped by ServerConfiguration.
MaxHistoryContinuationPoints (default 100, oldest-evicted) and disposed on
session close. releaseContinuationPoints is honoured via an override of
HistoryReleaseContinuationPoints (the base dispatcher routes release-only reads
there, never to the per-details arms). An unknown / evicted / released point
resumes to BadContinuationPointInvalid.
Processed and AtTime stay single-shot: neither details type carries a client
count cap, so the single-shot backend returns the complete result in one read
and there is no "full page" signal to page on (spec-conformant). Modified-value
history remains out of scope.
The pure paging decisions + CP store contract are unit-tested via HistoryPaging
+ InMemoryHistoryContinuationStore; the full multi-page round trip is driven
end-to-end through the node manager with an in-memory store + a series-backed
fake historian (the in-process harness is session-less).
- A.1 (false-rejection safety): restrict the structural fail-fast's confident-mismatch check to
the CLOSED set of built-in types ResolveBuiltInDataType emits (numeric families + Boolean/
String/DateTime/ByteString). Any other expected type (Enumeration, Guid, …) now defers to the
SDK, so a coercible write (Int32→Enumeration) is never false-rejected. + A7/A8 regression tests.
- C.1: guard BuildWriteFailureAuditEvent (under Lock) in try/catch like ReportAuditEvent, so a
SetChildValue surprise is swallowed+logged, never thrown out of the fire-and-forget continuation.
Three deferred 'surface the failed write' enhancements on the write-outcome
self-correction path in OtOpcUaNodeManager:
- Item A: synchronous structural fail-fast. EvaluateEquipmentWriteStructure
(pure static) rejects a structurally-invalid write INLINE (Bad sync) after
the authz gate but before the optimistic dispatch, so the SDK never applies
it. Null payload -> BadTypeMismatch; plus a confidence-gated cheap built-in
type compatibility check (numeric widening + BaseDataType wildcard tolerated;
uncertain cases defer to the SDK's own coercion).
- Item B: Bad-quality blip on device-write failure. On a revert,
RevertOptimisticWriteIfNeeded first publishes the still-applied optimistic
value with StatusCode BadDeviceFailure, then restores the prior value/status
(both under the existing Lock). Documents the queue-coalescing caveat (a slow
subscriber may see only the restored value -> the audit event is the reliable
signal).
- Item C: Part 8 AuditWriteUpdateEvent on device-write failure. Builds an
AuditWriteUpdateEventState (SourceNode=node, AttributeId=Value, OldValue=prior,
NewValue=attempted, ClientUserId from the threaded identity, Message carries
outcome.Reason) under Lock and reports it via Server.ReportEvent OUTSIDE Lock.
Guarded so auditing-disabled / report failure never breaks the revert.
Threads the writing identity's user-id + node into the continuation. Adds 6
unit tests for EvaluateEquipmentWriteStructure. Build clean (0 warnings);
158/158 OpcUaServer.Tests green.
Joseph Doherty