diff --git a/docs/drivers/MTConnect.md b/docs/drivers/MTConnect.md index 9ff329d1..ad0b7984 100644 --- a/docs/drivers/MTConnect.md +++ b/docs/drivers/MTConnect.md @@ -90,7 +90,7 @@ request/response handshake — is a different mechanism and remains deferred; se | `IReadable` | `ReadAsync` → the live `/sample` pump's index while it is delivering and healthy, else one `/current` per call | **Not the production data path** — see below | | `IWritable` | `WriteAsync` → `POST {agent}/{device}?{dataItemId}={value}` on the Agent's `AllowPut` surface | **Off by default** — needs `Write.Enabled` **and** the Agent's `AllowPut`; CONDITION items are refused | | `ISubscribable` | `SubscribeAsync`/`OnDataChange` — the shared `/sample` long-poll pump | **The production data path** | -| `IHostConnectivityProbe` | periodic headers-only `/probe` ping under `Probe.*` — the status is read, the device model is neither downloaded in full nor parsed (review 2026-10-21 T-7; it used to parse the whole model every tick) | Consumed — feeds `/hosts` (see [Known limitations](#known-limitations)) | +| `IHostConnectivityProbe` | periodic headers-only `/probe` ping under `Probe.*` — the status is read, the device model is neither downloaded in full nor parsed (review 2026-10-21 T-7; it used to parse the whole model every tick). Trade-off: a 200 from something that is not an Agent (a proxy's HTML page) now reads as reachable; the data paths still fail on it | Consumed — feeds `/hosts` (see [Known limitations](#known-limitations)) | | `IRediscoverable` | watches the Agent's `Header.instanceId` | No consumer wires it today — see [Known limitations](#known-limitations) | | `IDriverDiagnostics` | `GetDiagnostics()` — `/sample` effective count, lag, saturated chunks, count raises | Shown on `/hosts` (review 2026-10-21 T-5) | | `IAlarmSource` | CONDITION observations off the same `/sample` pump → Part 9 transitions keyed by the alarm tag's RawPath | Opt-in per raw tag (`alarm` object) — see [CONDITION alarms](#condition-alarms-626) | @@ -118,7 +118,10 @@ correct and tested, just not on the hot path. delivered since it started (a chunk, heartbeat or re-baseline) and its stream is not degraded, a read is answered from the pump's observation index — **read only**; the pump stays its single writer — with no Agent round trip. Otherwise (no subscription, a pump still catching up after a resubscribe, a degraded stream) it falls back to one -`/current`, whose per-batch index is now built over a tag map frozen once per binding. A served read also clears a +`/current`, whose per-batch index is now built over a tag map frozen once per binding. **Stale window:** a +stream that stalls without failing (no chunk, no heartbeat) still serves Good reads from the index until the client's +idle watchdog fires (2 × `HeartbeatMs` + `RequestTimeoutMs`) — the same bounded window subscriptions already have. During +an Agent-restart re-baseline reads fall back to `/current`. A served read also clears a read-path degradation an earlier failed `/current` left (the read path is answering). The `/current` fallback now checks the answer's `instanceId`: a changed one (an Agent restart seen by a browse-only driver with no pump) drops the cached `/probe` model and raises `OnRediscoveryNeeded` once per distinct id — it used to be detected only by the pump. @@ -349,7 +352,9 @@ and whether it was **saturated** (carried the full `count`). Three consecutive s unknown) lag reopen the stream at the cursor reached with `count` doubled, up to 16× `SampleCount` (max 65 536). A raised stream that delivers nothing (an Agent refusing the larger count) steps back to the last count that delivered and stops raising; at the ceiling a still-behind stream logs one Warning per pump. Every new pump starts from the -configured `SampleCount`. The driver now implements `IDriverDiagnostics`: `sampleCount` (effective), `sampleLag`, +configured `SampleCount`. An Agent that refuses the raised count with an `MTConnectError` under HTTP 200 goes through +the ordinary unusable-answer path first — one transient Degraded blip and a backoff — before the step-back. The +driver now implements `IDriverDiagnostics`: `sampleCount` (effective), `sampleLag`, `sampleSaturatedChunks` and `sampleCountRaises` ride the `/hosts` diagnostics bag. **A pump carries the session generation it was started against (review 2026-10-21 T-1).** A lifecycle restart @@ -441,7 +446,9 @@ go out as one `POST {agent}/{device}?id1=v1&id2=v2…` per device (the Agent's P several pairs) instead of one POST per slot. A batch closes at a slot naming another device or repeating a DataItem already in it, so two writes to one DataItem still land in request order. An Agent refusal of a multi-slot request is retried slot by slot, so a refused DataItem costs -only its own slot (`BadCommunicationError`); a transport failure fails that request's slots. +only its own slot (`BadCommunicationError`); a transport failure fails that request's slots. An Agent may apply +some pairs of a multi-pair PUT before refusing it, so the per-slot retry can re-send a pair that already landed — +harmless for value-setting (the Agent drops an identical repeat). **Device resolution needs no extra config.** `TagBinding.WriteTargetsByDataItemId`, built at bind time from a walk of the cached probe model, remembers which device declared diff --git a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.MTConnect/IMTConnectAgentClient.cs b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.MTConnect/IMTConnectAgentClient.cs index 76f46b7c..5ab06802 100644 --- a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.MTConnect/IMTConnectAgentClient.cs +++ b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.MTConnect/IMTConnectAgentClient.cs @@ -83,6 +83,11 @@ public interface IMTConnectAgentClient : IDisposable /// would for an unreachable or refusing Agent. The default delegates to /// and discards the model. /// + /// + /// Trade-off (review m5): a success status from something that is NOT an Agent — a proxy serving an HTML page + /// under 200 — now reads as reachable, where would have failed to parse it. The data + /// paths still report that misconfiguration (the read / stream fail to parse). + /// /// Cancellation token (the caller's deadline). /// A task completing when the Agent has answered with a success status. async Task PingAsync(CancellationToken ct) => _ = await ProbeAsync(ct).ConfigureAwait(false); diff --git a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.MTConnect/MTConnectDriver.cs b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.MTConnect/MTConnectDriver.cs index 4c5164de..853caf36 100644 --- a/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.MTConnect/MTConnectDriver.cs +++ b/src/Drivers/ZB.MOM.WW.OtOpcUa.Driver.MTConnect/MTConnectDriver.cs @@ -714,7 +714,13 @@ public sealed partial class MTConnectDriver // One read of the session reference: the client used below is the one that was live at this // instant, never a half-swapped pairing (see the _session remarks). - var client = Volatile.Read(ref _session)?.Client; + IMTConnectAgentClient? client; + long readGeneration; + lock (_subscriptionLock) + { + client = Volatile.Read(ref _session)?.Client; + readGeneration = _sessionGeneration; + } var options = Volatile.Read(ref _options); // Read once, for the same reason the session is: every reference in this batch must be @@ -749,9 +755,14 @@ public sealed partial class MTConnectDriver // rediscovery. Only while the session this read used is still installed: a read that raced a // re-initialise against a different Agent would otherwise announce a "restart" the operator caused. if (current.InstanceId != Interlocked.Read(ref _announcedInstanceId) - && ReferenceEquals(Volatile.Read(ref _session)?.Client, client)) + && IsCurrentSessionGeneration(readGeneration)) { AnnounceAgentRestart(current.InstanceId, options); + + // Review m6: the session's cursor still named the dead instance, so the next pump would open there and + // spend its first chunk on a clear + re-baseline (a Bad flicker). Move it to the new process — through + // the same generation-checked publish the pump uses, so it can only land on the session this read used. + PublishAgentCursor(readGeneration, current.InstanceId, current.NextSequence); } // Built from the SAME authored tags as the shared index, so the authored-vs-unknown @@ -810,6 +821,13 @@ public sealed partial class MTConnectDriver /// trusted as the Agent's current state: no pump, a pump bound to a retired session, one that has not yet /// delivered, or a degraded stream. Touches no health — the pump owns the stream's. /// + /// + /// Stale window (review m1). A stream that stalls WITHOUT failing (no chunk, no heartbeat) keeps + /// _streamDegraded clear until the client's idle watchdog fires (2 × HeartbeatMs + + /// RequestTimeoutMs); for that bounded window reads answer Good from an index nothing is updating, where a + /// /current would have noticed. Subscriptions have the identical exposure; the window is the same one + /// the watchdog already accepts for the data plane. + /// /// The batch. /// The tag table the batch is resolved against. /// One snapshot per reference in order, or to fall back to /current. @@ -1063,7 +1081,10 @@ public sealed partial class MTConnectDriver return; } - // The Agent refused the request as a whole: find out which slot(s) it objects to, one by one. + // The Agent refused the request as a whole: find out which slot(s) it objects to, one by one. (Review m4: + // an Agent may have applied some pairs before refusing — cppagent collects put errors per key — so the retry + // can re-send a pair that already landed. Harmless for value-setting: the Agent's duplicate filter drops an + // identical repeat.) foreach (var write in batch) { await WriteBatchAsync(client, options, [write], results, cancellationToken).ConfigureAwait(false); diff --git a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.MTConnect.Tests/MTConnectReadPathTests.cs b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.MTConnect.Tests/MTConnectReadPathTests.cs index 265e6906..605e810e 100644 --- a/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.MTConnect.Tests/MTConnectReadPathTests.cs +++ b/tests/Drivers/ZB.MOM.WW.OtOpcUa.Driver.MTConnect.Tests/MTConnectReadPathTests.cs @@ -130,6 +130,10 @@ public sealed class MTConnectReadPathTests raised.ShouldBe(1, "once per distinct instanceId, not once per read"); driver.CachedProbeModel.ShouldBeNull("the model describes a process that no longer exists"); + // Review m6: the session's cursor moves to the new process, so a later subscription does not open at the + // dead instance's sequence (and flicker through a clear + re-baseline on its first chunk). + driver.AgentInstanceId.ShouldBe(RestartedInstance); + driver.AgentNextSequence.ShouldBe(5); } finally {