Repository navigation
fix(spi): preserve the final word of full ESP32 DMA receives - #138
Conversation
|
@CodeRabbit review |
✅ Action performed
|
|
No actionable comments were generated in the recent review.
 |
| Layer / File(s) | Summary |
|---|---|
RX limit calculation and result correction components/hal/spi_slave_hal_iram.c |
For qualifying classic ESP32 DMA transfers, the HAL adds 32 bits to the programmed RX limit. Result correction uses that limit and clamps the count to the configured bit length when needed. |
Receive length and buffer validation components/esp_driver_spi/test_apps/slave/main/test_spi_slave.c |
An ESP32 test covers four SPI modes, eight receive capacities, and wire-length offsets. It checks the reported length, complete four-byte words, and 16 bytes beyond the configured capacity. |
Priority:
Estimated code review effort: 3 (Moderate) | ~20 minutes
Merge Risk: 
e87ee
-
No actionable issue is established for the SPI DMA receive fix; it is mergeable after normal checks.
Pre-merge checks: 
-
Check name Status Explanation Resolution Docstring Coverage Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Documentation-only edits and deletions within survi… Write docstrings for the functions missing them to satisfy the coverage threshold. -
Check name Status Explanation Title check The title clearly and concisely describes the main change: preserving the final word during full ESP32 DMA receives. Description check The description directly explains the ESP32 SPI DMA bug, the implementation approach, and the regression coverage. Linked Issues check Check skipped because no linked issues were found for this pull request. Out of Scope Changes check Check skipped because no linked issues were found for this pull request.
Full details: Docstring Coverage
-
Explanation
Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Documentation-only edits and deletions within surviving functions are included; unchanged diff context and fully deleted functions are excluded. Analyzed 3 functions across 2 files.
Resolution
Write docstrings for the functions missing them to satisfy the coverage threshold.
-
Finishing Touches: 💡 1
-
📝 Generate docstrings: 💡
-
- Commit to this branch
- Create a new PR
🧪 Generate unit tests (beta)
-
- Commit to this branch
- Create a new PR
-
- Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.
Comment @coderabbitai help to get the list of available commands.
An exactly full classic ESP32 SPI target DMA receive can lose its final word. Update ESP-IDF to toitware/esp-idf#138, which fixes the hardware receive limit and length accounting without enlarging DMA buffers. The Toit allocation-padding workaround has been removed. Keep the hardware regression cases for exact capacities of 4088, 4092, 4096, 4100, 4104, and 8192 bytes in modes 0 and 2, using two payload patterns. The controller holds chip select across DMA-sized chunks so it can exercise those capacities on a classic ESP32.
Classic ESP32 SPI DMA reception can lose its final four bytes when the controller fills the configured receive capacity. A standalone ESP-IDF target driven by an nRF5340 reproduces this without Toit, including at just four bytes and across the 4092-byte DMA descriptor boundary.
Program the hardware RX limit one word beyond the requested capacity while keeping DMA descriptors and allocations at the original size. Correct the length counter against that hardware limit, then clamp the reported length to the caller's capacity. TX-only and non-DMA paths retain their existing limits; the adjustment is specific to classic ESP32 and cannot overflow the register.
Add an internal-loopback regression covering four SPI modes, capacities from 4 to 8192 bytes, short/exact/overlong transfers (including one-bit boundaries), fresh payloads, and guard bytes. The 156 cases pass on an ESP32 DevKitC. On the original driver the test fails because a 31-bit transfer is reported as 32 bits. The separate external-controller baseline reports payload failures in 80 of 112 word-aligned cases, with the final word left untouched.
This replaces the allocation-padding workaround in toitlang/toit#3320. No Toit runtime is needed to run this regression. The Toit integration also passes all 511 nRF/ESP32 paired SPI fixture requests with the allocation-padding workaround removed.