Repository navigation
P0-A transactional 4I+4V START, firmware ACK/readback, safe STOP and independent SV PCAP evidence - #252
Conversation
…al encoded-state hash
… and token-bound START
…lated staging bank
… DeviceController
… without premature START
…sh readback; fail-closed timeout
…d replay CFG regressions
…e, malformed and negative ACK
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. |
… instead of dropping them
![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: 3dbd29f180
ℹ️ 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 (!identity_.capabilities.contains(QStringLiteral("SV-CONFIG-TXN-V1"), Qt::CaseInsensitive) || | ||
| identity_.bootId.size() != 16 || sessionGeneration_ == 0) { | ||
| setError(QStringLiteral("Firmware does not support verified SV configuration transactions; START refused.")); |
There was a problem hiding this comment.
When a device has the expected firmware version and the previously required capabilities but lacks SV-CONFIG-TXN-V1, identitySupportsCurrentContract() still makes firmwareIsCompatible() true, so SmartSessionController::reconcile() reports READY with startReady_ = true; every attempted start is then rejected here. Because the bare START path is also permanently disabled by this change, such firmware has no usable injection path while Studio presents it as ready and does not require an update. Add this capability to the compatibility contract or otherwise gate READY on it.
AGENTS.md reference: AGENTS.md:L11-L18
Useful? React with 👍 / 👎.
… without false commit
…irmware ACK/readback contract
…ntaminating portable engine header
…y profile is readback-verified and armed
…rbid PTP polls and live SET interleaving
…HOW/SET and retains GUI setpoint edits
…or boot-bound profile reuse
…ide generation in runtime snapshot
…tity; invalidate reuse after legacy edits
…hout breaking legacy serial protocol
…ve canonical hash, generation and PROFILE SHOW; fallback legacy unchanged
…an deploy without waiting for timeout
…onical readback and active hash reuse
…t and new identity session
…+4V readback and generation match; reject stale hash
…ntity changed mid-transaction
…nd absolute 30-second hard deadline
…0ms cool-down, 30s absolute hard stop
… startup self-check
…prevent indefinite PREPARING or auto-START
…licitly during recovery
… START/profile/PTP; reject different device identity
…r without stale READY state
…oof and auto-START authority
…deterministic hard-deadline regression
1 participant
P0-A implementation scope — fail-closed 4I+4V application transaction
Root cause: previous guided START appended
STARTto 8-channel SET/ENABLE + FREQ/SHAPE Qt queue without firmware-applied ACK. A partial or rejected SET could emit SV with stale/wrong channels.This PR builds on #247 head
f3e445616e9e7d8ba10e787ba4248a1a0396e557(stack #247 → #246 → #234; no merge to main). Reuses exactly one signal bank, existing UART command parser, DeviceController session generation, and bounded DeviceIoWorker queue.Code
SvScenarioTransaction: bounded 18-field presence bitmask (FREQ, 8 SET, 8 ENABLE, SHAPE), a single isolatedSvLiveSignalStatedraft, shared encoded-state 64-bit hash, 8-second expiry, invalid/reject taint, one atomic bank publish and generation/hash readback.CFG BEGIN <nonce> <boot_id>,CFG <legacy SET/FREQ/ENABLE/SHAPE>,CFG COMMIT <nonce> <hash>,CFG SHOW <nonce>, then token-boundSTART <nonce> <boot_id> <hash>. Bare START rejected. Firmware advertisesSV-CONFIG-TXN-V1, requires fresh heartbeat control lease; on STOP invalidates the transaction ticket. Explicit successful readback must precede START. No malloc, second supervisor/codec, or ISR changes.formatSignalCommandconversion authority, constructs exactly the same encoded state as firmware, and sends only the bounded exclusive CFG stage/COMMIT initially. Application ACK + readback of session, boot, nonce, hash, generation are required before actual START transmission. STOP/timeout/negative ACK fail closed.Deterministic regressions
tests/test_sv_scenario_transaction.cpptests atomic visibility, complete/partial sets, stale/replayed tokens, checksum mismatch, malformed data, rejected values, expiry, live generation drift, no START before readback.DeterministicSessionHarness.cpp: bare START rejected; valid COMMIT → READBACK → START ACK; negative/stale ACK, wrong generation, STOP during transaction; saturated command queue priority.Important blockers and prohibitions
Do NOT merge or ship without all exact-HEAD CI and physical acceptance. Transaction correctness at source/portable test level is not real on-wire 4I4V equivalence. Need isolated-bench ESP32-P4
IDENTIFYfirmware boot/build, serial CFG COMMIT/READBACK/START and SHOW, independently decoded 120s+3600s PCAP (all 8 channels, quality, smpCnt continuity), PTP SOURCE AUTO true MAC evidence + lock timing, resource/ISR/stack and 10x reconnect. Never force smpSynch, automatically flash, or claim physical PASS from CI. Known working rollback remains available.