fix(abcip,adminui): ParseEnum device wording + libplctag probe assertion (#574); AB forms preserve unknown keys + MaxConcurrentReads (#576); probe off for new AbCip driver (#577); docs T4:0.DN (#584)

Claude-Session: https://claude.ai/code/session_01Ef2ozxS7QPanGT8uoATbTe
This commit is contained in:
Joseph Doherty
2026-10-07 22:53:04 -04:00
parent 04b6563286
commit e588443ced
9 changed files with 251 additions and 15 deletions
+3 -3
View File
@@ -303,9 +303,9 @@ Same library as AB CIP — libplctag handles both protocols via the `plc` attrib
| I (Input) | 1 | 2 bytes | INT16/bit | `I:0`, `I:0/3` |
| S (Status) | 2 | 2 bytes | INT16/bit | `S:0`, `S:1/0` |
| B (Binary) | 3 | 2 bytes | INT16/bit | `B3:0`, `B3:0/5` |
| T (Timer) | 4 | 6 bytes | Struct | `T4:0.ACC`, `T4:0.PRE`, `T4:0/DN` |
| C (Counter) | 5 | 6 bytes | Struct | `C5:0.ACC`, `C5:0/DN`, `C5:0/CU` |
| R (Control) | 6 | 6 bytes | Struct | `R6:0.LEN`, `R6:0.POS`, `R6:0/DN` |
| T (Timer) | 4 | 6 bytes | Struct | `T4:0.ACC`, `T4:0.PRE`, `T4:0.DN` |
| C (Counter) | 5 | 6 bytes | Struct | `C5:0.ACC`, `C5:0.DN`, `C5:0.CU` |
| R (Control) | 6 | 6 bytes | Struct | `R6:0.LEN`, `R6:0.POS`, `R6:0.DN` |
| N (Integer) | 7 | 2 bytes | INT16 | `N7:0`, `N7:0/5`, `N10:50` |
| F (Float) | 8 | 4 bytes | FLOAT32 | `F8:0`, `F20:10` |
| ST (String) | 9 | 84 bytes | STRING | `ST9:0` (82 char max) |
+4 -4
View File
@@ -245,8 +245,8 @@ PCCC types are file-based. Int32/Float64/DateTime are not native to SLC/MicroLog
| Int32 | — (decompose in driver from two N words) | partial |
| Float32 | `F8:n` | ✔ |
| String | `ST9:n` | ✔ |
| Timer struct | `T4:n.ACC` / `.PRE` / `/DN` | ✔ |
| Counter struct | `C5:n.ACC` / `.PRE` / `/DN` | ✔ |
| Timer struct | `T4:n.ACC` / `.PRE` / `.DN` | ✔ |
| Counter struct | `C5:n.ACC` / `.PRE` / `.DN` | ✔ |
### Standard Scenario Mapping
@@ -260,8 +260,8 @@ PCCC types are file-based. Int32/Float64/DateTime are not native to SLC/MicroLog
| Ramp (Float32) | `F8:1` sine wave |
| Write-read-back (Bool / Int16 / Float32 / String) | `B3:1/0` / `N7:100` / `F8:100` / `ST9:100` |
| Array element write (Int16[10]) | `N7:200..209` |
| Timer fidelity | `T4:0.ACC`, `T4:0.PRE`, `T4:0/DN` |
| Counter fidelity | `C5:0.ACC`, `C5:0.PRE`, `C5:0/DN` |
| Timer fidelity | `T4:0.ACC`, `T4:0.PRE`, `T4:0.DN` |
| Counter fidelity | `C5:0.ACC`, `C5:0.PRE`, `C5:0.DN` |
| Connection-limit refusal | Driver harness toggle to simulate 4-conn limit |
| Bad on demand | Connection-refused toggle |
@@ -84,7 +84,7 @@ public static class AbCipDriverFactoryExtensions
? [.. dto.Devices.Select(d => new AbCipDeviceOptions(
HostAddress: d.HostAddress ?? throw new InvalidOperationException(
$"AB CIP config for '{driverInstanceId}' has a device missing HostAddress"),
PlcFamily: ParseEnum<AbCipPlcFamily>(d.PlcFamily, "device", driverInstanceId, "PlcFamily",
PlcFamily: ParseEnum<AbCipPlcFamily>(d.PlcFamily, tagName: null, driverInstanceId, "PlcFamily",
fallback: AbCipPlcFamily.ControlLogix),
DeviceName: d.DeviceName,
AllowPacking: d.AllowPacking,
@@ -130,7 +130,8 @@ public static class AbCipDriverFactoryExtensions
private static TimeSpan PositiveTimeoutOrDefault(int? ms, int defaultMs) =>
TimeSpan.FromMilliseconds(ms is int v and > 0 ? v : defaultMs);
// tagName null = a driver-level field: the error text names the driver config, not a tag.
// tagName null = a driver-level (or device-level) config field: the error text names the driver config,
// not a tag. Never pass a placeholder like "device" as tagName — it renders as "tag 'device'" (#574).
private static T ParseEnum<T>(string? raw, string? tagName, string driverInstanceId, string field,
T? fallback = null) where T : struct, Enum
{
@@ -0,0 +1,65 @@
using System.Text.Json;
using System.Text.Json.Nodes;
namespace ZB.MOM.WW.OtOpcUa.AdminUI.Components.Shared.Drivers;
/// <summary>
/// Preserve-unknown save for the typed driver forms (#576). A form deserialises the stored DriverConfig
/// into a typed options object and re-serialises it on save, which erases every key the type does not
/// model (a newer option, a hand-edited key, <c>rawTags</c>, a device's extra keys…). This overlays only
/// the paths the form OWNS onto the ORIGINAL JSON object: an owned path is replaced by the form's value,
/// or removed when the form emits nothing/null ("Default"); every other key in the original survives.
/// </summary>
internal static class DriverConfigJsonOverlay
{
/// <summary>Overlays the <paramref name="ownedPaths"/> of <paramref name="typedJson"/> onto <paramref name="originalJson"/>.</summary>
/// <param name="originalJson">The stored config as loaded; blank or non-object input yields <paramref name="typedJson"/> unchanged.</param>
/// <param name="typedJson">The form's own serialised options.</param>
/// <param name="ownedPaths">Dot-separated, case-insensitive property paths the form edits (e.g. <c>transport</c>, <c>probe.enabled</c>).</param>
public static string Apply(string? originalJson, string typedJson, IEnumerable<string> ownedPaths)
{
if (string.IsNullOrWhiteSpace(originalJson)) return typedJson;
JsonObject? original;
try { original = JsonNode.Parse(originalJson) as JsonObject; }
catch (JsonException) { return typedJson; }
if (original is null || JsonNode.Parse(typedJson) is not JsonObject typed) return typedJson;
foreach (var path in ownedPaths)
{
var segments = path.Split('.');
JsonObject? from = typed;
JsonObject? to = original;
for (var i = 0; i < segments.Length - 1 && to is not null; i++)
{
from = from is null ? null : Find(from, segments[i]) as JsonObject;
var next = Find(to, segments[i]) as JsonObject;
if (next is null && from is not null)
{
next = [];
to[segments[i]] = next;
}
to = next;
}
if (to is null) continue;
var leaf = segments[^1];
var value = from is null ? null : Find(from, leaf);
Remove(to, leaf);
if (value is not null) to[leaf] = value.DeepClone();
}
return original.ToJsonString();
}
private static JsonNode? Find(JsonObject o, string key)
{
foreach (var kv in o)
if (string.Equals(kv.Key, key, StringComparison.OrdinalIgnoreCase)) return kv.Value;
return null;
}
private static void Remove(JsonObject o, string key)
{
var hit = o.Select(kv => kv.Key).FirstOrDefault(k => string.Equals(k, key, StringComparison.OrdinalIgnoreCase));
if (hit is not null) o.Remove(hit);
}
}
@@ -18,6 +18,11 @@
<InputNumber @bind-Value="_form.TimeoutSeconds" @bind-Value:after="EmitAsync" class="form-control form-control-sm" />
<div class="form-text">Default libplctag call timeout. Default 2 s.</div>
</div>
<div class="col-md-3">
<label class="form-label">Max concurrent reads</label>
<InputNumber @bind-Value="_form.MaxConcurrentReads" @bind-Value:after="EmitAsync" class="form-control form-control-sm" />
<div class="form-text">Read work items in flight at once. Default 8; 1 = sequential.</div>
</div>
<div class="col-md-3">
<div class="form-check form-switch mt-4">
<InputCheckbox @bind-Value="_form.EnableControllerBrowse" @bind-Value:after="EmitAsync" class="form-check-input" id="enableBrowse" />
@@ -124,6 +129,20 @@
private FormModel _form = new();
private string? _lastParsed;
// The config exactly as loaded; the save overlays the form-owned keys onto it so keys this form does not
// model (unknown/newer options, rawTags, per-device extras) survive an unrelated edit (#576).
private string? _originalJson;
private static bool IsBlankConfig(string? json)
=> string.IsNullOrWhiteSpace(json) || json.AsSpan().Trim().SequenceEqual("{}");
/// <summary>Top-level/nested JSON paths this form edits; everything else in the stored config is preserved.</summary>
internal static readonly string[] OwnedPaths =
[
"timeout", "transport", "enableControllerBrowse", "enableDeclarationOnlyUdtGrouping",
"enableAlarmProjection", "alarmPollInterval", "maxConcurrentReads", "probeTimeoutSeconds",
"probe.enabled", "probe.interval", "probe.timeout", "probe.probeTagPath",
];
// Preserve the driver's existing Devices across a load→save (the device form authors the real endpoints;
// the merge reconstructs the Devices array from the device rows at deploy/probe time).
@@ -136,6 +155,11 @@
{
var opts = TryDeserialize(DriverConfigJson) ?? new AbCipDriverOptions();
_form = FormModel.FromOptions(opts);
// #577: a brand-new driver (nothing authored yet) must not default the probe ON with no tag path —
// that logs an operator warning per driver and runs no probe. An existing config keeps whatever
// it stores (an absent "probe" key still means "enabled" to the driver, so don't flip it on edit).
if (IsBlankConfig(DriverConfigJson)) _form.ProbeEnabled = false;
_originalJson = DriverConfigJson;
_preservedDevices = opts.Devices;
_lastParsed = DriverConfigJson;
}
@@ -145,16 +169,17 @@
/// Devices array forward (the device form authors the real endpoints; the merge reconstructs the array
/// from the device rows at deploy/probe time).</summary>
public string GetConfigJson()
=> Serialize(_form.ToOptions(_preservedDevices));
=> Serialize(_form.ToOptions(_preservedDevices), _originalJson);
/// <summary>Serializes options with the form's JSON settings (camelCase, enums as strings).</summary>
internal static string Serialize(AbCipDriverOptions options)
=> JsonSerializer.Serialize(options, _jsonOpts);
internal static string Serialize(AbCipDriverOptions options, string? originalJson = null)
=> DriverConfigJsonOverlay.Apply(originalJson, JsonSerializer.Serialize(options, _jsonOpts), OwnedPaths);
private async Task EmitAsync()
{
var json = GetConfigJson();
_lastParsed = json;
_originalJson = json;
await DriverConfigJsonChanged.InvokeAsync(json);
}
@@ -180,6 +205,7 @@
// PLC-5 / LogixPccc exclusion still applies). Never re-emit a concrete default here — that would pin
// every edited driver to it.
public AbCipTransport? Transport { get; set; }
public int MaxConcurrentReads { get; set; } = 8;
public bool EnableControllerBrowse { get; set; } = false;
public bool EnableDeclarationOnlyUdtGrouping { get; set; } = false;
@@ -200,6 +226,7 @@
{
TimeoutSeconds = (int)o.Timeout.TotalSeconds,
Transport = o.Transport,
MaxConcurrentReads = o.MaxConcurrentReads,
EnableControllerBrowse = o.EnableControllerBrowse,
EnableDeclarationOnlyUdtGrouping = o.EnableDeclarationOnlyUdtGrouping,
EnableAlarmProjection = o.EnableAlarmProjection,
@@ -224,6 +251,7 @@
},
Timeout = TimeSpan.FromSeconds(TimeoutSeconds),
Transport = Transport,
MaxConcurrentReads = MaxConcurrentReads,
EnableControllerBrowse = EnableControllerBrowse,
EnableAlarmProjection = EnableAlarmProjection,
AlarmPollInterval = TimeSpan.FromSeconds(AlarmPollIntervalSeconds),
@@ -90,6 +90,16 @@
private FormModel _form = new();
private string? _lastParsed;
// The config exactly as loaded; the save overlays the form-owned keys onto it so keys this form does not
// model (unknown/newer options, rawTags, per-device extras) survive an unrelated edit (#576).
private string? _originalJson;
/// <summary>JSON paths this form edits; everything else in the stored config is preserved.</summary>
internal static readonly string[] OwnedPaths =
[
"timeout", "transport", "probeTimeoutSeconds",
"probe.enabled", "probe.interval", "probe.timeout", "probe.probeAddress",
];
// Devices are separate v3 entities — this form doesn't edit them, but preserves the driver's existing
// Devices array verbatim across a load→save (the merge rebuilds it at deploy/probe time).
@@ -102,6 +112,7 @@
{
var opts = TryDeserialize(DriverConfigJson) ?? new AbLegacyDriverOptions();
_form = FormModel.FromOptions(opts);
_originalJson = DriverConfigJson;
_preservedDevices = opts.Devices;
_lastParsed = DriverConfigJson;
}
@@ -110,16 +121,17 @@
/// <summary>Serializes the current channel/protocol config to camelCase JSON, preserving the driver's
/// existing Devices array (authored as separate v3 entities).</summary>
public string GetConfigJson()
=> Serialize(_form.ToOptions(_preservedDevices));
=> Serialize(_form.ToOptions(_preservedDevices), _originalJson);
/// <summary>Serializes options with the form's JSON settings (camelCase, enums as strings).</summary>
internal static string Serialize(AbLegacyDriverOptions options)
=> JsonSerializer.Serialize(options, _jsonOpts);
internal static string Serialize(AbLegacyDriverOptions options, string? originalJson = null)
=> DriverConfigJsonOverlay.Apply(originalJson, JsonSerializer.Serialize(options, _jsonOpts), OwnedPaths);
private async Task EmitAsync()
{
var json = GetConfigJson();
_lastParsed = json;
_originalJson = json;
await DriverConfigJsonChanged.InvokeAsync(json);
}
@@ -64,6 +64,9 @@ public sealed class AbCipTransportSwitchTests
"ab1", """{"devices":[{"hostAddress":"ab://10.0.0.1/1,0","plcFamily":"Bogus"}]}"""));
existing.GetType().ShouldBe(bad.GetType());
existing.Message.ShouldContain("unknown PlcFamily 'Bogus'");
// #574: a device-level field must not be reported as a tag named 'device'.
existing.Message.ShouldNotContain("tag '");
existing.Message.ShouldStartWith("AB CIP config in 'ab1' has unknown PlcFamily 'Bogus'");
}
/// <summary>
@@ -145,6 +148,9 @@ public sealed class AbCipTransportSwitchTests
native.Message.ShouldNotBeNull();
native.Message!.ShouldContain("CIP handshake failed: Connection lost");
lib.Message!.ShouldNotContain("Connection lost");
// Positive (#574): the libplctag path ran and reported its own outcome (the bounded probe timeout),
// not merely "something other than the native text".
lib.Message!.ShouldContain("timed out");
}
/// <summary>
@@ -180,6 +186,9 @@ public sealed class AbCipTransportSwitchTests
absent.Ok.ShouldBeFalse();
absent.Message!.ShouldContain("CIP handshake failed: Connection lost");
lib.Message!.ShouldNotContain("Connection lost");
// Positive (#574): the libplctag path ran and reported its own outcome (the bounded probe timeout),
// not merely "something other than the native text".
lib.Message!.ShouldContain("timed out");
}
/// <summary>The probe deserialises with its own string-enum converter; a bad transport is a config error.</summary>
@@ -259,5 +268,6 @@ public sealed class AbCipTransportSwitchTests
unpinned.Message!.ShouldContain("CIP handshake failed: Connection lost");
pinned.Ok.ShouldBeFalse();
pinned.Message!.ShouldNotContain("Connection lost");
pinned.Message!.ShouldContain("timed out"); // positive: libplctag's own outcome (#574)
}
}
@@ -172,4 +172,77 @@ public sealed class AbCipDriverPageFormSerializationTests
form.Transport = null; // the "Default (...)" dropdown option
AbCipDriverForm.Serialize(form.ToOptions(loaded.Devices)).ShouldNotContain("transport", Case.Insensitive);
}
// ---- #576: preserve-unknown + MaxConcurrentReads ----
private const string FullConfig = """
{"transport":"Native","timeout":"00:00:05","maxConcurrentReads":3,"enableControllerBrowse":true,
"enableDeclarationOnlyUdtGrouping":true,"enableAlarmProjection":true,"alarmPollInterval":"00:00:02",
"probeTimeoutSeconds":12,
"probe":{"enabled":true,"interval":"00:00:10","timeout":"00:00:03","probeTagPath":"Program:Main.Hb","futureProbeKnob":7},
"devices":[{"hostAddress":"ab://10.0.0.1/1,0","plcFamily":"ControlLogix","deviceName":"PLC-1","futureDeviceKey":"keep"}],
"rawTags":[{"rawPath":"a/b"}],
"futureOption":{"nested":[1,2,3]},"anotherUnknown":"x"}
""";
[Fact]
public void Editing_one_field_preserves_every_other_option_and_unknown_keys()
{
var loaded = AbCipDriverForm.TryDeserialize(FullConfig)!;
var form = AbCipDriverForm.FormModel.FromOptions(loaded);
form.TimeoutSeconds = 9; // the one edit
var root = System.Text.Json.Nodes.JsonNode.Parse(
AbCipDriverForm.Serialize(form.ToOptions(loaded.Devices), FullConfig))!.AsObject();
root["timeout"]!.GetValue<string>().ShouldBe("00:00:09");
root["maxConcurrentReads"]!.GetValue<int>().ShouldBe(3); // was reset to 8 before #576
root["transport"]!.GetValue<string>().ShouldBe("Native");
root["enableControllerBrowse"]!.GetValue<bool>().ShouldBeTrue();
root["enableDeclarationOnlyUdtGrouping"]!.GetValue<bool>().ShouldBeTrue();
root["enableAlarmProjection"]!.GetValue<bool>().ShouldBeTrue();
root["alarmPollInterval"]!.GetValue<string>().ShouldBe("00:00:02");
root["probeTimeoutSeconds"]!.GetValue<int>().ShouldBe(12);
root["probe"]!["enabled"]!.GetValue<bool>().ShouldBeTrue();
root["probe"]!["interval"]!.GetValue<string>().ShouldBe("00:00:10");
root["probe"]!["timeout"]!.GetValue<string>().ShouldBe("00:00:03");
root["probe"]!["probeTagPath"]!.GetValue<string>().ShouldBe("Program:Main.Hb");
// unknown keys at every level survive
root["probe"]!["futureProbeKnob"]!.GetValue<int>().ShouldBe(7);
root["devices"]![0]!["futureDeviceKey"]!.GetValue<string>().ShouldBe("keep");
root["rawTags"]![0]!["rawPath"]!.GetValue<string>().ShouldBe("a/b");
root["futureOption"]!["nested"]!.AsArray().Count.ShouldBe(3);
root["anotherUnknown"]!.GetValue<string>().ShouldBe("x");
}
[Fact]
public void Choosing_Default_transport_and_clearing_the_probe_path_removes_those_keys_from_the_overlay()
{
var loaded = AbCipDriverForm.TryDeserialize(FullConfig)!;
var form = AbCipDriverForm.FormModel.FromOptions(loaded);
form.Transport = null;
form.ProbeTagPath = "";
var root = System.Text.Json.Nodes.JsonNode.Parse(
AbCipDriverForm.Serialize(form.ToOptions(loaded.Devices), FullConfig))!.AsObject();
root.ContainsKey("transport").ShouldBeFalse();
root["probe"]!.AsObject().ContainsKey("probeTagPath").ShouldBeFalse();
root["futureOption"].ShouldNotBeNull();
}
[Fact]
public void A_stored_PascalCase_key_is_replaced_not_duplicated()
{
var original = """{"Transport":"Native","TimeoutSeconds":1,"futureOption":true}""";
var loaded = AbCipDriverForm.TryDeserialize(original)!;
var form = AbCipDriverForm.FormModel.FromOptions(loaded);
form.Transport = AbCipTransport.Libplctag;
var json = AbCipDriverForm.Serialize(form.ToOptions(loaded.Devices), original);
System.Text.RegularExpressions.Regex.Matches(json, "transport", System.Text.RegularExpressions.RegexOptions.IgnoreCase)
.Count.ShouldBe(1);
json.ShouldContain("\"transport\":\"Libplctag\"");
json.ShouldContain("futureOption");
}
}
@@ -187,4 +187,51 @@ public sealed class AbLegacyDriverPageFormSerializationTests
razor.ShouldContain("<option value=\"@AbLegacyTransport.Libplctag\">libplctag</option>");
razor.ShouldNotContain("Default (Native");
}
// ---- #576: preserve-unknown ----
private const string FullConfig = """
{"transport":"Native","timeout":"00:00:05","probeTimeoutSeconds":12,
"probe":{"enabled":true,"interval":"00:00:10","timeout":"00:00:03","probeAddress":"S:1","futureProbeKnob":7},
"devices":[{"hostAddress":"ab://10.0.0.1/1,0","plcFamily":"Slc500","futureDeviceKey":"keep"}],
"rawTags":[{"rawPath":"a/b"}],
"futureOption":{"nested":[1,2,3]}}
""";
[Fact]
public void Editing_one_field_preserves_every_other_option_and_unknown_keys()
{
var loaded = AbLegacyDriverForm.TryDeserialize(FullConfig)!;
var form = AbLegacyDriverForm.FormModel.FromOptions(loaded);
form.TimeoutSeconds = 9;
var root = System.Text.Json.Nodes.JsonNode.Parse(
AbLegacyDriverForm.Serialize(form.ToOptions(loaded.Devices), FullConfig))!.AsObject();
root["timeout"]!.GetValue<string>().ShouldBe("00:00:09");
root["transport"]!.GetValue<string>().ShouldBe("Native");
root["probeTimeoutSeconds"]!.GetValue<int>().ShouldBe(12);
root["probe"]!["enabled"]!.GetValue<bool>().ShouldBeTrue();
root["probe"]!["interval"]!.GetValue<string>().ShouldBe("00:00:10");
root["probe"]!["timeout"]!.GetValue<string>().ShouldBe("00:00:03");
root["probe"]!["probeAddress"]!.GetValue<string>().ShouldBe("S:1");
root["probe"]!["futureProbeKnob"]!.GetValue<int>().ShouldBe(7);
root["devices"]![0]!["futureDeviceKey"]!.GetValue<string>().ShouldBe("keep");
root["rawTags"]![0]!["rawPath"]!.GetValue<string>().ShouldBe("a/b");
root["futureOption"]!["nested"]!.AsArray().Count.ShouldBe(3);
}
[Fact]
public void Choosing_Default_transport_removes_the_key_but_keeps_unknown_ones()
{
var loaded = AbLegacyDriverForm.TryDeserialize(FullConfig)!;
var form = AbLegacyDriverForm.FormModel.FromOptions(loaded);
form.Transport = null;
var root = System.Text.Json.Nodes.JsonNode.Parse(
AbLegacyDriverForm.Serialize(form.ToOptions(loaded.Devices), FullConfig))!.AsObject();
root.ContainsKey("transport").ShouldBeFalse();
root["futureOption"].ShouldNotBeNull();
}
}