docs(xmldoc): fill missing XML docs + strip tracking-ID comments across src
v2-ci / build (push) Failing after 41s
v2-ci / unit-tests (tests/Core/ZB.MOM.WW.OtOpcUa.Cluster.Tests) (push) Has been skipped
v2-ci / unit-tests (tests/Server/ZB.MOM.WW.OtOpcUa.ControlPlane.Tests) (push) Has been skipped
v2-ci / unit-tests (tests/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer.Tests) (push) Has been skipped
v2-ci / unit-tests (tests/Server/ZB.MOM.WW.OtOpcUa.Runtime.Tests) (push) Has been skipped
v2-ci / unit-tests (tests/Server/ZB.MOM.WW.OtOpcUa.Security.Tests) (push) Has been skipped
v2-ci / integration (tests/Server/ZB.MOM.WW.OtOpcUa.Host.IntegrationTests) (push) Has been skipped
v2-ci / integration (tests/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer.IntegrationTests) (push) Has been skipped

Adds <summary>/<param>/<returns>/<inheritdoc> where missing and removes
project bookkeeping IDs (task/tracking refs) from shipped code comments,
so the docs read cleanly and CommentChecker is quiet except for known
false positives (PLC/protocol terms, event/IEqualityComparer inheritdoc).
Doc/comment-only; no logic changed; solution builds clean.
This commit is contained in:
Joseph Doherty
2026-07-07 12:38:39 -04:00
parent 384dbd7d36
commit 9cad9ed0fc
375 changed files with 1899 additions and 2493 deletions
@@ -62,7 +62,6 @@ public sealed class S7Driver
/// Handle to the in-flight probe loop. Tracked (rather than fire-and-forget) so
/// <see cref="ShutdownAsync"/> can await it after cancelling — otherwise a probe
/// iteration still inside the <see cref="_gate"/> would race a disposed semaphore.
/// See code-review finding Driver.S7-006.
/// </summary>
private Task? _probeTask;
@@ -101,7 +100,7 @@ public sealed class S7Driver
/// <summary>
/// Active driver configuration. Seeded from the constructor argument, then replaced by
/// whatever <see cref="InitializeAsync"/> / <see cref="ReinitializeAsync"/> parse out of
/// the supplied <c>driverConfigJson</c> — see code-review finding Driver.S7-011. The
/// the supplied <c>driverConfigJson</c>. The
/// constructor value is the fallback used when the caller passes an empty / placeholder
/// JSON document (e.g. the <c>"{}"</c> some unit tests pass).
/// </summary>
@@ -125,22 +124,19 @@ public sealed class S7Driver
private DriverHealth _health = new(DriverState.Unknown, null, null);
private bool _disposed;
/// <summary>Gets the unique driver instance identifier.</summary>
/// <inheritdoc />
public string DriverInstanceId => _driverInstanceId;
/// <summary>Gets the driver type name.</summary>
/// <inheritdoc />
public string DriverType => "S7";
/// <summary>Initializes the driver with the provided configuration.</summary>
/// <param name="driverConfigJson">JSON configuration string.</param>
/// <param name="cancellationToken">Cancellation token.</param>
/// <returns>A task representing the asynchronous operation.</returns>
/// <inheritdoc />
public async Task InitializeAsync(string driverConfigJson, CancellationToken cancellationToken)
{
_health = new DriverHealth(DriverState.Initializing, null, null);
try
{
// Re-parse the supplied DriverConfig JSON so a config change delivered through the
// IDriver contract is honoured (Driver.S7-011). An empty / placeholder document
// IDriver contract is honoured. An empty / placeholder document
// (e.g. the "{}" some unit tests pass) keeps the constructor-supplied options.
if (HasConfigBody(driverConfigJson))
_options = S7DriverFactoryExtensions.ParseOptions(_driverInstanceId, driverConfigJson);
@@ -152,7 +148,7 @@ public sealed class S7Driver
// so a config typo fails fast at init instead of as a misleading per-read fault.
RejectUnsupportedTagConfigs();
// Legacy data-type seam (Driver.S7-013). The UnimplementedDataTypes set is now
// Legacy data-type seam. The UnimplementedDataTypes set is now
// empty (Phase 4d unblocked all five wide types), so this rejects nothing — kept as
// the single grep target / future seam should a type ever need re-gating at init.
RejectUnsupportedTagDataTypes();
@@ -194,7 +190,7 @@ public sealed class S7Driver
{
_probeCts = new CancellationTokenSource();
// Track the probe Task (not fire-and-forget) so ShutdownAsync can await it
// before disposing _gate / _probeCts (Driver.S7-006). Pass None to Task.Run so
// before disposing _gate / _probeCts. Pass None to Task.Run so
// the delegate always runs and the handle is always awaitable; the loop's own
// token check handles cancellation.
_probeTask = Task.Run(() => ProbeLoopAsync(_probeCts.Token), CancellationToken.None);
@@ -212,27 +208,22 @@ public sealed class S7Driver
}
}
/// <summary>Reinitializes the driver with a new configuration.</summary>
/// <param name="driverConfigJson">JSON configuration string.</param>
/// <param name="cancellationToken">Cancellation token.</param>
/// <returns>A task representing the asynchronous operation.</returns>
/// <inheritdoc />
public async Task ReinitializeAsync(string driverConfigJson, CancellationToken cancellationToken)
{
// InitializeAsync re-parses driverConfigJson, so a config change delivered here is
// applied in place rather than silently discarded (Driver.S7-011).
// applied in place rather than silently discarded.
await ShutdownAsync(cancellationToken).ConfigureAwait(false);
await InitializeAsync(driverConfigJson, cancellationToken).ConfigureAwait(false);
}
/// <summary>Shuts down the driver and releases resources.</summary>
/// <param name="cancellationToken">Cancellation token.</param>
/// <returns>A task representing the asynchronous operation.</returns>
/// <inheritdoc />
public async Task ShutdownAsync(CancellationToken cancellationToken)
{
// Signal cancellation to the probe + poll loops first, collect their Task handles,
// then await all of them with a bounded timeout BEFORE disposing the shared semaphore
// and CTS objects. Without the drain, a loop iteration mid-_gate would call Release()
// on (or WaitAsync against) a disposed semaphore — see code-review finding Driver.S7-006.
// on (or WaitAsync against) a disposed semaphore.
var drain = new List<Task>();
var probeCts = _probeCts;
@@ -271,13 +262,10 @@ public sealed class S7Driver
_health = new DriverHealth(DriverState.Unknown, _health.LastSuccessfulRead, null);
}
/// <summary>Gets the current driver health.</summary>
/// <returns>The current health state.</returns>
/// <inheritdoc />
public DriverHealth GetHealth() => _health;
/// <summary>Flushes optional caches to free memory.</summary>
/// <param name="cancellationToken">Cancellation token.</param>
/// <returns>A task representing the asynchronous operation.</returns>
/// <inheritdoc />
public Task FlushOptionalCachesAsync(CancellationToken cancellationToken) => Task.CompletedTask;
/// <summary>
@@ -379,8 +367,8 @@ public sealed class S7Driver
"Timer/Counter are read-only this phase; set Writable=false.");
}
// (e) Array tag declared Writable — WriteOneAsync has no WriteArrayAsync path yet
// (Driver.S7-015). Without this guard a writable array node is discovered and
// (e) Array tag declared Writable — WriteOneAsync has no WriteArrayAsync path yet.
// Without this guard a writable array node is discovered and
// accepted, then every write returns BadCommunicationError (InvalidCastException from
// BoxValueForWrite receiving a typed array) instead of the informative BadNotSupported
// the caller could act on. Wide-type arrays are already rejected above (guard-a); this
@@ -401,7 +389,7 @@ public sealed class S7Driver
/// <see cref="UnimplementedDataTypes"/> list — types <see cref="ReinterpretRawValue"/> /
/// <see cref="BoxValueForWrite"/> throw <see cref="NotSupportedException"/> for, which
/// would otherwise create live OPC UA nodes via <see cref="DiscoverAsync"/> that return
/// <c>BadNotSupported</c> on every access (code-review finding Driver.S7-013).
/// <c>BadNotSupported</c> on every access.
/// <b>The set is now empty</b> (Phase 4d unblocked all five wide types), so this rejects
/// nothing — kept as the seam / grep target should a type ever need re-gating at init.
/// </summary>
@@ -430,25 +418,20 @@ public sealed class S7Driver
/// </summary>
private static readonly HashSet<S7DataType> UnimplementedDataTypes = new();
/// <summary>
/// Approximate memory footprint. The Plc instance + one 240-960 byte PDU buffer is
/// under 4 KB; return 0 because the <see cref="IDriver"/> contract asks for a
/// driver-attributable growth number and S7.Net doesn't expose one.
/// </summary>
/// <inheritdoc />
// The Plc instance + one 240-960 byte PDU buffer is under 4 KB; returns 0 because the
// IDriver contract asks for a driver-attributable growth number and S7.Net doesn't expose one.
public long GetMemoryFootprint() => 0;
// ---- IReadable ----
/// <summary>Reads values from the specified tag references.</summary>
/// <param name="fullReferences">Tag references to read.</param>
/// <param name="cancellationToken">Cancellation token.</param>
/// <returns>A task representing the asynchronous operation returning a list of data value snapshots.</returns>
/// <inheritdoc />
public async Task<IReadOnlyList<DataValueSnapshot>> ReadAsync(
IReadOnlyList<string> fullReferences, CancellationToken cancellationToken)
{
// Validate the list before RequirePlc() so a null argument produces an
// ArgumentNullException (consistent with DiscoverAsync) rather than an
// InvalidOperationException from the not-initialized check — Driver.S7-003.
// InvalidOperationException from the not-initialized check.
ArgumentNullException.ThrowIfNull(fullReferences);
var plc = RequirePlc();
var now = DateTime.UtcNow;
@@ -480,8 +463,7 @@ public sealed class S7Driver
// PUT/GET-disabled (S7-1200/1500) / access-protection — a permanent
// configuration fault, NOT a transient one. Blind retry is wasted effort,
// so map it to BadNotSupported and flag the driver as a config alert
// (Faulted) rather than Degraded — per driver-specs.md §5 and
// code-review finding Driver.S7-007.
// (Faulted) rather than Degraded — per driver-specs.md §5.
results[i] = new DataValueSnapshot(null, StatusBadNotSupported, null, now);
_health = new DriverHealth(DriverState.Faulted, _health.LastSuccessfulRead,
"S7 access denied — enable PUT/GET communication in TIA Portal " +
@@ -900,7 +882,7 @@ public sealed class S7Driver
/// unsigned type: <c>bool</c>, <c>byte</c>, <c>ushort</c>, <c>uint</c>) into the
/// SEMANTIC type declared by the tag's <see cref="S7DataType"/>. No network I/O.
/// Factored out of <see cref="ReadOneAsync"/> so it can be exercised in unit tests
/// without a live PLC (Driver.S7-014).
/// without a live PLC.
/// </summary>
/// <param name="tag">Tag definition containing type information.</param>
/// <param name="addr">Parsed tag address.</param>
@@ -935,15 +917,12 @@ public sealed class S7Driver
// ---- IWritable ----
/// <summary>Writes values to the specified tags.</summary>
/// <param name="writes">Write requests containing tag references and values.</param>
/// <param name="cancellationToken">Cancellation token.</param>
/// <returns>A task representing the asynchronous operation returning a list of write results.</returns>
/// <inheritdoc />
public async Task<IReadOnlyList<WriteResult>> WriteAsync(
IReadOnlyList<WriteRequest> writes, CancellationToken cancellationToken)
{
// Same as ReadAsync — validate before RequirePlc() so a null argument is a
// typed argument error, not the "not initialized" surface (Driver.S7-003).
// typed argument error, not the "not initialized" surface.
ArgumentNullException.ThrowIfNull(writes);
var plc = RequirePlc();
var results = new WriteResult[writes.Count];
@@ -987,7 +966,7 @@ public sealed class S7Driver
}
catch (OperationCanceledException)
{
// Driver.S7-008: let cancellation propagate rather than turning it into
// Let cancellation propagate rather than turning it into
// a status code — the gate is still held so Release() runs in finally.
throw;
}
@@ -998,7 +977,7 @@ public sealed class S7Driver
catch (PlcException pex) when (IsAccessDenied(pex))
{
// PUT/GET-disabled / access-protection on write — same permanent
// configuration fault as on read (Driver.S7-007). BadNotSupported +
// configuration fault as on read. BadNotSupported +
// a config-alert health state, not a transient device failure.
results[i] = new WriteResult(StatusBadNotSupported);
_health = new DriverHealth(DriverState.Faulted, _health.LastSuccessfulRead,
@@ -1009,14 +988,14 @@ public sealed class S7Driver
{
// Genuine device-layer fault — degrade health so a PLC-down-during-writes
// scenario is visible to the operator (previously health was never updated
// on write failure — Driver.S7-008).
// on write failure).
results[i] = new WriteResult(StatusBadDeviceFailure);
_health = new DriverHealth(DriverState.Degraded, _health.LastSuccessfulRead, pex.Message);
}
catch (Exception ex)
{
// Socket/timeout/conversion failure. Map to BadCommunicationError (not
// BadInternalError) for transport faults; degrade health — Driver.S7-008.
// BadInternalError) for transport faults; degrade health.
results[i] = new WriteResult(StatusBadCommunicationError);
_health = new DriverHealth(DriverState.Degraded, _health.LastSuccessfulRead, ex.Message);
}
@@ -1028,7 +1007,7 @@ public sealed class S7Driver
private async Task WriteOneAsync(Plc plc, S7TagDefinition tag, object? value, CancellationToken ct)
{
// Defence-in-depth guard for Driver.S7-015: authored array tags are rejected at init
// Defence-in-depth guard: authored array tags are rejected at init
// (RejectUnsupportedTagConfigs guard-e), but a transient equipment-tag ref resolved by
// _resolver bypasses that path. Both should fail with NotSupportedException → BadNotSupported,
// not InvalidCastException → BadCommunicationError from BoxValueForWrite receiving an array.
@@ -1080,7 +1059,7 @@ public sealed class S7Driver
/// S7.Net's <c>Plc.WriteAsync</c> expects for each address size (bool → bool, byte
/// → byte, short → ushort, int → uint, float → uint-bits). No network I/O.
/// Factored out of <see cref="WriteOneAsync"/> so it can be exercised in unit tests
/// without a live PLC (Driver.S7-014).
/// without a live PLC.
/// </summary>
/// <param name="dataType">Target S7 data type.</param>
/// <param name="value">Value to box.</param>
@@ -1116,7 +1095,7 @@ public sealed class S7Driver
/// the response-code validator throws a plain <see cref="Exception"/> for the S7
/// <c>AccessingObjectNotAllowed</c> status, which lands as the inner exception. There is
/// no typed error code for it, so the inner message is the only discriminator
/// S7.Net exposes — see code-review finding Driver.S7-007.
/// S7.Net exposes.
/// </summary>
private static bool IsAccessDenied(PlcException pex)
{
@@ -1133,17 +1112,13 @@ public sealed class S7Driver
// ---- ITagDiscovery ----
/// <summary>
/// Run-once: <see cref="DiscoverAsync"/> emits the complete node set synchronously from
/// the configured tag table in a single pass — nothing fills in asynchronously after
/// connect, so a single discovery pass is sufficient.
/// </summary>
/// <inheritdoc />
// Run-once: DiscoverAsync emits the complete node set synchronously from the configured tag
// table in a single pass — nothing fills in asynchronously after connect, so a single
// discovery pass is sufficient.
public DiscoveryRediscoverPolicy RediscoverPolicy => DiscoveryRediscoverPolicy.Once;
/// <summary>Discovers tags and builds the OPC UA address space.</summary>
/// <param name="builder">Address space builder.</param>
/// <param name="cancellationToken">Cancellation token.</param>
/// <returns>A task representing the asynchronous operation.</returns>
/// <inheritdoc />
public Task DiscoverAsync(IAddressSpaceBuilder builder, CancellationToken cancellationToken)
{
ArgumentNullException.ThrowIfNull(builder);
@@ -1171,7 +1146,7 @@ public sealed class S7Driver
{
S7DataType.Bool => DriverDataType.Boolean,
S7DataType.Byte => DriverDataType.Int32, // no 8-bit in DriverDataType yet
// Driver.S7-002: UInt32 values > int.MaxValue (2^31-1) wrap negative when surfaced as
// UInt32 values > int.MaxValue (2^31-1) wrap negative when surfaced as
// Int32. That residual lossy note now applies ONLY to UInt32 — Int64/UInt64 map to
// their own DriverDataType members below (Phase 4d), so they are no longer lossy.
S7DataType.Int16 or S7DataType.UInt16 or S7DataType.Int32 or S7DataType.UInt32 => DriverDataType.Int32,
@@ -1186,11 +1161,7 @@ public sealed class S7Driver
// ---- ISubscribable (polling overlay) ----
/// <summary>Subscribes to changes on the specified tag references.</summary>
/// <param name="fullReferences">Tag references to subscribe to.</param>
/// <param name="publishingInterval">Polling interval.</param>
/// <param name="cancellationToken">Cancellation token.</param>
/// <returns>A task representing the asynchronous operation returning a subscription handle.</returns>
/// <inheritdoc />
public Task<ISubscriptionHandle> SubscribeAsync(
IReadOnlyList<string> fullReferences, TimeSpan publishingInterval, CancellationToken cancellationToken)
{
@@ -1205,15 +1176,12 @@ public sealed class S7Driver
var state = new SubscriptionState(handle, [.. fullReferences], interval, cts);
_subscriptions[id] = state;
// Track the poll Task so ShutdownAsync can await it after cancelling — a poll
// iteration mid-_gate would otherwise race the semaphore's disposal (Driver.S7-006).
// iteration mid-_gate would otherwise race the semaphore's disposal.
state.PollTask = Task.Run(() => PollLoopAsync(state, cts.Token), CancellationToken.None);
return Task.FromResult<ISubscriptionHandle>(handle);
}
/// <summary>Unsubscribes from a subscription.</summary>
/// <param name="handle">Subscription handle.</param>
/// <param name="cancellationToken">Cancellation token.</param>
/// <returns>A task representing the asynchronous operation.</returns>
/// <inheritdoc />
public Task UnsubscribeAsync(ISubscriptionHandle handle, CancellationToken cancellationToken)
{
if (handle is S7SubscriptionHandle h && _subscriptions.TryRemove(h.Id, out var state))
@@ -1227,7 +1195,7 @@ public sealed class S7Driver
/// <summary>
/// Upper bound on the poll-loop backoff window. After enough consecutive failures the
/// loop waits this long between retries instead of <see cref="SubscriptionState.Interval"/>,
/// so a subscription against a dropped / uninitialised driver doesn't spin (Driver.S7-009).
/// so a subscription against a dropped / uninitialised driver doesn't spin.
/// </summary>
private static readonly TimeSpan PollBackoffCap = TimeSpan.FromSeconds(30);
@@ -1235,7 +1203,7 @@ public sealed class S7Driver
/// Number of consecutive poll failures before the loop transitions the driver's
/// health to <see cref="DriverState.Degraded"/>. One stray failure can be transient;
/// a sustained run indicates the operator should see it. Threshold of 1 because the
/// first failure already lives in the LastError surface — see Driver.S7-009.
/// first failure already lives in the LastError surface.
/// </summary>
private const int PollFailureHealthThreshold = 1;
@@ -1283,7 +1251,7 @@ public sealed class S7Driver
/// <summary>
/// Logs the swallowed poll exception and, once <see cref="PollFailureHealthThreshold"/>
/// consecutive failures have accumulated, degrades the driver health so the failure
/// surfaces on the dashboard — see Driver.S7-009. The probe loop owns Running/Stopped
/// surfaces on the dashboard. The probe loop owns Running/Stopped
/// transitions for the host-connectivity surface, so we touch <see cref="_health"/>
/// rather than the probe state.
/// </summary>
@@ -1353,14 +1321,14 @@ public sealed class S7Driver
/// <summary>
/// Handle to this subscription's poll loop. Tracked so <see cref="ShutdownAsync"/>
/// can await it after cancelling — see code-review finding Driver.S7-006.
/// can await it after cancelling.
/// </summary>
public Task PollTask { get; set; } = Task.CompletedTask;
}
private sealed record S7SubscriptionHandle(long Id) : ISubscriptionHandle
{
/// <summary>Gets the diagnostic identifier for this subscription.</summary>
/// <inheritdoc />
public string DiagnosticId => $"s7-sub-{Id}";
}
@@ -1373,8 +1341,7 @@ public sealed class S7Driver
/// </summary>
public string HostName => $"{_options.Host}:{_options.Port}";
/// <summary>Gets the host connectivity statuses.</summary>
/// <returns>A list containing the current host status.</returns>
/// <inheritdoc />
public IReadOnlyList<HostConnectivityStatus> GetHostStatuses()
{
lock (_probeLock)
@@ -1433,7 +1400,7 @@ public sealed class S7Driver
/// <summary>Disposes the driver and releases resources.</summary>
public void Dispose()
{
// Driver.S7-010: avoid the sync-over-async DisposeAsync().AsTask().GetAwaiter().GetResult()
// Avoid the sync-over-async DisposeAsync().AsTask().GetAwaiter().GetResult()
// pattern (a known deadlock surface even when currently safe here). ShutdownAsync's
// body is effectively synchronous apart from waiting on probe/poll Tasks; do the same
// teardown directly, blocking only on the drain — and only with a bounded timeout so
@@ -1458,8 +1425,7 @@ public sealed class S7Driver
/// <summary>
/// Synchronous teardown — mirrors <see cref="ShutdownAsync"/> but blocks (with a bounded
/// timeout) on the probe + poll Tasks instead of awaiting them. Used by the sync
/// <see cref="Dispose"/> path so we don't sync-over-async <see cref="DisposeAsync"/>
/// (Driver.S7-010).
/// <see cref="Dispose"/> path so we don't sync-over-async <see cref="DisposeAsync"/>.
/// </summary>
private void SynchronousTeardown()
{