92749402b5
N3: a keyless blob carrying the addressResolved marker was served from its string by the driver but saved as HR[0] by the editor model / CSV path. The precedence rule now lives in ONE pure resolver in Driver.Modbus.Contracts (ModbusTagAddressResolution.Classify/Resolve): keys when region+address are both present, else a string-only addressString resolved with the driver's family (refused when it cannot parse), else refused. The marker never decides. Consumers that now ask it instead of restating it: the driver's tag reader (ModbusTagDefinitionFactory.FromTagConfig), the deploy gate (Inspect / DescribeRejection), the editor model (AddressShape / IsStringOnly / HasKeyAddress / ResolveWith, ToJson stamping, Validate), the CSV export cells and the editor hint. The model writes keys only when it loaded them or a field was authored - never defaults it did not resolve (also closes two divergences the new matrix found: malformed-string+marker and non-string addressString were saved as HR[0]). One table-driven test (ModbusAddressAuthorityAgreementTests) runs 15 persisted shapes through driver, deploy gate, model load/save/Validate and the real CSV export->import under all four families; breaking the CSV export or reverting the model to the marker rule fails it. Minors: m1 CSV address cells keep the string+marker while it still describes them (lossless unchanged round trip), drop it when contradicted, and the review grid reports the drop; m2 the pre-lane test uses the real export residual; m3 unmarked string-only + typed-cell case added; m4 stale docs/summary and the unused factory parameter removed. Refs #741 Claude-Session: https://claude.ai/code/session_01Ub9hGmYLQvn9XETzw7CMsT
231 lines
12 KiB
C#
231 lines
12 KiB
C#
using Shouldly;
|
|
using Xunit;
|
|
using ZB.MOM.WW.OtOpcUa.AdminUI.Uns;
|
|
using ZB.MOM.WW.OtOpcUa.AdminUI.Uns.TagEditors;
|
|
using ZB.MOM.WW.OtOpcUa.Configuration.Enums;
|
|
using ZB.MOM.WW.OtOpcUa.ControlPlane.AdminOperations;
|
|
using ZB.MOM.WW.OtOpcUa.Core.Abstractions;
|
|
using ZB.MOM.WW.OtOpcUa.Driver.Modbus;
|
|
|
|
namespace ZB.MOM.WW.OtOpcUa.AdminUI.Tests.Uns;
|
|
|
|
/// <summary>
|
|
/// Review r3 N3 (#741, fix round 4): <b>one authority for a Modbus tag's address</b>. Every persisted
|
|
/// <c>TagConfig</c> shape goes through EVERY consumer that decides, displays or rewrites the address. They
|
|
/// must all agree with the driver, under every family:
|
|
/// <list type="bullet">
|
|
/// <item>the driver's tag reader (<c>ModbusTagDefinitionFactory.FromTagConfig</c> with the driver's
|
|
/// family, the exact call <c>ModbusDriver</c> makes at initialize);</item>
|
|
/// <item>the deploy gate (<c>EquipmentTagConfigInspector.Inspect</c>);</item>
|
|
/// <item>the editor model's load (what its fields claim), save (incl. a Writable toggle) and
|
|
/// <c>Validate</c>;</item>
|
|
/// <item>the CSV export → import round trip through the real <c>RawTagCsvMapper</c> (the export residual,
|
|
/// not the stored blob, is the import base).</item>
|
|
/// </list>
|
|
/// The expected verdict per family is pinned in the table, so the authority itself cannot drift either.
|
|
/// Three rounds converged on the rule one consumer at a time; this matrix is what makes a new divergence
|
|
/// fail in one place.
|
|
/// </summary>
|
|
public sealed class ModbusAddressAuthorityAgreementTests
|
|
{
|
|
private static readonly (ModbusFamily Family, MelsecFamily Sub, string Label)[] Families =
|
|
[
|
|
(ModbusFamily.Generic, MelsecFamily.Q_L_iQR, "Generic"),
|
|
(ModbusFamily.DL205, MelsecFamily.Q_L_iQR, "DL205"),
|
|
(ModbusFamily.MELSEC, MelsecFamily.Q_L_iQR, "MELSEC-Q"),
|
|
(ModbusFamily.MELSEC, MelsecFamily.F_iQF, "MELSEC-F"),
|
|
];
|
|
|
|
/// <summary>
|
|
/// Shape name, persisted blob, and the expected driver verdict per family in the order of
|
|
/// <see cref="Families"/> (<c>IR5</c> = InputRegisters[5], <c>-</c> = refused).
|
|
/// </summary>
|
|
public static TheoryData<string, string, string> Shapes => new()
|
|
{
|
|
{ "keys only", """{"region":"InputRegisters","address":5}""", "IR5|IR5|IR5|IR5" },
|
|
{ "pre-lane keys + string", """{"region":"InputRegisters","address":5,"dataType":"Int16","addressString":"40011:F"}""", "IR5|IR5|IR5|IR5" },
|
|
{ "pre-lane stamped HR/0 + string",
|
|
"""{"region":"HoldingRegisters","address":0,"dataType":"Int16","byteOrder":"BigEndian","bitIndex":0,"stringLength":0,"addressString":"D100"}""",
|
|
"HR0|HR0|HR0|HR0" },
|
|
{ "string only", """{"addressString":"40011:F"}""", "HR10|HR10|HR10|HR10" },
|
|
{ "string only, family-native", """{"addressString":"D100"}""", "-|-|HR100|HR100" },
|
|
{ "string + marker + keys", """{"addressString":"D100","addressResolved":true,"region":"HoldingRegisters","address":100,"dataType":"Int16"}""", "HR100|HR100|HR100|HR100" },
|
|
{ "string + marker, no keys", """{"addressString":"40011:F","addressResolved":true}""", "HR10|HR10|HR10|HR10" },
|
|
{ "string + marker, no keys, family-native", """{"addressString":"D100","addressResolved":true}""", "-|-|HR100|HR100" },
|
|
{ "malformed string", """{"addressString":"ZZ9"}""", "-|-|-|-" },
|
|
{ "malformed string + marker", """{"addressString":"ZZ9","addressResolved":true}""", "-|-|-|-" },
|
|
{ "malformed string + keys", """{"region":"InputRegisters","address":5,"addressString":"ZZ9"}""", "IR5|IR5|IR5|IR5" },
|
|
{ "non-string addressString", """{"addressString":5}""", "-|-|-|-" },
|
|
{ "string + one key", """{"addressString":"40011:F","region":"InputRegisters"}""", "HR10|HR10|HR10|HR10" },
|
|
{ "no address", """{"writable":true}""", "-|-|-|-" },
|
|
{ "bit region, legacy Int16", """{"region":"Coils","address":3,"dataType":"Int16"}""", "CO3|CO3|CO3|CO3" },
|
|
};
|
|
|
|
[Theory]
|
|
[MemberData(nameof(Shapes))]
|
|
public void Every_consumer_agrees_with_the_driver_on_every_persisted_shape(string shape, string blob, string expected)
|
|
{
|
|
var want = expected.Split('|');
|
|
var anyServed = want.Any(v => v != "-");
|
|
|
|
// 1. The driver (the authority) — pinned, so the authority cannot drift unnoticed either.
|
|
for (var i = 0; i < Families.Length; i++)
|
|
{
|
|
Verdict(blob, i).ShouldBe(want[i], $"{shape}: driver / {Families[i].Label}");
|
|
}
|
|
|
|
// 2. The deploy gate: clean exactly when some family's driver serves the tag.
|
|
(EquipmentTagConfigInspector.Inspect(DriverTypeNames.Modbus, blob).Count == 0)
|
|
.ShouldBe(anyServed, $"{shape}: deploy gate accepts ⇔ some driver serves it");
|
|
|
|
// 3. Editor load: when the model presents its fields as the address, they ARE the driver's address.
|
|
var loaded = ModbusTagConfigModel.FromJson(blob);
|
|
if (!loaded.IsStringOnly)
|
|
{
|
|
for (var i = 0; i < Families.Length; i++)
|
|
{
|
|
if (want[i] == "-") continue;
|
|
FieldsVerdict(loaded).ShouldBe(want[i], $"{shape}: editor fields / {Families[i].Label}");
|
|
}
|
|
}
|
|
|
|
// 4. Editor save, untouched and after a Writable toggle (the round trip that remapped N1 / N3).
|
|
var untouched = ModbusTagConfigModel.FromJson(blob).ToJson();
|
|
var toggled = ModbusTagConfigModel.FromJson(blob);
|
|
toggled.Writable = false;
|
|
var toggledJson = toggled.ToJson();
|
|
for (var i = 0; i < Families.Length; i++)
|
|
{
|
|
Verdict(untouched, i).ShouldBe(want[i], $"{shape}: editor save / {Families[i].Label}");
|
|
Verdict(toggledJson, i).ShouldBe(want[i], $"{shape}: editor save after Writable toggle / {Families[i].Label}");
|
|
}
|
|
|
|
// 5. Validate: accepts exactly what some family's driver serves.
|
|
(ModbusTagConfigModel.FromJson(blob).Validate() is null).ShouldBe(anyServed, $"{shape}: Validate");
|
|
|
|
// 6. CSV export → import through the production mapper (the residual is the import base).
|
|
var csv = RawTagCsvMapper.Export(DriverTypeNames.Modbus,
|
|
[new RawCsvExportTag(null, "t", "Int16", TagAccessLevel.ReadWrite, false, null, blob)]);
|
|
var parsed = RawTagCsvMapper.Parse(DriverTypeNames.Modbus, csv);
|
|
parsed.FatalError.ShouldBeNull();
|
|
var row = parsed.Rows.ShouldHaveSingleItem();
|
|
if (!row.Ok)
|
|
{
|
|
anyServed.ShouldBeFalse($"{shape}: CSV import refused a tag the driver serves ({row.Error})");
|
|
}
|
|
else
|
|
{
|
|
for (var i = 0; i < Families.Length; i++)
|
|
{
|
|
Verdict(row.ImportRow!.Tag.TagConfig, i).ShouldBe(want[i], $"{shape}: CSV export → import / {Families[i].Label}");
|
|
}
|
|
}
|
|
}
|
|
|
|
private static string Verdict(string json, int familyIndex)
|
|
{
|
|
var (family, sub, _) = Families[familyIndex];
|
|
return ModbusTagDefinitionFactory.FromTagConfig(json, "T", family, sub, out var def)
|
|
? Short(def.Region) + def.Address
|
|
: "-";
|
|
}
|
|
|
|
private static string FieldsVerdict(ModbusTagConfigModel m) => Short(m.Region) + m.Address;
|
|
|
|
private static string Short(ModbusRegion r) => r switch
|
|
{
|
|
ModbusRegion.HoldingRegisters => "HR",
|
|
ModbusRegion.InputRegisters => "IR",
|
|
ModbusRegion.Coils => "CO",
|
|
ModbusRegion.DiscreteInputs => "DI",
|
|
_ => r.ToString(),
|
|
};
|
|
|
|
// ------------------------------------------------------------------ CSV metadata (review r3 m1 / m3)
|
|
|
|
private static RawCsvReviewRow RoundTrip(string blob, Func<string, string>? editCsv = null)
|
|
{
|
|
var csv = RawTagCsvMapper.Export(DriverTypeNames.Modbus,
|
|
[new RawCsvExportTag(null, "t", "Int16", TagAccessLevel.ReadWrite, false, null, blob)]);
|
|
if (editCsv is not null) csv = editCsv(csv);
|
|
return RawTagCsvMapper.Parse(DriverTypeNames.Modbus, csv).Rows.ShouldHaveSingleItem();
|
|
}
|
|
|
|
/// <summary>m1: an UNCHANGED export → import of a pinned row keeps its address string and marker.</summary>
|
|
[Fact]
|
|
public void An_unchanged_csv_round_trip_of_a_pinned_row_keeps_its_address_string_and_marker()
|
|
{
|
|
const string pinned = """{"addressString":"D100","addressResolved":true,"region":"HoldingRegisters","address":100,"dataType":"Int16","byteOrder":"BigEndian","bitIndex":0,"stringLength":0}""";
|
|
var row = RoundTrip(pinned);
|
|
row.Ok.ShouldBeTrue(row.Error);
|
|
var json = row.ImportRow!.Tag.TagConfig;
|
|
json.ShouldContain("\"addressString\":\"D100\"");
|
|
json.ShouldContain("\"addressResolved\":true");
|
|
row.TypedOverrodeFallback.ShouldBeEmpty("nothing was overridden");
|
|
ModbusTagConfigModel.FromJson(json).IsPinned.ShouldBeTrue();
|
|
}
|
|
|
|
/// <summary>m1 positive control: an address cell that CONTRADICTS the string drops it, and the grid says so.</summary>
|
|
[Fact]
|
|
public void A_csv_address_cell_that_contradicts_the_string_drops_it_and_the_review_grid_reports_it()
|
|
{
|
|
const string pinned = """{"addressString":"D100","addressResolved":true,"region":"HoldingRegisters","address":100,"dataType":"Int16","byteOrder":"BigEndian","bitIndex":0,"stringLength":0}""";
|
|
// Edit the exported Address cell 100 → 101.
|
|
var row = RoundTrip(pinned, csv => csv.Replace(",HoldingRegisters,100,", ",HoldingRegisters,101,", StringComparison.Ordinal));
|
|
row.Ok.ShouldBeTrue(row.Error);
|
|
var json = row.ImportRow!.Tag.TagConfig;
|
|
json.ShouldNotContain("addressString");
|
|
json.ShouldNotContain("addressResolved");
|
|
row.TypedOverrodeFallback.ShouldNotBeEmpty("the dropped string is reported, not silent");
|
|
ModbusTagDefinitionFactory.FromTagConfig(json, "T", ModbusFamily.MELSEC, MelsecFamily.Q_L_iQR, out var def).ShouldBeTrue();
|
|
def.Address.ShouldBe((ushort)101);
|
|
}
|
|
|
|
/// <summary>m3: the n5 case on an UNMARKED string-only row — typed address cells are applied and drop the string.</summary>
|
|
[Fact]
|
|
public void A_typed_csv_address_cell_on_an_unmarked_string_only_row_is_applied_and_drops_the_string()
|
|
{
|
|
var map = CsvColumnMap.Resolve(DriverTypeNames.Modbus).ShouldNotBeNull();
|
|
var merged = map.ApplyTypedColumns("""{"addressString":"40011:F"}""",
|
|
new Dictionary<string, string>(StringComparer.Ordinal) { ["Region"] = "InputRegisters", ["Address"] = "7" });
|
|
|
|
merged.ShouldNotContain("addressString");
|
|
foreach (var (family, sub, label) in Families)
|
|
{
|
|
ModbusTagDefinitionFactory.FromTagConfig(merged, "T", family, sub, out var def).ShouldBeTrue(label);
|
|
def.Region.ShouldBe(ModbusRegion.InputRegisters, label);
|
|
def.Address.ShouldBe((ushort)7, label);
|
|
}
|
|
}
|
|
|
|
/// <summary>N3, by name: the marker without keys is string-only for the editor, its save and its CSV export.</summary>
|
|
[Fact]
|
|
public void A_marker_without_keys_is_string_only_and_never_saved_as_HR0()
|
|
{
|
|
var m = ModbusTagConfigModel.FromJson("""{"addressString":"40011:F","addressResolved":true}""");
|
|
m.IsStringOnly.ShouldBeTrue();
|
|
m.IsPinned.ShouldBeFalse("a marker without keys pins nothing");
|
|
m.Writable = false;
|
|
var saved = m.ToJson();
|
|
saved.ShouldNotContain("\"address\":");
|
|
saved.ShouldNotContain("\"region\":");
|
|
CsvColumnMap.Resolve(DriverTypeNames.Modbus)!.ReadTypedColumns(saved).ShouldNotContainKey("Address");
|
|
}
|
|
|
|
/// <summary>The editor never writes default keys it did not resolve: a Writable toggle on an address-less blob
|
|
/// does not author HoldingRegisters[0]; setting a field does.</summary>
|
|
[Fact]
|
|
public void An_address_less_blob_gains_keys_only_when_a_field_is_authored()
|
|
{
|
|
var m = ModbusTagConfigModel.FromJson("{}");
|
|
m.Writable = false;
|
|
m.ToJson().ShouldNotContain("\"region\":");
|
|
m.HasKeyAddress.ShouldBeFalse();
|
|
|
|
m.Address = 0; // an explicit authoring of HR[0] is honoured
|
|
m.HasKeyAddress.ShouldBeTrue();
|
|
ModbusTagDefinitionFactory.FromTagConfig(m.ToJson(), "T", out var def).ShouldBeTrue();
|
|
def.Address.ShouldBe((ushort)0);
|
|
}
|
|
}
|