From cc140593a02ee2e05c20633dd28234a6d12446c7 Mon Sep 17 00:00:00 2001 From: Joseph Doherty Date: Thu, 22 Oct 2026 14:15:35 -0400 Subject: [PATCH] fix(focas): refuse String on PMC/PARAM/MACRO in the factory, editor and CLI (R07:U8) FocasDataType.String was authorable on any address, but no area read returns text: PMC fell through to a byte read, PARAM to Int32, MACRO to a double, and the String node received a boxed number at Good. One rule, FocasTagDefinitionFactory.DescribeDataTypeMismatch, now backs every path: - FromTagConfig rejects the pair, so the driver skips + reports it (BadNodeIdUnknown, never a Good number) and Inspect names the reason at deploy; - FocasTagConfigModel.Validate refuses it on the typed fields; - the AdminUI editor only offers String where it is readable and shows the rule's text inline (kept visible for a loaded blob that already carries it); - the CLI read/subscribe/probe/write commands refuse it at the boundary. String stays valid on FIXED: paths (Identity/Program name/mode text) and is ignored on ALARM: tags; an unparseable address keeps its own diagnosis. Razor change (FocasTagConfigEditor.razor) needs the Wave 4 live check. Refs #740 Claude-Session: https://claude.ai/code/session_01Ub9hGmYLQvn9XETzw7CMsT --- .../Commands/ProbeCommand.cs | 1 + .../Commands/ReadCommand.cs | 3 +- .../Commands/SubscribeCommand.cs | 3 +- .../Commands/WriteCommand.cs | 3 +- .../FocasCommandBase.cs | 13 +++ .../FocasTagDefinitionFactory.cs | 27 +++++ .../Uns/TagEditors/FocasTagConfigEditor.razor | 18 ++- .../Uns/TagEditors/FocasTagConfigModel.cs | 4 + .../FocasCommandBaseValidationTests.cs | 21 ++++ .../FocasStringDataTypeTests.cs | 104 ++++++++++++++++++ .../Uns/FocasTagConfigModelTests.cs | 22 ++++ 11 files changed, 214 insertions(+), 5 deletions(-) create mode 100644 tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.FOCAS.Tests/FocasStringDataTypeTests.cs diff --git a/src/Drivers/Cli/ZB.MOM.WW.OtOpcUa.Driver.FOCAS.Cli/Commands/ProbeCommand.cs b/src/Drivers/Cli/ZB.MOM.WW.OtOpcUa.Driver.FOCAS.Cli/Commands/ProbeCommand.cs index d2d46458..9c31bffe 100644 --- a/src/Drivers/Cli/ZB.MOM.WW.OtOpcUa.Driver.FOCAS.Cli/Commands/ProbeCommand.cs +++ b/src/Drivers/Cli/ZB.MOM.WW.OtOpcUa.Driver.FOCAS.Cli/Commands/ProbeCommand.cs @@ -28,6 +28,7 @@ public sealed class ProbeCommand : FocasCommandBase { ConfigureLogging(); ValidateOptions(); + ValidateDataType(Address, DataType); var ct = console.RegisterCancellationHandler(); var probeTag = new FocasTagDefinition( diff --git a/src/Drivers/Cli/ZB.MOM.WW.OtOpcUa.Driver.FOCAS.Cli/Commands/ReadCommand.cs b/src/Drivers/Cli/ZB.MOM.WW.OtOpcUa.Driver.FOCAS.Cli/Commands/ReadCommand.cs index df042344..9218a2c9 100644 --- a/src/Drivers/Cli/ZB.MOM.WW.OtOpcUa.Driver.FOCAS.Cli/Commands/ReadCommand.cs +++ b/src/Drivers/Cli/ZB.MOM.WW.OtOpcUa.Driver.FOCAS.Cli/Commands/ReadCommand.cs @@ -19,7 +19,7 @@ public sealed class ReadCommand : FocasCommandBase /// Gets the data type to interpret the address as. [CommandOption("type", 't', Description = - "Bit / Byte / Int16 / Int32 / Float32 / Float64 / String (default Int16).")] + "Bit / Byte / Int16 / Int32 / Float32 / Float64 / String (default Int16). String only on FIXED: paths.")] public FocasDataType DataType { get; init; } = FocasDataType.Int16; /// @@ -28,6 +28,7 @@ public sealed class ReadCommand : FocasCommandBase ConfigureLogging(); // Validate numeric option ranges before any driver work. ValidateOptions(); + ValidateDataType(Address, DataType); var ct = console.RegisterCancellationHandler(); var tagName = SynthesiseTagName(Address, DataType); diff --git a/src/Drivers/Cli/ZB.MOM.WW.OtOpcUa.Driver.FOCAS.Cli/Commands/SubscribeCommand.cs b/src/Drivers/Cli/ZB.MOM.WW.OtOpcUa.Driver.FOCAS.Cli/Commands/SubscribeCommand.cs index 1a3cb5f4..ee126fc0 100644 --- a/src/Drivers/Cli/ZB.MOM.WW.OtOpcUa.Driver.FOCAS.Cli/Commands/SubscribeCommand.cs +++ b/src/Drivers/Cli/ZB.MOM.WW.OtOpcUa.Driver.FOCAS.Cli/Commands/SubscribeCommand.cs @@ -18,7 +18,7 @@ public sealed class SubscribeCommand : FocasCommandBase /// Gets the data type of the address. [CommandOption("type", 't', Description = - "Bit / Byte / Int16 / Int32 / Float32 / Float64 / String (default Int16).")] + "Bit / Byte / Int16 / Int32 / Float32 / Float64 / String (default Int16). String only on FIXED: paths.")] public FocasDataType DataType { get; init; } = FocasDataType.Int16; /// Gets the polling interval in milliseconds. @@ -33,6 +33,7 @@ public sealed class SubscribeCommand : FocasCommandBase // --interval-ms) before any driver work so a zero/negative interval surfaces as a // clean CommandException rather than a tight-spinning poll loop. ValidateOptions(IntervalMs); + ValidateDataType(Address, DataType); var ct = console.RegisterCancellationHandler(); var tagName = ReadCommand.SynthesiseTagName(Address, DataType); diff --git a/src/Drivers/Cli/ZB.MOM.WW.OtOpcUa.Driver.FOCAS.Cli/Commands/WriteCommand.cs b/src/Drivers/Cli/ZB.MOM.WW.OtOpcUa.Driver.FOCAS.Cli/Commands/WriteCommand.cs index bd383041..22cfb431 100644 --- a/src/Drivers/Cli/ZB.MOM.WW.OtOpcUa.Driver.FOCAS.Cli/Commands/WriteCommand.cs +++ b/src/Drivers/Cli/ZB.MOM.WW.OtOpcUa.Driver.FOCAS.Cli/Commands/WriteCommand.cs @@ -20,7 +20,7 @@ public sealed class WriteCommand : FocasCommandBase /// Gets the data type of the value to write. [CommandOption("type", 't', Description = - "Bit / Byte / Int16 / Int32 / Float32 / Float64 / String (default Int16).")] + "Bit / Byte / Int16 / Int32 / Float32 / Float64 / String (default Int16). String only on FIXED: paths.")] public FocasDataType DataType { get; init; } = FocasDataType.Int16; /// Gets the value to write. @@ -37,6 +37,7 @@ public sealed class WriteCommand : FocasCommandBase // port/timeout surfaces as a clean CommandException rather than an opaque // downstream exception. ValidateOptions(); + ValidateDataType(Address, DataType); var ct = console.RegisterCancellationHandler(); var tagName = ReadCommand.SynthesiseTagName(Address, DataType); diff --git a/src/Drivers/Cli/ZB.MOM.WW.OtOpcUa.Driver.FOCAS.Cli/FocasCommandBase.cs b/src/Drivers/Cli/ZB.MOM.WW.OtOpcUa.Driver.FOCAS.Cli/FocasCommandBase.cs index 62965d52..e57ce8bf 100644 --- a/src/Drivers/Cli/ZB.MOM.WW.OtOpcUa.Driver.FOCAS.Cli/FocasCommandBase.cs +++ b/src/Drivers/Cli/ZB.MOM.WW.OtOpcUa.Driver.FOCAS.Cli/FocasCommandBase.cs @@ -89,4 +89,17 @@ public abstract class FocasCommandBase : DriverCommandBase throw new CliFx.Exceptions.CommandException( $"--interval-ms must be positive (got {iv})."); } + + /// + /// Refuses an --address / --type pair the driver cannot serve (String on a PMC / PARAM / MACRO + /// address — the driver skips such a tag, so the command would only ever print BadNodeIdUnknown). Same + /// rule as the driver and the AdminUI editor: . + /// + /// The FOCAS address. + /// The data type. + protected static void ValidateDataType(string address, FocasDataType type) + { + if (FocasTagDefinitionFactory.DescribeDataTypeMismatch(type, address) is { } error) + throw new CliFx.Exceptions.CommandException(error); + } } 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 d495dad1..b65150f6 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 @@ -53,6 +53,9 @@ public static class FocasTagDefinitionFactory // Strict enum read: a typo'd dataType rejects the tag (→ BadNodeIdUnknown) // instead of silently defaulting to a wrong-width Good. if (!TagConfigJson.TryReadEnumStrict(root, "dataType", FocasDataType.Int32, out var dataType)) return false; + // A data type no read of this address can produce (String on PMC / PARAM / MACRO) rejects the tag: + // served, it published a boxed number at Good into a String node. + if (DescribeDataTypeMismatch(dataType, address) is not null) return false; // FOCAS authored tags carry no `writable` key (writability lives on the Tag entity / node ACL): // absent ⇒ read-only. An explicit writable:true is honoured for the driver's internal write gate // (default-false, so a missing key stays read-only — the note is preserved). @@ -73,6 +76,26 @@ public static class FocasTagDefinitionFactory catch (InvalidOperationException) { return false; } } + /// + /// The ONE address/data-type compatibility rule, shared by the mapper (and so the driver's init and the + /// deploy inspector), the AdminUI tag editor and the CLI. is only + /// servable from the fixed tree (Identity / Program name / mode text …): a PMC, parameter or macro read + /// returns a number, which a String node would receive at Good. An ALARM: tag has no value, so its + /// data type is irrelevant; an unparseable address is left to the address check, which names it better. + /// + /// The authored data type. + /// The authored address. + /// The reason the pair cannot be served, or when it can. + public static string? DescribeDataTypeMismatch(FocasDataType dataType, string? address) + { + if (dataType != FocasDataType.String) return null; + var parsed = FocasAddress.TryParse(address); + if (parsed is null || parsed.Kind is FocasAreaKind.Fixed or FocasAreaKind.Alarm) return null; + return $"FOCAS data type 'String' is not readable at '{address!.Trim()}' — PMC, PARAM: and MACRO: reads " + + "return numbers (String is only valid on FIXED: paths such as FIXED:Identity/SeriesNumber). " + + "Choose Bit / Byte / Int16 / Int32 / Float32 / Float64."; + } + /// /// The tag's cross-driver alarm object, or when it carries none. /// Lenient like the host's TagConfigIntent: a non-integer severity is simply absent. @@ -236,6 +259,10 @@ public static class FocasTagDefinitionFactory } if (TagConfigJson.DescribeInvalidEnum(root, "dataType") is { } w) rejections.Add(w); + else if (addr.ValueKind == JsonValueKind.String + && TagConfigJson.TryReadEnumStrict(root, "dataType", FocasDataType.Int32, out var dataType) + && DescribeDataTypeMismatch(dataType, addr.GetString()) is { } mismatch) + rejections.Add(mismatch + " The driver will skip this tag and it will read BadNodeIdUnknown."); } /// diff --git a/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Components/Shared/Uns/TagEditors/FocasTagConfigEditor.razor b/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Components/Shared/Uns/TagEditors/FocasTagConfigEditor.razor index 8911b438..03fd6a15 100644 --- a/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Components/Shared/Uns/TagEditors/FocasTagConfigEditor.razor +++ b/src/Server/ZB.MOM.WW.OtOpcUa.AdminUI/Components/Shared/Uns/TagEditors/FocasTagConfigEditor.razor @@ -15,8 +15,22 @@
+ @foreach (var v in Enum.GetValues()) + { + @* String is only readable from the fixed tree (FocasTagDefinitionFactory.DescribeDataTypeMismatch): + offered there, and kept visible when a loaded blob already carries it so the select never lies. *@ + if (v != FocasDataType.String || _m.DataType == FocasDataType.String + || FocasTagDefinitionFactory.DescribeDataTypeMismatch(v, _m.Address) is null) + { + + } + } + + @if (FocasTagDefinitionFactory.DescribeDataTypeMismatch(_m.DataType, _m.Address) is { } typeError) + { +
@typeError
+ } +
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 91851b8e..68904dff 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 @@ -80,6 +80,10 @@ public sealed class FocasTagConfigModel // ahead of the parse consult below and is never replaced by it. if (FocasAddress.TryParse(Address) is null) return DescribeUnparseableAddress(Address); + // The driver's own address/data-type rule (String only on FIXED: paths) — checked on the typed fields so a + // model built in the editor is judged too, not only a loaded blob the parse gate below can see. + if (FocasTagDefinitionFactory.DescribeDataTypeMismatch(DataType, Address) is { } typeError) return typeError; + // Parse-gate backstop. ⚠️ It gates on the PARSE VERDICT, never on Inspect being non-empty: FOCAS's // inspector emits advisories for blobs the reader ACCEPTS (an unparseable address, an authored // 'writable':true on a read-only wire), and promoting those to authoring errors would make the modal diff --git a/tests/Drivers/Cli/ZB.MOM.WW.OtOpcUa.Driver.FOCAS.Cli.Tests/FocasCommandBaseValidationTests.cs b/tests/Drivers/Cli/ZB.MOM.WW.OtOpcUa.Driver.FOCAS.Cli.Tests/FocasCommandBaseValidationTests.cs index 2efff255..66dcabde 100644 --- a/tests/Drivers/Cli/ZB.MOM.WW.OtOpcUa.Driver.FOCAS.Cli.Tests/FocasCommandBaseValidationTests.cs +++ b/tests/Drivers/Cli/ZB.MOM.WW.OtOpcUa.Driver.FOCAS.Cli.Tests/FocasCommandBaseValidationTests.cs @@ -33,6 +33,27 @@ public sealed class FocasCommandBaseValidationTests /// Invokes option validation without interval. public void InvokeValidateNoInterval() => ValidateOptions(intervalMs: null); + + /// Invokes the address/data-type check. + public void InvokeValidateDataType(string address, FocasDataType type) => ValidateDataType(address, type); + } + + /// + /// #740 / R07:U8 — --type String on a PMC / PARAM / MACRO address is refused at the CLI boundary + /// (the driver would skip the tag and the read would print BadNodeIdUnknown); a fixed-tree path keeps it. + /// + [Theory] + [InlineData("R100", FocasDataType.String, false)] + [InlineData("MACRO:500", FocasDataType.String, false)] + [InlineData("PARAM:6711", FocasDataType.String, false)] + [InlineData("R100", FocasDataType.Byte, true)] + [InlineData("FIXED:Identity/SeriesNumber", FocasDataType.String, true)] + public void Validate_data_type_against_address(string address, FocasDataType type, bool ok) + { + var sut = new Probe { CncHost = "host", IntervalMs = 1000 }; + if (ok) Should.NotThrow(() => sut.InvokeValidateDataType(address, type)); + else Should.Throw(() => sut.InvokeValidateDataType(address, type)) + .Message.ShouldContain("String"); } /// Verifies that default options are accepted. diff --git a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.FOCAS.Tests/FocasStringDataTypeTests.cs b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.FOCAS.Tests/FocasStringDataTypeTests.cs new file mode 100644 index 00000000..600a205a --- /dev/null +++ b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.FOCAS.Tests/FocasStringDataTypeTests.cs @@ -0,0 +1,104 @@ +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; + +/// +/// #740 / R07:U8 — was authorable on any address, but no area read +/// produces a string: PMC fell through to a byte read, PARAM to an Int32, MACRO to a double, and a +/// String-typed node received a boxed number at Good. String is now refused on PMC / PARAM / MACRO by the +/// one shared check, everywhere a tag is authored or loaded; the fixed tree (Identity / Program name, …), +/// whose values really are strings, keeps it. +/// +[Trait("Category", "Unit")] +public sealed class FocasStringDataTypeTests +{ + private const string Host = "focas://10.0.0.5:8193"; + + private static string Config(string address, string dataType) => + $$"""{"address":"{{address}}","dataType":"{{dataType}}"}"""; + + /// String on an area read is refused by the mapper. + [Theory] + [InlineData("R100")] + [InlineData("X0.0")] + [InlineData("PARAM:6711")] + [InlineData("MACRO:500")] + public void Mapper_refuses_string_on_an_area_read(string address) + { + FocasTagDefinitionFactory.FromTagConfig(Config(address, "String"), "raw/T", out _).ShouldBeFalse(); + FocasTagDefinitionFactory.DescribeDataTypeMismatch(FocasDataType.String, address).ShouldNotBeNull(); + } + + /// Positive control: numeric types on the same addresses, and String on the fixed tree, still map. + [Theory] + [InlineData("R100", "Byte")] + [InlineData("PARAM:6711", "Int32")] + [InlineData("MACRO:500", "Float64")] + [InlineData("FIXED:Identity/SeriesNumber", "String")] + [InlineData("FIXED:Program/Name", "String")] + public void Mapper_accepts_numeric_area_reads_and_string_fixed_tree_paths(string address, string dataType) + { + FocasTagDefinitionFactory.FromTagConfig(Config(address, dataType), "raw/T", out var def).ShouldBeTrue(); + def.Address.ShouldBe(address); + } + + /// An ALARM: tag has no value, so its data type is irrelevant and String is not refused. + [Fact] + public void Mapper_accepts_string_on_an_alarm_tag() + { + FocasTagDefinitionFactory.FromTagConfig( + """{"address":"ALARM:*","dataType":"String","alarm":{"severity":500}}""", "raw/A", out _).ShouldBeTrue(); + } + + /// An unparseable address is left to the address check — it still draws THAT diagnosis. + [Fact] + public void Unparseable_address_keeps_its_own_diagnosis() + { + FocasTagDefinitionFactory.DescribeDataTypeMismatch(FocasDataType.String, "not-an-address").ShouldBeNull(); + FocasTagDefinitionFactory.FromTagConfig(Config("not-an-address", "String"), "raw/T", out _).ShouldBeTrue(); + } + + /// The deploy-time inspector names the reason rather than falling to the generic backstop. + [Fact] + public void Inspect_names_the_string_refusal() + { + var warnings = FocasTagDefinitionFactory.Inspect(Config("MACRO:500", "String")); + warnings.ShouldContain(w => w.Contains("String", StringComparison.Ordinal) && w.Contains("MACRO:500", StringComparison.Ordinal)); + FocasTagDefinitionFactory.Inspect(Config("MACRO:500", "Float64")).ShouldBeEmpty(); + } + + /// + /// At init a String area tag is skipped and reported (never served as a Good number); the numeric tag + /// beside it reads normally (positive control). + /// + [Fact] + public async Task Driver_skips_and_reports_a_string_area_tag() + { + var factory = new FakeFocasClientFactory + { + Customise = () => new FakeFocasClient { Values = { ["R100"] = (sbyte)7 } }, + }; + var drv = new FocasDriver(new FocasDriverOptions + { + Devices = [new FocasDeviceOptions(Host)], + RawTags = + [ + new RawTagEntry("raw/Str", Config("R100", "String"), false, Host), + new RawTagEntry("raw/Num", Config("R100", "Byte"), false, Host), + ], + Probe = new FocasProbeOptions { Enabled = false }, + }, "focas-u8", factory); + await drv.InitializeAsync("{}", CancellationToken.None); + + var snaps = await drv.ReadAsync(["raw/Str", "raw/Num"], CancellationToken.None); + snaps[0].StatusCode.ShouldBe(FocasStatusMapper.BadNodeIdUnknown); + snaps[0].Value.ShouldBeNull(); + snaps[1].StatusCode.ShouldBe(FocasStatusMapper.Good); + snaps[1].Value.ShouldBe((sbyte)7); + drv.GetSkippedTags()!.Sample.ShouldContain(s => s.RawPath == "raw/Str"); + await drv.ShutdownAsync(CancellationToken.None); + } +} 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 334637cd..b3f16e8b 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 @@ -169,4 +169,26 @@ public sealed class FocasTagConfigModelTests [InlineData("""{"address":"R100","dataType":2}""")] public void Validate_accepts_every_dataType_the_driver_accepts(string json) => FocasTagConfigModel.FromJson(json).Validate().ShouldBeNull(); + + /// + /// #740 / R07:U8 — String on a PMC / PARAM / MACRO address is refused by the driver (no area read + /// returns text), so the modal refuses it too, for a model built in the editor and for a loaded blob. + /// + [Theory] + [InlineData("R100")] + [InlineData("PARAM:6711")] + [InlineData("MACRO:500")] + public void Validate_refuses_string_on_an_area_address(string address) + { + new FocasTagConfigModel { Address = address, DataType = FocasDataType.String } + .Validate().ShouldNotBeNull().ShouldContain("String"); + FocasTagConfigModel.FromJson($$"""{"address":"{{address}}","dataType":"String"}""") + .Validate().ShouldNotBeNull().ShouldContain("String"); + } + + /// Positive control: the fixed tree's string fields keep String. + [Fact] + public void Validate_accepts_string_on_a_fixed_tree_path() + => new FocasTagConfigModel { Address = "FIXED:Identity/SeriesNumber", DataType = FocasDataType.String } + .Validate().ShouldBeNull(); }