Five drivers ignored the config handed to them on reinitialize, serving the
options their constructor captured — and the deployment sealed green anyway.
Fixed on both halves, as chosen.
Seam (load-bearing): DriverSpawnPlanner routes a changed DriverConfig to
ToStop + ToSpawn instead of an in-place delta, making the factory the single
parse authority. This REVERSES the deliberate decision documented at
DriverSpawnPlan.cs:49-50 ("a pure DriverConfig change stays an in-place delta
— no reconnect"). That reasoning was correct about the resilience pipeline and
wrong about the driver. The accepted price is a reconnect per config edit.
Per-driver, and this split was not anticipated:
- Re-parse in place — Modbus, AbLegacy, OpcUaClient. ParseOptions extracted
from each factory, called from InitializeAsync behind a HasConfigBody guard
so "{}" still keeps the constructor options.
- Respawn-only, deliberately NOT re-parsed — Sql and FOCAS. Each builds more
than options from config (Sql's dialect + resolved connection string,
FOCAS's client-factory backend, both injected at construction), so adopting
new options alone would run a NEW tag set against an OLD connection. Half a
re-parse is worse than none. SqlDriver's doc-comment asserted the opposite
premise — "config parsing belongs to the factory, which builds a fresh
instance" — which was false when written and is true only now.
Two green seals removed from ApplyChildDelta: it overwrote the cached Spec
synchronously BEFORE the child dequeued the message, so the host believed the
new config was live and the next reconcile computed no delta, sealing the
drift permanently; and it Tell'd with no Receive<ApplyResult> registered, so a
failed reinit — including Galaxy's deliberate NotSupportedException —
dead-lettered.
Exposed (not created) by the change: a factory throw is a CONFIG error, and
SpawnChild catches it and silently substitutes a stub. Only a brand-new driver
could reach that before; an ordinary config edit can now. Raised Warning ->
Error with an actionable message. It still does not fail the deployment —
doing so would let one malformed driver block a fleet deploy, so that is a
follow-up rather than a drive-by.
Tests: every pre-existing reinit test in these suites passes "{}", the exact
input a guarded re-parser treats as "keep constructor options" — blind to this
defect by construction. New tests pass CHANGED json; the Modbus one was
verified falsifiable by deleting the re-parse (goes red). Two tests asserted
the old behaviour and were rewritten, including the positive control in
DriverHostActorUnreadableArtifactTests, whose observable moved from
UnsubscribeAsync to ShutdownAsync now that teardown happens by stopping the
child rather than emptying its desired set.
Remaining full-suite failures are pre-existing and environmental, verified
identical on the pre-change tree: Host.IntegrationTests (3) and
Driver.AbLegacy.IntegrationTests (4, docker fixture-gated).
Wave-B EXEMPLAR. Retire the pre-declared/blob-parse tag model; the driver now
builds its RawPath -> definition table from the artifact's RawTagEntry set via a
pure mapper.
- Contracts: rename ModbusEquipmentTagParser -> ModbusTagDefinitionFactory;
TryParse(reference) -> FromTagConfig(tagConfig, rawPath) (def.Name = rawPath,
no leading-brace heuristic); add inverse ToTagConfig(def) serializer; keep all
strict-enum reads + guards; also read stringByteOrder/deadband/coalesceProhibited
so a RawTagEntry round-trips the full authored def. Inspect() unchanged.
- Options: Tags(IReadOnlyList<ModbusTagDefinition>) -> RawTags(IReadOnlyList<RawTagEntry>).
- Driver: _tagsByName -> _tagsByRawPath (Ordinal); resolver byRawPath-only; build
the table from RawTags at Initialize threading entry.WriteIdempotent onto each def;
Discover/Teardown updated.
- Factory: remove ModbusTagDto/BuildTag/ValidateStringLength + ConfigDto.Tags; bind
RawTags from driver-config JSON.
- CLI ModbusCommandBase.BuildOptions: serialise typed defs -> RawTagEntry via ToTagConfig.
- Tests: migrate Tags= -> RawTags via ModbusRawTags.Entries helper; parser tests ->
FromTagConfig(rawPath); factory String-length/enum tests re-seated on the mapper seam.
- Propagated rename to ControlPlane EquipmentTagConfigInspector (outside Modbus projects).
Modbus.Tests 315/315, Addressing.Tests 161/161, Cli.Tests 72/72 green.
Docker-gated Driver.Modbus.IntegrationTests left untouched (won't compile against
the new API; expected).
Adds <summary>/<param>/<returns>/<inheritdoc> where missing and removes
project bookkeeping IDs (task/tracking refs) from shipped code comments,
so the docs read cleanly and CommentChecker is quiet except for known
false positives (PLC/protocol terms, event/IEqualityComparer inheritdoc).
Doc/comment-only; no logic changed; solution builds clean.
Re-review at 7286d320. Modbus-013 (Low): bit RMW now routes the FC03 read through the
validated ReadRegisterBlockAsync (was raw-indexing readResp -> IndexOutOfRange on a truncated
PDU). Modbus-014 (Low): WriteAsync maps InvalidDataException to BadCommunicationError (was
BadInternalError), matching ReadAsync. + TDD.
- DecodeRegisterArray: add String and BitInRegister cases replacing the
default:throw; each element decoded by reusing DecodeRegister on its
contiguous register slice → string[] / bool[]
- ModbusEquipmentTagParser.TryParse: read optional arrayLength key from
TagConfig JSON and thread it into ModbusTagDefinition.ArrayCount
(null when absent or zero, preserving scalar behaviour)
- ModbusArrayTests: 8 new tests covering the two decode cases and the
equipment-tag parser/resolver path; 285/285 green
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.
Adds <summary>, <param>, <typeparam>, and <inheritdoc/> tags to public
members surfaced by commentchecker — resolves 5,847 of 5,869 issues
(99.6%) across three /fixdocs passes.
Driver.Cli.Common-007 + Driver.Cli.Common-008 resolution.
Driver.Cli.Common-007 (High, Correctness):
0x80550000 is the canonical OPC UA spec value for BadSecurityPolicyRejected,
not BadDeviceFailure. The correct spec value for BadDeviceFailure is
0x808B0000 (verified against OPC Foundation Opc.Ua.StatusCodes;
corroborated locally by Driver.Galaxy.Runtime.StatusCodeMap and both
Wonderware historian quality mappers which all hand-pin the correct
value).
The bug was duplicated across six driver modules:
- FocasStatusMapper.BadDeviceFailure
- AbCipStatusMapper.BadDeviceFailure
- AbLegacyStatusMapper.BadDeviceFailure
- TwinCATStatusMapper.BadDeviceFailure
- ModbusDriver.StatusBadDeviceFailure
- S7Driver.StatusBadDeviceFailure
Plus the SnapshotFormatter shortlist that named 0x80550000 as
BadDeviceFailure, and three downstream Modbus tests that asserted
against the wrong value (so CI was blind).
This commit fixes all six native-mapper constants, the formatter
shortlist, and the three Modbus tests in one pass. Added a regression
guard to FormatStatus_does_not_apply_pre_fix_wrong_names that pins
0x80550000 never renders as BadDeviceFailure (mirroring the existing
-001 wrong-name guards).
Behavior change: OPC UA clients consuming the native drivers now see
the canonical BadDeviceFailure (0x808B0000) on device-fault paths
instead of the misnamed BadSecurityPolicyRejected (0x80550000). Wire-
level status semantics now match operator-facing CLI labels.
Driver.Cli.Common-008 (Low, Testing):
Deleted the redundant FormatStatus_names_native_driver_emitted_codes
Theory — its five InlineData rows were already covered by the
well-known Theory in the same commit (5a9c459), and used a weaker
ShouldContain vs the well-known Theory's ShouldBe (exact match).
Verification:
- Driver.Cli.Common.Tests: 43/43 pass (was 48 after the -008 deletion).
- Driver.Modbus.Tests: 263/263 pass.
- Driver.AbCip.Tests: 262/262.
- Driver.AbLegacy.Tests: 157/157.
- Driver.FOCAS.Tests: 178/178.
- Driver.S7.Tests: 112/112.
- Driver.TwinCAT.Tests: 131/131.
Total: 1146 tests across the affected modules, all green.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
- Driver.Modbus-003: route every _health access through ReadHealth /
WriteHealth helpers backed by Volatile.Read / Volatile.Write so a
burst of concurrent ReadAsync callers always sees a complete snapshot.
- Driver.Modbus-007: promoted the Int64 / UInt64 → Int32 surfacing
caveat to a full <remarks> block; rewrote DisableFC23's doc to flag it
as reserved / no-op.
- Driver.Modbus-008: deleted stale duplicate doc, rewrote the
prohibition-block summaries to credit the shipped re-probe loop, and
removed the unused 'status' local in the ModbusException catch arm.
- Driver.Modbus-009: bind-time validation rejects StringLength < 1 for
String tags; ModbusTcpTransport clamps keep-alive intervals to whole
seconds (>=1).
- Driver.Modbus-010: documented WriteOnChangeOnly's cache-invalidation
policy (reads-only) and the write-only-tag caveat.
- Driver.Modbus-011: collected the scattered instance fields into a
single contiguous block at the top of ModbusDriver.
- Driver.Modbus-012: covered the previously-uncovered Reinitialize
state-hygiene, malformed/truncated/empty-bitmap response, and
DisposeAsync teardown paths.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Clear _tagsByName, _lastPublishedByRef, and _lastWrittenByRef in ShutdownAsync
(via the new shared TeardownAsync helper) so a ReinitializeAsync cycle starts
from a clean state, consistent with the existing _autoProhibited.Clear().
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
_lastPublishedByRef was a plain Dictionary<string, object> mutated inside
ShouldPublish, which runs on the PollGroupEngine onChange callback. The engine
runs one background Task per subscription, so a driver with two or more
subscriptions invokes ShouldPublish concurrently on separate threads. Concurrent
TryGetValue/indexer writes on a non-thread-safe Dictionary can corrupt internal
state, drop entries, or throw, crashing the poll loop.
Switch _lastPublishedByRef to ConcurrentDictionary<string, object>; its
TryGetValue and indexer-set operations are individually thread-safe, so the
deadband cache is now correct under concurrent multi-subscription publishing,
consistent with the lock-guarded sibling cache _lastWrittenByRef.
Add an xUnit + Shouldly regression test that runs 24 deadband-configured
single-tag subscriptions concurrently and asserts the poll loop survives without
faulting.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Group all 69 projects into category subfolders under src/ and tests/ so the
Rider Solution Explorer mirrors the module structure. Folders: Core, Server,
Drivers (with a nested Driver CLIs subfolder), Client, Tooling.
- Move every project folder on disk with git mv (history preserved as renames).
- Recompute relative paths in 57 .csproj files: cross-category ProjectReferences,
the lib/ HintPath+None refs in Driver.Historian.Wonderware, and the external
mxaccessgw refs in Driver.Galaxy and its test project.
- Rebuild ZB.MOM.WW.OtOpcUa.slnx with nested solution folders.
- Re-prefix project paths in functional scripts (e2e, compliance, smoke SQL,
integration, install).
Build green (0 errors); unit tests pass. Docs left for a separate pass.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Joseph Doherty