Skip to content

fix(sv): remove periodic waveform phase splice at 16-bit mirror smpCnt rollover - #28

Merged
masarray merged 2 commits into
mainfrom
fix/sv-mirror-16bit-rollover-scope-20261016
Oct 16, 2026
Merged

masarray merged 2 commits into
mainfrom
fix/sv-mirror-16bit-rollover-scope-20261016

Conversation

@masarray

Copy link
Copy Markdown
Owner

Field incident

Operator reports intermittent Process Bus Insight waveform jump/flicker while capturing ESP32-P4 4I+4V SV. Independent analysis of a sanitized 10.077 s, 80,648-frame PCAP (not committed): canonical 40,324 SV frames and diagnostic mirror 40,323, zero smpCnt jumps and duplicates in both, no PTP frames while Studio reports PTP STOPPED. The canonical stream wraps every 4000 samples; diagnostic mirror is a separate untagged broadcast stream with 16-bit smpCnt. Both streams carry byte-identical eight instantaneous channel values for all 40,323 sample pairs. The PCAP contains no sample-value discontinuity; Npcap host arrival jitter is not proof of publisher send jitter.

Deterministic view bug

In SvStreamState.AddScopeFrame, the analyzer correctly builds a monotonic unwrapped ScopeSampleFrame.Index to survive 4000 / 65536 rollover. However BuildCoherentScopeWindow preferred SampleCounter % (80 spc * 2 cycles = 160) rather than that unwrapped index. Since 65536 % 160 = 96, when the mirror wraps 65535 → 0, its newly received slot 0 belongs to a different phase than the retained prior samples. This visibly splices the waveform despite consecutive wire samples. The bug is masked for the canonical 4000 wrap because 4000 is a multiple of 160.

Fix

  • Use the already owned unwrapped sample index for the coherent scope slots. No new caches, no copy or allocation added to 4 kHz receive hot path, and no effect on raw SV decoder, standards fields, SCL mapping, per-stream ownership, power scaling, PTP or publisher firmware.
  • Regression uses generated 4I4V 50 Hz data; captures 320 samples just before 65535→0 and one after. For a stable repeated sine, 160-slot waveform must be identical immediately before/after and sequence/missing counters stay 0. Demonstrated discriminating legacy implementation yields up to 7608-count artificial seam, corrected monotonic slotting yields 0 for exact same data.

Other findings (outside scope)

ARStack Studio PTP reported STOPPED / UNLOCKED and smpSynch=0, corroborated by zero 0x88F7 in capture. SMV sample GPTimer is independent of PTP clock discipline; separate milestone arstack61850 #154 covers phase-locked PTP→smpCnt, and #162 next scope. Do not claim physical clock lock from packet continuity. Await green CI before merge.

@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Oct 16, 2026 •

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review ✅ Completed 2026-10-16T06:17:58.913841Z c7b993e PR opened
ℹ️ 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" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: c7b993e7e3

ℹ️ 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".

Comment thread src/ProcessBus.Iec61850.Raw/Analysis/RawProcessBusAnalyzer.cs
@masarray
masarray merged commit 8f88482 into main Oct 16, 2026
7 checks passed
@masarray
masarray deleted the fix/sv-mirror-16bit-rollover-scope-20261016 branch October 16, 2026 06:26

Copy link
Copy Markdown
Owner Author

Merged handoff — independent physical PCAP diagnosis, rollovers and final Windows candidate

  • Merged PR fix(sv): remove periodic waveform phase splice at 16-bit mirror smpCnt rollover #28 into main at 8f88482a356fdaabf74ed519c7bd02c95108fb43, tree aa524bf876bec87289d596ea591793f99e8354a2 exactly equals reviewed CI head c08c6da75170e926beb05f1195b73a629d26a611. 7/7 checks PASS incl CodeQL, Windows candidate release, runtime multi-stream, dependency review. 61/61 Windows TRX tests PASS; both review findings resolved.
  • Final Windows candidate GitHub Actions run #38860719197, artifact id 12028898615 (ProcessBusInsight-v1.4.0-beta.2-win-x64-candidate-64). ZIP portable SHA256 9e6958eda4e0ea09688a6786c14f1d35201827b2a6854904401cf547ebdc8bff matches SHA256SUMS.txt; manifest commit c08c6da... and ProcessBusInsight.exe present.
  • User-held 10.077 s classic PCAP: 80,648 total frames, 40,324 canonical tagged/multicast 0x4000 + 40,323 diagnostic untagged/broadcast 0x4F01, zero smpCnt jumps and duplicates on either; 40,323 paired 8-channel payloads exactly identical; all smpSynch=0 and quality word zero; 0 L2 PTP 0x88F7/UDP 319/320. Host Npcap arrival can show max ~15 ms batching even with contiguous smpCnt, thus no physical TX jitter proof. Never commit operator PCAP/identifiers.
  • Root cause fixed: BuildCoherentScopeWindow wrongly selected slots via raw 16-bit smpCnt % 160 instead of already-unwrapped sample index; 65536 % 160 = 96, creating a phase splice during the diagnostic mirror's wrap every ~16.384s. Synthetic discriminating example old algorithm spliced by ~7608 raw counts, new algorithm remains constant across wrap.
  • Review found second issue: ResolveSequenceWrapBase guessed maxObservedCounter+1 as rate modulus and could conceal packet loss right before counter wrap. Fixed to accept only observed supported boundaries without s.m.pRate (3999,4799,65535), else explicitly reject/hold last complete window. Adversarial deterministic regression 3998→0 with missing3999, and normal 65535→0. No second ring/parser/cache; no per-packet overhead.
  • Separate PTP milestone under ARStack #154/#162: operator diagnostics PTP STOPPED/UNLOCKED, hence no 0x88F7 expected, and released firmware free-runs GPTimer for SV, not continuously EMAC-PTP disciplined. PTP two-step SOURCE code not independently verified on wire, no claims of IEC 61850-9-3 or smpCnt↔PTP phase lock. PR #166 M3 Draft remains isolated.
  • Field acceptance remaining: Windows candidate 30+s capture of diagnostic mirror crossing its 16-bit wrap to confirm no flicker; separately START PTP SOURCE in Studio, check Announce/Sync/FollowUp & PTP 0x88F7 on receiving NIC, no external IED/switch required for basic visibility; full timing accuracy requires independent GM/PPS and hardware timestamp evidence.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant