feat(cobs): Fixed issue when exactly 255 bytes (non zero) are encoded. - #780
Conversation
jschuld
changed the title
|
✅Static analysis result - no issues found! ✅ |
| void test_edge_cases(espp::Logger &logger) { | ||
| logger.info("\n=== Edge Cases Test ==="); | ||
|
|
||
| // Test 1: Maximum block size (254 non-zero bytes) |
There was a problem hiding this comment.
Note: This tests seems to attempt to test the same as the test that is added. However, I am unsure if there is a specific reason that 254 is chosen, so I have opted to add an extra test.
There was a problem hiding this comment.
🟢 Approval recommended
The encoder fix addresses the described edge-case correctly and adds coverage for the regression, with only minor non-blocking nits noted.
Pull request overview
Fixes an edge-case bug in the COBS encoder where encoding an input of exactly 255 bytes (all non-zero) could drop the final byte, producing invalid COBS output.
Changes:
- Corrected block rollover handling in
Cobs::encode_packetto always reserve space for the next code byte when a block completes (code == 0xff). - Added an edge-case regression test in the COBS example to cover a 255-byte all-nonzero payload.
File summaries
| File | Description |
|---|---|
| components/cobs/src/cobs.cpp | Fixes encoder pointer advancement on block boundary to prevent losing a byte for the 255-byte all-nonzero case. |
| components/cobs/example/main/cobs_example.cpp | Adds a regression test case for 255-byte all-nonzero payloads (plus minor wording/formatting follow-ups noted in comments). |
Review details
Suppressed comments (1)
components/cobs/example/main/cobs_example.cpp:701
- The failure log message still says "block len", which is ambiguous; update it to the concrete 255-byte edge case so logs clearly match the scenario being tested.
logger.error(
"Test 5: FAIL - Packet of exactly block len with no zeros (decoded: {}, expected: {}, encoded: {})",
decoded.size(), block_len_packet.size(), encoded.size());
}
- Files reviewed: 2/2 changed files
- Comments generated: 2
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| codep = encode; | ||
| if (!byte) | ||
| ++encode; | ||
| ++encode; |
There was a problem hiding this comment.
If the loop ends because of no more data, we still want to ready the next encoding position for the delimiter to be added.
|
Thanks for the fix! |
Description
When encoding data of exactly 255 bytes in length, where the data is all non-zero, the last byte encoded is missing. This results in the encoded data not being valid COBS and can't be decoded.
Motivation and Context
While this is an edge case, it is a valid case and breaks any COBS decoding. This issue was identified when diagnosing issues where COBS decoding would fail intermittedly. A different COBS library was de decoding party. Initial suspicion was actual corruption of data on the line. However, after digging into this the issue was identified to be the COBS encoding for very specific cases (the above case)
How has this been tested?
Screenshots (if appropriate, e.g. schematic, board, console logs, lab pictures):
N/A
Types of changes
Checklist:
Software
.github/workflows/build.ymlfile to add my new test to the automated cloud build github action.