s31 stub support#111
Conversation
|
Warning Rate limit exceeded
You’ve run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📝 WalkthroughWalkthroughThis PR adds ESP32-S31 watchdog timer reset and stub support by defining register constants, implementing RTC WDT reset logic in the loader, and enabling firmware stub loading for the ESP32-S31 chip family. ChangesESP32-S31 Watchdog Reset Support
🎯 3 (Moderate) | ⏱️ ~20 minutes
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 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 |
|
@coderabbitai is the esp32-s31 complete, correct and solid? |
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chainLet me examine the PR in detail to assess whether the esp32-s31 stub support is complete, correct, and solid. 🏁 Script executed: Length of output: 10241 Let me dig deeper into the codebase to check for any missing pieces. 🏁 Script executed: Length of output: 20293 Let me look at the key sections in 🏁 Script executed: Length of output: 8748 Now let me verify the WDT register addresses against the reference implementation. 🏁 Script executed: Length of output: 3236
✅ What's correct in this PRWDT register constants (
Stub JSON (
Code integration (
Stub routing (
|
| Constant | TypeScript (existing) | Python esptool |
|---|---|---|
ESP32S31_SPI_REG_BASE |
0x20500000 |
0x20501000 |
ESP32S31_EFUSE_BLOCK1_ADDR |
BASEFUSEADDR + 0x044 |
EFUSE_BASE + 0x050 |
ESP32S31_MACFUSEADDR |
0x20715000 + 0x044 |
EFUSE_BASE + 0x050 |
These could affect flash ID reads and MAC address reads. They are pre-existing and out of scope for this PR, but worth tracking.
🔧 Minor nit (this PR)
src/stubs/esp32s31.json is missing a newline at the end of the file (all other stub JSON files have one).
Summary
This PR's additions are complete and correct — the WDT registers, stub file, and all integration points look solid. The pre-existing SPI_REG_BASE and EFUSE offset mismatches are the main risk for full end-to-end reliability and would be worth addressing in a follow-up PR.
Would you like me to open a follow-up issue for those pre-existing address discrepancies?
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/stubs/esp32s31.json (1)
1-8: ⚡ Quick winAdd stub provenance metadata for traceability.
Please add source metadata (repo URL + commit/tag) to this JSON so future stub audits don’t depend on PR archaeology.
♻️ Suggested update
{ + "_source": "https://github.com/Jason2866/esptool-legacy-flasher-stub/tree/g3gg0", + "_source_commit": "<commit-sha-or-tag>", "entry": 788533248, "text": "ARG3pwAvN9cBLwbOIswmykrIk4cHABMHR9tj6ucE7wCQaRMH9f+FRyqEY+TnAO8QQEABRe8AMHqBRQFF7wBQcyKF7wCwaLdXQUmTh/eEkUUoAD7EhUTBKAlJJSxjHZUAaABhJLJF7wBQDX0s/bcjoAcAkQddt+MKJf/jEJT+7xCAN2Hd7xAAOMm/CcW31wEvI6qn2oKAt6cALyOgpwCCgLfXAS+Dp0fbEwUADIKHQREixDfUAS8TBETbBsaTBgAMHEBjCtUAkwawDWMN1QAiRLJAQQGChxMFsA2Cl", "text_start": 788533248, "data": "ECIAL0wiAS+OEwAvjhMAL44TAC+OEwAvjhMAL44TAC+OEwAvjhMAL44TAC+OEwAvjhMAL44TAC+OEwAvjhMAL44TAC+OEwAvjhMAL44TAC+OEwAvjhMAL44TAC+qGgAvthoALwIbAC8uGwAvuBsAL2IbAC+MGgAv+hsAL3YcAC+cHAAvQhoAL4ocAC9CGgAvAh0ALxIdAC8WHQAvAhsAL24dAC+CHQAv", "data_start": 788647348, "bss_start": 788570112 }Based on learnings: Stubs in
src/stubs/*.jsonare sourced fromhttps://github.com/Jason2866/esptool-legacy-flasher-stub/tree/g3gg0; verify provenance against that fork and note source changes.🤖 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/stubs/esp32s31.json` around lines 1 - 8, The stub JSON (locatable by the top-level keys like "entry" and "text") lacks provenance metadata; add top-level source fields such as source_repo (https://github.com/Jason2866/esptool-legacy-flasher-stub/tree/g3gg0), source_commit_or_tag (the commit hash or tag used), source_path (original file path in the repo), retrieved_date, and a brief source_notes entry describing any modifications made to the stub bytes; ensure the values reflect verification against that fork/commit and include the commit/tag actually used.
🤖 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.
Nitpick comments:
In `@src/stubs/esp32s31.json`:
- Around line 1-8: The stub JSON (locatable by the top-level keys like "entry"
and "text") lacks provenance metadata; add top-level source fields such as
source_repo
(https://github.com/Jason2866/esptool-legacy-flasher-stub/tree/g3gg0),
source_commit_or_tag (the commit hash or tag used), source_path (original file
path in the repo), retrieved_date, and a brief source_notes entry describing any
modifications made to the stub bytes; ensure the values reflect verification
against that fork/commit and include the commit/tag actually used.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 3fdf8f19-0e86-4946-b440-9d8d7499e3b3
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (4)
src/const.tssrc/esp_loader.tssrc/stubs/esp32s31.jsonsrc/stubs/index.ts
Summary by CodeRabbit
New Features
Chores