Skip to content

P0: enforce shared IEC 61850-9-3 PTP SOURCE profile before START; truthful toggle - #265

Open
masarray wants to merge 16 commits into
fix/p0-ptp-start-firmware-provenance-20261020from
fix/p0-ptp-profile-contract-20261020
Open

masarray wants to merge 16 commits into
fix/p0-ptp-start-firmware-provenance-20261020from
fix/p0-ptp-profile-contract-20261020

Conversation

@masarray

Copy link
Copy Markdown
Owner

Physical incident

User's PR #264 Studio screenshot: PTP control immediately flips back OFF; firmware reports PTP configuration rejected: stop PTP first or correct the profile, SVSYNC DIAG events=0. Not a PTP 4kHz lock fault: the configuration was rejected before PTP START.

Root cause proven in code

PR #264 changed hidden QML Pdelay default to checked:false. Firmware validate_runtime_config enforces IEC/IEEE 61850-9-3 SOURCE cadence and hardware Pdelay: 1000 ms Announce, 1000 ms Sync, Pdelay responder enabled. CI passes because deterministic harness generated respondToPeerDelay:true and did not test actual QML contract. Source-level mismatch caused user-visible CONFIG failure.

Production correction

  • Shared portable inspect_ptp_power_profile_cadence(announce_ms,sync_ms,pdelay) alongside protocol options checker; SOURCE preflight in real Qt DeviceController rejects invalid profile locally with explanatory error before any PTPROLE/CONFIG; regular receiver/monitor roles remain configurable.
  • Studio SOURCE always requests peer delay per actual mandatory standard profile, independent of hidden/advanced checkbox. No fake no-Pdelay bypass. UI shows quiet profile requirement; non-SOURCE controls retain their choice.
  • Native board gives PTP configuration rejected: reason=SOURCE_REQUIRES_1S_ANNOUNCE_1S_SYNC_AND_PDELAY, PTP_RUNTIME_ALREADY_ACTIVE, INVALID_OR_UNSUPPORTED_PTP_PROFILE, or race reason; firmware reason is bounded, atomic and control-path only, with no PTP/SMV transmit hot-path modification.
  • Switch remains amber/STARTING during pending actual board ACK and only green once PTP RUNNING is verified, rather than momentarily green then snapped OFF. On genuine rejection, returns OFF with concrete diagnostic.
  • Deterministic regression for SOURCE Pdelay false, Sync 250, Announce 2000, valid retry; portable C++ cadence positives and negatives; structural studio regression for SOURCE UI + firmware reason. Preserve existing staged ACK/role/readback/rollback and internal flight recorder.

Exit / safety

CI must pass exact HEAD: Studio Qt/Linux+Windows harness, ESP32-P4 SOURCE/RECEIVER, Studio Native Preview matched firmware/installer, Windows lifecycle, C++/MSVC. Hardware Pdelay RX capability still must be verified on real ESP32-P4; no PTP lock or IED compatibility claims without Capture+on-board proof. PR stays unmerged, main unchanged.

@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Oct 20, 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-20T14:09:29.769511Z 654dd42 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: 654dd4284c

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

|| panel.device.ptpRunning || panel.startPending || panel.session.updatingFirmware)
return false
const accepted = panel.session.requestStartPtpConfigured(role, panel.ptpProfile())
const accepted = panel.session.requestStartPtpConfigured(role, panel.ptpProfile(role))

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1 Badge Apply the SOURCE profile before validating the role

When the board currently stores a RECEIVER or MONITOR profile with peer delay disabled, this call constructs a valid SOURCE profile with peer delay forced on, but startPtpConfigured() sends PROFILE PTPROLE SOURCE before sending that profile. The firmware's PTPROLE handler clones and validates the existing peer-delay-disabled configuration, so it rejects the role before the corrected PTP CONFIG can arrive; consequently SOURCE still cannot start after using the newly supported non-SOURCE setting. Apply the target profile before role validation or make the role/profile transition atomic, and cover the RECEIVER-with-Pdelay-off to SOURCE transition in the deterministic harness.

AGENTS.md reference: AGENTS.md:L232-L234

Useful? React with 👍 / 👎.

'checked: false' in ptp_panel and
'Require hardware peer-delay responses' in ptp_panel,
"Firmware tab must show exact build and SOURCE Pdelay must be explicit")
'respondToPeerDelay: role === "SOURCE" ? true : pdelayCheck.checked' in ptp_panel and

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1 Badge Update the earlier workspace contract assertion

This updates one structural assertion for the new role-aware ptpProfile(role) call but leaves the assertion at line 256 expecting the removed literal requestStartPtpConfigured(role, panel.ptpProfile()). As a result, the Studio CI command python3 apps/arstack_studio/tests/check_operator_workspace.py fails at this commit with main PTP must reuse the existing verified Advanced profile, so the workflow cannot pass until that earlier expectation is updated as well.

AGENTS.md reference: AGENTS.md:L236-L236

Useful? React with 👍 / 👎.

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