Repository navigation
Conversation
… exact-match status
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. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e44bdf13f4
ℹ️ 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".
| resetPtpStartup(); | ||
| ptpRunning_ = false; | ||
| ptpStatus_ = QStringLiteral("PTP start failed"); | ||
| ptpStatus_ = QStringLiteral("PTP start failed: %1").arg(reason); |
There was a problem hiding this comment.
Update the rejection harness for the new status
This changes the rejection result from the exact string PTP start failed to a reason-bearing string, but ptpGuidedRejectedConfigStops() still requires equality with the old value. A simulated PTP configuration rejected therefore makes arstack_session_harness return 21, and both .github/workflows/arstack-studio-qt.yml and .github/workflows/arstack-studio-native-preview.yml execute that harness, so the Studio CI jobs will fail until the deterministic expectation is updated.
AGENTS.md reference: AGENTS.md:L230-L236
Useful? React with 👍 / 👎.
| const accepted = panel.session.requestStartPtpConfigured(role, panel.ptpProfile()) | ||
| if (!accepted && panel.controller) | ||
| panel.controller.showMessage("PTP start blocked. Check timing diagnostics.", true) | ||
| panel.controller.showMessage("PTP could not start: " + panel.device.ptpStatus, true) |
There was a problem hiding this comment.
Show the synchronous request error instead of stale status
When requestStartPtpConfigured() fails synchronously—for example, because an expert interval field is empty or outside the firmware bounds—startPtpConfigured() records the actionable reason in lastError but does not update ptpStatus. This line then replaces the detailed deviceMessage toast with stale state such as PTP could not start: Lab TX stopped, hiding the validation failure the operator needs; use lastError or propagate the request failure reason instead.
AGENTS.md reference: AGENTS.md:L68-L72
Useful? React with 👍 / 👎.
…RCE bench startup
…l ACK diagnostics
…, role mismatch rollback
… clear only on fresh START
P0 hardware failure follow-up (no main merge)
Observed on user's ESP32-P4: USB board READY, firmware v0.1.1 shown instead of exact build, Studio reports PTP START FAILED and SV internal recorder cannot acquire because PTP SOURCE is not running. Old firmware/capture cannot prove a specific board-side PTP failure reason.
Reproduced source-level causes and fixes
respondToPeerDelay=true. SOURCE firmware explicitly refuses to start if RX hardware timestamp Pdelay responder cannot be initialized. This is an actual interface/capability mismatch, not a proof the particular board RX was unavailable. Default is now false, user can explicitly enable when an IEC profile/relay requires Pdelay. Requesting unsupported hardware always fails closed; no silent bypass or fake lock.PTP status=RUNNINGin the START response, but PTP2 role/discipline is periodic and may arrive afterwards. Studio formerly rolled back immediately if RUNNING was observed before PTP2. Now both facts are tracked independently under the original 5-second deadline, accepted in any order only when both match; STOPPED, wrong role, or timeout trigger verified safety STOP. Regression covers positive two orders, no-PTP2 timeout, role mismatch, no unverified success.PROFILE SHOWafter PTPROLE ACK. This works even if no unrelated periodic readback occurs and still demands matching role before CONFIG/START.PTP start rejected: source_reason=...to Studio; no polling/ISR or SMV 4kHz cost.Automated acceptance, physical blockers
Qt structural, deterministic Linux+Windows scenario and ownership tests, exact-SHA Windows Lifecycle, PTP SOURCE+RECEIVER ESP32-P4, and Native Preview embedded matching firmware/manifest/installer must pass. Hardware START and real relay LOCK remain unverified; require new Studio-only board test and on-board
SVSYNCflight recorder + Capture evidence. SOURCE without required Pdelay is for bench diagnosis, not claim of interoperability with relays mandating Pdelay. Release/main merge strictly blocked pending physical success.Integration layered on PR #262→#261→#260→#258→#257→#256→#254→#252; changes are not on main.