AbLegacyDriver stamped LastSuccessfulRead (init, read, write), the read's sample
timestamps and host-state change times from raw DateTime.UtcNow; only the backoff
path read the injectable Clock. All of them now read Clock(), so
NativePcccBridgeStatusHealthTests drives a manual clock (init at T0, read at T1)
and asserts LastSuccessfulRead == T1 exactly instead of spin-waiting for the wall
clock to pass the init stamp (#767). Without the driver change the 6 tests fail
(init stamp is wall-clock, not T0).
Directory.Build.props sets TreatWarningsAsErrors for every project since #766, so
the 75 per-csproj copies (64 src + 11 tests) are removed and the props comment
says it is now the only place. tools/mem-probe keeps its explicit opt-out.
Release build: 0 warnings, 0 errors; a planted CS0168 still fails the build in a
src project and in a test project that used to carry its own copy.
Claude-Session: https://claude.ai/code/session_01GT67AEHd2Acv1YPwGfXEoV
N3: a keyless blob carrying the addressResolved marker was served from its
string by the driver but saved as HR[0] by the editor model / CSV path. The
precedence rule now lives in ONE pure resolver in Driver.Modbus.Contracts
(ModbusTagAddressResolution.Classify/Resolve): keys when region+address are
both present, else a string-only addressString resolved with the driver's
family (refused when it cannot parse), else refused. The marker never decides.
Consumers that now ask it instead of restating it: the driver's tag reader
(ModbusTagDefinitionFactory.FromTagConfig), the deploy gate (Inspect /
DescribeRejection), the editor model (AddressShape / IsStringOnly /
HasKeyAddress / ResolveWith, ToJson stamping, Validate), the CSV export cells
and the editor hint. The model writes keys only when it loaded them or a field
was authored - never defaults it did not resolve (also closes two divergences
the new matrix found: malformed-string+marker and non-string addressString were
saved as HR[0]).
One table-driven test (ModbusAddressAuthorityAgreementTests) runs 15 persisted
shapes through driver, deploy gate, model load/save/Validate and the real CSV
export->import under all four families; breaking the CSV export or reverting
the model to the marker rule fails it.
Minors: m1 CSV address cells keep the string+marker while it still describes
them (lossless unchanged round trip), drop it when contradicted, and the review
grid reports the drop; m2 the pre-lane test uses the real export residual; m3
unmarked string-only + typed-cell case added; m4 stale docs/summary and the
unused factory parameter removed.
Refs #741
Claude-Session: https://claude.ai/code/session_01Ub9hGmYLQvn9XETzw7CMsT
N2: pre-#741 rows carry region/address beside an addressString with no marker
(the old driver never read the string, the old model preserved it as an
unknown key). The r1 rule resolved those from the string - a silent remap or
BadNodeIdUnknown on upgrade. The driver now loads from region+address whenever
both are present (classic, editor-pinned, pre-lane); only a blob with NO keys
is string-only and resolved through the parser. This is safe because the model
no longer stamps keys onto a string-only tag. The model distinguishes
string-only / legacy-string-beside-keys / pinned, and the editor hint says so.
Test matrix over every persisted shape (keys only, pre-lane keys+string,
pre-lane stamped HR/0+string, string only, marker+keys, marker no keys) under
every family; the pre-lane row also through a model save and a CSV
export->import.
n5: a typed CSV address cell is an authored address - it drops addressString
and the marker (new CsvTypedColumn.ClearsKeys) instead of being discarded.
n6: Validate skips the region/dataType/byteOrder enum checks on a string-only
tag (the driver ignores them). n7: docs/v2/modbus-addressing.md restates the
key-presence rule.
Refs #741
Claude-Session: https://claude.ai/code/session_01Ub9hGmYLQvn9XETzw7CMsT
N1: the m2 precedence trusted region/address whenever the keys were present,
but the AdminUI model stamps defaulted HoldingRegisters/0 on every save, so a
string-only tag (CSV import, or any editor toggle) silently became HR[0] -
reads AND writes. The resolved keys are now authoritative only when the editor
marked them ("addressResolved": true, written by Resolve); an unmarked
addressString is always resolved through the parser with the driver's family.
The model no longer stamps address fields for a string-only tag, CSV export
leaves its address cells blank, and the editor hint distinguishes string-only
from pinned tags. Tests: CSV import and editor save of a string-only tag, a
stamped-default blob, and the pinned positive control across families.
n1: docs/v2/modbus-addressing.md updated (FC23 opt-in, maxReadGap semantics,
coalescing scope, addressString/family rules, coil coercion, refusals).
n2: form round-trip tests for UseFC23ForMultiRegisterWrites /
DisableReadCoalescing. n4: the editor keeps unresolved address text when the
parent echoes back its own emit. n3: noted, no change.
Refs #741
Claude-Session: https://claude.ai/code/session_01Ub9hGmYLQvn9XETzw7CMsT
Coordinator rulings and Important findings from review r0:
1. FC23 is opt-in (new UseFC23ForMultiRegisterWrites, default false); a config
without it - including the persisted "disableFC23": false every form-saved
config carries - keeps the FC16 wire. DisableFC23 stays the hard kill-switch.
2. An explicit maxReadGap 0 means coalescing off again (its pre-lane meaning);
the gap-8 default applies only to an absent key.
3. Coils/DiscreteInputs tags persisted with a register dataType (the old
editor saved Int16) are coerced to Bool, never refused, with no deploy
warning (a warning can be promoted to a blocking error). The editor loads
them as Bool. Register region + Bool is still refused.
4. One comms-fault slot for reads and writes: a good read clears a write's
transport fault and vice versa.
Minors: Dispose writes its own Unknown health (m1); persisted region/address
are authoritative over addressString so a Family change cannot remap a live
tag (m2); a malformed-but-delivered reply no longer fails the unit fast (m3);
a fresh socket published after a concurrent dispose is torn down (m4); the
editor resets address text per tag (m5); master merged (m6); the FC23 latch
log says re-init restores it (m7). Back-compat tests use the old persisted
JSON shapes and fail without the fixes.
Refs #741
Claude-Session: https://claude.ai/code/session_01Ub9hGmYLQvn9XETzw7CMsT
U1: a tag's addressString (Modicon / mnemonic / DL205 V-memory / MELSEC
D-X-Y grammar) now resolves through ModbusAddressParser with the DRIVER's
Family and MelsecSubFamily at every initialize and is authoritative; the
family-unaware gates (deploy Inspect, AdminUI parse consult) accept what
some family resolves. The tag editor gets an address-text field that
resolves with the owning driver's family and persists both the string and
the resolved region/offset/type fields; a hand edit drops the string.
DisableFC23 now gates FC23: by default a 2..121-register holding write is
one FC23 write + read-back (a unit answering exception 01, or failing FC23 at
the transport, moves to FC16 for the rest of the generation); RTU framing
deframes the FC23 reply. Live-checked: the pymodbus fixture implements FC23.
Also: the AdminUI driver form serialises ModbusDriverOptions (TimeSpan
strings), which the factory DTO never read - every timing field on the form
was dead. The DTO now accepts both shapes (*Ms keys win).
Refs #741
Claude-Session: https://claude.ai/code/session_01Ub9hGmYLQvn9XETzw7CMsT
U2: MaxReadGap defaults to 8 and 0 now means adjacent-only rather than
off; a new DisableReadCoalescing switch (factory DTO + AdminUI form) turns it
off entirely. The planner also merges Coils/DiscreteInputs (FC01/FC02, gap in
bits), String scalars and BitInRegister tags - sixteen bit tags on one status
word are one FC03 instead of sixteen. Auto-prohibition and its re-probe /
bisection use the region's own function code (a coil range was re-probed
with FC04).
Refs #741
Claude-Session: https://claude.ai/code/session_01Ub9hGmYLQvn9XETzw7CMsT
P6: Coils/DiscreteInputs carry only Bool and register regions never Bool;
the factory (and so the deploy-gate Inspect and the AdminUI parse consult)
refuses a mismatch, and an absent dataType defaults per region (Bool for a
bit region). A HoldingRegisters+Bool tag used to throw out of the coalescing
planner and stall every tag of the subscription; the planner also skips any
incompatible pair defensively. The tag editor keeps the type compatible when
the region changes; the CLI refuses the pair up front. The array-write parity
test records the Coils divergence from DriverArrayWriteSupport (owned outside
this lane) as follow-up F1.
Refs #741
Claude-Session: https://claude.ai/code/session_01Ub9hGmYLQvn9XETzw7CMsT
Clears CommentChecker findings in src/ (3,280 -> 209, the remainder being false
positives like PLC-5/IEEE-754/M0.0) by adding missing summaries/params/returns,
normalising inheritdoc usage, and removing Gitea/review tracking IDs while keeping
the rationale they annotated. Comment-only: comment-stripped sources are identical
to the parent commit.
#556 item 1 — leading whitespace defeated every Inspect's pre-filter. Seven
factories (Modbus/S7/AbCip/AbLegacy/TwinCAT/FOCAS/Mqtt) tested
`reference[0] != '{'`, so a padded blob returned EMPTY and a tag the driver
refuses deployed clean. The check now trims first, matching the idiom
Sql/Calculation/MTConnect/OpcUaClient already used. The raw reference is still
what is handed to the mapper — the driver reads the stored blob, not a trimmed
copy — so the gate keeps mirroring the runtime rather than becoming lenient.
Both directions are pinned: a padded blob the mapper accepts still draws
nothing, and one that is not object-framed AFTER trimming still draws nothing
(it is not an equipment-tag TagConfig at all).
Six of the seven mappers hand the blob to a whitespace-tolerant
JsonDocument.Parse, so their verdict is unchanged by padding. MQTT is the
exception: both its readers carry the same framing gate, so a padded blob is
genuinely refused — which is why the untrimmed pre-filter was a real defect
there and not a cosmetic mismatch.
#556 item 2 — FOCAS and MQTT appended advisories unconditionally and then gated
the catch-all rejection message on `warnings.Count == 0`, so an advisory (which
fires on ACCEPTED blobs too, and says nothing about a refusal) silently
swallowed the only message naming a refusal nothing else can. The two classes
now accumulate in SEPARATE lists and the backstop gates on the rejection count
alone — structural, not string matching. MQTT's swallow was reachable in
production (a padded blob its readers refuse while every describe branch finds
the parsed document well-formed); FOCAS's backstop is currently unreachable by
audit, so its guard is protective.
#557 item 2 — decided and pinned: KEEP the union verdict. Evidence:
FromSparkplugTagConfig hard-codes PayloadFormat=Json and never consults the key;
only MqttSubscriptionManager (the Plain ingest path) decodes by it; and
MqttTagConfigModel.ToJson deliberately omits the key entirely in Sparkplug mode
(#513). So a Sparkplug driver genuinely serves the tag and rejecting it would be
stricter than the runtime. The cost — served behaviour depends on the driver's
MqttMode, which no tag row records — is paid by an ADVISORY naming the key and
the mode dependency, not by changing the verdict, so both gates keep agreeing.
The same invalid value on a blob the union REFUSES stays a rejection; the class
is decided by the accept gate, never by the key.
Falsified: reverting Modbus's trim reddens the padded-rejection theory;
neutering MQTT's backstop guard to the pre-#556 shape reddens the
advisory-plus-unnameable-rejection test; disabling the #557 advisory reddens the
contract pin; and for FOCAS, removing a describe branch alone stays green (the
backstop covers it) while additionally neutering the guard goes red.
Gitea #551.
1. CSV import ran no driver-typed config gate. Its typed columns were always
strictly parsed, but the free-form TagConfigJson column reached the database
unread, so a CSV could author a tag the driver rejects — one that deploys
green and reads BadNodeIdUnknown forever. RawTagCsvMapper.Parse now names the
offending row in the staged review grid, and RawTreeService enforces the same
gate on CreateTag/UpdateTag/ImportTags: a form cannot be the guarantee (the
API and any future caller reach none of them, and this repo has no bUnit).
A rejected import row is an ERROR — the batch inserts nothing and the row is
named — never a skip.
Note the brief's premise that manual entry rides ImportTagsAsync is wrong:
RawManualTagEntryModal commits row-by-row via CreateTagAsync, which is why
the gate had to land on all three write paths.
2/3. The seven pre-#549 Inspect methods ran a hand-list of DescribeInvalidEnum
checks, so every rejection cause not on the list was invisible. They are now
ACCEPT-GATE-FIRST (the MTConnect/Sql/OpcUaClient/Calculation shape): ask the
driver's own reader for the verdict, then explain the refusal, with a
catch-all so "the parse rejects but Inspect said nothing" is unreachable.
That closes S7's absent dataType, AbCip members[], Modbus stringByteOrder and
the type-dependent guards, plus causes the issue did not name (absent
region/address/tagPath/symbolPath/topic, TwinCAT Structure, MQTT's Sparkplug
tuple). FOCAS and MQTT keep advisories about ACCEPTED blobs, so they
diagnose rather than return empty.
Authoring half: ModbusTagConfigModel gains stringByteOrder;
AbCipTagConfigModel asks the new AbCipTagDefinitionFactory.DescribeInvalidMember;
S7TagConfigModel captures whether the SOURCE blob declared dataType and asks
S7TagDefinitionFactory.DataTypeKey/DeclaresDataType. One authority per rule.
4. Doc sweep: the six factory Inspect summaries, EquipmentTagConfigInspector's
class remarks and AdminOperationsActor's comment all claimed invalid enums are
"silently defaulted by the lenient runtime" — false since R2-11 Phase C made
the runtime reads strict. They describe rejections now.
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).
R2-11 Phase C. All six equipment-tag parsers (Modbus/S7/AbCip/AbLegacy/
TwinCAT/Focas) now read enum fields via the new TagConfigJson.TryReadEnumStrict:
absent -> fallback, valid -> parsed, present-but-invalid (typo) -> TryParse
returns false -> EquipmentTagRefResolver.TryResolve false -> driver surfaces
BadNodeIdUnknown, instead of the old lenient path that silently defaulted a
typo to a wrong-width Good.
Modbus flips all three enum fields (region/dataType/byteOrder); the other five
flip dataType. The deploy-time Deployment:TagConfigValidationMode=Error gate is
unchanged and remains the operator pre-flight.
Coverage:
- Six *EquipmentTagParserStrictnessTests inverted from Freeze_typo_* (lenient)
to Typo_*_rejects_the_tag + Valid_*_still_parses.
- TagConfigJsonTests.TryReadEnumStrict_rejects_only_invalid matrix.
- Driver-level end-to-end proof:
AbCipEquipmentTagTests.Driver_read_of_a_typod_dataType_ref_surfaces_BadNodeIdUnknown
drives the real AbCipDriver.ReadAsync through resolve->status.
Golden parity corpus has no typo'd enums, so the flip is a no-op there.
Full solution builds clean; all six driver suites + core + parity + gate green.
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.
- 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
Move ModbusDriverOptions (and companion option types) to a new
Driver.Modbus.Contracts sibling project. The contracts project
references only Driver.Modbus.Addressing (itself zero-dep and
Admin-safe) because ModbusDriverOptions.Probe/Family/Region
properties use enum types that live there.
Drop 'using ZB.MOM.WW.OtOpcUa.Core.Abstractions' and replace
<see cref="IHostConnectivityProbe"/> with <c>IHostConnectivityProbe</c>
per the approved decision — the using was doc-comment-only.
The runtime Driver.Modbus project gains a ProjectReference back to
.Contracts; the .slnx is updated accordingly.
Joseph Doherty