Driver config edits are silently discarded on Modbus/FOCAS/OpcUaClient (deployment still seals green) #516

Open
opened 2026-07-27 13:51:59 -04:00 by dohertj2 · 0 comments
Owner

Found while building the MTConnect driver (PR #506) and verified independently end-to-end against current master. Deliberately not fixed there — out of scope for that plan.

Symptom

An operator edits a Modbus / FOCAS / OpcUaClient driver's config and deploys. The driver keeps running the old config. ApplyResult(true) is returned and the deployment seals green.

Silent no-op — the worst shape, because it looks successful.

Cause

DriverInstanceActor assigns _driver once (DriverInstanceActor.cs:307) and never rebuilds it from the factory. Both config-change paths therefore hand the new JSON to that already-constructed instance:

  • HandleApplyDeltaAsync (L592) → _driver.ReinitializeAsync(msg.DriverConfigJson, …)
  • AdoptConfigDuringInit → the actor's InitializeAsync (L554) → _driver.InitializeAsync(driverConfigJson, …)

So driverConfigJson is the only channel by which a config change can reach a running driver.

But these three never read that parameter — they use the ctor-injected _options exclusively:

  • ModbusDriver.InitializeAsync (L195)
  • FocasDriver.InitializeAsync (L104)
  • OpcUaClientDriver.InitializeAsync (L160)

Their ReinitializeAsync delegates to InitializeAsync, so the new config is dropped on the floor.

Prior art in-tree

S7 does it correctly and documents why — see the comment at S7Driver.cs:283. MTConnect follows S7, and this was confirmed live on the PR #506 gate: a corrected tag config took effect on redeploy with no container restart.

Still to check

The per-driver survey was inconclusive for AbCip, TwinCAT and Galaxy — verify each one's InitializeAsync actually re-parses before closing this.

Suggested fix

Make InitializeAsync re-parse driverConfigJson following S7's pattern (an empty/blank body keeps the ctor options). Add a per-driver regression test: reinitialise with a changed config and assert the change is observable, not merely that the call returned true.

Found while building the MTConnect driver (PR #506) and verified independently end-to-end against current `master`. Deliberately **not** fixed there — out of scope for that plan. ## Symptom An operator edits a Modbus / FOCAS / OpcUaClient driver's config and deploys. The driver keeps running the **old** config. `ApplyResult(true)` is returned and the deployment seals **green**. Silent no-op — the worst shape, because it looks successful. ## Cause `DriverInstanceActor` assigns `_driver` **once** (`DriverInstanceActor.cs:307`) and never rebuilds it from the factory. Both config-change paths therefore hand the new JSON to that already-constructed instance: - `HandleApplyDeltaAsync` (`L592`) → `_driver.ReinitializeAsync(msg.DriverConfigJson, …)` - `AdoptConfigDuringInit` → the actor's `InitializeAsync` (`L554`) → `_driver.InitializeAsync(driverConfigJson, …)` So `driverConfigJson` is the **only** channel by which a config change can reach a running driver. But these three never read that parameter — they use the ctor-injected `_options` exclusively: - `ModbusDriver.InitializeAsync` (`L195`) - `FocasDriver.InitializeAsync` (`L104`) - `OpcUaClientDriver.InitializeAsync` (`L160`) Their `ReinitializeAsync` delegates to `InitializeAsync`, so the new config is dropped on the floor. ## Prior art in-tree **S7 does it correctly and documents why** — see the comment at `S7Driver.cs:283`. MTConnect follows S7, and this was confirmed live on the PR #506 gate: a corrected tag config took effect on redeploy with no container restart. ## Still to check The per-driver survey was **inconclusive for AbCip, TwinCAT and Galaxy** — verify each one's `InitializeAsync` actually re-parses before closing this. ## Suggested fix Make `InitializeAsync` re-parse `driverConfigJson` following S7's pattern (an empty/blank body keeps the ctor options). Add a per-driver regression test: reinitialise with a changed config and assert the change is **observable**, not merely that the call returned true.
Sign in to join this conversation.
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: dohertj2/lmxopcua#516