From b61fe388235480e7a82aa1c1cfe3d99122f0fdda Mon Sep 17 00:00:00 2001 From: Joseph Doherty Date: Wed, 5 Aug 2026 17:32:18 -0400 Subject: [PATCH] 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:, 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. --- docs/drivers/FOCAS.md | 58 ++- .../FocasAddress.cs | 178 +++++++++ .../FocasTagDefinitionFactory.cs | 15 + .../FocasAddress.cs | 99 ----- .../FocasCapabilityMatrix.cs | 6 +- .../FocasDriver.cs | 371 +++++++++++++----- .../Pickers/FOCASAddressPickerBody.razor | 80 +++- .../Drivers/Pickers/FocasAddressBuilder.cs | 76 +++- .../Uns/TagEditors/FocasTagConfigModel.cs | 28 +- .../FocasAddressFixedFormTests.cs | 90 +++++ .../FocasFixedTreeAddressingTests.cs | 275 +++++++++++++ .../Pickers/FocasAddressBuilderTests.cs | 62 ++- .../Uns/FocasTagConfigModelTests.cs | 32 +- .../Uns/RawBrowseCommitMapperTests.cs | 28 ++ 14 files changed, 1147 insertions(+), 251 deletions(-) create mode 100644 src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.FOCAS.Contracts/FocasAddress.cs delete mode 100644 src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.FOCAS/FocasAddress.cs create mode 100644 tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.FOCAS.Tests/FocasAddressFixedFormTests.cs create mode 100644 tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.FOCAS.Tests/FocasFixedTreeAddressingTests.cs diff --git a/docs/drivers/FOCAS.md b/docs/drivers/FOCAS.md index 83504579..326c11ae 100644 --- a/docs/drivers/FOCAS.md +++ b/docs/drivers/FOCAS.md @@ -87,12 +87,33 @@ on. | `X0.0` / `R100` / `R100.3` | PMC bit or byte | Letter + number; optional `.bit` for bit access | | `PARAM:1815` / `PARAM:1815/0` | CNC parameter | Number + optional axis index | | `MACRO:500` | Custom macro variable | System / user macro variable number | +| `FIXED:Identity/SeriesNumber` | Fixed-tree field | A path into the [fixed node tree](#fixed-node-tree) — read-only | + +The first three name a location the driver asks the CNC for on demand. +`FIXED:` does not: it names a field of the per-device fixed-tree snapshot the +driver's own poll loop already maintains, so a `FIXED:` read issues no wire +traffic. It exists because the fixed tree is the only part of the FOCAS surface +with no address of its own, and v3 publishes nodes only for authored tags — so +without a form to author, the fixed tree could not be published at all. + +**An address never carries the device.** A tag's device comes from its raw-tag +Device row; an address that also named a host would be a second, conflicting +routing channel. This is worth stating because it is exactly what went wrong in +[#540](https://gitea.dohertylan.com/dohertj2/lmxopcua/issues/540): browse leaves +advertised `{deviceHost}/{path}` as their reference, the browse-commit wrote that +into the address, and every committed tag read `BadNodeIdUnknown` forever. Addresses are validated against the per-device `Series` at `InitializeAsync` — a config referencing a number outside the documented range for that series fails at load time with an error message naming the limit. See [`docs/v2/focas-version-matrix.md`](../v2/focas-version-matrix.md) for the -authoritative range table. +authoritative range table. (`FIXED:` paths have no numeric range to check; a +path the machine does not expose reads `BadNodeIdUnknown`.) + +An address the parser cannot read is rejected **when you author it** — by the +`/raw` tag modal, and by the deploy-time inspector for config arriving via CSV +import or the API. It is not a warning you can deploy past into a driver that +skips the tag at startup. ## Backend selection @@ -150,7 +171,34 @@ timers without declaring per-address tags. } ``` -What gets populated (all under `FOCAS/{deviceHostAddress}/`): +`Enabled` defaults to **false** so an upgrade does not grow nodes on configs +that never asked for them. You do not have to set it to use the tree: **authoring +any `FIXED:` tag arms the poll loop by itself**, because committing those tags +already says you want it. The flag remains the way to publish the tree without +authoring anything (and the browser turns it on for the duration of a browse). + +### Authoring fixed-tree tags + +Browse the device (`/raw` → device → **Browse device…**, or the tag modal's +**Browse machine**) and commit the leaves you want. Each leaf carries its own +`FIXED:` address, so the commit produces working tags with no editing. The +manual composer next to the browser covers PMC / `PARAM:` / `MACRO:` only — +fixed-tree paths are discovered from the machine, not composed from parts. + +Two caveats worth knowing before you browse: + +- **A real CNC can report duplicate sibling names.** One observed Makino returns + two axis folders both called `AA`. Committing both is refused at commit time + (they would be the same RawPath), and the runtime cache is keyed by the same + name — so the `Axes/` subtree on such a machine cannot currently distinguish + them. Prefer the non-axis subtrees there until this is resolved. +- **Values are dated from the last answered poll, not from the read.** If a CNC + stops answering, its fixed-tree values go `BadCommunicationError` rather than + continuing to serve the last snapshot as `Good`; the budget is eight poll + cadences, floored at 30 s. + +What gets populated (all under `FOCAS/{deviceHostAddress}/`, addressed as +`FIXED:{subtree}/{field}`): | Subtree | Nodes | Source call | |---------|-------|-------------| @@ -195,9 +243,9 @@ The driver now attempts to auto-scale each axis position using the CNC's `cnc_ge - No upper-bound clamp on the figure magnitude returned by the CNC — a misbehaving figure could overflow `Math.Pow`; a max-figure guard should be added. - The negative-figure branch (falls back to manual `PositionDecimalPlaces`) is handled but not explicitly unit-tested. -**Still user-authored**: `PARAM:6711`, `MACRO:500`, `R100` etc. — specific -numbers whose meaning is MTB-specific. Those go under the device folder -alongside the fixed subtree. +**Still hand-authored**: `PARAM:6711`, `MACRO:500`, `R100` etc. — specific +numbers whose meaning is MTB-specific, and which no browse can discover. Those +go under the device folder alongside the fixed subtree. ## Alarm projection diff --git a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.FOCAS.Contracts/FocasAddress.cs b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.FOCAS.Contracts/FocasAddress.cs new file mode 100644 index 00000000..862f7e10 --- /dev/null +++ b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.FOCAS.Contracts/FocasAddress.cs @@ -0,0 +1,178 @@ +namespace ZB.MOM.WW.OtOpcUa.Driver.FOCAS; + +/// +/// Parsed FOCAS address covering the four addressing spaces a driver touches: +/// (letter + byte + optional bit — X0.0, R100, +/// F20.3), (CNC parameter number — +/// PARAM:1020, PARAM:1815/0 for bit 0), +/// (macro variable number — MACRO:100, MACRO:500), and +/// (a logical path into the driver's fixed tree — +/// FIXED:Identity/SeriesNumber, FIXED:Axes/X/AbsolutePosition). +/// +/// +/// PMC letters: X/Y (IO), F/G (signals between PMC + CNC), R (internal +/// relay), D (data table), C (counter), K (keep relay), A +/// (message display), E (extended relay), T (timer). Byte numbering is 0-based; +/// bit index when present is 0–7 and uses .N for PMC or /N for parameters. +/// +/// FIXED: is not an area read. The first three forms name a location the driver +/// asks the CNC for on demand; a FIXED: path names a field of the per-device fixed-tree +/// snapshot the driver's own poll loop already maintains (cnc_sysinfo, cnc_rddynamic2, +/// program info, timers, spindle data). It exists because the fixed tree is the only part of the +/// FOCAS surface with no address of its own, and v3 publishes nodes only for authored tags — so +/// without a form to author, the fixed tree could not be published at all (Gitea #540). +/// +/// +/// The path carries no device. A tag's device comes from its raw-tag Device row, and an +/// address that also named a host would be a second, conflicting routing channel — which is +/// precisely the defect #540 reported: the browse leaf's {host}/{path} full reference was +/// written into the address field verbatim and then failed to parse, so every browse-committed +/// fixed-tree tag read BadNodeIdUnknown forever. +/// +/// +public sealed record FocasAddress( + FocasAreaKind Kind, + string? PmcLetter, + int Number, + int? BitIndex, + string? FixedPath = null) +{ + /// The prefix that introduces a address. + public const string FixedPrefix = "FIXED:"; + + /// Gets the canonical string representation of this address. + public string Canonical => Kind switch + { + FocasAreaKind.Pmc => BitIndex is null + ? $"{PmcLetter}{Number}" + : $"{PmcLetter}{Number}.{BitIndex}", + FocasAreaKind.Parameter => BitIndex is null + ? $"PARAM:{Number}" + : $"PARAM:{Number}/{BitIndex}", + FocasAreaKind.Macro => $"MACRO:{Number}", + FocasAreaKind.Fixed => FixedPrefix + FixedPath, + _ => $"?{Number}", + }; + + /// + /// Builds the canonical authored address for a fixed-tree path. The one place the + /// FIXED: spelling is constructed, so the browse emitter and the parser cannot drift. + /// + /// The fixed-tree path, e.g. Identity/SeriesNumber. + /// The canonical authored address string. + public static string ForFixedPath(string path) => FixedPrefix + path; + + /// Attempts to parse a FOCAS address from the given string. + /// The address string to parse. + /// A FocasAddress if parsing succeeds; otherwise null. + public static FocasAddress? TryParse(string? value) + { + if (string.IsNullOrWhiteSpace(value)) return null; + var src = value.Trim(); + + if (src.StartsWith("PARAM:", StringComparison.OrdinalIgnoreCase)) + return ParseScoped(src["PARAM:".Length..], FocasAreaKind.Parameter, bitSeparator: '/'); + + if (src.StartsWith("MACRO:", StringComparison.OrdinalIgnoreCase)) + return ParseScoped(src["MACRO:".Length..], FocasAreaKind.Macro, bitSeparator: null); + + // Fixed-tree path. Tested BEFORE the PMC branch on purpose: "F" is a valid PMC letter, so a + // FIXED: address entering that branch would be diagnosed as a malformed PMC address rather + // than as the fixed-tree address it is. (It would still return null either way — this is + // about which rule owns the string, not about the result.) + if (src.StartsWith(FixedPrefix, StringComparison.OrdinalIgnoreCase)) + { + var path = NormalizeFixedPath(src[FixedPrefix.Length..]); + return path is null ? null : new FocasAddress(FocasAreaKind.Fixed, null, 0, null, path); + } + + // PMC path: letter + digits + optional .bit + if (src.Length < 2 || !char.IsLetter(src[0])) return null; + var letter = src[0..1].ToUpperInvariant(); + if (!IsValidPmcLetter(letter)) return null; + + var remainder = src[1..]; + int? bit = null; + var dotIdx = remainder.IndexOf('.'); + if (dotIdx >= 0) + { + if (!int.TryParse(remainder[(dotIdx + 1)..], out var bitValue) || bitValue is < 0 or > 7) + return null; + bit = bitValue; + remainder = remainder[..dotIdx]; + } + if (!int.TryParse(remainder, out var number) || number < 0) return null; + return new FocasAddress(FocasAreaKind.Pmc, letter, number, bit); + } + + private static FocasAddress? ParseScoped(string body, FocasAreaKind kind, char? bitSeparator) + { + int? bit = null; + if (bitSeparator is char sep) + { + var slashIdx = body.IndexOf(sep); + if (slashIdx >= 0) + { + if (!int.TryParse(body[(slashIdx + 1)..], out var bitValue) || bitValue is < 0 or > 31) + return null; + bit = bitValue; + body = body[..slashIdx]; + } + } + if (!int.TryParse(body, out var number) || number < 0) return null; + return new FocasAddress(kind, PmcLetter: null, number, bit); + } + + /// + /// Validates a fixed-tree path and returns it, or null when it is not a usable path. + /// Accepts Segment[/Segment…] where each segment is non-blank and carries no + /// : — a colon would collide with the PARAM:/MACRO:/FIXED: + /// prefix grammar and make the address ambiguous to read back. + /// + /// + /// Deliberately shape-only: it does NOT check the path against the fixed tree a particular + /// CNC actually exposes. What that tree contains is per-device and discovered at runtime + /// (spindles, program info and timers are each capability-gated), so a parser that judged + /// membership would have to reject addresses that are valid on the machine they were + /// authored against. An unknown path resolves to no value at read time, which is where the + /// device is known. + /// + private static string? NormalizeFixedPath(string body) + { + var path = body.Trim(); + if (path.Length == 0) return null; + if (path.Contains(':', StringComparison.Ordinal)) return null; + if (path[0] == '/' || path[^1] == '/') return null; + + foreach (var segment in path.Split('/')) + if (string.IsNullOrWhiteSpace(segment)) return null; + + return path; + } + + private static bool IsValidPmcLetter(string letter) => letter switch + { + "X" or "Y" or "F" or "G" or "R" or "D" or "C" or "K" or "A" or "E" or "T" => true, + _ => false, + }; +} + +/// Addressing-space kinds the driver understands. +public enum FocasAreaKind +{ + /// PMC area — letter + byte + optional bit (X0.0, R100). + Pmc, + + /// CNC parameter number (PARAM:1020, PARAM:1815/0). + Parameter, + + /// Macro variable number (MACRO:100). + Macro, + + /// + /// A logical path into the driver's per-device fixed tree (FIXED:Identity/SeriesNumber). + /// Served from the fixed-tree poll snapshot, never from an area read — and read-only, because + /// nothing in that tree is a writable location on the CNC. + /// + Fixed, +} diff --git a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.FOCAS.Contracts/FocasTagDefinitionFactory.cs b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.FOCAS.Contracts/FocasTagDefinitionFactory.cs index 7bef3447..3ece4da2 100644 --- a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.FOCAS.Contracts/FocasTagDefinitionFactory.cs +++ b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.FOCAS.Contracts/FocasTagDefinitionFactory.cs @@ -117,6 +117,21 @@ public static class FocasTagDefinitionFactory } var w = TagConfigJson.DescribeInvalidEnum(root, "dataType"); if (w is not null) warnings.Add(w); + // Gitea #540: an address the driver cannot parse is skipped at init and reads + // BadNodeIdUnknown forever, while driver health stays Healthy with zero errors. The + // AdminUI modal rejects it outright; this covers the paths that never touch the modal + // (CSV import, the API) by at least surfacing it on the deployment. + if (root.TryGetProperty("address", out var addr) + && addr.ValueKind == JsonValueKind.String + && addr.GetString() is { } address + && !string.IsNullOrWhiteSpace(address) + && FocasAddress.TryParse(address) is null) + { + warnings.Add( + $"FOCAS address '{address.Trim()}' is not parseable (expected X0.0 / R100, " + + "PARAM:1020, MACRO:500 or FIXED:Identity/SeriesNumber) — the driver will skip " + + "this tag at startup and it will read BadNodeIdUnknown."); + } if (root.TryGetProperty("writable", out var wr) && wr.ValueKind == JsonValueKind.True) warnings.Add("FOCAS wire is read-only — an authored 'writable:true' still fails at the wire with BadNotWritable."); } diff --git a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.FOCAS/FocasAddress.cs b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.FOCAS/FocasAddress.cs deleted file mode 100644 index df4434f0..00000000 --- a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.FOCAS/FocasAddress.cs +++ /dev/null @@ -1,99 +0,0 @@ -namespace ZB.MOM.WW.OtOpcUa.Driver.FOCAS; - -/// -/// Parsed FOCAS address covering the three addressing spaces a driver touches: -/// (letter + byte + optional bit — X0.0, R100, -/// F20.3), (CNC parameter number — -/// PARAM:1020, PARAM:1815/0 for bit 0), and -/// (macro variable number — MACRO:100, MACRO:500). -/// -/// -/// PMC letters: X/Y (IO), F/G (signals between PMC + CNC), R (internal -/// relay), D (data table), C (counter), K (keep relay), A -/// (message display), E (extended relay), T (timer). Byte numbering is 0-based; -/// bit index when present is 0–7 and uses .N for PMC or /N for parameters. -/// -public sealed record FocasAddress( - FocasAreaKind Kind, - string? PmcLetter, - int Number, - int? BitIndex) -{ - /// Gets the canonical string representation of this address. - public string Canonical => Kind switch - { - FocasAreaKind.Pmc => BitIndex is null - ? $"{PmcLetter}{Number}" - : $"{PmcLetter}{Number}.{BitIndex}", - FocasAreaKind.Parameter => BitIndex is null - ? $"PARAM:{Number}" - : $"PARAM:{Number}/{BitIndex}", - FocasAreaKind.Macro => $"MACRO:{Number}", - _ => $"?{Number}", - }; - - /// Attempts to parse a FOCAS address from the given string. - /// The address string to parse. - /// A FocasAddress if parsing succeeds; otherwise null. - public static FocasAddress? TryParse(string? value) - { - if (string.IsNullOrWhiteSpace(value)) return null; - var src = value.Trim(); - - if (src.StartsWith("PARAM:", StringComparison.OrdinalIgnoreCase)) - return ParseScoped(src["PARAM:".Length..], FocasAreaKind.Parameter, bitSeparator: '/'); - - if (src.StartsWith("MACRO:", StringComparison.OrdinalIgnoreCase)) - return ParseScoped(src["MACRO:".Length..], FocasAreaKind.Macro, bitSeparator: null); - - // PMC path: letter + digits + optional .bit - if (src.Length < 2 || !char.IsLetter(src[0])) return null; - var letter = src[0..1].ToUpperInvariant(); - if (!IsValidPmcLetter(letter)) return null; - - var remainder = src[1..]; - int? bit = null; - var dotIdx = remainder.IndexOf('.'); - if (dotIdx >= 0) - { - if (!int.TryParse(remainder[(dotIdx + 1)..], out var bitValue) || bitValue is < 0 or > 7) - return null; - bit = bitValue; - remainder = remainder[..dotIdx]; - } - if (!int.TryParse(remainder, out var number) || number < 0) return null; - return new FocasAddress(FocasAreaKind.Pmc, letter, number, bit); - } - - private static FocasAddress? ParseScoped(string body, FocasAreaKind kind, char? bitSeparator) - { - int? bit = null; - if (bitSeparator is char sep) - { - var slashIdx = body.IndexOf(sep); - if (slashIdx >= 0) - { - if (!int.TryParse(body[(slashIdx + 1)..], out var bitValue) || bitValue is < 0 or > 31) - return null; - bit = bitValue; - body = body[..slashIdx]; - } - } - if (!int.TryParse(body, out var number) || number < 0) return null; - return new FocasAddress(kind, PmcLetter: null, number, bit); - } - - private static bool IsValidPmcLetter(string letter) => letter switch - { - "X" or "Y" or "F" or "G" or "R" or "D" or "C" or "K" or "A" or "E" or "T" => true, - _ => false, - }; -} - -/// Addressing-space kinds the driver understands. -public enum FocasAreaKind -{ - Pmc, - Parameter, - Macro, -} diff --git a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.FOCAS/FocasCapabilityMatrix.cs b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.FOCAS/FocasCapabilityMatrix.cs index 4d11dff8..b8037e54 100644 --- a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.FOCAS/FocasCapabilityMatrix.cs +++ b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.FOCAS/FocasCapabilityMatrix.cs @@ -1,7 +1,7 @@ namespace ZB.MOM.WW.OtOpcUa.Driver.FOCAS; /// -/// Documented-API capability matrix — per CNC series, what ranges each +/// Documented-API capability matrix — per CNC series, what ranges each addressed /// supports. Authoritative source for the driver's /// pre-flight validation in . /// @@ -35,6 +35,10 @@ public static class FocasCapabilityMatrix FocasAreaKind.Macro => ValidateMacro(series, address.Number), FocasAreaKind.Parameter => ValidateParameter(series, address.Number), FocasAreaKind.Pmc => ValidatePmc(series, address.PmcLetter, address.Number), + // Fixed-tree paths are not area reads, so there is no per-series numeric range to check. + // What a given CNC actually exposes there is discovered at runtime and capability-gated + // per subtree; a path this machine does not have resolves to BadNodeIdUnknown at read. + FocasAreaKind.Fixed => null, _ => null, }; } diff --git a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.FOCAS/FocasDriver.cs b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.FOCAS/FocasDriver.cs index aea048ff..898d5a6d 100644 --- a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.FOCAS/FocasDriver.cs +++ b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.FOCAS/FocasDriver.cs @@ -49,6 +49,21 @@ public sealed class FocasDriver : IDriver, IReadable, IWritable, ITagDiscovery, // produced equipment tags are encountered for the first time. private readonly ConcurrentDictionary _parsedAddressesByTagName = new(StringComparer.OrdinalIgnoreCase); + /// + /// Whether this driver instance runs its per-device fixed-tree poll loop, and therefore + /// serves fixed-tree values. Computed at from the config flag + /// OR the presence of at least one authored tag. + /// + /// + /// The OR is load-bearing. FocasFixedTreeOptions.Enabled defaults to + /// so an upgrade does not grow nodes on configs that never asked for + /// them, and the universal browser force-enables it for the duration of a browse only. An + /// operator therefore browses the fixed tree, commits leaves from it, deploys — and lands on + /// a driver whose loop never starts, so every one of those tags reads Bad with a correct + /// address. Authoring a FIXED: tag IS the request for the tree; the config flag stays + /// as the way to publish it without authoring anything. + /// + private bool _fixedTreeActive; private FocasAlarmProjection? _alarmProjection; // _health is read/written from multiple threads (ReadAsync, WriteAsync, ProbeLoopAsync). // Volatile.Read/Write ensures every thread sees the latest reference without a lock — the @@ -248,7 +263,12 @@ public sealed class FocasDriver : IDriver, IReadable, IWritable, ITagDiscovery, if (_options.AlarmProjection.Enabled) _alarmProjection = new FocasAlarmProjection(this, _options.AlarmProjection.PollInterval, _logger); - if (_options.FixedTree.Enabled) + // See _fixedTreeActive: an authored FIXED: tag arms the loop on its own, because the + // config flag defaults off and the operator who committed those tags has already said + // what they want. + _fixedTreeActive = _options.FixedTree.Enabled + || _parsedAddressesByTagName.Values.Any(a => a.Kind == FocasAreaKind.Fixed); + if (_fixedTreeActive) { foreach (var state in _devices.Values) { @@ -306,6 +326,7 @@ public sealed class FocasDriver : IDriver, IReadable, IWritable, ITagDiscovery, _tagsByRawPath.Clear(); _resolver.Clear(); // no-op in v3 (the resolver reads the live table by closure); kept for symmetry _parsedAddressesByTagName.Clear(); + _fixedTreeActive = false; Volatile.Write(ref _health, new DriverHealth(DriverState.Unknown, Volatile.Read(ref _health).LastSuccessfulRead, null)); } @@ -377,7 +398,11 @@ public sealed class FocasDriver : IDriver, IReadable, IWritable, ITagDiscovery, // Fixed-tree T1 — fixed-tree references are synthesized from the cached // dynamic snapshot + sysinfo; no P/Invoke per Read since the poll loop // already fires them on cadence. - if (_options.FixedTree.Enabled && TryReadFixedTree(reference, now) is { } fx) + // + // This branch matches a reference that IS a "{host}/{path}" fixed-tree key. Authored + // tags do not take it — their reference is a RawPath — and are served further down by + // their FIXED: address instead (Gitea #540). + if (_fixedTreeActive && TryReadFixedTree(reference, now) is { } fx) { results[i] = fx; continue; @@ -396,8 +421,28 @@ public sealed class FocasDriver : IDriver, IReadable, IWritable, ITagDiscovery, try { - var client = await EnsureConnectedAsync(device, cancellationToken).ConfigureAwait(false); var parsed = ResolveParsedAddress(def); + + // A FIXED: tag names a field of this device's fixed-tree snapshot, which the + // fixed-tree loop already maintains — there is no area read to issue, and no + // connection to force. A null here means the path resolves to nothing on THIS + // device (an unsupported capability, a renamed axis, a typo), which is a genuine + // BadNodeIdUnknown; a device that has not answered yet reports a comms fault + // instead, because the address may well be fine. + if (parsed.Kind == FocasAreaKind.Fixed) + { + results[i] = TryReadFixedTreeOnDevice(device, parsed.FixedPath!, now) + ?? new DataValueSnapshot( + null, + device.LastFixedPollUtc is null + ? FocasStatusMapper.BadCommunicationError + : FocasStatusMapper.BadNodeIdUnknown, + null, + now); + continue; + } + + var client = await EnsureConnectedAsync(device, cancellationToken).ConfigureAwait(false); var (value, status) = await client.ReadAsync(parsed, def.DataType, cancellationToken).ConfigureAwait(false); results[i] = new DataValueSnapshot(value, status, now, now); @@ -459,8 +504,18 @@ public sealed class FocasDriver : IDriver, IReadable, IWritable, ITagDiscovery, try { - var client = await EnsureConnectedAsync(device, cancellationToken).ConfigureAwait(false); var parsed = ResolveParsedAddress(def); + + // Nothing in the fixed tree is a writable location on the CNC — it is a read-out of + // machine state (identity, positions, program, timers). Refuse before opening a + // connection rather than sending a write the wire would reject anyway. + if (parsed.Kind == FocasAreaKind.Fixed) + { + results[i] = new WriteResult(FocasStatusMapper.BadNotWritable); + continue; + } + + var client = await EnsureConnectedAsync(device, cancellationToken).ConfigureAwait(false); var status = await client.WriteAsync(parsed, def.DataType, w.Value, cancellationToken).ConfigureAwait(false); results[i] = new WriteResult(status); } @@ -525,31 +580,31 @@ public sealed class FocasDriver : IDriver, IReadable, IWritable, ITagDiscovery, // Fixed-tree T1 — Identity + Axes subtrees, populated once per session // from cnc_sysinfo + cnc_rdaxisname at init time and kept in DeviceState. - if (_options.FixedTree.Enabled + if (_fixedTreeActive && _devices.TryGetValue(device.HostAddress, out var state) && state.FixedTreeCache is { } cache) { var identity = deviceFolder.Folder("Identity", "Identity"); - EmitIdentityVariable(identity, device.HostAddress, "SeriesNumber", FocasDriverDataType.String); - EmitIdentityVariable(identity, device.HostAddress, "Version", FocasDriverDataType.String); - EmitIdentityVariable(identity, device.HostAddress, "MaxAxes", FocasDriverDataType.Int32); - EmitIdentityVariable(identity, device.HostAddress, "CncType", FocasDriverDataType.String); - EmitIdentityVariable(identity, device.HostAddress, "MtType", FocasDriverDataType.String); - EmitIdentityVariable(identity, device.HostAddress, "AxisCount", FocasDriverDataType.Int32); + EmitIdentityVariable(identity, "SeriesNumber", FocasDriverDataType.String); + EmitIdentityVariable(identity, "Version", FocasDriverDataType.String); + EmitIdentityVariable(identity, "MaxAxes", FocasDriverDataType.Int32); + EmitIdentityVariable(identity, "CncType", FocasDriverDataType.String); + EmitIdentityVariable(identity, "MtType", FocasDriverDataType.String); + EmitIdentityVariable(identity, "AxisCount", FocasDriverDataType.Int32); var axesFolder = deviceFolder.Folder("Axes", "Axes"); foreach (var axis in cache.Axes) { var axisFolder = axesFolder.Folder(axis.Display, axis.Display); - EmitAxisVariable(axisFolder, device.HostAddress, axis.Display, "AbsolutePosition"); - EmitAxisVariable(axisFolder, device.HostAddress, axis.Display, "MachinePosition"); - EmitAxisVariable(axisFolder, device.HostAddress, axis.Display, "RelativePosition"); - EmitAxisVariable(axisFolder, device.HostAddress, axis.Display, "DistanceToGo"); + EmitAxisVariable(axisFolder, axis.Display, "AbsolutePosition"); + EmitAxisVariable(axisFolder, axis.Display, "MachinePosition"); + EmitAxisVariable(axisFolder, axis.Display, "RelativePosition"); + EmitAxisVariable(axisFolder, axis.Display, "DistanceToGo"); if (cache.Capabilities.ServoLoad) - EmitAxisVariable(axisFolder, device.HostAddress, axis.Display, "ServoLoad"); + EmitAxisVariable(axisFolder, axis.Display, "ServoLoad"); } - EmitAxisVariable(axesFolder, device.HostAddress, "FeedRate", "Actual"); - EmitAxisVariable(axesFolder, device.HostAddress, "SpindleSpeed", "Actual"); + EmitAxisVariable(axesFolder, "FeedRate", "Actual"); + EmitAxisVariable(axesFolder, "SpindleSpeed", "Actual"); // Spindle subtree — one folder per discovered spindle, suppressed // entirely on series that don't export cnc_rdspdlname. Per-spindle @@ -563,9 +618,9 @@ public sealed class FocasDriver : IDriver, IReadable, IWritable, ITagDiscovery, var name = string.IsNullOrEmpty(s.Display) ? $"S{i + 1}" : s.Display; var spindleFolder = spindleRoot.Folder(name, name); if (cache.Capabilities.SpindleLoad) - EmitFixedVariable(spindleFolder, device.HostAddress, $"Spindle/{name}", "Load", DriverDataType.Int32); + EmitFixedVariable(spindleFolder, $"Spindle/{name}", "Load", DriverDataType.Int32); if (cache.Capabilities.SpindleMaxRpm && i < cache.SpindleMaxRpms.Count) - EmitFixedVariable(spindleFolder, device.HostAddress, $"Spindle/{name}", "MaxRpm", DriverDataType.Int32); + EmitFixedVariable(spindleFolder, $"Spindle/{name}", "MaxRpm", DriverDataType.Int32); } } @@ -573,26 +628,26 @@ public sealed class FocasDriver : IDriver, IReadable, IWritable, ITagDiscovery, if (cache.Capabilities.ProgramInfo) { var program = deviceFolder.Folder("Program", "Program"); - EmitFixedVariable(program, device.HostAddress, "Program", "Name", DriverDataType.String); - EmitFixedVariable(program, device.HostAddress, "Program", "ONumber", DriverDataType.Int32); - EmitFixedVariable(program, device.HostAddress, "Program", "Number", DriverDataType.Int32); - EmitFixedVariable(program, device.HostAddress, "Program", "MainNumber", DriverDataType.Int32); - EmitFixedVariable(program, device.HostAddress, "Program", "Sequence", DriverDataType.Int32); - EmitFixedVariable(program, device.HostAddress, "Program", "BlockCount", DriverDataType.Int32); + EmitFixedVariable(program, "Program", "Name", DriverDataType.String); + EmitFixedVariable(program, "Program", "ONumber", DriverDataType.Int32); + EmitFixedVariable(program, "Program", "Number", DriverDataType.Int32); + EmitFixedVariable(program, "Program", "MainNumber", DriverDataType.Int32); + EmitFixedVariable(program, "Program", "Sequence", DriverDataType.Int32); + EmitFixedVariable(program, "Program", "BlockCount", DriverDataType.Int32); var opMode = deviceFolder.Folder("OperationMode", "OperationMode"); - EmitFixedVariable(opMode, device.HostAddress, "OperationMode", "Mode", DriverDataType.Int32); - EmitFixedVariable(opMode, device.HostAddress, "OperationMode", "ModeText", DriverDataType.String); + EmitFixedVariable(opMode, "OperationMode", "Mode", DriverDataType.Int32); + EmitFixedVariable(opMode, "OperationMode", "ModeText", DriverDataType.String); } // Fixed-tree T3 — Timers subtree (power-on / operating / cutting / cycle). if (cache.Capabilities.Timers) { var timers = deviceFolder.Folder("Timers", "Timers"); - EmitFixedVariable(timers, device.HostAddress, "Timers", "PowerOnSeconds", DriverDataType.Float64); - EmitFixedVariable(timers, device.HostAddress, "Timers", "OperatingSeconds", DriverDataType.Float64); - EmitFixedVariable(timers, device.HostAddress, "Timers", "CuttingSeconds", DriverDataType.Float64); - EmitFixedVariable(timers, device.HostAddress, "Timers", "CycleSeconds", DriverDataType.Float64); + EmitFixedVariable(timers, "Timers", "PowerOnSeconds", DriverDataType.Float64); + EmitFixedVariable(timers, "Timers", "OperatingSeconds", DriverDataType.Float64); + EmitFixedVariable(timers, "Timers", "CuttingSeconds", DriverDataType.Float64); + EmitFixedVariable(timers, "Timers", "CycleSeconds", DriverDataType.Float64); } } @@ -622,10 +677,33 @@ public sealed class FocasDriver : IDriver, IReadable, IWritable, ITagDiscovery, private enum FocasDriverDataType { String, Int32, Float64 } + /// + /// Emit one Identity leaf. + /// + /// + /// + /// The emitted FullName is the authorable address, not the runtime cache key. + /// A captured browse leaf's id IS its FullName, and that string is what both the + /// /raw browse-commit mapper and the manual picker's tree-select write into the tag's + /// TagConfig.address. Emitting the host-prefixed cache key therefore authored an + /// address no FOCAS parser accepts: the driver skipped every such tag at init and each one + /// read BadNodeIdUnknown forever, while the deploy sealed green and driver health + /// stayed Healthy (Gitea #540). + /// + /// + /// The host is dropped rather than stripped later because a tag's device is already carried + /// by its raw-tag Device row. Two routing channels for one fact is what created the defect; + /// one channel is the fix. keeps the host for the internal + /// per-device caches, which do need to distinguish devices. + /// + /// + /// The address-space folder to emit into. + /// The identity field name. + /// The field's data type. private static void EmitIdentityVariable( - IAddressSpaceBuilder folder, string deviceHost, string field, FocasDriverDataType type) + IAddressSpaceBuilder folder, string field, FocasDriverDataType type) { - var fullName = FixedTreeReference(deviceHost, $"Identity/{field}"); + var fullName = FocasAddress.ForFixedPath($"Identity/{field}"); folder.Variable(field, field, new DriverAttributeInfo( FullName: fullName, DriverDataType: type switch @@ -643,10 +721,14 @@ public sealed class FocasDriver : IDriver, IReadable, IWritable, ITagDiscovery, NativeDataType: NativeName(type))); } + /// Emit one Axes leaf. See on the address shape. + /// The address-space folder to emit into. + /// The axis display name. + /// The per-axis field name. private static void EmitAxisVariable( - IAddressSpaceBuilder folder, string deviceHost, string axisName, string field) + IAddressSpaceBuilder folder, string axisName, string field) { - var fullName = FixedTreeReference(deviceHost, $"Axes/{axisName}/{field}"); + var fullName = FocasAddress.ForFixedPath($"Axes/{axisName}/{field}"); folder.Variable(field, field, new DriverAttributeInfo( FullName: fullName, DriverDataType: DriverDataType.Float64, @@ -675,13 +757,17 @@ public sealed class FocasDriver : IDriver, IReadable, IWritable, ITagDiscovery, /// /// Emit a variable under a named fixed-tree folder (Program, OperationMode, - /// …). Full-reference shape is {deviceHost}/{folderPath}/{field}. + /// …). Address shape is FIXED:{folderPath}/{field} — see + /// . /// + /// The address-space folder to emit into. + /// The fixed-tree folder path. + /// The field name. + /// The field's data type. private static void EmitFixedVariable( - IAddressSpaceBuilder folder, string deviceHost, string folderPath, - string field, DriverDataType type) + IAddressSpaceBuilder folder, string folderPath, string field, DriverDataType type) { - var fullName = FixedTreeReference(deviceHost, $"{folderPath}/{field}"); + var fullName = FocasAddress.ForFixedPath($"{folderPath}/{field}"); folder.Variable(field, field, new DriverAttributeInfo( FullName: fullName, DriverDataType: type, @@ -700,14 +786,19 @@ public sealed class FocasDriver : IDriver, IReadable, IWritable, ITagDiscovery, } /// - /// Canonical full-reference shape for a fixed-tree node. Keeps the device + /// Canonical runtime cache key for a fixed-tree node. Keeps the device /// host as a prefix so multi-device configs don't collide, and the rest is /// the path inside the tree. Matches what poll-loop snapshots publish + - /// what looks up. + /// what looks up. /// + /// + /// This is an internal key, not an authorable address — see + /// for why the two are deliberately different strings + /// (Gitea #540). + /// /// The host address of the device. /// The path within the fixed tree. - /// The canonical full reference for the node. + /// The canonical cache key for the node. internal static string FixedTreeReference(string deviceHost, string path) => $"{deviceHost}/{path}"; @@ -823,6 +914,10 @@ public sealed class FocasDriver : IDriver, IReadable, IWritable, ITagDiscovery, state.FixedTreeCache = new FocasFixedTreeCache( sys, [.. axes], [.. spindles], [.. spindleMaxRpms], caps); + // The bootstrap IS an answer from the device, and Identity comes entirely from it — + // so stamp liveness here as well as per tick, or a CNC whose axis poll never + // succeeds would serve a perfectly valid sysinfo as stale. + state.LastFixedPollUtc = DateTime.UtcNow; } catch (OperationCanceledException) when (ct.IsCancellationRequested) { return; } catch (Exception ex) @@ -872,6 +967,11 @@ public sealed class FocasDriver : IDriver, IReadable, IWritable, ITagDiscovery, if (i == 0) { firstAxisSnap = snap; PublishRateSnapshot(state, snap); } } + // The device answered this tick. Stamped after the axis pass rather than at the top + // of the loop: EnsureConnectedAsync can hand back a cached handle without any wire + // traffic, so it alone does not prove the CNC is still there. + state.LastFixedPollUtc = DateTime.UtcNow; + // Servo loads + spindle loads — both return bulk arrays, so folding // into the axis cadence is cheap. Each is gated by the bootstrap // capability probe — unsupported on this series = silent skip. @@ -1054,79 +1154,119 @@ public sealed class FocasDriver : IDriver, IReadable, IWritable, ITagDiscovery, foreach (var state in _devices.Values) { if (!reference.StartsWith(state.Options.HostAddress + "/", StringComparison.OrdinalIgnoreCase)) continue; - if (state.LastFixedSnapshots.TryGetValue(reference, out var raw)) - return new DataValueSnapshot(raw, FocasStatusMapper.Good, now, now); + if (TryReadFixedTreeOnDevice(state, reference[(state.Options.HostAddress.Length + 1)..], now) is { } hit) + return hit; + } + return null; + } - // Servo-load match: reference shape is "{host}/Axes/{name}/ServoLoad" - var suffixFull = reference[(state.Options.HostAddress.Length + 1)..]; - if (suffixFull.StartsWith("Axes/", StringComparison.Ordinal) && suffixFull.EndsWith("/ServoLoad", StringComparison.Ordinal)) - { - var axisName = suffixFull["Axes/".Length..^"/ServoLoad".Length]; - if (state.LastServoLoads.TryGetValue(axisName, out var load)) - return new DataValueSnapshot(load, FocasStatusMapper.Good, now, now); - } + /// + /// How long after a device's last answered fixed-tree poll its cached values stop being + /// served as Good. Eight axis-poll cadences, floored at 30 s so the sub-second default + /// (250 ms) does not make an ordinarily slow CNC flap. + /// + /// The configured fixed-tree poll interval. + /// The staleness budget. + internal static TimeSpan FixedTreeStaleAfter(TimeSpan pollInterval) => + TimeSpan.FromTicks(Math.Max(pollInterval.Ticks * 8, TimeSpan.FromSeconds(30).Ticks)); - // Spindle matches: "{host}/Spindle/{name}/Load" + "{host}/Spindle/{name}/MaxRpm" - if (suffixFull.StartsWith("Spindle/", StringComparison.Ordinal) - && state.FixedTreeCache is { } spindleCache) + /// + /// Resolves one fixed-tree (Identity/SeriesNumber, + /// Axes/X/AbsolutePosition, …) against a single device's caches. Returns null when + /// this device's fixed tree has no such path — the caller decides what that means. + /// + /// The device whose caches to read. + /// The fixed-tree path, with no device prefix. + /// The read timestamp. + /// The snapshot, or null when the path names nothing on this device. + /// + /// Values are dated from , not from + /// , and go BadCommunicationError once that stamp is older than + /// . A cache hit deliberately does NOT advance driver health: + /// the value came from the poll loop, not from this call, and crediting it would let a driver + /// serving nothing but fixed-tree tags report a fresh successful read while its CNC was gone. + /// + private DataValueSnapshot? TryReadFixedTreeOnDevice(DeviceState state, string path, DateTime now) + { + var polledUtc = state.LastFixedPollUtc; + if (polledUtc is null) return null; + var status = now - polledUtc.Value > FixedTreeStaleAfter(_options.FixedTree.PollInterval) + ? FocasStatusMapper.BadCommunicationError + : FocasStatusMapper.Good; + var sourceUtc = polledUtc.Value; + + if (state.LastFixedSnapshots.TryGetValue( + FixedTreeReference(state.Options.HostAddress, path), out var raw)) + return new DataValueSnapshot(raw, status, sourceUtc, now); + + // Servo-load match: path shape is "Axes/{name}/ServoLoad" + var suffixFull = path; + if (suffixFull.StartsWith("Axes/", StringComparison.Ordinal) && suffixFull.EndsWith("/ServoLoad", StringComparison.Ordinal)) + { + var axisName = suffixFull["Axes/".Length..^"/ServoLoad".Length]; + if (state.LastServoLoads.TryGetValue(axisName, out var load)) + return new DataValueSnapshot(load, status, sourceUtc, now); + } + + // Spindle matches: "Spindle/{name}/Load" + "Spindle/{name}/MaxRpm" + if (suffixFull.StartsWith("Spindle/", StringComparison.Ordinal) + && state.FixedTreeCache is { } spindleCache) + { + var tail = suffixFull["Spindle/".Length..]; + var slash = tail.IndexOf('/'); + if (slash > 0) { - var tail = suffixFull["Spindle/".Length..]; - var slash = tail.IndexOf('/'); - if (slash > 0) + var spindleName = tail[..slash]; + var field = tail[(slash + 1)..]; + var idx = -1; + for (var i = 0; i < spindleCache.Spindles.Count; i++) { - var spindleName = tail[..slash]; - var field = tail[(slash + 1)..]; - var idx = -1; - for (var i = 0; i < spindleCache.Spindles.Count; i++) + var s = spindleCache.Spindles[i]; + var display = string.IsNullOrEmpty(s.Display) ? $"S{i + 1}" : s.Display; + if (string.Equals(display, spindleName, StringComparison.OrdinalIgnoreCase)) { idx = i; break; } + } + if (idx >= 0) + { + object? value = field switch { - var s = spindleCache.Spindles[i]; - var display = string.IsNullOrEmpty(s.Display) ? $"S{i + 1}" : s.Display; - if (string.Equals(display, spindleName, StringComparison.OrdinalIgnoreCase)) { idx = i; break; } - } - if (idx >= 0) - { - object? value = field switch - { - "Load" => state.LastSpindleLoads.TryGetValue(idx, out var l) ? (object)l : null, - "MaxRpm" => idx < spindleCache.SpindleMaxRpms.Count ? (object)spindleCache.SpindleMaxRpms[idx] : null, - _ => null, - }; - if (value is not null) - return new DataValueSnapshot(value, FocasStatusMapper.Good, now, now); - } + "Load" => state.LastSpindleLoads.TryGetValue(idx, out var l) ? (object)l : null, + "MaxRpm" => idx < spindleCache.SpindleMaxRpms.Count ? (object)spindleCache.SpindleMaxRpms[idx] : null, + _ => null, + }; + if (value is not null) + return new DataValueSnapshot(value, status, sourceUtc, now); } } - // Identity strings + program / op-mode fields aren't cached as doubles — - // re-derive from the struct caches. - if (state.FixedTreeCache is { } cache) + } + // Identity strings + program / op-mode fields aren't cached as doubles — + // re-derive from the struct caches. + if (state.FixedTreeCache is { } cache) + { + var value = path switch { - var suffix = reference[(state.Options.HostAddress.Length + 1)..]; - var value = suffix switch - { - "Identity/SeriesNumber" => (object)cache.SysInfo.Series, - "Identity/Version" => cache.SysInfo.Version, - "Identity/MaxAxes" => cache.SysInfo.MaxAxis, - "Identity/CncType" => cache.SysInfo.CncType, - "Identity/MtType" => cache.SysInfo.MtType, - "Identity/AxisCount" => cache.SysInfo.AxesCount, - "Program/Name" => (object?)state.LastProgramInfo?.Name, - "Program/ONumber" => state.LastProgramInfo?.ONumber, - "Program/BlockCount" => state.LastProgramInfo?.BlockCount, - "Program/Number" => state.LastProgramAxisRef?.ProgramNumber, - "Program/MainNumber" => state.LastProgramAxisRef?.MainProgramNumber, - "Program/Sequence" => state.LastProgramAxisRef?.SequenceNumber, - "OperationMode/Mode" => state.LastProgramInfo?.Mode, - "OperationMode/ModeText" => state.LastProgramInfo is { } pi - ? FocasOpMode.ToText(pi.Mode) : null, - "Timers/PowerOnSeconds" => TimerValue(state, FocasTimerKind.PowerOn), - "Timers/OperatingSeconds" => TimerValue(state, FocasTimerKind.Operating), - "Timers/CuttingSeconds" => TimerValue(state, FocasTimerKind.Cutting), - "Timers/CycleSeconds" => TimerValue(state, FocasTimerKind.Cycle), - _ => null, - }; - if (value is not null) - return new DataValueSnapshot(value, FocasStatusMapper.Good, now, now); - } + "Identity/SeriesNumber" => (object)cache.SysInfo.Series, + "Identity/Version" => cache.SysInfo.Version, + "Identity/MaxAxes" => cache.SysInfo.MaxAxis, + "Identity/CncType" => cache.SysInfo.CncType, + "Identity/MtType" => cache.SysInfo.MtType, + "Identity/AxisCount" => cache.SysInfo.AxesCount, + "Program/Name" => (object?)state.LastProgramInfo?.Name, + "Program/ONumber" => state.LastProgramInfo?.ONumber, + "Program/BlockCount" => state.LastProgramInfo?.BlockCount, + "Program/Number" => state.LastProgramAxisRef?.ProgramNumber, + "Program/MainNumber" => state.LastProgramAxisRef?.MainProgramNumber, + "Program/Sequence" => state.LastProgramAxisRef?.SequenceNumber, + "OperationMode/Mode" => state.LastProgramInfo?.Mode, + "OperationMode/ModeText" => state.LastProgramInfo is { } pi + ? FocasOpMode.ToText(pi.Mode) : null, + "Timers/PowerOnSeconds" => TimerValue(state, FocasTimerKind.PowerOn), + "Timers/OperatingSeconds" => TimerValue(state, FocasTimerKind.Operating), + "Timers/CuttingSeconds" => TimerValue(state, FocasTimerKind.Cutting), + "Timers/CycleSeconds" => TimerValue(state, FocasTimerKind.Cycle), + _ => null, + }; + if (value is not null) + return new DataValueSnapshot(value, status, sourceUtc, now); } return null; } @@ -1357,6 +1497,21 @@ public sealed class FocasDriver : IDriver, IReadable, IWritable, ITagDiscovery, public CancellationTokenSource? FixedTreeCts { get; set; } /// Gets or sets the fixed-tree cache for this device. public FocasFixedTreeCache? FixedTreeCache { get; set; } + + /// + /// Gets or sets the UTC time the fixed-tree loop last got an answer out of this device + /// — set when the bootstrap completes and again after each poll tick's axis pass. Null + /// until the device has answered once. + /// + /// + /// This is what stops a dead CNC publishing its last values as Good forever. The + /// fixed-tree poll loop swallows a failed tick (LogDebug + retry) by design, so a device + /// that stops answering leaves the caches populated and unchanging with nothing marking + /// them stale. That was harmless while nothing could bind to the fixed tree; Gitea #540 + /// makes those nodes authorable, so reads are now dated from this stamp and go Bad once + /// it falls outside . + /// + public DateTime? LastFixedPollUtc { get; set; } /// /// Gets the last fixed-tree snapshots by field name. Double-typed so axis /// positions can carry the 10^PositionDecimalPlaces engineering-unit diff --git a/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Components/Shared/Drivers/Pickers/FOCASAddressPickerBody.razor b/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Components/Shared/Drivers/Pickers/FOCASAddressPickerBody.razor index 1f1cfe2d..5c0a17be 100644 --- a/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Components/Shared/Drivers/Pickers/FOCASAddressPickerBody.razor +++ b/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Components/Shared/Drivers/Pickers/FOCASAddressPickerBody.razor @@ -1,9 +1,16 @@ @* FOCAS address picker: - 1. Static parameter group + parameter ID → axis:5 (always available). + 1. Manual composition of the three addressed spaces — PMC (X0.0 / R100), CNC parameter + (PARAM:1020) and macro variable (MACRO:500) — always available. 2. (DriverOperator-gated, when the driver supports online discovery) Browse the FixedTree - device model captured by the universal DiscoveryDriverBrowser. FOCAS opens take a few - seconds longer than other drivers — its FixedTree cache fills post-connect and the browser - runs an UntilStable settle loop; the _opening spinner covers the wait. *@ + device model captured by the universal DiscoveryDriverBrowser, which yields FIXED: paths. + FOCAS opens take a few seconds longer than other drivers — its FixedTree cache fills + post-connect and the browser runs an UntilStable settle loop; the _opening spinner covers + the wait. + + Both branches now emit an address FocasAddress.TryParse accepts. Branch 1 used to offer + axis/spindle/program/status "parameter groups", which are not FOCAS addressing spaces, and + emitted "axis:5"; branch 2 committed the browse leaf's host-prefixed full reference. Neither + could be read (Gitea #540). *@ @implements IAsyncDisposable @using Microsoft.AspNetCore.Authorization @using ZB.MOM.WW.OtOpcUa.AdminUI.Browsing @@ -14,20 +21,43 @@ @inject IAuthorizationService AuthorizationService
-
- - + + +
+ @if (_kind == FocasManualAddressKind.Pmc) + { +
+ + +
+ }
- + + @bind="_number" @bind:after="OnChangedAsync" />
+ @if (_kind != FocasManualAddressKind.Macro) + { +
+ + +
+ }
@if (_canBrowse) @@ -68,7 +98,14 @@
Result: - @_built + @if (string.IsNullOrEmpty(_built)) + { + not a valid address + } + else + { + @_built + }
@code { @@ -79,8 +116,10 @@ /// Live accessor for the selected driver's DriverConfig JSON (browse connect config). [Parameter] public Func GetConfigJson { get; set; } = () => "{}"; - private string _group = "axis"; - private int _parameterId = 0; + private FocasManualAddressKind _kind = FocasManualAddressKind.Pmc; + private string _pmcLetter = "R"; + private int _number = 0; + private int? _bitIndex; private string _built = ""; private Guid _token = Guid.Empty; @@ -94,7 +133,7 @@ var authResult = await AuthorizationService.AuthorizeAsync(auth.User, null, AdminUiPolicies.DriverOperator); // CanBrowse constructs a throwaway driver — evaluate once, not per render. _canBrowse = authResult.Succeeded && BrowserService.CanBrowse(DriverType, GetConfigJson() ?? "{}"); - _built = FocasAddressBuilder.Build(_group, _parameterId); + _built = FocasAddressBuilder.Build(_kind, _pmcLetter, _number, _bitIndex); await CurrentAddressChanged.InvokeAsync(_built); } @@ -125,8 +164,9 @@ private async Task OnTreeSelectAsync(BrowseNode node) { - // Folders fire the same callback — commit only on a leaf. The captured NodeId is the - // driver FullName (FixedTree address), which the group/id builder cannot reconstruct, so + // Folders fire the same callback — commit only on a leaf. The captured NodeId is the driver + // FullName, which for a fixed-tree leaf is its canonical FIXED: address (Gitea #540); the + // manual builder composes only the three addressed spaces and cannot reconstruct it, so // commit it directly rather than through OnChangedAsync. if (node.Kind != BrowseNodeKind.Leaf) return; _built = node.NodeId; @@ -135,7 +175,7 @@ private async Task OnChangedAsync() { - _built = FocasAddressBuilder.Build(_group, _parameterId); + _built = FocasAddressBuilder.Build(_kind, _pmcLetter, _number, _bitIndex); await CurrentAddressChanged.InvokeAsync(_built); } diff --git a/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Components/Shared/Drivers/Pickers/FocasAddressBuilder.cs b/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Components/Shared/Drivers/Pickers/FocasAddressBuilder.cs index 94e2708d..3c52af1d 100644 --- a/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Components/Shared/Drivers/Pickers/FocasAddressBuilder.cs +++ b/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Components/Shared/Drivers/Pickers/FocasAddressBuilder.cs @@ -1,16 +1,76 @@ +using ZB.MOM.WW.OtOpcUa.Driver.FOCAS; + namespace ZB.MOM.WW.OtOpcUa.AdminUI.Components.Shared.Drivers.Pickers; /// -/// Pure static helper that converts a FOCAS parameter group + parameter ID -/// into the canonical address string (e.g. axis:5). +/// The addressing spaces the manual FOCAS picker can compose. Mirrors the parseable subset of +/// : is deliberately absent because a +/// fixed-tree path is discovered from the machine, not composed from parts — the picker's browse +/// tree supplies those. +/// +public enum FocasManualAddressKind +{ + /// PMC area — letter + byte + optional bit. + Pmc, + + /// CNC parameter number + optional bit. + Parameter, + + /// Macro variable number. + Macro, +} + +/// +/// Pure static helper that composes a canonical FOCAS address from the manual picker's parts. /// Extracted so unit tests can call it without bUnit. /// +/// +/// +/// It builds through rather than formatting a string, so what +/// it emits is parseable by construction and the two cannot drift. +/// +/// +/// It previously emitted $"{group}:{parameterId}" over a group list of +/// axis/spindle/program/status — none of which is a FOCAS addressing space — so the +/// picker's own default value, axis:5, was an address the driver rejects. Together with +/// the browse-commit defect this meant every UI-assisted authoring path produced unusable tags, +/// and only hand-typed addresses worked at all (Gitea #540). +/// +/// public static class FocasAddressBuilder { - /// Builds the canonical FOCAS address string from a parameter group and parameter ID. - /// The FOCAS parameter group (e.g. "axis"). - /// The parameter ID within the group. - /// The canonical address string in group:parameterId form (e.g. axis:5). - public static string Build(string group, int parameterId) - => $"{group}:{parameterId}"; + /// The PMC area letters the driver accepts, in the order the picker offers them. + public static IReadOnlyList PmcLetters { get; } = + ["X", "Y", "F", "G", "R", "D", "C", "K", "A", "E", "T"]; + + /// + /// Composes the canonical FOCAS address string, or an empty string when the parts do not form a + /// valid address (an out-of-range bit, a negative number, an unknown PMC letter). + /// + /// Which addressing space the address is in. + /// The PMC area letter; ignored unless is + /// . + /// The PMC byte, parameter number or macro number. + /// Optional bit index — 0–7 for PMC, 0–31 for a parameter. Null for a + /// whole-word address; always ignored for a macro, which has no bit form. + /// The canonical address, or an empty string when the parts are not addressable. + public static string Build( + FocasManualAddressKind kind, string? pmcLetter, int number, int? bitIndex) + { + FocasAddress? candidate = kind switch + { + FocasManualAddressKind.Pmc => new FocasAddress( + FocasAreaKind.Pmc, (pmcLetter ?? "").Trim().ToUpperInvariant(), number, bitIndex), + FocasManualAddressKind.Parameter => new FocasAddress( + FocasAreaKind.Parameter, null, number, bitIndex), + FocasManualAddressKind.Macro => new FocasAddress( + FocasAreaKind.Macro, null, number, null), + _ => null, + }; + + // Round-trip rather than trust the parts: the record accepts any values, the parser is the + // authority on which of them address anything, and an empty result is what the picker shows + // as "no address yet" instead of a plausible-looking string the driver would silently skip. + return candidate?.Canonical is { } text && FocasAddress.TryParse(text) is not null ? text : ""; + } } diff --git a/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Uns/TagEditors/FocasTagConfigModel.cs b/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Uns/TagEditors/FocasTagConfigModel.cs index ac159401..b7a22faf 100644 --- a/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Uns/TagEditors/FocasTagConfigModel.cs +++ b/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Uns/TagEditors/FocasTagConfigModel.cs @@ -47,6 +47,32 @@ public sealed class FocasTagConfigModel /// Validation hook; returns an error message or null when the model is valid. /// An error message describing the validation failure, or null when the model is valid. + /// + /// This runs the driver's real parser, not a non-empty check (Gitea #540). It used to + /// accept any non-blank string, which is how 20 tags carrying a browse full reference + /// (10.201.31.5:8193/Identity/SeriesNumber) were authored, deployed and sealed green — + /// the driver then skipped every one at init and they read BadNodeIdUnknown forever + /// with no signal anywhere. An address that cannot be parsed cannot be polled, so the + /// rejection belongs here, where the operator is still looking at the field. + /// lives in the driver's Contracts assembly precisely + /// so this project can call it without referencing the runtime driver (Gitea #526). + /// public string? Validate() - => string.IsNullOrWhiteSpace(Address) ? "Address is required." : null; + { + if (string.IsNullOrWhiteSpace(Address)) return "Address is required."; + return FocasAddress.TryParse(Address) is null ? DescribeUnparseableAddress(Address) : null; + } + + /// + /// The operator-facing rejection message for an address the FOCAS parser will not accept, + /// naming the four legal forms. Shared with the deploy-time inspector so the modal and the + /// deploy gate say the same thing about the same string. + /// + /// The rejected address. + /// The message. + internal static string DescribeUnparseableAddress(string address) => + $"'{address.Trim()}' is not a FOCAS address. Expected a PMC address (X0.0, R100, F20.3), " + + "a CNC parameter (PARAM:1020, PARAM:1815/0), a macro variable (MACRO:500), or a fixed-tree " + + "path (FIXED:Identity/SeriesNumber). A browse full reference including the device host is " + + "not an address — commit the leaf from the browser rather than pasting its id."; } diff --git a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.FOCAS.Tests/FocasAddressFixedFormTests.cs b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.FOCAS.Tests/FocasAddressFixedFormTests.cs new file mode 100644 index 00000000..2bbb14a0 --- /dev/null +++ b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.FOCAS.Tests/FocasAddressFixedFormTests.cs @@ -0,0 +1,90 @@ +using Shouldly; +using Xunit; +using ZB.MOM.WW.OtOpcUa.Driver.FOCAS; + +namespace ZB.MOM.WW.OtOpcUa.Driver.FOCAS.Tests; + +/// +/// The FIXED: address form added by Gitea #540, and the guarantee that adding it did not +/// disturb the three forms that were already there. +/// +[Trait("Category", "Unit")] +public sealed class FocasAddressFixedFormTests +{ + [Theory] + [InlineData("FIXED:Identity/SeriesNumber", "Identity/SeriesNumber")] + [InlineData("FIXED:Axes/X/AbsolutePosition", "Axes/X/AbsolutePosition")] + [InlineData("FIXED:Timers/PowerOnSeconds", "Timers/PowerOnSeconds")] + [InlineData("FIXED:Spindle/S1/Load", "Spindle/S1/Load")] + [InlineData("fixed:Program/Name", "Program/Name")] // prefix is case-insensitive + [InlineData(" FIXED:Program/Name ", "Program/Name")] // outer whitespace tolerated + public void Parses_a_fixed_tree_path(string input, string expectedPath) + { + var parsed = FocasAddress.TryParse(input); + + parsed.ShouldNotBeNull(); + parsed.Kind.ShouldBe(FocasAreaKind.Fixed); + parsed.FixedPath.ShouldBe(expectedPath); + parsed.Canonical.ShouldBe("FIXED:" + expectedPath); + } + + [Theory] + [InlineData("FIXED:")] // no path + [InlineData("FIXED: ")] + [InlineData("FIXED:/Identity/Series")] // leading separator + [InlineData("FIXED:Identity/Series/")] // trailing separator + [InlineData("FIXED:Identity//Series")] // empty segment + [InlineData("FIXED:Identity/ /Series")] // blank segment + [InlineData("FIXED:MACRO:500")] // a colon would make the grammar ambiguous + public void Rejects_a_path_that_is_not_addressable(string input) + => FocasAddress.TryParse(input).ShouldBeNull(); + + /// + /// The exact strings the defect produced. Each names a real fixed-tree field but carries the + /// device host, which is the tag's Device row's job — so they must still be rejected rather + /// than quietly accepted with the host treated as a path segment. + /// + [Theory] + [InlineData("10.201.31.5:8193/Identity/SeriesNumber")] + [InlineData("focas://10.201.31.5:8193/Axes/X/AbsolutePosition")] + [InlineData("axis:5")] // the manual picker's old default + public void Still_rejects_what_the_defect_authored(string input) + => FocasAddress.TryParse(input).ShouldBeNull(); + + /// + /// F is a valid PMC area letter, so the FIXED: prefix has to be tested before + /// the PMC branch. Both forms must survive that ordering. + /// + [Fact] + public void The_fixed_prefix_does_not_shadow_the_pmc_F_area() + { + FocasAddress.TryParse("F20.3")!.Kind.ShouldBe(FocasAreaKind.Pmc); + FocasAddress.TryParse("F20.3")!.PmcLetter.ShouldBe("F"); + FocasAddress.TryParse("FIXED:Program/Name")!.Kind.ShouldBe(FocasAreaKind.Fixed); + } + + [Theory] + [InlineData("R100")] + [InlineData("X0.0")] + [InlineData("PARAM:1020")] + [InlineData("PARAM:1815/0")] + [InlineData("MACRO:500")] + public void The_pre_existing_forms_still_round_trip(string input) + { + var parsed = FocasAddress.TryParse(input); + + parsed.ShouldNotBeNull(); + parsed.Kind.ShouldNotBe(FocasAreaKind.Fixed); + parsed.Canonical.ShouldBe(input); + } + + /// The one place the FIXED: spelling is built, so emitter and parser cannot drift. + [Fact] + public void ForFixedPath_produces_what_TryParse_accepts() + { + var built = FocasAddress.ForFixedPath("Identity/SeriesNumber"); + + built.ShouldBe("FIXED:Identity/SeriesNumber"); + FocasAddress.TryParse(built)!.FixedPath.ShouldBe("Identity/SeriesNumber"); + } +} diff --git a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.FOCAS.Tests/FocasFixedTreeAddressingTests.cs b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.FOCAS.Tests/FocasFixedTreeAddressingTests.cs new file mode 100644 index 00000000..f8f4a99a --- /dev/null +++ b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.FOCAS.Tests/FocasFixedTreeAddressingTests.cs @@ -0,0 +1,275 @@ +using Shouldly; +using Xunit; +using ZB.MOM.WW.OtOpcUa.Core.Abstractions; +using ZB.MOM.WW.OtOpcUa.Driver.FOCAS; + +namespace ZB.MOM.WW.OtOpcUa.Driver.FOCAS.Tests; + +/// +/// Gitea #540 — authoring a fixed-tree tag through /raw → Browse device… produced a tag +/// the driver could never read. The browse leaf's full reference was {deviceHost}/{path}, +/// the commit mapper wrote it verbatim into TagConfig.address, and +/// accepts no such form — so the driver skipped the tag at +/// init and every read returned BadNodeIdUnknown, indefinitely, while the deploy sealed +/// green and driver health stayed Healthy with zero errors. +/// +/// Found on a real Makino Pro 5 / Fanuc 30i: 0 of 20 browse-committed tags read, while two +/// hand-authored MACRO: tags on the same device read Good. +/// +/// +[Trait("Category", "Unit")] +public sealed class FocasFixedTreeAddressingTests +{ + private const string Host = "focas://10.0.0.5:8193"; + + private static FocasDriver NewDriver( + FakeFocasClientFactory factory, + bool fixedTreeEnabled, + params FocasTagDefinition[] tags) => + new(new FocasDriverOptions + { + Devices = [new FocasDeviceOptions(Host)], + Probe = new FocasProbeOptions { Enabled = false }, + FixedTree = new FocasFixedTreeOptions { Enabled = fixedTreeEnabled }, + RawTags = FocasRawTags.Entries(tags), + }, "drv-1", factory); + + private static async Task WaitForAsync(Func predicate, TimeSpan timeout) + { + var deadline = DateTime.UtcNow + timeout; + while (DateTime.UtcNow < deadline) + { + if (predicate()) return true; + await Task.Delay(25); + } + return predicate(); + } + + /// + /// The headline regression, stated over the WHOLE emitted tree rather than a sample: nothing + /// the browser can offer is unauthorable. Sampling is what let this ship — the tree looked + /// right, and no test ever asked whether the strings hanging off it were addresses. + /// + [Fact] + public async Task Every_browse_emitted_leaf_carries_an_address_the_driver_can_parse() + { + var drv = NewDriver(new FakeFocasClientFactory(), fixedTreeEnabled: true); + await drv.InitializeAsync("{}", CancellationToken.None); + (await WaitForAsync( + () => drv.GetDeviceState(Host)?.FixedTreeCache is not null, TimeSpan.FromSeconds(5))) + .ShouldBeTrue("the fixed-tree cache never filled, so the tree under test was empty"); + + var builder = new RecordingBuilder(); + await drv.DiscoverAsync(builder, CancellationToken.None); + + builder.Variables.ShouldNotBeEmpty(); + foreach (var (browseName, info) in builder.Variables) + { + FocasAddress.TryParse(info.FullName) + .ShouldNotBeNull($"leaf '{browseName}' emitted '{info.FullName}', which is not an address"); + // The specific shape that broke it: the device host leaking into the address, where it + // is both unparseable and a second routing channel competing with the tag's Device row. + info.FullName.ShouldNotContain("10.0.0.5"); + } + + await drv.ShutdownAsync(CancellationToken.None); + } + + /// + /// The end-to-end claim the issue is about: take what the browser offers, author it as a tag, + /// and read it. Anything short of this passed before the fix. + /// + [Fact] + public async Task A_leaf_authored_from_the_browse_tree_reads_its_value() + { + // 1. Browse. + var browseDrv = NewDriver(new FakeFocasClientFactory(), fixedTreeEnabled: true); + await browseDrv.InitializeAsync("{}", CancellationToken.None); + (await WaitForAsync( + () => browseDrv.GetDeviceState(Host)?.FixedTreeCache is not null, TimeSpan.FromSeconds(5))) + .ShouldBeTrue(); + var builder = new RecordingBuilder(); + await browseDrv.DiscoverAsync(builder, CancellationToken.None); + await browseDrv.ShutdownAsync(CancellationToken.None); + + var seriesLeaf = builder.Variables.Single(v => v.Info.FullName == "FIXED:Identity/SeriesNumber"); + + // 2. Commit it as a raw tag, exactly as the browse-commit mapper does — the leaf's full + // reference becomes the tag's address, and its RawPath is where the operator put it. + var runDrv = NewDriver( + new FakeFocasClientFactory(), + fixedTreeEnabled: true, + new FocasTagDefinition( + "focas-cnc/cnc/FOCAS/cnc/Identity/SeriesNumber", + Host, + seriesLeaf.Info.FullName, + FocasDataType.String, + Writable: false)); + await runDrv.InitializeAsync("{}", CancellationToken.None); + (await WaitForAsync( + () => runDrv.GetDeviceState(Host)?.FixedTreeCache is not null, TimeSpan.FromSeconds(5))) + .ShouldBeTrue(); + + // 3. Read it by RawPath, which is the reference the v3 server uses. + var read = await runDrv.ReadAsync(["focas-cnc/cnc/FOCAS/cnc/Identity/SeriesNumber"], CancellationToken.None); + + read[0].StatusCode.ShouldBe(FocasStatusMapper.Good); + read[0].Value.ShouldBe("30i"); // FakeFocasClient.SysInfo.Series + await runDrv.ShutdownAsync(CancellationToken.None); + } + + /// + /// FixedTree.Enabled defaults to false and the universal browser force-enables it only + /// for the duration of a browse. Without this, an operator would browse the tree, commit + /// leaves, deploy, and land on a driver whose poll loop never starts — every tag Bad, with a + /// perfectly correct address. Authoring a FIXED: tag is the request for the tree. + /// + [Fact] + public async Task An_authored_fixed_tag_arms_the_poll_loop_with_the_config_flag_off() + { + var drv = NewDriver( + new FakeFocasClientFactory(), + fixedTreeEnabled: false, + new FocasTagDefinition("raw/Program", Host, "FIXED:Program/Name", FocasDataType.String, Writable: false)); + + await drv.InitializeAsync("{}", CancellationToken.None); + + (await WaitForAsync( + () => drv.GetDeviceState(Host)?.FixedTreeCache is not null, TimeSpan.FromSeconds(5))) + .ShouldBeTrue("an authored FIXED: tag must start the fixed-tree loop on its own"); + await drv.ShutdownAsync(CancellationToken.None); + } + + /// A driver with no fixed-tree tags and the flag off must still not start the loop. + [Fact] + public async Task An_ordinary_tag_does_not_arm_the_poll_loop() + { + var drv = NewDriver( + new FakeFocasClientFactory(), + fixedTreeEnabled: false, + new FocasTagDefinition("raw/Run", Host, "R100", FocasDataType.Byte)); + + await drv.InitializeAsync("{}", CancellationToken.None); + await Task.Delay(300); + + drv.GetDeviceState(Host)!.FixedTreeCache.ShouldBeNull(); + await drv.ShutdownAsync(CancellationToken.None); + } + + /// + /// A path this machine does not expose is a genuine BadNodeIdUnknown — the address + /// parsed, it simply names nothing here (a capability the series lacks, a renamed axis). + /// + [Fact] + public async Task A_fixed_path_the_machine_does_not_expose_reads_BadNodeIdUnknown() + { + var drv = NewDriver( + new FakeFocasClientFactory(), + fixedTreeEnabled: true, + new FocasTagDefinition("raw/Ghost", Host, "FIXED:Axes/Q/AbsolutePosition", FocasDataType.Float64, Writable: false)); + await drv.InitializeAsync("{}", CancellationToken.None); + (await WaitForAsync( + () => drv.GetDeviceState(Host)?.LastFixedPollUtc is not null, TimeSpan.FromSeconds(5))) + .ShouldBeTrue(); + + var read = await drv.ReadAsync(["raw/Ghost"], CancellationToken.None); + + read[0].StatusCode.ShouldBe(FocasStatusMapper.BadNodeIdUnknown); + await drv.ShutdownAsync(CancellationToken.None); + } + + /// + /// Nothing in the fixed tree is a writable location on the CNC — it is a read-out of machine + /// state. Refused before a connection is opened. + /// + [Fact] + public async Task A_fixed_tree_tag_refuses_writes() + { + var drv = NewDriver( + new FakeFocasClientFactory(), + fixedTreeEnabled: true, + // Writable:true on purpose — the refusal must come from the address kind, not from the + // tag's own flag, or an operator could author their way past it. + new FocasTagDefinition("raw/Series", Host, "FIXED:Identity/SeriesNumber", FocasDataType.String, Writable: true)); + await drv.InitializeAsync("{}", CancellationToken.None); + + var results = await drv.WriteAsync( + [new WriteRequest("raw/Series", "nope")], CancellationToken.None); + + results[0].StatusCode.ShouldBe(FocasStatusMapper.BadNotWritable); + await drv.ShutdownAsync(CancellationToken.None); + } + + /// + /// A dead CNC must not keep serving its last fixed-tree values as Good. The poll loop + /// swallows a failed tick by design (LogDebug + retry), so nothing else marks the caches + /// stale — and unlike an area read, a fixed-tree read issues no traffic that could fail. + /// This was harmless while nothing could bind to the tree; it is not harmless now. + /// + [Fact] + public async Task Fixed_tree_values_go_bad_once_the_device_stops_answering() + { + var drv = NewDriver( + new FakeFocasClientFactory(), + fixedTreeEnabled: true, + new FocasTagDefinition("raw/Series", Host, "FIXED:Identity/SeriesNumber", FocasDataType.String, Writable: false)); + await drv.InitializeAsync("{}", CancellationToken.None); + (await WaitForAsync( + () => drv.GetDeviceState(Host)?.LastFixedPollUtc is not null, TimeSpan.FromSeconds(5))) + .ShouldBeTrue(); + + (await drv.ReadAsync(["raw/Series"], CancellationToken.None))[0] + .StatusCode.ShouldBe(FocasStatusMapper.Good); + + // Age the last answered poll past the staleness budget. Reaching into DeviceState rather + // than sleeping keeps this deterministic and instant; the budget itself is asserted below. + var state = drv.GetDeviceState(Host)!; + state.FixedTreeCts?.Cancel(); + state.LastFixedPollUtc = DateTime.UtcNow + - FocasDriver.FixedTreeStaleAfter(TimeSpan.FromMilliseconds(250)) + - TimeSpan.FromSeconds(1); + + var stale = await drv.ReadAsync(["raw/Series"], CancellationToken.None); + + stale[0].StatusCode.ShouldBe(FocasStatusMapper.BadCommunicationError); + await drv.ShutdownAsync(CancellationToken.None); + } + + /// + /// The staleness budget tracks the poll cadence but never drops below 30 s, so an ordinarily + /// slow CNC on the sub-second default cadence does not flap between Good and Bad. + /// + [Theory] + [InlineData(250, 30_000)] // default cadence — the 30 s floor wins + [InlineData(1_000, 30_000)] // 8 s of cadence still under the floor + [InlineData(10_000, 80_000)] // slow cadence — 8x the interval wins + public void Stale_budget_is_eight_cadences_floored_at_thirty_seconds(int pollMs, int expectedMs) + => FocasDriver.FixedTreeStaleAfter(TimeSpan.FromMilliseconds(pollMs)) + .ShouldBe(TimeSpan.FromMilliseconds(expectedMs)); + + /// Records what DiscoverAsync emits, flattening the folder tree. + private sealed class RecordingBuilder : IAddressSpaceBuilder + { + public List<(string BrowseName, string DisplayName)> Folders { get; } = []; + public List<(string BrowseName, DriverAttributeInfo Info)> Variables { get; } = []; + + public IAddressSpaceBuilder Folder(string browseName, string displayName) + { Folders.Add((browseName, displayName)); return this; } + + public IVariableHandle Variable(string browseName, string displayName, DriverAttributeInfo info) + { Variables.Add((browseName, info)); return new Handle(info.FullName); } + + public void AddProperty(string name, DriverDataType type, object? value) { } + + private sealed class Handle(string fullRef) : IVariableHandle + { + public string FullReference => fullRef; + public IAlarmConditionSink MarkAsAlarmCondition(AlarmConditionInfo info) => new NullSink(); + } + + private sealed class NullSink : IAlarmConditionSink + { + public void OnTransition(AlarmEventArgs args) { } + } + } +} diff --git a/tests/Server/ZB.MOM.WW.OtOpcUa.AdminUI.Tests/Pickers/FocasAddressBuilderTests.cs b/tests/Server/ZB.MOM.WW.OtOpcUa.AdminUI.Tests/Pickers/FocasAddressBuilderTests.cs index d95d057a..1edd0323 100644 --- a/tests/Server/ZB.MOM.WW.OtOpcUa.AdminUI.Tests/Pickers/FocasAddressBuilderTests.cs +++ b/tests/Server/ZB.MOM.WW.OtOpcUa.AdminUI.Tests/Pickers/FocasAddressBuilderTests.cs @@ -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; +/// +/// Gitea #540. The previous edition of this file asserted +/// Build("axis", 5) == "axis:5" 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. +/// 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(); + + /// + /// 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. + /// + [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"); + } + } + } } diff --git a/tests/Server/ZB.MOM.WW.OtOpcUa.AdminUI.Tests/Uns/FocasTagConfigModelTests.cs b/tests/Server/ZB.MOM.WW.OtOpcUa.AdminUI.Tests/Uns/FocasTagConfigModelTests.cs index f7dbca89..c5d2ae6c 100644 --- a/tests/Server/ZB.MOM.WW.OtOpcUa.AdminUI.Tests/Uns/FocasTagConfigModelTests.cs +++ b/tests/Server/ZB.MOM.WW.OtOpcUa.AdminUI.Tests/Uns/FocasTagConfigModelTests.cs @@ -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(); + + /// + /// Gitea #540. Validate 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. + /// + [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] diff --git a/tests/Server/ZB.MOM.WW.OtOpcUa.AdminUI.Tests/Uns/RawBrowseCommitMapperTests.cs b/tests/Server/ZB.MOM.WW.OtOpcUa.AdminUI.Tests/Uns/RawBrowseCommitMapperTests.cs index d4cb90b9..79aebcb0 100644 --- a/tests/Server/ZB.MOM.WW.OtOpcUa.AdminUI.Tests/Uns/RawBrowseCommitMapperTests.cs +++ b/tests/Server/ZB.MOM.WW.OtOpcUa.AdminUI.Tests/Uns/RawBrowseCommitMapperTests.cs @@ -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 NativeTypeFields(string nativeType) => new() { ["dataType"] = nativeType }; + /// + /// Gitea #540 — the commit path, judged by whether what it produces is authorable rather + /// than by which key it lands under. + /// + /// The mapper is not where that defect lived: it faithfully wrote the reference the FOCAS + /// browser handed it, which was {deviceHost}/{path} 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 o["address"] == reference passed happily throughout. + /// + /// + /// A leaf reference of the kind the FOCAS browser emits. + [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"); + } + /// /// The committed dataType 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