Skip to content

Add BLE stream classes.#1068

Merged
h2zero merged 1 commit intomasterfrom
stream-class
Mar 15, 2026
Merged

Add BLE stream classes.#1068
h2zero merged 1 commit intomasterfrom
stream-class

Conversation

@h2zero
Copy link
Owner

@h2zero h2zero commented Nov 18, 2025

Adds a data stream client and server classes.

The server class sends data to clients through notifications and accepts data written to the characteristic by clients.
The client writes data to the server characteristic and receives data through notifications.

Summary by CodeRabbit

  • New Features

    • Added a NimBLE Stream API for serial‑like, bidirectional BLE communication (server and client) with buffered I/O, MTU‑aware chunking, overflow handling, and automatic reconnect.
  • Documentation

    • Added three end‑to‑end examples with READMEs: stream server, echo server, and stream client demonstrating usage and testing steps.
  • Chores

    • Updated ignore rules and applied minor console/build formatting tidy-ups.

@h2zero h2zero marked this pull request as draft November 18, 2025 23:54
@h2zero h2zero force-pushed the stream-class branch 3 times, most recently from 1b7cb22 to 6a67147 Compare November 24, 2025 15:07
@h2zero h2zero force-pushed the stream-class branch 2 times, most recently from bcfcf75 to 9d832b4 Compare January 15, 2026 00:18
@coderabbitai
Copy link

coderabbitai bot commented Mar 14, 2026

Note

Reviews paused

It 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 reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review
📝 Walkthrough

Walkthrough

Yarrr: Introduces a new NimBLEStream subsystem (server + client) with ring-buffered TX/RX, MTU-aware chunking, drain/callout logic; adds three Arduino examples (Server, Client, Echo) with READMEs; small console/nimconfig preprocessor formatting tweaks; and updates .gitignore.

Changes

Cohort / File(s) Summary
Core Streaming Implementation
src/NimBLEStream.h, src/NimBLEStream.cpp, src/NimBLEDevice.h
Add NimBLEStream API and implementation: ByteRingBuffer, base NimBLEStream, NimBLEStreamServer and NimBLEStreamClient (begin/end, write/read/available, send/drain, MTU-aware chunking, TX drain events). Include NimBLEStream.h from NimBLEDevice.h.
Arduino Examples & Docs
examples/NimBLE_Stream_Server/NimBLE_Stream_Server.ino, examples/NimBLE_Stream_Server/README.md, examples/NimBLE_Stream_Client/NimBLE_Stream_Client.ino, examples/NimBLE_Stream_Client/README.md, examples/NimBLE_Stream_Echo/NimBLE_Stream_Echo.ino, examples/NimBLE_Stream_Echo/README.md
Add three Arduino sketches demonstrating server, client, and echo stream usage, RX overflow tracking, auto-discovery/connection/subscription flows, bidirectional read/write, and companion README guidance.
Console & Build Config
src/nimble/console/console.h, src/nimconfig.h
Adjust console_printf macro placement for C++ vs C expansion and apply minor preprocessor formatting edits in nimconfig.h (no behavioral change).
Repository Ignore
.gitignore
Add _codeql_detected_source_root entry to .gitignore.

Sequence Diagram(s)

sequenceDiagram
    participant App as Application
    participant Stream as NimBLEStreamServer
    participant BLE as BLE Stack
    participant Peer as Remote BLE Peer

    App->>Stream: begin(svcUUID, chrUUID)
    Stream->>BLE: register service & characteristic, set callbacks
    Peer->>BLE: connect & subscribe
    BLE->>Stream: onSubscribe -> update peer handle
    App->>Stream: write(data)
    Stream->>Stream: enqueue TX, schedule drain
    Stream->>BLE: send notification (MTU-limited chunks)
    BLE->>Peer: deliver notification
    Peer->>BLE: write characteristic (RX)
    BLE->>Stream: onWrite -> push RX into buffer
    App->>Stream: read() -> return RX data
    Peer->>BLE: disconnect
    BLE->>Stream: onDisconnect -> clear state
Loading
sequenceDiagram
    participant App as Application
    participant Stream as NimBLEStreamClient
    participant BLE as BLE Stack
    participant Server as Remote BLE Server

    App->>BLE: scan for server
    BLE->>App: report discovered server
    App->>Stream: begin(remoteChar, subscribe=true)
    Stream->>BLE: subscribe to notifications
    App->>Stream: write(data)
    Stream->>Stream: enqueue TX, schedule drain
    Stream->>BLE: write characteristic (MTU-limited)
    BLE->>Server: deliver write
    Server->>BLE: notify
    BLE->>Stream: notifyCallback -> push RX
    App->>Stream: read() -> return RX data
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Poem

Yarrr, the streams be cast o'er BLE seas,
Rings spin and MTUs carve bytes with ease,
Server, client, echo sing in tune,
Notifications dance beneath the moon,
Avast — a streaming bounty! 🏴‍☠️

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 56.10% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title 'Add BLE stream classes' directly summarizes the main change: introducing NimBLEStream, NimBLEStreamServer, and NimBLEStreamClient classes with supporting examples and documentation.

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

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch stream-class
📝 Coding Plan
  • Generate coding plan for human review comments

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

@h2zero h2zero marked this pull request as ready for review March 14, 2026 22:49
Copy link

@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: 4

🧹 Nitpick comments (2)
src/NimBLEStream.h (1)

110-111: Use {} for proper zero-initialization of aggregate types.

Initializing ble_npl_event and ble_npl_callout with {NULL} only initializes the first member and leaves remaining members uninitialized. Both are aggregate structs with multiple members that should be zero-initialized. Use {} instead for standard C++ aggregate initialization that ensures all members are properly zeroed.

♻️ Suggested change
-    ble_npl_event   m_txDrainEvent{NULL};
-    ble_npl_callout m_txDrainCallout{NULL};
+    ble_npl_event   m_txDrainEvent{};
+    ble_npl_callout m_txDrainCallout{};
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/NimBLEStream.h` around lines 110 - 111, The member initializers
m_txDrainEvent and m_txDrainCallout should use aggregate zero-initialization
instead of {NULL}; update the declarations of ble_npl_event m_txDrainEvent and
ble_npl_callout m_txDrainCallout in NimBLEStream.h to initialize with {} so all
struct members are zero-initialized (replace the current {NULL} initializers
with {} for the symbols m_txDrainEvent and m_txDrainCallout).
examples/NimBLE_Stream_Client/NimBLE_Stream_Client.ino (1)

75-77: Add a defensive null check for pServerDevice in connectToServer().

connectToServer() dereferences pServerDevice immediately (Line 76). A guard prevents accidental null dereference if call flow changes later.

🔧 Proposed fix
 bool connectToServer() {
+    if (pServerDevice == nullptr) {
+        Serial.println("No server selected");
+        return false;
+    }
     Serial.printf("Connecting to: %s\n", pServerDevice->getAddress().toString().c_str());
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@examples/NimBLE_Stream_Client/NimBLE_Stream_Client.ino` around lines 75 - 77,
The connectToServer() function dereferences pServerDevice immediately; add a
defensive null check at the start of connectToServer() to verify pServerDevice
is not null (e.g., if (pServerDevice == nullptr) { Serial.println("No server
device available"); return false; }) so you avoid null dereference, log a clear
message and return false early; update any callers if they rely on a different
return behavior.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@examples/NimBLE_Stream_Client/NimBLE_Stream_Client.ino`:
- Around line 155-162: The doConnect handling block (where doConnect is set
false and connectToServer() is called) can leave scanning stopped if onScanEnd
already ran; update the failure branch inside that block (in the same function
handling doConnect) to explicitly restart BLE scanning when connectToServer()
returns false by invoking the scan restart API used elsewhere in the code (e.g.,
the NimBLE scan start call or Scan->start()), and keep the existing logging;
reference the doConnect check and the connectToServer() call to locate where to
add the explicit scan restart.

In `@examples/NimBLE_Stream_Echo/NimBLE_Stream_Echo.ino`:
- Around line 25-26: The server is created in setup() via
NimBLEDevice::createServer() but advertising is not restarted after a client
disconnects, preventing new connections; add a BLE server callback by
subclassing NimBLEServerCallbacks (implement onDisconnect) and in onDisconnect
call the server advertising restart (e.g., server->getAdvertising()->start() or
NimBLEDevice::startAdvertising()) so advertising is resumed after disconnect;
register this callback with the server returned by NimBLEDevice::createServer()
in setup().

In `@src/NimBLEStream.cpp`:
- Around line 50-71: The destructor calls ble_npl_mutex_deinit unconditionally
even if ble_npl_mutex_init failed; add a member flag (e.g., bool
m_mutexInitialized) to ByteRingBuffer, set it true only after ble_npl_mutex_init
returns BLE_NPL_OK in the constructor, and in the ~ByteRingBuffer() only call
ble_npl_mutex_deinit(&m_mutex) when m_mutexInitialized is true (leave existing
m_buf free logic as-is).
- Around line 483-494: The pushRx implementation currently drops the whole
packet when m_rxBuf->freeSize() < len; change it to accept a partial write
instead: compute accepted = std::min(len, m_rxBuf->freeSize()), call
m_rxBuf->write(data, accepted) and return the number of bytes written;
optionally update the log to indicate a partial drop when accepted < len. Update
NimBLEStream::pushRx to use m_rxBuf->freeSize() and ByteRingBuffer::write()
accordingly.

---

Nitpick comments:
In `@examples/NimBLE_Stream_Client/NimBLE_Stream_Client.ino`:
- Around line 75-77: The connectToServer() function dereferences pServerDevice
immediately; add a defensive null check at the start of connectToServer() to
verify pServerDevice is not null (e.g., if (pServerDevice == nullptr) {
Serial.println("No server device available"); return false; }) so you avoid null
dereference, log a clear message and return false early; update any callers if
they rely on a different return behavior.

In `@src/NimBLEStream.h`:
- Around line 110-111: The member initializers m_txDrainEvent and
m_txDrainCallout should use aggregate zero-initialization instead of {NULL};
update the declarations of ble_npl_event m_txDrainEvent and ble_npl_callout
m_txDrainCallout in NimBLEStream.h to initialize with {} so all struct members
are zero-initialized (replace the current {NULL} initializers with {} for the
symbols m_txDrainEvent and m_txDrainCallout).

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 57228ec8-04c6-4beb-abb7-4aa6254cd25d

📥 Commits

Reviewing files that changed from the base of the PR and between 8576ae8 and 2abe280.

📒 Files selected for processing (12)
  • .gitignore
  • examples/NimBLE_Stream_Client/NimBLE_Stream_Client.ino
  • examples/NimBLE_Stream_Client/README.md
  • examples/NimBLE_Stream_Echo/NimBLE_Stream_Echo.ino
  • examples/NimBLE_Stream_Echo/README.md
  • examples/NimBLE_Stream_Server/NimBLE_Stream_Server.ino
  • examples/NimBLE_Stream_Server/README.md
  • src/NimBLEDevice.h
  • src/NimBLEStream.cpp
  • src/NimBLEStream.h
  • src/nimble/console/console.h
  • src/nimconfig.h

Copy link

@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

🧹 Nitpick comments (1)
examples/NimBLE_Stream_Client/NimBLE_Stream_Client.ino (1)

38-43: Consider copying device address instead of storing pointer.

The pServerDevice pointer references memory managed by the NimBLE scan results. While this pattern is common in NimBLE examples and typically works, the pointer may become invalid if scan results are cleared before connectToServer() is called. For more robust code, consider storing the address directly:

static NimBLEAddress serverAddress;
// In onResult:
serverAddress = *advertisedDevice->getAddress();
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@examples/NimBLE_Stream_Client/NimBLE_Stream_Client.ino` around lines 38 - 43,
The code stores a pointer to the scan result in pServerDevice which may become
invalid later; instead, copy and store the device address (use NimBLEAddress) in
onResult and use that address in connectToServer(), replacing uses of
pServerDevice with the saved serverAddress; update any references in functions
like connectToServer() and the onResult handler to construct/lookup a
NimBLEDevice from the saved NimBLEAddress rather than dereferencing the original
advertisedDevice pointer.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@examples/NimBLE_Stream_Client/NimBLE_Stream_Client.ino`:
- Around line 75-77: connectToServer() dereferences pServerDevice without
checking for null; add a null-check at the start of connectToServer() to handle
the case where pServerDevice is nullptr (log an error via Serial or return
false) before calling pServerDevice->getAddress(), ensuring the function returns
cleanly (e.g., false) when pServerDevice is not set.

---

Nitpick comments:
In `@examples/NimBLE_Stream_Client/NimBLE_Stream_Client.ino`:
- Around line 38-43: The code stores a pointer to the scan result in
pServerDevice which may become invalid later; instead, copy and store the device
address (use NimBLEAddress) in onResult and use that address in
connectToServer(), replacing uses of pServerDevice with the saved serverAddress;
update any references in functions like connectToServer() and the onResult
handler to construct/lookup a NimBLEDevice from the saved NimBLEAddress rather
than dereferencing the original advertisedDevice pointer.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: f7e95589-ed6d-4143-ae99-cee4bb3fb91b

📥 Commits

Reviewing files that changed from the base of the PR and between 2abe280 and c4e2970.

📒 Files selected for processing (3)
  • examples/NimBLE_Stream_Client/NimBLE_Stream_Client.ino
  • src/NimBLEStream.cpp
  • src/NimBLEStream.h

Copy link

@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.

🧹 Nitpick comments (1)
src/NimBLEStream.cpp (1)

682-693: Note: Debug log recursion prevention filter.

The string filtering to prevent log recursion is a pragmatic solution. Consider adding a brief comment explaining why this specific string triggers recursion (GAP event logging → characteristic write → more GAP events) for future maintainability.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/NimBLEStream.cpp` around lines 682 - 693, Add a brief explanatory comment
above the recursion-prevention filter: explain that the literal "handleGapEvent"
(variable filterStr) corresponds to GAP event logging paths which can cause a
feedback loop where logging the GAP event triggers a characteristic write and
thus generates further GAP events, so the code checks data/len for that
substring and returns early to drop the log entry and avoid recursion; reference
the variables filterStr, filterLen, data, and len so maintainers can find the
filter logic quickly.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In `@src/NimBLEStream.cpp`:
- Around line 682-693: Add a brief explanatory comment above the
recursion-prevention filter: explain that the literal "handleGapEvent" (variable
filterStr) corresponds to GAP event logging paths which can cause a feedback
loop where logging the GAP event triggers a characteristic write and thus
generates further GAP events, so the code checks data/len for that substring and
returns early to drop the log entry and avoid recursion; reference the variables
filterStr, filterLen, data, and len so maintainers can find the filter logic
quickly.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 44e19378-3c8c-438e-a9c5-2fcfec21372c

📥 Commits

Reviewing files that changed from the base of the PR and between 76a82ad and 6bae48d.

📒 Files selected for processing (2)
  • src/NimBLEStream.cpp
  • src/NimBLEStream.h

@h2zero h2zero requested a review from Copilot March 15, 2026 00:32
Copy link

@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: 3

🧹 Nitpick comments (1)
src/NimBLEStream.h (1)

93-98: Tighten the overflow callback contract.

The callback fires when a new chunk no longer fits in the available RX space, not just when len exceeds the total buffer capacity. Calling out the two outcomes—drop current data vs drop oldest buffered data—would make the header match pushRx() and set the right expectations for integrators.

Based on learnings, the preferred direction for RX overflow in NimBLEStream is a user-settable callback so the application can react when data is dropped.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@src/NimBLEStream.h` around lines 93 - 98, Update the setRxOverflowCallback
comment to exactly match pushRx() semantics: state that the
rx_overflow_callback_t is invoked when an incoming chunk cannot fit in the
available RX space (not merely when len > total buffer capacity), and document
the two outcomes the callback controls—return true to drop the oldest buffered
data to make room for the incoming chunk, or return false to drop the incoming
chunk; reference setRxOverflowCallback, rx_overflow_callback_t, pushRx(), and
m_rxOverflowCallback so integrators know where this behavior is enforced and
that they can react to dropped data.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@src/NimBLEStream.cpp`:
- Around line 488-514: The RX overflow handling in NimBLEStream.cpp uses
separate ByteRingBuffer calls (freeSize(), drop(), write()) allowing a
concurrent read() to make requiredSpace stale; fix by adding a single atomic
ByteRingBuffer method (e.g., dropAndWrite or ensureSpaceThenWrite) that acquires
the ring-buffer mutex and performs the drop + write decision and operations
under one lock, then replace the overflow branch in the function handling
m_rxBuf with a single call to that new atomic method; reference m_rxBuf,
freeSize(), drop(), write(), and read() when locating changes.
- Around line 758-765: In NimBLEStreamServer::ChrCallbacks::onWrite, detect
write-only mode (begin called with txBufSize == 0 and rxBufSize > 0) and
populate the peer handle so ready()/operator bool() reflect an active peer: if
m_parent's TX is disabled and m_parent->m_peerHandle is not set, call
m_parent->setPeerHandle(connInfo.getConnHandle()) (or assign
m_parent->m_peerHandle = connInfo.getConnHandle() using the class' existing
accessor) when handling onWrite; otherwise leave behavior unchanged. This
ensures write-only streams become "ready" even without onSubscribe(); if you
don't want to support write-only streams, instead update the API
contract/comments on ready()/operator bool() to state they only track
subscribers from onSubscribe().
- Around line 776-795: When NimBLEStreamServer::ChrCallbacks::onSubscribe
accepts a new subscriber (m_peerHandle updated) it must drop any queued unsent
bytes left from the previous peer to avoid delivering them to the new
connection; update onSubscribe to clear m_txBuf (or call the method that empties
it) whenever you retire the old peer (i.e., when ble_gap_conn_find fails and you
set m_peerHandle = BLE_HS_CONN_HANDLE_NONE, and also when assigning m_peerHandle
= connInfo.getConnHandle() for a new subscriber), and ensure send() behavior
remains unchanged except that queued data is flushed/cleared on peer rollover.

---

Nitpick comments:
In `@src/NimBLEStream.h`:
- Around line 93-98: Update the setRxOverflowCallback comment to exactly match
pushRx() semantics: state that the rx_overflow_callback_t is invoked when an
incoming chunk cannot fit in the available RX space (not merely when len > total
buffer capacity), and document the two outcomes the callback controls—return
true to drop the oldest buffered data to make room for the incoming chunk, or
return false to drop the incoming chunk; reference setRxOverflowCallback,
rx_overflow_callback_t, pushRx(), and m_rxOverflowCallback so integrators know
where this behavior is enforced and that they can react to dropped data.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 0ca87acc-8895-41c5-836f-899a61bc43aa

📥 Commits

Reviewing files that changed from the base of the PR and between 6bae48d and 4ea61fb.

📒 Files selected for processing (12)
  • .gitignore
  • examples/NimBLE_Stream_Client/NimBLE_Stream_Client.ino
  • examples/NimBLE_Stream_Client/README.md
  • examples/NimBLE_Stream_Echo/NimBLE_Stream_Echo.ino
  • examples/NimBLE_Stream_Echo/README.md
  • examples/NimBLE_Stream_Server/NimBLE_Stream_Server.ino
  • examples/NimBLE_Stream_Server/README.md
  • src/NimBLEDevice.h
  • src/NimBLEStream.cpp
  • src/NimBLEStream.h
  • src/nimble/console/console.h
  • src/nimconfig.h
✅ Files skipped from review due to trivial changes (2)
  • examples/NimBLE_Stream_Server/README.md
  • src/nimconfig.h
🚧 Files skipped from review as they are similar to previous changes (4)
  • examples/NimBLE_Stream_Echo/README.md
  • .gitignore
  • examples/NimBLE_Stream_Echo/NimBLE_Stream_Echo.ino
  • src/nimble/console/console.h

Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 10 out of 12 changed files in this pull request and generated 3 comments.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

You can also share your feedback on Copilot code review. Take the survey.

Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 10 out of 12 changed files in this pull request and generated 3 comments.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

You can also share your feedback on Copilot code review. Take the survey.

@h2zero
Copy link
Owner Author

h2zero commented Mar 15, 2026

@coderabbitai full review

@coderabbitai
Copy link

coderabbitai bot commented Mar 15, 2026

✅ Actions performed

Full review triggered.

Copy link

@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.

🧹 Nitpick comments (1)
examples/NimBLE_Stream_Server/NimBLE_Stream_Server.ino (1)

132-132: Consider platform portability for ESP.getFreeHeap().

This call is specific to ESP32/ESP8266 platforms. While the NimBLE-Arduino library primarily targets ESP32, adding a brief comment or a preprocessor guard would clarify the platform requirement for users porting to other Arduino-compatible boards.

💡 Optional: Add platform context
             // You can also use printf
-            bleStream.printf("Free heap: %d bytes\n", ESP.getFreeHeap());
+            // Note: ESP.getFreeHeap() is ESP32/ESP8266 specific
+            bleStream.printf("Free heap: %d bytes\n", ESP.getFreeHeap());

Or use a guard:

`#if` defined(ESP32) || defined(ESP8266)
            bleStream.printf("Free heap: %d bytes\n", ESP.getFreeHeap());
`#endif`
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@examples/NimBLE_Stream_Server/NimBLE_Stream_Server.ino` at line 132, The call
to ESP.getFreeHeap() inside bleStream.printf is ESP-specific and may break
portability; update the code around bleStream.printf("Free heap: %d bytes\n",
ESP.getFreeHeap()) to either wrap the call in a preprocessor guard (e.g., only
compile when ESP32 or ESP8266 macros are defined) or add a clarifying comment
that this line requires ESP platforms and should be removed/adjusted when
porting to other boards so compilation and runtime behavior remain correct.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In `@examples/NimBLE_Stream_Server/NimBLE_Stream_Server.ino`:
- Line 132: The call to ESP.getFreeHeap() inside bleStream.printf is
ESP-specific and may break portability; update the code around
bleStream.printf("Free heap: %d bytes\n", ESP.getFreeHeap()) to either wrap the
call in a preprocessor guard (e.g., only compile when ESP32 or ESP8266 macros
are defined) or add a clarifying comment that this line requires ESP platforms
and should be removed/adjusted when porting to other boards so compilation and
runtime behavior remain correct.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 32444b8d-96f5-496e-a45e-10c9a2279824

📥 Commits

Reviewing files that changed from the base of the PR and between cc47671 and dc66ca3.

📒 Files selected for processing (12)
  • .gitignore
  • examples/NimBLE_Stream_Client/NimBLE_Stream_Client.ino
  • examples/NimBLE_Stream_Client/README.md
  • examples/NimBLE_Stream_Echo/NimBLE_Stream_Echo.ino
  • examples/NimBLE_Stream_Echo/README.md
  • examples/NimBLE_Stream_Server/NimBLE_Stream_Server.ino
  • examples/NimBLE_Stream_Server/README.md
  • src/NimBLEDevice.h
  • src/NimBLEStream.cpp
  • src/NimBLEStream.h
  • src/nimble/console/console.h
  • src/nimconfig.h

Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 10 out of 12 changed files in this pull request and generated 5 comments.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

You can also share your feedback on Copilot code review. Take the survey.

@h2zero
Copy link
Owner Author

h2zero commented Mar 15, 2026

@coderabbitai review

@coderabbitai
Copy link

coderabbitai bot commented Mar 15, 2026

✅ Actions performed

Review triggered.

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.

Copy link

@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: 2

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

Inline comments:
In `@src/NimBLEStream.cpp`:
- Around line 804-807: The ready() implementation incorrectly returns true for
RX-only streams (m_txBufSize == 0) by returning m_rxBufSize > 0, which makes
operator bool() report "ready" without any subscriber/notification context;
change the RX-only branch in ready() to return false so readiness remains
subscriber/notification-centric (consistent with NimBLEStreamServer behavior)
and rely on subscriber notification logic to indicate readiness for RX-only
streams; update ready() where m_txBufSize is checked (and any callers like
operator bool()) to preserve this contract.
- Around line 483-488: NimBLEStream::pushRx currently early-returns on ready()
and dereferences m_rxBuf without null-check; change it to first verify m_rxBuf
is non-null and return 0 if it is null, then obtain ByteRingBuffer::Guard
g(*m_rxBuf) and push data into the buffer unconditionally (do not gate on
ready()), mirroring NimBLEStreamServer behavior; update references in pushRx to
use the null-checked m_rxBuf before creating the Guard so no null deref occurs,
and ensure any existing ready()-based logic that dropped RX data is removed or
relocated so onWrite from clients always queues incoming data.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 8b37a14e-0540-46e5-89f5-59dae4f5d0bf

📥 Commits

Reviewing files that changed from the base of the PR and between dc66ca3 and cce24a3.

📒 Files selected for processing (3)
  • examples/NimBLE_Stream_Client/NimBLE_Stream_Client.ino
  • src/NimBLEStream.cpp
  • src/NimBLEStream.h

Co-authored-by: doudar <17362216+doudar@users.noreply.github.com>
@h2zero h2zero merged commit f53028b into master Mar 15, 2026
42 checks passed
@h2zero h2zero deleted the stream-class branch March 15, 2026 22:49
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.

2 participants