diff --git a/docs/drivers/Mqtt.md b/docs/drivers/Mqtt.md index 6d1b921e..c01aeb83 100644 --- a/docs/drivers/Mqtt.md +++ b/docs/drivers/Mqtt.md @@ -2,7 +2,8 @@ In-process MQTT **broker-subscribe** driver. It holds one MQTTnet 5 client against one broker, subscribes to the topics its authored tags name, and forwards each PUBLISH straight to -`ISubscribable.OnDataChange`. Ingest is **push, not poll**, and **one-way** — there is no `IWritable`. +`ISubscribable.OnDataChange`. Ingest is **push, not poll**; writes go back out as ordinary publishes +(see [Write-through](#write-through)). **Two ingest shapes, selected by the driver's `Mode`:** `Plain` binds a tag to a concrete MQTT topic; `SparkplugB` binds it to a `group / edge node / device? / metric` tuple and decodes Eclipse-Tahu @@ -12,18 +13,18 @@ See [Mqtt-Test-Fixture.md](Mqtt-Test-Fixture.md) for the harness and [`../plans/2026-07-15-mqtt-sparkplug-driver-design.md`](../plans/2026-07-15-mqtt-sparkplug-driver-design.md) for the design. -> **Status.** P1 (plain MQTT) and P2 (Sparkplug B ingest) are both shipped and live-gated against the -> Mosquitto + Sparkplug-simulator fixture. Write-through (`IWritable` — NCMD/DCMD/plain publish) is -> **deferred** ([#508](https://gitea.dohertylan.com/dohertj2/lmxopcua/issues/508)); every MQTT node -> materializes read-only. See [Known gaps](#known-gaps) — every entry there carries a tracking issue -> (#507–#515). +> **Status.** P1 (plain MQTT), P2 (Sparkplug B ingest), write-through +> ([#508](https://gitea.dohertylan.com/dohertj2/lmxopcua/issues/508)) and the Sparkplug primary-host +> claim ([#511](https://gitea.dohertylan.com/dohertj2/lmxopcua/issues/511)) are all shipped and +> live-gated against the Mosquitto + Sparkplug-simulator fixture. See +> [Known gaps](#known-gaps) — every entry there carries a tracking issue. ## Project Layout | Project | Holds | |---|---| | `Driver.Mqtt.Contracts` | Config records, enums, `MqttTagDefinition` + the pure `MqttTagDefinitionFactory`, and the one shared `MqttJson.Options`. **Deliberately transport-free** — no MQTTnet reference | -| `Driver.Mqtt` | Runtime: `MqttDriver`, `MqttConnection` (connect + reconnect supervisor), `MqttSubscriptionManager`, `LastValueCache`, factory registration, `MqttDriverProbe` | +| `Driver.Mqtt` | Runtime: `MqttDriver`, `MqttConnection` (connect + reconnect supervisor), `MqttSubscriptionManager`, `LastValueCache`, `SparkplugIngestor`, `SparkplugPrimaryHost`, factory registration, `MqttDriverProbe` | | `Driver.Mqtt.Transport` | `MqttClientOptionsFactory` — TLS posture, PEM CA-pin chain validator, credentials, protocol mapping. Connection **policy** only; nothing here connects | | `Driver.Mqtt.Browser` | `MqttDriverBrowser` + `MqttBrowseSession` — the AdminUI address picker's `#`-observation session | @@ -113,8 +114,8 @@ driver are pure organisational grouping in the RawPath. "Mode": "SparkplugB", "Sparkplug": { "GroupId": "OtOpcUaSim", // REQUIRED — the driver's whole subscription scope - "HostId": "", // Host Application identity; receive-only (see below) - "ActAsPrimaryHost": false, // NOT IMPLEMENTED — logs a warning when true + "HostId": "", // Host Application identity — spBv1.0/STATE/{HostId} + "ActAsPrimaryHost": false, // claim the primary host id (Primary-gated; see below) "RequestRebirthOnGap": true, // a seq gap asks the edge node to re-announce (NCMD) "BirthObservationWindowSeconds": 15 // how long discovery collects births before calling it stable } @@ -124,8 +125,8 @@ driver are pure organisational grouping in the RawPath. | Key | Default | Notes | |---|---|---| | `GroupId` | `""` | **Required.** The driver subscribes exactly `spBv1.0/{GroupId}/#`. Blank ⇒ it connects, reports `Healthy`, and ingests **nothing**. It is a literal topic segment, so `/`, `+`, `#` are refused by the form | -| `HostId` | `""` | Sparkplug Host Application identity. **Receive-only** — the driver observes `STATE` and never publishes one | -| `ActAsPrimaryHost` | `false` | **Not implemented.** Setting it logs a startup **warning** rather than being silently inert — being a primary host is a three-part contract (retained `ONLINE`, a matching `OFFLINE` registered as the MQTT Will *at connect time*, an explicit `OFFLINE` on clean shutdown), and shipping the `ONLINE` half without the Will is strictly worse than shipping neither: a crashed node would leave a retained `ONLINE` asserting forever that a dead host is alive | +| `HostId` | `""` | Sparkplug Host Application identity — the `spBv1.0/STATE/{HostId}` topic. Optional for observation; **required** to claim. Set without `ActAsPrimaryHost`, the driver only observes | +| `ActAsPrimaryHost` | `false` | Claim the Sparkplug Primary Host Application identity. **Primary-gated** — only the node holding the redundancy Primary role claims it. See [Primary host](#primary-host-actasprimaryhost) | | `RequestRebirthOnGap` | `true` | On a detected sequence-number gap, publish a rebirth-request NCMD instead of running on stale metric state | | `BirthObservationWindowSeconds` | `15` | Discovery's birth-collection window. Must be ≥ 1 | @@ -203,7 +204,7 @@ survive a reconnect; a message fans out to *every* matching tag, and unauthored | `groupId` | string | — | **Required.** Must match the driver's `Sparkplug.GroupId` to ever receive a message | | `edgeNodeId` | string | — | **Required.** | | `deviceId` | string | *absent* | **Omit** for a metric published by the edge node itself (NBIRTH/NDATA). Absent is meaningfully different from `""` | -| `metricName` | string | — | **Required.** The tag's stable binding key across rebirths. **May contain `/`** — `Node Control/Rebirth` and `Properties/Serial Number` are canonical Sparkplug names, and the whole name is one value, never split | +| `metricName` | string | — | **Required.** The tag's stable binding key across rebirths. **May contain `/`** — `Node Control/Rebirth` and `Properties/Serial Number` are canonical Sparkplug names, and the whole name is one value, never split. Also how a flattened structured member is addressed: `Motor/Speed`, `Recipe/Target/0` (see [Structured metrics](#structured-metrics-template--dataset--propertyset)) | | `dataType` | `DriverDataType` | *from the birth* | **Optional override.** Absent ⇒ take whatever type the birth certificate declared. The AdminUI's "(from birth certificate)" option removes the key entirely | | `isHistorized` / `historianTagname` | — | — | Server-side historization, unchanged from Plain | @@ -221,6 +222,11 @@ bind. `metricName` is deliberately **exempt**: it is not a topic segment. // A node-level metric — note deviceId is ABSENT, not blank. {"groupId":"OtOpcUaSim","edgeNodeId":"EdgeA","metricName":"Temperature"} + +// A flattened Template member, and a flattened DataSet cell. `Motor` and `Recipe` are what the wire +// carries; nothing on the wire is ever called by these names. +{"groupId":"OtOpcUaSim","edgeNodeId":"EdgeA","deviceId":"Filler1","metricName":"Motor/Stator/Temp"} +{"groupId":"OtOpcUaSim","edgeNodeId":"EdgeA","deviceId":"Filler1","metricName":"Recipe/Target/1"} ``` ## Sparkplug B Ingest @@ -239,9 +245,11 @@ One subscription (`spBv1.0/{GroupId}/#`) feeds a per-edge-node state machine - **Death → STALE.** An NDEATH whose `bdSeq` matches the live session (the bdSeq tie is what stops a stale death from killing a fresh session) marks every metric under that edge node `Bad` / `BadNoCommunication`. A DDEATH does the same for one device. The next birth restores them. -- **Unsupported datatypes are skipped with a warning, never guessed**: `DataSet`, `Template`, - `PropertySet`, `PropertySetList`, `Unknown`. `Int8`→`Int16` and `UInt8`→`UInt16` widen; `*Array` - variants map to the element type with the array bit set. +- **Structured metrics flatten to sibling tags** — `Template` and `DataSet`; see + [Structured metrics](#structured-metrics-template--dataset--propertyset). +- **Unsupported datatypes are skipped with a warning, never guessed**: `PropertySetList`, `Unknown`, + extension values, and any `Template`/`DataSet` that cannot be flattened honestly. `Int8`→`Int16` and + `UInt8`→`UInt16` widen; `*Array` variants map to the element type with the array bit set. - **Ingest state is reset at every session/authoring seam** — a reconnect or a `ReinitializeAsync` must not carry a previous session's alias table into a new one. @@ -252,12 +260,127 @@ One subscription (`spBv1.0/{GroupId}/#`) feeds a per-edge-node state machine anti-storm, not optimisations: rediscovery triggers an address-space rebuild, and an edge node re-births freely — on its own schedule, on every reconnect, and once per rebirth NCMD. -> ⚠️ **Nothing consumes the signal today.** No runtime component subscribes to -> `IRediscoverable.OnRediscoveryNeeded`, and `DriverHostActor.HandleDiscoveredNodes` hard-returns — -> discovered-node injection is dormant in v3. The served address space comes from the deploy artifact, -> so **a DBIRTH introducing a new metric does not change the OPC UA tree**; you must redeploy. The -> driver's decision is observable only as an Information log line -> (`… requesting rediscovery.`). This is a v3 platform gap, not an MQTT one. +> ⚠️ **The signal is an operator prompt, not a rebuild.** `DriverInstanceActor` subscribes and the +> raise rides the driver-health snapshot to the AdminUI `/hosts` page as a **re-browse chip** — but the +> served address space still comes from the deploy artifact, so **a DBIRTH introducing a new metric does +> not change the OPC UA tree**. Re-browse the device and commit. That is deliberate: v3 authors raw tags +> through the `/raw` browse-commit flow, and grafting nodes at runtime would materialise ones nobody +> approved and no deployment artifact records. + +## Structured metrics (`Template` / `DataSet` / `PropertySet`) + +Sparkplug metrics are not all scalars. `SparkplugFlattener` +([#515](https://gitea.dohertylan.com/dohertj2/lmxopcua/issues/515)) turns the structured ones into +addressable tags: + +| Metric kind | Mapping | Example | +|---|---|---| +| `Template` (UDT instance) | members become **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` (a metric's `properties`) | node **metadata**, never tags | `engUnit`, `Range/Low` | + +Flattened members are authored exactly like any other Sparkplug metric — the composed name goes in +`metricName`. The browse picker offers them directly (the structured **parent** is deliberately *not* +offered: it has no value and no servable datatype, so a tag committed against it could never read). + +**Why flattening rather than OPC UA Structures.** Owner decision, 2026-07-31. Structures are truer to +Sparkplug's model but much harder to consume — flattened scalars are readable by any OPC UA client +with no type dictionary, and the result mirrors how the `/raw` tree already works. + +### Things worth knowing before you touch this + +- **The row index is always present, including on row 0.** `Recipe/Target/0`, never `Recipe/Target`. + 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 — so 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. Flattening at decode time alone is not enough; the + expansion happens in `SparkplugIngestor.PublishStructuredMembers`, after resolution. +- **Member datatype falls back to the birth's.** A DDATA template may omit its members' datatypes. + Guessing from the CLR type of the wire value would read a signed `Int32` of −42 out of its unsigned + field as 4294967254 — plausible, `Good`, and wrong. +- **Skip-and-warn survives, and refusal is whole.** A template **definition** (`is_definition`), an + unnamed member, an extension value, a column/type count mismatch, a short row, or a payload past + `MaxDepth` (5) / `MaxMembers` (512) refuses the **entire** metric rather than publishing part of it. + A silently half-flattened UDT looks exactly like a complete one in the picker. +- **Member paths contain `/`**, which `RawBrowseCommitMapper.ToRawSegment` + ([#509](https://gitea.dohertylan.com/dohertj2/lmxopcua/issues/509)) already handles: the slashed name + travels intact as `metricName` while the derived tag **Name** is coerced to a legal RawPath segment, + with a collision guard for two members that would sanitise onto one name. +- **The rediscovery signature tracks members, not the parent.** A UDT gaining or losing a member leaves + the parent metric's name identical, so a gate built on raw wire names would never move. + +`PropertySet` is projected as metadata rather than as sibling tags because it *describes* a metric +(units, ranges, quality hints) — surfacing it as tags would invent plant signals out of documentation. +`PropertySetList` and property extension values are dropped rather than stringified. + +## Primary host (`actAsPrimaryHost`) + +A Sparkplug **Primary Host Application** is the thing edge nodes watch to decide whether anyone is +listening — many stop publishing while their host is offline. Claiming it is a three-part contract, +and this driver ships all three or none of it +([#511](https://gitea.dohertylan.com/dohertj2/lmxopcua/issues/511)): + +1. a **retained** `{"online":true,"timestamp":…}` on `spBv1.0/STATE/{hostId}`, QoS 1, published after CONNACK; +2. a matching **retained `OFFLINE` armed as the MQTT Last Will**, so a crash still reports the truth; +3. an explicit `OFFLINE` on a clean stand-down, so edge nodes do not wait out a keep-alive timeout. + +Set `Sparkplug.HostId` and `Sparkplug.ActAsPrimaryHost: true`. A blank host id is **refused** with a +warning — the driver does not publish to `spBv1.0/STATE/`, which is a legal topic reaching nobody. + +### The claim is a whole 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 the claim gets its **own** broker connection (client id `{clientId}-spbhost`, forced +clean-session) whose lifetime *is* the claim: opened on gaining Primary, closed on losing it. Every leg +then follows from the connection's lifecycle rather than from bookkeeping that could drift. + +The ingest session is untouched by a role change — no dropped subscriptions, no lost birth cache, and +no data gap at the Secondary→Primary transition, which is the moment a plant most needs values flowing. + +### Primary-gated — and why the obvious alternatives are wrong + +Both nodes of a redundant pair subscribe independently (MQTT fan-out is per-connection, and the Primary +gate applies downstream of ingest), so an ungated claim means both claim the same host id. The sharp +failure is not the two fighting over the retained `STATE`. It is that the **Secondary's** death fires +*its* Will, publishing `OFFLINE` while the Primary is perfectly healthy — every edge node in the group +stops publishing, and redundancy has caused the outage the mechanism exists to prevent. + +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, and a crash would leave a +retained `ONLINE` asserting forever that a dead host is alive. + +Hence: bind the claim to a connection, and rebuild the connection on role change. + +| Node state | STATE connection | Retained `STATE` | +|---|---|---| +| Primary | open, Will armed | `ONLINE` | +| Secondary / Detached | **never opened** | — (whatever the Primary published) | +| Role unknown, driver peer present | not opened (fail closed) | — | +| Role unknown, no driver peer (single node) | open | `ONLINE` | + +The verdict comes from `PrimaryGatePolicy.ShouldServiceAsPrimary` via `IDriverPrimaryRoleSource` +(`Core.Abstractions`), published by `DriverHostActor` on every redundancy snapshot and **sampled on a +1-second timer** rather than evented — a process-singleton raising events at per-instance drivers +outlives every driver it holds a reference to. + +> **Accepted trade: a brief edge-visible `OFFLINE` during failover**, while the old Primary stands down +> and the new one connects and republishes. That is a genuine state transition — for that window there +> really is no host — not a glitch worth hiding. + +### Operational notes + +- **At most one primary host per host id per broker.** Two separate OtOpcUa deployments pointing at one + broker with the same `hostId` will fight, and nothing in either can detect the other. The gate covers + a redundant *pair*, not two independent deployments. +- **A reconnect republishes the birth.** A broker restart drops retained messages; without the + republish the host would be connected and believed offline by everyone, silently. +- **A failed claim never faults the driver.** A broker whose ACL denies `spBv1.0/STATE/#` costs a + warning and a plant-visible missing host, not every tag's value. Ingest is the data plane; the claim + is an assertion to other applications. +- **The driver still observes its own `STATE`.** The ingestor subscribes the configured host id + regardless, so a claiming node sees its own retained `ONLINE` come back. Nothing gates on it, so the + echo costs one log line. ## Connection + Failure Semantics @@ -446,9 +569,14 @@ identical words. See [TestConnectProbes.md](TestConnectProbes.md). reconnect/rebuild branches, CONNACK classification, and the Sparkplug half — golden-payload decode, topic parse/format, datatype map, alias/birth-cache rebuild, sequence + bdSeq-death ties, the rediscovery change gate, and the browse session's "publishes nothing" assertion. -- **Live (env-gated)** — `tests/Drivers/…Driver.Mqtt.IntegrationTests` (**15 tests**) against the +- **Live (env-gated)** — `tests/Drivers/…Driver.Mqtt.IntegrationTests` (**29 tests**) against the Mosquitto broker + the C# Sparkplug edge-node simulator; skips cleanly when `MQTT_FIXTURE_ENDPOINT` is unset. See [Mqtt-Test-Fixture.md](Mqtt-Test-Fixture.md) and `infra/README.md` §3. +
The primary-host legs (`MqttPrimaryHostLiveTests`) include the one the ordinary lifecycle cannot + reach: `AbandonClaimForTests` 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 explicit stand-down `OFFLINE` and pass against an implementation that never armed a Will + at all. - **AdminUI** — `MqttDriverFormModelTests` / `MqttTagConfigModelTests` cover both editors' round-trip, mode inference and validation. AdminUI has no bUnit, so the razor shells themselves are only ever proven by a live `/run` gate. @@ -466,7 +594,9 @@ identical words. See [TestConnectProbes.md](TestConnectProbes.md). Every row is tracked. None is a silent failure — each either refuses in words or logs. -**Closed 2026-07-31:** write-through ([#508](https://gitea.dohertylan.com/dohertj2/lmxopcua/issues/508), live-gated), the slashed-metric browse-commit +**Closed 2026-07-31:** write-through ([#508](https://gitea.dohertylan.com/dohertj2/lmxopcua/issues/508), live-gated), primary-host STATE publishing +([#511](https://gitea.dohertylan.com/dohertj2/lmxopcua/issues/511), live-gated), structured +`Template`/`DataSet`/`PropertySet` metrics ([#515](https://gitea.dohertylan.com/dohertj2/lmxopcua/issues/515), live-gated), the slashed-metric browse-commit ([#509](https://gitea.dohertylan.com/dohertj2/lmxopcua/issues/509)), the stale rebirth affordance ([#510](https://gitea.dohertylan.com/dohertj2/lmxopcua/issues/510)), the `.Browser` layering exception ([#512](https://gitea.dohertylan.com/dohertj2/lmxopcua/issues/512)), the stray Sparkplug `payloadFormat` ([#513](https://gitea.dohertylan.com/dohertj2/lmxopcua/issues/513)) and rebirth on a not-yet-birthed plant ([#514](https://gitea.dohertylan.com/dohertj2/lmxopcua/issues/514)). @@ -474,5 +604,3 @@ not-yet-birthed plant ([#514](https://gitea.dohertylan.com/dohertj2/lmxopcua/iss | Gap | Issue | Effect | Notes | |---|:---:|---|---| | **Rediscovery is advisory, not a rebuild** | [#507](https://gitea.dohertylan.com/dohertj2/lmxopcua/issues/507) (closed, superseded) | A DBIRTH introducing a metric does not change the served OPC UA tree; an operator must re-browse the device and commit | **No longer "inert".** `DriverInstanceActor` now subscribes to `OnRediscoveryNeeded` and the raise rides the driver-health snapshot to the AdminUI `/hosts` page as a **re-browse chip**. It deliberately does not inject nodes: v3 authors raw tags through the `/raw` browse-commit flow, and grafting nodes at runtime would materialise ones nobody approved and no deployment artifact records. #507's injection path was deleted rather than revived | -| **Primary-host STATE publishing** | [#511](https://gitea.dohertylan.com/dohertj2/lmxopcua/issues/511) | `ActAsPrimaryHost` does nothing | Logs a startup warning rather than being silently inert — see the `Sparkplug` sub-object table | -| **`DataSet` / `Template` / `PropertySet` metrics** | [#515](https://gitea.dohertylan.com/dohertj2/lmxopcua/issues/515) | Skipped with a warning | v1-unsupported; never coerced into a guessed type | diff --git a/src/Core/ZB.MOM.WW.OtOpcUa.Core.Abstractions/IDriverPrimaryRoleSource.cs b/src/Core/ZB.MOM.WW.OtOpcUa.Core.Abstractions/IDriverPrimaryRoleSource.cs new file mode 100644 index 00000000..b2780389 --- /dev/null +++ b/src/Core/ZB.MOM.WW.OtOpcUa.Core.Abstractions/IDriverPrimaryRoleSource.cs @@ -0,0 +1,90 @@ +namespace ZB.MOM.WW.OtOpcUa.Core.Abstractions; + +/// +/// A driver-facing snapshot of "should this node be the one asserting an exclusive identity on a +/// shared external system right now?". +/// +/// +/// +/// Why drivers need this at all. The driver layer has no notion of redundancy, by +/// design — a driver talks to a device and the Primary gate is applied above it, in +/// DriverHostActor, on the way in. That works because every gated operation is +/// inbound: a write, an alarm ack, an alerts publish. A Sparkplug primary-host claim is +/// not inbound. It is a standing assertion the driver makes to the broker, on its own +/// connection, for as long as it holds one — there is no message for the host actor to +/// intercept. So the decision has to reach the driver instead (Gitea #511). +/// +/// +/// The decision, not the role. Same discipline as the alarm-drain view in +/// Runtime: this publishes the verdict a single policy produced, so the policy stays the +/// one place that decides and a driver can never re-derive a subtly different rule from a raw +/// role. The verdict fed here is the data-plane one (an unknown role on a cluster that +/// has a driver peer denies), because two nodes claiming one Sparkplug host id is the harm the +/// gate exists to prevent — not a duplicate row, but a Secondary's Last Will publishing +/// OFFLINE over a live Primary's ONLINE and stopping every edge node in the +/// group. +/// +/// +/// Polled, not evented, and that is deliberate. A process-wide singleton raising an +/// event onto per-instance drivers outlives every driver it hands a reference to — the exact +/// missing--= leak shape the rediscovery signal had to be careful about. Consumers +/// sample this instead, so a driver that is torn down simply stops asking. The cost is up to +/// one poll interval of latency on a failover, which is inside the edge-visible +/// OFFLINE window a failover already implies. +/// +/// +public interface IDriverPrimaryRoleSource +{ + /// + /// Whether this node currently holds the Primary data-plane role — and may therefore claim an + /// exclusive external identity such as a Sparkplug host id. + /// + bool ShouldActAsPrimary { get; } + + /// + /// Records a fresh decision. Called by DriverHostActor on every redundancy snapshot. + /// + /// + /// The verdict PrimaryGatePolicy.ShouldServiceAsPrimary produced. + /// + void PublishPrimaryRole(bool shouldActAsPrimary); +} + +/// +/// The a driver uses when nothing publishes one — a +/// standalone host, a unit test, or any deployment with no redundancy manager wired. +/// +/// +/// +/// It answers , and the alternative is worse. Defaulting closed +/// would be the reflexive "fail-safe" choice, but there is no safety to gain: with nothing +/// publishing a role there is by definition no peer to collide with, and a closed default +/// would make an exclusive-identity claim silently never happen on the single-node deployment +/// that is the common case — an inert feature with no error, which is precisely the failure +/// mode Gitea #511 was filed about. +/// +/// +/// The risk this shifts onto the wiring is real and is guarded there instead: a redundant +/// deployment whose composition root forgets to supply the live source would fall back to this +/// and let both nodes claim. DriverFactoryBootstrapPrimaryRoleSourceTests pins that the +/// Host resolves a real one. +/// +/// +public sealed class UnmanagedDriverPrimaryRoleSource : IDriverPrimaryRoleSource +{ + /// The shared instance — it holds no state. + public static readonly UnmanagedDriverPrimaryRoleSource Instance = new(); + + private UnmanagedDriverPrimaryRoleSource() + { + } + + /// + public bool ShouldActAsPrimary => true; + + /// + /// No-op: there is no redundancy manager to publish anything here. + public void PublishPrimaryRole(bool shouldActAsPrimary) + { + } +} diff --git a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Mqtt.Browser/MqttBrowseSession.cs b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Mqtt.Browser/MqttBrowseSession.cs index cb240b42..2e37d579 100644 --- a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Mqtt.Browser/MqttBrowseSession.cs +++ b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Mqtt.Browser/MqttBrowseSession.cs @@ -585,20 +585,77 @@ internal sealed class MqttBrowseSession : IRebirthCapableBrowseSession continue; } - var metricPath = ownerPath + MetricSeparator + name; - if (metricPath.Length > MaxTopicChars) + // Gitea #515: a Template/DataSet metric contributes its flattened MEMBERS as leaves, never + // itself. The parent has no value and no servable datatype, so a leaf for it would offer + // the operator an address that can only ever commit a tag which never reads. + if (metric.ValueKind == SparkplugValueKind.Structured) { - Volatile.Write(ref _oversized, 1); + foreach (var member in metric.MemberList) + { + var memberName = $"{name}{SparkplugFlattener.MemberSeparator}{member.Path}"; + if (!IsUsableLabel(memberName)) + { + Volatile.Write(ref _oversized, 1); + continue; + } + + if (!AddMetricLeaf(owner, ownerPath, ownerScope, memberName, member.DataType, parsed.Type, alias: null)) + { + return; // node cap tripped + } + } + continue; } - var node = GetOrCreate(owner.Children, metricPath, name, BrowseNodeKind.Leaf, ownerScope); - if (node is null) return; // node cap tripped - - node.RecordMetric(new SparkplugMetricInfo(name, metric.DataType, parsed.Type, metric.Alias)); + if (!AddMetricLeaf(owner, ownerPath, ownerScope, name, metric.DataType, parsed.Type, metric.Alias)) + { + return; // node cap tripped + } } } + /// + /// Places one metric leaf under . + /// + /// The owning edge-node / device folder. + /// That folder's path. + /// The scope stamped on the leaf. + /// + /// The metric's Sparkplug name — for a flattened structured member, the composed + /// {parent}/{memberPath}. It travels intact: it is the binding key, and only the + /// display name is later coerced to a legal RawPath segment (Gitea #509). + /// + /// The declared datatype, or null. + /// The birth message type this leaf was seen in. + /// The metric's alias, or null — flattened members carry none of their own. + /// when the node cap tripped and the caller must stop. + private bool AddMetricLeaf( + TopicNode owner, + string ownerPath, + SparkplugScope ownerScope, + string metricName, + Org.Eclipse.Tahu.Protobuf.DataType? dataType, + SparkplugMessageType messageType, + ulong? alias) + { + var metricPath = ownerPath + MetricSeparator + metricName; + if (metricPath.Length > MaxTopicChars) + { + Volatile.Write(ref _oversized, 1); + return true; + } + + var node = GetOrCreate(owner.Children, metricPath, metricName, BrowseNodeKind.Leaf, ownerScope); + if (node is null) + { + return false; + } + + node.RecordMetric(new SparkplugMetricInfo(metricName, dataType, messageType, alias)); + return true; + } + /// /// Finds or creates one tree node under , honouring the node cap. /// @@ -698,6 +755,40 @@ internal sealed class MqttBrowseSession : IRebirthCapableBrowseSession ObserveBirth(groupId, edgeNodeId, device, metric); } + /// + /// Test seam feeding a birth whose metric is a structured Template (Gitea #515). + /// + /// The Sparkplug group id. + /// The Sparkplug edge-node id. + /// The device id, or for an NBIRTH. + /// The template metric's own name. + /// The template's members, as (name, datatype). + internal void ObserveTemplateBirthForTest( + string groupId, + string edgeNodeId, + string? device, + string metricName, + params (string Name, SparkplugDataType DataType)[] members) + { + var template = new Payload.Types.Template { Version = "1", IsDefinition = false }; + foreach (var (name, dataType) in members) + { + var member = new Payload.Types.Metric { Name = name, Datatype = (uint)dataType }; + + // A value is required: a member carrying only a name and a datatype is indistinguishable + // from a template DEFINITION's member, which the flattener deliberately refuses. + member.DoubleValue = 0; + template.Metrics.Add(member); + } + + ObserveBirth(groupId, edgeNodeId, device, new Payload.Types.Metric + { + Name = metricName, + Datatype = (uint)SparkplugDataType.Template, + TemplateValue = template, + }); + } + /// Test seam feeding a birth certificate that declares no metrics at all. /// The Sparkplug group id. /// The Sparkplug edge-node id. diff --git a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Mqtt.Contracts/MqttDriverOptions.cs b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Mqtt.Contracts/MqttDriverOptions.cs index 8acb9148..c40bae79 100644 --- a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Mqtt.Contracts/MqttDriverOptions.cs +++ b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Mqtt.Contracts/MqttDriverOptions.cs @@ -190,6 +190,12 @@ public sealed record MqttSparkplugOptions /// When true, this driver instance acts as the Sparkplug Primary Host /// Application. At most one primary host may exist per host-id per broker. /// + /// + /// Implemented by SparkplugPrimaryHost (Gitea #511) on a dedicated broker connection + /// whose lifetime is the claim, and gated on the redundancy Primary role so the two nodes + /// of a pair never both claim. Requires ; set without one, the driver warns + /// and declines rather than publishing to a malformed topic. + /// public bool ActAsPrimaryHost { get; init; } = false; /// diff --git a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Mqtt.Contracts/MqttLastWill.cs b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Mqtt.Contracts/MqttLastWill.cs new file mode 100644 index 00000000..a65a905d --- /dev/null +++ b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Mqtt.Contracts/MqttLastWill.cs @@ -0,0 +1,37 @@ +namespace ZB.MOM.WW.OtOpcUa.Driver.Mqtt; + +/// +/// An MQTT Last Will and Testament, as the client options factory wants it (Gitea #511). +/// +/// +/// +/// A Will can only be registered at CONNECT, and that single fact drives the whole design +/// of the Sparkplug primary-host claim. It rides in the CONNECT packet, so it cannot be +/// added, changed or withdrawn on a live session. A node that gains the Primary role after +/// connecting therefore cannot simply start publishing ONLINE — it would have no Will, +/// and a crash would leave a retained ONLINE asserting forever that a dead host is +/// alive. Equally, a node that registers a Will and later loses the role cannot withdraw it, +/// so its eventual disconnect would publish OFFLINE over the new Primary's live claim +/// and stop every edge node in the group. +/// +/// +/// The consequence is that the claim must be bound to the lifetime of a connection: +/// SparkplugPrimaryHost opens a dedicated connection when the node gains Primary and +/// closes it when the node loses Primary, rather than trying to mutate the ingest session it +/// shares with the rest of the driver. +/// +/// +/// Clean session matters too. A broker that resumes an existing session for the same +/// client id may discard the Will registered by the new CONNECT. The primary-host connection +/// forces clean-session on for that reason, regardless of the driver's configured value. +/// +/// +/// The topic the broker publishes the Will to. +/// The Will body. +/// Requested QoS, 0–2. +/// +/// Whether the broker retains the Will. For a Sparkplug host death certificate this is +/// : an edge node that connects after the host died must still learn +/// the host is offline, and only a retained message reaches it. +/// +public sealed record MqttLastWill(string Topic, byte[] Payload, int Qos, bool Retain); diff --git a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Mqtt.Contracts/Sparkplug/SparkplugCodec.cs b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Mqtt.Contracts/Sparkplug/SparkplugCodec.cs index b428ee19..309aa3c1 100644 --- a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Mqtt.Contracts/Sparkplug/SparkplugCodec.cs +++ b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Mqtt.Contracts/Sparkplug/SparkplugCodec.cs @@ -45,10 +45,19 @@ namespace ZB.MOM.WW.OtOpcUa.Driver.Mqtt.Sparkplug; /// wire, not of the tag. /// /// -/// DataSet and Template metrics are out of scope for v1. They decode to +/// Template and DataSet metrics decode to +/// with their members flattened into +/// (Gitea #515) — a template member Speed of metric +/// Motor becomes the addressable Motor/Speed. See +/// for the mapping and its bounds. Anything that cannot be flattened honestly stays /// with a null value — deliberately visible, so a -/// consumer can warn about a metric it cannot serve instead of silently treating it as one -/// that was never published. +/// consumer warns about a metric it cannot serve instead of silently treating it as one that was +/// never published, and never coerces it into a guessed scalar. +/// +/// +/// A metric's PropertySet is projected to +/// as node metadata, never as sibling values: a PropertySet describes a metric (units, ranges, +/// quality hints); it is not something the metric is. /// /// public static class SparkplugCodec @@ -161,7 +170,7 @@ public static class SparkplugCodec /// The projection. private static SparkplugMetric ProjectMetric(Payload.Types.Metric metric) { - var (kind, value) = ProjectValue(metric); + var (kind, value, members) = ProjectValue(metric); return new SparkplugMetric( Name: metric.HasName ? metric.Name : null, @@ -173,7 +182,22 @@ public static class SparkplugCodec DataType: metric.HasDatatype ? (TahuDataType)metric.Datatype : null, TimestampMs: metric.HasTimestamp ? metric.Timestamp : null, ValueKind: kind, - Value: value); + Value: value, + Members: members, + Properties: SparkplugFlattener.ProjectProperties(metric.Properties)); + } + + /// + /// Resolves a template member's value oneof. Internal to the assembly: shared with + /// , which walks nested templates and needs the identical + /// projection so a member and a top-level metric of the same type never decode differently. + /// + /// The template member, itself a generated Metric. + /// The value kind and the projected raw value. + internal static (SparkplugValueKind Kind, object? Value) ProjectMemberValue(Payload.Types.Metric metric) + { + var (kind, value, _) = ProjectValue(metric); + return (kind, value); } /// Resolves a metric's value oneof to a kind plus a raw CLR value. @@ -184,34 +208,52 @@ public static class SparkplugCodec /// that some datatypes have no spare sentinel, so a publisher that sets it means "null" even if /// a value field is also populated. /// - private static (SparkplugValueKind Kind, object? Value) ProjectValue(Payload.Types.Metric metric) + private static (SparkplugValueKind Kind, object? Value, IReadOnlyList Members) ProjectValue( + Payload.Types.Metric metric) { if (metric.HasIsNull && metric.IsNull) { - return (SparkplugValueKind.Null, null); + return (SparkplugValueKind.Null, null, []); + } + + // Gitea #515: structured values flatten to addressable members. A refusal to flatten falls + // through to Unsupported — the pre-#515 behaviour — rather than to a guessed scalar. + switch (metric.ValueCase) + { + case Payload.Types.Metric.ValueOneofCase.TemplateValue: + return SparkplugFlattener.TryFlattenTemplate(metric.TemplateValue, out var templateMembers) + ? (SparkplugValueKind.Structured, null, templateMembers) + : (SparkplugValueKind.Unsupported, null, []); + + case Payload.Types.Metric.ValueOneofCase.DatasetValue: + return SparkplugFlattener.TryFlattenDataSet(metric.DatasetValue, out var cells) + ? (SparkplugValueKind.Structured, null, cells) + : (SparkplugValueKind.Unsupported, null, []); } return metric.ValueCase switch { - Payload.Types.Metric.ValueOneofCase.IntValue => (SparkplugValueKind.Scalar, metric.IntValue), - Payload.Types.Metric.ValueOneofCase.LongValue => (SparkplugValueKind.Scalar, metric.LongValue), - Payload.Types.Metric.ValueOneofCase.FloatValue => (SparkplugValueKind.Scalar, metric.FloatValue), - Payload.Types.Metric.ValueOneofCase.DoubleValue => (SparkplugValueKind.Scalar, metric.DoubleValue), - Payload.Types.Metric.ValueOneofCase.BooleanValue => (SparkplugValueKind.Scalar, metric.BooleanValue), - Payload.Types.Metric.ValueOneofCase.StringValue => (SparkplugValueKind.Scalar, metric.StringValue), + Payload.Types.Metric.ValueOneofCase.IntValue => (SparkplugValueKind.Scalar, metric.IntValue, Empty), + Payload.Types.Metric.ValueOneofCase.LongValue => (SparkplugValueKind.Scalar, metric.LongValue, Empty), + Payload.Types.Metric.ValueOneofCase.FloatValue => (SparkplugValueKind.Scalar, metric.FloatValue, Empty), + Payload.Types.Metric.ValueOneofCase.DoubleValue => (SparkplugValueKind.Scalar, metric.DoubleValue, Empty), + Payload.Types.Metric.ValueOneofCase.BooleanValue => (SparkplugValueKind.Scalar, metric.BooleanValue, Empty), + Payload.Types.Metric.ValueOneofCase.StringValue => (SparkplugValueKind.Scalar, metric.StringValue, Empty), // Copied out of the ByteString: the projection must outlive the parsed message. Payload.Types.Metric.ValueOneofCase.BytesValue => - (SparkplugValueKind.Scalar, metric.BytesValue.ToByteArray()), + (SparkplugValueKind.Scalar, metric.BytesValue.ToByteArray(), Empty), - // v1 scope boundary — decoded as a visible refusal, not as an exception or a silent drop. - Payload.Types.Metric.ValueOneofCase.DatasetValue => (SparkplugValueKind.Unsupported, null), - Payload.Types.Metric.ValueOneofCase.TemplateValue => (SparkplugValueKind.Unsupported, null), - Payload.Types.Metric.ValueOneofCase.ExtensionValue => (SparkplugValueKind.Unsupported, null), + // An extension value is a third-party escape hatch with no defined semantics — there is + // nothing honest to flatten it into. + Payload.Types.Metric.ValueOneofCase.ExtensionValue => (SparkplugValueKind.Unsupported, null, Empty), - _ => (SparkplugValueKind.Absent, null), + _ => (SparkplugValueKind.Absent, null, Empty), }; } + + /// The shared empty member list — every scalar metric would otherwise allocate one. + private static readonly IReadOnlyList Empty = []; } /// @@ -279,13 +321,31 @@ public sealed record SparkplugPayload( /// switch over a dozen arms. /// /// +/// +/// The flattened members of a metric — a +/// Template or DataSet broken into addressable scalars (Gitea #515). Empty for every +/// other kind. Paths are relative to this metric; see . +/// +/// +/// The metric's PropertySet, projected as node metadata (Gitea #515). Empty when the metric +/// carried none. Never a value — a PropertySet describes the metric rather than being one. +/// public readonly record struct SparkplugMetric( string? Name, ulong? Alias, TahuDataType? DataType, ulong? TimestampMs, SparkplugValueKind ValueKind, - object? Value); + object? Value, + IReadOnlyList? Members = null, + IReadOnlyList? Properties = null) +{ + /// The flattened members, never null. + public IReadOnlyList MemberList => Members ?? []; + + /// The projected properties, never null. + public IReadOnlyList PropertyList => Properties ?? []; +} /// What a decoded metric's represents. /// @@ -305,9 +365,17 @@ public enum SparkplugValueKind Scalar, /// - /// A DataSet, Template or extension value — decoded and reported, but not - /// supported in v1. The consumer should warn and skip the metric rather than treat it as - /// missing. + /// An extension value, or a Template/DataSet that could not be flattened + /// honestly (see ). Decoded and reported, but not servable: + /// the consumer warns and skips the metric rather than treating it as missing — and never + /// coerces it into a guessed scalar. /// Unsupported, + + /// + /// A Template or DataSet whose members were flattened into + /// (Gitea #515). is + /// null — the parent has no scalar value of its own; its members carry the values. + /// + Structured, } diff --git a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Mqtt.Contracts/Sparkplug/SparkplugFlattener.cs b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Mqtt.Contracts/Sparkplug/SparkplugFlattener.cs new file mode 100644 index 00000000..7cb81bb8 --- /dev/null +++ b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Mqtt.Contracts/Sparkplug/SparkplugFlattener.cs @@ -0,0 +1,368 @@ +using Org.Eclipse.Tahu.Protobuf; +using TahuDataType = Org.Eclipse.Tahu.Protobuf.DataType; + +namespace ZB.MOM.WW.OtOpcUa.Driver.Mqtt.Sparkplug; + +/// +/// Flattens Sparkplug's structured metric values — Template (UDT instances) and +/// DataSet — into addressable scalar members, and projects a metric's PropertySet +/// into node metadata (Gitea #515). +/// +/// +/// +/// Flatten to siblings, by owner decision (2026-07-31). The alternative was mapping a +/// Template to an OPC UA Structure/UDT, which is truer to Sparkplug's model but far harder to +/// consume: flattened scalars are readable by any OPC UA client (ScadaBridge included) with no +/// type dictionary, and the result mirrors how the /raw tree already works. A member of +/// template metric Motor called Speed becomes the metric Motor/Speed. +/// +/// +/// Member paths contain /, which is already solved. That interaction is what +/// #515 flagged against #509, and #509 shipped first: +/// RawBrowseCommitMapper.ToRawSegment coerces the tag Name to a legal RawPath +/// segment while carrying the original slashed string through as metricName, plus a +/// collision guard that refuses two members which would sanitise onto one name. So the +/// flattened name travels intact as the binding key and only the display name is coerced. +/// +/// +/// Skip-and-warn survives, and that is a requirement rather than a leftover. Anything +/// this cannot flatten honestly — an extension value, a nested member with no name, a payload +/// past the bounds below — stays and is skipped +/// with a warning. A structured value coerced into a guessed scalar would bind a tag to data +/// that is not what its name says, which is worse than not binding it at all. +/// +/// +/// Bounded, because this walks an attacker-shaped object graph. A Template may contain +/// Templates, so the natural recursion is unbounded in a format anyone on the broker can +/// publish. and cap it; exceeding either +/// abandons the whole metric to Unsupported rather than publishing a truncated member +/// set, because a silently half-flattened UDT looks exactly like a complete one. +/// +/// +/// Nothing here throws — same contract as , for the same +/// reason: this runs on MQTTnet's shared dispatcher thread behind an unauthenticated firehose. +/// +/// +public static class SparkplugFlattener +{ + /// + /// How deeply nested a Template may be before the whole metric is refused. Five is far past + /// anything a real UDT library produces and still bounds the walk. + /// + public const int MaxDepth = 5; + + /// + /// Ceiling on the flattened members one metric may produce. A DataSet is columns × rows, so a + /// modest-looking table is the realistic way to reach this. + /// + public const int MaxMembers = 512; + + /// + /// Separator between a structured metric's name and its member path. The same character + /// Sparkplug itself uses for metric-name hierarchy, so Motor/Speed is what an edge node + /// would have called a flat metric of the same meaning. + /// + public const char MemberSeparator = '/'; + + /// + /// Flattens a Template into member paths relative to the owning metric. + /// + /// The template value. + /// The flattened members on success; empty otherwise. + /// + /// when the whole template flattened. means the + /// caller must treat the metric as . + /// + public static bool TryFlattenTemplate(Payload.Types.Template template, out IReadOnlyList members) + { + var collected = new List(); + var ok = template is not null && AppendTemplate(template, prefix: "", depth: 1, collected); + members = ok ? collected : []; + return ok; + } + + /// + /// Flattens a DataSet into one member per cell, named {column}/{rowIndex}. + /// + /// The dataset value. + /// The flattened members on success; empty otherwise. + /// + /// when the whole dataset flattened. means the + /// caller must treat the metric as . + /// + /// + /// + /// Per cell, not per column — and the row index is always present. The recorded + /// decision says "per-column tags, not one array-valued variable", and a column of N rows + /// is not a scalar, so per-column can only mean per-cell once N > 1. Emitting a bare + /// {column} for the single-row case and {column}/{row} otherwise would make + /// a tag's own name depend on how many rows the plant happened to publish — so a table + /// that grew from one row to two would rename every existing tag out from under its + /// authored binding. Uniform naming costs a /0 on the common single-row "current + /// values" shape and buys a stable address. + /// + /// + /// A row that disappears simply stops publishing, and its tag holds its last value — the + /// same as any metric an edge node stops sending. + /// + /// + public static bool TryFlattenDataSet(Payload.Types.DataSet dataSet, out IReadOnlyList members) + { + members = []; + if (dataSet is null) + { + return false; + } + + // Columns and types are parallel arrays. A publisher that disagrees with itself has produced a + // dataset whose cells cannot be typed, and guessing which array to trust would type a column + // by its neighbour — refuse instead. + if (dataSet.Columns.Count == 0 || dataSet.Types_.Count != dataSet.Columns.Count) + { + return false; + } + + if ((long)dataSet.Columns.Count * dataSet.Rows.Count > MaxMembers) + { + return false; + } + + var collected = new List(); + for (var row = 0; row < dataSet.Rows.Count; row++) + { + var elements = dataSet.Rows[row].Elements; + + // A short row is a malformed dataset, not a row of nulls: emitting the cells that ARE + // present would publish a table silently missing its right-hand columns. + if (elements.Count != dataSet.Columns.Count) + { + return false; + } + + for (var col = 0; col < dataSet.Columns.Count; col++) + { + var column = dataSet.Columns[col]; + if (string.IsNullOrEmpty(column)) + { + return false; // An unnamed column has no address. + } + + var (kind, value) = ProjectDataSetValue(elements[col]); + collected.Add(new SparkplugMember( + Path: $"{column}{MemberSeparator}{row.ToString(System.Globalization.CultureInfo.InvariantCulture)}", + DataType: (TahuDataType)dataSet.Types_[col], + ValueKind: kind, + Value: value)); + } + } + + members = collected; + return true; + } + + /// + /// Projects a metric's PropertySet into flat name/value metadata. + /// + /// The property set, or . + /// The projected properties; empty when there are none or the set is malformed. + /// + /// + /// Metadata, never tags — the recorded decision, and the honest reading of the + /// spec: a PropertySet describes a metric (engineering units, ranges, quality hints), it is + /// not a value the metric has. Surfacing it as sibling tags would invent plant signals out + /// of documentation. + /// + /// + /// Nested PropertySet/PropertySetList values are flattened one level with a + /// /-joined key, matching the metric-member convention; anything deeper, or an + /// extension value, is dropped rather than stringified. + /// + /// + public static IReadOnlyList ProjectProperties(Payload.Types.PropertySet? properties) + { + if (properties is null || properties.Keys.Count == 0 || properties.Keys.Count != properties.Values.Count) + { + return []; + } + + var collected = new List(); + AppendProperties(properties, prefix: "", depth: 1, collected); + return collected; + } + + private static void AppendProperties( + Payload.Types.PropertySet set, string prefix, int depth, List into) + { + if (set.Keys.Count != set.Values.Count || depth > 2 || into.Count >= MaxMembers) + { + return; + } + + for (var i = 0; i < set.Keys.Count && into.Count < MaxMembers; i++) + { + var key = set.Keys[i]; + if (string.IsNullOrEmpty(key)) + { + continue; + } + + var name = prefix.Length == 0 ? key : $"{prefix}{MemberSeparator}{key}"; + var value = set.Values[i]; + + if (value.HasIsNull && value.IsNull) + { + into.Add(new SparkplugProperty(name, null)); + continue; + } + + switch (value.ValueCase) + { + case Payload.Types.PropertyValue.ValueOneofCase.IntValue: + into.Add(new SparkplugProperty(name, value.IntValue)); + break; + case Payload.Types.PropertyValue.ValueOneofCase.LongValue: + into.Add(new SparkplugProperty(name, value.LongValue)); + break; + case Payload.Types.PropertyValue.ValueOneofCase.FloatValue: + into.Add(new SparkplugProperty(name, value.FloatValue)); + break; + case Payload.Types.PropertyValue.ValueOneofCase.DoubleValue: + into.Add(new SparkplugProperty(name, value.DoubleValue)); + break; + case Payload.Types.PropertyValue.ValueOneofCase.BooleanValue: + into.Add(new SparkplugProperty(name, value.BooleanValue)); + break; + case Payload.Types.PropertyValue.ValueOneofCase.StringValue: + into.Add(new SparkplugProperty(name, value.StringValue)); + break; + case Payload.Types.PropertyValue.ValueOneofCase.PropertysetValue: + AppendProperties(value.PropertysetValue, name, depth + 1, into); + break; + default: + // PropertySetList and extension values are dropped rather than stringified — a + // property whose displayed value is not what its type says is worse than an absent + // one, and these are documentation, not signal. + break; + } + } + } + + /// Walks one template level, appending members under . + /// as soon as anything cannot be flattened honestly. + private static bool AppendTemplate( + Payload.Types.Template template, string prefix, int depth, List into) + { + if (depth > MaxDepth) + { + return false; + } + + // A template DEFINITION carries the UDT's shape with no instance values. It is metadata an + // edge node publishes so consumers can understand instances — flattening it would materialise + // a parallel set of tags that never carry a value. + if (template.HasIsDefinition && template.IsDefinition) + { + return false; + } + + foreach (var member in template.Metrics) + { + if (into.Count >= MaxMembers) + { + return false; + } + + if (!member.HasName || string.IsNullOrEmpty(member.Name)) + { + // A template member is addressed by name; there is no alias fallback inside a template. + return false; + } + + var path = prefix.Length == 0 ? member.Name : $"{prefix}{MemberSeparator}{member.Name}"; + + if (member.ValueCase == Payload.Types.Metric.ValueOneofCase.TemplateValue) + { + if (!AppendTemplate(member.TemplateValue, path, depth + 1, into)) + { + return false; + } + + continue; + } + + if (member.ValueCase == Payload.Types.Metric.ValueOneofCase.DatasetValue) + { + if (!TryFlattenDataSet(member.DatasetValue, out var cells) + || into.Count + cells.Count > MaxMembers) + { + return false; + } + + foreach (var cell in cells) + { + into.Add(cell with { Path = $"{path}{MemberSeparator}{cell.Path}" }); + } + + continue; + } + + var (kind, value) = SparkplugCodec.ProjectMemberValue(member); + if (kind == SparkplugValueKind.Unsupported) + { + return false; // An extension value inside a template: refuse the whole metric. + } + + into.Add(new SparkplugMember( + Path: path, + DataType: member.HasDatatype ? (TahuDataType)member.Datatype : null, + ValueKind: kind, + Value: value)); + } + + return true; + } + + private static (SparkplugValueKind Kind, object? Value) ProjectDataSetValue( + Payload.Types.DataSet.Types.DataSetValue value) => value.ValueCase switch + { + Payload.Types.DataSet.Types.DataSetValue.ValueOneofCase.IntValue => (SparkplugValueKind.Scalar, value.IntValue), + Payload.Types.DataSet.Types.DataSetValue.ValueOneofCase.LongValue => (SparkplugValueKind.Scalar, value.LongValue), + Payload.Types.DataSet.Types.DataSetValue.ValueOneofCase.FloatValue => (SparkplugValueKind.Scalar, value.FloatValue), + Payload.Types.DataSet.Types.DataSetValue.ValueOneofCase.DoubleValue => (SparkplugValueKind.Scalar, value.DoubleValue), + Payload.Types.DataSet.Types.DataSetValue.ValueOneofCase.BooleanValue => (SparkplugValueKind.Scalar, value.BooleanValue), + Payload.Types.DataSet.Types.DataSetValue.ValueOneofCase.StringValue => (SparkplugValueKind.Scalar, value.StringValue), + + // A DataSetValue has no is_null field, so an empty oneof IS the null cell — unlike a Metric, + // where Absent and explicit-null are genuinely different states. + _ => (SparkplugValueKind.Null, null), + }; +} + +/// +/// One flattened member of a structured Sparkplug metric (Gitea #515). +/// +/// +/// The member path relative to the owning metric, /-joined — Speed for a +/// top-level template member, Motor/Speed for a nested one, Temp/0 for a dataset +/// cell. The consumer composes the full metric name as {parentName}/{Path}; it is kept +/// relative here because a DATA metric carries only the parent's alias, so the parent's name is +/// not known until the alias table has resolved it. +/// +/// +/// The member's declared datatype, or when it carried none — in which case +/// the consumer falls back to the datatype the birth installed for this member path. +/// +/// What means; never Unsupported here. +/// The raw wire value, with the same signed-integer caveat as a scalar metric. +public readonly record struct SparkplugMember( + string Path, + TahuDataType? DataType, + SparkplugValueKind ValueKind, + object? Value); + +/// One entry of a metric's PropertySet, projected as node metadata (Gitea #515). +/// The property key, /-joined for a nested set. +/// +/// The boxed scalar value, or for an explicitly-null property. +/// +public readonly record struct SparkplugProperty(string Name, object? Value); diff --git a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Mqtt.Contracts/Sparkplug/SparkplugStatePayload.cs b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Mqtt.Contracts/Sparkplug/SparkplugStatePayload.cs new file mode 100644 index 00000000..1065f109 --- /dev/null +++ b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Mqtt.Contracts/Sparkplug/SparkplugStatePayload.cs @@ -0,0 +1,141 @@ +using System.Text; +using System.Text.Json; + +namespace ZB.MOM.WW.OtOpcUa.Driver.Mqtt.Sparkplug; + +/// +/// Builds the Sparkplug v3.0 primary-host STATE bodies — the retained ONLINE birth +/// and the matching OFFLINE death that is registered as the MQTT Last Will (Gitea #511). +/// +/// +/// +/// Encode only. The receive side lives in SparkplugIngestor.TryReadStateOnline, +/// which additionally tolerates the pre-3.0 bare ONLINE/OFFLINE text form. This +/// type only ever produces the v3.0 JSON shape, matching +/// 's refusal to produce the legacy topic. +/// +/// +/// The two bodies share one timestamp, and that is a spec requirement rather than a +/// convenience. Sparkplug 3.0 pairs a host's birth and death certificates by their +/// timestamp field: an edge node that receives an OFFLINE whose timestamp is +/// older than the ONLINE it already holds is entitled to ignore it as a stale death +/// from a previous session. That rule is what makes the mechanism safe across a reconnect — +/// and it means the death body has to be built before CONNECT (a Will is registered in +/// the CONNECT packet) while the birth is published after CONNACK. Deriving each from +/// its own UtcNow would give them different timestamps and quietly break the pairing, +/// so mints both at once and there is no single-body overload to +/// reach for by accident. +/// +/// +public static class SparkplugStatePayload +{ + /// + /// QoS for both STATE bodies. Sparkplug 3.0 requires QoS 1 for the host birth and death — + /// unlike the QoS 0 that carries NBIRTH/DBIRTH/DATA. A host claim delivered at most once is a + /// claim that may simply never arrive. + /// + public const int StateQos = 1; + + /// + /// Mints the matched ONLINE/OFFLINE pair for one host session, both carrying + /// . + /// + /// The Sparkplug host id; used to build the topic. + /// + /// Sparkplug epoch milliseconds for this session. Supplied by the caller rather than read from + /// the clock here so the value is visible at the call site and testable. + /// + /// The topic plus both bodies. + /// is blank. + public static SparkplugStateSession ForSession(string hostId, long timestampMs) + { + ArgumentException.ThrowIfNullOrWhiteSpace(hostId); + + return new SparkplugStateSession( + Topic: SparkplugTopic.FormatState(hostId), + TimestampMs: timestampMs, + OnlineBody: Encode(online: true, timestampMs), + OfflineBody: Encode(online: false, timestampMs)); + } + + /// Encodes one STATE body: {"online":<bool>,"timestamp":<ms>}. + /// The online flag. + /// Sparkplug epoch milliseconds. + /// The UTF-8 body bytes. + /// + /// Hand-written rather than serialized from a DTO. The body is two fields, and the wire form + /// is fixed by the spec — a serializer would put naming policy, casing options and a shared + /// JsonSerializerOptions between this repo and a wire contract it does not control. + /// timestamp is emitted as a bare JSON number, never a string: a quoted timestamp + /// parses as a valid JSON object and is silently wrong to every conformant host. + /// + public static byte[] Encode(bool online, long timestampMs) => + Encoding.UTF8.GetBytes( + $"{{\"online\":{(online ? "true" : "false")},\"timestamp\":{timestampMs.ToString(System.Globalization.CultureInfo.InvariantCulture)}}}"); + + /// Reads the online flag back out of a v3.0 STATE body, for tests and diagnostics. + /// The STATE body bytes. + /// The decoded flag on success. + /// The decoded timestamp on success. + /// when the body is a well-formed v3.0 STATE object. + public static bool TryDecode(ReadOnlySpan body, out bool online, out long timestampMs) + { + online = false; + timestampMs = 0; + + if (body.IsEmpty) + { + return false; + } + + try + { + var reader = new Utf8JsonReader(body); + if (!JsonDocument.TryParseValue(ref reader, out var doc)) + { + return false; + } + + using (doc) + { + if (doc.RootElement.ValueKind != JsonValueKind.Object + || !doc.RootElement.TryGetProperty("online", out var flag) + || flag.ValueKind is not (JsonValueKind.True or JsonValueKind.False)) + { + return false; + } + + online = flag.GetBoolean(); + timestampMs = doc.RootElement.TryGetProperty("timestamp", out var ts) + && ts.ValueKind == JsonValueKind.Number + && ts.TryGetInt64(out var parsed) + ? parsed + : 0; + return true; + } + } + catch (JsonException) + { + return false; + } + } +} + +/// +/// The STATE topic plus the matched pair of bodies for one primary-host session (Gitea #511). +/// +/// The v3.0 STATE topic, spBv1.0/STATE/{hostId}. +/// The session timestamp carried by BOTH bodies. +/// +/// The birth certificate — published retained, at QoS 1, immediately after CONNACK. +/// +/// +/// The death certificate — registered as the MQTT Last Will in the CONNECT packet, and also +/// published explicitly on a clean stand-down so the broker holds it without waiting for a +/// keep-alive timeout. +/// +public sealed record SparkplugStateSession( + string Topic, + long TimestampMs, + byte[] OnlineBody, + byte[] OfflineBody); diff --git a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Mqtt.Transport/MqttClientOptionsFactory.cs b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Mqtt.Transport/MqttClientOptionsFactory.cs index 4e4e9278..8b0dcd69 100644 --- a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Mqtt.Transport/MqttClientOptionsFactory.cs +++ b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Mqtt.Transport/MqttClientOptionsFactory.cs @@ -41,20 +41,51 @@ public static class MqttClientOptionsFactory /// empty MQTTnet generates a random client id. /// /// Optional logger for the deferred CA-pin load; never receives credentials. + /// + /// Optional MQTT Last Will registered in the CONNECT packet (Gitea #511 — the Sparkplug + /// primary-host death certificate). See for why this can only be + /// supplied here. + /// + /// + /// Forces clean-session on or off, ignoring . Used + /// by the primary-host connection, which must connect clean so the broker cannot resume a + /// previous session and discard the Will — see . + /// public static MqttClientOptions BuildClientOptions( MqttDriverOptions options, string? clientIdSuffix, - ILogger? logger = null) + ILogger? logger = null, + MqttLastWill? lastWill = null, + bool? cleanSessionOverride = null) { ArgumentNullException.ThrowIfNull(options); var builder = new MqttClientOptionsBuilder() .WithTcpServer(options.Host, options.Port) .WithProtocolVersion(MapProtocolVersion(options.ProtocolVersion)) - .WithCleanSession(options.CleanSession) + .WithCleanSession(cleanSessionOverride ?? options.CleanSession) .WithKeepAlivePeriod(TimeSpan.FromSeconds(options.KeepAliveSeconds)) .WithTimeout(TimeSpan.FromSeconds(options.ConnectTimeoutSeconds)); + if (lastWill is { } will) + { + builder = builder + .WithWillTopic(will.Topic) + .WithWillPayload(will.Payload) + .WithWillQualityOfServiceLevel((MQTTnet.Protocol.MqttQualityOfServiceLevel)will.Qos) + .WithWillRetain(will.Retain); + + // MQTT 5.0 only. Left at its default the broker MAY hold the Will back for up to the + // session-expiry interval after an ungraceful disconnect, which for a host death + // certificate is precisely the wrong behaviour: the whole point is that edge nodes learn + // promptly. Zero means "publish it the moment the network connection is lost". + // The setter is harmless on 3.1.1 — the property simply has no wire representation there. + if (options.ProtocolVersion == MqttProtocolVersion.V500) + { + builder = builder.WithWillDelayInterval(0); + } + } + var clientId = string.Concat(options.ClientId ?? string.Empty, clientIdSuffix ?? string.Empty); if (!string.IsNullOrWhiteSpace(clientId)) { diff --git a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Mqtt/MqttConnection.cs b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Mqtt/MqttConnection.cs index 6582de2e..57219b83 100644 --- a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Mqtt/MqttConnection.cs +++ b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Mqtt/MqttConnection.cs @@ -286,11 +286,40 @@ public sealed class MqttConnection : IAsyncDisposable, IMqttSubscribeTransport, private Task? _reconnectWorker; private int _state = (int)MqttConnectionState.Disconnected; + /// Client-id suffix distinguishing this connection from the driver's other one; see the ctor. + private readonly string? _clientIdSuffix; + + /// The Will to register on every CONNECT, or null. See . + private readonly MqttLastWill? _lastWill; + + /// Clean-session override, or null to use the configured value. + private readonly bool? _cleanSessionOverride; + /// Stores configuration only — no network, no client instantiation, no supervisor. /// Broker connection settings. /// Driver instance id, used only for log/diagnostic correlation. /// Optional logger; never receives credentials. - public MqttConnection(MqttDriverOptions options, string driverId, ILogger? logger = null) + /// + /// Appended to the configured client id so a second connection from the same driver — the + /// Sparkplug primary-host claim (Gitea #511) — does not collide with the ingest session. Two + /// MQTT clients presenting one client id is not an error the broker reports: it silently + /// disconnects the older one, so the two would take turns evicting each other forever. + /// + /// + /// Optional Last Will registered in the CONNECT packet, and therefore rebuilt on every + /// reconnect attempt along with the rest of the options. See . + /// + /// + /// Forces clean-session on or off regardless of . + /// Null keeps the configured value. + /// + public MqttConnection( + MqttDriverOptions options, + string driverId, + ILogger? logger = null, + string? clientIdSuffix = null, + MqttLastWill? lastWill = null, + bool? cleanSessionOverride = null) { ArgumentNullException.ThrowIfNull(options); ArgumentException.ThrowIfNullOrWhiteSpace(driverId); @@ -298,6 +327,9 @@ public sealed class MqttConnection : IAsyncDisposable, IMqttSubscribeTransport, _options = options; _driverId = driverId; _logger = logger; + _clientIdSuffix = clientIdSuffix; + _lastWill = lastWill; + _cleanSessionOverride = cleanSessionOverride; } /// @@ -370,6 +402,47 @@ public sealed class MqttConnection : IAsyncDisposable, IMqttSubscribeTransport, /// internal Func? AfterConnectHookForTests { get; set; } + /// + /// Test seam: drops the session the way a crashed process does — no MQTT DISCONNECT, so + /// the broker treats the closed socket as an abnormal termination and publishes the Last Will. + /// + /// A task completing once the client is gone and the supervisor has stopped. + /// + /// + /// Exists because the Sparkplug primary-host contract's most important leg is the one a + /// normal teardown cannot reach (Gitea #511): DisposeAsync sends a clean + /// DISCONNECT, which by MQTT rule means the broker discards the Will. So an + /// in-suite test driven through the ordinary lifecycle can only ever observe the explicit + /// stand-down OFFLINE, and would pass identically against an implementation whose + /// Will was never armed at all. + /// + /// + /// Not a full substitute for the kill -9 the live gate runs by hand — that also + /// proves the process dies without any of this code running — but it is the closest an + /// in-process test can get, and it is what makes the Will's content observable. + /// + /// + internal async Task AbandonForTests() + { + // Mark disposed first, so the supervisor cannot reconnect into the gap and publish a fresh + // birth on top of the Will the broker is about to deliver. + Interlocked.Exchange(ref _disposed, 1); + SetState(MqttConnectionState.Disposed); + + try + { + await _lifetimeCts.CancelAsync().ConfigureAwait(false); + } + catch (ObjectDisposedException) + { + // Nothing to cancel. + } + + // Dispose WITHOUT DisconnectAsync: MQTTnet closes the channel, the broker sees a dead socket + // rather than a graceful goodbye, and the Will fires. This is the whole point of the seam. + Interlocked.Exchange(ref _client, null)?.Dispose(); + } + private bool Disposed => Volatile.Read(ref _disposed) != 0; /// @@ -568,7 +641,8 @@ public sealed class MqttConnection : IAsyncDisposable, IMqttSubscribeTransport, MqttClientOptions clientOptions; try { - clientOptions = MqttClientOptionsFactory.BuildClientOptions(_options, clientIdSuffix: null, _logger); + clientOptions = MqttClientOptionsFactory.BuildClientOptions( + _options, _clientIdSuffix, _logger, _lastWill, _cleanSessionOverride); } catch (Exception ex) { diff --git a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Mqtt/MqttDriver.cs b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Mqtt/MqttDriver.cs index 4f1e4d54..e8826724 100644 --- a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Mqtt/MqttDriver.cs +++ b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Mqtt/MqttDriver.cs @@ -185,6 +185,16 @@ public sealed class MqttDriver private MqttConnection? _connection; private CancellationTokenSource? _hostProbeCts; + /// The Primary data-plane verdict the Sparkplug host claim is gated on (Gitea #511). + private readonly IDriverPrimaryRoleSource _primaryRoleSource; + + /// + /// The Sparkplug primary-host claim, or when this configuration does not + /// ask for one. Owns its own broker connection — see for why + /// it cannot share the ingest session. + /// + private SparkplugPrimaryHost? _primaryHost; + private DriverHealth _health = new(DriverState.Unknown, null, null); private HostState _hostState = HostState.Unknown; private DateTime _hostStateChangedUtc = DateTime.UtcNow; @@ -198,7 +208,18 @@ public sealed class MqttDriver /// /// Stable logical id of this driver instance. /// Optional logger; never receives credentials or payload bodies. - public MqttDriver(MqttDriverOptions options, string driverInstanceId, ILogger? logger = null) + /// + /// Gitea #511: the Primary data-plane verdict the Sparkplug primary-host claim is gated on. + /// falls back to — which + /// always claims — for standalone hosts and tests. On a redundant pair that fallback is the + /// dual-claim this gate exists to prevent, so the Host resolves a real one and + /// DriverFactoryBootstrap does so with GetRequiredService. + /// + public MqttDriver( + MqttDriverOptions options, + string driverInstanceId, + ILogger? logger = null, + IDriverPrimaryRoleSource? primaryRoleSource = null) { ArgumentNullException.ThrowIfNull(options); ArgumentException.ThrowIfNullOrWhiteSpace(driverInstanceId); @@ -206,6 +227,7 @@ public sealed class MqttDriver _options = options; _driverInstanceId = driverInstanceId; _logger = logger; + _primaryRoleSource = primaryRoleSource ?? UnmanagedDriverPrimaryRoleSource.Instance; _subscriptions = CreateSubscriptionManager(options); _sparkplug = CreateSparkplugIngestor(options); RegisterAuthoredTags(options); @@ -1001,6 +1023,7 @@ public sealed class MqttDriver WriteHealth(new DriverHealth(DriverState.Healthy, connection.LastMessageUtc, null)); TransitionHost(HostState.Running); StartHostProbe(); + await StartPrimaryHostAsync(cancellationToken).ConfigureAwait(false); } catch (Exception ex) { @@ -1094,21 +1117,15 @@ public sealed class MqttDriver return null; } - if (options.Sparkplug is { ActAsPrimaryHost: true }) + // The flag no longer reads as if it does something it does not (Gitea #511) — but a request it + // CANNOT honour still warns loudly rather than being dropped, because the operator's plant is + // waiting on a STATE message that will never arrive. + if (SparkplugPrimaryHost.DescribeUnusable(options) is { } unusable) { - // Loud, because the flag reads as if it does something. Being a Sparkplug Primary Host is a - // three-part contract — a retained ONLINE after CONNECT, a matching retained OFFLINE - // registered as the MQTT Last Will AT CONNECT TIME, and an explicit OFFLINE on clean - // shutdown — and edge nodes stop publishing while their host is offline. Shipping the - // ONLINE half without the Will is strictly worse than shipping neither: a crashed node - // would leave a retained ONLINE asserting forever that a dead host is alive, which is the - // exact state the mechanism exists to prevent. The Will belongs in - // MqttClientOptionsFactory.BuildClientOptions; until it is there this flag stays inert and said so. _logger?.LogWarning( - "MQTT driver '{DriverId}': Sparkplug actAsPrimaryHost is set but primary-host STATE " - + "publishing is not implemented — this driver observes STATE and never publishes one. " - + "Edge nodes gated on this host id will not see it come online.", - _driverInstanceId); + "MQTT driver '{DriverId}': {Reason} Edge nodes gated on this host id will not see it come online.", + _driverInstanceId, + unusable); } var ingestor = new SparkplugIngestor( @@ -1263,9 +1280,55 @@ public sealed class MqttDriver o.Sparkplug, }; + /// + /// Opens the Sparkplug primary-host claim when this configuration asks for one (Gitea #511). + /// + /// + /// + /// Runs last, and can never fail the initialize. The claim is an advisory assertion + /// to other applications on the broker; the ingest path is this driver's data plane. A + /// broker that accepts the ingest session but rejects a publish on the STATE topic — an ACL + /// that does not grant spBv1.0/STATE/# is the ordinary way this happens — must cost + /// the operator a warning and a plant-visible missing host, not every tag's value. + /// already swallows and retries; the catch + /// here covers the rest so no future edit can turn this into a fault path by accident. + /// + /// + private async Task StartPrimaryHostAsync(CancellationToken cancellationToken) + { + if (!SparkplugPrimaryHost.CanClaim(_options)) + { + return; + } + + try + { + var host = new SparkplugPrimaryHost(_options, _driverInstanceId, _primaryRoleSource, _logger); + _primaryHost = host; + await host.StartAsync(cancellationToken).ConfigureAwait(false); + } + catch (Exception ex) + { + _logger?.LogWarning( + ex, + "MQTT driver '{DriverId}': the Sparkplug primary-host claim could not be started; ingest is unaffected.", + _driverInstanceId); + } + } + /// Shared teardown for , and the session rebuild. private async Task TeardownAsync() { + // Stand the host claim down FIRST, while the broker is still reachable and before the ingest + // session goes. Its explicit retained OFFLINE is what makes the transition immediate for edge + // nodes; leaving it to the Will costs them up to 1.5 keep-alive periods of believing a host + // that has already stopped is still online. + var primaryHost = Interlocked.Exchange(ref _primaryHost, null); + if (primaryHost is not null) + { + await primaryHost.DisposeAsync().ConfigureAwait(false); + } + var probe = Interlocked.Exchange(ref _hostProbeCts, null); if (probe is not null) { diff --git a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Mqtt/MqttDriverFactoryExtensions.cs b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Mqtt/MqttDriverFactoryExtensions.cs index dc6d9bea..cf6dc094 100644 --- a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Mqtt/MqttDriverFactoryExtensions.cs +++ b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Mqtt/MqttDriverFactoryExtensions.cs @@ -49,16 +49,30 @@ public static class MqttDriverFactoryExtensions /// /// The driver factory registry to register with. /// Optional logger factory used to create per-instance loggers. - public static void Register(DriverFactoryRegistry registry, ILoggerFactory? loggerFactory = null) + /// + /// Gitea #511: the Primary data-plane verdict the Sparkplug primary-host claim is gated on, + /// captured at registration time like . Null falls back to + /// (always claims) — correct for a standalone + /// host or a test, and the reason DriverFactoryBootstrap resolves it with + /// GetRequiredService rather than letting a redundant deployment reach that fallback. + /// + public static void Register( + DriverFactoryRegistry registry, + ILoggerFactory? loggerFactory = null, + IDriverPrimaryRoleSource? primaryRoleSource = null) { ArgumentNullException.ThrowIfNull(registry); - registry.Register(DriverTypeName, (id, json) => CreateInstance(id, json, loggerFactory)); + registry.Register(DriverTypeName, (id, json) => CreateInstance(id, json, loggerFactory, primaryRoleSource)); } /// Public for the Server-side bootstrapper + test consumers. /// Stable logical id of the driver instance. /// The DriverConfig JSON blob for the instance. /// Optional logger factory for the per-instance logger. + /// + /// Gitea #511: the Primary data-plane verdict gating the Sparkplug primary-host claim; null + /// falls back to . + /// /// A configured, not-yet-connected . /// /// or is blank. @@ -68,7 +82,8 @@ public static class MqttDriverFactoryExtensions public static MqttDriver CreateInstance( string driverInstanceId, string driverConfigJson, - ILoggerFactory? loggerFactory = null) + ILoggerFactory? loggerFactory = null, + IDriverPrimaryRoleSource? primaryRoleSource = null) { ArgumentException.ThrowIfNullOrWhiteSpace(driverInstanceId); ArgumentException.ThrowIfNullOrWhiteSpace(driverConfigJson); @@ -79,6 +94,7 @@ public static class MqttDriverFactoryExtensions ?? throw new InvalidOperationException( $"MQTT driver config for '{driverInstanceId}' deserialised to null"); - return new MqttDriver(options, driverInstanceId, loggerFactory?.CreateLogger()); + return new MqttDriver( + options, driverInstanceId, loggerFactory?.CreateLogger(), primaryRoleSource); } } diff --git a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Mqtt/Sparkplug/AliasTable.cs b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Mqtt/Sparkplug/AliasTable.cs index 45d66b0b..e5dc9030 100644 --- a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Mqtt/Sparkplug/AliasTable.cs +++ b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Mqtt/Sparkplug/AliasTable.cs @@ -128,6 +128,36 @@ public sealed class AliasTable aliasCollisions++; (collidedAliases ??= []).Add(alias); } + + // Gitea #515: a Template/DataSet metric also installs one binding per flattened member, + // named `{parent}/{memberPath}`. The parent's own binding above is kept — a DATA metric + // carries only the parent's ALIAS, so resolving that alias to the parent's name is the only + // way the member names can be composed at all. But the parent is not itself servable (its + // Sparkplug datatype maps to no DriverDataType), so it is the member bindings that browse + // enumerates and that authored tags bind to. + // + // Members carry NO alias, deliberately: aliases are per-metric on the wire, so inventing + // one here would create a number the edge node never assigned, which a later birth could + // legitimately hand to a different metric. + foreach (var member in metric.MemberList) + { + if (string.IsNullOrWhiteSpace(member.Path)) + { + rejectedUnnamed++; + continue; + } + + var memberBinding = new SparkplugMetricBinding( + $"{binding.Name}{SparkplugFlattener.MemberSeparator}{member.Path}", + Alias: null, + member.DataType ?? TahuDataType.Unknown); + + if (!byName.TryAdd(memberBinding.Name, memberBinding)) + { + duplicateNames++; + byName[memberBinding.Name] = memberBinding; + } + } } if (collidedAliases is not null) diff --git a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Mqtt/Sparkplug/SparkplugIngestor.cs b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Mqtt/Sparkplug/SparkplugIngestor.cs index a5c294ef..156d356e 100644 --- a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Mqtt/Sparkplug/SparkplugIngestor.cs +++ b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Mqtt/Sparkplug/SparkplugIngestor.cs @@ -73,9 +73,11 @@ namespace ZB.MOM.WW.OtOpcUa.Driver.Mqtt.Sparkplug; /// subscriber is contained, and the rebirth publish is dispatched off-thread. /// /// -/// Primary-host STATE publishing is deliberately NOT implemented — see -/// and the remarks on -/// . This type observes STATE; it never publishes one. +/// This type only ever observes STATE. Primary-host STATE publishing ships +/// (Gitea #511) but deliberately does not live here: the claim is bound to the lifetime of its +/// own dedicated connection, because an MQTT Will can only be registered at CONNECT. See +/// , and the remarks on for what +/// this side does with an observed one. /// /// public sealed class SparkplugIngestor @@ -1029,18 +1031,17 @@ public sealed class SparkplugIngestor /// /// /// - /// Receive only. This driver never publishes STATE, and - /// is therefore inert — deliberately, and - /// loudly. Being a Sparkplug Primary Host is not "publish an ONLINE message": it is a - /// three-part contract — a retained ONLINE STATE published after CONNECT, a matching - /// retained OFFLINE registered as the client's MQTT Last Will at CONNECT time, - /// and an explicit OFFLINE on clean shutdown. Edge nodes subscribe to it and stop publishing - /// while their host is offline. Shipping the ONLINE half without the Will is strictly worse - /// than shipping neither: an OtOpcUa node that crashes would leave a retained ONLINE asserting - /// forever that a dead host is alive, which is precisely the state the mechanism exists to - /// prevent. The Will has to be set inside MqttClientOptionsFactory.BuildClientOptions — a - /// different file, a different task — so this task implements the observation half and - /// warns when the flag is set. See the design doc §7/§8. + /// Receive only, on this connection. The publish half ships as + /// (Gitea #511), on a connection of its own — an MQTT Will + /// can only be registered in the CONNECT packet, so a claim that is acquired and released at + /// runtime cannot ride the long-lived ingest session this type is attached to. + /// + /// + /// A host claimed by THIS node is observed here too, and that is correct rather than an + /// oversight. The ingestor subscribes the configured host id whether or not this node + /// publishes it, so a node acting as primary host sees its own retained ONLINE come + /// back. Nothing here gates on it — is informational — so the + /// echo costs one log line at the transition and cannot feed back into the claim. /// /// /// Both payload forms are accepted on receive: the v3.0 JSON {"online":bool,...} and @@ -1176,6 +1177,15 @@ public sealed class SparkplugIngestor SparkplugMetric metric, DateTime? sourceUtc) { + // Gitea #515: a Template/DataSet has no value of its own — its flattened members do. Expand + // here, at the single funnel both the DATA and the birth-value paths already pass through, so + // neither can end up supporting structured metrics while the other silently does not. + if (metric.ValueKind == SparkplugValueKind.Structured) + { + PublishStructuredMembers(scope, binding, metric, sourceUtc); + return; + } + // Bound by NAME. The alias got us here; it is never the key. var key = new SparkplugMetricKey(scope.GroupId, scope.EdgeNodeId, scope.DeviceId, binding.Name); if (!_authored.ByMetric.TryGetValue(key, out var defs)) @@ -1286,6 +1296,62 @@ public sealed class SparkplugIngestor : BadTypeMismatch(def, binding, target, stamp, serverUtc); } + /// + /// Publishes each flattened member of a structured (Template / DataSet) metric as + /// the sibling metric {parent}/{memberPath} (Gitea #515). + /// + /// + /// + /// Each member is re-entered through as a scalar, rather + /// than duplicating the coercion, array-gate and authored-tag fan-out here. A second copy of + /// that logic would be free to drift — and the drift would be silent, since both paths + /// publish values that look plausible. + /// + /// + /// The member's datatype falls back to the BIRTH's. A DDATA template usually + /// re-declares its members' datatypes, but it is not obliged to; the binding installed at + /// birth is the authority when it does not. Guessing from the CLR type of the wire value + /// would type a signed Int32 of -42 as a UInt32 of 4294967254 — the exact + /// plausible-and-wrong failure ReinterpretSigned exists to prevent. + /// + /// + private void PublishStructuredMembers( + SparkplugScope scope, + SparkplugMetricBinding parent, + SparkplugMetric metric, + DateTime? sourceUtc) + { + var table = Births.Find(scope); + + foreach (var member in metric.MemberList) + { + if (string.IsNullOrWhiteSpace(member.Path)) + { + continue; + } + + var name = $"{parent.Name}{SparkplugFlattener.MemberSeparator}{member.Path}"; + + // Wire datatype first, birth binding second. Unknown last — which the typed layer refuses + // once, loudly, rather than coercing. + var dataType = member.DataType + ?? table?.ResolveByName(name)?.DataType + ?? Org.Eclipse.Tahu.Protobuf.DataType.Unknown; + + PublishMetric( + scope, + new SparkplugMetricBinding(name, Alias: null, dataType), + new SparkplugMetric( + Name: name, + Alias: null, + DataType: dataType, + TimestampMs: metric.TimestampMs, + ValueKind: member.ValueKind, + Value: member.Value), + sourceUtc); + } + } + /// /// The metric decoded, but does not fit the tag's effective type. Warned once per tag; the value /// itself is deliberately never logged — a payload value is plant data, not diagnostics. @@ -1369,9 +1435,7 @@ public sealed class SparkplugIngestor try { - subscribers(this, new SparkplugBirthObservedEventArgs( - scope, - [.. payload.Metrics.Where(m => m.Name is not null).Select(m => m.Name!)])); + subscribers(this, new SparkplugBirthObservedEventArgs(scope, [.. DiscoverableNames(payload)])); } catch (Exception ex) { @@ -1379,6 +1443,38 @@ public sealed class SparkplugIngestor } } + /// + /// The metric names a birth contributes to the discoverable address space — which is + /// what the rediscovery change gate must compare, not the raw wire names. + /// + /// + /// Gitea #515: a structured metric contributes its flattened MEMBERS and not itself. Comparing + /// raw wire names instead would make the gate blind to the change that matters most for a UDT: + /// a template gaining or losing a member leaves the parent metric's name identical, so the + /// signature would not move and the operator would never be told the tree changed. + /// + private static IEnumerable DiscoverableNames(SparkplugPayload payload) + { + foreach (var metric in payload.Metrics) + { + if (metric.Name is not { } name) + { + continue; + } + + if (metric.ValueKind != SparkplugValueKind.Structured) + { + yield return name; + continue; + } + + foreach (var member in metric.MemberList) + { + yield return $"{name}{SparkplugFlattener.MemberSeparator}{member.Path}"; + } + } + } + // ---- sequence + rebirth ---- private SequenceTracker TrackerFor(SparkplugNodeKey node) => diff --git a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Mqtt/Sparkplug/SparkplugPrimaryHost.cs b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Mqtt/Sparkplug/SparkplugPrimaryHost.cs new file mode 100644 index 00000000..7911c872 --- /dev/null +++ b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Mqtt/Sparkplug/SparkplugPrimaryHost.cs @@ -0,0 +1,470 @@ +using Microsoft.Extensions.Logging; +using ZB.MOM.WW.OtOpcUa.Core.Abstractions; + +namespace ZB.MOM.WW.OtOpcUa.Driver.Mqtt.Sparkplug; + +/// +/// Claims the Sparkplug Primary Host Application identity for this node — the three-part contract +/// promises and, until Gitea #511, did not +/// deliver: a retained ONLINE while the claim is held, a matching retained OFFLINE +/// registered as the MQTT Last Will, and an explicit OFFLINE on a clean stand-down. +/// +/// +/// +/// The claim is a whole dedicated connection, not a message. A Will can only be +/// registered in the CONNECT packet (see ), so a claim that must be +/// acquired and released at runtime cannot live on the driver's shared ingest session — that +/// session connects once and stays up, and its Will could be neither added on gaining the role +/// nor withdrawn on losing it. This type therefore opens its own connection when the +/// node gains Primary and closes it when the node loses Primary. The connection's lifetime +/// is the claim, which is what makes every leg of the contract fall out for free: +/// +/// the Will is registered because the connection was opened to hold the claim; +/// a kill -9 drops the connection, so the broker publishes the Will; +/// a clean stand-down publishes OFFLINE and then disconnects, so the retained +/// message is correct before the Will could ever fire. +/// +/// +/// +/// Why not rebuild the ingest connection instead. That was the shape first sketched on +/// the issue, and it works, but it drops every subscription and the whole birth cache on each +/// role change — producing a data gap on the Secondary→Primary transition, which is precisely +/// the moment a plant most needs values flowing. A second connection costs one extra client id +/// and leaves ingest untouched. +/// +/// +/// Primary-gated, by owner decision (2026-07-31). Both nodes of a redundant pair +/// subscribe independently — MQTT fan-out is per-connection and the Primary gate applies +/// downstream of ingest — so without gating, both would claim the same host id. The sharp +/// failure is not the two fighting over the retained STATE: it is that the +/// Secondary's death would publish OFFLINE, edge nodes would stop publishing, +/// and the surviving Primary would be perfectly healthy throughout. Redundancy would cause the +/// outage the mechanism exists to prevent. The gate is +/// , sampled on a timer. +/// +/// +/// Accepted trade: a brief edge-visible OFFLINE window during failover, while the +/// old Primary stands down and the new one connects and republishes. That is a genuine state +/// transition — for a window there really is no host — not a glitch to paper over. +/// +/// +/// Reconnect re-asserts the claim. The dedicated connection carries the same supervisor +/// as the ingest one, and Reconnected republishes the birth. Without that, a broker +/// restart would leave the host connected but its retained ONLINE gone — a live host +/// invisible to every edge node, with nothing in any log to say so. +/// +/// +public sealed class SparkplugPrimaryHost : IAsyncDisposable +{ + /// + /// Appended to the driver's configured client id for the STATE connection. Two MQTT clients + /// presenting one client id is not reported as an error — the broker silently evicts the older + /// one — so ingest and the host claim would take turns killing each other forever. + /// + internal const string ClientIdSuffix = "-spbhost"; + + /// How often the role gate is sampled. Bounds the failover republish latency. + private static readonly TimeSpan RoleSampleInterval = TimeSpan.FromSeconds(1); + + private readonly MqttDriverOptions _options; + private readonly string _hostId; + private readonly string _driverId; + private readonly IDriverPrimaryRoleSource _roleSource; + private readonly ILogger? _logger; + private readonly TimeProvider _timeProvider; + + /// Serializes claim/release against each other and against . + private readonly SemaphoreSlim _gate = new(1, 1); + + private CancellationTokenSource? _loopCts; + + /// The live claim, or null when this node is not currently the primary host. + private MqttConnection? _connection; + + /// The session bodies backing ; non-null exactly when it is. + private SparkplugStateSession? _session; + + private int _disposed; + + /// Creates a primary-host claim. Opens nothing — does that. + /// Broker connection settings; the host id comes from its Sparkplug section. + /// Driver instance id, for log correlation. + /// The Primary data-plane verdict this claim is gated on. + /// Optional logger. + /// Clock seam; defaults to . + /// + /// The options carry no Sparkplug host id — exists so a caller never has + /// to hit this. + /// + public SparkplugPrimaryHost( + MqttDriverOptions options, + string driverId, + IDriverPrimaryRoleSource roleSource, + ILogger? logger = null, + TimeProvider? timeProvider = null) + { + ArgumentNullException.ThrowIfNull(options); + ArgumentException.ThrowIfNullOrWhiteSpace(driverId); + ArgumentNullException.ThrowIfNull(roleSource); + + var hostId = options.Sparkplug?.HostId; + if (string.IsNullOrWhiteSpace(hostId)) + { + throw new ArgumentException( + "A Sparkplug primary-host claim needs a hostId; check CanClaim first.", nameof(options)); + } + + _options = options; + _hostId = hostId.Trim(); + _driverId = driverId; + _roleSource = roleSource; + _logger = logger; + _timeProvider = timeProvider ?? TimeProvider.System; + } + + /// + /// Whether asks for — and can support — a primary-host claim: + /// Sparkplug mode, actAsPrimaryHost set, and a non-blank hostId. + /// + /// The driver options to test. + /// when a claim should be made. + /// + /// The blank-host-id case is a configuration error the AdminUI form already refuses, but the + /// driver must not fault on it: the config could have been authored before that validation + /// existed. It is reported by and the flag is treated as off. + /// + public static bool CanClaim(MqttDriverOptions options) => + options is { Mode: MqttMode.SparkplugB, Sparkplug: { ActAsPrimaryHost: true } spb } + && !string.IsNullOrWhiteSpace(spb.HostId); + + /// + /// Describes why an actAsPrimaryHost request cannot be honoured, or null when it can be + /// (or was never made). + /// + /// The driver options to test. + /// An operator-facing reason, or . + public static string? DescribeUnusable(MqttDriverOptions options) + { + if (options?.Sparkplug is not { ActAsPrimaryHost: true } spb) + { + return null; + } + + if (options.Mode != MqttMode.SparkplugB) + { + return "actAsPrimaryHost is set but the driver is not in SparkplugB mode; there is no host state to publish."; + } + + return string.IsNullOrWhiteSpace(spb.HostId) + ? "actAsPrimaryHost is set but hostId is blank; a host claim has no topic to publish on." + : null; + } + + /// The STATE topic this claim publishes on. + public string StateTopic => SparkplugTopic.FormatState(_hostId); + + /// + /// Whether the claim is currently held — i.e. a retained ONLINE has been published and a + /// Will is armed behind it. + /// + public bool IsClaimed => Volatile.Read(ref _connection) is not null; + + /// + /// Test seam: how many times a claim connect has been attempted, successful or not. + /// Always ignored in production. + /// + /// + /// Without this, "a Secondary does not claim" is a vacuous assertion. The obvious test — + /// start a Secondary and assert is false — passes identically whether + /// the gate works or whether the gate is deleted and the connect simply fails against a broker + /// the test cannot reach. Counting attempts separates "did not try" from "tried and failed", + /// which is the whole distinction the gate is. + /// + internal int ClaimAttemptsForTests => Volatile.Read(ref _claimAttempts); + + private int _claimAttempts; + + /// + /// Test seam: kills the claim connection the way a crashed process does — no clean + /// DISCONNECT and no explicit stand-down publish — so the broker delivers the Last Will. + /// See for why this cannot be reached through the + /// ordinary lifecycle. + /// + /// A task completing once the connection is gone. + internal async Task AbandonClaimForTests() + { + // Stop the reconcile loop first, or the next tick sees no connection, decides the claim is + // lost, and re-claims — republishing ONLINE over the Will this is meant to observe. + var loop = Interlocked.Exchange(ref _loopCts, null); + if (loop is not null) + { + await loop.CancelAsync().ConfigureAwait(false); + loop.Dispose(); + } + + var connection = Interlocked.Exchange(ref _connection, null); + _session = null; + if (connection is not null) + { + await connection.AbandonForTests().ConfigureAwait(false); + } + } + + /// + /// Starts the role-reconcile loop. Returns once the first reconcile has run, so a caller that + /// starts as Primary has published its ONLINE before this completes. + /// + /// Cancels the initial reconcile. + /// A task completing after the first reconcile. + public async Task StartAsync(CancellationToken cancellationToken) + { + ObjectDisposedException.ThrowIf(Volatile.Read(ref _disposed) != 0, this); + + var cts = new CancellationTokenSource(); + var previous = Interlocked.Exchange(ref _loopCts, cts); + if (previous is not null) + { + await previous.CancelAsync().ConfigureAwait(false); + previous.Dispose(); + } + + // Reconcile once inline. A failure here is logged and swallowed, never rethrown: the driver's + // ingest path is the data plane, and refusing to start it because a broker would not accept a + // host claim would trade every tag's value for an advisory message. The loop retries. + await ReconcileAsync(cancellationToken).ConfigureAwait(false); + + _ = Task.Run(() => ReconcileLoopAsync(cts.Token), CancellationToken.None); + } + + /// + /// Stands the claim down cleanly: publishes an explicit retained OFFLINE, then + /// disconnects. Idempotent, and safe to call when no claim is held. + /// + /// Bounds the stand-down publish. + /// A task completing when the claim is released. + public async Task StopAsync(CancellationToken cancellationToken) + { + var loop = Interlocked.Exchange(ref _loopCts, null); + if (loop is not null) + { + await loop.CancelAsync().ConfigureAwait(false); + loop.Dispose(); + } + + await ReleaseAsync("shutdown", cancellationToken).ConfigureAwait(false); + } + + /// + public async ValueTask DisposeAsync() + { + if (Interlocked.Exchange(ref _disposed, 1) != 0) + { + return; + } + + // Bounded: dispose must not hang on an unreachable broker. A stand-down we cannot publish is + // covered by the Will the moment the socket closes, which is the whole point of having one. + using var deadline = new CancellationTokenSource(TimeSpan.FromSeconds(_options.ConnectTimeoutSeconds)); + try + { + await StopAsync(deadline.Token).ConfigureAwait(false); + } + catch (Exception ex) + { + _logger?.LogDebug( + ex, "MQTT driver '{DriverId}': Sparkplug primary-host stand-down failed during dispose.", _driverId); + } + + _gate.Dispose(); + } + + private async Task ReconcileLoopAsync(CancellationToken ct) + { + try + { + while (!ct.IsCancellationRequested) + { + await Task.Delay(RoleSampleInterval, _timeProvider, ct).ConfigureAwait(false); + await ReconcileAsync(ct).ConfigureAwait(false); + } + } + catch (OperationCanceledException) + { + // Shutdown. + } + catch (Exception ex) + { + // The loop must not die silently — a dead loop means the claim freezes in whatever state + // it was last in, including "claimed by a node that is no longer Primary". + _logger?.LogError( + ex, "MQTT driver '{DriverId}': Sparkplug primary-host reconcile loop stopped.", _driverId); + } + } + + /// + /// Brings the held claim into line with the current role verdict. Never throws — a transient + /// broker failure leaves the claim as it was and the next tick tries again. + /// + private async Task ReconcileAsync(CancellationToken cancellationToken) + { + try + { + if (_roleSource.ShouldActAsPrimary) + { + await ClaimAsync(cancellationToken).ConfigureAwait(false); + } + else + { + await ReleaseAsync("this node is not the redundancy Primary", cancellationToken).ConfigureAwait(false); + } + } + catch (OperationCanceledException) when (cancellationToken.IsCancellationRequested) + { + throw; + } + catch (Exception ex) + { + _logger?.LogWarning( + ex, + "MQTT driver '{DriverId}': could not reconcile the Sparkplug primary-host claim for '{HostId}'; will retry.", + _driverId, + _hostId); + } + } + + private async Task ClaimAsync(CancellationToken cancellationToken) + { + if (Volatile.Read(ref _connection) is not null) + { + return; // Already held; the connection's own supervisor keeps it that way. + } + + await _gate.WaitAsync(cancellationToken).ConfigureAwait(false); + try + { + if (_connection is not null || Volatile.Read(ref _disposed) != 0) + { + return; + } + + Interlocked.Increment(ref _claimAttempts); + + // ONE timestamp for both bodies — the Will is built here, before CONNECT, and the birth + // published after CONNACK. See SparkplugStatePayload's remarks on why they must match. + var session = SparkplugStatePayload.ForSession( + _hostId, _timeProvider.GetUtcNow().ToUnixTimeMilliseconds()); + + var connection = new MqttConnection( + _options, + _driverId, + _logger, + clientIdSuffix: ClientIdSuffix, + lastWill: new MqttLastWill( + session.Topic, session.OfflineBody, SparkplugStatePayload.StateQos, Retain: true), + // Forced clean, ignoring the driver's configured value: a broker that resumes a prior + // session for this client id may keep the OLD session's Will and discard the one this + // CONNECT carries — leaving a claim with no death certificate behind it, which is the + // single state this whole mechanism exists to make impossible. + cleanSessionOverride: true); + + // Re-assert on every reconnect. A broker restart drops retained messages; without this the + // host would be connected and believed offline by every edge node, silently. + connection.Reconnected += async ct => + { + await connection.PublishAsync( + session.Topic, session.OnlineBody, SparkplugStatePayload.StateQos, retain: true, ct) + .ConfigureAwait(false); + + _logger?.LogInformation( + "MQTT driver '{DriverId}': re-published Sparkplug primary-host ONLINE for '{HostId}' after reconnect.", + _driverId, + _hostId); + }; + + await connection.ConnectAsync(cancellationToken).ConfigureAwait(false); + + try + { + await connection.PublishAsync( + session.Topic, session.OnlineBody, SparkplugStatePayload.StateQos, retain: true, cancellationToken) + .ConfigureAwait(false); + } + catch + { + // A connection whose birth never landed is worse than no connection: it holds a Will + // that will publish OFFLINE for a claim that was never asserted, and it looks claimed + // to IsClaimed. Tear it down and let the next tick retry from scratch. + await connection.DisposeAsync().ConfigureAwait(false); + throw; + } + + _session = session; + Volatile.Write(ref _connection, connection); + + _logger?.LogInformation( + "MQTT driver '{DriverId}': claimed Sparkplug primary host '{HostId}' — retained ONLINE published on '{Topic}', " + + "OFFLINE armed as the Last Will.", + _driverId, + _hostId, + session.Topic); + } + finally + { + _gate.Release(); + } + } + + private async Task ReleaseAsync(string reason, CancellationToken cancellationToken) + { + if (Volatile.Read(ref _connection) is null) + { + return; + } + + await _gate.WaitAsync(CancellationToken.None).ConfigureAwait(false); + try + { + var connection = Interlocked.Exchange(ref _connection, null); + var session = _session; + _session = null; + if (connection is null || session is null) + { + return; + } + + try + { + // Explicit OFFLINE before disconnecting. The Will would eventually say the same thing, + // but only after the broker's keep-alive timeout notices the socket is gone — up to + // 1.5 keep-alive periods of edge nodes believing a host that has already stood down is + // still online. Publishing it makes the transition immediate. + await connection.PublishAsync( + session.Topic, session.OfflineBody, SparkplugStatePayload.StateQos, retain: true, cancellationToken) + .ConfigureAwait(false); + + _logger?.LogInformation( + "MQTT driver '{DriverId}': released Sparkplug primary host '{HostId}' ({Reason}) — retained OFFLINE published.", + _driverId, + _hostId, + reason); + } + catch (Exception ex) + { + // Not fatal, and not silent. The Will still covers this: dropping the connection below + // makes the broker publish the identical OFFLINE body. The only thing lost is promptness. + _logger?.LogWarning( + ex, + "MQTT driver '{DriverId}': could not publish the Sparkplug primary-host OFFLINE for '{HostId}'; " + + "the Last Will will deliver it when the session drops.", + _driverId, + _hostId); + } + + await connection.DisposeAsync().ConfigureAwait(false); + } + finally + { + _gate.Release(); + } + } +} diff --git a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Mqtt/ZB.MOM.WW.OtOpcUa.Driver.Mqtt.csproj b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Mqtt/ZB.MOM.WW.OtOpcUa.Driver.Mqtt.csproj index 79edc634..2a88db4e 100644 --- a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Mqtt/ZB.MOM.WW.OtOpcUa.Driver.Mqtt.csproj +++ b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Mqtt/ZB.MOM.WW.OtOpcUa.Driver.Mqtt.csproj @@ -25,6 +25,10 @@ + + diff --git a/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Components/Shared/Drivers/Forms/MqttDriverForm.razor b/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Components/Shared/Drivers/Forms/MqttDriverForm.razor index cb18f6bc..bef3d0d3 100644 --- a/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Components/Shared/Drivers/Forms/MqttDriverForm.razor +++ b/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Components/Shared/Drivers/Forms/MqttDriverForm.razor @@ -225,7 +225,7 @@
-
Sparkplug Host Application identity. Receive-only in this version.
+
Sparkplug Host Application identity — the spBv1.0/STATE/{hostId} topic. Required to act as Primary Host.
@@ -237,25 +237,42 @@
@if (_form.SparkplugActAsPrimaryHost) { - @* Shown rather than the flag being hidden: an operator who needs a primary host must - learn it is absent HERE, not from a plant that never sees a STATE message. The - driver logs the same warning at startup so the two cannot drift. *@ -
- + } + else + { +
+ +
+ } }
@@ -263,7 +280,6 @@ Sparkplug tags bind by the group / edge node / device / metric tuple, not by topic — author them with Browse device… on a device under this driver, which builds its tree from observed birth certificates. - MQTT tags are read-only (write-through is not implemented).
} diff --git a/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Components/Shared/Drivers/Forms/MqttDriverFormModel.cs b/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Components/Shared/Drivers/Forms/MqttDriverFormModel.cs index a003bc63..ebc3c40f 100644 --- a/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Components/Shared/Drivers/Forms/MqttDriverFormModel.cs +++ b/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Components/Shared/Drivers/Forms/MqttDriverFormModel.cs @@ -142,17 +142,23 @@ public sealed class MqttDriverFormModel public string SparkplugGroupId { get; set; } = ""; /// - /// Sparkplug mode: the Host Application identity. Optional, and receive-only in this - /// version — see . + /// Sparkplug mode: the Host Application identity — the spBv1.0/STATE/{hostId} topic the + /// driver observes, and publishes on when is set. + /// Optional for observation; required to claim. /// public string SparkplugHostId { get; set; } = ""; /// - /// Sparkplug mode: claim the Primary Host Application role. Not implemented — STATE - /// publishing is out of scope, and the driver logs a warning rather than being silently inert - /// when this is set. Surfaced (rather than hidden) so an operator who needs it learns that from - /// the form instead of from a quiet plant. + /// Sparkplug mode: claim the Primary Host Application role (Gitea #511) — a retained + /// ONLINE, a matching OFFLINE armed as the MQTT Last Will, and an explicit + /// OFFLINE on clean shutdown. /// + /// + /// Primary-gated: only the node holding the redundancy Primary role claims it, so the two + /// nodes of a pair cannot fight over one host id. At most one primary host may exist per host id + /// per broker, which the form states — it is not enforceable from here, since a second + /// deployment pointing at the same broker is invisible to this one. + /// public bool SparkplugActAsPrimaryHost { get; set; } /// Sparkplug mode: answer a detected sequence-number gap with a rebirth request (NCMD). diff --git a/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Components/Shared/Drivers/Forms/SparkplugPrimaryHostAdvice.cs b/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Components/Shared/Drivers/Forms/SparkplugPrimaryHostAdvice.cs new file mode 100644 index 00000000..1601d9aa --- /dev/null +++ b/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Components/Shared/Drivers/Forms/SparkplugPrimaryHostAdvice.cs @@ -0,0 +1,35 @@ +namespace ZB.MOM.WW.OtOpcUa.AdminUI.Components.Shared.Drivers.Forms; + +/// +/// The rule deciding which primary-host advisory MqttDriverForm shows when +/// Act as Primary Host is on (Gitea #511). +/// +/// +/// +/// A plain static rather than markup logic, because this repo has no bUnit. A condition +/// written inline in a .razor file is unreachable from any test, and the failure mode +/// here is specific: an operator who ticks the box with a blank host id must see a +/// refusal, not the ordinary "here is what will happen" note. Getting that backwards +/// tells them a claim is being made when the driver is in fact declining to make one. +/// +/// +/// Deliberately mirrors SparkplugPrimaryHost.DescribeUnusable's blank-host-id arm. The +/// mode arm is not duplicated: this panel only renders inside the SparkplugB branch of the +/// form, so mode cannot be wrong here. +/// +/// +public static class SparkplugPrimaryHostAdvice +{ + /// + /// Describes why a primary-host claim cannot be made for , or + /// when it can. + /// + /// The Sparkplug host id as typed into the form. + /// An operator-facing reason, or . + public static string? DescribeBlockingProblem(string? hostId) => + string.IsNullOrWhiteSpace(hostId) + ? "A Sparkplug host claim is published on spBv1.0/STATE/{hostId}, so it needs a Host ID. " + + "Set one above, or turn this off — the driver will otherwise log a warning at startup " + + "and publish nothing." + : null; +} diff --git a/src/Server/ZB.MOM.WW.OtOpcUa.Host/Drivers/DriverFactoryBootstrap.cs b/src/Server/ZB.MOM.WW.OtOpcUa.Host/Drivers/DriverFactoryBootstrap.cs index 3b743031..4943dee6 100644 --- a/src/Server/ZB.MOM.WW.OtOpcUa.Host/Drivers/DriverFactoryBootstrap.cs +++ b/src/Server/ZB.MOM.WW.OtOpcUa.Host/Drivers/DriverFactoryBootstrap.cs @@ -5,6 +5,7 @@ using ZB.MOM.WW.OtOpcUa.Core.Abstractions; using ZB.MOM.WW.OtOpcUa.Core.Hosting; using ZB.MOM.WW.OtOpcUa.Core.Resilience; using ZB.MOM.WW.Secrets.Abstractions; +using ZB.MOM.WW.OtOpcUa.Runtime.Redundancy; namespace ZB.MOM.WW.OtOpcUa.Host.Drivers; @@ -45,6 +46,19 @@ public static class DriverFactoryBootstrap /// The same service collection, for chaining. public static IServiceCollection AddOtOpcUaDriverFactories(this IServiceCollection services) { + // Gitea #511: the Primary data-plane verdict the MQTT driver's Sparkplug host claim is gated on. + // + // Registered HERE as well as in AddOtOpcUaRuntime, and both sides TryAdd the same concrete + // RedundancyRoleView, so whichever extension runs first creates it and the other resolves the + // SAME instance. That is deliberately not a defensive duplicate: the resolve below is + // GetRequiredService (a null would silently fall the driver back to "claim unconditionally", + // which on a redundant pair is the dual-claim this mechanism exists to prevent), so without + // this line the registration becomes order-dependent on two extensions a caller is free to + // invoke in either order — and any graph that wires driver factories without the full runtime + // fails at resolution. Two Host integration suites do exactly that, and caught it. + services.TryAddSingleton(); + services.TryAddSingleton(sp => sp.GetRequiredService()); + services.AddSingleton(sp => { var registry = new DriverFactoryRegistry(); @@ -55,7 +69,16 @@ public static class DriverFactoryBootstrap // The Galaxy driver resolves its Gateway.ApiKeySecretRef secret: arm through the // shared ISecretResolver (registered unconditionally by AddZbSecrets on every node). var secretResolver = sp.GetRequiredService(); - Register(registry, loggerFactory, secretResolver, scriptRoot); + // Gitea #511: the MQTT driver's Sparkplug primary-host claim is Primary-gated, and unlike + // every other gated operation it is a STANDING assertion on the driver's own connection — + // there is no inbound message for DriverHostActor to intercept — so the verdict has to + // reach the driver itself. Registered unconditionally on driver-role nodes by + // AddOtOpcUaRuntime. GetRequiredService, not GetService: a null here silently falls the + // driver back to "claim unconditionally", which on a redundant pair is the dual-claim this + // whole mechanism exists to prevent, and it would fail as a plant outage rather than as a + // startup error. + var primaryRoleSource = sp.GetRequiredService(); + Register(registry, loggerFactory, secretResolver, primaryRoleSource, scriptRoot); return registry; }); services.AddSingleton(sp => @@ -142,6 +165,7 @@ public static class DriverFactoryBootstrap DriverFactoryRegistry registry, ILoggerFactory? loggerFactory, ISecretResolver secretResolver, + IDriverPrimaryRoleSource primaryRoleSource, ZB.MOM.WW.OtOpcUa.Core.Scripting.ScriptRootLogger? scriptRoot = null) { Driver.AbCip.AbCipDriverFactoryExtensions.Register(registry); @@ -150,7 +174,7 @@ public static class DriverFactoryBootstrap Driver.FOCAS.FocasDriverFactoryExtensions.Register(registry); Driver.Galaxy.GalaxyDriverFactoryExtensions.Register(registry, secretResolver, loggerFactory); Driver.Modbus.ModbusDriverFactoryExtensions.Register(registry, loggerFactory); - Driver.Mqtt.MqttDriverFactoryExtensions.Register(registry, loggerFactory); + Driver.Mqtt.MqttDriverFactoryExtensions.Register(registry, loggerFactory, primaryRoleSource); // Tier A (the Register default): fully managed — HttpClient + System.Xml.Linq, no native SDK, // no COM. Tier C is the only tier that arms process-level recycle (MemoryRecycle hard-breach / // ScheduledRecycleScheduler), which would be wrong here: the driver is in-process and killing diff --git a/src/Server/ZB.MOM.WW.OtOpcUa.Runtime/Drivers/DriverHostActor.cs b/src/Server/ZB.MOM.WW.OtOpcUa.Runtime/Drivers/DriverHostActor.cs index 5359cfa5..6d257dbd 100644 --- a/src/Server/ZB.MOM.WW.OtOpcUa.Runtime/Drivers/DriverHostActor.cs +++ b/src/Server/ZB.MOM.WW.OtOpcUa.Runtime/Drivers/DriverHostActor.cs @@ -273,6 +273,19 @@ public sealed class DriverHostActor : ReceiveActor, IWithTimers /// today, the alarm store-and-forward drain. Null on nodes that do not wire one. private readonly IRedundancyRoleView? _redundancyRoleView; + /// + /// Where the Primary data-plane verdict is published for drivers that must know it + /// directly (Gitea #511). Null on nodes that wire no redundancy manager. + /// + /// + /// Note the different verdict from : that one carries + /// , which OPENS on an unknown role. A + /// Sparkplug host-id claim is an exclusive assertion on a shared broker, so it takes the + /// data-plane verdict, which closes. Publishing the drain verdict here would let both nodes of + /// a pair claim the host id during every boot window. + /// + private readonly IDriverPrimaryRoleSource? _primaryRoleSource; + /// /// Host of this node's LocalDb replication partner — the only node that holds a copy of this /// node's alarm queue, and therefore the only node it may ever stand down in favour of. @@ -408,7 +421,8 @@ public sealed class DriverHostActor : ReceiveActor, IWithTimers bool fetchAndCacheMode = false, IDeploymentArtifactFetcher? artifactFetcher = null, IAlarmStateStore? alarmStateStore = null, - ITelemetryLocalHub? telemetryHub = null) => + ITelemetryLocalHub? telemetryHub = null, + IDriverPrimaryRoleSource? primaryRoleSource = null) => // WARNING: this forwarding list is POSITIONAL, and Props.Create compiles it into an // expression tree. Six IActorRef? parameters and several interface-typed ones mean a // mis-ordered argument is usually type-compatible and therefore compiles clean, then binds @@ -419,7 +433,7 @@ public sealed class DriverHostActor : ReceiveActor, IWithTimers healthPublisher, virtualTagEvaluator, historyWriter, virtualTagHostOverride, loggerFactory, scriptRootLogger, scriptedAlarmHostOverride, invokerFactory, driverMemberCountProvider, deploymentArtifactCache, redundancyRoleView, replicationPeerHost, fetchAndCacheMode, artifactFetcher, - alarmStateStore, telemetryHub)); + alarmStateStore, telemetryHub, primaryRoleSource)); /// Initializes a new DriverHostActor with the specified dependencies. /// Database context factory for configuration database access. @@ -460,6 +474,12 @@ public sealed class DriverHostActor : ReceiveActor, IWithTimers /// Per-cluster mesh Phase 5: optional node-local live-telemetry hub each /// Primary-gated native-alarm transition is also emitted into, and which is threaded into the spawned /// VirtualTag + ScriptedAlarm hosts. Null (admin-only nodes / tests) skips the emit and passes null down. + /// Gitea #511: where the Primary data-plane verdict is published + /// for drivers that assert a standing exclusive identity on a shared external system — today the MQTT + /// driver's Sparkplug primary-host STATE claim. Deliberately a separate parameter from + /// even though the production object implements both: a test that + /// supplies only a role-view double must not silently also satisfy this, or its primary-role assertions + /// pass without the publish ever happening. Null on nodes that wire no redundancy manager. public DriverHostActor( IDbContextFactory? dbFactory, CommonsNodeId localNode, @@ -483,11 +503,13 @@ public sealed class DriverHostActor : ReceiveActor, IWithTimers bool fetchAndCacheMode = false, IDeploymentArtifactFetcher? artifactFetcher = null, IAlarmStateStore? alarmStateStore = null, - ITelemetryLocalHub? telemetryHub = null) + ITelemetryLocalHub? telemetryHub = null, + IDriverPrimaryRoleSource? primaryRoleSource = null) { _telemetryHub = telemetryHub; _deploymentArtifactCache = deploymentArtifactCache; _redundancyRoleView = redundancyRoleView; + _primaryRoleSource = primaryRoleSource; _replicationPeerHost = replicationPeerHost; _fetchAndCacheMode = fetchAndCacheMode; _artifactFetcher = artifactFetcher; @@ -1331,6 +1353,12 @@ public sealed class DriverHostActor : ReceiveActor, IWithTimers _redundancyRoleView?.Publish( PrimaryGatePolicy.ShouldDrainAlarmHistory(_localRole, queueSharingPeerIsPrimary: peerIsPrimary)); + + // The DATA-PLANE verdict, deliberately — the same one HandleRouteNodeWrite gates on, not the + // drain verdict published immediately above. A driver reading this claims an exclusive identity + // on a shared external system (Gitea #511: the Sparkplug host id), where two claimants is the + // harm, so an unknown role with a driver peer present must deny. + _primaryRoleSource?.PublishPrimaryRole(ShouldServiceAsPrimary()); } private void Stale() diff --git a/src/Server/ZB.MOM.WW.OtOpcUa.Runtime/Redundancy/IRedundancyRoleView.cs b/src/Server/ZB.MOM.WW.OtOpcUa.Runtime/Redundancy/IRedundancyRoleView.cs index a9c41dff..b450e9e2 100644 --- a/src/Server/ZB.MOM.WW.OtOpcUa.Runtime/Redundancy/IRedundancyRoleView.cs +++ b/src/Server/ZB.MOM.WW.OtOpcUa.Runtime/Redundancy/IRedundancyRoleView.cs @@ -1,3 +1,4 @@ +using ZB.MOM.WW.OtOpcUa.Core.Abstractions; using ZB.MOM.WW.OtOpcUa.Runtime.Drivers; namespace ZB.MOM.WW.OtOpcUa.Runtime.Redundancy; @@ -39,8 +40,22 @@ public interface IRedundancyRoleView void Publish(bool shouldDrainAlarmHistory); } -/// Thread-safe backed by a volatile field. -public sealed class RedundancyRoleView : IRedundancyRoleView +/// +/// Thread-safe + , each +/// backed by its own volatile field. +/// +/// +/// +/// Two verdicts, one object, on purpose. Both are published from the same +/// DriverHostActor snapshot handler and both are read off-actor, so a second class +/// would duplicate the publish plumbing to hold a second bool. They are kept as distinct +/// properties rather than collapsed into one because they are genuinely different +/// decisions from different policies — see 's remarks on why +/// the drain gate must open on an unknown role where the data-plane gate closes. Collapsing +/// them is the bug that live gate already found once. +/// +/// +public sealed class RedundancyRoleView : IRedundancyRoleView, IDriverPrimaryRoleSource { // Seeded with the decision for "role unknown" rather than a bare false. An unpublished view and a // node whose role nothing ever reports are genuinely the same situation, and they must behave the @@ -49,9 +64,22 @@ public sealed class RedundancyRoleView : IRedundancyRoleView private volatile bool _shouldDrainAlarmHistory = PrimaryGatePolicy.ShouldDrainAlarmHistory(localRole: null, queueSharingPeerIsPrimary: false); + // Seeded from the same policy that will later publish here, for the unknown-role/no-peer case: + // a node nothing has reported a role for, on a cluster with no driver peer, is a single-node + // deployment and may claim. The moment a snapshot arrives this is replaced by the real verdict, + // and a multi-driver cluster's unknown role denies. + private volatile bool _shouldActAsPrimary = + PrimaryGatePolicy.ShouldServiceAsPrimary(localRole: null, driverMemberCount: 1); + /// public bool ShouldDrainAlarmHistory => _shouldDrainAlarmHistory; + /// + public bool ShouldActAsPrimary => _shouldActAsPrimary; + /// public void Publish(bool shouldDrainAlarmHistory) => _shouldDrainAlarmHistory = shouldDrainAlarmHistory; + + /// + public void PublishPrimaryRole(bool shouldActAsPrimary) => _shouldActAsPrimary = shouldActAsPrimary; } diff --git a/src/Server/ZB.MOM.WW.OtOpcUa.Runtime/ServiceCollectionExtensions.cs b/src/Server/ZB.MOM.WW.OtOpcUa.Runtime/ServiceCollectionExtensions.cs index 12648380..793088f0 100644 --- a/src/Server/ZB.MOM.WW.OtOpcUa.Runtime/ServiceCollectionExtensions.cs +++ b/src/Server/ZB.MOM.WW.OtOpcUa.Runtime/ServiceCollectionExtensions.cs @@ -75,6 +75,22 @@ public static class ServiceCollectionExtensions // telemetry (never DPS) to the Phase-5 gRPC streaming service central dials in on. AddOtOpcUaRuntime // runs only inside Program.cs's hasDriver block, so this lands on driver-role nodes only. services.TryAddSingleton(); + + // Gitea #511: the Primary data-plane verdict, published for drivers that assert a standing + // exclusive identity on a shared external system (today the MQTT driver's Sparkplug host id). + // + // Registered HERE, unconditionally on every driver-role node, and NOT alongside the alarm + // drain's IRedundancyRoleView — which AddAlarmHistorian registers only when that section is + // enabled. Piggybacking on it would leave a redundant pair with no alarm historian configured + // with no primary-role source at all, and the driver's documented no-source fallback is + // "claim" — so both nodes of that pair would claim the same host id and their Wills would + // fight over the retained STATE. Exactly the failure #511 exists to prevent, reached by a + // registration accident in an unrelated section. + // + // Both interfaces are forwarded to ONE RedundancyRoleView instance, so a node that has both + // publishes two verdicts into the same object rather than into two that disagree. + services.TryAddSingleton(); + services.TryAddSingleton(sp => sp.GetRequiredService()); return services; } @@ -117,8 +133,10 @@ public static class ServiceCollectionExtensions // The view is a plain singleton so it exists even on nodes whose DriverHostActor never // publishes to it — an unpublished view reads as "no peer, drain", which is the correct - // posture for a deployment that runs no redundancy at all. - services.TryAddSingleton(); + // posture for a deployment that runs no redundancy at all. Forwarded to the same instance + // AddOtOpcUaRuntime registered for IDriverPrimaryRoleSource: one object, two verdicts. + services.TryAddSingleton(); + services.TryAddSingleton(sp => sp.GetRequiredService()); // THE GATE ONLY APPLIES TO A REPLICATED QUEUE. Standing down is only ever safe because some // other node holds the same rows and will send them instead — and the only node that does is @@ -499,7 +517,11 @@ public static class ServiceCollectionExtensions // Phase 5: the node-local live-telemetry hub (registered in AddOtOpcUaRuntime). The host // emits its Primary-gated native-alarm transitions into it and threads it into the // VirtualTag + ScriptedAlarm hosts it spawns. - telemetryHub: resolver.GetService()), + telemetryHub: resolver.GetService(), + // Gitea #511: where this actor publishes its Primary DATA-PLANE verdict for drivers + // that claim an exclusive external identity. Registered unconditionally by + // AddOtOpcUaRuntime, unlike redundancyRoleView above — see the comment there. + primaryRoleSource: resolver.GetService()), DriverHostActorName); registry.Register(driverHost); diff --git a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Mqtt.IntegrationTests/MqttPrimaryHostLiveTests.cs b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Mqtt.IntegrationTests/MqttPrimaryHostLiveTests.cs new file mode 100644 index 00000000..fc9f6825 --- /dev/null +++ b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Mqtt.IntegrationTests/MqttPrimaryHostLiveTests.cs @@ -0,0 +1,379 @@ +using System.Buffers; +using System.Collections.Concurrent; +using MQTTnet; +using Shouldly; +using Xunit; +using ZB.MOM.WW.OtOpcUa.Core.Abstractions; +using ZB.MOM.WW.OtOpcUa.Driver.Mqtt.Sparkplug; + +namespace ZB.MOM.WW.OtOpcUa.Driver.Mqtt.IntegrationTests; + +/// +/// Live gate for the Sparkplug primary-host claim (Gitea #511), against the Mosquitto fixture. +/// +/// +/// +/// The issue's acceptance criteria, leg by leg: the broker holds a retained ONLINE while +/// the claim runs; an abrupt death leaves OFFLINE (the Will fires); a clean shutdown +/// leaves OFFLINE; and the pair-node question is answered rather than left to chance — +/// a node that is not Primary claims nothing, and a role change moves the claim. +/// +/// +/// Each test uses its own host id. A Sparkplug host id is a broker-wide singleton by +/// definition, and the retained STATE outlives the test that wrote it — two tests sharing an id +/// would see each other's leftovers, and would do so intermittently depending on ordering. +/// +/// +/// Retained messages are cleaned up. A retained ONLINE for a host that no longer +/// exists is exactly the lie this feature prevents, so leaving one on a shared fixture would be +/// poor manners at best and a confusing false positive for the next run at worst. +/// +/// +[Trait("Category", "Integration")] +[Collection("MqttFixture")] +public sealed class MqttPrimaryHostLiveTests(MqttFixture fixture) +{ + private static readonly TimeSpan Timeout = TimeSpan.FromSeconds(15); + + private void SkipIfUnconfigured() => Assert.SkipWhen(fixture.SkipReason is not null, fixture.SkipReason ?? ""); + + [Fact] + public async Task A_primary_publishes_a_retained_ONLINE_that_a_late_subscriber_still_sees() + { + // Retention is the point, not an optimisation: an edge node that connects AFTER the host came + // up must still learn the host is alive. A non-retained ONLINE reaches only whoever happened to + // be subscribed at that instant, and every later joiner sees an absent host forever. + SkipIfUnconfigured(); + + var hostId = NewHostId(); + var ct = TestContext.Current.CancellationToken; + await using var host = NewHost(hostId, isPrimary: true); + + await host.StartAsync(ct); + host.IsClaimed.ShouldBeTrue("the fixture broker accepted the claim connection"); + + // Subscribe only NOW — after the birth was published — so only a retained message can satisfy it. + await using var watcher = await StateWatcher.StartAsync(fixture, hostId, ct); + var body = await watcher.WaitForOneAsync(Timeout, ct); + + SparkplugStatePayload.TryDecode(body, out var online, out var timestamp).ShouldBeTrue(); + online.ShouldBeTrue(); + timestamp.ShouldBeGreaterThan(0); + + await host.StopAsync(ct); + await ClearRetainedAsync(hostId, ct); + } + + [Fact] + public async Task An_abrupt_death_leaves_a_retained_OFFLINE_because_the_will_fires() + { + // THE leg the whole design exists for, and the one no unit test can reach: proof that the + // broker is holding our death certificate and delivers it when the socket dies without a + // goodbye. Without it a crashed node would leave a retained ONLINE asserting forever that a + // dead host is alive — strictly worse than never publishing at all. + SkipIfUnconfigured(); + + var hostId = NewHostId(); + var ct = TestContext.Current.CancellationToken; + var host = NewHost(hostId, isPrimary: true); + + await host.StartAsync(ct); + host.IsClaimed.ShouldBeTrue(); + + await using var watcher = await StateWatcher.StartAsync(fixture, hostId, ct); + var birth = await watcher.WaitForOneAsync(Timeout, ct); + SparkplugStatePayload.TryDecode(birth, out var wasOnline, out var birthTimestamp).ShouldBeTrue(); + wasOnline.ShouldBeTrue(); + + // Kill the socket with no DISCONNECT. A clean teardown would make the broker DISCARD the Will + // by protocol rule, so this test would then be observing the explicit stand-down publish and + // would pass against an implementation that never armed a Will. + await host.AbandonClaimForTests(); + + var death = await watcher.WaitForMatchAsync( + p => SparkplugStatePayload.TryDecode(p, out var o, out _) && !o, Timeout, ct); + + SparkplugStatePayload.TryDecode(death, out _, out var deathTimestamp).ShouldBeTrue(); + + // Sparkplug 3.0 pairs a host's birth and death by timestamp — an edge node may ignore an + // OFFLINE older than the ONLINE it holds. Mismatched timestamps produce two well-formed + // messages and a death certificate conformant hosts are entitled to throw away. + deathTimestamp.ShouldBe(birthTimestamp); + + await host.DisposeAsync(); + await ClearRetainedAsync(hostId, ct); + } + + [Fact] + public async Task A_clean_shutdown_leaves_a_retained_OFFLINE_without_waiting_for_the_will() + { + SkipIfUnconfigured(); + + var hostId = NewHostId(); + var ct = TestContext.Current.CancellationToken; + var host = NewHost(hostId, isPrimary: true); + + await host.StartAsync(ct); + await using var watcher = await StateWatcher.StartAsync(fixture, hostId, ct); + await watcher.WaitForMatchAsync( + p => SparkplugStatePayload.TryDecode(p, out var o, out _) && o, Timeout, ct); + + await host.StopAsync(ct); + + // Promptly — the assertion is that this arrives well inside the keep-alive window it would take + // the broker to notice a dead socket, which is why the explicit publish exists at all. + await watcher.WaitForMatchAsync( + p => SparkplugStatePayload.TryDecode(p, out var o, out _) && !o, Timeout, ct); + + await host.DisposeAsync(); + await ClearRetainedAsync(hostId, ct); + } + + [Fact] + public async Task A_node_that_is_not_primary_publishes_nothing_at_all() + { + // The pair-node answer. Not merely "does not win the race" — a Secondary must never open a + // claim connection, because the Will it would register is the hazard: its own death would then + // publish OFFLINE over a perfectly healthy Primary's ONLINE and stop the whole group. + SkipIfUnconfigured(); + + var hostId = NewHostId(); + var ct = TestContext.Current.CancellationToken; + await using var watcher = await StateWatcher.StartAsync(fixture, hostId, ct); + await using var host = NewHost(hostId, isPrimary: false); + + await host.StartAsync(ct); + host.ClaimAttemptsForTests.ShouldBe(0); + + // Give the reconcile loop several ticks to misbehave in. + await Task.Delay(TimeSpan.FromSeconds(3), ct); + + host.IsClaimed.ShouldBeFalse(); + host.ClaimAttemptsForTests.ShouldBe(0); + watcher.Count.ShouldBe(0, "a Secondary must not touch the STATE topic"); + } + + [Fact] + public async Task A_failover_moves_the_claim_offline_then_online_again() + { + // Both halves of the accepted trade, observed: losing Primary publishes OFFLINE (a genuine + // state transition — for a window there really is no host), and regaining it republishes + // ONLINE. The republish is the half that would be easy to omit; without it a node that failed + // back would run as Primary while every edge node believed it was gone. + SkipIfUnconfigured(); + + var hostId = NewHostId(); + var ct = TestContext.Current.CancellationToken; + var role = new MutableRole(isPrimary: true); + await using var host = new SparkplugPrimaryHost(Options(hostId), "live-failover", role); + + await using var watcher = await StateWatcher.StartAsync(fixture, hostId, ct); + + await host.StartAsync(ct); + await watcher.WaitForMatchAsync( + p => SparkplugStatePayload.TryDecode(p, out var o, out _) && o, Timeout, ct); + + role.IsPrimary = false; + await watcher.WaitForMatchAsync( + p => SparkplugStatePayload.TryDecode(p, out var o, out _) && !o, Timeout, ct); + await WaitUntilAsync(() => !host.IsClaimed, ct); + + var offlineSeen = watcher.CountMatching( + p => SparkplugStatePayload.TryDecode(p, out var o, out _) && !o); + + role.IsPrimary = true; + await WaitUntilAsync(() => host.IsClaimed, ct); + + // A SECOND online, after the offline — not merely "an online exists", which the birth already + // satisfied before the failover even started. + await WaitUntilAsync( + () => watcher.CountMatching(p => SparkplugStatePayload.TryDecode(p, out var o, out _) && o) >= 2, + ct); + + offlineSeen.ShouldBeGreaterThanOrEqualTo(1); + + await host.StopAsync(ct); + await ClearRetainedAsync(hostId, ct); + } + + // ---- helpers ----------------------------------------------------------------------------------- + + /// A per-test host id: the retained STATE outlives the test that wrote it. + private static string NewHostId() => $"otopcua-live-{Guid.NewGuid():N}"[..28]; + + private MqttDriverOptions Options(string hostId) => new() + { + Host = fixture.PlainHost, + Port = fixture.PlainPort, + Username = fixture.Username, + Password = fixture.Password, + UseTls = false, + ClientId = $"otopcua-live-{Guid.NewGuid():N}"[..20], + ConnectTimeoutSeconds = 10, + Mode = MqttMode.SparkplugB, + Sparkplug = new MqttSparkplugOptions + { + GroupId = "OtOpcUaSim", + HostId = hostId, + ActAsPrimaryHost = true, + }, + }; + + private SparkplugPrimaryHost NewHost(string hostId, bool isPrimary) => + new(Options(hostId), $"live-{hostId}", new MutableRole(isPrimary)); + + /// + /// Clears the retained STATE this test left behind — a zero-length retained publish is MQTT's + /// "forget this". Leaving a retained ONLINE for a host that no longer exists is precisely the + /// lie the feature prevents. + /// + private async Task ClearRetainedAsync(string hostId, CancellationToken ct) + { + var client = new MqttClientFactory().CreateMqttClient(); + try + { + await client.ConnectAsync( + new MqttClientOptionsBuilder() + .WithTcpServer(fixture.PlainHost, fixture.PlainPort) + .WithCredentials(fixture.Username, fixture.Password) + .WithClientId($"otopcua-live-clean-{Guid.NewGuid():N}"[..23]) + .WithTimeout(TimeSpan.FromSeconds(10)) + .Build(), + ct); + + await client.PublishAsync( + new MqttApplicationMessageBuilder() + .WithTopic(SparkplugTopic.FormatState(hostId)) + .WithPayload(Array.Empty()) + .WithRetainFlag(true) + .Build(), + ct); + + await client.DisconnectAsync(cancellationToken: ct); + } + finally + { + client.Dispose(); + } + } + + private static async Task WaitUntilAsync(Func condition, CancellationToken ct) + { + var deadline = DateTime.UtcNow + Timeout; + while (!condition()) + { + if (DateTime.UtcNow >= deadline) + { + throw new TimeoutException($"Condition was not satisfied within {Timeout}."); + } + + await Task.Delay(100, ct); + } + } + + /// A role source a test can flip, standing in for a redundancy failover. + private sealed class MutableRole(bool isPrimary) : IDriverPrimaryRoleSource + { + private volatile bool _isPrimary = isPrimary; + + public bool IsPrimary + { + get => _isPrimary; + set => _isPrimary = value; + } + + public bool ShouldActAsPrimary => _isPrimary; + + public void PublishPrimaryRole(bool shouldActAsPrimary) => _isPrimary = shouldActAsPrimary; + } + + /// Subscribes one host's STATE topic and records every body seen, in order. + private sealed class StateWatcher : IAsyncDisposable + { + private readonly IMqttClient _client; + private readonly ConcurrentQueue _bodies = new(); + + private StateWatcher(IMqttClient client) => _client = client; + + public int Count => _bodies.Count; + + public static async Task StartAsync(MqttFixture fixture, string hostId, CancellationToken ct) + { + var client = new MqttClientFactory().CreateMqttClient(); + var watcher = new StateWatcher(client); + + client.ApplicationMessageReceivedAsync += args => + { + watcher._bodies.Enqueue(args.ApplicationMessage.Payload.ToArray()); + return Task.CompletedTask; + }; + + await client.ConnectAsync( + new MqttClientOptionsBuilder() + .WithTcpServer(fixture.PlainHost, fixture.PlainPort) + .WithCredentials(fixture.Username, fixture.Password) + .WithClientId($"otopcua-live-state-{Guid.NewGuid():N}"[..23]) + .WithTimeout(TimeSpan.FromSeconds(10)) + .Build(), + ct); + + await client.SubscribeAsync( + new MqttClientSubscribeOptionsBuilder() + .WithTopicFilter(f => f.WithTopic(SparkplugTopic.FormatState(hostId))) + .Build(), + ct); + + return watcher; + } + + public int CountMatching(Func predicate) => _bodies.Count(b => Safe(predicate, b)); + + public async Task WaitForOneAsync(TimeSpan timeout, CancellationToken ct) + => await WaitForMatchAsync(_ => true, timeout, ct); + + public async Task WaitForMatchAsync(Func predicate, TimeSpan timeout, CancellationToken ct) + { + var deadline = DateTime.UtcNow + timeout; + while (true) + { + if (_bodies.FirstOrDefault(b => Safe(predicate, b)) is { } match) + { + return match; + } + + if (DateTime.UtcNow >= deadline) + { + throw new TimeoutException( + $"No matching STATE message within {timeout} ({_bodies.Count} seen, none matching)."); + } + + await Task.Delay(100, ct); + } + } + + private static bool Safe(Func predicate, byte[] body) + { + try { return predicate(body); } + catch { return false; } + } + + public async ValueTask DisposeAsync() + { + try + { + if (_client.IsConnected) + { + await _client.DisconnectAsync(); + } + } + catch + { + // Teardown only. + } + finally + { + _client.Dispose(); + } + } + } +} diff --git a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Mqtt.IntegrationTests/SparkplugSimulator/Program.cs b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Mqtt.IntegrationTests/SparkplugSimulator/Program.cs index db045291..87b81759 100644 --- a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Mqtt.IntegrationTests/SparkplugSimulator/Program.cs +++ b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Mqtt.IntegrationTests/SparkplugSimulator/Program.cs @@ -81,7 +81,14 @@ foreach (var node in nodes) "Filler1", new SparkplugMetricSeed("Temperature", 10, DataType.Float, 55.5f), new SparkplugMetricSeed("FillCount", 11, DataType.Int64, 1000L), - new SparkplugMetricSeed("Jammed", 12, DataType.Boolean, false)); + new SparkplugMetricSeed("Jammed", 12, DataType.Boolean, false), + + // Structured metrics for the Gitea #515 gate. They live on the DEVICE rather than the node + // deliberately: a DDATA template is the case a naive implementation gets wrong, because its + // metric carries the parent's ALIAS and no name, so the member names cannot be composed until + // the alias table has resolved the parent. + new SparkplugMetricSeed("Motor", 13, DataType.Template, BuildMotorTemplate()), + new SparkplugMetricSeed("Recipe", 14, DataType.DataSet, BuildRecipeDataSet())); edges.Add(edge); } @@ -112,3 +119,55 @@ return 0; static string Env(string name, string fallback) => Environment.GetEnvironmentVariable(name) is { Length: > 0 } value ? value : fallback; + +// A UDT instance: `Motor` with a `Speed` double, a `Running` bool and a nested `Stator` template. +// Flattens to `Motor/Speed`, `Motor/Running` and `Motor/Stator/Temp` (Gitea #515). +static Payload.Types.Template BuildMotorTemplate() +{ + var stator = new Payload.Types.Template { Version = "1", IsDefinition = false }; + stator.Metrics.Add(new Payload.Types.Metric + { + Name = "Temp", Datatype = (uint)DataType.Double, DoubleValue = 71.5, + }); + + var motor = new Payload.Types.Template { Version = "1", IsDefinition = false }; + motor.Metrics.Add(new Payload.Types.Metric + { + Name = "Speed", Datatype = (uint)DataType.Double, DoubleValue = 1500, + }); + motor.Metrics.Add(new Payload.Types.Metric + { + Name = "Running", Datatype = (uint)DataType.Boolean, BooleanValue = true, + }); + + // Nested one level: proves the recursion, and proves the `/`-joined path composes correctly rather + // than only working for flat templates. + motor.Metrics.Add(new Payload.Types.Metric + { + Name = "Stator", Datatype = (uint)DataType.Template, TemplateValue = stator, + }); + + return motor; +} + +// A two-column, two-row table. Flattens to `Recipe/Step/0`, `Recipe/Target/0`, `Recipe/Step/1`, +// `Recipe/Target/1` — the row index is always present, so a table that grows a row never renames the +// tags already authored against it. +static Payload.Types.DataSet BuildRecipeDataSet() +{ + var dataSet = new Payload.Types.DataSet { NumOfColumns = 2UL }; + dataSet.Columns.Add("Step"); + dataSet.Columns.Add("Target"); + dataSet.Types_.Add((uint)DataType.Int32); + dataSet.Types_.Add((uint)DataType.Double); + + for (var row = 0; row < 2; row++) + { + var r = new Payload.Types.DataSet.Types.Row(); + r.Elements.Add(new Payload.Types.DataSet.Types.DataSetValue { IntValue = (uint)row }); + r.Elements.Add(new Payload.Types.DataSet.Types.DataSetValue { DoubleValue = 20d + row }); + dataSet.Rows.Add(r); + } + + return dataSet; +} diff --git a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Mqtt.IntegrationTests/SparkplugSimulator/SparkplugEdgeNode.cs b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Mqtt.IntegrationTests/SparkplugSimulator/SparkplugEdgeNode.cs index 0140af0d..425cafdc 100644 --- a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Mqtt.IntegrationTests/SparkplugSimulator/SparkplugEdgeNode.cs +++ b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Mqtt.IntegrationTests/SparkplugSimulator/SparkplugEdgeNode.cs @@ -756,6 +756,18 @@ public sealed class SparkplugEdgeNode : IAsyncDisposable case DataType.File: metric.BytesValue = ByteString.CopyFrom((byte[])value); break; + + // Structured values (Gitea #515). The seed carries the already-built protobuf message + // rather than a scalar, because a Template's shape IS its value — there is nothing simpler + // to convert from, and inventing a mini-DSL here would put a second, divergent notion of + // "what a template is" between the simulator and the wire it is supposed to reproduce. + case DataType.Template: + metric.TemplateValue = (Payload.Types.Template)value; + break; + case DataType.DataSet: + metric.DatasetValue = (Payload.Types.DataSet)value; + break; + default: throw new NotSupportedException( $"The simulator does not encode Sparkplug datatype {dataType}; v1 of the driver does " @@ -824,9 +836,83 @@ public sealed class SparkplugEdgeNode : IAsyncDisposable DataType.Uint8 or DataType.Uint16 or DataType.Uint32 or DataType.Uint64 => (uint)tick, DataType.Boolean => tick % 2 == 0, DataType.String or DataType.Text or DataType.Uuid => $"tick-{tick}", + + // Structured values move too, or the live gate could not tell a flattened member that is being + // fed from one that was merely published once at birth and then went quiet. + DataType.Template => AdvanceTemplate((Payload.Types.Template)seed.Value!, tick), + DataType.DataSet => AdvanceDataSet((Payload.Types.DataSet)seed.Value!, tick), + _ => seed.Value, }; + /// + /// Rebuilds a template with its numeric members nudged, preserving names, datatypes and order. + /// + private static Payload.Types.Template AdvanceTemplate(Payload.Types.Template seed, int tick) + { + var next = new Payload.Types.Template { Version = seed.Version, IsDefinition = false }; + foreach (var member in seed.Metrics) + { + var clone = member.Clone(); + switch ((DataType)clone.Datatype) + { + case DataType.Double: + clone.DoubleValue = 1500d + (tick % 100); + break; + case DataType.Float: + clone.FloatValue = 40f + (tick % 20) / 10f; + break; + case DataType.Int32: + clone.IntValue = unchecked((uint)(-tick)); + break; + case DataType.Boolean: + clone.BooleanValue = tick % 2 == 0; + break; + default: + break; // leave anything else at its seeded value + } + + clone.Timestamp = NowMs(); + next.Metrics.Add(clone); + } + + return next; + } + + /// Rebuilds a dataset with its numeric cells nudged, preserving columns, types and shape. + private static Payload.Types.DataSet AdvanceDataSet(Payload.Types.DataSet seed, int tick) + { + var next = new Payload.Types.DataSet { NumOfColumns = seed.NumOfColumns }; + next.Columns.AddRange(seed.Columns); + next.Types_.AddRange(seed.Types_); + + for (var row = 0; row < seed.Rows.Count; row++) + { + var cloned = new Payload.Types.DataSet.Types.Row(); + for (var col = 0; col < seed.Rows[row].Elements.Count; col++) + { + var cell = seed.Rows[row].Elements[col].Clone(); + switch ((DataType)seed.Types_[col]) + { + case DataType.Double: + cell.DoubleValue = 20d + row + (tick % 10) / 10d; + break; + case DataType.Int32: + cell.IntValue = (uint)(tick + row); + break; + default: + break; + } + + cloned.Elements.Add(cell); + } + + next.Rows.Add(cloned); + } + + return next; + } + private void ConfigureTls(MqttClientTlsOptionsBuilder tls) { if (!_options.UseTls) diff --git a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Mqtt.IntegrationTests/SparkplugStructuredLiveTests.cs b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Mqtt.IntegrationTests/SparkplugStructuredLiveTests.cs new file mode 100644 index 00000000..1ccb00c5 --- /dev/null +++ b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Mqtt.IntegrationTests/SparkplugStructuredLiveTests.cs @@ -0,0 +1,332 @@ +using Org.Eclipse.Tahu.Protobuf; +using Shouldly; +using Xunit; +using ZB.MOM.WW.OtOpcUa.Core.Abstractions; +using ZB.MOM.WW.OtOpcUa.Driver.Mqtt.SparkplugSimulator; + +namespace ZB.MOM.WW.OtOpcUa.Driver.Mqtt.IntegrationTests; + +/// +/// Live gate for structured Sparkplug metrics (Gitea #515): a Template and a +/// DataSet published by a real edge node over the Mosquitto fixture, flattened into +/// addressable OPC UA tags. +/// +/// +/// +/// What this adds over the offline suite. The offline tests build decoded payloads by +/// hand, so they prove the mapping but not the wire: that a nested template really does +/// round-trip through protobuf with its member names intact, and — the case a naive +/// implementation gets wrong — that a DDATA structured metric arrives carrying the parent's +/// alias and nothing else, so the member names cannot be composed until the alias table +/// has resolved the parent. +/// +/// +/// The issue's acceptance criterion, verbatim: "a simulator publishing a Template and a +/// DataSet metric produces addressable, correctly-typed OPC UA nodes, live-verified; +/// every still-unsupported kind continues to skip with a warning." +/// +/// +[Trait("Category", "Integration")] +[Collection("MqttFixture")] +public sealed class SparkplugStructuredLiveTests(MqttFixture fixture) +{ + private const uint Good = 0x00000000u; + private static readonly TimeSpan Timeout = TimeSpan.FromSeconds(20); + + private void SkipIfUnconfigured() => Assert.SkipWhen(fixture.SkipReason is not null, fixture.SkipReason ?? ""); + + [Fact] + public async Task A_template_metric_flattens_to_addressable_tags_across_birth_and_data() + { + SkipIfUnconfigured(); + + var ct = TestContext.Current.CancellationToken; + var group = NewGroup("tmpl"); + const string node = "EdgeA"; + const string device = "Filler1"; + + const string speedPath = "Fixture/Sparkplug/Motor/Speed"; + const string runningPath = "Fixture/Sparkplug/Motor/Running"; + const string statorPath = "Fixture/Sparkplug/Motor/Stator/Temp"; + + await using var sim = new SparkplugEdgeNode(SimOptions(group, node)); + sim.SetDeviceMetrics(device, new SparkplugMetricSeed("Motor", 13, DataType.Template, MotorTemplate(speed: 1500, running: true, statorTemp: 71.5))); + + await sim.ConnectAsync(ct); + await sim.BirthAsync(ct); + + await using var driver = new MqttDriver( + SparkplugOptions( + group, + Tag(speedPath, group, node, device, "Motor/Speed"), + Tag(runningPath, group, node, device, "Motor/Running"), + Tag(statorPath, group, node, device, "Motor/Stator/Temp")), + "mqtt-live-sp-template"); + var recorder = new Recorder(driver); + + await driver.InitializeAsync(string.Empty, ct); + await driver.SubscribeAsync([speedPath, runningPath, statorPath], TimeSpan.FromSeconds(1), ct); + + // The DBIRTH is a full snapshot, so these are the birth's own member values — including the + // NESTED template's, which is what proves the `/`-joined path composes past one level. + var birth = await recorder.WaitAsync(speedPath, v => Equals(v.Value, 1500d), Timeout, ct); + birth.Snapshot.StatusCode.ShouldBe(Good); + await recorder.WaitAsync(runningPath, v => Equals(v.Value, true), Timeout, ct); + await recorder.WaitAsync(statorPath, v => Equals(v.Value, 71.5d), Timeout, ct); + + // …and now DDATA. The metric on the wire carries the PARENT's alias and no name at all, so a + // driver that flattened only at decode time — without resolving the parent alias to its name + // first — would have nothing to compose `Motor/Speed` from and would feed none of these. + await sim.PublishDeviceDataAsync( + device, [("Motor", MotorTemplate(speed: 1750, running: false, statorTemp: 68.25))], ct); + + var data = await recorder.WaitAsync(speedPath, v => Equals(v.Value, 1750d), Timeout, ct); + data.Snapshot.StatusCode.ShouldBe(Good); + data.FullReference.ShouldBe(speedPath, "values publish under the tag's RawPath, never the metric name"); + + await recorder.WaitAsync(runningPath, v => Equals(v.Value, false), Timeout, ct); + await recorder.WaitAsync(statorPath, v => Equals(v.Value, 68.25d), Timeout, ct); + + // The read path serves the same last values under the same keys. + var read = await driver.ReadAsync([speedPath, runningPath, statorPath], ct); + read.Select(r => r.StatusCode).ShouldAllBe(s => s == Good); + read[0].Value.ShouldBe(1750d); + } + + [Fact] + public async Task A_dataset_metric_flattens_to_one_tag_per_cell() + { + SkipIfUnconfigured(); + + var ct = TestContext.Current.CancellationToken; + var group = NewGroup("dset"); + const string node = "EdgeA"; + const string device = "Filler1"; + + const string step0Path = "Fixture/Sparkplug/Recipe/Step0"; + const string target1Path = "Fixture/Sparkplug/Recipe/Target1"; + + await using var sim = new SparkplugEdgeNode(SimOptions(group, node)); + sim.SetDeviceMetrics(device, new SparkplugMetricSeed("Recipe", 14, DataType.DataSet, RecipeDataSet(baseTarget: 20d))); + + await sim.ConnectAsync(ct); + await sim.BirthAsync(ct); + + await using var driver = new MqttDriver( + SparkplugOptions( + group, + // The row index is always present, including on row 0 — the naming is uniform so a + // table that grows a row never renames the tags already authored against it. + Tag(step0Path, group, node, device, "Recipe/Step/0"), + Tag(target1Path, group, node, device, "Recipe/Target/1")), + "mqtt-live-sp-dataset"); + var recorder = new Recorder(driver); + + await driver.InitializeAsync(string.Empty, ct); + await driver.SubscribeAsync([step0Path, target1Path], TimeSpan.FromSeconds(1), ct); + + // Cells are typed per COLUMN: Step is Int32, Target is Double. A dataset whose columns and + // types disagree is refused outright, so a wrong type here is not a near-miss — it is the + // difference between a typed tag and no tag. + await recorder.WaitAsync(step0Path, v => Equals(v.Value, 0), Timeout, ct); + await recorder.WaitAsync(target1Path, v => Equals(v.Value, 21d), Timeout, ct); + + await sim.PublishDeviceDataAsync(device, [("Recipe", RecipeDataSet(baseTarget: 30d))], ct); + + var target = await recorder.WaitAsync(target1Path, v => Equals(v.Value, 31d), Timeout, ct); + target.Snapshot.StatusCode.ShouldBe(Good); + } + + [Fact] + public async Task A_template_definition_is_still_skipped_rather_than_flattened() + { + // Skip-and-warn survives — the requirement the issue is explicit about. A definition names its + // members and carries no values, so flattening it would materialise tags that can never read, + // indistinguishable in the picker from an instance's. + SkipIfUnconfigured(); + + var ct = TestContext.Current.CancellationToken; + var group = NewGroup("tdef"); + const string node = "EdgeA"; + const string device = "Filler1"; + + const string speedPath = "Fixture/Sparkplug/Proto/Speed"; + const string livePath = "Fixture/Sparkplug/Proto/Live"; + + var definition = new Payload.Types.Template { Version = "1", IsDefinition = true }; + definition.Metrics.Add(new Payload.Types.Metric { Name = "Speed", Datatype = (uint)DataType.Double }); + + await using var sim = new SparkplugEdgeNode(SimOptions(group, node)); + sim.SetDeviceMetrics( + device, + new SparkplugMetricSeed("MotorType", 15, DataType.Template, definition), + + // The positive control, and it is load-bearing: without it "Speed never arrived" is + // satisfied by any failure to connect, birth or subscribe, and the test would pass against + // a driver that ingests nothing at all. + new SparkplugMetricSeed("Live", 16, DataType.Double, 42.5d)); + + await sim.ConnectAsync(ct); + await sim.BirthAsync(ct); + + await using var driver = new MqttDriver( + SparkplugOptions( + group, + Tag(speedPath, group, node, device, "MotorType/Speed"), + Tag(livePath, group, node, device, "Live")), + "mqtt-live-sp-tdef"); + var recorder = new Recorder(driver); + + await driver.InitializeAsync(string.Empty, ct); + await driver.SubscribeAsync([speedPath, livePath], TimeSpan.FromSeconds(1), ct); + + // Wait for the control FIRST, so the absence assertion below is made at a moment the birth is + // known to have been processed. + await recorder.WaitAsync(livePath, v => Equals(v.Value, 42.5d), Timeout, ct); + + recorder.Count(speedPath).ShouldBe(0, "a template definition carries no values to publish"); + + var read = await driver.ReadAsync([speedPath], ct); + read[0].StatusCode.ShouldNotBe(Good, "a tag bound to an unflattened member has never had a value"); + } + + // ---- helpers ----------------------------------------------------------------------------------- + + private static string NewGroup(string label) => $"OtOpcUaLive-{label}-{Guid.NewGuid():N}"[..40]; + + private static Payload.Types.Template MotorTemplate(double speed, bool running, double statorTemp) + { + var stator = new Payload.Types.Template { Version = "1", IsDefinition = false }; + stator.Metrics.Add(new Payload.Types.Metric + { + Name = "Temp", Datatype = (uint)DataType.Double, DoubleValue = statorTemp, + }); + + var motor = new Payload.Types.Template { Version = "1", IsDefinition = false }; + motor.Metrics.Add(new Payload.Types.Metric + { + Name = "Speed", Datatype = (uint)DataType.Double, DoubleValue = speed, + }); + motor.Metrics.Add(new Payload.Types.Metric + { + Name = "Running", Datatype = (uint)DataType.Boolean, BooleanValue = running, + }); + motor.Metrics.Add(new Payload.Types.Metric + { + Name = "Stator", Datatype = (uint)DataType.Template, TemplateValue = stator, + }); + return motor; + } + + private static Payload.Types.DataSet RecipeDataSet(double baseTarget) + { + var dataSet = new Payload.Types.DataSet { NumOfColumns = 2UL }; + dataSet.Columns.Add("Step"); + dataSet.Columns.Add("Target"); + dataSet.Types_.Add((uint)DataType.Int32); + dataSet.Types_.Add((uint)DataType.Double); + + for (var row = 0; row < 2; row++) + { + var r = new Payload.Types.DataSet.Types.Row(); + r.Elements.Add(new Payload.Types.DataSet.Types.DataSetValue { IntValue = (uint)row }); + r.Elements.Add(new Payload.Types.DataSet.Types.DataSetValue { DoubleValue = baseTarget + row }); + dataSet.Rows.Add(r); + } + + return dataSet; + } + + private MqttDriverOptions SparkplugOptions(string groupId, params RawTagEntry[] rawTags) => new() + { + Host = fixture.TlsHost, + Port = fixture.TlsPort, + UseTls = true, + CaCertificatePath = fixture.CaCertificatePath, + AllowUntrustedServerCertificate = fixture.CaCertificatePath is null, + Username = fixture.Username, + Password = fixture.Password, + ClientId = $"otopcua-live-spx-{Guid.NewGuid():N}", + ConnectTimeoutSeconds = 15, + Mode = MqttMode.SparkplugB, + Sparkplug = new MqttSparkplugOptions { GroupId = groupId, RequestRebirthOnGap = true }, + RawTags = rawTags, + }; + + private SparkplugEdgeNodeOptions SimOptions(string groupId, string edgeNodeId) => new() + { + Host = fixture.TlsHost, + Port = fixture.TlsPort, + UseTls = true, + CaCertificatePath = fixture.CaCertificatePath, + AllowUntrustedServerCertificate = fixture.CaCertificatePath is null, + Username = fixture.Username, + Password = fixture.Password, + GroupId = groupId, + EdgeNodeId = edgeNodeId, + }; + + private static RawTagEntry Tag( + string rawPath, string groupId, string edgeNodeId, string? deviceId, string metricName) + { + var device = deviceId is null ? "" : $"\"deviceId\":\"{deviceId}\","; + return new RawTagEntry( + rawPath, + $$"""{"groupId":"{{groupId}}","edgeNodeId":"{{edgeNodeId}}",{{device}}"metricName":"{{metricName}}"}""", + WriteIdempotent: false); + } + + /// Records every published data change, so a test can wait on one and count others. + private sealed class Recorder + { + private readonly List _events = []; + private readonly Lock _gate = new(); + + public Recorder(MqttDriver driver) => driver.OnDataChange += (_, e) => + { + lock (_gate) + { + _events.Add(e); + } + }; + + public int Count(string rawPath) + { + lock (_gate) + { + return _events.Count(e => e.FullReference == rawPath); + } + } + + public async Task WaitAsync( + string rawPath, Func predicate, TimeSpan timeout, CancellationToken ct) + { + var deadline = DateTime.UtcNow + timeout; + while (true) + { + lock (_gate) + { + foreach (var candidate in _events) + { + if (candidate.FullReference == rawPath && predicate(candidate.Snapshot)) + { + return candidate; + } + } + } + + if (DateTime.UtcNow >= deadline) + { + lock (_gate) + { + throw new TimeoutException( + $"No matching value for '{rawPath}' within {timeout}. Seen: " + + string.Join(", ", _events.Select(e => $"{e.FullReference}={e.Snapshot.Value}"))); + } + } + + await Task.Delay(100, ct); + } + } + } +} diff --git a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Mqtt.Tests/MqttBrowseSessionTests.cs b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Mqtt.Tests/MqttBrowseSessionTests.cs index 39cffaa9..5427e985 100644 --- a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Mqtt.Tests/MqttBrowseSessionTests.cs +++ b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Mqtt.Tests/MqttBrowseSessionTests.cs @@ -486,6 +486,31 @@ public sealed class MqttBrowseSessionTests s.PublishCountForTest.ShouldBe(0); // passive — browsing a live plant broker publishes nothing } + [Fact] + public async Task SparkplugTree_TemplateMetric_OffersItsMembersAndNotItself() + { + // Gitea #515. The parent has no value and no servable datatype, so a leaf for `Motor` would + // offer the operator an address that can only ever commit a tag which never reads — and the + // picker gives no hint that one leaf is real and the other is not. + await using var s = new MqttBrowseSession(MqttMode.SparkplugB); + s.ObserveTemplateBirthForTest( + "Plant1", "EdgeA", "Filler1", "Motor", + ("Speed", SparkplugDataType.Double), + ("Torque", SparkplugDataType.Double)); + + var metrics = await s.ExpandAsync("Plant1/EdgeA/Filler1", TestContext.Current.CancellationToken); + + metrics.Select(n => n.DisplayName).ShouldBe(["Motor/Speed", "Motor/Torque"]); + metrics.ShouldNotContain(n => n.DisplayName == "Motor"); + metrics[0].Kind.ShouldBe(BrowseNodeKind.Leaf); + + // The metric name travels INTACT as the node id — it is the binding key. Only the derived tag + // Name is later coerced to a legal RawPath segment, by RawBrowseCommitMapper (Gitea #509). + metrics[0].NodeId.ShouldBe("Plant1/EdgeA/Filler1::Motor/Speed"); + + s.PublishCountForTest.ShouldBe(0); + } + [Fact] public async Task SparkplugTree_NodeLevelMetrics_HangDirectlyUnderTheEdgeNode() { diff --git a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Mqtt.Tests/SparkplugCodecTests.cs b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Mqtt.Tests/SparkplugCodecTests.cs index 77cbff89..aefaca6a 100644 --- a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Mqtt.Tests/SparkplugCodecTests.cs +++ b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Mqtt.Tests/SparkplugCodecTests.cs @@ -186,35 +186,82 @@ public sealed class SparkplugCodecTests } /// - /// DataSet and Template metrics are out of scope for v1. They must decode to an - /// explicit "unsupported" kind — never an exception on the MQTT dispatcher thread, and never a - /// silently-dropped metric that looks identical to a metric that was never published. + /// A DataSet metric decodes to with its cells + /// flattened (Gitea #515) — never an exception on the MQTT dispatcher thread, and never a + /// silently-dropped metric that looks identical to one that was never published. /// [Fact] - public void Decode_GoldenNbirth_DataSetMetric_IsUnsupported_NotAThrow() + public void Decode_GoldenNbirth_DataSetMetric_FlattensToCells() { var payload = SparkplugCodec.Decode(SparkplugGoldenPayloads.Read(SparkplugGoldenPayloads.NbirthFile)); var config = payload.Metrics.Single(m => m.Name == "Config"); - config.ValueKind.ShouldBe(SparkplugValueKind.Unsupported); + config.ValueKind.ShouldBe(SparkplugValueKind.Structured); + + // The PARENT carries no value — its members do. A parent that reported a value would be a + // guess about which cell "the" value is. config.Value.ShouldBeNull(); config.DataType.ShouldBe(DataType.DataSet); + + var cell = config.MemberList.Single(); + cell.Path.ShouldBe("col0/0"); + cell.DataType.ShouldBe(DataType.Int32); + cell.Value.ShouldBe(1U); } - /// A Template metric takes the same unsupported path as DataSet. + /// A Template metric flattens its members to sibling paths. [Fact] - public void Decode_TemplateMetric_IsUnsupported() + public void Decode_TemplateMetric_FlattensMembersToSiblingPaths() { + var template = new Payload.Types.Template { Version = "1", IsDefinition = false }; + template.Metrics.Add(new Payload.Types.Metric + { + Name = "Speed", Datatype = (uint)DataType.Double, DoubleValue = 12.5, + }); + template.Metrics.Add(new Payload.Types.Metric + { + Name = "Running", Datatype = (uint)DataType.Boolean, BooleanValue = true, + }); + var payload = new Payload(); payload.Metrics.Add(new Payload.Types.Metric { Name = "Motor", Datatype = (uint)DataType.Template, - TemplateValue = new Payload.Types.Template { Version = "1", IsDefinition = false }, + TemplateValue = template, }); - SparkplugCodec.Decode(payload.ToByteArray()).Metrics.Single().ValueKind - .ShouldBe(SparkplugValueKind.Unsupported); + var metric = SparkplugCodec.Decode(payload.ToByteArray()).Metrics.Single(); + + metric.ValueKind.ShouldBe(SparkplugValueKind.Structured); + metric.MemberList.Select(m => m.Path).ShouldBe(["Speed", "Running"]); + metric.MemberList[0].Value.ShouldBe(12.5); + metric.MemberList[1].Value.ShouldBe(true); + } + + /// + /// A template definition is refused rather than flattened, and refusal is still the + /// visible kind. + /// + [Fact] + public void Decode_TemplateDefinition_StaysUnsupported() + { + // A definition is the UDT's SHAPE, published so consumers can understand instances — its member + // metrics carry no values. Flattening it would materialise a parallel set of tags that can + // never read, indistinguishable in the picker from the real instance's. + var definition = new Payload.Types.Template { Version = "1", IsDefinition = true }; + definition.Metrics.Add(new Payload.Types.Metric { Name = "Speed", Datatype = (uint)DataType.Double }); + + var payload = new Payload(); + payload.Metrics.Add(new Payload.Types.Metric + { + Name = "MotorType", Datatype = (uint)DataType.Template, TemplateValue = definition, + }); + + var metric = SparkplugCodec.Decode(payload.ToByteArray()).Metrics.Single(); + + metric.ValueKind.ShouldBe(SparkplugValueKind.Unsupported); + metric.MemberList.ShouldBeEmpty(); } /// Payload and per-metric timestamps survive as raw Sparkplug epoch milliseconds. diff --git a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Mqtt.Tests/SparkplugFlattenerTests.cs b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Mqtt.Tests/SparkplugFlattenerTests.cs new file mode 100644 index 00000000..5b63c931 --- /dev/null +++ b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Mqtt.Tests/SparkplugFlattenerTests.cs @@ -0,0 +1,294 @@ +using Google.Protobuf; +using Org.Eclipse.Tahu.Protobuf; +using Shouldly; +using Xunit; +using ZB.MOM.WW.OtOpcUa.Driver.Mqtt.Sparkplug; +using TahuDataType = Org.Eclipse.Tahu.Protobuf.DataType; + +namespace ZB.MOM.WW.OtOpcUa.Driver.Mqtt.Tests; + +/// +/// Unit tests for — the Template/DataSet/PropertySet mapping +/// (Gitea #515). +/// +/// +/// Two properties dominate this file. First, what is flattened is addressable and correctly +/// typed. Second — and the one that would be easy to lose in a later edit — +/// skip-and-warn survives: anything that cannot be flattened honestly stays +/// rather than becoming a guessed scalar, because a +/// tag bound to data that is not what its name says is worse than a tag that never binds. +/// +[Trait("Category", "Unit")] +public sealed class SparkplugFlattenerTests +{ + // ---- Template ------------------------------------------------------------------------------- + + [Fact] + public void A_nested_template_flattens_to_slash_joined_paths() + { + // `/` is the separator Sparkplug itself uses for metric-name hierarchy, so `Motor/Stator/Temp` + // is exactly what an edge node would have called a flat metric of the same meaning. + var stator = Template( + Member("Temp", TahuDataType.Double, m => m.DoubleValue = 71.5), + Member("Current", TahuDataType.Float, m => m.FloatValue = 3.25f)); + + var motor = Template( + Member("Speed", TahuDataType.Double, m => m.DoubleValue = 1500), + MemberTemplate("Stator", stator)); + + SparkplugFlattener.TryFlattenTemplate(motor, out var members).ShouldBeTrue(); + + members.Select(m => m.Path).ShouldBe(["Speed", "Stator/Temp", "Stator/Current"]); + members[1].DataType.ShouldBe(TahuDataType.Double); + members[2].Value.ShouldBe(3.25f); + } + + [Fact] + public void A_template_member_with_no_name_refuses_the_whole_metric() + { + // A template member is addressed by name; there is no alias fallback inside a template. Emitting + // the members that DO have names would publish a UDT silently missing part of itself, which + // reads in the picker exactly like a complete one. + var template = Template( + Member("Speed", TahuDataType.Double, m => m.DoubleValue = 1), + new Payload.Types.Metric { Datatype = (uint)TahuDataType.Double, DoubleValue = 2 }); + + SparkplugFlattener.TryFlattenTemplate(template, out var members).ShouldBeFalse(); + members.ShouldBeEmpty(); + } + + [Fact] + public void An_extension_value_inside_a_template_refuses_the_whole_metric() + { + // An extension value is a third-party escape hatch with no defined semantics. There is nothing + // honest to flatten it into, and the sibling members cannot be trusted to be the whole story. + var template = Template( + Member("Speed", TahuDataType.Double, m => m.DoubleValue = 1), + Member("Odd", TahuDataType.Unknown, m => m.ExtensionValue = new Payload.Types.Metric.Types.MetricValueExtension())); + + SparkplugFlattener.TryFlattenTemplate(template, out _).ShouldBeFalse(); + } + + [Fact] + public void A_template_definition_is_refused() + { + // A definition carries the UDT's shape with no instance values — metadata for consumers, not + // signal. Flattening it would materialise tags that can never read. + var definition = Template(Member("Speed", TahuDataType.Double, _ => { })); + definition.IsDefinition = true; + + SparkplugFlattener.TryFlattenTemplate(definition, out _).ShouldBeFalse(); + } + + [Fact] + public void Nesting_past_the_depth_bound_refuses_the_whole_metric_rather_than_truncating() + { + // This walks an object graph anyone on the broker can publish, so the recursion must be bounded. + // Refusing whole beats truncating: a silently half-flattened UDT looks exactly like a complete + // one, and the operator would author tags against a subtree that quietly stops partway down. + var deepest = Template(Member("Leaf", TahuDataType.Double, m => m.DoubleValue = 1)); + var current = deepest; + for (var i = 0; i < SparkplugFlattener.MaxDepth + 1; i++) + { + current = Template(MemberTemplate($"L{i}", current)); + } + + SparkplugFlattener.TryFlattenTemplate(current, out var members).ShouldBeFalse(); + members.ShouldBeEmpty(); + + // Falsifying control: the same shape one level inside the bound flattens fine, so the refusal + // above is the bound doing its job rather than nesting being broken outright. + var shallow = Template(MemberTemplate("L0", deepest)); + SparkplugFlattener.TryFlattenTemplate(shallow, out var ok).ShouldBeTrue(); + ok.Single().Path.ShouldBe("L0/Leaf"); + } + + // ---- DataSet --------------------------------------------------------------------------------- + + [Fact] + public void A_dataset_flattens_to_one_member_per_cell_with_the_row_index_always_present() + { + // Uniform naming, deliberately. Emitting a bare `{column}` for a single row and + // `{column}/{row}` otherwise would make a tag's own name depend on how many rows the plant + // happened to publish, so a table growing from one row to two would rename every existing tag + // out from under its authored binding. + var dataSet = DataSet( + columns: ["Temp", "Pressure"], + types: [TahuDataType.Double, TahuDataType.Int32], + rows: + [ + [Cell(v => v.DoubleValue = 20.5), Cell(v => v.IntValue = 3)], + [Cell(v => v.DoubleValue = 21.0), Cell(v => v.IntValue = 4)], + ]); + + SparkplugFlattener.TryFlattenDataSet(dataSet, out var members).ShouldBeTrue(); + + members.Select(m => m.Path).ShouldBe(["Temp/0", "Pressure/0", "Temp/1", "Pressure/1"]); + members[0].DataType.ShouldBe(TahuDataType.Double); + members[1].DataType.ShouldBe(TahuDataType.Int32); + members[3].Value.ShouldBe(4U); + } + + [Fact] + public void A_single_row_dataset_still_carries_the_row_index() + => Flatten(DataSet(["Temp"], [TahuDataType.Double], [[Cell(v => v.DoubleValue = 1)]])) + .Single().Path.ShouldBe("Temp/0"); + + [Fact] + public void An_empty_cell_is_a_null_member_not_a_zero() + { + // A DataSetValue has no is_null field, so an empty oneof IS the null cell. Reading it as the + // proto default would publish 0 for a cell the plant left blank — plausible, Good, and wrong. + var members = Flatten(DataSet(["Temp"], [TahuDataType.Double], [[new Payload.Types.DataSet.Types.DataSetValue()]])); + + members.Single().ValueKind.ShouldBe(SparkplugValueKind.Null); + members.Single().Value.ShouldBeNull(); + } + + [Fact] + public void A_column_and_type_count_mismatch_refuses_the_dataset() + // Columns and types are parallel arrays. Guessing which to trust would type a column by its + // neighbour, and every cell in it would publish at Good quality as the wrong type. + => SparkplugFlattener.TryFlattenDataSet( + DataSet(["Temp", "Pressure"], [TahuDataType.Double], [] ), out _) + .ShouldBeFalse(); + + [Fact] + public void A_short_row_refuses_the_dataset() + // Not a row of nulls: emitting the cells that ARE present publishes a table silently missing + // its right-hand columns. + => SparkplugFlattener.TryFlattenDataSet( + DataSet(["Temp", "Pressure"], [TahuDataType.Double, TahuDataType.Int32], + [[Cell(v => v.DoubleValue = 1)]]), + out _) + .ShouldBeFalse(); + + [Fact] + public void An_unnamed_column_refuses_the_dataset() + => SparkplugFlattener.TryFlattenDataSet( + DataSet(["", "Pressure"], [TahuDataType.Double, TahuDataType.Int32], + [[Cell(v => v.DoubleValue = 1), Cell(v => v.IntValue = 2)]]), + out _) + .ShouldBeFalse(); + + [Fact] + public void A_dataset_past_the_member_bound_refuses_rather_than_truncating() + { + var columns = Enumerable.Range(0, 8).Select(i => $"c{i}").ToArray(); + var types = Enumerable.Repeat(TahuDataType.Int32, 8).ToArray(); + var rows = Enumerable.Range(0, 100) + .Select(_ => Enumerable.Range(0, 8).Select(_ => Cell(v => v.IntValue = 1)).ToArray()) + .ToArray(); + + // 800 cells, past MaxMembers (512). + SparkplugFlattener.TryFlattenDataSet(DataSet(columns, types, rows), out var members).ShouldBeFalse(); + members.ShouldBeEmpty(); + } + + // ---- PropertySet ------------------------------------------------------------------------------ + + [Fact] + public void Properties_project_to_flat_metadata_including_one_nested_level() + { + var nested = new Payload.Types.PropertySet(); + nested.Keys.Add("Low"); + nested.Values.Add(new Payload.Types.PropertyValue { DoubleValue = 0 }); + + var set = new Payload.Types.PropertySet(); + set.Keys.Add("engUnit"); + set.Values.Add(new Payload.Types.PropertyValue { StringValue = "degC" }); + set.Keys.Add("Range"); + set.Values.Add(new Payload.Types.PropertyValue { PropertysetValue = nested }); + + var projected = SparkplugFlattener.ProjectProperties(set); + + projected.Select(p => p.Name).ShouldBe(["engUnit", "Range/Low"]); + projected[0].Value.ShouldBe("degC"); + } + + [Fact] + public void Properties_never_become_members() + { + // The recorded decision, and the honest reading of the spec: a PropertySet describes a metric + // (units, ranges, quality hints) — it is not a value the metric HAS. Surfacing it as sibling + // tags would invent plant signals out of documentation. + var set = new Payload.Types.PropertySet(); + set.Keys.Add("engUnit"); + set.Values.Add(new Payload.Types.PropertyValue { StringValue = "degC" }); + + var payload = new Payload(); + var metric = new Payload.Types.Metric + { + Name = "Temperature", Datatype = (uint)TahuDataType.Double, DoubleValue = 20.5, Properties = set, + }; + payload.Metrics.Add(metric); + + var decoded = SparkplugCodec.Decode(payload.ToByteArray()).Metrics.Single(); + + decoded.ValueKind.ShouldBe(SparkplugValueKind.Scalar); + decoded.MemberList.ShouldBeEmpty(); + decoded.PropertyList.Single().Name.ShouldBe("engUnit"); + } + + [Fact] + public void A_malformed_property_set_projects_to_nothing_rather_than_throwing() + { + var set = new Payload.Types.PropertySet(); + set.Keys.Add("orphan"); // no matching value + + SparkplugFlattener.ProjectProperties(set).ShouldBeEmpty(); + SparkplugFlattener.ProjectProperties(null).ShouldBeEmpty(); + } + + // ---- helpers ---------------------------------------------------------------------------------- + + private static IReadOnlyList Flatten(Payload.Types.DataSet dataSet) + { + SparkplugFlattener.TryFlattenDataSet(dataSet, out var members).ShouldBeTrue(); + return members; + } + + private static Payload.Types.Template Template(params Payload.Types.Metric[] members) + { + var template = new Payload.Types.Template { Version = "1", IsDefinition = false }; + template.Metrics.AddRange(members); + return template; + } + + private static Payload.Types.Metric Member( + string name, TahuDataType dataType, Action setValue) + { + var metric = new Payload.Types.Metric { Name = name, Datatype = (uint)dataType }; + setValue(metric); + return metric; + } + + private static Payload.Types.Metric MemberTemplate(string name, Payload.Types.Template value) => + new() { Name = name, Datatype = (uint)TahuDataType.Template, TemplateValue = value }; + + private static Payload.Types.DataSet.Types.DataSetValue Cell( + Action setValue) + { + var cell = new Payload.Types.DataSet.Types.DataSetValue(); + setValue(cell); + return cell; + } + + private static Payload.Types.DataSet DataSet( + string[] columns, + TahuDataType[] types, + Payload.Types.DataSet.Types.DataSetValue[][] rows) + { + var dataSet = new Payload.Types.DataSet { NumOfColumns = (ulong)columns.Length }; + dataSet.Columns.AddRange(columns); + dataSet.Types_.AddRange(types.Select(t => (uint)t)); + foreach (var cells in rows) + { + var row = new Payload.Types.DataSet.Types.Row(); + row.Elements.AddRange(cells); + dataSet.Rows.Add(row); + } + + return dataSet; + } +} diff --git a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Mqtt.Tests/SparkplugGoldenPayloads.cs b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Mqtt.Tests/SparkplugGoldenPayloads.cs index 29b0544f..e052f7e4 100644 --- a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Mqtt.Tests/SparkplugGoldenPayloads.cs +++ b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Mqtt.Tests/SparkplugGoldenPayloads.cs @@ -132,8 +132,9 @@ internal static class SparkplugGoldenPayloads IntValue = unchecked((uint)CountBirthValue), }); - // DataSet is explicitly out of scope for v1 — present here so "unsupported" is a decoded, - // testable outcome rather than an exception on the MQTT dispatcher thread. + // A single-column, single-row DataSet. Gitea #515 flattens it to the addressable member + // `Config/col0/0`; before that it decoded as an explicit "unsupported" kind. Kept in the + // golden vector either way — it is the only structured metric on the wire-format path. var dataSet = new Payload.Types.DataSet { NumOfColumns = 1UL }; dataSet.Columns.Add("col0"); dataSet.Types_.Add((uint)DataType.Int32); diff --git a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Mqtt.Tests/SparkplugPrimaryHostTests.cs b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Mqtt.Tests/SparkplugPrimaryHostTests.cs new file mode 100644 index 00000000..451a060c --- /dev/null +++ b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Mqtt.Tests/SparkplugPrimaryHostTests.cs @@ -0,0 +1,263 @@ +using System.Text; +using MQTTnet; +using Shouldly; +using Xunit; +using ZB.MOM.WW.OtOpcUa.Core.Abstractions; +using ZB.MOM.WW.OtOpcUa.Driver.Mqtt.Sparkplug; + +namespace ZB.MOM.WW.OtOpcUa.Driver.Mqtt.Tests; + +/// +/// Unit tests for the Sparkplug primary-host claim (Gitea #511) — the parts that are decidable +/// without a broker: the STATE bodies, the Will registration, and the gate that decides whether a +/// claim is attempted at all. +/// +/// +/// The claim/release state machine itself needs a broker and is covered by +/// MqttPrimaryHostLiveTests in the integration suite. What is pinned here is everything a +/// live test could not tell apart from a working implementation by accident — above all that the +/// birth and the Will carry the SAME timestamp, which a live test would see as two well-formed +/// messages. +/// +[Trait("Category", "Unit")] +public sealed class SparkplugPrimaryHostTests +{ + private static MqttDriverOptions Options( + bool actAsPrimaryHost = true, + string hostId = "otopcua-primary", + MqttMode mode = MqttMode.SparkplugB) + => new() + { + Host = "broker.example", + Port = 1883, + ClientId = "otopcua", + Mode = mode, + Sparkplug = new MqttSparkplugOptions + { + GroupId = "OtOpcUaSim", + HostId = hostId, + ActAsPrimaryHost = actAsPrimaryHost, + }, + }; + + // ---- The matched-timestamp invariant -------------------------------------------------------- + + [Fact] + public void The_birth_and_the_will_carry_the_same_timestamp() + { + // Sparkplug 3.0 pairs a host's birth and death certificates by their timestamp, so an edge node + // may ignore an OFFLINE older than the ONLINE it holds. The two bodies are necessarily built at + // different moments — the Will goes in the CONNECT packet, the birth is published after CONNACK + // — so reading the clock twice is the natural mistake, and it produces two individually + // well-formed messages that no live test would flag. + var session = SparkplugStatePayload.ForSession("otopcua-primary", 1_770_000_000_123); + + SparkplugStatePayload.TryDecode(session.OnlineBody, out var online, out var birthTs).ShouldBeTrue(); + SparkplugStatePayload.TryDecode(session.OfflineBody, out var offline, out var deathTs).ShouldBeTrue(); + + online.ShouldBeTrue(); + offline.ShouldBeFalse(); + birthTs.ShouldBe(1_770_000_000_123); + deathTs.ShouldBe(birthTs); + } + + [Fact] + public void The_state_topic_is_the_v3_form() + => SparkplugStatePayload.ForSession("scada-primary", 1).Topic.ShouldBe("spBv1.0/STATE/scada-primary"); + + [Fact] + public void The_timestamp_is_a_bare_json_number() + { + // A quoted timestamp still parses as a valid JSON object, so a serializer choosing string + // formatting for a long would be silently wrong to every conformant host. + var body = Encoding.UTF8.GetString(SparkplugStatePayload.Encode(online: true, 1_770_000_000_123)); + + body.ShouldBe("{\"online\":true,\"timestamp\":1770000000123}"); + } + + [Fact] + public void The_body_survives_a_non_invariant_culture() + { + var previous = Thread.CurrentThread.CurrentCulture; + Thread.CurrentThread.CurrentCulture = new System.Globalization.CultureInfo("de-DE"); + try + { + Encoding.UTF8.GetString(SparkplugStatePayload.Encode(online: false, 1_770_000_000_123)) + .ShouldBe("{\"online\":false,\"timestamp\":1770000000123}"); + } + finally + { + Thread.CurrentThread.CurrentCulture = previous; + } + } + + // ---- The Will actually reaches the CONNECT packet -------------------------------------------- + + [Fact] + public void The_will_is_registered_on_the_client_options_retained_at_qos1() + { + // The single most important assertion in this file. A retained ONLINE with no Will behind it is + // strictly worse than publishing nothing: a crashed node then asserts forever that a dead host + // is alive. This is the check that the death certificate is armed at CONNECT rather than merely + // constructed. + var session = SparkplugStatePayload.ForSession("otopcua-primary", 42); + var will = new MqttLastWill(session.Topic, session.OfflineBody, SparkplugStatePayload.StateQos, Retain: true); + + var built = MqttClientOptionsFactory.BuildClientOptions( + Options(), SparkplugPrimaryHost.ClientIdSuffix, logger: null, lastWill: will, cleanSessionOverride: true); + + built.WillTopic.ShouldBe("spBv1.0/STATE/otopcua-primary"); + built.WillRetain.ShouldBeTrue(); + built.WillQualityOfServiceLevel.ShouldBe(MQTTnet.Protocol.MqttQualityOfServiceLevel.AtLeastOnce); + SparkplugStatePayload.TryDecode(built.WillPayload, out var online, out _).ShouldBeTrue(); + online.ShouldBeFalse(); + } + + [Fact] + public void No_will_is_registered_when_none_is_supplied() + // Every other connection this repo opens — ingest, browse, probe — must stay Will-free. A Will + // leaking onto the ingest session would publish a host death when ingest merely reconnects. + => MqttClientOptionsFactory.BuildClientOptions(Options(), clientIdSuffix: null).WillTopic.ShouldBeNull(); + + [Fact] + public void The_claim_connection_forces_clean_session_over_the_configured_value() + { + // A broker resuming a previous session for this client id may keep the OLD session's Will and + // discard the one this CONNECT carries — a claim with no death certificate behind it. + var persistent = Options() with { CleanSession = false }; + + MqttClientOptionsFactory.BuildClientOptions(persistent, "-spbhost", cleanSessionOverride: true) + .CleanSession.ShouldBeTrue(); + + // ...and the override is genuinely an override, not a constant. + MqttClientOptionsFactory.BuildClientOptions(persistent, clientIdSuffix: null) + .CleanSession.ShouldBeFalse(); + } + + [Fact] + public void The_claim_connection_uses_a_distinct_client_id() + { + // Two MQTT clients presenting one client id is not an error the broker reports: it evicts the + // older one. Ingest and the claim would take turns killing each other, forever, looking like a + // flapping broker. + var ingest = MqttClientOptionsFactory.BuildClientOptions(Options(), clientIdSuffix: null).ClientId; + var claim = MqttClientOptionsFactory + .BuildClientOptions(Options(), SparkplugPrimaryHost.ClientIdSuffix).ClientId; + + claim.ShouldNotBe(ingest); + claim.ShouldBe("otopcua-spbhost"); + } + + // ---- Whether a claim is attempted at all ----------------------------------------------------- + + [Fact] + public void A_configured_claim_is_claimable() + => SparkplugPrimaryHost.CanClaim(Options()).ShouldBeTrue(); + + [Theory] + [InlineData("")] + [InlineData(" ")] + public void A_blank_host_id_is_refused_rather_than_publishing_to_a_malformed_topic(string hostId) + { + // "spBv1.0/STATE/" is a legal MQTT topic, so a publish would succeed and reach nobody. + SparkplugPrimaryHost.CanClaim(Options(hostId: hostId)).ShouldBeFalse(); + SparkplugPrimaryHost.DescribeUnusable(Options(hostId: hostId)).ShouldNotBeNullOrWhiteSpace(); + } + + [Fact] + public void The_flag_is_ignored_outside_sparkplug_mode_and_says_so() + { + var plain = Options(mode: MqttMode.Plain); + + SparkplugPrimaryHost.CanClaim(plain).ShouldBeFalse(); + SparkplugPrimaryHost.DescribeUnusable(plain).ShouldNotBeNullOrWhiteSpace(); + } + + [Fact] + public void An_unset_flag_produces_no_complaint() + { + // DescribeUnusable must stay silent about a configuration that never asked for anything, or the + // driver warns on every ordinary Sparkplug deployment. + SparkplugPrimaryHost.CanClaim(Options(actAsPrimaryHost: false)).ShouldBeFalse(); + SparkplugPrimaryHost.DescribeUnusable(Options(actAsPrimaryHost: false)).ShouldBeNull(); + SparkplugPrimaryHost.DescribeUnusable(Options(actAsPrimaryHost: false, hostId: "")).ShouldBeNull(); + } + + [Fact] + public void Constructing_a_claim_without_a_host_id_throws_rather_than_half_working() + => Should.Throw(() => + new SparkplugPrimaryHost(Options(hostId: " "), "drv-1", UnmanagedDriverPrimaryRoleSource.Instance)); + + // ---- The gate --------------------------------------------------------------------------------- + + [Fact] + public void A_driver_with_no_role_source_claims() + { + // The documented fallback for a standalone host or a test: with nothing publishing a role there + // is no peer to collide with, and defaulting closed would make the feature silently inert on the + // single-node deployment that is its commonest use. + UnmanagedDriverPrimaryRoleSource.Instance.ShouldActAsPrimary.ShouldBeTrue(); + + // Publishing into it is a no-op rather than a throw, so a caller need not special-case it. + UnmanagedDriverPrimaryRoleSource.Instance.PublishPrimaryRole(false); + UnmanagedDriverPrimaryRoleSource.Instance.ShouldActAsPrimary.ShouldBeTrue(); + } + + [Fact] + public async Task A_secondary_never_even_attempts_a_claim_connection() + { + // The whole point of gating. Asserted on ATTEMPTS, not on IsClaimed: both nodes point at the + // same unreachable broker here, so IsClaimed is false either way and an IsClaimed-only + // assertion would pass just as happily with the gate deleted. See ClaimAttemptsForTests. + var host = new SparkplugPrimaryHost( + Unreachable(), "drv-1", new FixedRole(shouldActAsPrimary: false)); + + await host.StartAsync(TestContext.Current.CancellationToken); + + host.ClaimAttemptsForTests.ShouldBe(0); + host.IsClaimed.ShouldBeFalse(); + await host.DisposeAsync(); + } + + [Fact] + public async Task A_primary_attempts_the_claim_and_survives_a_broker_it_cannot_reach() + { + // The falsifying control for the test above — same options, same broker, opposite verdict — so + // "0 attempts" there is a fact about the gate rather than about the fixture. + // + // It also pins the swallow: the claim is advisory and ingest is the data plane, so a broker that + // refuses it must cost a warning, not the driver. And IsClaimed must stay honest — a connection + // whose ONLINE never landed is not a claim. + var host = new SparkplugPrimaryHost(Unreachable(), "drv-1", new FixedRole(shouldActAsPrimary: true)); + + await Should.NotThrowAsync(() => host.StartAsync(TestContext.Current.CancellationToken)); + + host.ClaimAttemptsForTests.ShouldBe(1); + host.IsClaimed.ShouldBeFalse(); + await host.DisposeAsync(); + } + + /// A broker that refuses instantly, so a connect attempt is observable but not slow. + private static MqttDriverOptions Unreachable() => + Options() with { Host = "127.0.0.1", Port = 1, ConnectTimeoutSeconds = 1 }; + + [Fact] + public async Task Stopping_an_unclaimed_host_is_a_no_op() + { + var host = new SparkplugPrimaryHost(Options(), "drv-1", new FixedRole(shouldActAsPrimary: false)); + + await Should.NotThrowAsync(() => host.StopAsync(TestContext.Current.CancellationToken)); + + // Twice: teardown runs from both ShutdownAsync and DisposeAsync, so a second release that threw + // — or that published a second OFFLINE — would surface on every ordinary driver shutdown. + await Should.NotThrowAsync(async () => await host.DisposeAsync()); + await Should.NotThrowAsync(async () => await host.DisposeAsync()); + } + + private sealed class FixedRole(bool shouldActAsPrimary) : IDriverPrimaryRoleSource + { + public bool ShouldActAsPrimary { get; private set; } = shouldActAsPrimary; + + public void PublishPrimaryRole(bool value) => ShouldActAsPrimary = value; + } +} diff --git a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Mqtt.Tests/SparkplugStructuredIngestTests.cs b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Mqtt.Tests/SparkplugStructuredIngestTests.cs new file mode 100644 index 00000000..d99be5df --- /dev/null +++ b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.Mqtt.Tests/SparkplugStructuredIngestTests.cs @@ -0,0 +1,283 @@ +using Google.Protobuf; +using Org.Eclipse.Tahu.Protobuf; +using Shouldly; +using Xunit; +using ZB.MOM.WW.OtOpcUa.Core.Abstractions; +using ZB.MOM.WW.OtOpcUa.Driver.Mqtt.Sparkplug; +using TahuDataType = Org.Eclipse.Tahu.Protobuf.DataType; + +namespace ZB.MOM.WW.OtOpcUa.Driver.Mqtt.Tests; + +/// +/// End-to-end ingest of flattened structured metrics (Gitea #515): a tag authored against +/// Motor/Speed is fed by a DBIRTH/DDATA carrying a Template metric named +/// Motor. +/// +/// +/// pins the mapping in isolation; this file pins that the +/// chain actually connects — flatten → alias-table binding → authored-tag lookup → publish. Those +/// are four separate places a member name is composed, and a mismatch in any one of them produces +/// a driver that decodes structured metrics perfectly and feeds nothing. +/// +[Trait("Category", "Unit")] +public sealed class SparkplugStructuredIngestTests +{ + private const string Group = "Plant1"; + private const string Node = "EdgeA"; + private const string Device = "Filler1"; + private const ulong MotorAlias = 20UL; + private const ulong TableAlias = 21UL; + private const uint StatusGood = 0x00000000u; + + private const string SpeedPath = "Plant/Mqtt/spb/MotorSpeed"; + private const string TempCellPath = "Plant/Mqtt/spb/TempCell0"; + + [Fact] + public async Task A_template_member_feeds_the_tag_authored_against_its_flattened_name() + { + // The whole point of #515, asserted end to end. `Motor` is what the wire carries; `Motor/Speed` + // is what an operator authors, and nothing on the wire is ever called that. + var (ing, fired) = await NewIngestorAsync(Tag(SpeedPath, Blob("Motor/Speed", "Float64"))); + + ing.Dispatch(Topic(SparkplugMessageType.NBIRTH), Nbirth()); + ing.Dispatch(Topic(SparkplugMessageType.DBIRTH, Device), Birth(seq: 1, MotorTemplateMetric(speed: 1500, running: true))); + + fired.ShouldContainKey(SpeedPath); + fired[SpeedPath].Value.ShouldBe(1500d); + fired[SpeedPath].StatusCode.ShouldBe(StatusGood); + } + + [Fact] + public async Task A_ddata_template_carrying_only_the_parents_alias_still_reaches_the_member_tag() + { + // The load-bearing case, and the one a naive implementation gets wrong. A DDATA metric carries + // an ALIAS and no name — so the member names cannot be composed until the alias table has + // resolved the parent's name. Flattening at decode time alone is not enough. + var (ing, fired) = await NewIngestorAsync(Tag(SpeedPath, Blob("Motor/Speed", "Float64"))); + + ing.Dispatch(Topic(SparkplugMessageType.NBIRTH), Nbirth()); + ing.Dispatch(Topic(SparkplugMessageType.DBIRTH, Device), Birth(seq: 1, MotorTemplateMetric(speed: 1500, running: true))); + fired.Clear(); + + // seq 2, alias only — no Name on the metric at all. + var data = new Payload { Seq = 2UL, Timestamp = 1721822405000UL }; + data.Metrics.Add(new Payload.Types.Metric + { + Alias = MotorAlias, + TemplateValue = TemplateOf(speed: 1750, running: false), + }); + + ing.Dispatch(Topic(SparkplugMessageType.DDATA, Device), Decode(data)); + + fired.ShouldContainKey(SpeedPath); + fired[SpeedPath].Value.ShouldBe(1750d); + } + + [Fact] + public async Task A_dataset_cell_feeds_the_tag_authored_against_its_indexed_name() + { + var (ing, fired) = await NewIngestorAsync(Tag(TempCellPath, Blob("Table/Temp/0", "Float64"))); + + ing.Dispatch(Topic(SparkplugMessageType.NBIRTH), Nbirth()); + ing.Dispatch(Topic(SparkplugMessageType.DBIRTH, Device), Birth(seq: 1, TableDataSetMetric(20.5, 21.5))); + + fired.ShouldContainKey(TempCellPath); + fired[TempCellPath].Value.ShouldBe(20.5d); + } + + [Fact] + public async Task A_member_that_carries_no_datatype_falls_back_to_the_birth_binding() + { + // A DDATA template is not obliged to re-declare its members' datatypes. Guessing from the CLR + // type of the wire value would read a signed Int32 of -42 out of its unsigned field as + // 4294967254 — plausible, Good quality, and wrong. + var (ing, fired) = await NewIngestorAsync(Tag(SpeedPath, Blob("Motor/Offset", "Int32"))); + + var birthTemplate = new Payload.Types.Template { Version = "1", IsDefinition = false }; + birthTemplate.Metrics.Add(new Payload.Types.Metric + { + Name = "Offset", Datatype = (uint)TahuDataType.Int32, IntValue = unchecked((uint)-42), + }); + + ing.Dispatch(Topic(SparkplugMessageType.NBIRTH), Nbirth()); + ing.Dispatch(Topic(SparkplugMessageType.DBIRTH, Device), Birth(seq: 1, new Payload.Types.Metric + { + Name = "Motor", Alias = MotorAlias, Datatype = (uint)TahuDataType.Template, TemplateValue = birthTemplate, + })); + + fired[SpeedPath].Value.ShouldBe(-42); + fired.Clear(); + + // DDATA member with NO datatype of its own. + var dataTemplate = new Payload.Types.Template { Version = "1", IsDefinition = false }; + dataTemplate.Metrics.Add(new Payload.Types.Metric { Name = "Offset", IntValue = unchecked((uint)-7) }); + + var data = new Payload { Seq = 2UL, Timestamp = 1721822405000UL }; + data.Metrics.Add(new Payload.Types.Metric { Alias = MotorAlias, TemplateValue = dataTemplate }); + ing.Dispatch(Topic(SparkplugMessageType.DDATA, Device), Decode(data)); + + // -7, not 4294967289: the birth's Int32 binding drove ReinterpretSigned. + fired[SpeedPath].Value.ShouldBe(-7); + } + + [Fact] + public async Task A_tag_authored_against_the_structured_parent_itself_is_never_fed() + { + // The parent has no value — its members do. Feeding it anything would require choosing which + // member "the" value is, which is exactly the guess #515 refuses to make. + var (ing, fired) = await NewIngestorAsync(Tag(SpeedPath, Blob("Motor", "Float64"))); + + ing.Dispatch(Topic(SparkplugMessageType.NBIRTH), Nbirth()); + ing.Dispatch(Topic(SparkplugMessageType.DBIRTH, Device), Birth(seq: 1, MotorTemplateMetric(speed: 1500, running: true))); + + fired.ShouldNotContainKey(SpeedPath); + } + + [Fact] + public async Task An_unflattenable_structured_metric_still_feeds_nothing_rather_than_a_guess() + { + // Skip-and-warn survives — the explicit requirement on the issue. A template DEFINITION is the + // realistic instance of this: it names members and carries no values. + var definition = new Payload.Types.Template { Version = "1", IsDefinition = true }; + definition.Metrics.Add(new Payload.Types.Metric { Name = "Speed", Datatype = (uint)TahuDataType.Double }); + + var (ing, fired) = await NewIngestorAsync(Tag(SpeedPath, Blob("Motor/Speed", "Float64"))); + + ing.Dispatch(Topic(SparkplugMessageType.NBIRTH), Nbirth()); + ing.Dispatch(Topic(SparkplugMessageType.DBIRTH, Device), Birth(seq: 1, new Payload.Types.Metric + { + Name = "Motor", Alias = MotorAlias, Datatype = (uint)TahuDataType.Template, TemplateValue = definition, + })); + + fired.ShouldNotContainKey(SpeedPath); + } + + [Fact] + public async Task The_rediscovery_signature_tracks_members_not_the_parent_metric_name() + { + // A UDT gaining or losing a member leaves the PARENT metric's name identical, so a gate built + // on raw wire names would never move — and the operator would never be told the tree changed, + // which for a UDT is the change that matters most. + var (ing, _) = await NewIngestorAsync(Tag(SpeedPath, Blob("Motor/Speed", "Float64"))); + + var observed = new List>(); + ing.BirthObserved += (_, e) => observed.Add(e.MetricNames); + + ing.Dispatch(Topic(SparkplugMessageType.NBIRTH), Nbirth()); + ing.Dispatch(Topic(SparkplugMessageType.DBIRTH, Device), Birth(seq: 1, MotorTemplateMetric(speed: 1, running: true))); + + var deviceBirth = observed[^1]; + deviceBirth.ShouldBe(["Motor/Speed", "Motor/Running"], ignoreOrder: true); + deviceBirth.ShouldNotContain("Motor"); + } + + // ---- helpers --------------------------------------------------------------------------------- + + /// + /// A registered and subscribed ingestor. The subscribe step is not optional scaffolding: + /// SparkplugIngestor.Raise only fires OnDataChange for a RawPath that holds a + /// subscription handle, so an unsubscribed ingestor caches values silently and every assertion + /// in this file would read as "no value" whether or not the flattening worked. + /// + private static async Task<(SparkplugIngestor Ingestor, Dictionary Fired)> NewIngestorAsync( + params RawTagEntry[] tags) + { + var options = new MqttDriverOptions + { + Mode = MqttMode.SparkplugB, + Sparkplug = new MqttSparkplugOptions { GroupId = Group, RequestRebirthOnGap = false }, + RawTags = tags, + }; + + var ingestor = new SparkplugIngestor( + options, + "drv-1", + subscribeTransport: new GrantingSubscribeTransport(), + publishTransport: null, + logger: null, + cache: new LastValueCache()); + ingestor.Register(tags); + await ingestor.SubscribeAsync( + [.. ingestor.AuthoredRawPaths], TimeSpan.Zero, TestContext.Current.CancellationToken); + + var fired = new Dictionary(StringComparer.Ordinal); + ingestor.OnDataChange += (_, e) => fired[e.FullReference] = e.Snapshot; + return (ingestor, fired); + } + + /// A SUBACK that grants everything; the tests here are about values, not subscribe policy. + private sealed class GrantingSubscribeTransport : IMqttSubscribeTransport + { + public Task> SubscribeAsync( + IReadOnlyList filters, CancellationToken cancellationToken) + => Task.FromResult>( + [.. filters.Select(f => new MqttSubscribeOutcome(f.Topic, true, "GrantedQoS1"))]); + } + + private static RawTagEntry Tag(string rawPath, string tagConfig) => new(rawPath, tagConfig, WriteIdempotent: false); + + private static string Blob(string metric, string dataType) => + $$"""{"groupId":"{{Group}}","edgeNodeId":"{{Node}}","deviceId":"{{Device}}","metricName":"{{metric}}","dataType":"{{dataType}}"}"""; + + private static SparkplugTopic Topic(SparkplugMessageType type, string? device = null) => + new(type, Group, Node, device, null); + + private static SparkplugPayload Decode(Payload payload) => SparkplugCodec.Decode(payload.ToByteArray()); + + private static SparkplugPayload Nbirth() + { + var payload = new Payload { Seq = 0UL, Timestamp = 1721822400000UL }; + payload.Metrics.Add(new Payload.Types.Metric + { + Name = "bdSeq", Datatype = (uint)TahuDataType.Uint64, LongValue = 1UL, + }); + return Decode(payload); + } + + private static SparkplugPayload Birth(ulong seq, params Payload.Types.Metric[] metrics) + { + var payload = new Payload { Seq = seq, Timestamp = 1721822400000UL }; + payload.Metrics.AddRange(metrics); + return Decode(payload); + } + + private static Payload.Types.Template TemplateOf(double speed, bool running) + { + var template = new Payload.Types.Template { Version = "1", IsDefinition = false }; + template.Metrics.Add(new Payload.Types.Metric + { + Name = "Speed", Datatype = (uint)TahuDataType.Double, DoubleValue = speed, + }); + template.Metrics.Add(new Payload.Types.Metric + { + Name = "Running", Datatype = (uint)TahuDataType.Boolean, BooleanValue = running, + }); + return template; + } + + private static Payload.Types.Metric MotorTemplateMetric(double speed, bool running) => new() + { + Name = "Motor", + Alias = MotorAlias, + Datatype = (uint)TahuDataType.Template, + TemplateValue = TemplateOf(speed, running), + }; + + private static Payload.Types.Metric TableDataSetMetric(params double[] temps) + { + var dataSet = new Payload.Types.DataSet { NumOfColumns = 1UL }; + dataSet.Columns.Add("Temp"); + dataSet.Types_.Add((uint)TahuDataType.Double); + foreach (var temp in temps) + { + var row = new Payload.Types.DataSet.Types.Row(); + row.Elements.Add(new Payload.Types.DataSet.Types.DataSetValue { DoubleValue = temp }); + dataSet.Rows.Add(row); + } + + return new Payload.Types.Metric + { + Name = "Table", Alias = TableAlias, Datatype = (uint)TahuDataType.DataSet, DatasetValue = dataSet, + }; + } +}