diff --git a/MainWindow.xaml.cs b/MainWindow.xaml.cs index 6d5df29b8..50aa07f5b 100644 --- a/MainWindow.xaml.cs +++ b/MainWindow.xaml.cs @@ -37,6 +37,8 @@ public partial class MainWindow : Window, INotifyPropertyChanged private readonly Dictionary _reportPulseUntil = new(StringComparer.OrdinalIgnoreCase); private readonly Dictionary _pointHighlightUntil = new(StringComparer.OrdinalIgnoreCase); private readonly Dictionary> _controlFeedbackIndex = new(StringComparer.OrdinalIgnoreCase); + // UI-only scheduling ownership; never a second control/session authority. + private readonly HashSet _controlRefreshInProgress = new(); private readonly DispatcherTimer _uiFlushTimer; private readonly DispatcherTimer _progressAnimationTimer; // One timer per window: concurrent negative command results replace the @@ -1449,70 +1451,95 @@ private async Task RefreshControlValuesAsync(Iec61850MonitorDevice device, bool if (!device.IsConnected) return; - // Never gate inspection on an operable-only UI projection. - var candidates = Iec61850ControlIdentity.DistinctSelected(device.Signals) - .Where(signal => Iec61850ControlIdentity.NeedsInspection(signal, force)) - .ToArray(); - if (candidates.Length == 0) + // The native runtime serializes inspection and Operate on one + // _controlCommandGate. Enqueuing three parallel UI inspections cannot + // improve MMS throughput, but places extra reads ahead of user control. + // One refresh owner per device; do not create another inspection queue. + if (!_controlRefreshInProgress.Add(device)) { - RebuildControlFeedbackIndex(device); + if (force && ReferenceEquals(SelectedDevice, device)) + SetStatus($"{device.Name}: control inspection is already running; retry Refresh once it completes."); return; } - using var throttle = new SemaphoreSlim(3, 3); - await Task.WhenAll(candidates.Select(async signal => + try { - // Claim before waiting: another expansion/refresh must not enqueue - // the same target while this batch is waiting for its local slot. - signal.ControlInspectionBusy = true; - var entered = false; - try - { - await throttle.WaitAsync(_applicationCancellation.Token); - entered = true; - var capabilities = await _runtime.InspectControlAsync( - device.DeviceId, - signal, - _applicationCancellation.Token); - if (!signal.ControlCommandBusy) - { - signal.ControlCurrentValue = capabilities.CurrentValue; - signal.ControlInspectionEvidence = - $"{capabilities.ControlModelReference} | {capabilities.ControlModelText} | " + - $"{capabilities.EngineControlServiceStatus} | {capabilities.DiscoveryEvidence}"; - signal.ControlLastResult = capabilities.EngineControlServiceStatus; - if (!capabilities.SupportsOperate) - AddLog("INFO", device.Name, - $"Read-only control inspection {signal.ObjectReference}: " + - signal.ControlInspectionEvidence); - } - } - catch (OperationCanceledException) + var candidates = Iec61850ControlIdentity.DistinctSelected(device.Signals) + .Where(signal => Iec61850ControlIdentity.NeedsInspection(signal, force)) + .ToArray(); + if (candidates.Length == 0) { - // Application shutdown or device removal. + RebuildControlFeedbackIndex(device); + return; } - catch (Exception ex) + + var completed = 0; + foreach (var signal in candidates) { - if (!signal.ControlCommandBusy) + // Input/confirmation has priority over the next diagnostic + // read. Never place another inspection behind an active + // operator command on this association. + await System.Windows.Threading.Dispatcher.Yield(DispatcherPriority.Background); + if (!device.IsConnected || _applicationCancellation.IsCancellationRequested || + device.CommandSignals.Any(active => active.ControlCommandBusy)) + break; + if (!Iec61850ControlIdentity.NeedsInspection(signal, force)) + continue; + + signal.ControlInspectionBusy = true; + try + { + var capabilities = await _runtime.InspectControlAsync( + device.DeviceId, + signal, + _applicationCancellation.Token); + if (!signal.ControlCommandBusy) + { + signal.ControlCurrentValue = capabilities.CurrentValue; + signal.ControlInspectionEvidence = + $"{capabilities.ControlModelReference} | {capabilities.ControlModelText} | " + + $"{capabilities.EngineControlServiceStatus} | {capabilities.DiscoveryEvidence}"; + signal.ControlLastResult = capabilities.EngineControlServiceStatus; + if (!capabilities.SupportsOperate) + AddLog("INFO", device.Name, + $"Read-only control inspection {signal.ObjectReference}: " + + signal.ControlInspectionEvidence); + } + completed++; + } + catch (OperationCanceledException) + { + // Application shutdown or device removal. + break; + } + catch (Exception ex) { - signal.ControlInspectionEvidence = - $"{signal.ObjectReference}: inspection failed; {ex.GetType().Name}: {ex.Message}"; - signal.ControlLastResult = $"Status read failed: {ex.Message}"; + if (!signal.ControlCommandBusy) + { + signal.ControlInspectionEvidence = + $"{signal.ObjectReference}: inspection failed; {ex.GetType().Name}: {ex.Message}"; + signal.ControlLastResult = $"Status read failed: {ex.Message}"; + } + } + finally + { + signal.ControlInspectionBusy = false; } } - finally + + device.RefreshCommandSignalProjection(); + RebuildControlFeedbackIndex(device); + if (ReferenceEquals(SelectedDevice, device)) { - signal.ControlInspectionBusy = false; - if (entered) throttle.Release(); + ResizeCommandDockForVerifiedControls(); + SetStatus($"{device.Name}: completed {completed}/{candidates.Length} control inspection(s)."); } - })); - - device.RefreshCommandSignalProjection(); - RebuildControlFeedbackIndex(device); - if (ReferenceEquals(SelectedDevice, device)) - ResizeCommandDockForVerifiedControls(); - TryAutoExpandCommandPanelOnce(device); - SetStatus($"{device.Name}: refreshed {candidates.Length} control value(s)."); + TryAutoExpandCommandPanelOnce(device); + } + finally + { + _controlRefreshInProgress.Remove(device); + } } private bool RejectCommandWhenIedFlightUnavailable( diff --git a/docs/CONTROL_PRIORITY_INSPECTION_2026-10-20.md b/docs/CONTROL_PRIORITY_INSPECTION_2026-10-20.md new file mode 100644 index 000000000..bcfae2eb4 --- /dev/null +++ b/docs/CONTROL_PRIORITY_INSPECTION_2026-10-20.md @@ -0,0 +1,37 @@ +# P0-T — command-priority control inspection scheduling (trial, 2026-10-20) + +## Exact source and physical evidence + +Field diagnostic (2026-10-20 13:27:52 +07) used ARSAS \`99d39c77bb9d68a5c6d964c731d868196f23f993\` with engine \`ba21d882f67de22c10d3155b449aac77aa8b4899\`, one connected SIEBCU association. No site identifier, IP address, capture or raw diagnostic is stored in this repository. + +Three confirmed SBO enhanced commands: +- CSWI1 Open: UI click-to-result 7,964.429 ms, native control 7,808.130 ms, final feedback 13.675 ms. +- CSWI2 Close: UI click-to-result 8,099.409 ms, native control 7,973.275 ms, final feedback 8.436 ms. +- CSWI1 Close: UI click-to-result 13,323.694 ms, native control 13,224.153 ms, final feedback 8.731 ms. + +All three reported two MMS control service responses, positive CommandTermination, and matching final process feedback. The old diagnostic cannot distinguish SBOw MMS-response latency, Operate MMS-response latency, or post-acceptance CommandTermination delay. **Do not attribute 8–13 seconds to UI, relay, or wire RTT without the stage measurement introduced by engine draft PR #183.** + +## Source-proven concurrency issue and correction + +The existing WPF Command Dock refresh launched a \`Task.WhenAll\` over all selected control targets with a local three-request semaphore. The native client \`InspectControlAsync\` serializes *every* inspection and command through \`_controlCommandGate\`, so there is no native throughput benefit from the three-way UI queue. It places unrelated control inspections ahead of operator-initiated Operate. When the dock reopens repeatedly, it can also compete for the same control admission slot. + +This companion patch (stacked on consumer PR #635) makes read-only inspection deliberately sequential, keeps one UI refresh owner per device, rechecks \`NeedsInspection\` just before each request, and yields to WPF input priority before scheduling another MMS inspection. When any same-device command is active, no additional inspection begins. The one inspection already admitted may finish before a command; this change **does not preempt a wire request**. The native service still serializes the association; no priority queue, parallel MMS write, arbitrary delay, or new process-value cache is introduced. + +## Safety, synergies and limits + +- No changes to SBOw/Operate/Cancel, \`ctlNum\`, service timeout, interlock, synchro, Test, CommandTermination, process feedback verification, or uncertain-wire single-flight/quarantine. +- Explicit Refresh still requests forced inspection when no refresh is active. If a refresh is already active, the UI clearly states that the operator can retry on completion rather than silently duplicating MMS requests. +- This patch intentionally does **not** touch Browser, FC=SV/OR/BL, IED-card progress, reporting projection, or SCL leaf datasets. Consumer leaf PR #636 owns those files. +- Source-contract regression covers sequential admission, UI input yield, command busy exclusion, refresh lifetime, and retained native safety calls. +- Expected queue improvement: 0 *pre-enqueued* parallel WPF inspections instead of up to 2 waiting behind an active inspection. **Not a measured 8–13 s latency reduction.** +- Physical validation still required using one exact combined SHA. Capture engine PR #183 ordered \`serviceMs\` and \`postResponseWaitMs\` for SBOw/Operate, plus actual Ethernet capture if service time remains high. Windows WPF build, full regression/CI and relay FAT are mandatory prior to merging/releasing. + +## Test plan + +1. Run Build ARSAS Windows workflow and \`P109S\`/\`P109T\` regression tests on the exact head. +2. Connect the relay, expand/collapse Command Dock repeatedly and verify verified StatusOnly rows aren't read again. +3. With background read active, stage and confirm a different operable CSWI command. Observe no newly queued inspection after command claim. +4. Verify one MMS SBOw/Operate pair and positive termination; no second/automatic retry. After reconnect, verify fresh inspection and status. +5. Repeat exactly the previously slow three commands and compare stage timings, not only total click-to-result. + +Draft only; not field-qualified. diff --git a/tests/ARSAS.Tests/P109TControlInspectionPriorityRegressionTests.cs b/tests/ARSAS.Tests/P109TControlInspectionPriorityRegressionTests.cs new file mode 100644 index 000000000..03d3ddda0 --- /dev/null +++ b/tests/ARSAS.Tests/P109TControlInspectionPriorityRegressionTests.cs @@ -0,0 +1,56 @@ +using System; +using System.IO; + +namespace ARSAS.Tests; + +public sealed class P109TControlInspectionPriorityRegressionTests +{ + [Fact] + public void DockRefreshNeverQueuesParallelNativeInspectionsAheadOfACommand() + { + var root = new DirectoryInfo(AppContext.BaseDirectory); + while (root != null && !File.Exists(Path.Combine(root.FullName, "MainWindow.xaml.cs"))) + root = root.Parent; + Assert.NotNull(root); + var main = File.ReadAllText(Path.Combine(root!.FullName, "MainWindow.xaml.cs")); + + var start = main.IndexOf( + "private async Task RefreshControlValuesAsync(Iec61850MonitorDevice device, bool force = false)", + StringComparison.Ordinal); + var finish = main.IndexOf("private bool RejectCommandWhenIedFlightUnavailable(", start, StringComparison.Ordinal); + Assert.True(start >= 0 && finish > start); + + var refresh = main[start..finish]; + Assert.Contains("_controlRefreshInProgress.Add(device)", refresh); + Assert.Contains("_controlRefreshInProgress.Remove(device)", refresh); + Assert.Contains("foreach (var signal in candidates)", refresh); + Assert.Contains("await System.Windows.Threading.Dispatcher.Yield(DispatcherPriority.Background);", refresh); + Assert.Contains("device.CommandSignals.Any(active => active.ControlCommandBusy)", refresh); + Assert.Contains("Iec61850ControlIdentity.NeedsInspection(signal, force)", refresh); + Assert.Contains("signal.ControlInspectionBusy = true;", refresh); + Assert.Contains("signal.ControlInspectionBusy = false;", refresh); + Assert.DoesNotContain("Task.WhenAll(candidates.Select", refresh); + Assert.DoesNotContain("new SemaphoreSlim(3, 3)", refresh); + } + + [Fact] + public void ControlExecutionRetainsTheSameNativeSingleFlightAndSafetyChecks() + { + var root = new DirectoryInfo(AppContext.BaseDirectory); + while (root != null && !File.Exists(Path.Combine(root.FullName, "Services", "NativeIec61850Client.cs"))) + root = root.Parent; + Assert.NotNull(root); + var client = File.ReadAllText(Path.Combine(root!.FullName, "Services", "NativeIec61850Client.cs")); + var start = client.IndexOf("public async Task ExecuteControlAsync(", StringComparison.Ordinal); + var end = client.IndexOf("private static Iec61850ControlCapabilities BuildControlCapabilities(", start, StringComparison.Ordinal); + Assert.True(start >= 0); + var control = end > start ? client[start..end] : client[start..]; + + Assert.Contains("_controlCommandGate.WaitAsync(cancellationToken)", control); + Assert.Contains("InterlockCheck = request.InterlockCheck", control); + Assert.Contains("SynchroCheck = request.SynchroCheck", control); + Assert.Contains("AutoSelect = true", control); + Assert.Contains("control.OperateAsync(nativeRequest, cancellationToken)", control); + Assert.Contains("WaitForNativeControlFeedbackAsync(", control); + } +}