dohertj2/lmxopcua
1
0
Fork 0
Code Issues 32 Pull Requests Actions Packages Projects Releases Wiki Activity

fix(abcip): final review I4 — a type mismatch is a refusal, not a comms fault

Browse Source 
A native TypeMismatch (read status, write gate, or CIP 0xFF/0x2107) used
to evict the runtime and degrade health on every poll; for a sole lease
holder that meant Forward Close + Forward Open per poll, and an all-
mismatched driver tripped the comms-loss watchdog. It now reports
BadTypeMismatch, keeps the handle, leaves health alone and warns once per
tag per instance — reads, whole-UDT reads and writes, plus the libplctag
width check. Tests (red against the pre-fix driver): single create over N
polls, unchanged health, one warning, and one Forward Open / zero Forward
Close over the real native runtime against FakeCipTarget. AbLegacy has no
TypeMismatch path (its native runtime has no type check).

Claude-Session: https://claude.ai/code/session_01Ef2ozxS7QPanGT8uoATbTe
This commit is contained in:
Joseph Doherty
2026-10-07 21:40:04 -04:00
parent 2cea1a0687
commit 352ea449e9
5 changed files with 247 additions and 8 deletions
Download Patch File Download Diff File Expand all files Collapse all files
src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip/AbCipDriver.cs
+75 -4
View File
@@ -195,6 +195,18 @@ public sealed class AbCipDriver : IDriver, IReadable, IWritable, ITagDiscovery,
// Configured-vs-effective rendering of _transport for the init log, e.g. "Native (default)".
private readonly string _transportDescription;
// Tags whose type-mismatch Warning has already been logged (final review I4): a mis-authored tag is
// polled forever, so the Warning is once per tag per driver instance, not once per poll.
private readonly System.Collections.Concurrent.ConcurrentDictionary<string, byte> _typeMismatchWarned =
new(StringComparer.OrdinalIgnoreCase);
/// <summary>Logs a type-mismatch Warning the first time <paramref name="tagName"/> reports one on this instance.</summary>
private void WarnTypeMismatchOnce(string tagName, string message, params object?[] args)
{
if (_typeMismatchWarned.TryAdd(tagName, 0))
_logger.LogWarning(message, args);
}
/// <summary>Upper bound on the poll-loop failure backoff — adopts the S7-proven 30 s cap fleet-wide (05/STAB-8).</summary>
private static readonly TimeSpan PollBackoffCap = TimeSpan.FromSeconds(30);
@@ -807,6 +819,19 @@ public sealed class AbCipDriver : IDriver, IReadable, IWritable, ITagDiscovery,
await runtime.ReadAsync(ct).ConfigureAwait(false);
var status = runtime.GetStatus();
if (status == (int)AbStatus.TypeMismatch)
{
// Final review I4: the controller ANSWERED with a type that disagrees with the authored
// one — a deterministic config error, not a comms fault. Keep the handle (evicting would
// re-create it — and, for a sole lease holder, re-Forward-Open — on every poll), leave
// health alone, warn once per tag.
results[fb.OriginalIndex] = new DataValueSnapshot(null, AbCipStatusMapper.BadTypeMismatch, null, now);
WarnTypeMismatchOnce(def.Name,
"AbCip read of {Tag} on device {Device}: the controller's type does not match the authored " +
"{DataType}; reporting BadTypeMismatch until the tag's dataType is corrected (logged once per tag)",
reference, def.DeviceHostAddress, def.DataType);
return ReadOutcome.None;
}
if (status != 0)
{
// Evict the stale handle so the next call re-creates it.
@@ -834,11 +859,13 @@ public sealed class AbCipDriver : IDriver, IReadable, IWritable, ITagDiscovery,
&& ExpectedScalarWidth(def.DataType) is { } width
&& runtime.GetSize() is var size && size >= 0 && size != width)
{
// Same class as the native TypeMismatch above (final review I4): deterministic, device
// answered — no Degraded, warn once per tag.
results[fb.OriginalIndex] = new DataValueSnapshot(null, AbCipStatusMapper.BadTypeMismatch, null, now);
_logger.LogWarning(
"AbCip read of {Tag} on device {Device} returned {Size} bytes but its authored type {DataType} is {Width} bytes wide; check the tag's dataType",
WarnTypeMismatchOnce(def.Name,
"AbCip read of {Tag} on device {Device} returned {Size} bytes but its authored type {DataType} is {Width} bytes wide; check the tag's dataType (logged once per tag)",
reference, def.DeviceHostAddress, size, def.DataType, width);
return ReadOutcome.Degraded($"type mismatch reading {reference}: {size} bytes for {def.DataType}");
return ReadOutcome.None;
}
// Review I-1 — an array tag (the EXPLICIT IsArray flag) decodes the whole buffer into an
// element-typed CLR array (int[]/float[]/bool[]/string[]…), INCLUDING a 1-element array
@@ -865,6 +892,14 @@ public sealed class AbCipDriver : IDriver, IReadable, IWritable, ITagDiscovery,
results[fb.OriginalIndex] = new DataValueSnapshot(null, AbCipStatusMapper.BadNotSupported, null, now);
return ReadOutcome.None;
}
catch (AbTransportException ate) when (ate.Status == AbStatus.TypeMismatch)
{
// Final review I4: deterministic, keep the handle, no health signal, warn once per tag.
results[fb.OriginalIndex] = new DataValueSnapshot(null, AbCipStatusMapper.BadTypeMismatch, null, now);
WarnTypeMismatchOnce(def.Name, "AbCip read of {Tag} on device {Device} refused: {Reason} (logged once per tag)",
reference, def.DeviceHostAddress, ate.Message);
return ReadOutcome.None;
}
catch (Exception ex)
{
// Transport exception — evict so the next read creates a fresh handle. An AbTransportException
@@ -911,6 +946,16 @@ public sealed class AbCipDriver : IDriver, IReadable, IWritable, ITagDiscovery,
await runtime.ReadAsync(ct).ConfigureAwait(false);
var status = runtime.GetStatus();
if (status == (int)AbStatus.TypeMismatch)
{
// Final review I4: deterministic — keep the handle, no health signal, warn once.
StampGroupStatus(group, results, now, AbCipStatusMapper.BadTypeMismatch);
WarnTypeMismatchOnce(parent.Name,
"AbCip whole-UDT read of {Parent} on device {Device}: the controller's type does not match " +
"the authored type; {MemberCount} members report BadTypeMismatch (logged once per tag)",
group.ParentName, parent.DeviceHostAddress, group.Members.Count);
return ReadOutcome.None;
}
if (status != 0)
{
EvictRuntime(device, parent.Name);
@@ -944,6 +989,14 @@ public sealed class AbCipDriver : IDriver, IReadable, IWritable, ITagDiscovery,
StampGroupStatus(group, results, now, AbCipStatusMapper.BadNotSupported);
return ReadOutcome.None;
}
catch (AbTransportException ate) when (ate.Status == AbStatus.TypeMismatch)
{
StampGroupStatus(group, results, now, AbCipStatusMapper.BadTypeMismatch);
WarnTypeMismatchOnce(parent.Name,
"AbCip whole-UDT read of {Parent} on device {Device} refused: {Reason} (logged once per tag)",
group.ParentName, parent.DeviceHostAddress, ate.Message);
return ReadOutcome.None;
}
catch (Exception ex)
{
EvictRuntime(device, parent.Name);
@@ -1027,7 +1080,15 @@ public sealed class AbCipDriver : IDriver, IReadable, IWritable, ITagDiscovery,
opLock.Release();
}
if (status != 0)
if (status == (int)AbStatus.TypeMismatch)
{
// Final review I4: deterministic — keep the handle, leave health, warn once per tag.
results[i] = new WriteResult(AbCipStatusMapper.BadTypeMismatch);
WarnTypeMismatchOnce(def.Name,
"AbCip write to {Tag} on device {Device} refused: the controller's type does not match the authored {DataType} (logged once per tag)",
w.FullReference, def.DeviceHostAddress, def.DataType);
}
else if (status != 0)
{
EvictRuntime(device, def.Name);
results[i] = new WriteResult(AbCipStatusMapper.MapStatus(status));
@@ -1089,6 +1150,16 @@ public sealed class AbCipDriver : IDriver, IReadable, IWritable, ITagDiscovery,
"AbCip write refused (not supported) for tag {Tag} on device {Device}",
w.FullReference, def.DeviceHostAddress);
}
catch (AbTransportException ate) when (ate.Status == AbStatus.TypeMismatch)
{
// Final review I4: the controller's type disagrees with the authored one (native type gate, or
// CIP 0xFF/0x2107 from the controller) — deterministic: BadTypeMismatch, keep the handle, leave
// health alone, warn once per tag. Retrying cannot succeed until the config is corrected.
results[i] = new WriteResult(AbCipStatusMapper.BadTypeMismatch);
WarnTypeMismatchOnce(def.Name,
"AbCip write to {Tag} on device {Device} refused: {Reason} (logged once per tag)",
w.FullReference, def.DeviceHostAddress, ate.Message);
}
catch (Exception ex)
{
// Transport / wire error — evict so the next write creates a fresh handle.
src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.AbCip/NativeAbCipTagFactory.cs
+5 -2
View File
@@ -7,9 +7,12 @@ namespace ZB.MOM.WW.OtOpcUa.Driver.AbCip;
/// connection from <see cref="DeviceSessionPool.Shared"/>, so all tags on one device share one CIP
/// connection and one request scheduler. Stateless; safe to share across devices.
/// </summary>
internal sealed class NativeAbCipTagFactory : IAbCipTagFactory
internal sealed class NativeAbCipTagFactory(DeviceSessionPool? pool = null) : IAbCipTagFactory
{
// Tests pass a private pool (and a fake target); production uses the process-wide one.
private readonly DeviceSessionPool _pool = pool ?? DeviceSessionPool.Shared;
/// <inheritdoc />
public IAbCipTagRuntime Create(AbCipTagCreateParams createParams) =>
new NativeAbCipTagRuntime(createParams, DeviceSessionPool.Shared);
new NativeAbCipTagRuntime(createParams, _pool);
}