Skip to content

feat(L2CAP): add disconnect API and harden CoC send/error handling #396

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
mickeyl wants to merge 1 commit into
base: master
Choose a base branch
from
Open

feat(L2CAP): add disconnect API and harden CoC send/error handling #396

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions examples/L2CAP/L2CAP_Client/main/idf_component.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,6 @@
dependencies:
local/esp-nimble-cpp:
path: ../../../../../esp-nimble-cpp/
mickeyl/esp-hpl:
git: https://github.com/mickeyl/esp-hpl.git
version: "1.1.0"
166 changes: 127 additions & 39 deletions examples/L2CAP/L2CAP_Client/main/main.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,22 +1,28 @@
#include <NimBLEDevice.h>
#include <esp_hpl.hpp>
#include <esp_timer.h>

// See the following for generating UUIDs:
// https://www.uuidgenerator.net/

// The remote service we wish to connect to.
static BLEUUID serviceUUID("dcbc7255-1e9e-49a0-a360-b0430b6c6905");
// The characteristic of the remote service we are interested in.
static BLEUUID charUUID("371a55c8-f251-4ad2-90b3-c7c195b049be");

#define L2CAP_CHANNEL 150
#define L2CAP_PSM 192
#define L2CAP_MTU 5000
#define INITIAL_PAYLOAD_SIZE 64
#define BLOCKS_BEFORE_DOUBLE 50
#define MAX_PAYLOAD_SIZE 4900

const BLEAdvertisedDevice* theDevice = NULL;
BLEClient* theClient = NULL;
BLEL2CAPChannel* theChannel = NULL;

size_t bytesSent = 0;
size_t bytesReceived = 0;
size_t currentPayloadSize = INITIAL_PAYLOAD_SIZE;
uint32_t blocksSent = 0;
uint64_t startTime = 0;

// Heap monitoring
size_t initialHeap = 0;
size_t lastHeap = 0;
size_t heapDecreaseCount = 0;
const size_t HEAP_LEAK_THRESHOLD = 10; // Warn after 10 consecutive decreases

class L2CAPChannelCallbacks: public BLEL2CAPChannelCallbacks {

Expand All @@ -43,7 +49,7 @@ class MyClientCallbacks: public BLEClientCallbacks {
printf("GAP connected\n");
pClient->setDataLen(251);

theChannel = BLEL2CAPChannel::connect(pClient, L2CAP_CHANNEL, L2CAP_MTU, new L2CAPChannelCallbacks());
theChannel = BLEL2CAPChannel::connect(pClient, L2CAP_PSM, L2CAP_MTU, new L2CAPChannelCallbacks());
}

void onDisconnect(BLEClient* pClient, int reason) {
Expand All @@ -61,23 +67,72 @@ class MyAdvertisedDeviceCallbacks: public BLEAdvertisedDeviceCallbacks {
if (theDevice) { return; }
printf("BLE Advertised Device found: %s\n", advertisedDevice->toString().c_str());

if (!advertisedDevice->haveServiceUUID()) { return; }
if (!advertisedDevice->isAdvertisingService(serviceUUID)) { return; }
// Look for device named "l2cap"
if (advertisedDevice->haveName() && advertisedDevice->getName() == "l2cap") {
printf("Found l2cap device!\n");
BLEDevice::getScan()->stop();
theDevice = advertisedDevice;
}
}
};

void statusTask(void *pvParameters) {
while (true) {
vTaskDelay(1000 / portTICK_PERIOD_MS);

if (startTime > 0 && blocksSent > 0) {
uint64_t currentTime = esp_timer_get_time();
double elapsedSeconds = (currentTime - startTime) / 1000000.0;
double bytesPerSecond = 0.0;
double kbPerSecond = 0.0;
if (elapsedSeconds > 0.0) {
bytesPerSecond = bytesSent / elapsedSeconds;
kbPerSecond = bytesPerSecond / 1024.0;
}

printf("Found the device we're interested in!\n");
BLEDevice::getScan()->stop();
// Heap monitoring
size_t currentHeap = esp_get_free_heap_size();
size_t minHeap = esp_get_minimum_free_heap_size();

// Hand over the device to the other task
theDevice = advertisedDevice;
// Track heap for leak detection
if (initialHeap == 0) {
initialHeap = currentHeap;
lastHeap = currentHeap;
}

// Check for consistent heap decrease
if (currentHeap < lastHeap) {
heapDecreaseCount++;
if (heapDecreaseCount >= HEAP_LEAK_THRESHOLD) {
printf("\n⚠️ WARNING: POSSIBLE MEMORY LEAK DETECTED! ⚠️\n");
printf("Heap has decreased %zu times in a row\n", heapDecreaseCount);
printf("Initial heap: %zu, Current heap: %zu, Lost: %zu bytes\n",
initialHeap, currentHeap, initialHeap - currentHeap);
}
} else if (currentHeap >= lastHeap) {
heapDecreaseCount = 0; // Reset counter if heap stabilizes or increases
}
lastHeap = currentHeap;

printf("\n=== STATUS UPDATE ===\n");
printf("Blocks sent: %lu\n", (unsigned long)blocksSent);
printf("Total bytes sent: %zu\n", bytesSent);
printf("Current payload size: %zu bytes\n", currentPayloadSize);
printf("Elapsed time: %.1f seconds\n", elapsedSeconds);
printf("Bandwidth: %.2f KB/s (%.2f Mbps)\n", kbPerSecond, (bytesPerSecond * 8) / 1000000.0);
printf("Heap: %zu free (min: %zu), Used since start: %zu\n",
currentHeap, minHeap, initialHeap > 0 ? initialHeap - currentHeap : 0);
printf("==================\n\n");
}
}
Comment on lines +79 to 127
Copy link
Copy Markdown

@coderabbitai coderabbitai bot Mar 16, 2026

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
rg -n -C2 'size_t bytesSent|size_t currentPayloadSize|uint32_t blocksSent|uint64_t startTime' examples/L2CAP/L2CAP_Client/main/main.cpp
rg -n -C2 'bytesSent \+=|blocksSent\+\+|currentPayloadSize =|startTime =' examples/L2CAP/L2CAP_Client/main/main.cpp
rg -n -C2 'xTaskCreate\(connectTask|xTaskCreate\(statusTask' examples/L2CAP/L2CAP_Client/main/main.cpp

Repository: h2zero/esp-nimble-cpp

Length of output: 1794

Protect the shared cargo with a lock, ye scurvy dog.

connectTask writes bytesSent, blocksSent, currentPayloadSize, and startTime whilst statusTask reads 'em concurrently without synchronization. These be racy globals, and the 64-bit startTime be especially treacherous on 32-bit MCUs where reads and writes of that treasure require multiple instructions.

Use xSemaphoreMutex or similar protection to guard these shared variables.

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

In `@examples/L2CAP/L2CAP_Client/main/main.cpp` around lines 79 - 123, The
statusTask reads globals bytesSent, blocksSent, currentPayloadSize, and
startTime without synchronization while connectTask writes them, causing data
races (and unsafe 64-bit access on 32-bit MCUs); create a FreeRTOS mutex (e.g.,
xSemaphoreHandle / SemaphoreHandle_t via xSemaphoreCreateMutex) during
initialization, then wrap all reads in statusTask and all writes/updates in
connectTask with xSemaphoreTake(..., portMAX_DELAY) / xSemaphoreGive(...) to
protect access to those variables and ensure the mutex is created before tasks
start.

};
}

void connectTask(void *pvParameters) {

uint8_t sequenceNumber = 0;

while (true) {

if (!theDevice) {
vTaskDelay(1000 / portTICK_PERIOD_MS);
continue;
Expand All @@ -96,7 +151,7 @@ void connectTask(void *pvParameters) {
break;
}
vTaskDelay(2000 / portTICK_PERIOD_MS);
continue;
continue;
}

if (!theChannel) {
Expand All @@ -112,22 +167,58 @@ void connectTask(void *pvParameters) {
}

while (theChannel->isConnected()) {
// Create framed packet: [seqno 8bit] [16bit payload length] [payload]
std::vector<uint8_t> packet;
packet.reserve(3 + currentPayloadSize);

// Add sequence number (8 bits)
packet.push_back(sequenceNumber);

// Add payload length (16 bits, big endian - network byte order)
uint16_t payloadLen = currentPayloadSize;
packet.push_back((payloadLen >> 8) & 0xFF); // High byte first
packet.push_back(payloadLen & 0xFF); // Low byte second

/*
static auto initialDelay = true;
if (initialDelay) {
printf("Waiting gracefully 3 seconds before sending data\n");
vTaskDelay(3000 / portTICK_PERIOD_MS);
initialDelay = false;
};
*/
std::vector<uint8_t> data(5000, sequenceNumber++);
if (theChannel->write(data)) {
bytesSent += data.size();
// Add payload
for (size_t i = 0; i < currentPayloadSize; i++) {
packet.push_back(i & 0xFF);
}

if (theChannel->write(packet)) {
if (startTime == 0) {
startTime = esp_timer_get_time();
}
bytesSent += packet.size();
blocksSent++;

// Print every block since we're sending slowly now
printf("Sent block %lu (seq=%d, payload=%zu bytes, frame_size=%zu)\n",
(unsigned long)blocksSent, sequenceNumber, currentPayloadSize, packet.size());

sequenceNumber++;

// After every 50 blocks, double payload size
if (blocksSent % BLOCKS_BEFORE_DOUBLE == 0) {
size_t newSize = currentPayloadSize * 2;

// Cap at maximum safe payload size
if (newSize > MAX_PAYLOAD_SIZE) {
if (currentPayloadSize < MAX_PAYLOAD_SIZE) {
currentPayloadSize = MAX_PAYLOAD_SIZE;
printf("\n=== Reached maximum payload size of %zu bytes after %lu blocks ===\n", currentPayloadSize, (unsigned long)blocksSent);
}
// Already at max, don't increase further
} else {
currentPayloadSize = newSize;
printf("\n=== Doubling payload size to %zu bytes after %lu blocks ===\n", currentPayloadSize, (unsigned long)blocksSent);
}
}
} else {
printf("failed to send!\n");
abort();
abort();
Comment on lines 216 to +218
Copy link
Copy Markdown

@coderabbitai coderabbitai bot Mar 16, 2026

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Don’t abort() on a routine send failure, matey.

write() already collapses disconnects and exhausted retries into false. Crashing the whole app here throws away the very stats this stress example is trying to gather; back off and unwind to the reconnect path instead.

🏴‍☠️ Proposed fix 
             } else {
-                printf("failed to send!\n");
-                abort();
+                printf("failed to send, backing off\n");
+                vTaskDelay(1000 / portTICK_PERIOD_MS);
+                break;
             }
📝 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
} else {
printf("failed to send!\n");
abort();
abort();
} else {
printf("failed to send, backing off\n");
vTaskDelay(1000 / portTICK_PERIOD_MS);
break;
}
🤖 Prompt for AI Agents 
Verify each finding against the current code and only fix it if needed.

In `@examples/L2CAP/L2CAP_Client/main/main.cpp` around lines 212 - 214, When
write() fails in the send branch (currently printing "failed to send!" then
calling abort()), don't terminate the process; instead unwind into the reconnect
logic: remove the abort() call, keep the failure log, update any send-failure
counters/statistics, close the connection/socket resource, set the client state
to disconnected (the same state used by your reconnect path), and return from
the send routine so the existing reconnect logic will run; reference the write()
call and the abort() call in your changes to locate and replace the behavior.

}

// No delay - send as fast as possible
}

vTaskDelay(1000 / portTICK_PERIOD_MS);
Expand All @@ -136,9 +227,13 @@ void connectTask(void *pvParameters) {

extern "C"
void app_main(void) {
// Install high performance logging before any output
esp_hpl::HighPerformanceLogger::init();

printf("Starting L2CAP client example\n");

xTaskCreate(connectTask, "connectTask", 5000, NULL, 1, NULL);
xTaskCreate(statusTask, "statusTask", 3000, NULL, 1, NULL);

BLEDevice::init("L2CAP-Client");
BLEDevice::setMTU(BLE_ATT_MTU_MAX);
Expand All @@ -151,15 +246,8 @@ void app_main(void) {
scan->setActiveScan(true);
scan->start(25 * 1000, false);

int numberOfSeconds = 0;

while (bytesSent == 0) {
vTaskDelay(10 / portTICK_PERIOD_MS);
}

// Main task just waits
while (true) {
vTaskDelay(1000 / portTICK_PERIOD_MS);
int bytesSentPerSeconds = bytesSent / ++numberOfSeconds;
printf("Bandwidth: %d b/sec = %d KB/sec\n", bytesSentPerSeconds, bytesSentPerSeconds / 1024);
}
}
3 changes: 3 additions & 0 deletions examples/L2CAP/L2CAP_Server/main/idf_component.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,6 @@
dependencies:
local/esp-nimble-cpp:
path: ../../../../../esp-nimble-cpp/
mickeyl/esp-hpl:
git: https://github.com/mickeyl/esp-hpl.git
version: "1.1.0"
Loading