Skip to content

[Bugfix] Set missing build flag for platformio builds#418

Open
h2zero wants to merge 1 commit intomasterfrom
bugfix/pio-build
Open

[Bugfix] Set missing build flag for platformio builds#418
h2zero wants to merge 1 commit intomasterfrom
bugfix/pio-build

Conversation

@h2zero
Copy link
Copy Markdown
Owner

@h2zero h2zero commented Mar 29, 2026

Fixes #407

Summary by CodeRabbit

  • Chores
    • Updated library build configuration with new compiler flags to enhance the build process and improve compilation support.

@coderabbitai
Copy link
Copy Markdown

coderabbitai bot commented Mar 29, 2026

📝 Walkthrough

Walkthrough

Arr, a new build configuration block be added to library.json, introducin' a flags array with the compiler define -DCONFIG_NIMBLE_CPP_IDF=1. Naught else be changin', just this fine addition to set sail with proper build flags, yarrr!

Changes

Cohort / File(s) Summary
Build Configuration
library.json
Added top-level build object with flags array containin' the compiler define -DCONFIG_NIMBLE_CPP_IDF=1 fer NimBLE C++ ESP-IDF configuration.

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~3 minutes

Poem

⚓ A single flag be set upon the sea,
Configure NimBLE with C++ spree,
Five lines added to chart our course true,
ESP-IDF be knowin' what to do! 🏴‍☠️

🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately describes the main change: adding a missing build flag for platformio builds to the library.json configuration.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch bugfix/pio-build

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

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

Copy link
Copy Markdown

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@library.json`:
- Around line 12-16: Remove the global -DCONFIG_NIMBLE_CPP_IDF=1 from
library.json and instead only define CONFIG_NIMBLE_CPP_IDF for true ESP-IDF
builds (e.g., move the flag into the ESP-IDF-specific build environment or rely
on the IDF's own config), and then fix the fragile preprocessor expressions that
mix && and || by adding explicit parentheses: update the conditional in
src/NimBLEClient.cpp (the preprocessor condition around the client feature at
~line 553) and the conditional in src/NimBLEServer.cpp (the server feature at
~line 1075) to parenthesize the && and || groups so they evaluate correctly when
CONFIG_NIMBLE_CPP_IDF is defined or not; also ensure NimBLEDevice.cpp still uses
the non-IDF header branch when the IDF define is not present.
🪄 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

Run ID: e89e79f1-0a23-4e8c-8ec5-5a53ae8ce78d

📥 Commits

Reviewing files that changed from the base of the PR and between dd32365 and 1c06afd.

📒 Files selected for processing (1)
  • library.json

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.

ble_gap.h no such file

1 participant