SV core: unify generic analysis and measurement ownership - #45
Conversation
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: fcc05de1f4
ℹ️ 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".
| else if (noAsdu > 1) | ||
| Add(findings, SampledValuesPublisherValidationSeverity.Info, "SV_NOFASDU_PACKING", $"nofASDU={noAsdu} ASDU(s) per Ethernet frame.", "The publisher will pack sequential samples into one SavPdu."); |
There was a problem hiding this comment.
For nofASDU=2..8, this validator now reports that sequential samples will be packed, but the inspected GUI publish loop in SvPublisherViewModel.cs still constructs exactly one ASDU per frame, while the CLI path in Program.cs calls PublishNextAsync, which explicitly throws for these profiles. Thus the GUI can transmit frames inconsistent with the SCL configuration and the CLI aborts at runtime; keep these profiles unsupported until both callers use the batch API.
AGENTS.md reference: AGENTS.md:L173-L177
Useful? React with 👍 / 👎.
| if (normalizedChannel.StartsWith("V", StringComparison.OrdinalIgnoreCase) || | ||
| normalizedChannel.Contains("TVTR", StringComparison.OrdinalIgnoreCase) || | ||
| normalizedChannel.Contains("VolSv", StringComparison.OrdinalIgnoreCase)) |
There was a problem hiding this comment.
When Kind is empty and a fully qualified current-channel reference starts with V because of its IED name—for example VT01/LD0/TCTR1.AmpSv.instMag.i—the prefix check returns Voltage before reaching the explicit TCTR/AmpSv evidence. With otherwise valid fixed-layout evidence, Resolve then labels the current as volts and applies the 0.01 voltage multiplier instead of the 0.001 current multiplier; prioritize the semantic SCL tokens over the arbitrary leading identifier.
AGENTS.md reference: AGENTS.md:L144-L147
Useful? React with 👍 / 👎.
| } | ||
| private static void Text(ICollection<SvEvidenceFieldChange> changes, string category, string field, string baseline, string candidate, SvEvidenceChangeSeverity severity, string message) { if (!string.Equals(baseline ?? string.Empty, candidate ?? string.Empty, StringComparison.Ordinal)) changes.Add(Change(category, field, severity, baseline, candidate, message)); } | ||
| private static void Issue(ICollection<SvEvidenceFieldChange> changes, string field, int baseline, int candidate, SvEvidenceChangeSeverity severity) { if (baseline != candidate) changes.Add(Change("Runtime", field, candidate > baseline ? severity : SvEvidenceChangeSeverity.Info, baseline.ToString(CultureInfo.InvariantCulture), candidate.ToString(CultureInfo.InvariantCulture), candidate > baseline ? $"{field} increased." : $"{field} decreased.")); } | ||
| private static void Rate(ICollection<SvEvidenceFieldChange> changes, string field, double? baseline, double? candidate) { if (!baseline.HasValue || !candidate.HasValue) return; var tolerance = Math.Abs(baseline.Value) * RateTolerancePercent / 100.0; if (Math.Abs(candidate.Value - baseline.Value) > tolerance) changes.Add(Change("Rate", field, SvEvidenceChangeSeverity.Warning, baseline.Value.ToString("0.###", CultureInfo.InvariantCulture), candidate.Value.ToString("0.###", CultureInfo.InvariantCulture), "Observed rate moved outside the comparison tolerance.")); } |
There was a problem hiding this comment.
If the baseline has an observed sample rate but the candidate cannot calculate one—for example because its observation window contains only one timestamped frame—this early return records no change. A comparison with otherwise identical evidence is consequently marked unchanged and may report NO REGRESSION DETECTED even though timing evidence disappeared; distinguish baseline-missing, candidate-missing, and both-missing cases, treating candidate loss as at least a warning.
AGENTS.md reference: AGENTS.md:L173-L178
Useful? React with 👍 / 👎.
1 participant
Purpose
Make
ARIEC61850the single source of truth for reusable IEC 61850 Sampled Values protocol, measurement, evidence, Publisher-support, and Subscriber-analysis behavior that had diverged inside ARSVIN's embedded engine.Migrated reusable core
seqOfDatainspectionsmpCntcontinuity, wrap, restart, duplicate, gap, and out-of-order analysisGeneric architecture rule
Manufacturer or product identity never selects parser layout, ordered dataset mapping, scaling, quality interpretation, nominal frequency, or stream-health result. Runtime semantics come from protocol evidence, SCL, an explicit standard layout, reviewed user context, or known-injection evidence.
Validation
Head
fcc05de1f4b11560195e2589aa032e1d4c4b95f7passed.NET CIrun 235:No unresolved review thread remains.
Merge boundary
The reusable engine is ready to merge based on deterministic automated validation and its guarded/evidence-labelled behavior. The following remain post-merge field acceptance tests before calibrated, interoperability, or conformance claims are made:
nofASDU=1and multi-ASDU replayThis merge does not claim formal IEC 61850 conformance or calibrated measurement validation.