Skip to content

P0 Studio: reliable ESP32-P4 reconnect and truthful firmware detection on startup - #247

Open
masarray wants to merge 14 commits into
fix/studio-auto-sync-10s-evidence-20261018from
fix/studio-smart-serial-reconnect-20261019
Open

masarray wants to merge 14 commits into
fix/studio-auto-sync-10s-evidence-20261018from
fix/studio-smart-serial-reconnect-20261019

Conversation

@masarray

Copy link
Copy Markdown
Owner

Field report, 19 October 2026

Operator opens Windows Studio after successful firmware flash; ribbon reports Firmware required / No ESP32 board detected, despite on-board IDENTIFY subsequently proving exact bundle build match (7dc488c2c49bde03), protocol 1 and required capabilities.

Root cause in GUI (not board flashing)

  • Serial identity timeout was 3 probes 650ms apart, easily exhausted while a board emits PTP/status logs or USB is settling.
  • At a semantic timeout, Supervisor could wrongly classify a high-confidence COM port as needing firmware installation. USB VID/PID/COM is a transport candidate, never firmware evidence.
  • Once IdentificationState::Unidentified was reached, periodic discovery timer explicitly refused to try again, so reopening GUI repeated the same failure.
  • Auto discovery prematurely attempted only one recommended serial port instead of falling back through others.
  • No priority was given to a last previously verified port, even though a known COM (here COM15) can be remembered as a hint.

Fix

  • 5 IDENTIFY attempts at 1100ms intervals (bounded, no unending serial burst); automatic reconnection following timeout has a finite cooldown.
  • Persist previously semantically verified port as QSettings discovery hint, never trust a cached port as verified identity. Probe other ports if the hint fails.
  • Distinguish UNKNOWN / UNIDENTIFIED from incompatible firmware. No automatic firmware reinstallation prompt from IDENTIFY timeout. Manual port selection may offer explicitly labeled optional Firmware recovery; actual flash still requires user's action and ROM chip/revision verification.
  • Update Qt ribbon messages and tests. No firmware, PTP, SV wire, 4I4V, or phase logic is changed.

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.

@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Oct 18, 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-18T23:30:25.807348Z ae235dd 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: 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".

Comment on lines +28 to +29
[[nodiscard]] static constexpr int identityMaxAttempts() noexcept { return 5; }
[[nodiscard]] static constexpr int identityRetryIntervalMs() noexcept { return 1100; }

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 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 👍 / 👎.

Comment on lines +64 to +68
++unidentifiedDiscoveryTicks_;
if (unidentifiedDiscoveryTicks_ < 2) return;
}
if (!device_->deviceVerified() && !device_->discovering() && !device_->connected()) {
unidentifiedDiscoveryTicks_ = 0;

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge 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 👍 / 👎.

Copy link
Copy Markdown
Owner Author

FINAL EXACT-HEAD CI & FIELD HANDOFF — 19 Oct 2026. Head f3e445616e9e7d8ba10e787ba4248a1a0396e557. All four path-scoped workflows PASSED: Product Naming Guard, ARStack Studio Qt (includes deterministic supervisor/session harness and Windows ownership), ARStack Studio Windows Lifecycle (close/exit/immediate reopen), and ARStack Studio Native Preview. Installer workflow https://github.com/masarray/arstack61850/actions/runs/39176901930 SUCCESS, including matched ESP32-P4 firmware compilation, manifest/SHA verification, packaged GUI smoke, Windows Inno clean-install/upgrade/uninstall and NSIS v0.1.1 migration. Windows GUI installer+portable artifact https://github.com/masarray/arstack61850/actions/runs/39176901930/artifacts/12154683930 ; firmware artifact https://github.com/masarray/arstack61850/actions/runs/39176901930/artifacts/12154951119.

Physical log supplied by operator conclusively showed existing board was already verified and exact build matched old bundle: Device verified=yes, firmware version=0.1.1, build=7dc488c2c49bde03, protocol 1, required capabilities. Initial startup timed out after three IDENTIFY probes (650ms), causing a false "Firmware required" warning, but reconnect later received ARStack identity successfully. Fix: no firmware-missing inference from USB/IDENTIFY timeout, 5 probes at 1.1s intervals, bounded re-discovery cooldown, persistent last-semantically-identified COM hint (only for priority; never proof), fallback through other ports, preserve device identity pinning across transient recovery timeouts, and manual-only ROM-verified firmware recovery. Firmware version/protocol compatibility remains hard gate before START; port metadata never bypasses handshake. No SV/PTP wire or firmware-source behavior changed. Do not tell user to reflash an already compatible board just because Studio is updated.

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.

Copy link
Copy Markdown
Owner Author

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.

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