diff --git a/CONTROL_MODEL_COMMAND_UX_AUDIT.md b/CONTROL_MODEL_COMMAND_UX_AUDIT.md new file mode 100644 index 000000000..d5ba746d4 --- /dev/null +++ b/CONTROL_MODEL_COMMAND_UX_AUDIT.md @@ -0,0 +1,50 @@ +# IEC 61850 ctlModel and command UX audit + +## Reported symptoms + +1. A Data Object with `ctlModel=StatusOnly` was still rendered with Open/Close buttons. +2. Inspecting that object produced a red `InvalidOperationException` even though `StatusOnly` is a valid IEC 61850 control-model value. +3. On an SBO object, the command value could briefly show **Closed** and then return to **Open** in the row, while a second attempt appeared to work. + +## Root cause + +### Status-only object + +The ARIEC61850 Smart Control service reads the live `ctlModel` correctly. It intentionally refuses to create an executable control session for `StatusOnly` and reports `ctlModel=StatusOnly` in the exception. + +ArIED previously treated that exception as a generic inspection failure. The command-row layout was selected only from the semantic CDC/object name, so a `CSWI/XCBR/XSWI.Pos` row still received Open/Close controls even after the IED had explicitly declared the object read-only. + +### SBO value appears to revert + +The command verifier and the live report/poll monitor are independent producers of `ControlCurrentValue`: + +- the control sequence publishes its final feedback observation; +- the normal monitor continues to publish report or cyclic samples every UI batch. + +A sample requested before process movement can arrive after a newer command sample and overwrite the badge. This is a presentation race; the audited SBO/SBOw engine still executes one immutable Select/Operate sequence and this patch does **not** add an automatic retry. + +## Changes + +- Parse all five live `ctlModel` states from the native model text or inspection evidence: + - Status only + - Direct Operate, normal security + - Select Before Operate, normal security + - Direct Operate, enhanced security + - Select Before Operate, enhanced security +- Treat `StatusOnly` and unresolved `Unknown` as read-only states. +- Derive row action visibility from the resolved `ctlModel`, not only the CDC/object name. +- Replace the status-only inspection error with a normal informational diagnostic. +- Coalesce command-row feedback while `ControlIsBusy`; publish the latest observation when the operation completes so a stale report/poll sample cannot create a false Close→Open flicker. +- Show the active SBO sequence in the row result (`SBO Select → Operate` or `SBOw → Operate`). +- Preserve the safety model: no command retry, no bypass of Live control armed, and no change to the native IEC 61850 command sequence. + +## Required live validation + +- `ctlModel=0`: model column says **Status only** and no Open/Close/On/Off/Set command is shown. +- `ctlModel=1`: **Direct Operate (DO) • Normal security** and the semantic command buttons remain available. +- `ctlModel=2`: **Select Before Operate (SBO) • Normal security** and one click performs Select→Operate. +- `ctlModel=3`: **Direct Operate (DO) • Enhanced security** and the result reflects CommandTermination. +- `ctlModel=4`: **Select Before Operate (SBO) • Enhanced security** and one click performs SBOw→Operate→CommandTermination. +- During one SBO command, the value badge does not briefly publish an older report/poll sample. + +If the physical IED really changes Closed→Open after this UI stabilization, capture the MMS Oper response, LastApplError/CommandTermination, status report, and event log. That would prove an IED/interlocking/process-state behavior rather than the presentation race fixed here. diff --git a/MainWindow.ControlDiagnostics.cs b/MainWindow.ControlDiagnostics.cs new file mode 100644 index 000000000..087d9dd67 --- /dev/null +++ b/MainWindow.ControlDiagnostics.cs @@ -0,0 +1,78 @@ +using ArIED61850Tester.Models; + +namespace ArIED61850Tester; + +public partial class MainWindow +{ + private bool _controlDiagnosticNormalizerInstalled; + + protected override void OnContentRendered(EventArgs e) + { + base.OnContentRendered(e); + if (_controlDiagnosticNormalizerInstalled) + return; + + _controlDiagnosticNormalizerInstalled = true; + + // The native Smart Control service deliberately rejects ctlModel=StatusOnly + // when asked to open an executable command session. For the explorer this is + // valid live-model information, not a communication failure. Replace the + // original diagnostic subscriber after the window is ready so status-only + // objects do not raise a red application error. + _runtime.Diagnostic -= Runtime_Diagnostic; + _runtime.Diagnostic += Runtime_DiagnosticWithControlModelClassification; + } + + private void Runtime_DiagnosticWithControlModelClassification(DiagnosticEntry entry) + { + if (IsStatusOnlyControlInspection(entry.Message)) + { + Runtime_Diagnostic(new DiagnosticEntry + { + Time = entry.Time, + Level = "INFO", + Source = entry.Source, + Message = $"{ExtractControlReference(entry.Message)}: ctlModel=StatusOnly; this is a read-only status object and command actions are disabled." + }); + return; + } + + if (IsUnknownControlModelInspection(entry.Message)) + { + Runtime_Diagnostic(new DiagnosticEntry + { + Time = entry.Time, + Level = "WARN", + Source = entry.Source, + Message = $"{ExtractControlReference(entry.Message)}: ctlModel could not be resolved; command actions remain disabled until the live model is known." + }); + return; + } + + Runtime_Diagnostic(entry); + } + + private static bool IsStatusOnlyControlInspection(string? message) + => !string.IsNullOrWhiteSpace(message) && + message.Contains("Control inspection failed", StringComparison.OrdinalIgnoreCase) && + (message.Contains("ctlModel=StatusOnly", StringComparison.OrdinalIgnoreCase) || + message.Contains("ctlModel=Status only", StringComparison.OrdinalIgnoreCase)); + + private static bool IsUnknownControlModelInspection(string? message) + => !string.IsNullOrWhiteSpace(message) && + message.Contains("Control inspection failed", StringComparison.OrdinalIgnoreCase) && + message.Contains("ctlModel=Unknown", StringComparison.OrdinalIgnoreCase); + + private static string ExtractControlReference(string message) + { + const string marker = "Control inspection failed for "; + var start = message.IndexOf(marker, StringComparison.OrdinalIgnoreCase); + if (start < 0) + return "IEC 61850 control object"; + + start += marker.Length; + var end = message.IndexOf(':', start); + var reference = end > start ? message[start..end] : message[start..]; + return string.IsNullOrWhiteSpace(reference) ? "IEC 61850 control object" : reference.Trim(); + } +} diff --git a/Models/SignalDefinition.cs b/Models/SignalDefinition.cs index 4f2b370c7..7e5864218 100644 --- a/Models/SignalDefinition.cs +++ b/Models/SignalDefinition.cs @@ -17,8 +17,11 @@ public class SignalDefinition : ObservableObject private string _controlModelReference = string.Empty; private string _controlStatusReference = string.Empty; private string _controlModelText = "Auto-detect"; + private Iec61850ControlModelKind _controlModelKind = Iec61850ControlModelKind.Unknown; + private bool _controlModelResolved; private string _controlValueType = string.Empty; private string _controlCurrentValue = "-"; + private string? _deferredControlCurrentValue; private string _controlSetPointText = string.Empty; private string _controlLastResult = string.Empty; private bool _controlIsBusy; @@ -80,30 +83,57 @@ public string ControlCdc set { if (!Set(ref _controlCdc, value?.Trim() ?? string.Empty)) return; - Raise(nameof(ControlActionLabel)); - Raise(nameof(IsPositionControl)); - Raise(nameof(IsRaiseOnlyControl)); - Raise(nameof(IsLowerOnlyControl)); - Raise(nameof(IsRaiseLowerControl)); - Raise(nameof(IsBooleanControl)); - Raise(nameof(IsSetPointControl)); - Raise(nameof(IsGenericControl)); + RaiseControlActionProperties(); } } public string ControlModelReference { get => _controlModelReference; set => Set(ref _controlModelReference, value?.Trim() ?? string.Empty); } public string ControlStatusReference { get => _controlStatusReference; set => Set(ref _controlStatusReference, value?.Trim() ?? string.Empty); } - public string ControlModelText { get => _controlModelText; set => Set(ref _controlModelText, string.IsNullOrWhiteSpace(value) ? "Auto-detect" : value.Trim()); } + public string ControlModelText + { + get => _controlModelText; + set + { + var normalized = string.IsNullOrWhiteSpace(value) ? "Auto-detect" : value.Trim(); + var changed = Set(ref _controlModelText, normalized); + + if (normalized.Contains("auto-detect", StringComparison.OrdinalIgnoreCase)) + ApplyControlModel(Iec61850ControlModelKind.Unknown, resolved: false, updateDisplay: false); + else + UpdateControlModelFromEvidence(normalized, updateDisplay: false); + + if (changed) + Raise(nameof(SignalPropertiesSummary)); + } + } + public Iec61850ControlModelKind ControlModelKind => _controlModelKind; + public bool ControlModelResolved => _controlModelResolved; + public bool ControlSupportsOperate => _controlModelResolved && _controlModelKind is + Iec61850ControlModelKind.DirectNormal or + Iec61850ControlModelKind.SboNormal or + Iec61850ControlModelKind.DirectEnhanced or + Iec61850ControlModelKind.SboEnhanced; + public bool IsReadOnlyControl => _controlModelResolved && !ControlSupportsOperate; public string ControlValueType { get => _controlValueType; set => Set(ref _controlValueType, value?.Trim() ?? string.Empty); } - /// Current process feedback shown in the fast Command Panel. This is runtime-only and is never persisted as live truth. + /// + /// Current process feedback shown in the fast Command Panel. While a command is in + /// progress, report/poll updates are coalesced and the final command observation is + /// published atomically. This prevents a stale pre-operate sample from making an SBO + /// command look as though it changed and immediately reverted. + /// public string ControlCurrentValue { get => _controlCurrentValue; set { var normalized = NormalizeControlDisplayValue(value); - if (Set(ref _controlCurrentValue, normalized)) - Raise(nameof(ControlCurrentTone)); + if (ControlIsBusy) + { + _deferredControlCurrentValue = normalized; + return; + } + + ApplyControlCurrentValue(normalized); } } @@ -120,10 +150,43 @@ public string ControlCurrentTone } public string ControlSetPointText { get => _controlSetPointText; set => Set(ref _controlSetPointText, value?.Trim() ?? string.Empty); } - public string ControlLastResult { get => _controlLastResult; set => Set(ref _controlLastResult, value?.Trim() ?? string.Empty); } - public bool ControlIsBusy { get => _controlIsBusy; set => Set(ref _controlIsBusy, value); } + public string ControlLastResult + { + get => _controlLastResult; + set + { + var normalized = value?.Trim() ?? string.Empty; + UpdateControlModelFromEvidence(normalized, updateDisplay: true); + normalized = NormalizeControlResultText(normalized); + Set(ref _controlLastResult, normalized); + } + } + public bool ControlIsBusy + { + get => _controlIsBusy; + set + { + if (!Set(ref _controlIsBusy, value)) + return; - public bool IsPositionControl + if (value) + { + _deferredControlCurrentValue = null; + return; + } + + if (_deferredControlCurrentValue != null) + { + var deferred = _deferredControlCurrentValue; + _deferredControlCurrentValue = null; + ApplyControlCurrentValue(deferred); + } + } + } + + private bool CanExposeControlActions => !_controlModelResolved || ControlSupportsOperate; + + private bool IsPositionSemanticControl { get { @@ -137,35 +200,38 @@ public bool IsPositionControl } } - public bool IsRaiseOnlyControl => ContainsControlToken("TapOpR") || ContainsControlToken("Raise"); - public bool IsLowerOnlyControl => ContainsControlToken("TapOpL") || ContainsControlToken("Lower"); + public bool IsPositionControl => CanExposeControlActions && IsPositionSemanticControl; + public bool IsRaiseOnlyControl => CanExposeControlActions && (ContainsControlToken("TapOpR") || ContainsControlToken("Raise")); + public bool IsLowerOnlyControl => CanExposeControlActions && (ContainsControlToken("TapOpL") || ContainsControlToken("Lower")); public bool IsRaiseLowerControl { get { - if (IsPositionControl || IsRaiseOnlyControl || IsLowerOnlyControl) return false; + if (!CanExposeControlActions || IsPositionControl || IsRaiseOnlyControl || IsLowerOnlyControl) return false; var cdc = (ControlCdc ?? string.Empty).Trim().ToUpperInvariant(); return cdc is "INC" or "ISC" or "INC/ISC"; } } - public bool IsBooleanControl => !IsPositionControl && !IsRaiseOnlyControl && !IsLowerOnlyControl && !IsRaiseLowerControl && + public bool IsBooleanControl => CanExposeControlActions && !IsPositionControl && !IsRaiseOnlyControl && !IsLowerOnlyControl && !IsRaiseLowerControl && (ControlCdc ?? string.Empty).Trim().Equals("SPC", StringComparison.OrdinalIgnoreCase); public bool IsSetPointControl { get { - if (IsPositionControl || IsRaiseOnlyControl || IsLowerOnlyControl || IsRaiseLowerControl || IsBooleanControl) return false; + if (!CanExposeControlActions || IsPositionControl || IsRaiseOnlyControl || IsLowerOnlyControl || IsRaiseLowerControl || IsBooleanControl) return false; var cdc = (ControlCdc ?? string.Empty).Trim().ToUpperInvariant(); return cdc is "APC" or "BAC" or "BSC"; } } - public bool IsGenericControl => !IsPositionControl && !IsRaiseOnlyControl && !IsLowerOnlyControl && - !IsRaiseLowerControl && !IsBooleanControl && !IsSetPointControl; + public bool IsGenericControl => !CanExposeControlActions || + (!IsPositionControl && !IsRaiseOnlyControl && !IsLowerOnlyControl && + !IsRaiseLowerControl && !IsBooleanControl && !IsSetPointControl); public string ControlActionLabel { get { + if (IsReadOnlyControl) return "Read only"; if (IsPositionControl) return "Open / Close"; if (IsRaiseOnlyControl) return "Raise"; if (IsLowerOnlyControl) return "Lower"; @@ -189,13 +255,150 @@ public string ControlActionLabel } } + private void ApplyControlCurrentValue(string normalized) + { + if (Set(ref _controlCurrentValue, normalized, nameof(ControlCurrentValue))) + Raise(nameof(ControlCurrentTone)); + } + + private void UpdateControlModelFromEvidence(string? evidence, bool updateDisplay) + { + if (!TryParseControlModel(evidence, out var model)) + return; + + ApplyControlModel(model, resolved: true, updateDisplay); + } + + private void ApplyControlModel(Iec61850ControlModelKind model, bool resolved, bool updateDisplay) + { + var modelChanged = _controlModelKind != model; + var resolvedChanged = _controlModelResolved != resolved; + _controlModelKind = model; + _controlModelResolved = resolved; + + if (updateDisplay && resolved) + { + var friendly = FriendlyControlModel(model); + if (!string.Equals(_controlModelText, friendly, StringComparison.Ordinal)) + { + _controlModelText = friendly; + Raise(nameof(ControlModelText)); + } + } + + if (!modelChanged && !resolvedChanged) + return; + + Raise(nameof(ControlModelKind)); + Raise(nameof(ControlModelResolved)); + Raise(nameof(ControlSupportsOperate)); + Raise(nameof(IsReadOnlyControl)); + RaiseControlActionProperties(); + } + + private void RaiseControlActionProperties() + { + Raise(nameof(ControlActionLabel)); + Raise(nameof(IsPositionControl)); + Raise(nameof(IsRaiseOnlyControl)); + Raise(nameof(IsLowerOnlyControl)); + Raise(nameof(IsRaiseLowerControl)); + Raise(nameof(IsBooleanControl)); + Raise(nameof(IsSetPointControl)); + Raise(nameof(IsGenericControl)); + Raise(nameof(SignalPropertiesSummary)); + } + + private string NormalizeControlResultText(string text) + { + if (IsReadOnlyControl && ControlModelKind == Iec61850ControlModelKind.StatusOnly) + return "Status only — read-only object; commands disabled by the IED ctlModel."; + if (IsReadOnlyControl && ControlModelKind == Iec61850ControlModelKind.Unknown) + return "Unknown ctlModel — commands disabled until the live control model is resolved."; + + if (string.IsNullOrWhiteSpace(text)) + return string.Empty; + + var sequence = ControlModelKind switch + { + Iec61850ControlModelKind.SboEnhanced => "SBOw → Operate", + Iec61850ControlModelKind.SboNormal => "SBO Select → Operate", + _ => string.Empty + }; + if (string.IsNullOrWhiteSpace(sequence) || text.Contains("SBO", StringComparison.OrdinalIgnoreCase)) + return text; + + return text.StartsWith("Sending", StringComparison.OrdinalIgnoreCase) || + text.StartsWith("Feedback", StringComparison.OrdinalIgnoreCase) || + text.StartsWith("Command", StringComparison.OrdinalIgnoreCase) || + text.StartsWith("Control", StringComparison.OrdinalIgnoreCase) + ? $"{sequence} • {text}" + : text; + } + + private static bool TryParseControlModel(string? text, out Iec61850ControlModelKind model) + { + model = Iec61850ControlModelKind.Unknown; + if (string.IsNullOrWhiteSpace(text) || text.Contains("auto-detect", StringComparison.OrdinalIgnoreCase)) + return false; + + var normalized = text.Trim().ToLowerInvariant(); + var numeric = Regex.Match(normalized, @"ctlmodel\s*[:=]\s*([0-4])", RegexOptions.IgnoreCase); + if (numeric.Success) + { + model = numeric.Groups[1].Value switch + { + "0" => Iec61850ControlModelKind.StatusOnly, + "1" => Iec61850ControlModelKind.DirectNormal, + "2" => Iec61850ControlModelKind.SboNormal, + "3" => Iec61850ControlModelKind.DirectEnhanced, + "4" => Iec61850ControlModelKind.SboEnhanced, + _ => Iec61850ControlModelKind.Unknown + }; + return true; + } + + if (normalized.Contains("statusonly") || normalized.Contains("status only")) + model = Iec61850ControlModelKind.StatusOnly; + else if (normalized.Contains("selectbeforeoperateenhanced") || + (normalized.Contains("sbo") && normalized.Contains("enhanced"))) + model = Iec61850ControlModelKind.SboEnhanced; + else if (normalized.Contains("selectbeforeoperatenormal") || + (normalized.Contains("sbo") && normalized.Contains("normal"))) + model = Iec61850ControlModelKind.SboNormal; + else if (normalized.Contains("directenhanced") || + (normalized.Contains("direct") && normalized.Contains("enhanced"))) + model = Iec61850ControlModelKind.DirectEnhanced; + else if (normalized.Contains("directnormal") || + (normalized.Contains("direct") && normalized.Contains("normal"))) + model = Iec61850ControlModelKind.DirectNormal; + else if (normalized == "unknown" || + (normalized.Contains("ctlmodel") && normalized.Contains("unknown"))) + model = Iec61850ControlModelKind.Unknown; + else + return false; + + return true; + } + + private static string FriendlyControlModel(Iec61850ControlModelKind model) + => model switch + { + Iec61850ControlModelKind.DirectNormal => "Direct Operate (DO) • Normal security", + Iec61850ControlModelKind.SboNormal => "Select Before Operate (SBO) • Normal security", + Iec61850ControlModelKind.DirectEnhanced => "Direct Operate (DO) • Enhanced security", + Iec61850ControlModelKind.SboEnhanced => "Select Before Operate (SBO) • Enhanced security", + Iec61850ControlModelKind.StatusOnly => "Status only", + _ => "Unknown" + }; + private bool ContainsControlToken(string token) => $"{Name} {ObjectReference}".Contains(token, StringComparison.OrdinalIgnoreCase); private string NormalizeControlDisplayValue(string? value) { var text = string.IsNullOrWhiteSpace(value) ? "-" : value.Trim(); - if (IsPositionControl && ArIED61850Tester.Services.Iec61850ValueFormatter.TryNormalizeDbpos(text, out var code)) + if (IsPositionSemanticControl && ArIED61850Tester.Services.Iec61850ValueFormatter.TryNormalizeDbpos(text, out var code)) { return code switch { @@ -209,8 +412,8 @@ private string NormalizeControlDisplayValue(string? value) if (bool.TryParse(text, out var boolean)) return boolean ? "True" : "False"; - if (text.Equals("ON", StringComparison.OrdinalIgnoreCase)) return IsPositionControl ? "Closed" : "True"; - if (text.Equals("OFF", StringComparison.OrdinalIgnoreCase)) return IsPositionControl ? "Open" : "False"; + if (text.Equals("ON", StringComparison.OrdinalIgnoreCase)) return IsPositionSemanticControl ? "Closed" : "True"; + if (text.Equals("OFF", StringComparison.OrdinalIgnoreCase)) return IsPositionSemanticControl ? "Open" : "False"; return text; }