Add register-backed I2C targets

[floitsch]

Summary

• Add i2c.RegisterTarget, an autonomous native register map for common I2C peripherals.
• Serve register reads directly from C++/ISR-safe storage so the target is immediately ready and does not need Toit-side clock stretching.
• Support one- and two-byte register addresses, wrapping reads, Toit-side register updates, and dropped-write accounting.
• Allocate the complete register backing store during construction; callbacks perform no allocation and never report asynchronous OOM.

Dependencies

• Builds on Add asynchronous I2C target support #3154.
• Requires feat(i2c): add synchronous slave transmit provider toitware/esp-idf#122.

Testing

• Two-device hardware tests on ESP32 and ESP32-S3.
• Covers address widths, register address widths, wrapping, partial updates, concurrent access, oversized/dropped writes, and controller/target teardown and reuse.

_{Stack created with GitHub Stacks CLI • Give Feedback 💬}

[floitsch]

@CodeRabbit review

[coderabbitai]

✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

[coderabbitai]

Walkthrough

Adds a public RegisterTarget API backed by an ESP32 native register resource. The implementation supports indexed and bulk access, register-address encoding, ISR transfers, dropped-write tracking, cleanup, native bindings, and board-level integration tests.

Changes

I2C register target

Layer / File(s)	Summary
Public API and primitive bindings lib/i2c.toit, src/compiler/propagation/type_primitive_i2c.cc, src/primitive.h	Adds RegisterTarget, register access methods, lifecycle methods, dropped-write reporting, and native primitive declarations. Register reads now encode addresses using the configured register size and byte order.
ESP32 register resource and callbacks src/resources/i2c_esp32.cc, src/tags.h, third_party/esp-idf	Adds register storage, I2C ISR callbacks, creation and cleanup, indexed and bulk operations, dropped-write counting, resource registration, and the ESP-IDF submodule update.
ESP32 integration and API validation tests/hw/esp32/i2c-register-target-*.toit	Adds board tests for register transfers, wrapping, addressing, buffering, broadcast writes, reconfiguration, closure, validation, local APIs, and dropped writes.

Estimated code review effort: 4 (Complex) | ~60 minutes

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly and concisely describes the main change: adding register-backed I2C targets.
Description check	✅ Passed	The description accurately explains the new RegisterTarget API, implementation behavior, dependencies, and hardware testing.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

✨ Finishing Touches 💡 1

🛠️ Fix failing CI checks 💡

• Create stacked PR
• Commit on current branch

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch floitsch/i2c.20-register-target

---

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.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[coderabbitai]

Actionable comments posted: 3

🧹 Nitpick comments (3)

src/resources/i2c_esp32.cc (2)

193-193: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Protect dropped_write_count_ like the counters in I2cTargetResource.

receive_from_isr increments dropped_write_count_ from ISR context. dropped_write_count() reads it from a Toit task without synchronization. I2cTargetResource uses spinlock_ for the equivalent counters. Add the same spinlock, or document why an unsynchronized word access is sufficient here.

Also applies to: 241-241, 253-253

🤖 Prompt for 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.

In `@src/resources/i2c_esp32.cc` at line 193, Protect all accesses to
dropped_write_count_ in receive_from_isr, dropped_write_count(), and the
additional counter-update sites with the same spinlock_ synchronization used by
I2cTargetResource; otherwise document a concrete guarantee that unsynchronized
ISR/task word accesses are safe.

---

597-599: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Move the stretch-support guard to the start of register_target_create.

When SOC_I2C_SLAVE_CAN_GET_STRETCH_CAUSE is 0, FAIL(UNSUPPORTED) returns unconditionally. The remaining declarations and setup are unreachable. The macro is correct for address-match clock stretching, including I2C_SLAVE_STRETCH_CAUSE_ADDRESS_MATCH.

🤖 Prompt for 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.

In `@src/resources/i2c_esp32.cc` around lines 597 - 599, Move the `#if`
!SOC_I2C_SLAVE_CAN_GET_STRETCH_CAUSE FAIL(UNSUPPORTED) guard to the beginning of
register_target_create, before any declarations or setup. Preserve the existing
guard condition and unsupported return behavior, including support for
I2C_SLAVE_STRETCH_CAUSE_ADDRESS_MATCH.

tests/hw/esp32/i2c-register-target-shared.toit (1)

273-283: 📐 Maintainability & Code Quality | 🔵 Trivial | 🏗️ Heavy lift

Add a test with overlapping register access.

Line 275 runs the controller operation after board 2 has stopped accessing target. Lines 276 and 282 then require FINISH before board 2 reads the register data. This serializes every tested controller transfer and Toit-side register operation.

Add a hardware test that performs target.read or target.write while board 1 performs controller reads and writes. This validates the concurrent ISR and Toit access contract stated in the PR objective.

🤖 Prompt for 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.

In `@tests/hw/esp32/i2c-register-target-shared.toit` around lines 273 - 283, Add a
hardware test in the shared I2C register-target test suite that overlaps a
Toit-side target.read or target.write with board 1 controller read/write
activity, rather than waiting for the controller operation to finish first.
Reuse the existing expect-after/expect-dropped-after helpers and synchronization
protocol where appropriate, and verify the register access completes correctly
under concurrent ISR and Toit access.

🤖 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 `@lib/i2c.toit`:
- Around line 694-698: Update read-bytes to validate reg before
byte-order_.put-uint, rejecting values outside the register-size range with
OUT_OF_RANGE; preserve the existing encoding and device_.read-address flow for
valid values.

In `@src/resources/i2c_esp32.cc`:
- Around line 191-227: Update receive_from_isr so the selected register address
remains separate from the write cursor: preserve register_pointer_ at the
decoded address while writing data through a separate cursor, or restore it
after the write. Ensure transmit_pointer_ and subsequent transmit_done_from_isr
advancement start from the selected address, so a read advances from address 0
to 1 regardless of prior written bytes.
- Around line 535-562: Replace RTC_IRAM_ATTR with IRAM_ATTR on
register_target_receive_handler, register_target_transmit_handler, and
register_target_transmit_done_handler so all three callbacks reside in IRAM and
pass ESP-IDF’s IRAM-safe registration checks.

---

Nitpick comments:
In `@src/resources/i2c_esp32.cc`:
- Line 193: Protect all accesses to dropped_write_count_ in receive_from_isr,
dropped_write_count(), and the additional counter-update sites with the same
spinlock_ synchronization used by I2cTargetResource; otherwise document a
concrete guarantee that unsynchronized ISR/task word accesses are safe.
- Around line 597-599: Move the `#if` !SOC_I2C_SLAVE_CAN_GET_STRETCH_CAUSE
FAIL(UNSUPPORTED) guard to the beginning of register_target_create, before any
declarations or setup. Preserve the existing guard condition and unsupported
return behavior, including support for I2C_SLAVE_STRETCH_CAUSE_ADDRESS_MATCH.

In `@tests/hw/esp32/i2c-register-target-shared.toit`:
- Around line 273-283: Add a hardware test in the shared I2C register-target
test suite that overlaps a Toit-side target.read or target.write with board 1
controller read/write activity, rather than waiting for the controller operation
to finish first. Reuse the existing expect-after/expect-dropped-after helpers
and synchronization protocol where appropriate, and verify the register access
completes correctly under concurrent ISR and Toit access.

🪄 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: 6d313d41-649d-4a9f-992c-99e13e98ea5c

📥 Commits

Reviewing files that changed from the base of the PR and between d47a4e5 and 612d88d.

📒 Files selected for processing (9)

• lib/i2c.toit
• src/compiler/propagation/type_primitive_i2c.cc
• src/primitive.h
• src/resources/i2c_esp32.cc
• src/tags.h
• tests/hw/esp32/i2c-register-target-board1.toit
• tests/hw/esp32/i2c-register-target-board2.toit
• tests/hw/esp32/i2c-register-target-shared.toit
• third_party/esp-idf