Commit Graph

12 Commits

Author SHA1 Message Date
Joseph Doherty f5475868f7 fix(inspect): trim before the framing check; stop advisories swallowing the backstop (#556); pin the MQTT payloadFormat mode contract (#557 item 2)
#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.
2026-08-10 03:07:18 -04:00
Joseph Doherty 05b424788f fix(tagconfig): close the four #551 residuals — accept-gate-first Inspect, three authoring rules, CSV/service gate, doc sweep
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.
2026-08-10 00:08:06 -04:00
Joseph Doherty a53be2af65 v3(b1-twincat): apply Modbus exemplar pattern to TwinCAT (multi-device)
- Rename TwinCATEquipmentTagParser -> TwinCATTagDefinitionFactory; TryParse ->
  FromTagConfig(tagConfig, rawPath, out def). Drop leading-'{' heuristic + the
  deviceHostAddress key; def.Name = rawPath. Keep strict-enum + Inspect; add a
  Structure guard (sole tag path now) + inverse ToTagConfig.
- Options: Tags -> RawTags (RawTagEntry); keep Devices + protocol fields.
- Driver: _tagsByRawPath (Ordinal); byName-only resolver; build table from
  _options.RawTags via FromTagConfig, threading WriteIdempotent + DeviceName.
  Multi-device: ResolveDeviceHost matches entry.DeviceName against options.Devices
  (by name, then host) -> def.DeviceHostAddress (TODO(v3 WaveC): live host from the
  Device row's DeviceConfig). DiscoverAsync now emits from the table.
- Factory: retire pre-declared tags[] DTO path; bind rawTags; keep Devices.
- Cli: BuildOptions serialises typed defs -> RawTagEntry via ToTagConfig.
- Tests: TwinCATRawTags helper; migrate Tags= -> RawTags=; rename parser tests to
  FromTagConfig; equipment/resolve-host/array tests delivered via RawTags.

Gate: Contracts + Driver build green; Driver.TwinCAT.Tests 192 pass, Cli.Tests 70 pass.
2026-07-15 20:11:23 -04:00
Joseph Doherty e8db0a683f fix(drivers): flip runtime tag parsers to strict enum parsing (#457)
v2-ci / build (pull_request) Successful in 4m12s
v2-ci / unit-tests (pull_request) Failing after 10m15s
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.
2026-07-15 11:28:34 -04:00
Joseph Doherty 4f5bbd253c feat(driver-twincat): shared TagConfigJson readers, writable key, Inspect warnings (R2-11) 2026-07-13 11:00:31 -04:00
Joseph Doherty 9cad9ed0fc docs(xmldoc): fill missing XML docs + strip tracking-ID comments across src
v2-ci / build (push) Failing after 41s
v2-ci / unit-tests (tests/Core/ZB.MOM.WW.OtOpcUa.Cluster.Tests) (push) Has been skipped
v2-ci / unit-tests (tests/Server/ZB.MOM.WW.OtOpcUa.ControlPlane.Tests) (push) Has been skipped
v2-ci / unit-tests (tests/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer.Tests) (push) Has been skipped
v2-ci / unit-tests (tests/Server/ZB.MOM.WW.OtOpcUa.Runtime.Tests) (push) Has been skipped
v2-ci / unit-tests (tests/Server/ZB.MOM.WW.OtOpcUa.Security.Tests) (push) Has been skipped
v2-ci / integration (tests/Server/ZB.MOM.WW.OtOpcUa.Host.IntegrationTests) (push) Has been skipped
v2-ci / integration (tests/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer.IntegrationTests) (push) Has been skipped
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.
2026-07-07 12:38:39 -04:00
Joseph Doherty 98b27fc1b6 fix(code-review): resolve Batch 1 open findings (AdminUI auth, AlarmHistorian dispose guards, docs)
- AdminUI-001: gate Script editor pages at Administrator,Designer + loosen ScriptAnalysis backend to match
- AdminUI-004: explicit [Authorize] on FleetStatus/Alert/ScriptLog hubs
- Core.AlarmHistorian-014: ObjectDisposedException guards on GetStatus/RetryDeadLettered (+ regression test)
- Core.Scripting.Abstractions-004/-007: Deadband tolerance doc + stale ScriptedAlarms.md path
- Host-003: correct config-overlay precedence in ServiceHosting.md
- Configuration-014: LdapGroupRoleMapping collation-dependency doc
- Driver.TwinCAT.Contracts-002: Structure enum doc (discovery-only sentinel)
2026-06-20 22:30:33 -04:00
Joseph Doherty 0c1d5f7f88 review(Driver.TwinCAT.Contracts): first review; fix ReadArrayLength doc
First review at 7286d320. -001 (Low): ReadArrayLength doc corrected (positive int32 via
TryGetInt32, not uint). -002 (Structure datatype doc) Open. Consistent with the sibling
Driver.TwinCAT-017 ArrayLength flow.
2026-06-19 12:22:53 -04:00
Joseph Doherty 3e74239532 feat(twincat): 1-D array symbol read via ADS + IsArray discovery 2026-06-16 21:59:17 -04:00
Joseph Doherty 5ebf541f54 feat(twincat): resolve equipment-tag refs (read + write) via EquipmentTagRefResolver 2026-06-13 11:24:12 -04:00
Joseph Doherty f2f6eeb74e feat(drivers): expose ProbeTimeoutSeconds on every driver Options class
Adds a uniform [Range(1, 60)] ProbeTimeoutSeconds property to all 9
driver Options classes (Modbus 5s, AbCip 5s, AbLegacy 5s, S7 5s,
TwinCAT 10s, FOCAS 10s, OpcUaClient 15s, Galaxy 30s, Historian 15s).
Powers the AdminUI Test Connect button (Phase 7 of the plan).
2026-05-28 09:21:50 -04:00
Joseph Doherty a88721ce31 refactor(driver-twincat): extract TwinCATDriverOptions to .Contracts
Move TwinCATDriverOptions and TwinCATDataType enum to a new
Driver.TwinCAT.Contracts sibling project. TwinCATDataTypeExtensions
(which uses DriverDataType from Core.Abstractions) stays in the
runtime driver as TwinCATDataTypeExtensions.cs.

Replace two doc-comment references:
  <see cref="Core.Abstractions.PollGroupEngine"/> → <c>PollGroupEngine</c>
  <see cref="TwinCATAmsAddress.TryParse"/> → <c>TwinCATAmsAddress.TryParse</c>
per the approved decision — no compilable usings were present.

The runtime Driver.TwinCAT project gains a ProjectReference to .Contracts;
the .slnx is updated accordingly.
2026-05-28 09:01:28 -04:00