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
This commit is contained in:
@@ -28,6 +28,7 @@ public sealed class ProbeCommand : FocasCommandBase
|
||||
{
|
||||
ConfigureLogging();
|
||||
ValidateOptions();
|
||||
ValidateDataType(Address, DataType);
|
||||
var ct = console.RegisterCancellationHandler();
|
||||
|
||||
var probeTag = new FocasTagDefinition(
|
||||
|
||||
@@ -19,7 +19,7 @@ public sealed class ReadCommand : FocasCommandBase
|
||||
|
||||
/// <summary>Gets the data type to interpret the address as.</summary>
|
||||
[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;
|
||||
|
||||
/// <inheritdoc />
|
||||
@@ -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);
|
||||
|
||||
@@ -18,7 +18,7 @@ public sealed class SubscribeCommand : FocasCommandBase
|
||||
|
||||
/// <summary>Gets the data type of the address.</summary>
|
||||
[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;
|
||||
|
||||
/// <summary>Gets the polling interval in milliseconds.</summary>
|
||||
@@ -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);
|
||||
|
||||
@@ -20,7 +20,7 @@ public sealed class WriteCommand : FocasCommandBase
|
||||
|
||||
/// <summary>Gets the data type of the value to write.</summary>
|
||||
[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;
|
||||
|
||||
/// <summary>Gets the value to write.</summary>
|
||||
@@ -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);
|
||||
|
||||
@@ -89,4 +89,17 @@ public abstract class FocasCommandBase : DriverCommandBase
|
||||
throw new CliFx.Exceptions.CommandException(
|
||||
$"--interval-ms must be positive (got {iv}).");
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
/// Refuses an <c>--address</c> / <c>--type</c> 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: <see cref="FocasTagDefinitionFactory.DescribeDataTypeMismatch"/>.
|
||||
/// </summary>
|
||||
/// <param name="address">The FOCAS address.</param>
|
||||
/// <param name="type">The data type.</param>
|
||||
protected static void ValidateDataType(string address, FocasDataType type)
|
||||
{
|
||||
if (FocasTagDefinitionFactory.DescribeDataTypeMismatch(type, address) is { } error)
|
||||
throw new CliFx.Exceptions.CommandException(error);
|
||||
}
|
||||
}
|
||||
|
||||
@@ -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; }
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
/// 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. <see cref="FocasDataType.String"/> 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 <c>ALARM:</c> tag has no value, so its
|
||||
/// data type is irrelevant; an unparseable address is left to the address check, which names it better.
|
||||
/// </summary>
|
||||
/// <param name="dataType">The authored data type.</param>
|
||||
/// <param name="address">The authored address.</param>
|
||||
/// <returns>The reason the pair cannot be served, or <see langword="null"/> when it can.</returns>
|
||||
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.";
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
/// The tag's cross-driver <c>alarm</c> object, or <see langword="null"/> when it carries none.
|
||||
/// Lenient like the host's <c>TagConfigIntent</c>: a non-integer <c>severity</c> is simply absent.
|
||||
@@ -236,6 +259,10 @@ public static class FocasTagDefinitionFactory
|
||||
}
|
||||
|
||||
if (TagConfigJson.DescribeInvalidEnum<FocasDataType>(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.");
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
|
||||
+16
-2
@@ -15,8 +15,22 @@
|
||||
</div>
|
||||
<div class="col-md-3"><label class="form-label">Data type</label>
|
||||
<select class="form-select form-select-sm" value="@_m.DataType" @onchange="@(e => Update(() => _m.DataType = ParseEnum(e.Value, FocasDataType.Int32)))">
|
||||
@foreach (var v in Enum.GetValues<FocasDataType>()) { <option value="@v">@v</option> }
|
||||
</select></div>
|
||||
@foreach (var v in Enum.GetValues<FocasDataType>())
|
||||
{
|
||||
@* 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)
|
||||
{
|
||||
<option value="@v">@v</option>
|
||||
}
|
||||
}
|
||||
</select>
|
||||
@if (FocasTagDefinitionFactory.DescribeDataTypeMismatch(_m.DataType, _m.Address) is { } typeError)
|
||||
{
|
||||
<div class="form-text text-danger small">@typeError</div>
|
||||
}
|
||||
</div>
|
||||
<div class="col-12">
|
||||
<div class="form-check">
|
||||
<input type="checkbox" class="form-check-input" id="focas-writable" disabled checked="false" />
|
||||
|
||||
@@ -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
|
||||
|
||||
+21
@@ -33,6 +33,27 @@ public sealed class FocasCommandBaseValidationTests
|
||||
|
||||
/// <summary>Invokes option validation without interval.</summary>
|
||||
public void InvokeValidateNoInterval() => ValidateOptions(intervalMs: null);
|
||||
|
||||
/// <summary>Invokes the address/data-type check.</summary>
|
||||
public void InvokeValidateDataType(string address, FocasDataType type) => ValidateDataType(address, type);
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
/// #740 / R07:U8 — <c>--type String</c> 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.
|
||||
/// </summary>
|
||||
[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<CliFx.Exceptions.CommandException>(() => sut.InvokeValidateDataType(address, type))
|
||||
.Message.ShouldContain("String");
|
||||
}
|
||||
|
||||
/// <summary>Verifies that default options are accepted.</summary>
|
||||
|
||||
@@ -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;
|
||||
|
||||
/// <summary>
|
||||
/// #740 / R07:U8 — <see cref="FocasDataType.String"/> 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.
|
||||
/// </summary>
|
||||
[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}}"}""";
|
||||
|
||||
/// <summary>String on an area read is refused by the mapper.</summary>
|
||||
[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();
|
||||
}
|
||||
|
||||
/// <summary>Positive control: numeric types on the same addresses, and String on the fixed tree, still map.</summary>
|
||||
[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);
|
||||
}
|
||||
|
||||
/// <summary>An ALARM: tag has no value, so its data type is irrelevant and String is not refused.</summary>
|
||||
[Fact]
|
||||
public void Mapper_accepts_string_on_an_alarm_tag()
|
||||
{
|
||||
FocasTagDefinitionFactory.FromTagConfig(
|
||||
"""{"address":"ALARM:*","dataType":"String","alarm":{"severity":500}}""", "raw/A", out _).ShouldBeTrue();
|
||||
}
|
||||
|
||||
/// <summary>An unparseable address is left to the address check — it still draws THAT diagnosis.</summary>
|
||||
[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();
|
||||
}
|
||||
|
||||
/// <summary>The deploy-time inspector names the reason rather than falling to the generic backstop.</summary>
|
||||
[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();
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
/// 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).
|
||||
/// </summary>
|
||||
[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);
|
||||
}
|
||||
}
|
||||
@@ -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();
|
||||
|
||||
/// <summary>
|
||||
/// #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.
|
||||
/// </summary>
|
||||
[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");
|
||||
}
|
||||
|
||||
/// <summary>Positive control: the fixed tree's string fields keep String.</summary>
|
||||
[Fact]
|
||||
public void Validate_accepts_string_on_a_fixed_tree_path()
|
||||
=> new FocasTagConfigModel { Address = "FIXED:Identity/SeriesNumber", DataType = FocasDataType.String }
|
||||
.Validate().ShouldBeNull();
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user