fix(drivers): #630 re-review - S7 warns on gate-wait timeout; AbCip publishes fresh tag tables by reference swap
Claude-Session: https://claude.ai/code/session_01Ef2ozxS7QPanGT8uoATbTe
This commit is contained in:
@@ -106,10 +106,10 @@ public sealed class AbCipDriver : IDriver, IReadable, IWritable, ITagDiscovery,
|
|||||||
private readonly Dictionary<string, DeviceState> _devicesByRoutingKey = new(StringComparer.OrdinalIgnoreCase);
|
private readonly Dictionary<string, DeviceState> _devicesByRoutingKey = new(StringComparer.OrdinalIgnoreCase);
|
||||||
// v3 authored RawPath → definition table (parents + fanned-out UDT members), keyed case-sensitive
|
// v3 authored RawPath → definition table (parents + fanned-out UDT members), keyed case-sensitive
|
||||||
// ordinal. Built in InitializeAsync from _options.RawTags via the pure mapper.
|
// ordinal. Built in InitializeAsync from _options.RawTags via the pure mapper.
|
||||||
private readonly Dictionary<string, AbCipTagDefinition> _tagsByRawPath = new(StringComparer.Ordinal);
|
private volatile Dictionary<string, AbCipTagDefinition> _tagsByRawPath = new(StringComparer.Ordinal);
|
||||||
// Top-level authored definitions (parents + flat tags, NOT synthesised members), in author order —
|
// Top-level authored definitions (parents + flat tags, NOT synthesised members), in author order —
|
||||||
// the source DiscoverAsync groups by device. Members are synthesised inside the discovery/read paths.
|
// the source DiscoverAsync groups by device. Members are synthesised inside the discovery/read paths.
|
||||||
private readonly List<AbCipTagDefinition> _declaredTags = new();
|
private volatile List<AbCipTagDefinition> _declaredTags = new();
|
||||||
|
|
||||||
// Resolves a read/write/subscribe fullReference (always a RawPath in v3) to a tag definition by a
|
// Resolves a read/write/subscribe fullReference (always a RawPath in v3) to a tag definition by a
|
||||||
// single hit on the authored _tagsByRawPath table; a miss is a miss (the pre-v3 blob-parse fallback
|
// single hit on the authored _tagsByRawPath table; a miss is a miss (the pre-v3 blob-parse fallback
|
||||||
@@ -377,6 +377,8 @@ public sealed class AbCipDriver : IDriver, IReadable, IWritable, ITagDiscovery,
|
|||||||
// threaded onto the def (they live on the RawTagEntry, not inside the blob). A mapper miss is
|
// threaded onto the def (they live on the RawTagEntry, not inside the blob). A mapper miss is
|
||||||
// skipped (logged), never thrown — a bad tag must not fail the whole driver init.
|
// skipped (logged), never thrown — a bad tag must not fail the whole driver init.
|
||||||
_skippedTags.BeginRebuild(_options.RawTags.Count);
|
_skippedTags.BeginRebuild(_options.RawTags.Count);
|
||||||
|
var builtTagsByRawPath = new Dictionary<string, AbCipTagDefinition>(StringComparer.Ordinal);
|
||||||
|
var builtDeclaredTags = new List<AbCipTagDefinition>();
|
||||||
foreach (var entry in _options.RawTags)
|
foreach (var entry in _options.RawTags)
|
||||||
{
|
{
|
||||||
if (!AbCipTagDefinitionFactory.FromTagConfig(entry.TagConfig, entry.RawPath, out var mapped))
|
if (!AbCipTagDefinitionFactory.FromTagConfig(entry.TagConfig, entry.RawPath, out var mapped))
|
||||||
@@ -391,13 +393,13 @@ public sealed class AbCipDriver : IDriver, IReadable, IWritable, ITagDiscovery,
|
|||||||
|
|
||||||
// Duplicate-key check: a collision means two authored tags share the same RawPath.
|
// Duplicate-key check: a collision means two authored tags share the same RawPath.
|
||||||
// Fail fast at init time with a diagnostic rather than silently clobbering.
|
// Fail fast at init time with a diagnostic rather than silently clobbering.
|
||||||
if (_tagsByRawPath.TryGetValue(tag.Name, out var existingTag))
|
if (builtTagsByRawPath.TryGetValue(tag.Name, out var existingTag))
|
||||||
throw new InvalidOperationException(
|
throw new InvalidOperationException(
|
||||||
$"AbCip tag name collision: '{tag.Name}' is declared more than once. " +
|
$"AbCip tag name collision: '{tag.Name}' is declared more than once. " +
|
||||||
$"Existing entry DeviceHostAddress='{existingTag.DeviceHostAddress}', " +
|
$"Existing entry DeviceHostAddress='{existingTag.DeviceHostAddress}', " +
|
||||||
$"TagPath='{existingTag.TagPath}'. Rename or remove the duplicate.");
|
$"TagPath='{existingTag.TagPath}'. Rename or remove the duplicate.");
|
||||||
_tagsByRawPath[tag.Name] = tag;
|
builtTagsByRawPath[tag.Name] = tag;
|
||||||
_declaredTags.Add(tag);
|
builtDeclaredTags.Add(tag);
|
||||||
|
|
||||||
if (tag.DataType == AbCipDataType.Structure && tag.Members is { Count: > 0 })
|
if (tag.DataType == AbCipDataType.Structure && tag.Members is { Count: > 0 })
|
||||||
{
|
{
|
||||||
@@ -420,15 +422,18 @@ public sealed class AbCipDriver : IDriver, IReadable, IWritable, ITagDiscovery,
|
|||||||
// Member fan-out duplicate check: a member-path collision means two
|
// Member fan-out duplicate check: a member-path collision means two
|
||||||
// configured structure tags produce the same member path, or a member
|
// configured structure tags produce the same member path, or a member
|
||||||
// name collides with an independently-declared tag.
|
// name collides with an independently-declared tag.
|
||||||
if (_tagsByRawPath.TryGetValue(memberTag.Name, out var existingMember))
|
if (builtTagsByRawPath.TryGetValue(memberTag.Name, out var existingMember))
|
||||||
throw new InvalidOperationException(
|
throw new InvalidOperationException(
|
||||||
$"AbCip tag name collision: '{memberTag.Name}' is produced by both " +
|
$"AbCip tag name collision: '{memberTag.Name}' is produced by both " +
|
||||||
$"'{tag.Name}.{member.Name}' (member fan-out) and an existing tag " +
|
$"'{tag.Name}.{member.Name}' (member fan-out) and an existing tag " +
|
||||||
$"'{existingMember.Name}'. Rename one of the configured tags to resolve.");
|
$"'{existingMember.Name}'. Rename one of the configured tags to resolve.");
|
||||||
_tagsByRawPath[memberTag.Name] = memberTag;
|
builtTagsByRawPath[memberTag.Name] = memberTag;
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
// Publish the finished tables by reference swap — readers see the old or the new, never a half-built one.
|
||||||
|
_tagsByRawPath = builtTagsByRawPath;
|
||||||
|
_declaredTags = builtDeclaredTags;
|
||||||
_skippedTags.Commit();
|
_skippedTags.Commit();
|
||||||
|
|
||||||
// Probe loops — one per device when enabled + a ProbeTagPath is configured.
|
// Probe loops — one per device when enabled + a ProbeTagPath is configured.
|
||||||
@@ -498,8 +503,8 @@ public sealed class AbCipDriver : IDriver, IReadable, IWritable, ITagDiscovery,
|
|||||||
_devices.Clear();
|
_devices.Clear();
|
||||||
_devicesByRoutingKey.Clear();
|
_devicesByRoutingKey.Clear();
|
||||||
// Rebuilt wholesale by Initialize; stale entries would trip its member fan-out collision check.
|
// Rebuilt wholesale by Initialize; stale entries would trip its member fan-out collision check.
|
||||||
_tagsByRawPath.Clear();
|
_tagsByRawPath = new(StringComparer.Ordinal); // swap, never Clear(): the retained poll engine / discovery may be enumerating
|
||||||
_declaredTags.Clear();
|
_declaredTags = new();
|
||||||
}
|
}
|
||||||
|
|
||||||
/// <inheritdoc />
|
/// <inheritdoc />
|
||||||
@@ -546,8 +551,8 @@ public sealed class AbCipDriver : IDriver, IReadable, IWritable, ITagDiscovery,
|
|||||||
}
|
}
|
||||||
_devices.Clear();
|
_devices.Clear();
|
||||||
_devicesByRoutingKey.Clear();
|
_devicesByRoutingKey.Clear();
|
||||||
_tagsByRawPath.Clear();
|
_tagsByRawPath = new(StringComparer.Ordinal);
|
||||||
_declaredTags.Clear();
|
_declaredTags = new();
|
||||||
_resolver.Clear(); // no-op in v3 (the resolver reads the live table by closure); kept for symmetry
|
_resolver.Clear(); // no-op in v3 (the resolver reads the live table by closure); kept for symmetry
|
||||||
_health = new DriverHealth(DriverState.Unknown, _health.LastSuccessfulRead, null);
|
_health = new DriverHealth(DriverState.Unknown, _health.LastSuccessfulRead, null);
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -400,6 +400,10 @@ public sealed class S7Driver
|
|||||||
// teardown, in which case we proceed exactly as the un-gated path always did.
|
// teardown, in which case we proceed exactly as the un-gated path always did.
|
||||||
var gated = false;
|
var gated = false;
|
||||||
try { gated = await _gate.WaitAsync(DrainTimeout).ConfigureAwait(false); } catch { }
|
try { gated = await _gate.WaitAsync(DrainTimeout).ConfigureAwait(false); } catch { }
|
||||||
|
if (!gated)
|
||||||
|
_logger.LogWarning(
|
||||||
|
"S7 driver {DriverInstanceId}: disposing the S7 connection while an operation still holds the gate (waited {Timeout})",
|
||||||
|
_driverInstanceId, DrainTimeout);
|
||||||
try
|
try
|
||||||
{
|
{
|
||||||
_initialized = false;
|
_initialized = false;
|
||||||
|
|||||||
Reference in New Issue
Block a user