Restore all mandatory IED DataSet signals in selection wizard - #172
Conversation
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 65510f3b32
ℹ️ 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".
| { | ||
| protected override void OnInitialized(EventArgs e) | ||
| { | ||
| var merge = Iec61850DataSetSignalInventoryService.EnsureMandatorySignals(_device); |
There was a problem hiding this comment.
Running recovery only from OnInitialized is too late for the flows that need it: ConnectAndConfigureDeviceAsync opens the wizard only when device.SignalCount > 0, and OpenSignalSelectionWizardAsync also returns early for an empty inventory, so an IED whose mandatory DataSet signals are all removed by normal curation can never reach this call. The Connect All path additionally performs discovery with openWizard: false, so it never recovers these signals or restores their saved selections. Merge the catalog into the inventory immediately after discovery and before selection restoration and zero-count checks.
Useful? React with 👍 / 👎.
| if (membership is not null && | ||
| !string.Equals(signal.DataSetReference, membership.DataSetReference, StringComparison.OrdinalIgnoreCase)) | ||
| { | ||
| signal.DataSetReference = membership.DataSetReference; | ||
| changed = true; |
There was a problem hiding this comment.
When a signal belongs to multiple DataSets, this unconditionally replaces an existing DataSetReference with the alphabetically first membership, while ApplyEngineDataSetAuthority preserves any nonempty ReportControlReference. A previously valid DataSet/RCB pair can therefore become an RCB for one DataSet paired with another DataSet, and the report planner will build a static plan from that mismatched pair. Select a membership associated with the chosen RCB, or retain an existing authoritative pair when both references are already populated.
Useful? React with 👍 / 👎.
Problem
Valid IEC 61850 signals that are members of an IED static DataSet can disappear before the Signal Selection Wizard is shown. The user therefore cannot find them on first connect or later through Edit Signal, before selection state is even involved.
Root cause
ARSAS applies application-level SCADA/readability curation while building the normal discovered signal workspace. That curation is useful for reducing engineering noise, but it must not override ARIEC's authoritative DataSet membership and primary-value classification.
Final fix
device.LiveDiscoveryModel;GetMandatoryPrimarySignals()only — ARSAS does not infer which DataSet members are primary;IsSelected = falseunless the normal user-selection workflow selects them;Architecture hardening from final audit
$ -> .reference canonicalization from the recovery service;DesignReference,ObservedReference, primary/canonical/effective/observed MMS references);Not probed, notready/readable, so inventory presence is not confused with MMS read success;Regression coverage
Not probeduntil actual acquisition reads it.Exact-head validation
Head:
65510f3b328d04db42ccdf25528f23cbb2889c8fMerge decision
Final software audit found no remaining merge blocker. Physical IED verification remains valuable as post-merge field validation for the originally affected relay, but it is no longer a software merge gate per the requested integration decision.