fix(focas): browse-committed fixed-tree tags are readable (Gitea #540)

Authoring FOCAS tags through /raw -> Browse device... produced tags the driver could never read. Every committed tag read BadNodeIdUnknown forever, and the failure was silent end to end: the AdminUI accepted it, the deploy gate sealed green, and the driver reported Healthy with 0 errors while ignoring the tags. Net effect: the whole fixed-tree surface could not be published through any v3 authoring path. Found cutting wonder-app-vd03 over to v3 against a real Makino Pro 5 / Fanuc 30i, where 0 of 20 committed tags read while two hand-authored MACRO: tags on the same device read Good. A captured browse leaf's id IS its DriverAttributeInfo.FullName, and the fixed tree emitted "{deviceHost}/{path}" there -- an internal cache key, not an address. Both authoring paths wrote it verbatim into TagConfig.address, and FocasAddress.TryParse accepts no such form, so the driver skipped the tags at init. Fixed at the emitter rather than by stripping the host at commit, which fixes the browse-commit flow and the manual picker's tree-select together and leaves one canonical address form: - FocasAddress gains a fourth kind, FIXED:<path>, and moves to Contracts so the AdminUI and ControlPlane can parse without referencing the runtime driver (#526). A FIXED: read is served from the fixed-tree snapshot the poll loop already maintains -- no area read, no connection. Writes refuse: nothing in that tree is a writable location on the CNC. - The address carries no device. A tag's device comes from its Device row, and a second routing channel for one fact is what created this defect. - An authored FIXED: tag arms the fixed-tree poll loop by itself. Enabled defaults false and the browser force-enables it only for a browse, so the operator would otherwise browse, commit, deploy, and land on a driver whose loop never starts -- every tag Bad with a correct address. - Fixed-tree values are now dated from the last answered poll and go BadCommunicationError once it is stale (8 cadences, floored at 30s). The poll loop swallows a failed tick by design, so a dead CNC would otherwise serve its last snapshot as Good indefinitely -- harmless while nothing could bind to the tree, not harmless now. Guardrails, so this class cannot recur silently: - FocasTagConfigModel.Validate() runs the real parser instead of a non-empty check, and FocasTagDefinitionFactory.Inspect() warns at the deploy gate, which covers CSV/API config that never touches the modal. - The manual picker offered axis/spindle/program/status "parameter groups" -- none of which is a FOCAS addressing space -- and its own default value, axis:5, was unusable. It now composes PMC / PARAM: / MACRO: through FocasAddress, so what it emits is parseable by construction. Tests assert addresses ROUND-TRIP through the parser rather than matching an expected string: the previous FocasAddressBuilderTests pinned "axis:5" exactly, which is how a green suite coexisted with a picker nobody could use. Verified red-first against the old emitter. FOCAS 308, AdminUI 1095, ControlPlane 169, Core 581 -- all pass, 0 warnings. The remaining half of #540 -- a driver reporting Healthy/0 errors while skipping tags at init -- is fleet-wide (eight drivers do this) and is filed separately rather than patched per-driver.
2026-08-05 17:32:18 -04:00
parent 4e4619514c
commit b61fe38823
14 changed files with 1147 additions and 251 deletions
@@ -1,16 +1,66 @@
 using Shouldly;
 using Xunit;
 using ZB.MOM.WW.OtOpcUa.AdminUI.Components.Shared.Drivers.Pickers;
+using ZB.MOM.WW.OtOpcUa.Driver.FOCAS;

 namespace ZB.MOM.WW.OtOpcUa.AdminUI.Tests.Pickers;

+/// <summary>
+///     Gitea #540. The previous edition of this file asserted
+///     <c>Build("axis", 5) == "axis:5"</c> for four "parameter groups" that are not FOCAS
+///     addressing spaces — it pinned the exact string the driver could not parse, which is why
+///     a fully-green suite coexisted with a picker whose default value was unusable. Every case
+///     here therefore asserts the address ROUND-TRIPS through the driver's parser, not merely
+///     that it has the expected shape.
+/// </summary>
 public sealed class FocasAddressBuilderTests
 {
    [Theory]
-    [InlineData("axis", 5, "axis:5")]
-    [InlineData("spindle", 0, "spindle:0")]
-    [InlineData("program", 100, "program:100")]
-    [InlineData("status", 1, "status:1")]
-    public void Build_Canonical(string group, int parameterId, string expected)
-        => FocasAddressBuilder.Build(group, parameterId).ShouldBe(expected);
+    [InlineData(FocasManualAddressKind.Pmc, "R", 100, null, "R100")]
+    [InlineData(FocasManualAddressKind.Pmc, "X", 0, 0, "X0.0")]
+    [InlineData(FocasManualAddressKind.Pmc, "F", 20, 3, "F20.3")]
+    [InlineData(FocasManualAddressKind.Pmc, "r", 100, null, "R100")]      // letter is normalised
+    [InlineData(FocasManualAddressKind.Parameter, null, 1020, null, "PARAM:1020")]
+    [InlineData(FocasManualAddressKind.Parameter, null, 1815, 0, "PARAM:1815/0")]
+    [InlineData(FocasManualAddressKind.Macro, null, 500, null, "MACRO:500")]
+    [InlineData(FocasManualAddressKind.Macro, null, 500, 3, "MACRO:500")]  // macros have no bit form
+    public void Build_produces_an_address_the_driver_can_parse(
+        FocasManualAddressKind kind, string? letter, int number, int? bit, string expected)
+    {
+        var built = FocasAddressBuilder.Build(kind, letter, number, bit);
+
+        built.ShouldBe(expected);
+        FocasAddress.TryParse(built).ShouldNotBeNull();
+    }
+
+    [Theory]
+    [InlineData(FocasManualAddressKind.Pmc, "Z", 10, null)]     // not a PMC area letter
+    [InlineData(FocasManualAddressKind.Pmc, "", 10, null)]      // no area chosen
+    [InlineData(FocasManualAddressKind.Pmc, "R", 10, 8)]        // PMC bits are 0-7
+    [InlineData(FocasManualAddressKind.Pmc, "R", -1, null)]     // negative byte
+    [InlineData(FocasManualAddressKind.Parameter, null, 1815, 32)] // parameter bits are 0-31
+    [InlineData(FocasManualAddressKind.Parameter, null, -5, null)]
+    public void Build_returns_empty_rather_than_an_unusable_address(
+        FocasManualAddressKind kind, string? letter, int number, int? bit)
+        => FocasAddressBuilder.Build(kind, letter, number, bit).ShouldBeEmpty();
+
+    /// <summary>
+    ///     The contract that matters, stated over the whole offered surface rather than a sample:
+    ///     nothing the picker can compose is unreadable. Every PMC area the picker lists, at a
+    ///     whole-word address and at each legal bit.
+    /// </summary>
+    [Fact]
+    public void Every_offered_pmc_area_composes_a_parseable_address()
+    {
+        foreach (var letter in FocasAddressBuilder.PmcLetters)
+        {
+            foreach (int? bit in new int?[] { null, 0, 7 })
+            {
+                var built = FocasAddressBuilder.Build(FocasManualAddressKind.Pmc, letter, 12, bit);
+
+                built.ShouldNotBeEmpty($"PMC area '{letter}' bit '{bit}' composed nothing");
+                FocasAddress.TryParse(built).ShouldNotBeNull($"'{built}' does not parse");
+            }
+        }
+    }
 }
@@ -74,10 +74,36 @@ public sealed class FocasTagConfigModelTests
        new FocasTagConfigModel { Address = "" }.Validate().ShouldNotBeNull();
    }

-    [Fact]
-    public void Validate_returns_null_when_address_filled()
+    [Theory]
+    [InlineData("R100")]
+    [InlineData("X0.0")]
+    [InlineData("PARAM:1815/0")]
+    [InlineData("MACRO:500")]
+    [InlineData("FIXED:Identity/SeriesNumber")]
+    public void Validate_returns_null_for_every_address_form_the_driver_accepts(string address)
+        => new FocasTagConfigModel { Address = address }.Validate().ShouldBeNull();
+
+    /// <summary>
+    ///     Gitea #540. <c>Validate</c> used to accept any non-blank string, so a browse full
+    ///     reference sailed through authoring, through the deploy gate, and was then skipped at
+    ///     driver init — where the only trace was a Debug-level log line nobody reads. The first
+    ///     case here is verbatim what the AdminUI showed for one of the 20 dead tags on the Makino.
+    /// </summary>
+    [Theory]
+    [InlineData("10.201.31.5:8193/Identity/SeriesNumber")]
+    [InlineData("focas://10.201.31.5:8193/Axes/X/AbsolutePosition")]
+    [InlineData("axis:5")]
+    [InlineData("Z99")]                  // Z is not a PMC area letter
+    [InlineData("PARAM:notanumber")]
+    [InlineData("FIXED:")]
+    public void Validate_rejects_an_address_the_driver_would_skip(string address)
    {
-        new FocasTagConfigModel { Address = "R100" }.Validate().ShouldBeNull();
+        var error = new FocasTagConfigModel { Address = address }.Validate();
+
+        error.ShouldNotBeNull();
+        // The operator has to be able to act on it, so the message names the legal forms.
+        error.ShouldContain("PARAM:");
+        error.ShouldContain("FIXED:");
    }

    [Fact]
@@ -3,6 +3,7 @@ using System.Text.Json.Nodes;
 using Shouldly;
 using Xunit;
 using ZB.MOM.WW.OtOpcUa.AdminUI.Uns;
+using ZB.MOM.WW.OtOpcUa.AdminUI.Uns.TagEditors;
 using ZB.MOM.WW.OtOpcUa.Commons.Types;
 using ZB.MOM.WW.OtOpcUa.Configuration.Enums;
 using ZB.MOM.WW.OtOpcUa.Core.Abstractions;
@@ -107,6 +108,33 @@ public sealed class RawBrowseCommitMapperTests
    private static Dictionary<string, string> NativeTypeFields(string nativeType) =>
        new() { ["dataType"] = nativeType };

+    /// <summary>
+    ///     Gitea #540 — the commit path, judged by whether what it produces is authorable rather
+    ///     than by which key it lands under.
+    ///     <para>
+    ///     The mapper is not where that defect lived: it faithfully wrote the reference the FOCAS
+    ///     browser handed it, which was <c>{deviceHost}/{path}</c> and parses as nothing. The fix
+    ///     was in what the driver emits, so the check that keeps it fixed is that a leaf reference
+    ///     survives the commit AND passes the same validation the modal applies — a mapper test
+    ///     asserting only <c>o["address"] == reference</c> passed happily throughout.
+    ///     </para>
+    /// </summary>
+    /// <param name="reference">A leaf reference of the kind the FOCAS browser emits.</param>
+    [Theory]
+    [InlineData("FIXED:Identity/SeriesNumber")]
+    [InlineData("FIXED:Axes/X/AbsolutePosition")]
+    [InlineData("FIXED:Spindle/S1/Load")]
+    [InlineData("FIXED:Timers/PowerOnSeconds")]
+    [InlineData("MACRO:3901")]
+    public void A_committed_FOCAS_leaf_is_an_address_the_authoring_gate_accepts(string reference)
+    {
+        var json = RawBrowseCommitMapper.BuildTagConfig(DriverTypeNames.FOCAS, reference);
+
+        var model = FocasTagConfigModel.FromJson(json);
+        model.Address.ShouldBe(reference);
+        model.Validate().ShouldBeNull($"'{reference}' committed to a tag the driver would skip");
+    }
+
    /// <summary>
    ///     The committed <c>dataType</c> is the type the CONTROLLER reported, not the model default.
    ///     AB CIP / TwinCAT / FOCAS decode the wire bytes by this field, so a browsed REAL left at the