fix(FrFTL): correct flash page state tracking to prevent unnecessary wear and improve error handling - #7584
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughFrFTL centralizes erased-page detection, records successful partial erases, validates translation-table loads and page-info reads, and resolves at least 16 translation-table pages during writes and trims. Initialization spacing is adjusted without logic changes. ChangesFrFTL state and maintenance
Estimated code review effort: 2 (Simple) | ~10 minutes 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 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 `@radio/src/drivers/frftl.cpp`:
- Around line 162-166: Remove the redundant isFlashErased probe from the
ERASE_REQUIRED branch in setPhysicalPageState. Reuse the result already obtained
by resolveUnknownState, or introduce a setter variant that accepts the known
erase status, while preserving the existing ERASED transition behavior.
🪄 Autofix (Beta)
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: 1df9361c-bbb6-4e3f-9b1d-b05aaa122415
📒 Files selected for processing (1)
radio/src/drivers/frftl.cpp
a847f68 to
0bc0bf5
Compare
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 `@radio/src/thirdparty/stb`:
- Line 1: Update the radio/src/thirdparty/stb submodule checkout to commit
5c205738, matching the repository’s committed gitlink, and ensure the updated
submodule state is staged and pushed consistently.
🪄 Autofix (Beta)
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: 633d4080-41d4-4e73-a3d4-481e3c20efce
📒 Files selected for processing (2)
radio/src/drivers/frftl.cppradio/src/thirdparty/stb
0bc0bf5 to
f65aae1
Compare
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 `@radio/src/drivers/frftl.cpp`:
- Around line 701-711: Update lockTTPages so locking and staging
translation-table buffers is failure atomic: when ttPageNo > 0, load and
validate the master page before mutating the initially loaded buffer, or
preserve and restore its prior state if the master load fails. Ensure any false
return leaves both buffers unchanged while retaining the existing successful
locking behavior.
🪄 Autofix (Beta)
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: c4d3a36a-04db-4b96-9f8c-91898ba0e567
📒 Files selected for processing (1)
radio/src/drivers/frftl.cpp
|
Note Docstrings generation - SUCCESS |
Docstrings generation was requested by @coderabbitai[bot]. * #7584 (comment) The following files were modified: * `radio/src/drivers/frftl.cpp`
2b3445b to
f732f2d
Compare
|
@pfeerick How come code rabbit will automatically generate docstrings for me? |
f732f2d to
f0ec8db
Compare
|
This can merge now, already did extensive load test and everything is intact. |
…wear and improve error handling (#7584)
…wear and improve error handling (#7584)
This fix aligned to the changes in the standalone FTL project: https://github.com/richardclli/Flash-Translation-Layer
The changes already tested against heavy workload of FAT file access and modification based on a set of EdgeTX SDcard images for 1000 rounds to ensure integrity. The testing code is in the standalone project.
Summary by CodeRabbit