Repository navigation
P0 SMV: atomic 8-channel firmware scenario commit with token/hash ACK and verified START - #251
Conversation
… commit and fingerprint
…ied scenario readback
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: a3a6e6371f
ℹ️ 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".
| if (has_extra_token(&save) || g_running.load(std::memory_order_acquire) || | ||
| !g_scenario.begin(token, g_signal_bank)) { | ||
| ESP_LOGE(kTag, "SCENARIO transaction=%lu rejected BEGIN", static_cast<unsigned long>(token)); |
There was a problem hiding this comment.
After SCENARIO BEGIN succeeds, the firmware keeps g_scenario active indefinitely unless it later receives COMMIT, ABORT, or STOP. If Studio disconnects or crashes during the batch, reconnect identifies successfully but the automatic PROFILE SHOW/profile-sync commands are rejected by live_control.cpp while staging is active, so Studio cannot become start-ready and therefore cannot issue a new BEGIN to recover. Bind the transaction to a bounded deadline/session and invalidate it when that ownership is lost.
AGENTS.md reference: AGENTS.md:L158-L166
Useful? React with 👍 / 👎.
| bool DeviceController::stop() { | ||
| resetScenarioStart(); | ||
| return sendCommand(QStringLiteral("STOP")); |
There was a problem hiding this comment.
If Stop or failScenarioStart() runs while the initial exclusive BEGIN/SET/ENABLE/COMMIT batch is still being written, this priority STOP does not cancel the remaining batch: DeviceIoWorker::queuePrioritySvStop() removes only START commands and preserves the queued staging commands. They consequently execute after STOP; if STOP invalidated an already-written BEGIN, the remaining SETs are treated as ordinary live edits and mutate the bank non-atomically despite the cancellation. Drop the whole pending scenario transaction before enqueueing the safety STOP.
AGENTS.md reference: AGENTS.md:L160-L163
Useful? React with 👍 / 👎.
| if (scenarioStartStage_ != ScenarioStartStage::idle) { | ||
| failScenarioStart(QStringLiteral("SCENARIO serial queue rejected: ") + message); | ||
| return; | ||
| } |
There was a problem hiding this comment.
While a scenario is pending, this treats every commandRejected signal as failure of that scenario, even when the rejected command is unrelated. For example, PtpExpertPanel.qml requests a refresh every second while PTP is running; if that request lands while the exclusive scenario batch is queued, the worker rejects the PTP command and this handler aborts the otherwise valid SV start and sends STOP. Include the rejected command/transaction in the signal or suppress unrelated requests without corrupting the scenario state.
AGENTS.md reference: AGENTS.md:L86-L86
Useful? React with 👍 / 👎.
1 participant
P0-A: deterministic 4I+4V firmware-applied transaction (stacked on #250)
Target: Change the existing GUI → serial worker → ESP32-P4 live signal bank control path from queued FIFO only to staged, validated, firmware-committed, verified readback before START. PTP ISR, MAC timestamp gate and SV payload serialization are untouched.
Root cause at #247
DeviceController::sendCommandBatch()reports asynchronous Qt invocation admission, not firmware ACK. EachFREQ/SET/ENABLE/SHAPEcommand immediately published to the live signal bank; a missing/rejected SET could result in partially configured publisher output. A subsequent bare START could reuse obsolete state.Changes
SvLiveSignalStateandSvLiveSignalBank. New fixed-capacity, control-task-onlySvLiveScenarioTransactionstages one frequency, 8 value/phase/quality, 8 enable flags and one current CT shape; all required before one atomicbank.publish. Same versioned little-endian FNV-1a semantic fingerprint computed on host and device. No separate realtime SSOT.SCENARIO-TX-V1,SCENARIO BEGIN/COMMIT/SHOW/START/ABORT, token/hash/applied generation ACKs; reject incomplete/invalid/mismatched bank, stale generation, malformed SET, and START with stale/no commit. Pending untrusted SETs cannot leak into the published bank.COMMITthenREADBACKmatching token/hash/generation, finally sendsSCENARIO START, waits for same verified ACK. Stage deadlines are bounded, STOP cancels pending START via P0 Studio: cancel queued SV START with bounded priority STOP and block bare START #250 reserved safety slot.SCENARIO-TX-V1; older compatible devices without the feature require an explicit operator-approved update before new START; no automatic flashing.tests/test_live_signal.cpp, deterministic Qt token/reject/boot scope tests and updated operator workspace assertions.Explicit current evidence limits
A software CI PASS is not evidence of hardware STOP, 4I+4V on-wire RMS/phases or PTP SOURCE AUTO <=10s. Hardware acceptance requires exact-firmware
IDENTIFY, full serial transaction log (token + generation + hash), subsequent readback, independently decoded PCAP under isolated lab SOURCE with 4000fps continuity, and measured CPU/stack/ISR metrics. Prior historical captures belong to different runs and cannot validate this build.Invariants / caution
No changes to
smpSynch, SOURCE class/traceability, 4000fps ISR, MAC descriptor gating, 1 ASDU, APPID/VID/stream ID or waveform engine. Fixed one scenario staging object, bounded existing serial queue/STOP. STOP is never claimed complete from queue acceptance. This PR is not production-ready until exact-head CI and physical acceptance. No merge to main. Rollback to #250/eb7448b2or known good bench build as appropriate.