feat(i2c): add synchronous slave transmit provider - #122
Conversation
e63b8f7 to
7b6ac1f
Compare
7b6ac1f to
e589619
Compare
|
@CodeRabbit review |
✅ Action performedReview finished.
|
WalkthroughThe PR adds synchronous transmit and transmit-completion callbacks to the version-two I2C slave driver. It updates ISR FIFO handling and callback validation. It also corrects 10-bit I2C master read-after-restart address sequencing. ChangesI2C slave transmit callbacks
10-bit I2C master read addressing
Estimated code review effort: 3 (Moderate) | ~20 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches 💡 1🛠️ Fix failing CI checks 💡
📝 Generate docstrings
🧪 Generate unit tests (beta)
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 |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@components/esp_driver_i2c/i2c_private.h`:
- Line 224: Replace tx_data_count in components/esp_driver_i2c/i2c_private.h:224
with state that tracks callback-origin bytes still pending in the FIFO. Update
FIFO loading in components/esp_driver_i2c/i2c_slave_v2.c:73-92 to account
separately for callback and ring-buffer data, and update transmitted calculation
at lines 126-133 to count only callback bytes consumed. At lines 575-576,
serialize callback-mode transitions with pending TX state or preserve source
accounting for already queued data.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: e3e64984-9bc2-46fb-b503-b4be14c11b85
📒 Files selected for processing (5)
components/esp_driver_i2c/i2c_master.ccomponents/esp_driver_i2c/i2c_private.hcomponents/esp_driver_i2c/i2c_slave_v2.ccomponents/esp_driver_i2c/include/driver/i2c_slave.hcomponents/esp_driver_i2c/include/driver/i2c_types.h
738be03 to
a3ff58b
Compare
## Summary - Make asynchronous controller callbacks identify the actual transaction device rather than searching by raw address. - Apply per-device ACK policy and retain NACK/timeout status until the transaction is terminal, producing exactly one accurate completion callback. - Delay normal completion until every command batch has been sent, clear stale interrupt state before a new transfer, and mask terminal timeout interrupts. - Recover after NACK/timeout with a bounded FSM reset and return `ESP_ERR_INVALID_STATE` for an unexpectedly busy bus instead of entering synchronous bus-clear logic. ## Dependencies - Builds on #122. - Required by toitlang/toit#3156. ## Testing Exercised by Toit's two-board asynchronous controller suite on ESP32 and ESP32-S3: probes, scan, NACK and timeout recovery, ACK policy, 7-bit/10-bit identity, FIFO/command boundaries, concurrent callers, RMT timing probes, and clock stretching. The same callback and recovery gaps are still present in the inspected ESP-IDF v6.1-dev checkout, so this patch remains a focused delta from the current upstream API. <sub>Stack created with <a href="https://github.com/github/gh-stack">GitHub Stacks CLI</a> • <a href="https://gh.io/stacks-feedback">Give Feedback 💬</a></sub>
Summary
Dependencies
Testing
Exercised by Toit's autonomous register-target hardware suite on ESP32 and ESP32-S3, including 7-bit/10-bit addressing, register wrapping, updates, and oversized writes.
Stack created with GitHub Stacks CLI • Give Feedback 💬