Repository navigation
Conversation
…timeout and user cancellation
…ed firmware installer
…fallback contracts
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: 799b66481f
ℹ️ 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".
| boardStillRunning) { | ||
| const QString cause = QStringLiteral("Board PTP remained ACTIVE after one verified " | ||
| "STOP; manual STOP is required before START."); | ||
| ptpLastStartFailure_ = cause; |
There was a problem hiding this comment.
When the first stale-runtime rejection occurs, this assigns a terminal-sounding value to ptpLastStartFailure_, but the successful STOP → role/config → START path only calls resetPtpStartup(), which does not clear that property. Consequently, after the intended automatic recovery succeeds and PTP is running, the Time Sync panel still displays “PTP start did not complete” and exported diagnostics still report that manual STOP is required. Clear or defer this failure state unless the recovery actually terminates unsuccessfully.
Useful? React with 👍 / 👎.
| if (StudioClipboard.copyText(evidence)) { | ||
| panel.copyFeedback = "Firmware diagnostics copied." | ||
| } else { | ||
| panel.copyFeedback = "Clipboard unavailable. Open technical details to review diagnostics." |
There was a problem hiding this comment.
If clipboard access fails, this message directs the operator to technical details, but that view displays only firmware.logText; the value that failed to copy is session.diagnosticsText(), which also contains session state, device identity and telemetry, PTP/SV evidence, and the device serial log. In the failure scenario the requested evidence therefore remains unavailable from this panel despite the recovery instruction. Show the captured evidence in a selectable fallback, as the Time Sync panel does, or direct the user to a view containing the same text.
Useful? React with 👍 / 👎.
1 participant
Physical incident on ESP32-P4 after PR #265: Studio once showed SOURCE ACTIVE with PTP TX, then displayed OFF/START FAILED while firmware rejected
PROFILE PTPROLEwithstop PTP before changing operating role. Copy diagnostics in Advanced → Time Sync appeared to do nothing.Root cause / invariant: Board PTP runtime can stay active after GUI presentation marks it OFF; sending a mutating role request in that state is correctly rejected by firmware. Do NOT force role changes or fake OFF. The production Studio START state machine now recognizes this exact physical rejection and performs ONE bounded
PTP STOP→ explicit ACK → boardPTP SHOW STOPPEDreadback → same role/config START; no ad hoc sleeps and no re-entry on generic profile errors. A second ACTIVE rejection, STOP timeout or queue failure is terminal STOP-UNVERIFIED and blocks unproven START until manual STOP/board evidence. Operator STOP cancels retained START intent. No changes to SOURCE cadence, SV publisher, PTP hot paths, ISR, smpSynch authority or firmware build.Clipboard root: Time Sync panel lacked the module import that FirmwarePanel already had for its registered StudioClipboard singleton; the native adapter also returned void while UI unconditionally claimed “copied” with no clipboard readback. Explicit module import; adapter returns true only after OS clipboard contains exact multi-line diagnostics; on error Studio exposes a selectable modal copy fallback (no scripts). Firmware tab similarly reports truthful feedback. Native standalone Qt clipboard harness uses production adapter and tests empty, multiline, repeated copies and stale data. Windows+Linux Studio Qt CI and exact Windows Native Preview packaging run it; structural tests guard import/fallback path.
Regression matrix: stale runtime triggers exactly one verified STOP and resumes the same role; missed STOP readback blocks START; second rejected role no infinite loops or fabricated OFF; manual STOP cancels recovery; original positive verified START and negative wrong role/config remain. No main merge/public-release or physical PTP/SV certification without board evidence.