Remove the periodic 5-minute device re-browse (tag-set drift detection) from FOCAS/TwinCAT/MTConnect/AbCip #523

New Issue

2026-07-28T13:26:41-04:00

dohertj2 commented

2026-07-28 13:26:41 -04:00

Decision (owner, 2026-07-28): remove the periodic re-browse. Shipped in e77c8a35, merged to master in 914d70bf. This issue tracks taking it back out.

What is in `master` today

DriverInstanceActor starts a periodic timer on every Connected entry for drivers that declare ITagDiscovery.SupportsOnlineDiscovery and report AuthoredDiscoveryRefs. Each tick runs a full DiscoverAsync against the live device and diffs the result against the authored tag set, raising the /hosts re-browse prompt on a change.

Four drivers qualify: FOCAS, TwinCAT, MTConnect, AbCip. Interval is DriverInstanceActor.DefaultDriftCheckInterval = 5 minutes, a constructor parameter with no appsettings key — so a deployment cannot currently turn it off or slow it down without a code change. That is the core objection: it is recurring, unconfigurable, unsolicited traffic to real PLCs, CNCs and MTConnect Agents, and it was never live-gated.

What to remove

DriverInstanceActor: DriftCheckTick, StartDriftCheck, StopDriftCheck, HandleDriftCheckAsync, QualifiesForDriftCheck, _driftCheckInterval, _lastDriftSignature, DefaultDriftCheckInterval, DriftBrowseTimeout, CollectLeafRefs, the driftCheckInterval params on Props/ctor, the ReceiveAsync<DriftCheckTick> registration and the StartDriftCheck()/StopDriftCheck() call sites.
src/Server/ZB.MOM.WW.OtOpcUa.Runtime/Drivers/TagSetDrift.cs (whole file).
Tests: TagSetDriftDetectorTests, DriverInstanceActorDriftCheckTests.
Driver members: AuthoredDiscoveryRefs + DiscoveryStreamIncludesAuthoredTags on FocasDriver, TwinCATDriver, AbCipDriver, MTConnectDriver.
ITagDiscovery: the AuthoredDiscoveryRefs and DiscoveryStreamIncludesAuthoredTags members. Check for external implementors first — these are default-implemented interface members, so removing them is source-compatible for anyone who did not override them, but a driver in a sibling repo that did override would break.
Docs: the drift paragraphs in CLAUDE.md §Change Detection and the 2026-07-28 entry in deferment.md §9.

Do NOT remove the IRediscoverable consumption itself (09a401b8). That is a separate, event-driven signal — the driver tells us, we do not poll — and it costs nothing when nothing changes.

Consequence to record, not to argue

AbCip and FOCAS browse a live backend but implement no IRediscoverable, so after this removal they have no change signal at all: a PLC program re-download that adds or removes tags will be invisible until someone re-browses under /raw by hand. TwinCAT (ADS symbol-version) and MTConnect (Agent instanceId) keep their native event-driven signals and lose only the redundant polling.

If that turns out to matter for AbCip/FOCAS later, the event-driven shape is the one to reach for — a device-side change notification, or a drift check that runs only when something else already indicates a change (a reconnect, a deploy) rather than on a wall-clock timer.

Alternative considered and NOT chosen

Gating the interval behind an appsettings key (default off) would keep the capability available without the unsolicited traffic. The owner's decision is removal; recorded here only so the option is not re-discovered from scratch.

**Decision (owner, 2026-07-28): remove the periodic re-browse.** Shipped in `e77c8a35`, merged to `master` in `914d70bf`. This issue tracks taking it back out. ## What is in `master` today `DriverInstanceActor` starts a periodic timer on every Connected entry for drivers that declare `ITagDiscovery.SupportsOnlineDiscovery` **and** report `AuthoredDiscoveryRefs`. Each tick runs a full `DiscoverAsync` against the live device and diffs the result against the authored tag set, raising the `/hosts` re-browse prompt on a change. **Four drivers qualify: FOCAS, TwinCAT, MTConnect, AbCip.** Interval is `DriverInstanceActor.DefaultDriftCheckInterval` = **5 minutes**, a constructor parameter with no appsettings key — so a deployment cannot currently turn it off or slow it down without a code change. That is the core objection: it is recurring, unconfigurable, unsolicited traffic to real PLCs, CNCs and MTConnect Agents, and it was never live-gated. ## What to remove - `DriverInstanceActor`: `DriftCheckTick`, `StartDriftCheck`, `StopDriftCheck`, `HandleDriftCheckAsync`, `QualifiesForDriftCheck`, `_driftCheckInterval`, `_lastDriftSignature`, `DefaultDriftCheckInterval`, `DriftBrowseTimeout`, `CollectLeafRefs`, the `driftCheckInterval` params on `Props`/ctor, the `ReceiveAsync<DriftCheckTick>` registration and the `StartDriftCheck()`/`StopDriftCheck()` call sites. - `src/Server/ZB.MOM.WW.OtOpcUa.Runtime/Drivers/TagSetDrift.cs` (whole file). - Tests: `TagSetDriftDetectorTests`, `DriverInstanceActorDriftCheckTests`. - Driver members: `AuthoredDiscoveryRefs` + `DiscoveryStreamIncludesAuthoredTags` on `FocasDriver`, `TwinCATDriver`, `AbCipDriver`, `MTConnectDriver`. - `ITagDiscovery`: the `AuthoredDiscoveryRefs` and `DiscoveryStreamIncludesAuthoredTags` members. **Check for external implementors first** — these are default-implemented interface members, so removing them is source-compatible for anyone who did not override them, but a driver in a sibling repo that did override would break. - Docs: the drift paragraphs in `CLAUDE.md` §Change Detection and the 2026-07-28 entry in `deferment.md` §9. **Do NOT remove** the `IRediscoverable` consumption itself (`09a401b8`). That is a separate, event-driven signal — the driver tells us, we do not poll — and it costs nothing when nothing changes. ## Consequence to record, not to argue **AbCip and FOCAS browse a live backend but implement no `IRediscoverable`**, so after this removal they have **no change signal at all**: a PLC program re-download that adds or removes tags will be invisible until someone re-browses under `/raw` by hand. TwinCAT (ADS symbol-version) and MTConnect (Agent `instanceId`) keep their native event-driven signals and lose only the redundant polling. If that turns out to matter for AbCip/FOCAS later, the event-driven shape is the one to reach for — a device-side change notification, or a drift check that runs **only** when something else already indicates a change (a reconnect, a deploy) rather than on a wall-clock timer. ## Alternative considered and NOT chosen Gating the interval behind an appsettings key (default off) would keep the capability available without the unsolicited traffic. The owner's decision is removal; recorded here only so the option is not re-discovered from scratch.

dohertj2 referenced this issue from a commit

2026-07-28 13:27:43 -04:00

docs: record the merge, correct §9's commit SHAs, and flag drift for removal (#523)

dohertj2 referenced this issue from a commit

2026-07-28 13:31:56 -04:00

docs(deferment): bring §1-§8 current — the register had drifted from its own tree

Sign in to join this conversation.