Filter transient CSWI Close before the Value Viewer - #3
Conversation
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 64aa083bc8
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| return; | ||
| } | ||
|
|
||
| var key = NormalizeReference(reference); |
There was a problem hiding this comment.
When two monitored IEDs expose the same IEC reference, such as CSWI1.Pos.stVal, this key collapses their PointUpdated streams. An Open/non-Closed update from device B can cancel device A's pending Closed, or a Closed from B can provide the stable evidence used by A's command, because the rest of the UI keys live points with DeviceId but this filter drops it. Include snapshot.Point.DeviceId in the pending/stable key so devices cannot interfere with each other.
Useful? React with 👍 / 👎.
| Signal = signal, | ||
| BeforeValue = NormalizeControlState(signal.ControlCurrentValue), | ||
| RequestedValue = NormalizeControlState(requestedValue) | ||
| BeforeValue = NormalizeControlState(signal.ControlCurrentValue) |
There was a problem hiding this comment.
For a Close issued while the row already shows Closed, this still registers an active wait state and then clears prior stable evidence. Because the process value may not change, no fresh PointUpdated/ControlCurrentValue event is guaranteed; when the successful Feedback confirmed: Closed result arrives, the handler rewrites it to waiting and eventually times out. Keep the previous exception for BeforeValue == Closed so idempotent Close commands do not get reported as unconfirmed.
Useful? React with 👍 / 👎.
| private static bool IsControlFailureResult(string result) | ||
| => result.Contains("failed", StringComparison.OrdinalIgnoreCase) || | ||
| result.Contains("rejected", StringComparison.OrdinalIgnoreCase) || | ||
| result.Contains("cancelled", StringComparison.OrdinalIgnoreCase) || | ||
| result.Contains("unsupported", StringComparison.OrdinalIgnoreCase); |
There was a problem hiding this comment.
If the control service returns a terminal non-success such as Feedback timeout: or Validation:, this predicate does not match it, so the active Close state remains installed. A few seconds later the 15-second expiry overwrites the real failure/timeout message with Command accepted... stable Closed ... not confirmed, which hides the actual result from the operator. Classify timeout/validation non-success results as terminal here, or otherwise remove the active state when the command result is not successful.
Useful? React with 👍 / 👎.
Regression confirmed
The previous workaround reacted after the control-result diagnostic. At that point the transient
Closedsample had already entered the normal WPF point-update queue, and the command-row model could publish a deferred optimisticClosedagain whenControlIsBusyended. That explains why the latest build still flashedClosedand could hold it longer than before.Corrected approach
PointUpdatedbefore snapshots enter the 100 ms Value Viewer batch.CSWI*.Pos.stVal = Closedfor 350 ms.Command accepted — waiting for stable Closed process feedback….Safety
Expected OLSF501 behavior