Repository navigation
feat(M4-P0B): three-stream binary V2 envelope with bounded CRC and atomic decode - #335
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. |
![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: db29357931
ℹ️ 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 (!valid_roster(p)) return std::nullopt; | ||
| std::size_t total = header_bytes; | ||
| for (const auto& profile : p) { | ||
| const auto size = SvDeviceProfileBinaryCodec::encoded_size(profile); |
There was a problem hiding this comment.
When two input profiles have matching MAC/APPID/VLAN fields and equally oversized sv_id_length values, valid_roster() compares their IDs before the nested V1 codec validates those lengths. The resulting string_view comparison reads beyond the fixed 96-byte arrays instead of returning invalid_profile; an AddressSanitizer reproduction with lengths of 65535 reports a stack-buffer-overflow. Validate each profile through the existing V1 validation path before performing identity comparisons.
AGENTS.md reference: AGENTS.md:L57-L64
Useful? React with 👍 / 👎.
| p.leaf_count == 16U && | ||
| p.payload_size_bytes == 64U && | ||
| p.asdu_options.sample_synchronized; |
There was a problem hiding this comment.
A roster containing sixteen INT32 leaves per stream passes both encode and decode successfully, despite having no Quality leaves: these count/size checks do not establish the required eight ordered INT32+Quality pairs. The nested V1 codec intentionally accepts broader layouts, whereas the existing deployment gates in esp32p4_profile_support.hpp and compiled_profile_adapter.hpp reject them, along with unsupported ASDU options. Consequently, the new envelope can publish a supposedly validated M4 roster that the device rejects. Reuse a shared device-capability validator so this boundary preserves the existing layout and option restrictions.
AGENTS.md reference: AGENTS.md:L30-L31
Useful? React with 👍 / 👎.
db29357 to
bc353ab
Compare
M4-P0B — Binary 3-stream roster protocol envelope (host foundation)
Stacked on P0A #334 until it merges. Implements the binary contract, not MCU start/stop/TX tasks.
Implemented
M4ThreeSvBinaryCodecwith fixed 28-byte versioned ARM3 / V2 header and exactly 3 independently validated, length-prefixed existing ARSV V1 binary device records.encode_into(span<uint8_t>)/decode(span<const uint8_t>), bounded profile lengths, big-endian numbers, CRC32 across envelope, nested per-record CRC, zero-reserved fields and full length equality.Still NOT delivered in this PR
Firmware V2 serial transfer/atomic staging/commit/readback; three EMAC TX streams; PTP-aligned scheduler; three isolated waveform-bank controls; GUI navigation. Follow #333 and the ARD for those packages.
Merge strategy
mainafter P0A merges.