Skip to content

NimBLELog Timestamps#1166

Closed
vlucent wants to merge 3 commits into
h2zero:masterfrom
vlucent:timestamps
Closed

NimBLELog Timestamps#1166
vlucent wants to merge 3 commits into
h2zero:masterfrom
vlucent:timestamps

Conversation

@vlucent

@vlucent vlucent commented Jun 29, 2026

Copy link
Copy Markdown
Contributor

adds timestamps to NimBLELog messages :)

Summary by CodeRabbit

  • Bug Fixes
    • Fixed an issue where tick-to-millisecond conversion could recurse incorrectly, ensuring elapsed time values are now converted properly.
  • Improvements
    • Enhanced logging: Arduino logs now include an elapsed timestamp and added a critical-level log macro.
    • Enabled formatted host logging output when log levels permit it, and wired standard log macros to the updated logger.
    • Updated connection flow debugging to emit additional log messages.
  • Configuration
    • Set explicit default logging levels to control which log messages are active.

@coderabbitai

coderabbitai Bot commented Jun 29, 2026

Copy link
Copy Markdown

Review Change Stack

📝 Walkthrough

Walkthrough

Arrr, the NimBLE logs now carry millis() timestamps through Arduino and Freertos logging paths, extra connect debug prints were added, and tick-to-millisecond conversion now calls the proper Freertos helper.

Changes

Logging output updates

Layer / File(s) Summary
Logging level configuration
src/nimconfig.h
MYNEWT_VAL_BLE_HS_LOG_LVL and MYNEWT_VAL_NIMBLE_CPP_LOG_LEVEL are actively defined.
Arduino log macros
src/NimBLELog.h
The Arduino branch includes <Arduino.h>, prefixes enabled log output with millis(), and adds NIMBLE_LOGC.
Freertos log backend and connect debug
src/nimble/porting/npl/freertos/include/nimble/nimble_npl_os_log.h, src/nimble/nimble/host/src/ble_gap.c
The non-ESP_PLATFORM log backend now prints formatted output via vprintf, and ble_gap_connect(...) emits added debug lines in the central-connect flow.

Tick conversion fix

Layer / File(s) Summary
Tick-to-ms delegate
src/nimble/porting/npl/freertos/include/nimble/nimble_npl_os.h
ble_npl_time_ticks_to_ms(...) now returns npl_freertos_time_ticks_to_ms(ticks, out_ms) instead of calling itself.

Sequence Diagram(s)

sequenceDiagram
  participant ble_gap_connect
  participant BLE_HS_LOG
  participant NIMBLE_LOG
  participant vprintf
  ble_gap_connect->>BLE_HS_LOG: emit connect debug text
  BLE_HS_LOG->>NIMBLE_LOG: map to enabled log macro
  NIMBLE_LOG->>vprintf: print formatted line
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Poem

Arrr, the logs now sparkle with time,
And connect-path chatter marks the climb.
One helper sails straight, no loop in sight,
A tidy patch under starlit night.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ 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 matches the main change: adding timestamps to NimBLE log output.
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.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Actionable comments posted: 7

🤖 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 `@src/nimble/nimble/host/src/ble_gap.c`:
- Line 5894: Remove the stray debug printf from the BLE gap logging path in
ble_gap.c: the leftover printf between the log header and ble_gap_log_conn()
should be deleted so it does not corrupt the intended log output. Locate the
code in the connection logging flow around ble_gap_log_conn and keep only the
existing log header and structured logging call.
- Around line 5812-5813: Remove the leftover debug output from the connect path
in the BLE GAP connect function, since the `printf` and `BLE_HS_LOG(INFO, ...)`
calls are still executed before the `BLE_ROLE_CENTRAL` guard and can run even
when the function later returns `BLE_HS_ENOTSUP`. Delete both debug statements
from that path so only the intended connect logic remains.

In `@src/nimble/porting/npl/freertos/include/nimble/nimble_npl_os_log.h`:
- Around line 81-109: The BLE_HS_LOG_* macros in nimble_npl_os_log.h use an
undefined tag identifier in every disabled-level fallback, which will break
compilation wherever these macros are expanded. Update the `#else` branches for
BLE_HS_LOG_CRITICAL, BLE_HS_LOG_ERROR, BLE_HS_LOG_WARN, BLE_HS_LOG_INFO, and
BLE_HS_LOG_DEBUG to use a harmless no-op expression instead of referencing tag,
keeping the existing logging macro names and level guards intact.
- Line 71: Remove the unconditional debug printf from the generated NimBLE log
path in nimble_npl_os_log.h; the log macro/function should not emit anything
before the level check. Locate the generated logging wrapper in the NPL OS log
header and delete the printf("OH SO YOU MADE IT!!!\n") call so logging only
happens through the existing severity-gated code path.
- Line 72: The non-ESP logging path in nimble_npl_os_log.h still uses
_BLE_LOG_LEVEL_VALUE(lvl) even though that macro is only defined under
ESP_PLATFORM, so this branch will fail to compile. Update the logging macro
expansion in the non-ESP path of the affected log helper to either define the
same level-mapping macro there or switch the comparison to the raw LOG_LEVEL_*
constants used by the NimBLE log wrappers.

In `@src/nimconfig.h`:
- Line 109: The NimBLE CPP wrapper log level is being force-enabled at DEBUG by
the MYNEWT_VAL_NIMBLE_CPP_LOG_LEVEL definition in nimconfig.h. Revert this
default change by keeping that macro commented out unless enabling it
intentionally is required, so the NimBLE Cpp wrapper configuration does not
impose verbose logging on all consumers.
- Line 21: The host log level default is being forced on in the configuration
header, which makes the library always build with maximum BLE HS logging. Revert
the MYNEWT_VAL_BLE_HS_LOG_LVL setting in nimconfig.h to a commented-out user
opt-in default so consumers can choose it explicitly, and leave the surrounding
configuration pattern consistent with the other MYNEWT_VAL_* options.
🪄 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: afa996b7-000e-4cf7-becf-492ab303b3fc

📥 Commits

Reviewing files that changed from the base of the PR and between 505c190 and 219707d.

📒 Files selected for processing (4)
  • src/NimBLELog.h
  • src/nimble/nimble/host/src/ble_gap.c
  • src/nimble/porting/npl/freertos/include/nimble/nimble_npl_os_log.h
  • src/nimconfig.h
🚧 Files skipped from review as they are similar to previous changes (1)
  • src/NimBLELog.h

Comment thread src/nimble/nimble/host/src/ble_gap.c Outdated
Comment on lines +5812 to +5813
printf("HIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIII\n");
BLE_HS_LOG(INFO, "YOUUUUUUUUUUUUUUUUUUUUUUUUUUUUUUUUUU\n");

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

📐 Maintainability & Code Quality | 🔴 Critical | ⚡ Quick win

Arr, scrub these debug prints from the connect path before they board master!

printf("HIIII...") and BLE_HS_LOG(INFO, "YOUUU...") be obvious debug leftovers. They also sit above the #if MYNEWT_VAL(BLE_ROLE_CENTRAL) guard, so they fire even when central role is disabled and the function returns BLE_HS_ENOTSUP. Remove 'em.

🏴‍☠️ Suggested fix
 {
-printf("HIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIII\n");
-BLE_HS_LOG(INFO, "YOUUUUUUUUUUUUUUUUUUUUUUUUUUUUUUUUUU\n");
 `#if` MYNEWT_VAL(BLE_ROLE_CENTRAL)
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
printf("HIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIIII\n");
BLE_HS_LOG(INFO, "YOUUUUUUUUUUUUUUUUUUUUUUUUUUUUUUUUUU\n");
🤖 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/nimble/nimble/host/src/ble_gap.c` around lines 5812 - 5813, Remove the
leftover debug output from the connect path in the BLE GAP connect function,
since the `printf` and `BLE_HS_LOG(INFO, ...)` calls are still executed before
the `BLE_ROLE_CENTRAL` guard and can run even when the function later returns
`BLE_HS_ENOTSUP`. Delete both debug statements from that path so only the
intended connect logic remains.

Comment thread src/nimble/nimble/host/src/ble_gap.c Outdated
}

BLE_HS_LOG(INFO, "GAP procedure initiated: connect; ");
printf("HHIIIIIIIIIIIIIIIIIIIIIII");

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

📐 Maintainability & Code Quality | 🔴 Critical | ⚡ Quick win

Another stowaway printf — toss it overboard.

printf("HHIIIIIIIIIIIIIIIIIIIIIII") is a debug leftover wedged between the log header and ble_gap_log_conn(...) (and it lacks a newline, garbling the intended log line). Remove it.

🏴‍☠️ Suggested fix
     BLE_HS_LOG(INFO, "GAP procedure initiated: connect; ");
-    printf("HHIIIIIIIIIIIIIIIIIIIIIII");
     ble_gap_log_conn(own_addr_type, peer_addr, conn_params);
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
printf("HHIIIIIIIIIIIIIIIIIIIIIII");
🤖 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/nimble/nimble/host/src/ble_gap.c` at line 5894, Remove the stray debug
printf from the BLE gap logging path in ble_gap.c: the leftover printf between
the log header and ble_gap_log_conn() should be deleted so it does not corrupt
the intended log output. Locate the code in the connection logging flow around
ble_gap_log_conn and keep only the existing log header and structured logging
call.

static inline void _BLE_NPL_LOG_CAT( \
BLE_NPL_LOG_MODULE, _BLE_NPL_LOG_CAT(_, lvl))(const char *fmt, ...) \
{ \
printf("OH SO YOU MADE IT!!!\n"); \

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🩺 Stability & Availability | 🔴 Critical | ⚡ Quick win

Walk this plank! A debug printf fires on EVERY single log call.

printf("OH SO YOU MADE IT!!!\n") runs unconditionally inside the generated log function, before the level check — it'll flood the console no matter the configured level. Strike it from the deck before merging.

🏴‍☠️ Suggested fix
     {                                                                        \
-        printf("OH SO YOU MADE IT!!!\n");                                    \
         if (MYNEWT_VAL_BLE_HS_LOG_LVL <= _BLE_LOG_LEVEL_VALUE(lvl)) {       \
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
printf("OH SO YOU MADE IT!!!\n"); \
🤖 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/nimble/porting/npl/freertos/include/nimble/nimble_npl_os_log.h` at line
71, Remove the unconditional debug printf from the generated NimBLE log path in
nimble_npl_os_log.h; the log macro/function should not emit anything before the
level check. Locate the generated logging wrapper in the NPL OS log header and
delete the printf("OH SO YOU MADE IT!!!\n") call so logging only happens through
the existing severity-gated code path.

BLE_NPL_LOG_MODULE, _BLE_NPL_LOG_CAT(_, lvl))(const char *fmt, ...) \
{ \
printf("OH SO YOU MADE IT!!!\n"); \
if (MYNEWT_VAL_BLE_HS_LOG_LVL <= _BLE_LOG_LEVEL_VALUE(lvl)) { \

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🩺 Stability & Availability | 🟠 Major | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Hunt for _BLE_LOG_LEVEL_VALUE and LOG_LEVEL_* visibility outside the ESP block
rg -nP '_BLE_LOG_LEVEL_VALUE(_[A-Z]+)?\b' src/nimble/porting/npl/freertos/include/nimble/nimble_npl_os_log.h
echo '--- search other headers possibly included on non-ESP path ---'
fd -t f 'syscfg.h' src | xargs -r rg -nP '_BLE_LOG_LEVEL_VALUE|LOG_LEVEL_DEBUG' 
fd -t f 'NimBLELog.h' src | xargs -r rg -nP '_BLE_LOG_LEVEL_VALUE|LOG_LEVEL_DEBUG'

Repository: h2zero/NimBLE-Arduino

Length of output: 740


🏁 Script executed:

#!/bin/bash
set -euo pipefail

file='src/nimble/porting/npl/freertos/include/nimble/nimble_npl_os_log.h'

echo '--- outline ---'
ast-grep outline "$file" --view expanded || true

echo '--- first 140 lines ---'
sed -n '1,140p' "$file" | cat -n

echo '--- search for MYNEWT_VAL_BLE_HS_LOG_LVL and _BLE_LOG_LEVEL_VALUE definitions/usages ---'
rg -n '_BLE_LOG_LEVEL_VALUE|MYNEWT_VAL_BLE_HS_LOG_LVL|LOG_LEVEL_' "$file" src/nimble -g '!**/build/**'

Repository: h2zero/NimBLE-Arduino

Length of output: 11612


🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo '--- direct definitions/usages of _BLE_LOG_LEVEL_VALUE across repo ---'
rg -n '_BLE_LOG_LEVEL_VALUE\b' src

echo '--- inspect NimBLELog.h ---'
file='src/nimble/porting/nimble/include/NimBLELog.h'
if [ -f "$file" ]; then
  sed -n '1,220p' "$file" | cat -n
else
  echo "missing: $file"
fi

echo '--- inspect syscfg.h candidates ---'
fd -t f 'syscfg.h' src | while read -r f; do
  echo "--- $f ---"
  sed -n '1,220p' "$f" | cat -n
done

Repository: h2zero/NimBLE-Arduino

Length of output: 7708


Define _BLE_LOG_LEVEL_VALUE on the non-ESP path Arrr, this branch still expands _BLE_LOG_LEVEL_VALUE(lvl), but that macro is only declared inside #ifdef ESP_PLATFORM above. I couldn’t find another definition in the repo, so this path will break the build unless ye add the mapping here or switch to the raw LOG_LEVEL_* constants.

🤖 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/nimble/porting/npl/freertos/include/nimble/nimble_npl_os_log.h` at line
72, The non-ESP logging path in nimble_npl_os_log.h still uses
_BLE_LOG_LEVEL_VALUE(lvl) even though that macro is only defined under
ESP_PLATFORM, so this branch will fail to compile. Update the logging macro
expansion in the non-ESP path of the affected log helper to either define the
same level-mapping macro there or switch the comparison to the raw LOG_LEVEL_*
constants used by the NimBLE log wrappers.

Source: Linters/SAST tools

Comment on lines +81 to +109
# if MYNEWT_VAL_BLE_HS_LOG_LVL <= 4
# define BLE_HS_LOG_CRITICAL(...) NIMBLE_LOGC(BLE_NPL_LOG_MODULE, __VA_ARGS__)
# else
# define BLE_HS_LOG_CRITICAL(...) (void)tag
# endif

# if MYNEWT_VAL_BLE_HS_LOG_LVL <= 3
# define BLE_HS_LOG_ERROR(...) NIMBLE_LOGE(BLE_NPL_LOG_MODULE, __VA_ARGS__)
# else
# define BLE_HS_LOG_ERROR(...) (void)tag
# endif

# if MYNEWT_VAL_BLE_HS_LOG_LVL <= 2
# define BLE_HS_LOG_WARN(...) NIMBLE_LOGW(BLE_NPL_LOG_MODULE, __VA_ARGS__)
# else
# define BLE_HS_LOG_WARN(...) (void)tag
# endif

# if MYNEWT_VAL_BLE_HS_LOG_LVL <= 1
# define BLE_HS_LOG_INFO(...) NIMBLE_LOGI(BLE_NPL_LOG_MODULE, __VA_ARGS__)
# else
# define BLE_HS_LOG_INFO(...) (void)tag
# endif

# if MYNEWT_VAL_BLE_HS_LOG_LVL <= 0
# define BLE_HS_LOG_DEBUG(...) NIMBLE_LOGD(BLE_NPL_LOG_MODULE, __VA_ARGS__)
# else
# define BLE_HS_LOG_DEBUG(...) (void)tag
# endif

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🩺 Stability & Availability | 🔴 Critical | ⚡ Quick win

Belay that — the disabled-level branches expand to (void)tag, but there be no tag in sight.

Each #else fallback (Lines 84, 90, 96, 102, 108) maps to (void)tag, yet no tag identifier is declared at these call sites — any disabled level will trigger an "undeclared identifier" compile error wherever BLE_HS_LOG_* is used. Use a harmless no-op like (void)0 instead.

🏴‍☠️ Suggested fix (apply to each disabled branch)
-    #   define BLE_HS_LOG_CRITICAL(...) (void)tag
+    #   define BLE_HS_LOG_CRITICAL(...) (void)0
...
-    #   define BLE_HS_LOG_ERROR(...) (void)tag
+    #   define BLE_HS_LOG_ERROR(...) (void)0
...
-    #   define BLE_HS_LOG_WARN(...) (void)tag
+    #   define BLE_HS_LOG_WARN(...) (void)0
...
-    #   define BLE_HS_LOG_INFO(...) (void)tag
+    #   define BLE_HS_LOG_INFO(...) (void)0
...
-    #   define BLE_HS_LOG_DEBUG(...) (void)tag
+    #   define BLE_HS_LOG_DEBUG(...) (void)0
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
# if MYNEWT_VAL_BLE_HS_LOG_LVL <= 4
# define BLE_HS_LOG_CRITICAL(...) NIMBLE_LOGC(BLE_NPL_LOG_MODULE, __VA_ARGS__)
# else
# define BLE_HS_LOG_CRITICAL(...) (void)tag
# endif
# if MYNEWT_VAL_BLE_HS_LOG_LVL <= 3
# define BLE_HS_LOG_ERROR(...) NIMBLE_LOGE(BLE_NPL_LOG_MODULE, __VA_ARGS__)
# else
# define BLE_HS_LOG_ERROR(...) (void)tag
# endif
# if MYNEWT_VAL_BLE_HS_LOG_LVL <= 2
# define BLE_HS_LOG_WARN(...) NIMBLE_LOGW(BLE_NPL_LOG_MODULE, __VA_ARGS__)
# else
# define BLE_HS_LOG_WARN(...) (void)tag
# endif
# if MYNEWT_VAL_BLE_HS_LOG_LVL <= 1
# define BLE_HS_LOG_INFO(...) NIMBLE_LOGI(BLE_NPL_LOG_MODULE, __VA_ARGS__)
# else
# define BLE_HS_LOG_INFO(...) (void)tag
# endif
# if MYNEWT_VAL_BLE_HS_LOG_LVL <= 0
# define BLE_HS_LOG_DEBUG(...) NIMBLE_LOGD(BLE_NPL_LOG_MODULE, __VA_ARGS__)
# else
# define BLE_HS_LOG_DEBUG(...) (void)tag
# endif
# if MYNEWT_VAL_BLE_HS_LOG_LVL <= 4
# define BLE_HS_LOG_CRITICAL(...) NIMBLE_LOGC(BLE_NPL_LOG_MODULE, __VA_ARGS__)
# else
# define BLE_HS_LOG_CRITICAL(...) (void)0
# endif
# if MYNEWT_VAL_BLE_HS_LOG_LVL <= 3
# define BLE_HS_LOG_ERROR(...) NIMBLE_LOGE(BLE_NPL_LOG_MODULE, __VA_ARGS__)
# else
# define BLE_HS_LOG_ERROR(...) (void)0
# endif
# if MYNEWT_VAL_BLE_HS_LOG_LVL <= 2
# define BLE_HS_LOG_WARN(...) NIMBLE_LOGW(BLE_NPL_LOG_MODULE, __VA_ARGS__)
# else
# define BLE_HS_LOG_WARN(...) (void)0
# endif
# if MYNEWT_VAL_BLE_HS_LOG_LVL <= 1
# define BLE_HS_LOG_INFO(...) NIMBLE_LOGI(BLE_NPL_LOG_MODULE, __VA_ARGS__)
# else
# define BLE_HS_LOG_INFO(...) (void)0
# endif
# if MYNEWT_VAL_BLE_HS_LOG_LVL <= 0
# define BLE_HS_LOG_DEBUG(...) NIMBLE_LOGD(BLE_NPL_LOG_MODULE, __VA_ARGS__)
# else
# define BLE_HS_LOG_DEBUG(...) (void)0
# endif
🤖 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/nimble/porting/npl/freertos/include/nimble/nimble_npl_os_log.h` around
lines 81 - 109, The BLE_HS_LOG_* macros in nimble_npl_os_log.h use an undefined
tag identifier in every disabled-level fallback, which will break compilation
wherever these macros are expanded. Update the `#else` branches for
BLE_HS_LOG_CRITICAL, BLE_HS_LOG_ERROR, BLE_HS_LOG_WARN, BLE_HS_LOG_INFO, and
BLE_HS_LOG_DEBUG to use a harmless no-op expression instead of referencing tag,
keeping the existing logging macro names and level guards intact.

Comment thread src/nimconfig.h Outdated
* Uses approx. 32kB of flash memory.
*/
// #define MYNEWT_VAL_BLE_HS_LOG_LVL 5
#define MYNEWT_VAL_BLE_HS_LOG_LVL 0

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win

Avast! Ye've un-commented the host log level to full DEBUG fer every soul who sails with this library.

MYNEWT_VAL_BLE_HS_LOG_LVL was meant to stay commented (user-opt-in). Hard-setting it to 0 forces maximum verbosity and ~32kB of flash on all consumers — looks like a debug leftover from this voyage. Restore it to a commented default unless this is truly intended, arr.

🏴‍☠️ Suggested fix
-#define MYNEWT_VAL_BLE_HS_LOG_LVL 0
+// `#define` MYNEWT_VAL_BLE_HS_LOG_LVL 5
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
#define MYNEWT_VAL_BLE_HS_LOG_LVL 0
// `#define` MYNEWT_VAL_BLE_HS_LOG_LVL 5
🤖 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/nimconfig.h` at line 21, The host log level default is being forced on in
the configuration header, which makes the library always build with maximum BLE
HS logging. Revert the MYNEWT_VAL_BLE_HS_LOG_LVL setting in nimconfig.h to a
commented-out user opt-in default so consumers can choose it explicitly, and
leave the surrounding configuration pattern consistent with the other
MYNEWT_VAL_* options.

Comment thread src/nimconfig.h Outdated
* Uses approx. 32kB of flash memory.
*/
// #define MYNEWT_VAL_NIMBLE_CPP_LOG_LEVEL 0
#define MYNEWT_VAL_NIMBLE_CPP_LOG_LEVEL 4

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win

Same treasure-trap here, matey — the CPP wrapper log level be force-enabled to DEBUG.

MYNEWT_VAL_NIMBLE_CPP_LOG_LEVEL was previously commented; setting it active to 4 (DEBUG) saddles every consumer with verbose logs and extra flash. Keep it commented unless this default change is deliberate.

🏴‍☠️ Suggested fix
-#define MYNEWT_VAL_NIMBLE_CPP_LOG_LEVEL 4
+// `#define` MYNEWT_VAL_NIMBLE_CPP_LOG_LEVEL 0
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
#define MYNEWT_VAL_NIMBLE_CPP_LOG_LEVEL 4
// `#define` MYNEWT_VAL_NIMBLE_CPP_LOG_LEVEL 0
🤖 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/nimconfig.h` at line 109, The NimBLE CPP wrapper log level is being
force-enabled at DEBUG by the MYNEWT_VAL_NIMBLE_CPP_LOG_LEVEL definition in
nimconfig.h. Revert this default change by keeping that macro commented out
unless enabling it intentionally is required, so the NimBLE Cpp wrapper
configuration does not impose verbose logging on all consumers.

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