Repository navigation
chore: drop the 64-bit division and keep INFO_UF2.TXT text in flash only - #74
Conversation
|
Warning Review limit reachedYou've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. Next included review available in 59 minutes. View limit details
No actionable comments were generated in the recent review.
 |
| Layer / File(s) | Summary |
|---|---|
Convert serial DFU timeout lib/sdk11/components/libraries/bootloader_dfu/bootloader.c |
For nonzero timeouts, the timer tick calculation now uses (timeout_ms * 32768u + 500u) / 1000u instead of APP_TIMER_TICKS(timeout_ms). |
UF2 information initialization
| Layer / File(s) | Summary |
|---|---|
Initialize UF2 information buffer src/usb/uf2/ghostfat.c |
The base UF2 information text is now held in infoUf2Text and copied into infoUf2File during uf2_init() before SoftDevice information is appended. |
Priority:
Estimated code review effort: 2 (Simple) | ~10 minutes
Change: Refactor
Suggested reviewers: hathach
Merge Risk: 
ed0aa
-
No concrete merge-blocking risk remains in the reviewed changes.
Pre-merge checks: 
-
Check name Status Explanation Resolution Docstring Coverage Docstring coverage is 50.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 2 functions across 2 files. Write docstrings for the functions missing them to satisfy the coverage threshold. -
Check name Status Explanation Description Check Check skipped - CodeRabbit’s high-level summary is enabled. Title check The title names the two changes in the PR: dropping the 64-bit division in the startup-timeout calculation and keeping INFO_UF2.TXT text in flash only. The raw summary confirms both changes in bootloa… 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: Title check
-
Explanation
The title names the two changes in the PR: dropping the 64-bit division in the startup-timeout calculation and keeping INFO_UF2.TXT text in flash only. The raw summary confirms both changes in bootloader.c and ghostfat.c. The title is specific and accurate, and it follows a conventional commit style. It uses the term 'chore:' prefix, which is acceptable. The title is 72 characters, slightly above the usual 50-72 range, but it is still readable.
-
-
- 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.
A rabbit checks the timer’s pace,
Then copies text into its place.
The UF2 words are set in line,
SoftDevice details join in time.
Two small changes, the boot steps flow,
And off through fields the rabbits go.
Comment @coderabbitai help to get the list of available commands.
jamesarich
changed the title
|
@coderabbitai review |
✅ Action performed
|
APP_TIMER_TICKS is 64-bit and pulls __udivmoddi4 into the image for one call site. The 32-bit form gives the same tick count for any timeout under 131 s.
ed0aaff to
bb12eea
Compare
Checklist
tools/build_all.py(or CI's board matrix) passesDescription of Change
About 980 B smaller on every board, no behavior change.
APP_TIMER_TICKSwas the only 64-bit division in the image and pulled in__udivmoddi4; the startup timeout is now computed in 32 bits with the same rounding for any timeout under 131 s. The 384 BINFO_UF2.TXTbuffer was initialized data, so its text sat in flash and RAM; it is now built at runtime from a const string, which also makesuf2_initsafe to call twice.No 64-bit division symbols remain on any board. Tested merged on four boards: the 3 s UF2 timeout behaves as before and
INFO_UF2.TXTreads correctly.Summary by CodeRabbit