7b86059daf681892a094eece3f4221eb53f69f4e
64 Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
 Joseph Doherty
|
1ea1da8236 |
refactor(runtime): delete the dead in-place ApplyDelta seam; config change = respawn only (#665)
Since #516 a DriverConfig change is stop + respawn, so DriverSpawnPlan.ToApplyDelta was always empty and the in-place path had no producer. #663 and #664 were both latent bugs in it. This deletes it: - host: ApplyChildDelta, _pendingDelta, HandleApplyResult and HandleDeltaApplied - plan: DriverSpawnPlan.ToApplyDelta - actor: the ApplyDelta, ApplyResult and DeltaApplied messages, the Connected reinit handler and the mid-connect AdoptConfigDuringInit - tests: the actor tests that drove the seam IDriver.ReinitializeAsync and each driver's implementation and unit tests stay. Comments and docs that cited AdoptConfigDuringInit as the reason a driver must release prior state at init now cite the IDriver contract and overlapping retries, which still apply. Claude-Session: https://claude.ai/code/session_01Ef2ozxS7QPanGT8uoATbTe |
||
|
Joseph Doherty
|
ad108f6a02 |
test(mqtt): tag 16 wall-clock-bound tests Category=SlowUnit; document local opt-out (#606)
Claude-Session: https://claude.ai/code/session_01Ef2ozxS7QPanGT8uoATbTe |
||
|
Joseph Doherty
|
7628402893 |
fix(drivers): R5 minors - MTConnect session publish+generation atomic and superseded pump joined; MQTT birth-typed writes use the birth's own datatype; AbLegacy dead init join removed + Dispose join test; docs (#602)
Claude-Session: https://claude.ai/code/session_01Ef2ozxS7QPanGT8uoATbTe |
||
|
Joseph Doherty
|
b29642c5da |
Merge #602 stability: FOCAS/AbLegacy loop joins on shutdown, Postgres untyped string binds, MTConnect pump generation, MQTT birth-typed writes + QoS1 plain writes + claim backoff
Claude-Session: https://claude.ai/code/session_01Ef2ozxS7QPanGT8uoATbTe |
||
|
Joseph Doherty
|
275b240ebd |
fix(mqtt): back off the Sparkplug primary-host claim loop after failures (#602 STAB-27)
Claude-Session: https://claude.ai/code/session_01Ef2ozxS7QPanGT8uoATbTe |
||
|
Joseph Doherty
|
1f4f4fca8f |
fix(mqtt): birth-typed Sparkplug writes and QoS 1 for Plain writes (#602 STAB-25/26)
Claude-Session: https://claude.ai/code/session_01Ef2ozxS7QPanGT8uoATbTe |
||
|
Joseph Doherty
|
e9e35baa70 |
Merge #600.1/#603: provision changed-to-historized tags; retire dormant VirtualTagEngine; cache ScriptSandbox references; one LocalDb replication predicate for registration and drain gate
Claude-Session: https://claude.ai/code/session_01Ef2ozxS7QPanGT8uoATbTe |
||
|
Joseph Doherty
|
a8703c6afe |
refactor(virtualtags): retire dormant VirtualTagEngine/Source/Definition + TimerTriggerScheduler (#603)
TimerTriggerScheduler was welded to the engine and has no production consumer. Claude-Session: https://claude.ai/code/session_01Ef2ozxS7QPanGT8uoATbTe |
||
|
Joseph Doherty
|
7aa7758864 |
build: scoped OTOPCUA0001 NoWarn in 13 test projects, error under src/, pin SDK via global.json (#595 #606)
Claude-Session: https://claude.ai/code/session_01Ef2ozxS7QPanGT8uoATbTe |
||
|
Joseph Doherty
|
6874a562f3 | test+docs(mqtt): end-to-end Bad-then-identical-replay recovery | ||
|
Joseph Doherty
|
b7d20d14a8 | fix(mqtt/sparkplug): initial push on subscribe + BadNoCommunication on connection loss | ||
|
Joseph Doherty
|
ac2f8dc801 | fix(mqtt): stale observed refs BadNoCommunication on connection loss so the identical replay recovers (05/STAB-18 shape a) | ||
|
Joseph Doherty
|
7af9baab39 | fix(mqtt): initial push of cached snapshots on (re)subscribe — the push-model forceRaise (05/STAB-18 shape b) | ||
|
Joseph Doherty
|
50035b36bb | test(mqtt): pin status-code half of republish suppression; note Sparkplug bypass | ||
|
Joseph Doherty
|
2c708f5560 | perf(mqtt): suppress unchanged-value republish fan-out | ||
|
Joseph Doherty
|
acea65eb96 | merge: MQTT browser+contracts review findings resolved (Driver.Mqtt.Browser-002..-005, Driver.Mqtt.Contracts-004..-007) | ||
|
Joseph Doherty
|
93f70504bd |
fix(mqtt-contracts): strict retainSeed, real bounds instead of dead [Range]s, DateTime edges, closed topic formatting
Driver.Mqtt.Contracts-004/-005/-006/-007. -004: retainSeed was the last behavioural key read leniently, so "retainSeed":"false" — what a CSV import produces — silently became the default TRUE, the INVERSE of the authored intent. Both mappers now read it through TryReadBoolStrict (the qos absent/valid/present-but-invalid split) and DescribeRejection names it. The authoring side needed no hand-mirrored rule: MqttTagConfigModel.Validate already consults this same parse through TagConfigParseGate, so the save is blocked with the inspector's wording — pinned by a new AdminUI parity theory pair. -005: the seven [Range] attributes on MqttDriverOptions/MqttPlainOptions never ran — the options are bound by JsonSerializer, and nothing in the four MQTT projects calls Validator.TryValidateObject. Two of them CONTRADICTED the real authority (KeepAlive's [Range(1,…)] forbade the zero Normalize admits as legal MQTT; MaxPayloadBytes' forbade the non-positive value its own remarks document as falling back to 1 MiB), so wiring them up would have broken deliberate behaviour. Deleted, with each property naming where its bound actually lives. The one escape they papered over is closed: an out-of-range defaultQos rode into the SUBSCRIBE verbatim, and MqttSubscriptionManager's constructor now clamps it exactly as the AdminUI driver form already persists. -006: the DateTime write arm cast a negative long to ulong, so 1969-12-31 published a timestamp ~584 million years in the future; it is now refused as ValueNotRepresentable. An Unspecified kind was handed to ToUniversalTime(), which reads it as LOCAL — the same authored instant encoded differently on the two nodes of a redundant pair; it is now read as UTC. -007: Format/FormatState enforced only null/empty, and nothing at all on deviceId, so the type was not round-trip-closed. They now apply the parser's IsValidSegment (plus '/', which only the format side can see). MqttWriteEncoder's own guards stay — TryEncode returns an error rather than throwing, and that contract does not change. |
||
|
Joseph Doherty
|
eddf5f57a5 |
fix(mqtt-browser): converge the plain-mode label guard, make the node cap atomic, total the sibling sort
Driver.Mqtt.Browser-002/-003/-004/-005. -002: ObservePlainTopic's pre-pass judged segment LENGTH only, so a plain topic segment carrying a newline or an ANSI escape became the picker's displayed label — a rule ObserveSparkplugBirth already applied via IsUsableLabel. The control-character half is now single-sourced (HasControlCharacter) and the plain marker text converges with the Sparkplug one. An EMPTY topic level stays accepted on purpose: it is legal MQTT and its terminal leaf is a real committable address. -003: GetOrCreate read _nodeCount and incremented it later, so N observers could each see cap-1 and all insert. The slot is now reserved with one Interlocked.Increment and given back on both non-consuming paths. -004: Project ordered a level by Segment alone, but the level is keyed by Path precisely because two Sparkplug siblings CAN share a Segment — leaving the tie to ConcurrentDictionary enumeration order, which .NET's randomised string hashing makes differ per process. .ThenBy(Path) makes the sort total. -005: no behaviour change — DefaultNodeCap's doc now states the real per-session ceiling and records that the aggregate across sessions is bounded by the reaper's idle TTL, not by any session cap. |
||
|
Joseph Doherty
|
3f85ba985e |
fix(mqtt): stop disposing the primary-host gate a live reconcile still holds
Driver.Mqtt-006 (Low, concurrency). DisposeAsync disposed _gate while StopAsync cancels the reconcile loop WITHOUT awaiting it, so a ReconcileAsync already in flight could be inside _gate.WaitAsync, or on its way to the finally-release, when the semaphore went away. The release then threw ObjectDisposedException naming SemaphoreSlim, logged as a reconcile failure — telling an operator nothing about what happened and hiding whatever the reconcile was really failing on. Left undisposed, matching MqttConnection's documented rule for its own lifecycle primitives and its stated rationale: a late callback racing teardown should be a benign no-op rather than an ObjectDisposedException naming the wrong type. Two lifecycle types in one module disagreeing about this is what gets copied into the third. Nothing is leaked — the gate holds no timer and no registration — and ClaimAsync still re-checks _disposed under it, so no claim can be created after dispose either way. Test is deterministic rather than timing-dependent: the gate is taken before the connect starts, so a MiniBroker in blackhole mode (TCP accepted, CONNECT never answered) accepting the socket proves the claim is inside the gate when dispose runs. RED: log.Captured contained "ObjectDisposedException". |
||
|
Joseph Doherty
|
2298485bda |
fix(mqtt): a reconnect that re-establishes nothing tears the session down
Driver.Mqtt-007 (Low, error handling). OnReconnectedAsync throws when a re-subscribe is granted zero filters — MqttConnection's contract is that a throwing Reconnected subscriber tears the fresh session down and retries under backoff, because a connected-but-deaf driver is the one outcome this path must never serve. But when the live subscribed topics resolved to zero filters in the first place it logged a warning and returned, reaching the same "nothing was re-established" state by a different route and giving it the opposite answer. Throw there too, and say so in the remarks so the type states ONE rule. It costs no other tag — every live topic is in the set that resolved to nothing, so there is nothing left flowing to protect, which is the whole reason the partial- rejection case is exempt — and it is not permanent: the retry runs against whatever Register has published by then. Test reaches the state the only way production can: Register prunes _subscribedTopics of the topics its new table no longer carries, but a SUBSCRIBE already in flight records its own topic AFTER that prune ran, so a deploy landing inside the round-trip leaves a live topic no authored tag resolves to. Paired with the control that a reconnect with no live subscriptions at all stays a no-op — throwing there would tear down the session of every driver whose tags have not been subscribed yet. |
||
|
Joseph Doherty
|
5d4dd17d7f |
fix(mqtt-transport): warn when a configured password is dropped for want of a username
Driver.Mqtt.Transport-002 (Low, correctness). Credentials were attached only when Username was non-empty, so a config carrying a Password and no Username connected anonymously with the secret silently discarded — the deployment reads as authenticated and is not, which is the direction an operator cannot see from the config. Warned rather than sent. MQTT 3.1.1 may not carry a password without a username, so the password-only CONNECT is not available on every protocol version this factory serves, and synthesizing one under V500 only would change the wire behaviour of deployments that connect happily today. The configured values stay authoritative; the drop just stops being silent. The secret is never formatted into the message. Tests: the warning fires and does NOT contain the password, plus the falsifying control that an ordinary anonymous config (neither credential set) stays silent — without it the fix could warn on every connect and still pass. |
||
|
Joseph Doherty
|
d68009106b | merge: Mqtt runtime + Transport code review — deaf-reconnect subscription reset (High), teardown-before-reinit session leak (High), timeout normalization, dead CA-pin warning; 12 findings | ||
|
Joseph Doherty
|
2b4b3fcbbf |
fix(mqtt): session-rebind subscriptions, re-init teardown, timeout clamp
Findings from the per-module review of Driver.Mqtt at
|
||
|
Joseph Doherty
|
3118213772 |
fix(mqtt-browser): reject a wildcard rebirth scope before any NCMD leaves
SparkplugRebirthScope (the AdminUI's typed input) rejects '+'/'#' and its own
remarks say it is 'advisory, not the gate' because MqttBrowseSession
.ResolveRebirthTargets 'refuses an unusable one BEFORE any message leaves'. That
claim was false: the bare {group}/{edgeNode} branch — honoured on purpose, since a
node that has not birthed is the prime rebirth target — gated on IsUsableLabel,
which checks length and control characters only. A scope of 'Plant1/#' therefore
published spBv1.0/Plant1/NCMD/# , a PUBLISH the MQTT spec forbids and a conformant
broker answers by closing the browse connection; '+' would have fanned one
operator click across every matching edge node.
The check is a separate IsUsableRebirthSegment rather than a tightening of
IsUsableLabel: that helper also judges Sparkplug METRIC names, which are arbitrary
publisher-chosen UTF-8 and never part of a topic, so tightening it there would drop
legitimate metrics from the tree. Cases 1 and 2 cannot reach this shape — their ids
come off SparkplugTopic.TryParse, which already rejects both characters.
Driver.Mqtt.Tests 724 (was 708).
|
||
|
Joseph Doherty
|
6eace4bbba |
fix(mqtt-contracts): refuse unpublishable write destinations and advise on them at deploy
A wildcard writeTopic was published verbatim. MQTT-3.3.2-2 forbids a wildcard in a PUBLISH topic and a conformant broker answers a protocol violation by closing the connection — which for this driver is the one connection every subscription lives on, so a single write to a mis-authored tag took the whole driver dark. The Sparkplug side had the same hole plus a worse one: the binding tuple IS the NCMD/DCMD topic, so a '/' in groupId/edgeNodeId/deviceId silently addressed a different edge node than the tag names. MqttWriteEncoder now refuses both as NoWriteDestination (-> BadNotWritable); the read path is untouched. metricName is exempt on purpose — it rides in the payload and Sparkplug's own canonical names use '/'. MqttTagDefinitionFactory.Inspect gained the matching ADVISORIES (never rejections — the mappers accept these blobs, and the gate must never be stricter than the runtime). Both rules were already hard errors in the AdminUI's typed editor, but that is one authoring path of several: the raw-JSON textarea, CSV import and the API all reach the deploy gate directly, so the authoring surface was stricter than the gate — the drift class the #549-#559 arc exists to close. Shared shape rules single-sourced in the new internal MqttAddressRules. Driver.Mqtt.Tests 724 (was 708); AdminUI.Tests 1466; ControlPlane.Tests 261. |
||
|
Joseph Doherty
|
621a9e5dc2 |
fix(mqtt): runtime mappers no longer refuse leading-whitespace TagConfig — JsonDocument.Parse + ValueKind is the single framing authority (#559)
FromTagConfig and FromSparkplugTagConfig each carried a framing pre-check
(`tagConfig[0] != '{'`) evaluated on the UNTRIMMED string, so a TagConfig blob
with leading whitespace was refused by MQTT alone and the tag read
BadNodeIdUnknown forever — while the other six structural-parse drivers
(Modbus, S7, AbCip, AbLegacy, TwinCAT, FOCAS) hand the identical blob to
JsonDocument.Parse, which skips leading whitespace per the JSON spec.
The clause is dropped (the blank guard stays). JsonDocument.Parse plus the
root-ValueKind check now decide framing, exactly as for the six siblings: an
array/number/string root fails ValueKind, non-JSON is a JsonException. The gate
was a validator, not a discriminator — no caller (MqttSubscriptionManager,
SparkplugIngestor, MqttTagConfigModel, Inspect) used the first byte to pick a
format — so verdicts are identical for every input except whitespace-padded
JSON objects, which now get their content's verdict.
Inspect needs no logic change (it has trimmed since #556 and hands the RAW
reference to the mappers), but its comments claimed the padded-blob refusal was
production-reachable; rewritten. That shape was also MQTT's only vehicle for the
catch-all backstop, which is now unreachable-today like FOCAS's — so its test is
replaced by the same count-relation pin, and the backstop code stays as
defence-in-depth.
Tests: both mappers pinned by padded-vs-unpadded DEFINITION equality (a record,
so field-wise), padded non-object/malformed negative controls, an Inspect
verdict+inspection parity theory over the accepted AND rejected corpus with an
anti-vacuity control, and a padded MQTT corpus entry in the AdminUI
authoring/runtime parity suite.
|
||
|
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. |
||
|
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. |
||
|
Joseph Doherty
|
b815446940 |
refactor(drivers): delete GetMemoryFootprint / FlushOptionalCachesAsync (#525)
Both `IDriver` members had no production call site. Their only consumer was `MemoryTracking`, deleted with the driver-tier recycle machinery in #522, and `GetMemoryFootprint`'s doc-comment claimed "polled every 30s by Core" — which had not been true since v3, even before #522 removed the poller. Owner decision on #525 was (a) delete rather than (b) build the process-wide watchdog that would consume them. The driver-side implementations are in git history and were mostly reusable; what was always missing is the trigger, and a member nobody calls is how the misleading doc-comment survived this long. Removed from `IDriver`, all 12 driver implementations, and every test stub — plus the support code that existed only to feed them, which is the part a member-only deletion would have left behind as dead weight: - MTConnect's three per-entry byte-weight constants, its cached `ProbeModelDataItemCount`, and `CountDataItems` — a full device-tree walk run on every cache fill to populate a number with no remaining reader. - OpcUaClient's `_discoveredNodeCount` field and the discovery-pass write to it. - MQTT's `ApproxBytesPerAuthoredTag`. Two behavioural notes, neither a change in what the software does: - `AbCipTemplateCache` now has no production reset path. Its only caller was AB CIP's flush, which itself had no caller — so nothing changes except how visible the fact is. Recorded on the type, along with the correction that its comment's claim about a device `ReinitializeAsync` also flushing it was never true. A UDT redefined on a live controller keeps its stale shape until the driver instance is rebuilt. - MTConnect's `/probe` cache is still dropped, by `InvalidateProbeModel()` on an Agent restart. That is now `internal` rather than `private` so the tests which used the flush purely to empty the cache drive the real path instead — they now cover a case that actually occurs in production. Several unrelated doc comments cited `FlushOptionalCachesAsync` as the canonical CAS example and are repointed to it. Tests: the ones that existed only to assert on the removed members are deleted (3 OpcUaClient, 3 Galaxy, 2 MQTT, 1 MTConnect, 1 AbCip). Worth noting one was vacuous — `FlushOptionalCachesAsync_resets_footprint_counter` asserted a footprint of 0 before the flush and 0 after with no discovery in between, so it would have passed against a flush that did nothing at all. Full suite green except 7 failures verified identical on master before and after (4 AbLegacy integration with the compose profile down, 3 Host.IntegrationTests), checked by name rather than assumed. Docs corrected where they described this as live: `DriverLifecycle.md` (which carried the #525 placeholder), `drivers/AbCip.md`, `drivers/AbLegacy.md`, and `v2/driver-stability.md` — whose "polls this every 30 s" + per-driver `DriverConfig` memory budget were never built as written and are now banner-ed as design intent rather than description. Claude-Session: https://claude.ai/code/session_015p7wGqy3YpZNCpDzTpGMKo |
||
|
Joseph Doherty
|
803f4e6efe |
refactor(sql): split ISqlDialect/SqlServerDialect into a .Dialect leaf (#526)
`Driver.Sql.Browser` referenced the runtime `Driver.Sql` project. The AdminUI references that browser, so the whole SQL driver runtime — and `ZB.MOM.WW.OtOpcUa.Core` through it — sat in the AdminUI's build/deploy graph. This is the same layering violation #512 removed from the MQTT browser, in a second driver, and it is why #512's expected removal of `Core.dll` from the AdminUI's output did not happen. MEASURED, both directions, on a clean AdminUI build: before: ZB.MOM.WW.OtOpcUa.Core.dll + ZB.MOM.WW.OtOpcUa.Driver.Sql.dll present after: both absent The old csproj comment's premise was sound and survives — the dialect's catalog SQL *is* the browse engine, so it must be shared rather than duplicated. What was wrong was the conclusion that sharing it required referencing the runtime driver. `ISqlDialect`/`SqlServerDialect` depend on nothing in `Driver.Sql`, so they move to a `.Dialect` leaf that both consume. This is the `.Transport` shape, not the cheap one: `ISqlDialect` exposes `DbProviderFactory` and `SqlServerDialect` returns `SqlClientFactory.Instance`, so `.Contracts` (deliberately provider-free) cannot hold them. Assembly name is `.Dialect`; the NAMESPACE stays `ZB.MOM.WW.OtOpcUa.Driver.Sql`, matching the `Driver.Mqtt.Transport` precedent — the extraction cost zero `using` changes. Two dependencies the cut edge surfaced as the implicit ones they were: - The AdminUI itself read `SqlDriver.DriverTypeName` in three places — the tag editor map, the tag config validator, and the Sql address picker. All three now read `DriverTypeNames.Sql`, which is exactly what that constant resolves to and exactly the repoint `SqlDriverBrowser`'s own comment had deferred until a factory was registered against the shared constant. One exists now. - `SqlDriverFormContractTests` asserts the AdminUI form's emitted JSON against the runtime factory that reads it, so it genuinely needs the runtime driver. `AdminUI.Tests` now references it explicitly. A test project referencing a runtime driver is fine — tests are not in the AdminUI's deploy graph. New guard: `DriverBrowserLayeringTests` (Commons.Tests) scans the csproj graph of every `*.Browser` project in the tree, transitively. It reads csproj files rather than loaded assemblies because the cost is a build-graph cost — the compiler emits no assembly reference for an unused `ProjectReference`, so `MqttBrowserLayeringTests`' assembly-reference approach cannot see that case. The two are complementary and each now points at the other. Falsified three ways, each red then green again: the direct reference re-added to the browser; the same reference one hop out via `.Dialect` (proving the walk is really transitive); and both controls — browsers-discovered and references-parsed — which exist so an empty discovery cannot pass the absence assertion vacuously. #526's claim that the reference "carries no comment at all" was wrong; it carried a detailed one from the scaffold commit. Corrected on the issue and in the design doc's project table, which had recorded the deviation as deliberate. Claude-Session: https://claude.ai/code/session_015p7wGqy3YpZNCpDzTpGMKo |
||
|
Joseph Doherty
|
c74ccc55b5 |
feat(mqtt): flatten Sparkplug Template/DataSet metrics to sibling tags (#515)
`SparkplugCodec` handled scalars; `DataSet`, `Template` and `PropertySet` were skipped
with a warning. They now flatten to addressable tags (owner decision, recorded on the
issue):
Template → sibling metrics, `/`-joined, recursively: Motor → Motor/Speed, Motor/Stator/Temp
DataSet → one metric per cell, {column}/{rowIndex}: Recipe → Recipe/Step/0, Recipe/Target/1
PropertySet → node metadata, never tags — it describes a metric rather than being one
Flattening was chosen over OPC UA Structures because flattened scalars are consumable by
any client with no type dictionary, and the result mirrors how the `/raw` tree works.
**The row index is always present, including on row 0.** Emitting a bare column name for
a single-row table and an indexed one otherwise would make a tag's name depend on how many
rows the plant published — a table growing from one row to two would rename every tag
already authored against it.
**A DATA metric carries the parent's alias and no name**, so member names cannot be
composed until the alias table has resolved the parent. Expansion therefore happens in
`SparkplugIngestor.PublishStructuredMembers` — after resolution, at the single funnel both
the DATA and birth-value paths already pass through — not at decode time. The alias table
installs a binding per member so browse enumerates them and a DDATA member that omits its
datatype falls back to the birth's (guessing from the CLR type would read a signed Int32 of
-42 out of its unsigned field as 4294967254).
**Skip-and-warn survives, and refusal is whole** — the explicit requirement on the issue.
A template definition, an unnamed member, an extension value, a column/type mismatch, a
short row, or a payload past MaxDepth(5)/MaxMembers(512) refuses the ENTIRE metric. A
silently half-flattened UDT looks exactly like a complete one in the picker.
The browse picker offers the members and deliberately NOT the structured parent, which has
no value and no servable datatype. The rediscovery signature likewise tracks members rather
than the parent, or a UDT gaining a member would leave the signature unmoved.
Member paths contain `/`, which #509 already solved: the slashed name travels intact as
`metricName` while the derived tag Name is coerced to a legal RawPath segment.
Live-gated 3/3 against the Mosquitto fixture with the project-owned simulator extended to
publish a nested Template and a 2x2 DataSet. Falsified at three points: dropping the member
bindings, not expanding the parent, and refusing to flatten templates each turn exactly the
expected tests red.
Also fixes a regression from the #511 commit in the same branch: `AddOtOpcUaDriverFactories`
resolved `IDriverPrimaryRoleSource` with `GetRequiredService` but only `AddOtOpcUaRuntime`
registered it, so a graph wiring driver factories without the full runtime failed at
resolution. Two Host integration suites do exactly that and caught it; both extensions now
TryAdd the same concrete `RedundancyRoleView`, so the registration is order-independent and
the fail-loud resolve is kept.
Claude-Session: https://claude.ai/code/session_015p7wGqy3YpZNCpDzTpGMKo
|
||
|
Joseph Doherty
|
7f2108dcf2 |
feat(mqtt): Sparkplug primary-host STATE publishing, Primary-gated (#511)
`ActAsPrimaryHost` was accepted by config and by the AdminUI form and did nothing but
log a warning. It now delivers the whole three-part contract or none of it: a retained
`ONLINE` on `spBv1.0/STATE/{hostId}`, a matching retained `OFFLINE` armed as the MQTT
Last Will, and an explicit `OFFLINE` on a clean stand-down.
**The claim is a dedicated connection, not a message.** A Will can only be registered in
the CONNECT packet — it cannot be added to a live session, changed, or withdrawn. So a
claim that is acquired and released at runtime cannot ride the long-lived ingest session.
`SparkplugPrimaryHost` opens its own connection (client id `{clientId}-spbhost`, forced
clean-session) when the node gains Primary and closes it when the node loses Primary; the
connection's lifetime *is* the claim, so every leg follows from its lifecycle. Ingest is
untouched by a role change — no dropped subscriptions, no lost birth cache, no data gap at
the Secondary->Primary transition.
**Primary-gated** (owner decision, recorded on the issue). Both pair nodes subscribe
independently, so an ungated claim means both claim the same host id — and the sharp
failure is not the two fighting over the retained STATE, it is that the SECONDARY's death
fires its Will and publishes OFFLINE while the Primary is healthy, stopping every edge node
in the group. Registering the Will only on whichever node is Primary at connect time fails
the other way: a node that later gains Primary would publish ONLINE with no Will behind it.
New seam: `IDriverPrimaryRoleSource` (`Core.Abstractions`) carrying the *data-plane* verdict
(`PrimaryGatePolicy.ShouldServiceAsPrimary`), not the alarm-drain verdict — an unknown role
with a driver peer present must deny. Registered unconditionally by `AddOtOpcUaRuntime`,
deliberately NOT alongside `IRedundancyRoleView`, which `AddAlarmHistorian` registers only
when that section is enabled: piggybacking would leave a pair with no alarm historian
falling back to "claim unconditionally". Polled on a 1s timer rather than evented — a
process singleton raising events at per-instance drivers outlives every driver it holds.
Accepted trade: a brief edge-visible OFFLINE window during failover. That is a genuine
state transition, not a glitch to hide.
Live-gated 5/5 against the Mosquitto fixture, including the Will leg — `AbandonForTests`
drops the socket with no DISCONNECT, because a clean disconnect makes the broker discard
the Will by protocol rule, so a test driven through `StopAsync` would observe the
stand-down OFFLINE and pass against an implementation that never armed a Will. Falsified:
removing the Will turns that leg and only that leg red.
Also corrects two stale claims found en route: the AdminUI form still said MQTT tags are
read-only (#508 shipped), and the driver doc still said nothing consumes `IRediscoverable`.
Claude-Session: https://claude.ai/code/session_015p7wGqy3YpZNCpDzTpGMKo
|
||
|
Joseph Doherty
|
72be19225d |
feat(mqtt): write-through (IWritable) — Plain publish + Sparkplug NCMD/DCMD (#508)
MqttDriver implemented no IWritable, so every MQTT node materialized read-only
regardless of what the operator authored. It now publishes.
Plain -> the tag's writeTopic; UTF-8 text (Raw/Scalar) or a bare JSON
literal (Json)
Sparkplug -> DCMD for a device metric, NCMD for an edge-node metric, carrying
the metric BY NAME with an explicit Datatype and no seq
Both at QoS 0, retain:false. Retain is deliberately not configurable — a retained
command is replayed by the broker on every future subscribe, including the
device's own reconnect, so one stale write would be re-applied forever.
writeTopic is a new, separate TagConfig key, and its ABSENCE is meaningful: the
write is refused BadNotWritable and the node is discovered ViewOnly. The issue's
"do not skip" warning drove the design — the one outcome worse than read-only
nodes is a node that looks writable, returns Good and publishes nothing. Reusing
the read topic would be worse still: it would echo back and read as a confirmed
device value. Browse-time writability is computed by the SAME encoder the write
path uses, so a node cannot advertise writability it would then refuse.
Good means published, not applied — MQTT has no application-level write ack, so
this is Galaxy's optimistic-Good posture and inherits its property: a failed
DEVICE write can never be surfaced. A failed PUBLISH is reported Bad; the
optimism covers what the device did with the command, never whether it was sent.
WriteIdempotent stays default-off.
The encoder is pure and lives in .Contracts, so both wire shapes are testable
without a broker. Unrepresentable values and types with no Sparkplug encoding are
refused rather than coerced — a Reference must not silently become a String on
the wire, and "not-a-number" must not become 0 on a live plant.
LIVE GATE PASSED against the real Mosquitto fixture on 10.100.0.35: 21/21
integration tests, including 6 new ones that observe the published message with
an INDEPENDENT broker subscriber — a fake asserting "publish was called" could
not tell the feature working from the feature publishing to the wrong topic.
The gate earned its keep: the NCMD test initially failed because NCMD is a SHARED
channel — the driver publishes its own rebirth request (Node Control/Rebirth) on
the very topic an edge-node metric write uses, and it arrived first. Correct
driver behaviour, wrong test; the watcher now matches on metric name. Documented,
because anything consuming that topic needs to know. DCMD has no contention.
Offline: 24 encoder + 9 driver tests. The four new status constants are NAMED so
StatusCodeParityTests can see them (#497/#519) — it went 258 -> 262 and validated
each against the pinned SDK.
Also corrects the Mqtt.md "Known gaps" table: prunes the six rows closed in this
round, and fixes the #507 row, which still described rediscovery as inert after
that issue was closed as superseded and its injection path deleted.
Claude-Session: https://claude.ai/code/session_015p7wGqy3YpZNCpDzTpGMKo
|
||
|
Joseph Doherty
|
7f53bd96ec |
fix(mqtt): close #509, #510, #512, #513, #514
Five MQTT tracker items — four AdminUI/authoring defects plus the .Browser layering exception. No runtime driver behaviour changes. #513 payloadFormat in a Sparkplug blob. MqttTagConfigModel.ToJson wrote the key unconditionally. Sharper than the issue states: PayloadFormat is the one Plain field with a NON-NULLABLE typed backing, so the write did not preserve a retyped tag's leftover — it INVENTED "payloadFormat":"Json" in a Sparkplug tag that never had one. Now Plain-only; preserve-unknown-keys is pinned unchanged. #509 slashed Sparkplug metric names. The mapper already carried metricName verbatim; only the tag Name (a RawPath segment) tripped RawPaths.ValidateSegment, which refused the batch whole — including Node Control/Rebirth, mandatory on every Sparkplug edge node. ToRawSegment coerces the Name and leaves the address untouched. Coercion is not injective, so DescribeNameCollisions refuses a colliding set BY NAME: silently merging two metrics into one tag would be worse than the refusal it replaces. Applies to every driver, not just MQTT — the argument is about what a Name is, not about who produced it. The sanitiser is pinned against RawPaths.ValidateSegment rather than a copied charset. #510 stale rebirth affordance. _canRebirth was probed once at browse-open and sessions are reaped for idleness, so a modal left open offered an action whose token no longer resolved. Re-probed on arm AND on confirm (the reap can land between the two, and confirm is the one path that publishes to a live broker); the affordance retires with an explanation instead of vanishing or erroring. #514 rebirth on a quiet plant. The tree is birth-driven and births are never retained, so a plant that birthed before the window opened showed an empty tree — and the scope came only from a tree selection, so the one action that would fill the tree could not be armed. The session already accepted a bare {group}/{edgeNode} ("the prime rebirth target"); this adds the affordance. Edge-node scope only, not a group broadcast. The rule is extracted to SparkplugRebirthScope so it is testable — this repo has no bUnit — and is advisory: the session re-resolves and refuses before publishing. #512 .Browser -> .Driver layering. TWO extractions were needed, not the one the issue anticipated: it costed this as "the client-options builder", but the browse session decodes births itself, so moving only that would have left the reference exactly where it was. - MqttClientOptionsFactory (TLS, PEM CA-pin validator, credentials, protocol mapping) -> new leaf .Transport project. NOT .Contracts: that is deliberately transport-free and MqttClientOptions is an MQTTnet type, so .Contracts would hand MQTTnet to ControlPlane. This is the fix the .Browser csproj comment had already prescribed, against the issue's suggestion. - The Sparkplug protocol layer (SparkplugTopic/MessageType, SparkplugCodec, RebirthRequester) -> .Contracts, beside the Tahu wire schema it encodes. Still exactly one copy of the TLS/CA-pin logic; MqttBrowserLayeringTests guards the graph both ways. Measured, not assumed: Driver.Mqtt.dll, Polly.Core and Serilog are now absent from the AdminUI's output. Core.dll remains, because Driver.Sql.Browser has the SAME violation, undocumented — the old comment blamed this browser for Core and was wrong. Filed separately rather than fixed in passing. Full suite green apart from the 7 known pre-existing failures (4 AbLegacy integration, compose profiles not up; 3 Host.IntegrationTests). Claude-Session: https://claude.ai/code/session_015p7wGqy3YpZNCpDzTpGMKo |
||
|
Joseph Doherty
|
8d9155682d |
fix(mqtt): browse-commit writes a bindable address + a Request-rebirth affordance
Two gaps the Task 23/24 review found, both blocking Task 26's live gate.
Gap 1 — browse-commit produced a silently dead MQTT tag. RawBrowseCommitMapper
had no `Mqtt` case, so a committed leaf fell through to the generic
`{"address": …}` key. Neither MqttTagDefinitionFactory entry point reads
`address`: the tag deployed clean and reported BadNodeIdUnknown forever, with
no signal at commit time. Predates Task 23 (Plain was affected too), but Task 23
built the Sparkplug metric tree precisely so an operator could browse and commit
a binding.
The address is a DESCRIPTOR, not a reference string, and it cannot be recovered
from the browse node id: `{group}/{node}[/{device}]::{metric}` where a metric
name legitimately contains `/` (`Node Control/Rebirth`) — the ambiguity
MetricSeparator's remarks already name. So the session STATES it, via a new
`AttributeInfo.AddressFields` seam, and the mapper reads it. Which keys are
emitted is also how the mapper learns Plain vs Sparkplug — the driver type
reaching it is just `Mqtt`, and the mode lives on the driver config. Key names
are single-sourced in the new `MqttTagConfigKeys` (producer, mapper, factory),
with the literals pinned by a test so a symmetric rename cannot silently unbind
already-persisted blobs. A leaf with no stated address is refused at commit in
words rather than committed dead.
Gap 2 — RequestRebirthAsync had no UI. Task 23 shipped it backend-only; Task 26
step 1 assumes the button, and Task 25's runbook notes the picker tree stays
empty until a birth lands, so it is the only way to fill it on demand. Added to
the /raw browse modal: scope = the clicked tree node (device/metric resolve up to
their edge node, group fans out and is refused whole past 32), an explicit
two-click confirm naming the resolved scope and its consequence, the outcome or
the refusal shown rather than swallowed. Offered only for a Sparkplug session
(new `IRebirthCapableBrowseSession.RebirthAvailable` — one session class serves
both modes, so a type test alone would draw a button that can only throw) and
only to a DriverOperator; the server-side gate remains the boundary.
Tests: MQTT 545 → 581, AdminUI 781 → 800. The round-trip tests feed the emitted
blob to the REAL factory and were falsified by mutating the emitted key name.
Claude-Session: https://claude.ai/code/session_01GASWkNEi68FSCtvr6rLoEW
|
||
|
Joseph Doherty
|
64ec7aa43a |
feat(mqtt): Sparkplug birth-driven browser tree + scoped Request-rebirth action
Unseals the Sparkplug branch of the MQTT address-picker browser that Task 10 deliberately sealed shut, and adds the first (and only) sanctioned publish on a browse session. - MqttBrowseSession serves Group -> EdgeNode -> [Device] -> Metric in Sparkplug mode, decoded from observed NBIRTH/DBIRTH certificates (the only Sparkplug messages that name their metrics). Node-level metrics hang directly under their edge node -- no synthesised device folder, because the authored address tuple genuinely has deviceId = null for them. Metric node ids use a '::' separator: a metric name may contain '/', so slash-joining would make a node-level metric indistinguishable from a device folder. - AttributesAsync is purely birth-derived in Sparkplug mode. The plain path's UTF-8 inference / payload-snippet machinery does not run and no payload bytes are retained: a Sparkplug body is protobuf and would be reported as "(N bytes, binary)" for every metric in the plant. A datatype ToDriverDataType cannot map is reported as the Sparkplug type name, never coerced. - RequestRebirthAsync(scope) is the one publishing member, reached only by an explicit operator action. It routes through the counted PublishAsync chokepoint via a private RebirthTransport adapter, so PublishCountForTest still proves that OpenAsync/RootAsync/ExpandAsync/AttributesAsync/Observe/ DisposeAsync publish nothing -- now in both modes. A device or metric scope resolves up to its owning edge node (NCMD is node-addressed); group scope enumerates observed edge nodes and is refused whole past MaxGroupRebirthNodes = 32. - BrowserSessionService.RequestRebirthAsync gates on the same DriverOperator policy that gates the picker, evaluated server-side where the action happens rather than only at render time, fails closed, and Info-logs the scope and the published count. Exposed through the new IRebirthCapableBrowseSession contract so "does this session write?" stays a type question. - ToBrowseOptions' Last-Will landmine re-verified: MqttDriverOptions still carries nothing will-shaped (an NDEATH is a broker-published will and would be invisible to PublishCountForTest), now pinned by a reflection guard. Falsifiability: injecting a publish into RootAsync reddens the Sparkplug and plain passivity tests; double-publishing per target reddens all four one-NCMD-per-node assertions. 572/572 MQTT tests, 24/24 AdminUI browsing tests. Claude-Session: https://claude.ai/code/session_01GASWkNEi68FSCtvr6rLoEW |
||
|
Joseph Doherty
|
6d7a458c4d |
fix(mqtt): reset Sparkplug ingest state at every session/authoring seam
Task 21 review — two Criticals sharing one root cause, plus four follow-ups.
C1/C2 root cause: `OnReconnectedAsync` opened with `Births.Clear(); _trackers.Clear();`
— "not housekeeping, the correctness step" — and that reset was missing from the
other two seams where the view underneath a surviving cache changes.
- C1: the ingestor OUTLIVES a session rebuild. `SessionIdentity` is a strict
superset of `IngestIdentity`, so changing only Host/Port/ClientId/credentials/
TLS/a timeout gives `!SameSession && SameIngest` — `ReinitializeAsync` tears the
session down and re-establishes on the same instance, and `TeardownAsync` touches
no ingest state (the resilience layer re-running `InitializeAsync` is the second
path). A node that rebound alias 5 during the changeover would publish Pressure's
value under the Temperature RawPath at Good. Hoisted into `ResetSessionState`,
called from `AttachTo` (earliest seam — a persistent session can push between
CONNACK and our SUBSCRIBE), `EstablishAsync` and `OnReconnectedAsync`.
- C2: `Register` pruned trackers but not births. While a node is unauthored its
traffic is dropped at `Dispatch`, so its cached birth FREEZES rather than
refreshing; drop-then-re-add across a week mis-routes at Good, and `Births` grew
monotonically. Now evicted on edge-node departure.
Deliberately NARROWER than "absent from ByScope": a device with no authored tags
under a still-authored node keeps receiving traffic, so its birth is live, not
frozen — evicting it would discard correct state. Pinned both ways.
- I1: a birth whose seq is unreadable produced a permanent debounce-paced rebirth
loop. "A birth arrived" is now tracked separately from "the birth established a
baseline". Writing the test showed the cap belonged wider than the review scoped
it: data-before-birth and unknown-alias loop identically (20 messages → 23 NCMDs
against a cap of 3), so all three missing-metadata reasons share one per-node
consecutive cap, re-armed by any applied birth. A seq gap stays uncapped — it
self-limits.
- I2: the oversize `WarnOnce` key was the raw topic, off a group-wide `#`
subscription. `HandleMessage` now parses and filters to authored nodes BEFORE the
size check (also skipping the protobuf parse for discarded traffic), and `_warned`
carries a hard ceiling so a future bad derivation degrades to silence.
- I3: array metrics slipped the unsupported-type gate — `ToDriverDataType` returns
the ELEMENT type, so a String-typed tag published a packed `byte[]` as base64 at
Good. Gated on `IsSparkplugArray`.
- I4: driver-level Sparkplug coverage for the composition-root lines T22 left
untested — the `OnDataChange` re-raise, `ReadAsync` off the shared cache,
subscribe/unsubscribe dispatch, and both directions of the mode gate.
Minors: M1 the oversize test fed unparseable bytes and passed with the size check
deleted (now a real NBIRTH + an at-the-ceiling control); M2 answered in-place (an
absent seq is refused WITHOUT becoming the baseline, so the NCMD assertion is the
complete check); M3 `HostStateObserved` raise wrapped; M5 the legacy `STATE/{host}`
form is now subscribed, so the parser's tolerance is reachable in production; M8
`ResolveBinding` prefers the NAME over a disagreeing alias; M9 a narrowing float
conversion refuses instead of publishing ±Infinity at Good (a publisher's own
infinity still passes through). M4 was already resolved by T22.
Also fixed a race in the new tests: `publish.Topics.Clear()` after a call that
dispatches rebirths off-thread must drain first — it made the C1 sequence-baseline
pin pass vacuously in the first falsifiability run.
Falsifiability: dropping the reset from AttachTo+EstablishAsync reddens 4 (C1);
dropping the Register eviction reddens 2 (C2); widening the eviction predicate to
the reviewer's literal phrasing reddens the keep-live-births pin (C2b). 545/545 MQTT
unit tests green (was 524); whole-solution `--no-incremental` build clean under TWAE.
Claude-Session: https://claude.ai/code/session_01GASWkNEi68FSCtvr6rLoEW
|
||
|
Joseph Doherty
|
6917d8805d |
feat(mqtt): Sparkplug UntilStable discovery + rediscover-on-DBIRTH
RediscoverPolicy is now mode-dependent: UntilStable in Sparkplug B (a tag's dataType is optional -- the birth certificate declares it, so the discovered tree fills in asynchronously), Once in Plain (nothing about the authored set arrives on the wire). DiscoverAsync resolves each Sparkplug tag's datatype per pass from the live BirthCache, with the ingest path's own precedence: authored dataType wins, else the birth's, else the record default. An unsupported Sparkplug type (DataSet/Template/PropertySet/Unknown) falls back rather than blanking the tag. The authored tag SET is never conditional on a birth -- an authored tag is part of the declared configuration, not something the plant grants by publishing. SparkplugIngestor.BirthObserved is wired to RaiseRediscoveryNeeded behind a two-part change gate, which is the anti-storm mechanism rather than an optimisation: fire only for a scope an authored tag binds, and only when the birth's metric-name SET (ordered-distinct, ordinal) differs from the last one seen for that scope. Edge nodes re-birth freely -- on their own timer, on every reconnect via the late-join rebirth fan-out, and once per rebirth NCMD the gap policy sends -- and firing on each of those would make a healthy plant a permanent address-space-rebuild loop. No extra debounce: what survives the gate is a real edit to the served tree, and delaying it would need its own trailing-edge flush to avoid dropping the last change. The signature map is NOT cleared on death/reconnect/ingest rebuild (the ingestor drops its whole birth cache on reconnect, but "the driver forgot" is not "the address space changed"); it is pruned when a redeploy stops authoring a scope. ScopeHint is the "Mqtt" discovery folder, deliberately not the Sparkplug scope path the plan named: the discovered tree is flat, so an edge-node/device path would name a subtree that does not exist and a consumer scoping a rebuild on it would rebuild nothing. The scope is carried in Reason instead. Falsifiability: always-fire reddens the identical-rebirth + reordered-rebirth pins; always-authored-datatype reddens the birth-fill pin; dropping the authored-scope filter reddens the unauthored-device pin. 524/524 MQTT unit tests green (was 513). Claude-Session: https://claude.ai/code/session_01GASWkNEi68FSCtvr6rLoEW |
||
|
Joseph Doherty
|
8538fb798b |
feat(mqtt): Sparkplug ingest state machine (birth/alias/seq/rebirth/death→stale)
Task 21 — the design doc's §3.6 correctness core. `SparkplugIngestor` owns the
authored `(group, node, device?, metric) → RawPath` index, the live `BirthCache`,
one `SequenceTracker` per authored edge node, and the rebirth policy; it routes
NBIRTH/DBIRTH/NDATA/DDATA/NDEATH/DDEATH/STATE into `OnDataChange` +
`LastValueCache`, or into a bounded, per-node-debounced rebirth NCMD.
Invariants held (and each pinned by a falsifiability control):
- Published reference is the RawPath, never a composed Sparkplug reference —
`DriverHostActor`'s dual-namespace fan-out is RawPath-keyed.
- Tags bind by stable metric NAME; the alias is only the per-birth lookup, so an
alias reused across a rebirth cannot mis-route.
- Every value goes through `SparkplugMetricBinding.Reinterpret` before publish.
- NDEATH never reaches `SequenceTracker.Accept` (it carries no seq); it is tied to
its birth by bdSeq instead.
- An invalid payload mutates nothing — no tracker, no alias table, no eviction.
- `HandleMessage` never throws on MQTTnet's dispatcher thread.
Supporting changes:
- `MqttTagDefinitionFactory.FromSparkplugTagConfig` — the driver had no way to
parse the Sparkplug descriptor keys at all; the P1 stub fields on
`MqttTagDefinition` were never populated. Mode selects the parser, never a
blob heuristic. `dataType` becomes optional in the Sparkplug shape (the birth
declares it), recorded via the new `DataTypeAuthored` flag.
- `MqttConnection` implements `IMqttPublishTransport` (bounded, refusal throws).
- `MqttDriver` dispatches every capability by mode, subscribes `spBv1.0/{group}/#`
(+ the configured STATE topic) once at connect, and re-subscribes + requests a
late-join rebirth on reconnect. `IngestIdentity` now names `MqttSparkplugOptions`,
closing the silent same-ingest gap its own ⚠️ comment warned about.
Primary-host STATE *publishing* is explicitly out of scope and warned about at
construction: it needs the retained-OFFLINE Last Will set at CONNECT time, and
shipping the ONLINE half alone is worse than shipping neither.
58 new tests (MQTT suite 455 → 513, all green).
Claude-Session: https://claude.ai/code/session_01GASWkNEi68FSCtvr6rLoEW
|
||
|
Joseph Doherty
|
31a98a1bf5 |
feat(mqtt): RebirthRequester NCMD encode
Claude-Session: https://claude.ai/code/session_01GASWkNEi68FSCtvr6rLoEW |
||
|
Joseph Doherty
|
3fdf24659f |
fix(mqtt): AcceptBirth is NBIRTH-only -- a DBIRTH routed there wipes bdSeq
Self-review of the commit before this one caught a defect in the contract it published rather than in the code it ran. AcceptBirth's doc said it records "a birth (NBIRTH/DBIRTH)". That is wrong on both halves of this type, and the wrong version plants exactly the defect the bdSeq tie exists to prevent. Only the NBIRTH restarts the Sparkplug sequence; a DBIRTH carries the next number in the edge node's ongoing stream, so it belongs to Accept. And only the NBIRTH and NDEATH carry bdSeq at all -- a DBIRTH has none to pass. So a Task 21 that followed the old doc and routed a DBIRTH to AcceptBirth would do two wrong things at once: rebase the sequence mid-stream, and wipe the node's session token to null. A stale Last Will arriving afterwards would then find nothing to compare against, fall through the deliberate fail-toward-stale rule in IsDeathForCurrentSession, and kill the live node. Renamed AcceptBirth -> AcceptNodeBirth so the DBIRTH call site reads wrong at a glance rather than only in prose, documented the hazard on both methods, and added DeviceBirth_ContinuesTheNodeSequence_AndLeavesTheSessionTokenAlone to pin the shape at this type's own boundary -- the ingest state machine now has something to fail against if it wires the call sites the other way round. Falsifiability: mutating Accept to clear _birthBdSeq reddens exactly the new test (1 RED), reverted. 29 tests, 438/438 for the MQTT suite. Claude-Session: https://claude.ai/code/session_01GASWkNEi68FSCtvr6rLoEW |
||
|
Joseph Doherty
|
e4e313cf1c |
feat(mqtt): SequenceTracker seq-gap + bdSeq death-tie
SequenceTracker (Driver.Mqtt/Sparkplug/) holds one edge node's Sparkplug stream-continuity state -- design doc SS3.6 invariant #3. Two independent halves: the wrapping 0-255 seq counter that reports whether a message was missed, and the bdSeq session token that ties an NDEATH to the NBIRTH it belongs to. Next-expected is (last + 1) & 0xFF, so 255 -> 0 is the sequence continuing. The boundary is the whole task because it fails differently in each direction: read the wrap as a gap and every edge node is asked to rebirth once per 256 messages, so a busy node spends its life republishing births instead of data; miss a real gap and the driver serves values it knows are behind the device, at Good quality, indefinitely. Both directions are asserted, plus two full wrap cycles so an off-by-one that only misfires at one value has nowhere to hide. A gap is reported once and the tracker then resynchronizes onto the observed value -- holding the baseline at the pre-gap number would make every following message report a gap too, so one lost message would become a permanent rebirth storm, the same pathology as a wrong wrap reached by a different route. An out-of-range seq is refused, never masked. The codec hands seq over as a ulong precisely so a spec-violating publisher's value is not truncated into a plausible-looking one, and masking would finish the job it declined to do: with the baseline at 255, a seq of 256 masks to 0 -- exactly what the wrap expects next -- and the bogus message would be accepted as contiguous. Refused values do not become the baseline either, so the next genuine message is still measured against the last credible one. A birth restarts the sequence via AcceptBirth rather than Accept, so it is never itself a gap; a gap after a birth is still detected. An in-range but non-zero birth seq is adopted rather than refused -- refusing it would demand a fresh rebirth for every message from an otherwise-followable publisher. The first message on a virgin tracker cannot be a gap (nothing to measure it against) but leaves IsBirthSynchronized false, which is how Task 21 tells a mid-stream late join from a synchronized node. bdSeq exists to stop a late Last Will from killing a live node: a node's connection drops, it reconnects and publishes a fresh NBIRTH, and only then does the broker notice the old connection died and deliver its will carrying the PREVIOUS session's bdSeq. Without the compare that stale NDEATH drives every tag of a freshly-born healthy node to Bad. Unknowns fail toward stale deliberately -- only a positive mismatch of two known tokens discards a death, because acting on a death wrongly self-corrects at the next birth (invariant #4) while ignoring a real one leaves a dead node's values flowing Good forever. bdSeq is not a top-level field, so TryReadBdSeq extracts it from the metric named bdSeq and runs it through ReinterpretSigned first: read raw, a negative Int64 bdSeq becomes an enormous ulong -- a plausible-looking session token minted out of nonsense. Falsifiability verified, each mutation reverted after observing RED: (a) no-wrap expected -> 2 RED; (b) Accept always true -> 5 RED; (c) bdSeq compare always matches -> 2 RED; (d) mask out-of-range seq into range -> 2 RED. 28 tests, 437/437 for the MQTT suite. Claude-Session: https://claude.ai/code/session_01GASWkNEi68FSCtvr6rLoEW |
||
|
Joseph Doherty
|
2afef00eaa |
feat(mqtt): AliasTable/BirthCache — bind-by-name, rebuild-per-birth
Design §3.6 invariants #1/#2. A Sparkplug alias is scoped to ONE birth: after a rebirth an edge node may point alias 5 at a different metric. So authored tags bind by the stable metric NAME and the alias is a per-birth cache only — RebuildFromBirth builds both indexes fresh and publishes them in one assignment, never merging or upserting, including for an empty birth (which legitimately clears the scope). BirthCache keys scopes by the full (group, edgeNode, device?) triple and applies the spec's node→device rules: an NBIRTH invalidates every DBIRTH under that node (returning their metric sets for the STALE fan-out), a death evicts rather than blanks so data-before-rebirth stays detectable. Reads are lock-free on MQTTnet's dispatcher thread — an immutable snapshot swapped atomically per scope, the same discipline MqttSubscriptionManager.AuthoredTable uses, with rebuilds landing in place so a held table reference always observes the newest birth. Running values are deliberately NOT stored here — LastValueCache owns those, keyed by RawPath. SparkplugMetricBinding.Reinterpret exposes the birth datatype to the codec's two's-complement fix, since a DATA metric carries no datatype. Falsifiability: mutating RebuildFromBirth to merge reddened 5/19 tests (both plan invariants); making the alias carry metric identity across a birth reddened 3/19, including the headline alias-reuse case. Claude-Session: https://claude.ai/code/session_01GASWkNEi68FSCtvr6rLoEW |
||
|
Joseph Doherty
|
b245f380b1 |
feat(mqtt): Sparkplug topic parse/format + datatype map
SparkplugTopic (Driver.Mqtt/Sparkplug/) parses/formats spBv1.0 topics for
every message type (NBIRTH/DBIRTH/NDATA/DDATA/NDEATH/DDEATH/NCMD/DCMD/STATE).
TryParse never throws -- it is fed every topic a live spBv1.0/{group}/#
subscription delivers -- and rejects device/node-scope mismatches and MQTT
wildcard chars in a segment. STATE is handled honestly rather than force-fit
into the {group}/{type}/{node} mould: it parses both the v3.0
spBv1.0/STATE/{hostId} form and the legacy no-prefix STATE/{hostId} form
(tolerated on receive only -- Format/FormatState always emit v3.0). Format
gives Task 20's NCMD/DCMD write path a builder instead of hand-concatenation.
SparkplugDataType is a `global using` alias for the vendored proto's
generated Org.Eclipse.Tahu.Protobuf.DataType, not a second hand-duplicated
enum -- Metric.Datatype is a raw wire uint32 (no enum-typed field forces a
second CLR type to exist), and SparkplugCodec (Task 16, landed concurrently)
already casts straight to the generated type. A duplicate enum would be the
same enum-drift hazard this repo already names systemic (CLAUDE.md's driver
enum-serialization bug) and would force every downstream task to cast
between two value-compatible-but-nominally-different enums. ToDriverDataType()
maps per design doc SS3.5: Int8/UInt8 widen to Int16/UInt16 (no 8-bit
DriverDataType member), Float/Double to Float32/Float64 (there is no
DriverDataType.Double), Text/UUID/Bytes/File to String, *Array variants to
their element type (IsSparkplugArray carries the array bit separately), and
DataSet/Template/PropertySet/PropertySetList/Unknown return null -- an
explicit "unsupported, caller must skip+warn" rather than a guessed String.
The completeness test enumerates the live generated DataType member set
(via the alias) and asserts every member is mapped or on the explicit
unsupported list, so a future Tahu proto change is caught automatically
instead of silently falling through a stale duplicate enum's default.
Falsifiability verified by hand for three defect shapes (each reverted after
observing RED): wrong Int8 widening, a dropped mapping falling through
undetected, and inverted node/device topic scoping.
Claude-Session: https://claude.ai/code/session_01GASWkNEi68FSCtvr6rLoEW
|
||
|
Joseph Doherty
|
2589774480 |
feat(mqtt): SparkplugCodec decode + golden payload vectors
Decodes Sparkplug-B wire bytes into a driver-side projection (SparkplugPayload /
SparkplugMetric / SparkplugValueKind) for the Task 21 ingest state machine.
- Never throws, for ANY input. It sits on MQTTnet's shared dispatcher thread, so
an escaping exception would stall delivery for every subscription on the
connection, not one tag. Garbage, truncation, zero-length, a valid protobuf of
another schema and an over-nested Template all resolve to a verdict.
- Zero-length input is INVALID, not "a payload with no metrics" — protobuf would
parse it as all-defaults, and that is how a truncated-to-nothing body gets
mistaken for a well-formed one.
- Explicit presence throughout (Has{Seq,Name,Alias,Datatype,Timestamp}), never a
zero-check: an NBIRTH legitimately carries seq = 0, and every DATA metric after
a birth carries an alias with no name and no datatype.
- Values are projected RAW, boxed, with the value oneof reported as an explicit
ValueKind — Absent / Null / Scalar / Unsupported all mean different things and
three of them carry a null value. DataSet/Template/extension decode as
Unsupported (v1 scope) rather than throwing or silently vanishing.
- ReinterpretSigned() undoes Sparkplug's two's-complement-in-an-unsigned-field
encoding of Int8/16/32/64. Kept out of decode because a DATA metric carries no
datatype — only the consumer, holding the birth's alias table, knows which
applies. Skipping it publishes 4294967254 for a tag whose value is -42.
- Datatype is carried as the generated Org.Eclipse.Tahu.Protobuf.DataType; the
SparkplugDataType map is Task 17's and is applied downstream (Task 21).
Golden vectors: nbirth.bin (seq=0, named/aliased catalog, a negative Int32, an
is_null metric, a DataSet metric) + ndata.bin (alias-only metrics, no name, no
datatype) are hand-built by SparkplugGoldenPayloads, committed, and pinned by a
drift guard that rebuilds and byte-compares them on every run — plus a test that
they actually reach the output directory, since a missing copy item is invisible
in source.
Claude-Session: https://claude.ai/code/session_01GASWkNEi68FSCtvr6rLoEW
|
||
|
Joseph Doherty
|
1ae26675ac |
feat(mqtt): vendor Tahu sparkplug_b.proto + Grpc.Tools codegen
Vendors the NORMATIVE (proto2) Eclipse Tahu sparkplug_b.proto and compiles it
in .Contracts with build-time Grpc.Tools, message-only (GrpcServices="None") --
Sparkplug rides MQTT, there is no gRPC service, so no Grpc.Core.Api is pulled in.
Provenance rides in the file header: repo, path, pinned commit
5736e404889d4b95910613040a99ba79589ffb13, permalink, git blob SHA-1
bf72ab5f..., content SHA-256 4432c5c4..., EPL-2.0, and a re-verify recipe. The
body below line 38 is byte-for-byte upstream; the blob SHA-1 matches the tree
entry at that commit, so the copy is provably genuine and not reconstructed.
Chose the proto2 file over upstream's sibling sparkplug_b_c_sharp.proto: the
sibling is a lossy proto3 restatement that drops explicit presence, and protoc
generates valid C# from proto2 into the identical namespace
(Org.Eclipse.Tahu.Protobuf, PascalCased from the package -- no
csharp_namespace option). Presence matters downstream: an NBIRTH legitimately
carries seq = 0 and a DATA metric legitimately omits `name`, so Has{Seq,Name,
Alias,IsNull,Datatype} is the difference between "absent" and "present, zero".
.Contracts stays transport-free: Google.Protobuf is a serialization dependency
with a framework-only graph, Grpc.Tools is PrivateAssets=all, and the resolved
graph is exactly {Google.Protobuf, Grpc.Tools, Core.Abstractions} -- no MQTTnet.
Codegen tests pin the round-trip, the namespace, the Sparkplug field numbers,
the DataType spec indices, and protoc's enum-name mangling (UInt64 -> Uint64,
UUID -> Uuid) that Task 17's datatype map has to spell correctly.
Claude-Session: https://claude.ai/code/session_01GASWkNEi68FSCtvr6rLoEW
|
||
|
Joseph Doherty
|
2409d05a28 |
fix(mqtt): a broker-refused CONNECT no longer reports a Healthy driver
MQTTnet 5 does not throw on an unsuccessful CONNACK — it returns the reason in MqttClientConnectResult.ResultCode and v5 removed v4's ThrowOnNonSuccessfulConnectResponse, so ConnectCoreAsync's unconditional SetState(Connected) sealed a wrong-password deployment green: DriverState.Healthy / HostState.Running on a session that never authenticated. Caught by the Task-13 Mosquitto fixture; 240 offline tests missed it. ConnectCoreAsync now inspects the CONNACK and raises MqttConnectRejectedException. Credentials/identity/protocol/Last-Will refusals are unrecoverable and set Faulted, which stops the reconnect supervisor; transient refusals (ServerUnavailable, ServerBusy, QuotaExceeded, UseAnotherServer, ConnectionRateExceeded, and anything unrecognised) stay Reconnecting under backoff. MqttDriverProbe now shares the rejection wording, so the AdminUI Test-connect button and the running driver can no longer disagree. Claude-Session: https://claude.ai/code/session_01GASWkNEi68FSCtvr6rLoEW |
||
|
Joseph Doherty
|
a13ae926a8 |
fix(mqtt): gate DisposeAsync against in-flight lifecycle ops (C1) + rebuild-branch tests (I1)
C1 (Critical) — DisposeAsync went straight to TeardownAsync with no lifecycle-gate
wait, reopening the orphaned-connection class. Interleaving: ReinitializeAsync's
rebuild branch holds the gate mid-InitializeCoreAsync, past its own teardown but
before `_connection = connection`; an ungated dispose sees a null connection, does
nothing, and sets `_disposed`; the rebuild then publishes a live connection plus a
host-probe loop that every later dispose short-circuits past. Not proven reachable
today (DriverInstanceActor.PostStop calls the gated ShutdownAsync), but MqttDriver
publicly implements IAsyncDisposable, which invites `await using`.
- DisposeAsync now takes the gate with the same bounded wait + fallback teardown
ShutdownAsync uses, and sets `_disposed` BEFORE the wait.
- InitializeCoreAsync re-checks `_disposed` after the connect and disposes the
connection it just built rather than publishing it — this closes the residual
window on the bounded-timeout fallback path.
- The doc comment no longer claims parity with ShutdownAsync it did not have, and
stops conflating "don't Dispose() the semaphore object" with "don't WaitAsync".
I1 (Important) — the SameSession == false rebuild branch had no test driving it.
Adds three: an endpoint-changing delta rebuilds and Faults on a refused connect;
an ingest-only delta (MaxPayloadBytes) ALSO rebuilds, pinning that IngestIdentity
is nested inside SessionIdentity; and DisposeAsync serializes behind a lifecycle
operation parked at a new internal BeforeConnectHookForTests seam (mirroring
MqttConnection's AfterConnectHookForTests, which exists for the same race class).
Minors: comments recording that IngestIdentity names no Sparkplug field and must
grow one in P2 (tasks 21/22), and why _options/_subscriptions/_authoredRawPaths
get looser memory discipline than _health/_hostState.
Falsifiability: reverting DisposeAsync to the ungated form reddens exactly the new
serialization test ("Shouldly.ShouldAssertException : raced"); forcing SameSession
to always-true reddens exactly the two new rebuild tests. 240/240 MQTT tests pass;
forced rebuild of the driver project is 0 warnings under TreatWarningsAsErrors.
Claude-Session: https://claude.ai/code/session_01GASWkNEi68FSCtvr6rLoEW
|
||
|
Joseph Doherty
|
f79d13e2d8 |
feat(mqtt): factory + DriverTypeNames.Mqtt + host factory/probe registration
Task 9. Adds MqttDriverFactoryExtensions (DriverTypeName = DriverTypeNames.Mqtt, direct deserialization of MqttDriverOptions — no intermediate DTO, mirroring OpcUaClientDriverFactoryExtensions), the DriverTypeNames.Mqtt constant, and both host wiring sites: the factory in DriverFactoryBootstrap.Register and the probe in AddOtOpcUaDriverProbes (the admin-node path Program.cs calls in its hasAdmin block — a probe wired only on driver nodes makes Test-connect silently dead). Carried-forward items: - Converges every MQTT config-parsing seam onto ONE JsonSerializerOptions — MqttJson.Options, in .Contracts alongside MqttDriverOptions. It replaces the probe's internal JsonOpts and the browser's separate private copy; the factory, the probe, MqttDriver.ParseOptions and MqttDriverBrowser now all parse through it. .Contracts is the only assembly all four consumers reference, and the browser's reference to the runtime .Driver project is a documented layering exception scheduled for removal — anchoring the options there would resurrect the duplicate the day it goes away. - Replaces the "Mqtt" literals in MqttDriverProbe, MqttDriverBrowser and MqttDriver with the constant (string value unchanged). - Tightens MqttDriverProbeTests.ProbeAsync_EnumAsName: it asserted only Ok == false + non-empty message, which is also exactly what a JSON-parse failure produces — so it stayed green under the very regression it names. It now asserts the probe got past the parse and reached the network. Falsifiability: deleting JsonStringEnumConverter from MqttJson.Options reddens 9 tests across 4 suites, including the tightened probe test (message becomes "Config JSON is invalid: The JSON value could not be converted to MqttProtocolVersion") — which the pre-fix assertions would have passed. Also references the MQTT driver from Core.Abstractions.Tests so DriverTypeNamesGuardTests' reflective bin scan discovers the new factory and the constant/factory parity check stays honest. Claude-Session: https://claude.ai/code/session_01GASWkNEi68FSCtvr6rLoEW |
||
|
Joseph Doherty
|
420692b6e5 |
feat(mqtt): MqttDriver shell — lifecycle + authored-only discovery (Once)
Composes MqttConnection + MqttSubscriptionManager + LastValueCache into the IDriver / ITagDiscovery / ISubscribable / IReadable / IHostConnectivityProbe / IRediscoverable capability set. - Discovery replays ONLY the authored raw tags (RediscoverPolicy = Once, SupportsOnlineDiscovery = false) — a chatty broker can never auto-provision. - Composition order is register -> AttachTo -> ConnectAsync; the manager's Reconnected handler is passed through unwrapped so its throw-on-total-failure still tears a deaf session down. - ReinitializeAsync applies a tag-only delta in place and never faults on a bad one; a session-changing delta rebuilds. - ReadAsync serves the last-value cache and degrades per reference (BadWaitingForInitialData), never throwing for the batch. - FlushOptionalCachesAsync is a no-op: the last-value cache IS IReadable. - Adds MqttDriverOptions.RawTags (the authored-tag delivery mechanism every other driver's options DTO already has) and promotes MaxPayloadBytes from a manager ctor knob to an operator-facing key. - Converges on MqttDriverProbe.JsonOpts rather than a second, divergent JsonSerializerOptions. Claude-Session: https://claude.ai/code/session_01GASWkNEi68FSCtvr6rLoEW |