Repository navigation
Conversation
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: ae235dd0e7
ℹ️ 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".
| [[nodiscard]] static constexpr int identityMaxAttempts() noexcept { return 5; } | ||
| [[nodiscard]] static constexpr int identityRetryIntervalMs() noexcept { return 1100; } |
There was a problem hiding this comment.
Align the controller IDENTIFY policy with the worker
Changing only DeviceIoWorker to 5 attempts/1100 ms leaves DeviceController::identityMaxAttempts() and identityRetryIntervalMs() at 3/650. Consequently, checkFirmwareContract() makes both boundedIdentifyPolicy and workerPolicyAligned false and exits with code 11; the Studio Qt, native-preview, and release workflows all invoke --check-firmware-contract, so this change breaks those required checks on every platform. Update the controller-side contract and its assertions together with these worker values.
AGENTS.md reference: AGENTS.md:L230-L236
Useful? React with 👍 / 👎.
| ++unidentifiedDiscoveryTicks_; | ||
| if (unidentifiedDiscoveryTicks_ < 2) return; | ||
| } | ||
| if (!device_->deviceVerified() && !device_->discovering() && !device_->connected()) { | ||
| unidentifiedDiscoveryTicks_ = 0; |
There was a problem hiding this comment.
Cap automatic rediscovery after terminal timeouts
When a non-ARStack or permanently unresponsive serial device remains visible, every terminal Unidentified result reaches this threshold, resets the counter, probes every visible port again, and then repeats indefinitely on the periodic timer. The five-attempt limit only bounds each individual port visit; it does not bound the reconnect policy, so Studio can continually reopen unrelated COM ports and send IDENTIFY until shutdown. Add a maximum rediscovery count or transition to a terminal state requiring explicit retry.
AGENTS.md reference: AGENTS.md:L80-L84
Useful? React with 👍 / 👎.
|
FINAL EXACT-HEAD CI & FIELD HANDOFF — 19 Oct 2026. Head Physical log supplied by operator conclusively showed existing board was already verified and exact build matched old bundle: Not yet field verified on user's Windows/COM15: require repeated cold reopen and hot replug tests, exact semantic ID verification and no false required prompt. PR stays OPEN/unmerged until bench acceptance. |
|
Cross-thread P0 engineering audit / next execution order — 19 Oct 2026 Source baseline: PR #247 f3e4456 on PR #246 7dc488c; both unmerged and not in main. Operator confirms false Install Firmware prompt no longer repeats. Do not alter verified board-detection behavior speculatively. HIGHEST P0 GAP verified from source: DeviceController::startWithScenario constructs exclusive FIFO FREQ + SET/ENABLE for eight channels + SHAPE + START from QML table snapshot. However DeviceController::sendCommandBatch only returns successful asynchronous QMetaObject::invokeMethod queue dispatch, not worker admission, serial write, firmware-applied state, or applied-scenario ACK. The embedded UART parser applies individual SET/ENABLE. Worker FIFO ordering is not a device-atomic configuration transaction. Next small PR should provide canonical typed scenario/normalized scaling, generation-safe firmware commit/ACK/readback, negative failure testing, and independent same-run GUI->SHOW->PCAP waveform equivalence. Do not label queued as applied. PTP evidence: Historical 120s good capture had LOCAL=1 with 0 smpCnt gaps. Experimental preflight capture yielded 480087 SV frames all NOT SYNC. Source bug repaired in #246 7dc488c but current hardware <=10s LOCAL not accepted. Preserve independent sample phase and real MAC TX evidence + 3 clean live epochs. Do not force smpSynch=1 or claim GLOBAL=2. Preflight idle runs a 4kHz GPTimer ISR; measure CPU, stack, jitter before claiming optimized. Lab acceptance: 10 cold opens, 10 hot replug/COM renumber, verified current firmware without reinstall prompt, waveform and eight channels/quality/frequency exactly encoded from GUI, 120s + 3600s canonical SV soak at 4000fps with no publisher loss, PTP SOURCE AUTO <=10s conditional after ready warm preflight, no SV before START, immediate sync revocation on evidence failure. Host/Npcap arrival jitter is not MAC sample-clock jitter. Next thread should read AGENTS.md and docs/STUDIO_SV_PTP_NO_REGRESSION_BASELINE_20261018.md, docs/ARSTACK_SV_PTP_LOCK_BASELINE_20261018.md and source of Main.qml, SignalMatrix.qml, WorkflowBar.qml, DeviceController.cpp, DeviceIoWorker.cpp, SmartSessionController.cpp, StudioDeviceController.hpp, embedded live_control.cpp/app_main.cpp, live_signal_state.hpp, sv_ptp_phase_authority.hpp, sv_tx_egress_evidence.hpp. Historical audits may be outdated; verify actual source and exact SHA. Details are in the operator's 19-Oct handoff document. No merge to main solely from green CI. |
Field report, 19 October 2026
Operator opens Windows Studio after successful firmware flash; ribbon reports Firmware required / No ESP32 board detected, despite on-board
IDENTIFYsubsequently proving exact bundle build match (7dc488c2c49bde03), protocol 1 and required capabilities.Root cause in GUI (not board flashing)
IdentificationState::Unidentifiedwas reached, periodic discovery timer explicitly refused to try again, so reopening GUI repeated the same failure.Fix
QSettingsdiscovery hint, never trust a cached port as verified identity. Probe other ports if the hint fails.Acceptance
Run Native Preview exact-head, Qt Studio, Windows Lifecycle, C++ and deterministic session harness. Test cold GUI startup/closing/reopening with running ESP32-P4 and with delayed IDENTIFY, multiple COM ports and unsupported device. Show matching firmware as current, not required. Ensure firmware mismatch still appears as required when semantically verified and incompatible. Manual firmware-recovery remains guarded. No merge to main without bench validation.