Repository navigation
fix(bench): millisecond PTP epoch seeding and resilient SV session lease with diagnostic evidence - #283
Conversation
…ad of truncated seconds
…ease stop and record heartbeat delays
…atched SVLIFE readback
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
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[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: 3c99e06da2
ℹ️ 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".
| // A 2.5-second lease caused unnecessary SV STOP during transient Windows USB | ||
| // stalls. Allow several missed scheduling slots, but still fail CLOSED after | ||
| // six seconds without a real device-consumed heartbeat. | ||
| constexpr std::uint32_t kControlLeaseTimeoutMs = 6000U; |
There was a problem hiding this comment.
When Studio or its USB transport disappears while SV is running, this raises the worst-case shutdown latency from roughly 2.75 seconds to 6.25 seconds, but the repository's operational contracts still require termination in approximately 2.5–3 seconds (docs/SMV_STUDIO_DEVICE_SUPERVISOR_AUDIT.md:214 and docs/SMV_STUDIO_SMART_UX_PROGRESS.md:121). The documented hardware kill test will therefore fail, and operators can receive output for more than twice the promised interval; either preserve the existing bound or update and requalify every consumer and acceptance procedure that relies on it.
AGENTS.md reference: AGENTS.md:L236-L236
Useful? React with 👍 / 👎.
| const std::uint64_t relative_epoch_us = elapsed_us + | ||
| (bench ? static_cast<std::uint64_t>( | ||
| config.bench_utc_epoch_millisecond) * 1000ULL : 0ULL); |
There was a problem hiding this comment.
This protocol timing fix changes both the seconds-versus-milliseconds parser and the carry from a fractional seed into the next UTC second, but no deterministic regression exercises either branch. A case such as a .999 seed plus elapsed time crossing the second boundary, alongside a legacy seconds input, is practical to test and is needed to prevent a later arithmetic/parser change from silently restoring a nearly one-second GM offset.
AGENTS.md reference: AGENTS.md:L230-L234
Useful? React with 👍 / 👎.
Physical capture findings (2026-10-22, firmware 60165e6)
Raw 122.3 s Ethernet PCAP: 489217 SV, 125 each PTP Announce/Sync/Follow_Up, 0 missing/duplicate/invalid, PTP & SV both VID0/PCP4, matching GM identity. smpSynch 0→2 after ~21.25s; no Pdelay_Req/Resp at all, so a SIPROTEC receiver did NOT demonstrate clock lock in this isolated capture. SVLIFE stop_seq=0 / NONE throughout supplied log; spontaneous STOP cannot be attributed to this capture.
Fixed
SVLIFEnow reports max heartbeat gap and count of >2000ms gaps to prove future stop cause; board already preserves stop_seq/cause/hb_age across restart.Not yet proven
No direct SIPROTEC slave clock lock, Pdelay request/response capture, or longer-duration stability/actual disappearance of STOP proven. DO NOT merge to main or stable-release without physical qualification. Run all exact-SHA ESP32, C++, Qt, Windows lifecycle & installer checks.
2026-10-23 PR hygiene disposition — SUPERSEDED, NO MERGE. Exact branch HEAD
3c99e06da2fce4cbcca10cf81d365a3506c9c170was checked with GitHub compare and is an ancestor of integration #286 at0e6c860f61a8780d09064369de5d29eb8c1b88c5(behind_by=0). Closed as a historical stacked checkpoint; its commits remain in the integration lineage and its branch is not deleted. This closure does NOT assert hardware acceptance or authorize merging #286. Divergent branches (especially #284's exclusive AUTO_FAILOVER commits), physical test candidate #285, and feature-unique PRs remain open for separate review.