Skip to content

chore: remove dead main.c code and share the BLE config constants - #79

Merged
jamesarich merged 2 commits into
masterfrom
chore/main-cleanup
Oct 25, 2026
Merged

jamesarich merged 2 commits into
masterfrom
chore/main-cleanup

Conversation

@jamesarich

@jamesarich jamesarich commented Oct 24, 2026 •

Copy link
Copy Markdown
Collaborator

Checklist

  • PR title specifically describes the change
  • tools/build_all.py (or CI's board matrix) passes

Description of Change

Removes the dead _sd_inited assignment and first guard (it stays live after an OTA session), the unused bootloader_mbr_addrs_populate, and moves the BLE event length, MTU and connection config constants into one header. 16 B smaller, no behavior change.

Summary by CodeRabbit

  • Improvements
    • Bootloader images are approximately 3.3 KB smaller, helping reduce flash usage.
    • BLE firmware updates size data packets according to the configured ATT MTU limit.
    • OTA startup now initializes the SoftDevice unconditionally.
  • Documentation
    • Updated bootloader notes to clarify that certain operations must complete within the watchdog period.
    • Added a changelog entry describing the smaller bootloader images.

@coderabbitai

coderabbitai Bot commented Oct 24, 2026 •

Copy link
Copy Markdown

Review in Change Stack →

Warning

Review limit reached

You'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 16 minutes.

Check out review usage here.

View limit details

Limit details: You’ve used the included review currently available.

Learn how review limits work.

Review configuration:

Run configuration
  • Source mode: Auto
  • Repository: Unavailable
  • Branch: Unavailable
  • File: No configuration file from this repository applied
  • Commit: Unavailable
  • Layers: Organization UI
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: 91088f72-ac1c-4c7e-8807-29029db7c6ab
Commits

Reviewing files that changed from the base of the PR and between 9327eb2 and 94410c3.

Files selected for processing: 8
  • AGENTS.md
  • lib/sdk11/components/ble/ble_services/ble_dfu/ble_dfu.c
  • lib/sdk11/components/libraries/bootloader_dfu/bootloader.c
  • lib/sdk11/components/libraries/bootloader_dfu/bootloader.h
  • lib/sdk11/components/libraries/bootloader_dfu/bootloader_settings.c
  • lib/sdk11/components/libraries/bootloader_dfu/dfu_transport_ble.c
  • src/ble_dfu_cfg.h
  • src/main.c

No actionable comments were generated in the recent review.

Recent review info
Run configuration
  • Source mode: Auto
  • Repository: Unavailable
  • Branch: Unavailable
  • File: No configuration file from this repository applied
  • Commit: Unavailable
  • Layers: Organization UI
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: 08ae24f6-e37d-4189-98e6-38df02fb421e
Commits

Reviewing files that changed from the base of the PR and between 22f4736 and 9327eb2.

Files selected for processing: 10
  • AGENTS.md
  • Makefile
  • changelog.md
  • lib/sdk11/components/ble/ble_services/ble_dfu/ble_dfu.c
  • lib/sdk11/components/libraries/bootloader_dfu/bootloader.c
  • lib/sdk11/components/libraries/bootloader_dfu/bootloader.h
  • lib/sdk11/components/libraries/bootloader_dfu/bootloader_settings.c
  • lib/sdk11/components/libraries/bootloader_dfu/dfu_transport_ble.c
  • src/ble_dfu_cfg.h
  • src/main.c
Files with no reviewable changes: 2
  • lib/sdk11/components/libraries/bootloader_dfu/bootloader.h
  • lib/sdk11/components/libraries/bootloader_dfu/bootloader_settings.c

Included review availability: This review used your included allowance. Your plan provides up to 1 included review per hour; 0 remain after this review.

Walkthrough

The changes centralize BLE configuration, update the OTA initialization path, remove bootloader address-population code, and enable link-time optimization in the build.

Changes: BLE configuration

Layer / File(s) Summary
Shared BLE settings and consumers
src/ble_dfu_cfg.h, lib/sdk11/components/ble/ble_services/ble_dfu/ble_dfu.c, lib/sdk11/components/libraries/bootloader_dfu/dfu_transport_ble.c, src/main.c
The shared header defines BLE settings used by the DFU service, BLE transport, and main module. The DFU packet length now uses BLEGATT_ATT_MTU_MAX - 3.

OTA bootloader changes

Layer / File(s) Summary
Remove MBR address population
lib/sdk11/components/libraries/bootloader_dfu/bootloader.h, lib/sdk11/components/libraries/bootloader_dfu/bootloader_settings.c, src/main.c
The bootloader_mbr_addrs_populate function, its public declaration, and a commented-out call were removed.
OTA initialization and watchdog notes
src/main.c, lib/sdk11/components/libraries/bootloader_dfu/bootloader.c
The OTA path now calls mbr_init_sd() unconditionally. A prior _sd_inited assignment was removed. Comments describe the persistent 90-second watchdog constraint.

Build optimization

Layer / File(s) Summary
Enable and document LTO
Makefile, changelog.md, AGENTS.md
The compiler flags add -flto. The changelog records the reported flash-size change, and the GCC guidance no longer mentions the diagnostic pragma workaround.

Priority: Low

Estimated code review effort: 3 (Moderate) | ~20 minutes

Change: Refactor

Suggested reviewers: rcarteraz

Merge Risk: Minimal · up to 9327e

No merge-blocking issue is established; the change is ready for normal build and board-matrix checks.

Pre-merge checks: Passed 4 · Failed 1

Failed Failed checks: 1 warning

Check name Status Explanation Resolution
Docstring Coverage Warning 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 4 functions across 5 files. (3 skipped: 3… Write docstrings for the functions missing them to satisfy the coverage threshold.
Passed Passed checks: 4 passed
Check name Status Explanation
Description Check Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check Passed The title clearly summarizes the main changes: removing unused code from main.c and centralizing shared BLE configuration constants.
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.
Full details: Docstring Coverage

Explanation

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 4 functions across 5 files. (3 skipped: 3 unsupported.)

Resolution

Write docstrings for the functions missing them to satisfy the coverage threshold.

Finishing Touches: 💡 1
⚔️ Resolve merge conflicts: 💡
  • Resolve merge conflict in branch chore/main-cleanup
  • 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.

❤️ Share

A rabbit checks the build flags bright,
Then hops through BLE settings right.
The OTA path gets one clear call,
Old address writes no longer stall.
Three kilobytes, the notes proclaim,
And carrots cheer the smaller frame.

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

@jamesarich
jamesarich changed the base branch from master to build/lto October 24, 2026 11:55
@jamesarich

Copy link
Copy Markdown
Collaborator Author

@coderabbitai review

@coderabbitai

coderabbitai Bot commented Oct 25, 2026 •

Copy link
Copy Markdown
⚠️ Action not completed

Review rate limited.

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.

Base automatically changed from build/lto to master October 25, 2026 22:40
@jamesarich
jamesarich added this pull request to the merge queue Oct 25, 2026
Merged via the queue into master with commit 2a04c9c Oct 25, 2026
23 checks passed
@jamesarich
jamesarich deleted the chore/main-cleanup branch October 25, 2026 23:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant