From d5bd4226eebad7698023a151066a539d79557e70 Mon Sep 17 00:00:00 2001 From: Joseph Doherty Date: Sat, 25 Jul 2026 01:11:56 -0400 Subject: [PATCH] docs(mqtt): Sparkplug P2 live-verified; MQTT driver complete MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Task 26 — the P2 milestone gate. Ran the live /run verification on an isolated docker-dev rig (project otopcua-mqtt, ports 9210/4850-4851/14350, image built from this branch) against the real Mosquitto TLS+auth broker and the C# Sparkplug edge-node simulator on 10.100.0.35, and recorded the result. The gate found three defects. Two are the same defect class the P1 gate found twice — a hand-maintained AdminUI surface left behind by a driver-side feature — and the third is a pre-existing cross-driver bug that only a Sparkplug flow could surface. 1. CRITICAL, FIXED — MqttDriverForm still shipped its P1 Sparkplug PLACEHOLDER. Switching Mode to SparkplugB rendered a "not available yet" notice and NO Group ID field, so with Sparkplug ingest fully shipped there was still no way to author a Sparkplug driver from the AdminUI at all. Sparkplug.GroupId is the driver's entire subscription filter (spBv1.0/{GroupId}/#): blank ⇒ connected, Healthy, ingesting nothing. Now authors all five Sparkplug keys, MERGES over the existing sub-object rather than replacing it (so a key a newer driver adds inside it survives an older AdminUI), leaves it untouched in Plain mode, and validates the group id as the topic segment it is. Pinned by 8 MqttDriverFormModelTests cases, verified falsifiable — 8 RED with the fix stubbed out. 2. PRE-EXISTING + CROSS-DRIVER, FIXED — every node label in the shared DriverBrowseTree was an . In a Blazor Web App, blazor.web.js's enhanced-navigation click interceptor resolves a bare "#" against , so clicking ANY browse-tree node label navigated the whole AdminUI to "/", tore down the circuit, and destroyed the hosting modal — losing the browse session AND the tag selection. @onclick:preventDefault does not help: it suppresses the browser's default action, not Blazor's own interceptor. Labels are now } else { - @item.Node.DisplayName + } @if (item.Node.Kind == BrowseNodeKind.Leaf) { 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 8349837c..cb18f6bc 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 @@ -8,8 +8,11 @@ pure MqttDriverFormModel — enums must round-trip by NAME (this repo's systemic driver-enum bug). The Sparkplug sub-object (groupId / hostId / actAsPrimaryHost / requestRebirthOnGap / - birthObservationWindowSeconds) is P2 (Task 21+): the placeholder below mirrors how MqttTagConfigEditor - stubs its Sparkplug branch, and any existing sparkplug keys are preserved untouched. *@ + birthObservationWindowSeconds) is authored by the Mode == SparkplugB branch below. It is MERGED over + whatever the inbound blob had rather than replacing it, and in Plain mode it is not touched at all — + see MqttDriverFormModel.ToJson. Until the P2 live gate this branch was a "not implemented yet" + placeholder, which left the group id — the driver's entire subscription filter — unauthorable from + the UI after Sparkplug ingest had shipped. *@ @using ZB.MOM.WW.OtOpcUa.AdminUI.Components.Shared.Drivers @using ZB.MOM.WW.OtOpcUa.Driver.Mqtt @@ -171,7 +174,7 @@ } -
Plain = topic-bound tags. Sparkplug B is not implemented yet.
+
Plain = topic-bound tags. Sparkplug B = birth-described metrics under one group.
@@ -201,12 +204,66 @@ } else { + @* Sparkplug B. The group id is the ONE mandatory field: it is the driver's entire + subscription filter (spBv1.0/{GroupId}/#), so a blank one leaves the driver connected, + Healthy, and ingesting nothing. Validate() surfaces that inline; like every other knob + on this form it is ADVISORY — DriverConfigModal saves regardless — which is why the + model clamps on serialize instead of relying on the operator reading the notice. *@ +
+ + +
+ Subscribes spBv1.0/@(string.IsNullOrWhiteSpace(_form.SparkplugGroupId) ? "{GroupId}" : _form.SparkplugGroupId)/#. + No /, +, or #. +
+
+
+ + +
Default 15 s. How long browse/discovery collects births before calling the metric set stable.
+
+
+ + +
Sparkplug Host Application identity. Receive-only in this version.
+
+
+
+ + +
+
A detected seq gap asks the edge node to re-announce (NCMD), rather than running on stale metric state.
+
+ + +
+
+ + @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. *@ +
+ +
+ } +
} 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 820ce797..a003bc63 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 @@ -46,11 +46,21 @@ public sealed class MqttDriverFormModel private const string RawTagsKey = nameof(MqttDriverOptions.RawTags); /// - /// The property name for the P2 Sparkplug sub-object; never - /// authored here and preserved verbatim from the inbound blob. + /// The property name for the Sparkplug sub-object. Authored here + /// in mode and otherwise preserved verbatim from the inbound + /// blob — see for why the mode decides which of the two happens. /// private const string SparkplugKey = nameof(MqttDriverOptions.Sparkplug); + /// + /// Characters that cannot appear in a Sparkplug group id. It is a literal MQTT topic segment + /// inside the driver's one subscription filter spBv1.0/{GroupId}/#, so a / would + /// silently widen the filter and a +/# is not even legal mid-segment. Mirrors + /// MqttTagConfigModel's identical rule on the tag side, so the driver and the tags bound + /// to it cannot disagree about what a group id may be. + /// + private static readonly char[] SparkplugGroupIdIllegalChars = ['/', '+', '#']; + // --- Broker connection ---------------------------------------------------------------------- /// Broker hostname or IP address. @@ -122,9 +132,37 @@ public sealed class MqttDriverFormModel /// Plain mode: default subscription QoS applied when a tag's own qos is unset. public int DefaultQos { get; set; } = 1; + // --- Sparkplug B ---------------------------------------------------------------------------- + /// - /// The inbound blob's keys, retained so the P2 Sparkplug sub-object and any key a newer - /// driver adds survive a load→save. + /// Sparkplug mode: the group id. Required — it is the driver's entire subscription scope + /// (spBv1.0/{GroupId}/#), so a blank one subscribes to nothing and the driver ingests + /// nothing while still reporting a healthy broker connection. + /// + public string SparkplugGroupId { get; set; } = ""; + + /// + /// Sparkplug mode: the Host Application identity. Optional, and receive-only in this + /// version — see . + /// + 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. + /// + public bool SparkplugActAsPrimaryHost { get; set; } + + /// Sparkplug mode: answer a detected sequence-number gap with a rebirth request (NCMD). + public bool SparkplugRequestRebirthOnGap { get; set; } = true; + + /// Sparkplug mode: how long browse/discovery collects births before calling the set stable. + public int SparkplugBirthObservationWindowSeconds { get; set; } = 15; + + /// + /// The inbound blob's keys, retained so any key a newer driver adds survives a load→save. /// private JsonObject _bag = new(); @@ -172,6 +210,16 @@ public sealed class MqttDriverFormModel MaxPayloadBytes = o.MaxPayloadBytes, TopicPrefix = o.Plain?.TopicPrefix ?? "", DefaultQos = o.Plain?.DefaultQos ?? new MqttPlainOptions().DefaultQos, + SparkplugGroupId = o.Sparkplug?.GroupId ?? "", + SparkplugHostId = o.Sparkplug?.HostId ?? "", + SparkplugActAsPrimaryHost = o.Sparkplug?.ActAsPrimaryHost ?? false, + // Defaulted from the record, not from a literal, so the form shows the driver's own default + // for a blob that has no Sparkplug sub-object at all. + SparkplugRequestRebirthOnGap = + o.Sparkplug?.RequestRebirthOnGap ?? new MqttSparkplugOptions().RequestRebirthOnGap, + SparkplugBirthObservationWindowSeconds = + o.Sparkplug?.BirthObservationWindowSeconds + ?? new MqttSparkplugOptions().BirthObservationWindowSeconds, _bag = bag, }; } @@ -181,8 +229,9 @@ public sealed class MqttDriverFormModel /// range declares, so an operator who ignores /// and saves anyway still cannot persist a driver-bricking blob — a /// connectTimeoutSeconds: 0 is exactly the operator-authorable brick this repo has hit - /// before. stays null and - /// stays empty; both are handled by . + /// before. stays empty (the deploy artifact owns it) and + /// is emitted only in + /// mode; both are finished by . /// /// The clamped, driver-legal options record. public MqttDriverOptions ToOptions() => new() @@ -208,23 +257,45 @@ public sealed class MqttDriverFormModel TopicPrefix = TopicPrefix.Trim(), DefaultQos = Math.Clamp(DefaultQos, 0, 2), }, + // Null in Plain mode so ToJson leaves whatever the inbound blob had; see its remarks. + Sparkplug = Mode == MqttMode.SparkplugB + ? new MqttSparkplugOptions + { + GroupId = SparkplugGroupId.Trim(), + HostId = SparkplugHostId.Trim(), + ActAsPrimaryHost = SparkplugActAsPrimaryHost, + RequestRebirthOnGap = SparkplugRequestRebirthOnGap, + BirthObservationWindowSeconds = Math.Max(1, SparkplugBirthObservationWindowSeconds), + } + : null, }; /// /// Serialises the authored fields over the preserved key bag and returns the DriverConfig /// JSON. Keys are PascalCase and enums are names, because the serialisation runs through - /// — see the type remarks. Sparkplug is left exactly as the - /// inbound blob had it (P2 authors it) and RawTags is removed (the deploy artifact owns it). + /// — see the type remarks. RawTags is removed (the deploy + /// artifact owns it). /// + /// + /// Sparkplug is merged, never replaced, and only in Sparkplug mode. In + /// the sub-object is left exactly as the inbound blob had it, so an + /// operator who flips a Sparkplug driver to Plain to look at it and flips back does not lose the + /// group id. In the five fields this form owns are written + /// over the existing sub-object rather than replacing it, so a key a newer driver adds + /// inside Sparkplug survives an older AdminUI — the same preserve-what-you-do-not-author + /// discipline the top-level bag applies. + /// /// The serialised DriverConfig JSON string. public string ToJson() { var typed = JsonSerializer.SerializeToNode(ToOptions(), MqttJson.Options)!.AsObject(); - // Never let this form's (always-null / always-empty) placeholders overwrite what it does not own. - typed.Remove(SparkplugKey); + // Never let this form's always-empty placeholder overwrite what it does not own. typed.Remove(RawTagsKey); + // Pulled out of the flat copy loop below: unlike every other key, this one merges. + var authoredSparkplug = TakeIgnoringCase(typed, SparkplugKey) as JsonObject; + foreach (var (key, value) in typed.ToList()) { // Case-insensitive replace: MqttJson.Options binds a hand-edited camelCase blob happily, so @@ -233,6 +304,24 @@ public sealed class MqttDriverFormModel _bag[key] = value?.DeepClone(); } + if (authoredSparkplug is not null) + { + // Merge over whatever was already there, under the key name the bag already uses so a + // camelCase hand-edited blob does not end up with both "sparkplug" and "Sparkplug". + var existingKey = FindIgnoringCase(_bag, SparkplugKey) ?? SparkplugKey; + if (_bag[existingKey] is not JsonObject target) + { + target = new JsonObject(); + _bag[existingKey] = target; + } + + foreach (var (key, value) in authoredSparkplug.ToList()) + { + RemoveIgnoringCase(target, key); + target[key] = value?.DeepClone(); + } + } + RemoveIgnoringCase(_bag, RawTagsKey); return TagConfigJson.Serialize(_bag); } @@ -262,6 +351,28 @@ public sealed class MqttDriverFormModel } if (MaxPayloadBytes < 1) { return "Max payload bytes must be at least 1."; } if (DefaultQos is < 0 or > 2) { return "Default QoS must be 0, 1 or 2."; } + + if (Mode == MqttMode.SparkplugB) + { + var groupId = SparkplugGroupId.Trim(); + if (groupId.Length == 0) + { + return "A Sparkplug group ID is required — it is the driver's whole subscription scope " + + "(spBv1.0/{GroupId}/#), so a blank one ingests nothing."; + } + + if (groupId.IndexOfAny(SparkplugGroupIdIllegalChars) >= 0) + { + return $"Sparkplug group ID '{groupId}' contains a character ('/', '+', or '#') that " + + "cannot appear in an MQTT topic segment."; + } + + if (SparkplugBirthObservationWindowSeconds < 1) + { + return "Birth observation window must be at least 1 second."; + } + } + return null; } @@ -284,6 +395,29 @@ public sealed class MqttDriverFormModel } } + /// The actual key in matching case-insensitively. + /// The object to search. + /// The key name to look for. + /// The matching key as it is spelled in , or null. + private static string? FindIgnoringCase(JsonObject o, string name) + => o.Select(p => p.Key) + .FirstOrDefault(k => string.Equals(k, name, StringComparison.OrdinalIgnoreCase)); + + /// Removes and returns the value of a case-insensitively matched key. + /// The object to mutate. + /// The key name to take. + /// The detached value, or null when the key was absent (or its value was null). + private static JsonNode? TakeIgnoringCase(JsonObject o, string name) + { + if (FindIgnoringCase(o, name) is not { } key) { return null; } + + var value = o[key]; + o.Remove(key); + + // Detached before return: a node still parented to `typed` cannot be re-parented into the bag. + return value?.DeepClone(); + } + /// Binds the blob through the shared options; null on blank/malformed input. /// The raw DriverConfig JSON. /// The bound options, or null. diff --git a/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Components/Shared/Raw/RawBrowseModal.razor b/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Components/Shared/Raw/RawBrowseModal.razor index c0ec5ca4..144a112f 100644 --- a/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Components/Shared/Raw/RawBrowseModal.razor +++ b/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Components/Shared/Raw/RawBrowseModal.razor @@ -70,7 +70,22 @@ else {
- Browser open +
+ Browser open + @* An observation window ACCUMULATES — the session keeps recording every + message after the tree was first rendered — but the tree itself loads + exactly once, when it is created. Without this the operator sees the + t=0 snapshot forever: on MQTT the first render is usually empty (a + topic that has not published yet is invisible) and on Sparkplug it is + almost always empty, because births are never retained. Re-keying the + tree re-runs its root load against the SAME session, so nothing + reconnects and nothing already observed is lost. *@ + +
@@ -80,7 +95,7 @@
- @@ -245,6 +260,12 @@ private bool _busy; private List _commitErrors = new(); + /// + /// Bumped to force a fresh DriverBrowseTree against the same session — see the Refresh + /// button's remarks. Seeded from the session so re-opening a modal always starts a new generation. + /// + private int _treeGeneration; + // Request-rebirth affordance (MQTT/Sparkplug only). _canOperate is defence in depth — the real gate // is server-side in BrowserSessionService.RequestRebirthAsync, which fails closed. private bool _canOperate; @@ -519,6 +540,24 @@ } } + /// + /// Re-reads the root of the open session's observed tree. Only the tree is rebuilt: the session, + /// its broker connection and everything it has recorded are untouched, so this is strictly a + /// re-render and never re-observes from scratch. + /// + /// + /// The tag selection is deliberately kept — _selectedIds is keyed by browse node id + /// and a refresh does not renumber anything, so an operator who ticked leaves, waited for more + /// of the plant to appear, and refreshed does not silently lose the ticks. The rebirth scope IS + /// cleared, because the node it pointed at may no longer be in the rebuilt tree and a stale + /// armed scope is exactly the thing that panel's two-click confirm exists to prevent. + /// + private void RefreshTree() + { + _treeGeneration++; + ResetRebirthState(); + } + /// Clears every rebirth-panel field (modal re-open, browser close). private void ResetRebirthState() { diff --git a/tests/Server/ZB.MOM.WW.OtOpcUa.AdminUI.Tests/Uns/MqttDriverFormModelTests.cs b/tests/Server/ZB.MOM.WW.OtOpcUa.AdminUI.Tests/Uns/MqttDriverFormModelTests.cs index 5c5e1bd3..f360bdb8 100644 --- a/tests/Server/ZB.MOM.WW.OtOpcUa.AdminUI.Tests/Uns/MqttDriverFormModelTests.cs +++ b/tests/Server/ZB.MOM.WW.OtOpcUa.AdminUI.Tests/Uns/MqttDriverFormModelTests.cs @@ -184,13 +184,121 @@ public sealed class MqttDriverFormModelTests var json = MqttDriverFormModel.FromJson(inbound).ToJson(); var o = Parse(json); - // Sparkplug is P2 (Task 21+) — this form never authors it, and must never drop it. + // The blob has no Mode key, so it is Plain — and in Plain mode this form does not author the + // Sparkplug sub-object at all, so it must come back byte-identical. o["sparkplug"].ShouldNotBeNull(); o["sparkplug"]!["GroupId"]!.GetValue().ShouldBe("G1"); o["sparkplug"]!["ActAsPrimaryHost"]!.GetValue().ShouldBeTrue(); o["aFutureKey"]!.GetValue().ShouldBe(42); } + // --- Sparkplug B authoring ------------------------------------------------------------------- + // + // These pin the defect the P2 live gate found: MqttDriverForm shipped its Sparkplug branch as a + // "not implemented yet" placeholder, so once Sparkplug ingest landed the group id — the driver's + // ENTIRE subscription filter, spBv1.0/{GroupId}/# — could not be authored from the AdminUI at all. + // The driver deployed connected, Healthy, and ingesting nothing. + + [Fact] + public void Sparkplug_mode_round_trips_the_group_id_through_a_load_save_load() + { + var authored = MqttDriverFormModel.FromJson("""{"Host":"h"}"""); + authored.Mode = MqttMode.SparkplugB; + authored.SparkplugGroupId = "OtOpcUaSim"; + authored.SparkplugRequestRebirthOnGap = true; + + var reloaded = MqttDriverFormModel.FromJson(authored.ToJson()); + + reloaded.Mode.ShouldBe(MqttMode.SparkplugB); + reloaded.SparkplugGroupId.ShouldBe("OtOpcUaSim"); + reloaded.SparkplugRequestRebirthOnGap.ShouldBeTrue(); + } + + [Fact] + public void Sparkplug_mode_emits_a_group_id_the_driver_options_actually_bind() + { + // The whole point of the round-trip: what this form writes must deserialize back through the + // SAME shared MqttJson.Options the runtime factory uses, or the driver sees a null group. + var authored = MqttDriverFormModel.FromJson(null); + authored.Mode = MqttMode.SparkplugB; + authored.SparkplugGroupId = "OtOpcUaSim"; + + var options = JsonSerializer.Deserialize(authored.ToJson(), MqttJson.Options)!; + + options.Mode.ShouldBe(MqttMode.SparkplugB); + options.Sparkplug.ShouldNotBeNull(); + options.Sparkplug!.GroupId.ShouldBe("OtOpcUaSim"); + } + + [Fact] + public void Sparkplug_authoring_merges_over_the_subobject_rather_than_replacing_it() + { + // Same preserve-what-you-do-not-author discipline the top-level bag applies: a key a newer + // driver adds INSIDE the sub-object must survive an older AdminUI's save. + const string inbound = """ + {"Host":"h","Mode":"SparkplugB","sparkplug":{"GroupId":"Old","aFutureSpbKey":7}} + """; + + var model = MqttDriverFormModel.FromJson(inbound); + model.SparkplugGroupId = "New"; + + var o = Parse(model.ToJson()); + + o["sparkplug"]!["GroupId"]!.GetValue().ShouldBe("New"); + o["sparkplug"]!["aFutureSpbKey"]!.GetValue().ShouldBe(7); + } + + [Fact] + public void A_camelCase_sparkplug_key_is_merged_not_duplicated() + { + const string inbound = """{"Host":"h","Mode":"SparkplugB","sparkplug":{"GroupId":"Old"}}"""; + + var model = MqttDriverFormModel.FromJson(inbound); + model.SparkplugGroupId = "New"; + + var o = Parse(model.ToJson()); + + o.Count(p => string.Equals(p.Key, "Sparkplug", StringComparison.OrdinalIgnoreCase)).ShouldBe(1); + o["sparkplug"]!["GroupId"]!.GetValue().ShouldBe("New"); + } + + [Fact] + public void Plain_mode_never_writes_a_sparkplug_subobject_onto_a_blob_that_had_none() + { + var model = MqttDriverFormModel.FromJson("""{"Host":"h"}"""); + model.SparkplugGroupId = "TypedThenSwitchedAway"; // Mode is still Plain. + + HasKeyIgnoringCase(Parse(model.ToJson()), "Sparkplug").ShouldBeFalse(); + } + + [Fact] + public void A_blank_group_id_is_refused_in_sparkplug_mode_only() + { + var model = MqttDriverFormModel.FromJson("""{"Host":"h"}"""); + + // Plain does not care — it binds by topic. + model.Validate().ShouldBeNull(); + + model.Mode = MqttMode.SparkplugB; + model.Validate().ShouldNotBeNull(); + model.Validate()!.ShouldContain("Sparkplug group ID is required"); + } + + [Theory] + [InlineData("Plant/1")] + [InlineData("Plant+1")] + [InlineData("Plant#1")] + public void A_group_id_carrying_a_topic_metacharacter_is_refused(string groupId) + { + // It is a literal segment inside spBv1.0/{GroupId}/#, so '/' silently widens the filter and + // '+'/'#' are not even legal mid-segment. Mirrors MqttTagConfigModel's tag-side rule. + var model = MqttDriverFormModel.FromJson("""{"Host":"h"}"""); + model.Mode = MqttMode.SparkplugB; + model.SparkplugGroupId = groupId; + + model.Validate()!.ShouldContain("cannot appear in an MQTT topic segment"); + } + [Fact] public void A_camelCase_inbound_key_is_replaced_not_duplicated() {