From 25a9994a177898d218b8827da6b5fc7c69e2934e Mon Sep 17 00:00:00 2001 From: "Dr. Michael Lauer" Date: Fri, 16 Oct 2026 14:32:08 +0200 Subject: [PATCH 1/5] fix(l2cap): propagate send failures without replaying partial SDUs --- .github/workflows/l2cap-tests.yml | 17 +++++++ src/NimBLEL2CAPChannel.cpp | 77 ++++++++++-------------------- tests/l2cap/CMakeLists.txt | 10 ++++ tests/l2cap/README.md | 13 +++++ tests/l2cap/SendErrorsTests.cpp | 60 +++++++++++++++++++++++ tests/l2cap/stubs/Host.h | 68 ++++++++++++++++++++++++++ tests/l2cap/stubs/host/ble_gap.h | 1 + tests/l2cap/stubs/host/ble_l2cap.h | 1 + tests/l2cap/stubs/os/os_mbuf.h | 1 + tests/l2cap/stubs/syscfg/syscfg.h | 1 + 10 files changed, 198 insertions(+), 51 deletions(-) create mode 100644 .github/workflows/l2cap-tests.yml create mode 100644 tests/l2cap/CMakeLists.txt create mode 100644 tests/l2cap/README.md create mode 100644 tests/l2cap/SendErrorsTests.cpp create mode 100644 tests/l2cap/stubs/Host.h create mode 100644 tests/l2cap/stubs/host/ble_gap.h create mode 100644 tests/l2cap/stubs/host/ble_l2cap.h create mode 100644 tests/l2cap/stubs/os/os_mbuf.h create mode 100644 tests/l2cap/stubs/syscfg/syscfg.h diff --git a/.github/workflows/l2cap-tests.yml b/.github/workflows/l2cap-tests.yml new file mode 100644 index 00000000..52badcec --- /dev/null +++ b/.github/workflows/l2cap-tests.yml @@ -0,0 +1,17 @@ +name: L2CAP native tests +on: + workflow_dispatch: + pull_request: + push: + branches: [master] +jobs: + native: + runs-on: ubuntu-latest + strategy: + matrix: + cxx_flags: ["", "-fsanitize=address,undefined -fno-omit-frame-pointer"] + steps: + - uses: actions/checkout@v5 + - run: cmake -S tests/l2cap -B build/l2cap-tests -DCMAKE_CXX_FLAGS="${{ matrix.cxx_flags }}" + - run: cmake --build build/l2cap-tests + - run: ctest --test-dir build/l2cap-tests --output-on-failure diff --git a/src/NimBLEL2CAPChannel.cpp b/src/NimBLEL2CAPChannel.cpp index 36b740ad..8868ad86 100644 --- a/src/NimBLEL2CAPChannel.cpp +++ b/src/NimBLEL2CAPChannel.cpp @@ -8,6 +8,7 @@ # include "NimBLEClient.h" # include "NimBLELog.h" # include "NimBLEUtils.h" +# include # ifdef USING_NIMBLE_ARDUINO_HEADERS # include "nimble/nimble/host/include/host/ble_gap.h" @@ -21,9 +22,6 @@ // Round-up integer division # define CEIL_DIVIDE(a, b) (((a) + (b) - 1) / (b)) # define ROUND_DIVIDE(a, b) (((a) + (b) / 2) / (b)) -// Retry -constexpr uint32_t RetryTimeout = 50; -constexpr int RetryCounter = 3; NimBLEL2CAPChannel::NimBLEL2CAPChannel(uint16_t psm, uint16_t mtu, NimBLEL2CAPChannelCallbacks* callbacks) : psm(psm), mtu(mtu), callbacks(callbacks) { @@ -105,54 +103,30 @@ int NimBLEL2CAPChannel::writeFragment(std::vector::const_iterator begin return -BLE_HS_EBADDATA; } - auto retries = RetryCounter; - - while (retries--) { - auto txd = os_mbuf_get_pkthdr(&_coc_mbuf_pool, 0); - if (!txd) { - NIMBLE_LOGE(LOG_TAG, "Can't os_mbuf_get_pkthdr."); - return -BLE_HS_ENOMEM; - } - auto append = os_mbuf_append(txd, &(*begin), toSend); - if (append != 0) { - NIMBLE_LOGE(LOG_TAG, "Can't os_mbuf_append: %d", append); - return append; - } + auto* txd = os_mbuf_get_pkthdr(&_coc_mbuf_pool, 0); + if (!txd) { + return BLE_HS_ENOMEM; + } + const int append = os_mbuf_append(txd, &*begin, toSend); + if (append != 0) { + os_mbuf_free_chain(txd); + return append; + } - auto res = ble_l2cap_send(channel, txd); - switch (res) { - case 0: - NIMBLE_LOGD(LOG_TAG, "L2CAP COC 0x%04X sent %d bytes.", this->psm, toSend); - return 0; - - case BLE_HS_ESTALLED: - stalled = true; - NIMBLE_LOGD(LOG_TAG, "L2CAP COC 0x%04X sent %d bytes.", this->psm, toSend); - NIMBLE_LOGW(LOG_TAG, - "ble_l2cap_send returned BLE_HS_ESTALLED. Next send will wait for unstalled event..."); - return 0; - - case BLE_HS_ENOMEM: - case BLE_HS_EAGAIN: - /* ble_l2cap_send already consumed and freed txd on these errors */ - NIMBLE_LOGD(LOG_TAG, "ble_l2cap_send returned %d (consumed buffer). Retrying shortly...", res); - ble_npl_time_delay(ble_npl_time_ms_to_ticks32(RetryTimeout)); - continue; - - case BLE_HS_EBUSY: - /* Channel busy; txd not consumed */ - NIMBLE_LOGD(LOG_TAG, "ble_l2cap_send returned %d (busy). Retrying shortly...", res); - os_mbuf_free_chain(txd); - ble_npl_time_delay(ble_npl_time_ms_to_ticks32(RetryTimeout)); - continue; - - default: - NIMBLE_LOGE(LOG_TAG, "ble_l2cap_send failed: %d", res); - return res; - } + const int rc = ble_l2cap_send(channel, txd); + if (rc == BLE_HS_ESTALLED) { + // NimBLE owns the SDU and will finish it when credits arrive. + stalled = true; + return 0; + } + // These early validation/busy errors retain caller ownership. All other + // native send results consume the SDU, including ENOMEM and EAGAIN. + if (rc == BLE_HS_EBADDATA || rc == BLE_HS_EBUSY) { + os_mbuf_free_chain(txd); } - NIMBLE_LOGE(LOG_TAG, "Retries exhausted, dropping %d bytes to send.", toSend); - return -BLE_HS_EREJECT; + // A native error can follow partial transmission of this SDU. Resubmitting + // it could duplicate bytes; let write() abort the incomplete stream. + return rc; } # if MYNEWT_VAL(BLE_ROLE_CENTRAL) @@ -194,8 +168,9 @@ bool NimBLEL2CAPChannel::write(const std::vector& bytes) { auto start = bytes.begin(); while (start != bytes.end()) { - auto end = start + mtu < bytes.end() ? start + mtu : bytes.end(); - if (writeFragment(start, end) < 0) { + auto end = start + std::min(mtu, bytes.end() - start); + if (writeFragment(start, end) != 0) { + disconnect(); return false; } start = end; diff --git a/tests/l2cap/CMakeLists.txt b/tests/l2cap/CMakeLists.txt new file mode 100644 index 00000000..8bfc65fa --- /dev/null +++ b/tests/l2cap/CMakeLists.txt @@ -0,0 +1,10 @@ +cmake_minimum_required(VERSION 3.20) +project(L2CAPTests LANGUAGES CXX) +enable_testing() +add_executable(l2cap_send_tests SendErrorsTests.cpp ../../src/NimBLEL2CAPChannel.cpp) +target_compile_features(l2cap_send_tests PRIVATE cxx_std_17) +target_include_directories(l2cap_send_tests PRIVATE stubs ../../src) +target_compile_options(l2cap_send_tests PRIVATE -UNDEBUG -include ${CMAKE_CURRENT_SOURCE_DIR}/stubs/Host.h) +foreach(case errors partial ownership append success stalled) + add_test(NAME l2cap_send_${case} COMMAND l2cap_send_tests ${case}) +endforeach() diff --git a/tests/l2cap/README.md b/tests/l2cap/README.md new file mode 100644 index 00000000..50e710d2 --- /dev/null +++ b/tests/l2cap/README.md @@ -0,0 +1,13 @@ +# L2CAP send regression tests + +The tests compile the production `NimBLEL2CAPChannel.cpp` with minimal native +host replacements. They check positive native error propagation, abort after a +partly submitted SDU without replay, caller/native mbuf ownership, append +failure cleanup, successful fragmentation and accepted credit-stalled sends. +They do not simulate RF, controller buffers or concurrency. + +```sh +cmake -S tests/l2cap -B build/l2cap-tests +cmake --build build/l2cap-tests +ctest --test-dir build/l2cap-tests --output-on-failure +``` diff --git a/tests/l2cap/SendErrorsTests.cpp b/tests/l2cap/SendErrorsTests.cpp new file mode 100644 index 00000000..2236d733 --- /dev/null +++ b/tests/l2cap/SendErrorsTests.cpp @@ -0,0 +1,60 @@ +#include "NimBLEL2CAPChannel.h" +#include +#include + +static void require(bool value,const char* message) { if (!value) throw std::runtime_error(message); } +struct Channel : NimBLEL2CAPChannel { + ble_l2cap_chan native; + Channel() : NimBLEL2CAPChannel(129,64,new NimBLEL2CAPChannelCallbacks) { + ble_l2cap_event e{};e.connect.chan=&native;handleConnectionEvent(&e); + } +}; +int main(int argc,char** argv) { + try { + const std::string test=argc>1?argv[1]:"errors"; + int sends=0; + if (test=="errors" || test=="ownership") { + const std::vector codes=test=="errors" ? + std::vector{BLE_HS_ENOMEM,BLE_HS_EAGAIN,BLE_HS_ENOTCONN,BLE_HS_EINVAL} : + std::vector{BLE_HS_EBADDATA,BLE_HS_EBUSY}; + for (int error:codes) { + Channel c;sends=0;hostDisconnects=0; + hostSend=[&](os_mbuf* b) { + ++sends; + if (error!=BLE_HS_EBADDATA && error!=BLE_HS_EBUSY) os_mbuf_free_chain(b); + return error; + }; + require(!c.write(std::vector(20,0x55)),"positive error reported success"); + require(sends==1,"failed SDU was retried or later fragment submitted"); + require(hostDisconnects==1,"failed stream was not disconnected"); + require(liveMbufs==0,"send buffer leaked or freed twice"); + } + } else if (test=="partial") { + Channel c;std::vector delivered; + hostSend=[&](os_mbuf* b) { + ++sends; + const size_t length=sends==1?b->bytes.size():2; + delivered.insert(delivered.end(),b->bytes.begin(),b->bytes.begin()+length); + os_mbuf_free_chain(b);return sends==1?0:BLE_HS_ENOMEM; + }; + require(!c.write(std::vector(24,0x55)),"partially sent stream reported success"); + require(sends==2 && delivered.size()==10,"partial SDU prefix replayed or remaining fragment sent"); + require(hostDisconnects==1 && liveMbufs==0,"partial-send abort/ownership failed"); + } else if (test=="append") { + Channel c;appendError=BLE_HS_ENOMEM; + hostSend=[&](os_mbuf*) { ++sends;return 0; }; + require(!c.write({1,2,3}),"append failure reported success"); + require(sends==0 && liveMbufs==0 && hostDisconnects==1,"append error leaked buffer or submitted data"); + } else if (test=="success") { + Channel c;std::vector received;std::vector input; + for (int i=0;i<20;++i) input.push_back(uint8_t(i)); + hostSend=[&](os_mbuf* b) {++sends;received.insert(received.end(),b->bytes.begin(),b->bytes.end());return os_mbuf_free_chain(b);}; + require(c.write(input) && sends==3 && received==input && liveMbufs==0 && hostDisconnects==0,"valid fragmented write failed"); + } else if (test=="stalled") { + Channel c; + hostSend=[&](os_mbuf* b) {++sends;os_mbuf_free_chain(b);return BLE_HS_ESTALLED;}; + require(c.write({1,2,3}) && sends==1 && liveMbufs==0 && hostDisconnects==0,"accepted stalled SDU incorrectly treated as error"); + } else { throw std::runtime_error("unknown test"); } + std::cout< +#include +#include +#include +#include +#include +#include +#include +struct os_mempool {}; +struct os_mbuf_pool {}; +using os_membuf_t = uint8_t; +struct os_mbuf { std::vector bytes; }; +#define OS_MEMPOOL_SIZE(n,s) ((n)*(s)) +#define OS_MBUF_PKTLEN(m) ((m)->bytes.size()) +inline int liveMbufs=0, appendError=0; +inline int os_mempool_init(os_mempool*,size_t,size_t,void*,const char*) { return 0; } +inline int os_mbuf_pool_init(os_mbuf_pool*,os_mempool*,size_t,size_t) { return 0; } +inline os_mbuf* os_mbuf_get_pkthdr(os_mbuf_pool*,int) { ++liveMbufs; return new os_mbuf; } +inline int os_mbuf_append(os_mbuf* b,const void* p,size_t n) { + if (appendError) return appendError; + auto a=static_cast(p); b->bytes.insert(b->bytes.end(),a,a+n); return 0; +} +inline int os_mbuf_free_chain(os_mbuf* b) { --liveMbufs; delete b; return 0; } +inline int os_mbuf_copydata(os_mbuf* b,size_t off,size_t n,void* p) { memcpy(p,b->bytes.data()+off,n); return 0; } +constexpr int BLE_HS_EAGAIN=1, BLE_HS_ENOMEM=6, BLE_HS_ENOTCONN=7, BLE_HS_EINVAL=3, + BLE_HS_EALREADY=2, BLE_HS_EBADDATA=10, BLE_HS_EBUSY=15, BLE_HS_EREJECT=14, + BLE_HS_ESTALLED=31, BLE_HS_EUNKNOWN=17; +constexpr uint16_t BLE_HS_CONN_HANDLE_NONE=0xffff; +constexpr uint32_t BLE_NPL_TIME_FOREVER=UINT32_MAX; +inline uint32_t ble_npl_time_ms_to_ticks32(uint32_t ms) { return ms; } +inline void ble_npl_time_delay(uint32_t) {} // Retry count, not elapsed radio time, is tested. +struct ble_l2cap_chan {}; +struct ble_l2cap_chan_info { uint16_t peer_coc_mtu=8,our_coc_mtu=8,peer_l2cap_mtu=8,our_l2cap_mtu=8; }; +constexpr int BLE_L2CAP_EVENT_COC_CONNECTED=1,BLE_L2CAP_EVENT_COC_DISCONNECTED=2, + BLE_L2CAP_EVENT_COC_ACCEPT=3,BLE_L2CAP_EVENT_COC_DATA_RECEIVED=4,BLE_L2CAP_EVENT_COC_TX_UNSTALLED=5; +struct ble_l2cap_event { + int type=0; + struct { ble_l2cap_chan* chan; } connect{},accept{},disconnect{}; + struct { os_mbuf* sdu_rx; } receive{}; + struct { int status; ble_l2cap_chan* chan; } tx_unstalled{}; +}; +inline std::function hostSend; +inline int hostDisconnects=0; +inline int ble_l2cap_get_chan_info(ble_l2cap_chan*,ble_l2cap_chan_info* i) { *i={}; return 0; } +inline uint16_t ble_l2cap_get_conn_handle(ble_l2cap_chan*) { return 1; } +inline int ble_l2cap_send(ble_l2cap_chan*,os_mbuf* b) { return hostSend(b); } +inline int ble_l2cap_disconnect(ble_l2cap_chan*) { ++hostDisconnects; return 0; } +inline int ble_l2cap_recv_ready(ble_l2cap_chan*,os_mbuf* b) { return os_mbuf_free_chain(b); } +class NimBLEUtils { + public: + struct TaskData { mutable int m_flags=0; }; + static bool taskWait(const TaskData&,uint32_t) { assert(false && "credit wait not exercised by send error tests"); return false; } + static void taskRelease(const TaskData&,int=0) {} + static const char* returnCodeToString(int) { return "stub"; } +}; diff --git a/tests/l2cap/stubs/host/ble_gap.h b/tests/l2cap/stubs/host/ble_gap.h new file mode 100644 index 00000000..dcdc9c8a --- /dev/null +++ b/tests/l2cap/stubs/host/ble_gap.h @@ -0,0 +1 @@ +#include "Host.h" diff --git a/tests/l2cap/stubs/host/ble_l2cap.h b/tests/l2cap/stubs/host/ble_l2cap.h new file mode 100644 index 00000000..dcdc9c8a --- /dev/null +++ b/tests/l2cap/stubs/host/ble_l2cap.h @@ -0,0 +1 @@ +#include "Host.h" diff --git a/tests/l2cap/stubs/os/os_mbuf.h b/tests/l2cap/stubs/os/os_mbuf.h new file mode 100644 index 00000000..dcdc9c8a --- /dev/null +++ b/tests/l2cap/stubs/os/os_mbuf.h @@ -0,0 +1 @@ +#include "Host.h" diff --git a/tests/l2cap/stubs/syscfg/syscfg.h b/tests/l2cap/stubs/syscfg/syscfg.h new file mode 100644 index 00000000..dcdc9c8a --- /dev/null +++ b/tests/l2cap/stubs/syscfg/syscfg.h @@ -0,0 +1 @@ +#include "Host.h" From 56ffa2408e6b4967062c93af26e9f64602b67c34 Mon Sep 17 00:00:00 2001 From: "Dr. Michael Lauer" Date: Fri, 16 Oct 2026 14:45:20 +0200 Subject: [PATCH 2/5] fix(l2cap): serialize writes and drain channel lifecycle work --- .github/tests/l2cap-build/.gitignore | 5 + .github/tests/l2cap-build/CMakeLists.txt | 6 + .github/tests/l2cap-build/main/CMakeLists.txt | 1 + .github/tests/l2cap-build/main/main.cpp | 49 ++ .github/tests/l2cap-build/sdkconfig.defaults | 4 + .github/tests/l2cap-build/sdkconfig.deferred | 1 + .github/workflows/l2cap-build.yml | 38 ++ Kconfig | 63 ++- Makefile | 16 + src/NimBLEDevice.cpp | 10 + src/NimBLEL2CAPChannel.cpp | 452 ++++++++++++++---- src/NimBLEL2CAPChannel.h | 58 ++- tests/l2cap/CMakeLists.txt | 30 +- tests/l2cap/L2CAPTests.cpp | 298 ++++++++++++ tests/l2cap/README.md | 43 +- tests/l2cap/SendErrorsTests.cpp | 60 --- tests/l2cap/stubs/Host.h | 301 ++++++++++-- tests/l2cap/stubs/freertos/FreeRTOS.h | 1 + tests/l2cap/stubs/freertos/queue.h | 1 + tests/l2cap/stubs/freertos/semphr.h | 1 + tests/l2cap/stubs/freertos/task.h | 1 + tests/l2cap/stubs/host/ble_hs.h | 1 + tests/l2cap/stubs/nimble/nimble_npl.h | 1 + tests/l2cap/stubs/nimble/nimble_port.h | 1 + 24 files changed, 1206 insertions(+), 236 deletions(-) create mode 100644 .github/tests/l2cap-build/.gitignore create mode 100644 .github/tests/l2cap-build/CMakeLists.txt create mode 100644 .github/tests/l2cap-build/main/CMakeLists.txt create mode 100644 .github/tests/l2cap-build/main/main.cpp create mode 100644 .github/tests/l2cap-build/sdkconfig.defaults create mode 100644 .github/tests/l2cap-build/sdkconfig.deferred create mode 100644 .github/workflows/l2cap-build.yml create mode 100644 Makefile create mode 100644 tests/l2cap/L2CAPTests.cpp delete mode 100644 tests/l2cap/SendErrorsTests.cpp create mode 100644 tests/l2cap/stubs/freertos/FreeRTOS.h create mode 100644 tests/l2cap/stubs/freertos/queue.h create mode 100644 tests/l2cap/stubs/freertos/semphr.h create mode 100644 tests/l2cap/stubs/freertos/task.h create mode 100644 tests/l2cap/stubs/host/ble_hs.h create mode 100644 tests/l2cap/stubs/nimble/nimble_npl.h create mode 100644 tests/l2cap/stubs/nimble/nimble_port.h diff --git a/.github/tests/l2cap-build/.gitignore b/.github/tests/l2cap-build/.gitignore new file mode 100644 index 00000000..6006c87e --- /dev/null +++ b/.github/tests/l2cap-build/.gitignore @@ -0,0 +1,5 @@ +build*/ +sdkconfig +sdkconfig.old +managed_components/ +dependencies.lock diff --git a/.github/tests/l2cap-build/CMakeLists.txt b/.github/tests/l2cap-build/CMakeLists.txt new file mode 100644 index 00000000..ef77e2d8 --- /dev/null +++ b/.github/tests/l2cap-build/CMakeLists.txt @@ -0,0 +1,6 @@ +cmake_minimum_required(VERSION 3.16) +get_filename_component(NIMBLE_CPP_ROOT "${CMAKE_CURRENT_LIST_DIR}/../../.." ABSOLUTE) +set(EXTRA_COMPONENT_DIRS "${NIMBLE_CPP_ROOT}") +set(COMPONENTS main) +include($ENV{IDF_PATH}/tools/cmake/project.cmake) +project(nimble_l2cap_build) diff --git a/.github/tests/l2cap-build/main/CMakeLists.txt b/.github/tests/l2cap-build/main/CMakeLists.txt new file mode 100644 index 00000000..d13d789e --- /dev/null +++ b/.github/tests/l2cap-build/main/CMakeLists.txt @@ -0,0 +1 @@ +idf_component_register(SRCS "main.cpp" REQUIRES esp-nimble-cpp) diff --git a/.github/tests/l2cap-build/main/main.cpp b/.github/tests/l2cap-build/main/main.cpp new file mode 100644 index 00000000..4e0aea4c --- /dev/null +++ b/.github/tests/l2cap-build/main/main.cpp @@ -0,0 +1,49 @@ +#include "NimBLEDevice.h" +#include "NimBLEL2CAPServer.h" +#include "NimBLEL2CAPChannel.h" +#include "freertos/queue.h" +#include +#include + +// Compile/link probe and minimal worker-driven echo pattern. The service lives +// for the process lifetime; no callback deletes its channel or blocks for TX. +struct EchoJob { + NimBLEL2CAPChannel* channel; + uint32_t generation; + std::vector data; +}; +static QueueHandle_t jobs; +static std::atomic generation{0}; + +class EchoCallbacks : public NimBLEL2CAPChannelCallbacks { + void onConnect(NimBLEL2CAPChannel*, uint16_t) override { ++generation; } + void onDisconnect(NimBLEL2CAPChannel*) override { ++generation; } + void onRead(NimBLEL2CAPChannel* channel, std::vector& bytes) override { + auto* job = new EchoJob{channel, generation.load(), bytes}; + if (xQueueSend(jobs, &job, 0) != pdTRUE) { + delete job; + channel->disconnect(); + } + } +}; + +static void echoWorker(void*) { + EchoJob* job; + while (true) { + if (xQueueReceive(jobs, &job, portMAX_DELAY) != pdTRUE) continue; + if (job->generation == generation.load() && job->channel->isConnected()) { + job->channel->write(job->data); + } + delete job; + } +} + +extern "C" void app_main() { + jobs = xQueueCreate(4, sizeof(EchoJob*)); + assert(jobs); + const auto created = xTaskCreate(echoWorker, "coc-echo", 4096, nullptr, 5, nullptr); + assert(created == pdPASS); + NimBLEDevice::init("coc-build-probe"); + auto* channel = NimBLEDevice::createL2CAPServer()->createService(129, 4096, new EchoCallbacks); + (void)channel->isIdle(); +} diff --git a/.github/tests/l2cap-build/sdkconfig.defaults b/.github/tests/l2cap-build/sdkconfig.defaults new file mode 100644 index 00000000..23e62ae2 --- /dev/null +++ b/.github/tests/l2cap-build/sdkconfig.defaults @@ -0,0 +1,4 @@ +CONFIG_BT_ENABLED=y +CONFIG_BT_NIMBLE_ENABLED=y +CONFIG_BT_NIMBLE_L2CAP_COC_MAX_NUM=1 +CONFIG_NIMBLE_CPP_L2CAP_DEFERRED_READ_CALLBACKS=n diff --git a/.github/tests/l2cap-build/sdkconfig.deferred b/.github/tests/l2cap-build/sdkconfig.deferred new file mode 100644 index 00000000..a0d55aee --- /dev/null +++ b/.github/tests/l2cap-build/sdkconfig.deferred @@ -0,0 +1 @@ +CONFIG_NIMBLE_CPP_L2CAP_DEFERRED_READ_CALLBACKS=y diff --git a/.github/workflows/l2cap-build.yml b/.github/workflows/l2cap-build.yml new file mode 100644 index 00000000..f79911b0 --- /dev/null +++ b/.github/workflows/l2cap-build.yml @@ -0,0 +1,38 @@ +name: L2CAP CoC build +on: + workflow_dispatch: + pull_request: + push: + branches: [master] +jobs: + coc: + runs-on: ubuntu-latest + strategy: + fail-fast: false + matrix: + idf: [v5.4.3, v5.5.1] + callbacks: [inline, deferred] + container: espressif/idf:${{ matrix.idf }} + steps: + - uses: actions/checkout@v5 + with: + path: esp-nimble-cpp + - name: Compile and link CoC implementation + env: + IDF_TARGET: esp32s3 + CALLBACKS: ${{ matrix.callbacks }} + shell: bash + run: | + . "${IDF_PATH}/export.sh" + cd esp-nimble-cpp/.github/tests/l2cap-build + defaults="sdkconfig.defaults" + if [ "$CALLBACKS" = deferred ]; then defaults="$defaults;sdkconfig.deferred"; fi + idf.py -D "SDKCONFIG_DEFAULTS=$defaults" build + python - "$CALLBACKS" <<'PYTHON' + import sys + from pathlib import Path + config = Path("sdkconfig").read_text().splitlines() + assert "CONFIG_BT_NIMBLE_L2CAP_COC_MAX_NUM=1" in config + enabled = "CONFIG_NIMBLE_CPP_L2CAP_DEFERRED_READ_CALLBACKS=y" in config + assert enabled == (sys.argv[1] == "deferred") + PYTHON diff --git a/Kconfig b/Kconfig index bfd47962..f253211e 100644 --- a/Kconfig +++ b/Kconfig @@ -197,4 +197,65 @@ config NIMBLE_CPP_DEBUG_ASSERT_ENABLED Enabling this option will add debug asserts to the NimBLE CPP library. This will use approximately 1kB of flash memory. -endmenu \ No newline at end of file +config NIMBLE_CPP_L2CAP_SDU_BUFFER_COUNT + int "L2CAP SDU buffer count." + range 2 32 + default 5 + help + Number of full-SDU mbufs allocated per NimBLEL2CAPChannel instance for + local CoC send/receive work. Each buffer is sized to the channel MTU, so + increasing this directly increases per-channel heap consumption. + + Higher values improve tolerance for bursty traffic, overlapping send and + receive work, deferred read callback lag, and other short periods where + the channel needs more than one full SDU buffer ready at once. + + Lower values reduce memory footprint, but make the channel more fragile + under load. If this value is too small for the negotiated MTU and the way + the application consumes callbacks, local allocations may fail and large + SDUs can start timing out or disconnecting even though CoC MTU negotiation + itself succeeded. + + The default value of 5 preserves the currently validated behavior. Reduce + it only when memory is tight and the application traffic pattern is known + to be light; increase it if callbacks may lag or if multiple large SDUs + must be in flight locally. + +config NIMBLE_CPP_L2CAP_DEFERRED_READ_CALLBACKS + bool "Defer L2CAP read callbacks to a worker task." + default "n" + help + When enabled, NimBLEL2CAPChannel rearms receive first on the NimBLE host task + and then dispatches onConnect() and onRead() callbacks on a shared worker task. + Disconnect callbacks always run on that worker, outside the NimBLE host lock. + This reduces the risk of doing heavy application work directly on the NimBLE host path. + +if NIMBLE_CPP_L2CAP_DEFERRED_READ_CALLBACKS + +config NIMBLE_CPP_L2CAP_CALLBACK_TASK_STACK_SIZE + int "L2CAP callback worker stack size." + range 2048 32768 + default 4096 + help + Stack size in bytes for the shared worker task used to invoke deferred + L2CAP read callbacks. + +config NIMBLE_CPP_L2CAP_CALLBACK_TASK_PRIORITY + int "L2CAP callback worker task priority." + range 1 24 + default 5 + help + FreeRTOS priority for the shared worker task used to invoke deferred + L2CAP read callbacks. + +config NIMBLE_CPP_L2CAP_CALLBACK_QUEUE_LENGTH + int "L2CAP callback queue length." + range 4 128 + default 16 + help + Number of queued deferred L2CAP read callbacks. Overflow aborts the + connection; additional reserved slots guarantee lifecycle delivery. + +endif + +endmenu diff --git a/Makefile b/Makefile new file mode 100644 index 00000000..b44d0ee9 --- /dev/null +++ b/Makefile @@ -0,0 +1,16 @@ +.DEFAULT_GOAL := help +SANITIZER ?= none +L2CAP_BUILD ?= build/l2cap-$(SANITIZER) +SAN_FLAGS_none := +SAN_FLAGS_address := -fsanitize=address,undefined -fno-omit-frame-pointer +SAN_FLAGS_thread := -fsanitize=thread -fno-omit-frame-pointer + +.PHONY: help test-l2cap +help: ## Show development targets. + @awk 'BEGIN { print "Usage: make [SANITIZER=none|address|thread] [L2CAP_BUILD=path]\n" } /^[a-zA-Z0-9_.-]+:.*##/ { split($$0, a, ":.*## "); printf " %-18s %s\n", a[1], a[2] }' $(MAKEFILE_LIST) + +test-l2cap: ## Build and run the L2CAP concurrency and error tests. + @case "$(SANITIZER)" in none|address|thread) ;; *) echo "Unknown SANITIZER: use none, address or thread" >&2; exit 2;; esac + cmake -S tests/l2cap -B "$(L2CAP_BUILD)" -DCMAKE_CXX_FLAGS="$(SAN_FLAGS_$(SANITIZER))" + cmake --build "$(L2CAP_BUILD)" + ctest --test-dir "$(L2CAP_BUILD)" --output-on-failure diff --git a/src/NimBLEDevice.cpp b/src/NimBLEDevice.cpp index e125aeb9..9504a4ea 100644 --- a/src/NimBLEDevice.cpp +++ b/src/NimBLEDevice.cpp @@ -1030,7 +1030,17 @@ bool NimBLEDevice::deinit(bool clearAll) { int rc = 0; if (m_initialized) { rc = nimble_port_stop(); + if (rc != 0) { return false; } if (rc == 0) { +# if MYNEWT_VAL(BLE_ROLE_PERIPHERAL) && MYNEWT_VAL(BLE_L2CAP_COC_MAX_NUM) + // The stopped host cannot enqueue new channel jobs. Destroy CoC + // events while the NPL function table and event pool still exist; + // nimble_port_deinit() releases both on ESP-IDF. + if (clearAll && m_pL2CAPServer != nullptr) { + delete m_pL2CAPServer; + m_pL2CAPServer = nullptr; + } +# endif # if MYNEWT_VAL(BLE_ROLE_OBSERVER) if (NimBLEDevice::m_pScan != nullptr) { NimBLEDevice::m_pScan->onHostDeinit(); diff --git a/src/NimBLEL2CAPChannel.cpp b/src/NimBLEL2CAPChannel.cpp index 8868ad86..169eb2f9 100644 --- a/src/NimBLEL2CAPChannel.cpp +++ b/src/NimBLEL2CAPChannel.cpp @@ -8,6 +8,13 @@ # include "NimBLEClient.h" # include "NimBLELog.h" # include "NimBLEUtils.h" +# include "freertos/queue.h" +# include "freertos/task.h" +# ifdef USING_NIMBLE_ARDUINO_HEADERS +# include "nimble/porting/nimble/include/nimble/nimble_port.h" +# else +# include "nimble/nimble_port.h" +# endif # include # ifdef USING_NIMBLE_ARDUINO_HEADERS @@ -16,47 +23,238 @@ # include "host/ble_gap.h" # endif -// L2CAP buffer block size -# define L2CAP_BUF_BLOCK_SIZE (250) -# define L2CAP_BUF_SIZE_MTUS_PER_CHANNEL (3) -// Round-up integer division -# define CEIL_DIVIDE(a, b) (((a) + (b) - 1) / (b)) -# define ROUND_DIVIDE(a, b) (((a) + (b) / 2) / (b)) +// Allocate one full SDU per mbuf, matching NimBLE's own CoC examples. +# define L2CAP_SDU_BUFFER_COUNT CONFIG_NIMBLE_CPP_L2CAP_SDU_BUFFER_COUNT +// Retry +constexpr uint32_t SendTimeoutMs = 2000; +#ifndef CONFIG_NIMBLE_CPP_L2CAP_CALLBACK_QUEUE_LENGTH +#define CONFIG_NIMBLE_CPP_L2CAP_CALLBACK_QUEUE_LENGTH 16 +#define CONFIG_NIMBLE_CPP_L2CAP_CALLBACK_TASK_STACK_SIZE 4096 +#define CONFIG_NIMBLE_CPP_L2CAP_CALLBACK_TASK_PRIORITY 5 +#endif + +static void logPoolSanityWarning(const char* tag, uint16_t psm, const ble_l2cap_chan_info& info) { + const uint16_t negotiatedCocMtu = info.peer_coc_mtu < info.our_coc_mtu ? info.peer_coc_mtu : info.our_coc_mtu; + const uint16_t effectiveL2capMtu = info.peer_l2cap_mtu < info.our_l2cap_mtu ? info.peer_l2cap_mtu : info.our_l2cap_mtu; + + if (negotiatedCocMtu == 0 || effectiveL2capMtu == 0) { + return; + } + + const uint32_t fragmentsPerSdu = (negotiatedCocMtu + effectiveL2capMtu - 1) / effectiveL2capMtu; + const uint32_t msys1BlockCount = MYNEWT_VAL(MSYS_1_BLOCK_COUNT); + const uint32_t msys1BlockSize = MYNEWT_VAL(MSYS_1_BLOCK_SIZE); + const uint32_t cocMps = MYNEWT_VAL(BLE_L2CAP_COC_MPS); + const uint32_t aclFromLlCount = MYNEWT_VAL(BLE_TRANSPORT_ACL_FROM_LL_COUNT); + const uint32_t localSduBufferCount = L2CAP_SDU_BUFFER_COUNT; + + NIMBLE_LOGI(tag, + "L2CAP COC 0x%04X path geometry: negotiated_coc=%u effective_l2cap=%u fragments_per_sdu=%lu mps=%lu local_sdu_bufs=%lu msys1=%lux%lu acl_from_ll=%lu", + psm, + negotiatedCocMtu, + effectiveL2capMtu, + (unsigned long)fragmentsPerSdu, + (unsigned long)cocMps, + (unsigned long)localSduBufferCount, + (unsigned long)msys1BlockCount, + (unsigned long)msys1BlockSize, + (unsigned long)aclFromLlCount); + + if (effectiveL2capMtu < negotiatedCocMtu && + (fragmentsPerSdu >= msys1BlockCount || fragmentsPerSdu * 2 >= msys1BlockCount || fragmentsPerSdu >= aclFromLlCount)) { + NIMBLE_LOGW(tag, + "L2CAP COC 0x%04X large-SDU risk: negotiated MTU %u requires %lu fragments at L2CAP MTU %u. Current pools (MSYS_1=%lux%lu, ACL_FROM_LL=%lu) may cause ENOMEM/timeouts under load.", + psm, + negotiatedCocMtu, + (unsigned long)fragmentsPerSdu, + effectiveL2capMtu, + (unsigned long)msys1BlockCount, + (unsigned long)msys1BlockSize, + (unsigned long)aclFromLlCount); + } + + if (localSduBufferCount < 3) { + NIMBLE_LOGW(tag, + "L2CAP COC 0x%04X local SDU buffer count is %lu. Very small per-channel SDU pools reduce heap use but can make large MTUs fragile when send/receive work overlaps or callbacks lag.", + psm, + (unsigned long)localSduBufferCount); + } +} + +enum CallbackKind : uint8_t { Connected, Read, Disconnected }; +struct DeferredReadItem { + NimBLEL2CAPChannel* channel; + std::vector* data; + uint32_t generation; + uint8_t kind; +}; +static QueueHandle_t s_l2capDeferredReadQueue = nullptr; +static TaskHandle_t s_l2capDeferredReadTask = nullptr; +static size_t s_l2capChannelCount = 0; +static constexpr size_t LifecycleSlots = 2 * MYNEWT_VAL(BLE_L2CAP_COC_MAX_NUM); + +void deferredReadWorker(void*) { + DeferredReadItem item{}; + while (true) { + if (xQueueReceive(s_l2capDeferredReadQueue, &item, portMAX_DELAY) != pdTRUE) { continue; } + auto* ch = item.channel; + if (item.generation == ch->m_generation.load()) { + if (item.kind == Connected) { + ch->callbacks->onConnect(ch, ch->m_negotiatedMTU.load()); + } else if (item.kind == Read && ch->isConnected()) { + ch->callbacks->onRead(ch, *item.data); + } else if (item.kind == Disconnected) { + // Off the host task: callbacks may join CAN RX or finish a command. + ch->callbacks->onDisconnect(ch); + while (ch->m_pendingHostJobs.load() || ch->m_writers.load()) { vTaskDelay(1); } + ch->m_state.store(NimBLEL2CAPChannel::State::idle); + } + } + delete item.data; + ch->m_pendingDeferredReads.fetch_sub(1); + } +} + +bool ensureDeferredReadWorker() { + // Channel construction belongs to the application's initialization task. + if (s_l2capDeferredReadQueue && s_l2capDeferredReadTask) { return true; } + s_l2capDeferredReadQueue = xQueueCreate(CONFIG_NIMBLE_CPP_L2CAP_CALLBACK_QUEUE_LENGTH + LifecycleSlots, + sizeof(DeferredReadItem)); + if (!s_l2capDeferredReadQueue) { return false; } + return xTaskCreate(deferredReadWorker, "nimble_l2cap_cb", CONFIG_NIMBLE_CPP_L2CAP_CALLBACK_TASK_STACK_SIZE, + nullptr, CONFIG_NIMBLE_CPP_L2CAP_CALLBACK_TASK_PRIORITY, &s_l2capDeferredReadTask) == pdPASS; +} + +void NimBLEL2CAPChannel::dispatchCallback(uint8_t kind, std::vector* data) { + // The host task is the only producer. Reserve two lifecycle slots per + // channel; reconnect is refused until its disconnect callback has finished. + if (kind == Read && uxQueueSpacesAvailable(s_l2capDeferredReadQueue) <= LifecycleSlots) { + delete data; + disconnect(); + return; + } + DeferredReadItem item{this, data, m_generation.load(), kind}; + m_pendingDeferredReads.fetch_add(1); + auto result = xQueueSend(s_l2capDeferredReadQueue, &item, 0); + assert(result == pdTRUE); +} + +struct NimBLEL2CAPChannel::TxCompletion { + SemaphoreHandle_t ready = xSemaphoreCreateBinary(); + std::atomic status{BLE_HS_EUNKNOWN}; + std::atomic completed{false}; + ~TxCompletion() { vSemaphoreDelete(ready); } + void reset() { + // The whole-PDU mutex allows only one waiter. A timeout closes the + // session, and accept waits for the old host job before reusing this. + xSemaphoreTake(ready, 0); + status.store(BLE_HS_EUNKNOWN); + completed.store(false); + } + void finish(int value) { + if (completed.exchange(true)) { return; } + status.store(value); + xSemaphoreGive(ready); + } +}; +void NimBLEL2CAPChannel::transmitOnHost(ble_npl_event* event) { + auto* ch = static_cast(ble_npl_event_get_arg(event)); + auto* buffer = ch->m_txBuffer.exchange(nullptr); + if (!buffer) { return; } + if (!ch->isConnected() || ch->m_txGeneration.load() != ch->m_generation.load() || !ch->channel) { + os_mbuf_free_chain(buffer); + ch->m_completion->finish(BLE_HS_ENOTCONN); + } else { + // Arm before send. An immediate TX_UNSTALLED must not be discarded. + ch->m_activeSend = ch->m_completion.get(); + int rc = ble_l2cap_send(ch->channel, buffer); + // Pinned NimBLE consumes the SDU even on errors from continue_tx. + // Only these two early returns leave ownership with the caller. + if (rc == BLE_HS_EBADDATA || rc == BLE_HS_EBUSY) { os_mbuf_free_chain(buffer); } + if (rc != BLE_HS_ESTALLED) { + ch->m_completion->finish(rc); + ch->m_activeSend = nullptr; + } + } + ch->m_pendingHostJobs.fetch_sub(1); +} + +void NimBLEL2CAPChannel::disconnectOnHost(ble_npl_event* event) { + auto* ch = static_cast(ble_npl_event_get_arg(event)); + if (ch->m_activeSend) { + ch->m_activeSend->finish(BLE_HS_ENOTCONN); + ch->m_activeSend = nullptr; + } + if (ch->channel) { ble_l2cap_disconnect(ch->channel); } + ch->m_pendingHostJobs.fetch_sub(1); +} NimBLEL2CAPChannel::NimBLEL2CAPChannel(uint16_t psm, uint16_t mtu, NimBLEL2CAPChannelCallbacks* callbacks) : psm(psm), mtu(mtu), callbacks(callbacks) { assert(mtu); // fail here, if MTU is too little assert(callbacks); // fail here, if no callbacks are given - assert(setupMemPool()); // fail here, if the memory pool could not be setup + const bool poolReady = setupMemPool(); + assert(poolReady); // fail here, if the memory pool could not be setup + const bool workerReady = ensureDeferredReadWorker(); + assert(workerReady); + ++s_l2capChannelCount; + m_writeMutex = xSemaphoreCreateMutex(); + assert(m_writeMutex); + m_completion = std::make_unique(); + assert(m_completion->ready); + ble_npl_event_init(&m_txEvent, transmitOnHost, this); + ble_npl_event_init(&m_disconnectEvent, disconnectOnHost, this); NIMBLE_LOGI(LOG_TAG, "L2CAP COC 0x%04X initialized w/ L2CAP MTU %i", this->psm, this->mtu); }; NimBLEL2CAPChannel::~NimBLEL2CAPChannel() { + while (m_pendingDeferredReads.load() || m_pendingHostJobs.load() || m_writers.load()) { + vTaskDelay(pdMS_TO_TICKS(1)); + } + ble_npl_event_deinit(&m_txEvent); + ble_npl_event_deinit(&m_disconnectEvent); teardownMemPool(); + // All producers and callbacks have drained before the last channel is + // destroyed. Keeping the shared worker would retain its internal stack + // and queue across a full Bluetooth shutdown. + if (--s_l2capChannelCount == 0) { + vTaskDelete(s_l2capDeferredReadTask); + s_l2capDeferredReadTask = nullptr; + vQueueDelete(s_l2capDeferredReadQueue); + s_l2capDeferredReadQueue = nullptr; + } + NIMBLE_LOGI(LOG_TAG, "L2CAP COC 0x%04X shutdown and freed.", this->psm); } bool NimBLEL2CAPChannel::setupMemPool() { - const size_t buf_blocks = CEIL_DIVIDE(mtu, L2CAP_BUF_BLOCK_SIZE) * L2CAP_BUF_SIZE_MTUS_PER_CHANNEL; - NIMBLE_LOGD(LOG_TAG, "Computed number of buf_blocks = %d", buf_blocks); + const size_t buf_blocks = L2CAP_SDU_BUFFER_COUNT; + // Pool block size includes both headers. Without this headroom a full + // local-MTU packet would silently need more than one configured buffer. + const size_t blockSize = size_t(mtu) + sizeof(os_mbuf) + sizeof(os_mbuf_pkthdr); + if (blockSize > UINT16_MAX) { return false; } + NIMBLE_LOGD(LOG_TAG, "Allocating %d L2CAP SDU buffers of %d bytes", buf_blocks, mtu); + + memset(&_coc_mempool, 0, sizeof(_coc_mempool)); + memset(&_coc_mbuf_pool, 0, sizeof(_coc_mbuf_pool)); - _coc_memory = malloc(OS_MEMPOOL_SIZE(buf_blocks, L2CAP_BUF_BLOCK_SIZE) * sizeof(os_membuf_t)); + _coc_memory = malloc(OS_MEMPOOL_SIZE(buf_blocks, blockSize) * sizeof(os_membuf_t)); if (_coc_memory == 0) { NIMBLE_LOGE(LOG_TAG, "Can't allocate _coc_memory: %d", errno); return false; } - auto rc = os_mempool_init(&_coc_mempool, buf_blocks, L2CAP_BUF_BLOCK_SIZE, _coc_memory, "appbuf"); + auto rc = os_mempool_init(&_coc_mempool, buf_blocks, blockSize, _coc_memory, "appbuf"); if (rc != 0) { NIMBLE_LOGE(LOG_TAG, "Can't os_mempool_init: %d", rc); return false; } - auto rc2 = os_mbuf_pool_init(&_coc_mbuf_pool, &_coc_mempool, L2CAP_BUF_BLOCK_SIZE, buf_blocks); + auto rc2 = os_mbuf_pool_init(&_coc_mbuf_pool, &_coc_mempool, blockSize, buf_blocks); if (rc2 != 0) { - NIMBLE_LOGE(LOG_TAG, "Can't os_mbuf_pool_init: %d", rc); + NIMBLE_LOGE(LOG_TAG, "Can't os_mbuf_pool_init: %d", rc2); return false; } @@ -70,63 +268,49 @@ bool NimBLEL2CAPChannel::setupMemPool() { } void NimBLEL2CAPChannel::teardownMemPool() { + if (m_writeMutex) { + vSemaphoreDelete(m_writeMutex); + m_writeMutex = nullptr; + } if (this->callbacks) { delete this->callbacks; + this->callbacks = nullptr; } if (this->receiveBuffer) { free(this->receiveBuffer); + this->receiveBuffer = nullptr; + } + if (_coc_mempool.name) { + os_mempool_unregister(&_coc_mempool); + memset(&_coc_mempool, 0, sizeof(_coc_mempool)); + memset(&_coc_mbuf_pool, 0, sizeof(_coc_mbuf_pool)); } if (_coc_memory) { free(_coc_memory); + _coc_memory = nullptr; } } int NimBLEL2CAPChannel::writeFragment(std::vector::const_iterator begin, std::vector::const_iterator end) { - auto toSend = end - begin; - - if (stalled) { - NIMBLE_LOGD(LOG_TAG, "L2CAP Channel waiting for unstall..."); - NimBLEUtils::TaskData taskData; - m_pTaskData = &taskData; - NimBLEUtils::taskWait(taskData, BLE_NPL_TIME_FOREVER); - m_pTaskData = nullptr; - stalled = false; - NIMBLE_LOGD(LOG_TAG, "L2CAP Channel unstalled!"); - } - - struct ble_l2cap_chan_info info; - ble_l2cap_get_chan_info(channel, &info); - // Take the minimum of our and peer MTU - auto mtu = info.peer_coc_mtu < info.our_coc_mtu ? info.peer_coc_mtu : info.our_coc_mtu; - - if (toSend > mtu) { - return -BLE_HS_EBADDATA; - } - - auto* txd = os_mbuf_get_pkthdr(&_coc_mbuf_pool, 0); - if (!txd) { - return BLE_HS_ENOMEM; + auto* buffer = os_mbuf_get_pkthdr(&_coc_mbuf_pool, 0); + if (!buffer) { return BLE_HS_ENOMEM; } + int rc = os_mbuf_append(buffer, &*begin, end - begin); + if (rc) { os_mbuf_free_chain(buffer); return rc; } + auto* completion = m_completion.get(); + completion->reset(); + m_txGeneration.store(m_generation.load()); + m_pendingHostJobs.fetch_add(1); + m_txBuffer.store(buffer); + if (xTaskGetCurrentTaskHandle() == m_hostTask.load()) { + transmitOnHost(&m_txEvent); + // A host callback must never wait for host progress. + if (xSemaphoreTake(completion->ready, 0) != pdTRUE) { return BLE_HS_ETIMEOUT; } + } else { + // At most one TX event and one disconnect event are outstanding per channel. + ble_npl_eventq_put(nimble_port_get_dflt_eventq(), &m_txEvent); + if (xSemaphoreTake(completion->ready, pdMS_TO_TICKS(SendTimeoutMs)) != pdTRUE) { return BLE_HS_ETIMEOUT; } } - const int append = os_mbuf_append(txd, &*begin, toSend); - if (append != 0) { - os_mbuf_free_chain(txd); - return append; - } - - const int rc = ble_l2cap_send(channel, txd); - if (rc == BLE_HS_ESTALLED) { - // NimBLE owns the SDU and will finish it when credits arrive. - stalled = true; - return 0; - } - // These early validation/busy errors retain caller ownership. All other - // native send results consume the SDU, including ENOMEM and EAGAIN. - if (rc == BLE_HS_EBADDATA || rc == BLE_HS_EBUSY) { - os_mbuf_free_chain(txd); - } - // A native error can follow partial transmission of this SDU. Resubmitting - // it could duplicate bytes; let write() abort the incomplete stream. - return rc; + return completion->status.load(); } # if MYNEWT_VAL(BLE_ROLE_CENTRAL) @@ -146,90 +330,136 @@ NimBLEL2CAPChannel* NimBLEL2CAPChannel::connect(NimBLEClient* cli auto sdu_rx = os_mbuf_get_pkthdr(&channel->_coc_mbuf_pool, 0); if (!sdu_rx) { NIMBLE_LOGE(LOG_TAG, "Can't allocate SDU buffer: %d, %s", errno, strerror(errno)); + delete channel; return nullptr; } auto rc = ble_l2cap_connect(client->getConnHandle(), psm, mtu, sdu_rx, NimBLEL2CAPChannel::handleL2capEvent, channel); if (rc != 0) { NIMBLE_LOGE(LOG_TAG, "ble_l2cap_connect failed: %d", rc); + os_mbuf_free_chain(sdu_rx); + delete channel; + return nullptr; } return channel; } # endif // MYNEWT_VAL(BLE_ROLE_CENTRAL) bool NimBLEL2CAPChannel::write(const std::vector& bytes) { - if (!this->channel) { - NIMBLE_LOGW(LOG_TAG, "L2CAP Channel not open"); - return false; - } - - struct ble_l2cap_chan_info info; - ble_l2cap_get_chan_info(channel, &info); - auto mtu = info.peer_coc_mtu < info.our_coc_mtu ? info.peer_coc_mtu : info.our_coc_mtu; - + const auto generation = m_generation.load(); + m_writers.fetch_add(1); + struct WriterExit { std::atomic& count; ~WriterExit() { count.fetch_sub(1); } } exit{m_writers}; + if (!isConnected()) { return false; } + auto wait = xTaskGetCurrentTaskHandle() == m_hostTask.load() ? 0 : pdMS_TO_TICKS(SendTimeoutMs); + if (xSemaphoreTake(m_writeMutex, wait) != pdTRUE) { disconnect(); return false; } + struct Unlock { SemaphoreHandle_t mutex; ~Unlock() { xSemaphoreGive(mutex); } } unlock{m_writeMutex}; + if (!isConnected() || generation != m_generation.load()) { return false; } + const auto width = m_txSduWidth.load(); + if (!width) { disconnect(); return false; } auto start = bytes.begin(); while (start != bytes.end()) { - auto end = start + std::min(mtu, bytes.end() - start); - if (writeFragment(start, end) != 0) { - disconnect(); - return false; - } + if (!isConnected()) { return false; } + const auto length = std::min(width, bytes.end() - start); + auto end = start + length; + if (writeFragment(start, end) != 0) { disconnect(); return false; } start = end; } return true; } bool NimBLEL2CAPChannel::disconnect() { - if (!this->channel) { - NIMBLE_LOGW(LOG_TAG, "L2CAP Channel not open"); - return false; - } - - int rc = ble_l2cap_disconnect(this->channel); - if (rc != 0 && rc != BLE_HS_ENOTCONN && rc != BLE_HS_EALREADY) { - NIMBLE_LOGE(LOG_TAG, "ble_l2cap_disconnect failed: rc=%d %s", rc, NimBLEUtils::returnCodeToString(rc)); - return false; - } - + auto state = m_state.load(); + do { + if (state == State::idle || state == State::closing) { return false; } + } while (!m_state.compare_exchange_weak(state, State::closing)); + m_pendingHostJobs.fetch_add(1); + if (xTaskGetCurrentTaskHandle() == m_hostTask.load()) { disconnectOnHost(&m_disconnectEvent); } + else { ble_npl_eventq_put(nimble_port_get_dflt_eventq(), &m_disconnectEvent); } return true; } uint16_t NimBLEL2CAPChannel::getConnHandle() const { - if (!this->channel) { - return BLE_HS_CONN_HANDLE_NONE; - } - return ble_l2cap_get_conn_handle(this->channel); + return m_connHandle.load(); } // private int NimBLEL2CAPChannel::handleConnectionEvent(struct ble_l2cap_event* event) { + m_hostTask = xTaskGetCurrentTaskHandle(); channel = event->connect.chan; + m_generation.fetch_add(1); + m_connHandle.store(ble_l2cap_get_conn_handle(channel)); + m_state.store(State::open); struct ble_l2cap_chan_info info; - ble_l2cap_get_chan_info(channel, &info); + int rc = ble_l2cap_get_chan_info(channel, &info); + if (rc != 0) { + NIMBLE_LOGE(LOG_TAG, "L2CAP COC 0x%04X connected but ble_l2cap_get_chan_info failed: %d", psm, rc); + disconnect(); + return rc; + } NIMBLE_LOGI(LOG_TAG, - "L2CAP COC 0x%04X connected. Local MTU = %d [%d], remote MTU = %d [%d].", + "L2CAP COC 0x%04X connected. scid=0x%04X dcid=0x%04X local_l2cap=%d local_coc=%d peer_l2cap=%d peer_coc=%d.", psm, - info.our_coc_mtu, + info.scid, + info.dcid, info.our_l2cap_mtu, - info.peer_coc_mtu, - info.peer_l2cap_mtu); - if (info.our_coc_mtu > info.peer_coc_mtu) { + info.our_coc_mtu, + info.peer_l2cap_mtu, + info.peer_coc_mtu); + logPoolSanityWarning(LOG_TAG, psm, info); + if (info.our_coc_mtu > 0 && info.peer_coc_mtu > 0 && info.our_coc_mtu > info.peer_coc_mtu) { NIMBLE_LOGW(LOG_TAG, "L2CAP COC 0x%04X connected, but local MTU is bigger than remote MTU.", psm); } - auto mtu = info.peer_coc_mtu < info.our_coc_mtu ? info.peer_coc_mtu : info.our_coc_mtu; - callbacks->onConnect(this, mtu); + + uint16_t negotiatedMTU = 0; + if (info.our_coc_mtu > 0 && info.peer_coc_mtu > 0) { + negotiatedMTU = info.peer_coc_mtu < info.our_coc_mtu ? info.peer_coc_mtu : info.our_coc_mtu; + } else if (info.our_l2cap_mtu > 0 && info.peer_l2cap_mtu > 0) { + negotiatedMTU = info.peer_l2cap_mtu < info.our_l2cap_mtu ? info.peer_l2cap_mtu : info.our_l2cap_mtu; + NIMBLE_LOGW(LOG_TAG, + "L2CAP COC 0x%04X connected with zero CoC MTU info; falling back to L2CAP MTU %u for callback.", + psm, + negotiatedMTU); + } else { + negotiatedMTU = mtu; + NIMBLE_LOGW(LOG_TAG, + "L2CAP COC 0x%04X connected with no peer MTU info; falling back to configured MTU %u for callback.", + psm, + negotiatedMTU); + } + + m_negotiatedMTU.store(negotiatedMTU); + uint16_t txSduWidth = negotiatedMTU; + m_txSduWidth.store(txSduWidth); +#if CONFIG_NIMBLE_CPP_L2CAP_DEFERRED_READ_CALLBACKS + dispatchCallback(Connected); +#else + callbacks->onConnect(this, negotiatedMTU); +#endif return 0; } int NimBLEL2CAPChannel::handleAcceptEvent(struct ble_l2cap_event* event) { NIMBLE_LOGI(LOG_TAG, "L2CAP COC 0x%04X accept.", psm); - if (!callbacks->shouldAcceptConnection(this)) { + m_hostTask = xTaskGetCurrentTaskHandle(); + if (m_state.load() != State::idle || m_pendingDeferredReads.load() || m_pendingHostJobs.load() || m_writers.load() || + !callbacks->shouldAcceptConnection(this)) { NIMBLE_LOGI(LOG_TAG, "L2CAP COC 0x%04X refused by delegate.", psm); return -1; } struct os_mbuf* sdu_rx = os_mbuf_get_pkthdr(&_coc_mbuf_pool, 0); - assert(sdu_rx != NULL); - ble_l2cap_recv_ready(event->accept.chan, sdu_rx); + if (!sdu_rx) { + NIMBLE_LOGE(LOG_TAG, "L2CAP COC 0x%04X could not allocate receive SDU.", psm); + return BLE_HS_ENOMEM; + } + + int rc = ble_l2cap_recv_ready(event->accept.chan, sdu_rx); + if (rc != 0) { + NIMBLE_LOGE(LOG_TAG, "L2CAP COC 0x%04X ble_l2cap_recv_ready failed during accept: %d", psm, rc); + os_mbuf_free_chain(sdu_rx); + return rc; + } + + m_state.store(State::accepted); return 0; } @@ -251,7 +481,6 @@ int NimBLEL2CAPChannel::handleDataReceivedEvent(struct ble_l2cap_event* event) { assert(res == 0); std::vector incomingData(receiveBuffer, receiveBuffer + rx_len); - callbacks->onRead(this, incomingData); struct os_mbuf* next = os_mbuf_get_pkthdr(&_coc_mbuf_pool, 0); assert(next != NULL); @@ -259,22 +488,35 @@ int NimBLEL2CAPChannel::handleDataReceivedEvent(struct ble_l2cap_event* event) { res = ble_l2cap_recv_ready(channel, next); assert(res == 0); +#if CONFIG_NIMBLE_CPP_L2CAP_DEFERRED_READ_CALLBACKS + dispatchCallback(Read, new std::vector(std::move(incomingData))); +#else + if (isConnected()) { callbacks->onRead(this, incomingData); } +#endif + return 0; } int NimBLEL2CAPChannel::handleTxUnstalledEvent(struct ble_l2cap_event* event) { - if (m_pTaskData != nullptr) { - NimBLEUtils::taskRelease(*m_pTaskData, event->tx_unstalled.status); + if (event->tx_unstalled.chan == channel && m_activeSend) { + m_activeSend->finish(event->tx_unstalled.status); + m_activeSend = nullptr; } - - NIMBLE_LOGI(LOG_TAG, "L2CAP COC 0x%04X transmit unstalled.", psm); return 0; } int NimBLEL2CAPChannel::handleDisconnectionEvent(struct ble_l2cap_event* event) { - NIMBLE_LOGI(LOG_TAG, "L2CAP COC 0x%04X disconnected.", psm); - channel = NULL; - callbacks->onDisconnect(this); + if (!channel || event->disconnect.chan != channel) { return 0; } + // NimBLE invokes this while holding its host lock. Never wait or invoke + // application cleanup here; that cleanup can join a producer using NimBLE. + m_state.store(State::closing); + m_connHandle.store(BLE_HS_CONN_HANDLE_NONE); + channel = nullptr; + if (m_activeSend) { + m_activeSend->finish(BLE_HS_ENOTCONN); + m_activeSend = nullptr; + } + dispatchCallback(Disconnected); return 0; } diff --git a/src/NimBLEL2CAPChannel.h b/src/NimBLEL2CAPChannel.h index 0e196bdb..a2f970f5 100644 --- a/src/NimBLEL2CAPChannel.h +++ b/src/NimBLEL2CAPChannel.h @@ -11,9 +11,11 @@ # include "inttypes.h" # ifdef USING_NIMBLE_ARDUINO_HEADERS # include "nimble/nimble/host/include/host/ble_l2cap.h" +# include "nimble/nimble/host/include/host/ble_hs.h" # include "nimble/porting/nimble/include/os/os_mbuf.h" # else # include "host/ble_l2cap.h" +# include "host/ble_hs.h" # include "os/os_mbuf.h" # endif @@ -26,6 +28,15 @@ # include # include +# include +# ifdef USING_NIMBLE_ARDUINO_HEADERS +# include "nimble/nimble/include/nimble/nimble_npl.h" +# else +# include "nimble/nimble_npl.h" +# endif +# include "freertos/FreeRTOS.h" +# include "freertos/semphr.h" +# include "freertos/task.h" class NimBLEClient; class NimBLEL2CAPChannelCallbacks; @@ -44,7 +55,9 @@ class NimBLEL2CAPChannel { /// @param[in] mtu The MTU to use. Note that this is the local MTU. Upon opening the channel, /// the final MTU will be negotiated to be the minimum of local and remote. /// @param[in] callbacks The callbacks to use. NOTE that these callbacks are called from the - /// context of the NimBLE bluetooth task (`nimble_host`) and MUST be handled as fast as possible. + /// context of the NimBLE bluetooth task (`nimble_host`) unless deferred callbacks are + /// enabled. In that mode onConnect()/onRead() run on a shared worker. + /// onDisconnect() always runs on that worker, serialized after deferred reads. /// @return True if the channel was opened successfully, false otherwise. static NimBLEL2CAPChannel* connect(NimBLEClient* client, uint16_t psm, uint16_t mtu, NimBLEL2CAPChannelCallbacks* callbacks); @@ -54,7 +67,9 @@ class NimBLEL2CAPChannel { /// @return true on success, after the data has been sent. /// @return false, if the data can't be sent. /// - /// NOTE: This function will block until the data has been sent or an error occurred. + /// Whole writes are serialized. Credit waits are bounded; an incomplete write + /// aborts the connection. Call from a worker task for blocking operation. + /// Owners must disconnect and let callbacks finish before deleting a channel. bool write(const std::vector& bytes); /// @brief Disconnect this L2CAP channel. @@ -64,13 +79,18 @@ class NimBLEL2CAPChannel { /// @brief Get the connection handle associated with this channel. /// @return Connection handle, or BLE_HS_CONN_HANDLE_NONE if not connected. uint16_t getConnHandle() const; - /// @return True, if the channel is connected. False, otherwise. - bool isConnected() const { return !!channel; } + bool isConnected() const { return m_state.load() == State::open; } + // Stop accepting new connections before using this to drain a channel. + bool isIdle() const { + return m_state.load() == State::idle && !m_pendingDeferredReads.load() && + !m_pendingHostJobs.load() && !m_writers.load(); + } + + ~NimBLEL2CAPChannel(); protected: NimBLEL2CAPChannel(uint16_t psm, uint16_t mtu, NimBLEL2CAPChannelCallbacks* callbacks); - ~NimBLEL2CAPChannel(); int handleConnectionEvent(struct ble_l2cap_event* event); int handleAcceptEvent(struct ble_l2cap_event* event); @@ -80,6 +100,7 @@ class NimBLEL2CAPChannel { private: friend class NimBLEL2CAPServer; + friend void deferredReadWorker(void*); static constexpr const char* LOG_TAG = "NimBLEL2CAPChannel"; const uint16_t psm; // PSM of the channel @@ -93,9 +114,30 @@ class NimBLEL2CAPChannel { struct os_mempool _coc_mempool; struct os_mbuf_pool _coc_mbuf_pool; - // Runtime handling - std::atomic stalled{false}; - NimBLEUtils::TaskData* m_pTaskData{nullptr}; + // Only host events access the native channel and active SDU. Never hold a + // wrapper lock while entering NimBLE: disconnect callbacks hold ble_hs_lock. + enum class State { idle, accepted, open, closing }; + std::atomic m_state{State::idle}; + std::atomic m_generation{0}; + std::atomic m_negotiatedMTU{0}; + std::atomic m_txSduWidth{0}; + std::atomic m_connHandle{BLE_HS_CONN_HANDLE_NONE}; + std::atomic m_pendingDeferredReads{0}; // includes running callbacks + std::atomic m_pendingHostJobs{0}; + std::atomic m_writers{0}; + SemaphoreHandle_t m_writeMutex{nullptr}; + struct TxCompletion; + std::unique_ptr m_completion; + std::atomic m_txBuffer{nullptr}; + std::atomic m_txGeneration{0}; + TxCompletion* m_activeSend{nullptr}; // host task only + ble_npl_event m_txEvent{}; + ble_npl_event m_disconnectEvent{}; + std::atomic m_hostTask{nullptr}; + static void transmitOnHost(ble_npl_event* event); + static void disconnectOnHost(ble_npl_event* event); + void dispatchCallback(uint8_t kind, std::vector* data = nullptr); + // Allocate / deallocate NimBLE memory pool bool setupMemPool(); diff --git a/tests/l2cap/CMakeLists.txt b/tests/l2cap/CMakeLists.txt index 8bfc65fa..9110aba8 100644 --- a/tests/l2cap/CMakeLists.txt +++ b/tests/l2cap/CMakeLists.txt @@ -1,10 +1,28 @@ cmake_minimum_required(VERSION 3.20) project(L2CAPTests LANGUAGES CXX) enable_testing() -add_executable(l2cap_send_tests SendErrorsTests.cpp ../../src/NimBLEL2CAPChannel.cpp) -target_compile_features(l2cap_send_tests PRIVATE cxx_std_17) -target_include_directories(l2cap_send_tests PRIVATE stubs ../../src) -target_compile_options(l2cap_send_tests PRIVATE -UNDEBUG -include ${CMAKE_CURRENT_SOURCE_DIR}/stubs/Host.h) -foreach(case errors partial ownership append success stalled) - add_test(NAME l2cap_send_${case} COMMAND l2cap_send_tests ${case}) +find_package(Threads REQUIRED) +add_executable(l2cap_tests L2CAPTests.cpp ../../src/NimBLEL2CAPChannel.cpp) +target_compile_features(l2cap_tests PRIVATE cxx_std_17) +target_include_directories(l2cap_tests PRIVATE stubs ../../src) +target_compile_options(l2cap_tests PRIVATE -include ${CMAKE_CURRENT_SOURCE_DIR}/stubs/Host.h) +target_link_libraries(l2cap_tests PRIVATE Threads::Threads) +add_test(NAME l2cap_errors COMMAND l2cap_tests errors) +add_test(NAME l2cap_early_completion COMMAND l2cap_tests early) +add_test(NAME l2cap_disconnect COMMAND l2cap_tests disconnect) +set_tests_properties(l2cap_errors l2cap_early_completion l2cap_disconnect PROPERTIES TIMEOUT 10) + +foreach(case serialize timeout queued_timeout callbacks reconnect ownership lifecycle partial append success pool) + add_test(NAME l2cap_${case} COMMAND l2cap_tests ${case}) + set_tests_properties(l2cap_${case} PROPERTIES TIMEOUT 10) endforeach() + +add_executable(l2cap_inline_tests L2CAPTests.cpp ../../src/NimBLEL2CAPChannel.cpp) +target_compile_features(l2cap_inline_tests PRIVATE cxx_std_17) +target_compile_definitions(l2cap_inline_tests PRIVATE CONFIG_NIMBLE_CPP_L2CAP_DEFERRED_READ_CALLBACKS=0) +target_include_directories(l2cap_inline_tests PRIVATE stubs ../../src) +target_compile_options(l2cap_inline_tests PRIVATE -include ${CMAKE_CURRENT_SOURCE_DIR}/stubs/Host.h) +target_link_libraries(l2cap_inline_tests PRIVATE Threads::Threads) +add_test(NAME l2cap_inline_errors COMMAND l2cap_inline_tests errors) + +add_test(NAME l2cap_inline_lifecycle COMMAND l2cap_inline_tests lifecycle) diff --git a/tests/l2cap/L2CAPTests.cpp b/tests/l2cap/L2CAPTests.cpp new file mode 100644 index 00000000..b51c748a --- /dev/null +++ b/tests/l2cap/L2CAPTests.cpp @@ -0,0 +1,298 @@ +#include "NimBLEL2CAPChannel.h" +#include +#include +#include +using namespace std::chrono_literals; +static void require(bool condition, const char* message) { + if (!condition) throw std::runtime_error(message); +} +static void onHost(std::function fn) { + struct Call { + ble_npl_event event; + std::function fn; + std::shared_ptr> done; + }; + auto done = std::make_shared>(); + auto future = done->get_future(); + auto* call = new Call{{}, std::move(fn), done}; + ble_npl_event_init( + &call->event, + [](ble_npl_event* e) { + std::unique_ptr c(static_cast(ble_npl_event_get_arg(e))); + c->fn(); + c->done->set_value(); + }, + call); + ble_npl_eventq_put(nimble_port_get_dflt_eventq(), &call->event); + future.get(); +} +struct Channel : NimBLEL2CAPChannel { + ble_l2cap_chan native; + Channel(NimBLEL2CAPChannelCallbacks* cb = new NimBLEL2CAPChannelCallbacks) : NimBLEL2CAPChannel(129, 64, cb) { + onHost([&] { + ble_l2cap_event e{}; + e.connect.chan = &native; + handleConnectionEvent(&e); + }); + } + ~Channel() { disconnected(); } + bool accept() { + int result = -1; + onHost([&] { + ble_l2cap_event e{}; + e.accept.chan = &native; + result = handleAcceptEvent(&e); + }); + return result == 0; + } + void connected() { + onHost([&] { + ble_l2cap_event e{}; + e.connect.chan = &native; + handleConnectionEvent(&e); + }); + } + void unstalled(int status = 0) { + ble_l2cap_event e{}; + e.tx_unstalled.status = status; + e.tx_unstalled.chan = &native; + handleTxUnstalledEvent(&e); + } + void disconnected() { + onHost([&] { + ble_l2cap_event e{}; + e.disconnect.chan = &native; + handleDisconnectionEvent(&e); + }); + } + void receive(std::vector bytes) { + onHost([&] { + ble_l2cap_event e{}; + e.receive.sdu_rx = new os_mbuf{std::move(bytes)}; + handleDataReceivedEvent(&e); + }); + } +}; +int main(int argc, char** argv) { + try { + const std::string name = argc > 1 ? argv[1] : "errors"; + if (name == "pool") { + Channel c; + require(hostPayloadCapacity >= 64, "one configured buffer cannot hold a full local-MTU SDU"); + } else if (name == "lifecycle") { + nimble_port_get_dflt_eventq(); + const int baselineQueues = hostQueueCount.load(); + for (int cycle = 0; cycle < 30; ++cycle) { + { + Channel first; + { + Channel second; + require(hostTaskCount == 1 && hostQueueCount == baselineQueues + 1, "channels did not share worker"); + } + require(hostTaskCount == 1 && hostQueueCount == baselineQueues + 1, "worker stopped before last channel"); + } + require(hostTaskCount == 0 && hostQueueCount == baselineQueues, "worker or queue leaked after shutdown"); + } + } else if (name == "errors") { + for (auto error : {BLE_HS_ENOMEM, BLE_HS_EAGAIN, BLE_HS_ENOTCONN, BLE_HS_EINVAL}) { + Channel c; + int sends = 0; + hostDisconnects = 0; + hostSend = [&](os_mbuf* b) { + ++sends; + delete b; + return error; + }; + const bool success = c.write(std::vector(20, 0x55)); + require(!success, "positive send error incorrectly reported as success"); + require(sends == 1, "continued after failed PDU fragment"); + onHost([] {}); + require(hostDisconnects == 1, "failed PDU did not abort stream"); + } + } else if (name == "partial") { + Channel c; + int sends = 0; + std::vector delivered; + hostDisconnects = 0; + hostSend = [&](os_mbuf* b) { + ++sends; + const size_t size = sends == 1 ? b->bytes.size() : 2; + delivered.insert(delivered.end(), b->bytes.begin(), b->bytes.begin() + size); + delete b; + return sends == 1 ? 0 : BLE_HS_ENOMEM; + }; + require(!c.write(std::vector(24, 0x55)), "partial stream reported success"); + onHost([] {}); + require(sends == 2 && delivered.size() == 10 && hostDisconnects == 1, + "partial SDU was replayed or incomplete stream not aborted"); + require(liveMbufs == 0, "partial send leaked mbuf"); + } else if (name == "append") { + Channel c; + std::atomic sends{0}; + hostDisconnects = 0; + appendError = BLE_HS_ENOMEM; + hostSend = [&](os_mbuf* b) { ++sends; delete b; return 0; }; + require(!c.write({1, 2, 3}), "append error reported success"); + onHost([] {}); + require(sends == 0 && liveMbufs == 0 && hostDisconnects == 1, + "append error leaked mbuf or submitted data"); + } else if (name == "success") { + Channel c; + int sends = 0; + std::vector input, received; + for (int i = 0; i < 20; ++i) input.push_back(uint8_t(i)); + hostSend = [&](os_mbuf* b) { + ++sends; + received.insert(received.end(), b->bytes.begin(), b->bytes.end()); + delete b; + return 0; + }; + require(c.write(input) && sends == 3 && received == input && liveMbufs == 0, + "valid fragmented write failed"); + } else if (name == "early") { + Channel c; + hostSend = [&](os_mbuf* b) { + delete b; + c.unstalled(); + return BLE_HS_ESTALLED; + }; + const auto start = std::chrono::steady_clock::now(); + bool result = c.write({1, 2, 3}); + require(result, "early TX_UNSTALLED completion lost"); + require(std::chrono::steady_clock::now() - start < 500ms, "early completion was drained before wait"); + } else if (name == "disconnect") { + Channel c; + std::promise submitted; + hostSend = [&](os_mbuf* b) { + delete b; + submitted.set_value(); + return BLE_HS_ESTALLED; + }; + auto writer = std::async(std::launch::async, [&] { return c.write({1, 2, 3}); }); + submitted.get_future().wait(); + c.disconnected(); + bool woke = writer.wait_for(300ms) == std::future_status::ready; + if (!woke) + onHost([&] { c.unstalled(BLE_HS_ENOTCONN); }); // let old implementation exit before reporting failure + const bool success = writer.get(); + require(woke, "disconnect did not wake blocked sender"); + require(!success, "disconnected send reported success"); + } else if (name == "serialize") { + Channel c; + std::mutex mutex; + std::vector sent; + hostSend = [&](os_mbuf* b) { + std::lock_guard l(mutex); + sent.push_back(b->bytes.front()); + delete b; + vTaskDelay(2); + return 0; + }; + auto a = std::async(std::launch::async, [&] { return c.write(std::vector(80, 1)); }); + auto b = std::async(std::launch::async, [&] { return c.write(std::vector(80, 2)); }); + require(!c.accept(), "accepted another connection during active writes"); + require(a.get() && b.get(), "concurrent write failed"); + require(sent.size() == 20, "wrong fragment count"); + int changes = 0; + for (size_t i = 1; i < sent.size(); ++i) changes += sent[i] != sent[i - 1]; + require(changes == 1, "whole-PDU fragments interleaved"); + } else if (name == "timeout") { + Channel c; + std::atomic sends{0}; + hostDisconnects = 0; + hostSend = [&](os_mbuf* b) { + ++sends; + delete b; + return BLE_HS_ESTALLED; + }; + auto start = std::chrono::steady_clock::now(); + require(!c.write(std::vector(20, 1)), "stalled timeout reported success"); + require(std::chrono::steady_clock::now() - start < 3s, "send timeout unbounded"); + onHost([] {}); + require(hostDisconnects == 1 && sends == 1, "timeout did not abort remaining fragments"); + } else if (name == "queued_timeout") { + Channel c; + std::promise entered, release; + auto gate = release.get_future().share(); + std::atomic sends{0}; + hostDisconnects = 0; + hostSend = [&](os_mbuf* b) { + ++sends; + delete b; + return 0; + }; + auto blocked = std::async(std::launch::async, [&] { + onHost([&] { + entered.set_value(); + gate.wait(); + }); + }); + entered.get_future().wait(); + bool success = c.write(std::vector(20, 1)); + release.set_value(); + blocked.get(); + onHost([] {}); + require(!success && sends == 0, "queued SDU was sent after timeout closed session"); + require(hostDisconnects == 1, "queued timeout did not request disconnect"); + } else if (name == "reconnect") { + Channel c; + require(!c.accept(), "accepted a second connection while open"); + hostSend = [&](os_mbuf* b) { + delete b; + return BLE_HS_ESTALLED; + }; + require(!c.write({1}), "stalled write did not time out"); + require(!c.accept(), "accepted before native disconnect and cleanup"); + c.disconnected(); + bool accepted = false; + for (int i = 0; i < 200 && !accepted; ++i) { + accepted = c.accept(); + if (!accepted) vTaskDelay(1); + } + require(accepted, "cleanup did not allow reconnect"); + require(!c.accept(), "accepted twice before connected event"); + c.connected(); + hostSend = [&](os_mbuf* b) { + delete b; + return 0; + }; + require(c.write(std::vector(80, 2)), "old timeout contaminated new connection"); + } else if (name == "ownership") { + for (auto error : {BLE_HS_EBADDATA, BLE_HS_EBUSY}) { + Channel c; + hostSend = [=](os_mbuf*) { return error; }; // Native early errors retain caller ownership. + require(!c.write({1}), "early native error reported success"); + } + } else if (name == "callbacks") { + struct Callbacks : NimBLEL2CAPChannelCallbacks { + std::promise entered, release; + std::shared_future gate = release.get_future().share(); + std::atomic reads{0}, closes{0}; + void onRead(NimBLEL2CAPChannel*, std::vector&) override { + ++reads; + entered.set_value(); + gate.wait(); + } + void onDisconnect(NimBLEL2CAPChannel*) override { ++closes; } + }; + auto* callbacks = new Callbacks; + Channel c(callbacks); + hostDisconnects = 0; + c.receive({1}); + callbacks->entered.get_future().wait(); + for (int i = 0; i < 8; ++i) c.receive({2}); + require(hostDisconnects == 1, "full callback queue did not abort"); + require(callbacks->reads == 1, "queue overflow invoked an inline read"); + c.disconnected(); + require(callbacks->closes == 0, "disconnect overlapped running read"); + callbacks->release.set_value(); + for (int i = 0; i < 100 && callbacks->closes == 0; ++i) vTaskDelay(1); + require(callbacks->closes == 1 && callbacks->reads == 1, "stale reads ran after disconnect"); + } + std::cout << name << " passed\n"; + return 0; + } catch (const std::exception& e) { + std::cerr << e.what() << '\n'; + return 1; + } +} diff --git a/tests/l2cap/README.md b/tests/l2cap/README.md index 50e710d2..ca39e689 100644 --- a/tests/l2cap/README.md +++ b/tests/l2cap/README.md @@ -1,13 +1,38 @@ -# L2CAP send regression tests +# L2CAP host tests -The tests compile the production `NimBLEL2CAPChannel.cpp` with minimal native -host replacements. They check positive native error propagation, abort after a -partly submitted SDU without replay, caller/native mbuf ownership, append -failure cleanup, successful fragmentation and accepted credit-stalled sends. -They do not simulate RF, controller buffers or concurrency. +These tests compile the production `NimBLEL2CAPChannel.cpp` with small replacements for NimBLE and FreeRTOS. Separate host, callback and writer threads exercise send errors, buffer ownership, early credit notifications, disconnects, whole-message serialization, timeout cleanup, reconnect and callback queue overflow. ```sh -cmake -S tests/l2cap -B build/l2cap-tests -cmake --build build/l2cap-tests -ctest --test-dir build/l2cap-tests --output-on-failure +make test-l2cap +make test-l2cap SANITIZER=address +make test-l2cap SANITIZER=thread ``` + +Clang, CMake and a C++17 compiler are required. Address builds also enable undefined-behavior checks. The deferred-callback configuration and an inline-callback build are both compiled. + +The fake radio does not validate ESP32 scheduling, RF behavior, controller credits or memory-pool sizing. Real-device transfer and disconnect tests remain necessary. Timeouts deliberately exercise the production two-second deadline. The lifecycle cases repeatedly create and destroy channels and verify that the shared callback task and queue are released after the last channel. + +## Callback and ownership contract + +Whole writes are serialized and a failed incomplete write closes the channel. +Inline onConnect/onRead callbacks remain on the host by default. They must not +block; use an application worker for writes that may need credits. The optional +read-offload mode moves those callbacks to a shared worker. Disconnect callbacks +always use that worker and follow running deferred reads. Stop accepting, request +disconnect and wait for `isIdle()` before deleting an application-owned channel; +never delete it inside one of its callbacks. The library owns/deletes callback +objects passed to the constructor/service. Creating/destroying channels belongs +to one application initialization/management task. + +This change allocates full-SDU mbufs (five by default) instead of subdividing +SDUs into 250-byte pool blocks. Pool count is configurable; larger MTUs/counts +cost more per-channel heap. The shared worker is released after the last channel. + +CI compiles a CoC-enabled probe with inline and deferred callbacks on IDF 5.4.3 +and 5.5.1. These are compile/link checks; real radio tests remain separate. + +`.github/tests/l2cap-build/main/main.cpp` shows bounded, worker-driven echo. +Callbacks queue copied bytes, overflow disconnects, and a generation counter +rejects old queued work after reconnect. Its service/channel live for the process +lifetime; a product with teardown must also stop/drain its application worker +before deleting the channel. The CI probe does not advertise or perform RF tests. diff --git a/tests/l2cap/SendErrorsTests.cpp b/tests/l2cap/SendErrorsTests.cpp deleted file mode 100644 index 2236d733..00000000 --- a/tests/l2cap/SendErrorsTests.cpp +++ /dev/null @@ -1,60 +0,0 @@ -#include "NimBLEL2CAPChannel.h" -#include -#include - -static void require(bool value,const char* message) { if (!value) throw std::runtime_error(message); } -struct Channel : NimBLEL2CAPChannel { - ble_l2cap_chan native; - Channel() : NimBLEL2CAPChannel(129,64,new NimBLEL2CAPChannelCallbacks) { - ble_l2cap_event e{};e.connect.chan=&native;handleConnectionEvent(&e); - } -}; -int main(int argc,char** argv) { - try { - const std::string test=argc>1?argv[1]:"errors"; - int sends=0; - if (test=="errors" || test=="ownership") { - const std::vector codes=test=="errors" ? - std::vector{BLE_HS_ENOMEM,BLE_HS_EAGAIN,BLE_HS_ENOTCONN,BLE_HS_EINVAL} : - std::vector{BLE_HS_EBADDATA,BLE_HS_EBUSY}; - for (int error:codes) { - Channel c;sends=0;hostDisconnects=0; - hostSend=[&](os_mbuf* b) { - ++sends; - if (error!=BLE_HS_EBADDATA && error!=BLE_HS_EBUSY) os_mbuf_free_chain(b); - return error; - }; - require(!c.write(std::vector(20,0x55)),"positive error reported success"); - require(sends==1,"failed SDU was retried or later fragment submitted"); - require(hostDisconnects==1,"failed stream was not disconnected"); - require(liveMbufs==0,"send buffer leaked or freed twice"); - } - } else if (test=="partial") { - Channel c;std::vector delivered; - hostSend=[&](os_mbuf* b) { - ++sends; - const size_t length=sends==1?b->bytes.size():2; - delivered.insert(delivered.end(),b->bytes.begin(),b->bytes.begin()+length); - os_mbuf_free_chain(b);return sends==1?0:BLE_HS_ENOMEM; - }; - require(!c.write(std::vector(24,0x55)),"partially sent stream reported success"); - require(sends==2 && delivered.size()==10,"partial SDU prefix replayed or remaining fragment sent"); - require(hostDisconnects==1 && liveMbufs==0,"partial-send abort/ownership failed"); - } else if (test=="append") { - Channel c;appendError=BLE_HS_ENOMEM; - hostSend=[&](os_mbuf*) { ++sends;return 0; }; - require(!c.write({1,2,3}),"append failure reported success"); - require(sends==0 && liveMbufs==0 && hostDisconnects==1,"append error leaked buffer or submitted data"); - } else if (test=="success") { - Channel c;std::vector received;std::vector input; - for (int i=0;i<20;++i) input.push_back(uint8_t(i)); - hostSend=[&](os_mbuf* b) {++sends;received.insert(received.end(),b->bytes.begin(),b->bytes.end());return os_mbuf_free_chain(b);}; - require(c.write(input) && sends==3 && received==input && liveMbufs==0 && hostDisconnects==0,"valid fragmented write failed"); - } else if (test=="stalled") { - Channel c; - hostSend=[&](os_mbuf* b) {++sends;os_mbuf_free_chain(b);return BLE_HS_ESTALLED;}; - require(c.write({1,2,3}) && sends==1 && liveMbufs==0 && hostDisconnects==0,"accepted stalled SDU incorrectly treated as error"); - } else { throw std::runtime_error("unknown test"); } - std::cout< #include #include #include +#include +#include #include #include #include #include +#include +#include #include -struct os_mempool {}; +using TickType_t = uint32_t; +using BaseType_t = int; +constexpr int pdTRUE = 1, pdFALSE = 0, pdPASS = 1; +constexpr TickType_t portMAX_DELAY = UINT32_MAX; +#define pdMS_TO_TICKS(x) (x) +struct HostSemaphore { + std::mutex mutex; + std::condition_variable cv; + bool available = false; +}; +using SemaphoreHandle_t = HostSemaphore*; +inline SemaphoreHandle_t xSemaphoreCreateBinary() { + return new HostSemaphore; +} +inline SemaphoreHandle_t xSemaphoreCreateMutex() { + auto s = new HostSemaphore; + s->available = true; + return s; +} +inline int xSemaphoreTake(SemaphoreHandle_t s, TickType_t timeout) { + std::unique_lock l(s->mutex); + if (!s->cv.wait_for(l, std::chrono::milliseconds(timeout == portMAX_DELAY ? 2000 : timeout), [&] { + return s->available; + })) + return pdFALSE; + s->available = false; + return pdTRUE; +} +inline int xSemaphoreGive(SemaphoreHandle_t s) { + std::lock_guard l(s->mutex); + s->available = true; + s->cv.notify_one(); + return pdTRUE; +} +inline void vSemaphoreDelete(SemaphoreHandle_t s) { + delete s; +} +inline void vTaskDelay(uint32_t ms) { + std::this_thread::sleep_for(std::chrono::milliseconds(ms)); +} +inline void ble_npl_time_delay(uint32_t ms) { + vTaskDelay(ms); +} +inline uint32_t ble_npl_time_ms_to_ticks32(uint32_t ms) { + return ms; +} +struct os_mempool { + const char* name = nullptr; +}; struct os_mbuf_pool {}; +struct os_mbuf_pkthdr { uint32_t length; }; +inline size_t hostPayloadCapacity = 0; using os_membuf_t = uint8_t; -struct os_mbuf { std::vector bytes; }; -#define OS_MEMPOOL_SIZE(n,s) ((n)*(s)) -#define OS_MBUF_PKTLEN(m) ((m)->bytes.size()) -inline int liveMbufs=0, appendError=0; -inline int os_mempool_init(os_mempool*,size_t,size_t,void*,const char*) { return 0; } -inline int os_mbuf_pool_init(os_mbuf_pool*,os_mempool*,size_t,size_t) { return 0; } -inline os_mbuf* os_mbuf_get_pkthdr(os_mbuf_pool*,int) { ++liveMbufs; return new os_mbuf; } -inline int os_mbuf_append(os_mbuf* b,const void* p,size_t n) { - if (appendError) return appendError; - auto a=static_cast(p); b->bytes.insert(b->bytes.end(),a,a+n); return 0; -} -inline int os_mbuf_free_chain(os_mbuf* b) { --liveMbufs; delete b; return 0; } -inline int os_mbuf_copydata(os_mbuf* b,size_t off,size_t n,void* p) { memcpy(p,b->bytes.data()+off,n); return 0; } -constexpr int BLE_HS_EAGAIN=1, BLE_HS_ENOMEM=6, BLE_HS_ENOTCONN=7, BLE_HS_EINVAL=3, - BLE_HS_EALREADY=2, BLE_HS_EBADDATA=10, BLE_HS_EBUSY=15, BLE_HS_EREJECT=14, - BLE_HS_ESTALLED=31, BLE_HS_EUNKNOWN=17; -constexpr uint16_t BLE_HS_CONN_HANDLE_NONE=0xffff; -constexpr uint32_t BLE_NPL_TIME_FOREVER=UINT32_MAX; -inline uint32_t ble_npl_time_ms_to_ticks32(uint32_t ms) { return ms; } -inline void ble_npl_time_delay(uint32_t) {} // Retry count, not elapsed radio time, is tested. +inline std::atomic liveMbufs{0}; +inline std::atomic appendError{0}; +struct os_mbuf { + std::vector bytes; + os_mbuf(std::vector data = {}) : bytes(std::move(data)) { ++liveMbufs; } + ~os_mbuf() { --liveMbufs; } +}; +#define OS_MEMPOOL_SIZE(n, s) ((n) * (s)) +#define OS_MBUF_PKTLEN(m) ((m)->bytes.size()) +inline int os_mempool_init(os_mempool* p, size_t, size_t, void*, const char* n) { + p->name = n; + return 0; +} +inline int os_mbuf_pool_init(os_mbuf_pool*, os_mempool*, size_t blockSize, size_t) { + hostPayloadCapacity = blockSize - sizeof(os_mbuf) - sizeof(os_mbuf_pkthdr); + return 0; +} +inline void os_mempool_unregister(os_mempool*) {} +inline os_mbuf* os_mbuf_get_pkthdr(os_mbuf_pool*, int) { + return new os_mbuf; +} +inline int os_mbuf_append(os_mbuf* b, const void* p, size_t n) { + if (appendError.load()) return appendError.load(); + auto a = (const uint8_t*)p; + b->bytes.insert(b->bytes.end(), a, a + n); + return 0; +} +inline int os_mbuf_free_chain(os_mbuf* b) { + delete b; + return 0; +} +inline int os_mbuf_copydata(os_mbuf* b, size_t off, size_t n, void* p) { + memcpy(p, b->bytes.data() + off, n); + return 0; +} +constexpr int BLE_HS_EAGAIN = 1, BLE_HS_ENOMEM = 6, BLE_HS_ENOTCONN = 7, BLE_HS_EINVAL = 3, BLE_HS_EALREADY = 2, + BLE_HS_EBADDATA = 10, BLE_HS_EBUSY = 15, BLE_HS_EREJECT = 14, BLE_HS_ESTALLED = 31, BLE_HS_EUNKNOWN = 17, + BLE_HS_ETIMEOUT = 13; +constexpr uint16_t BLE_HS_CONN_HANDLE_NONE = 0xffff; struct ble_l2cap_chan {}; -struct ble_l2cap_chan_info { uint16_t peer_coc_mtu=8,our_coc_mtu=8,peer_l2cap_mtu=8,our_l2cap_mtu=8; }; -constexpr int BLE_L2CAP_EVENT_COC_CONNECTED=1,BLE_L2CAP_EVENT_COC_DISCONNECTED=2, - BLE_L2CAP_EVENT_COC_ACCEPT=3,BLE_L2CAP_EVENT_COC_DATA_RECEIVED=4,BLE_L2CAP_EVENT_COC_TX_UNSTALLED=5; +struct ble_l2cap_chan_info { + uint16_t peer_coc_mtu = 8, our_coc_mtu = 8, peer_l2cap_mtu = 8, our_l2cap_mtu = 8, scid = 1, dcid = 2, psm = 129; +}; +constexpr int BLE_L2CAP_EVENT_COC_CONNECTED = 1, BLE_L2CAP_EVENT_COC_DISCONNECTED = 2, BLE_L2CAP_EVENT_COC_ACCEPT = 3, + BLE_L2CAP_EVENT_COC_DATA_RECEIVED = 4, BLE_L2CAP_EVENT_COC_TX_UNSTALLED = 5; struct ble_l2cap_event { - int type=0; - struct { ble_l2cap_chan* chan; } connect{},accept{},disconnect{}; - struct { os_mbuf* sdu_rx; } receive{}; - struct { int status; ble_l2cap_chan* chan; } tx_unstalled{}; + int type = 0; + struct { + ble_l2cap_chan* chan; + } connect{}, accept{}, disconnect{}; + struct { + os_mbuf* sdu_rx; + } receive{}; + struct { + int status; + ble_l2cap_chan* chan; + } tx_unstalled{}; }; inline std::function hostSend; -inline int hostDisconnects=0; -inline int ble_l2cap_get_chan_info(ble_l2cap_chan*,ble_l2cap_chan_info* i) { *i={}; return 0; } -inline uint16_t ble_l2cap_get_conn_handle(ble_l2cap_chan*) { return 1; } -inline int ble_l2cap_send(ble_l2cap_chan*,os_mbuf* b) { return hostSend(b); } -inline int ble_l2cap_disconnect(ble_l2cap_chan*) { ++hostDisconnects; return 0; } -inline int ble_l2cap_recv_ready(ble_l2cap_chan*,os_mbuf* b) { return os_mbuf_free_chain(b); } +inline std::atomic hostDisconnects{0}; +inline int ble_l2cap_get_chan_info(ble_l2cap_chan* c, ble_l2cap_chan_info* i) { + if (!c) return BLE_HS_ENOTCONN; + *i = {}; + return 0; +} +inline uint16_t ble_l2cap_get_conn_handle(ble_l2cap_chan*) { + return 1; +} +inline int ble_l2cap_send(ble_l2cap_chan*, os_mbuf* b) { + return hostSend(b); +} +inline int ble_l2cap_disconnect(ble_l2cap_chan*) { + ++hostDisconnects; + return 0; +} +inline int ble_l2cap_recv_ready(ble_l2cap_chan*, os_mbuf* b) { + delete b; + return 0; +} class NimBLEUtils { - public: - struct TaskData { mutable int m_flags=0; }; - static bool taskWait(const TaskData&,uint32_t) { assert(false && "credit wait not exercised by send error tests"); return false; } - static void taskRelease(const TaskData&,int=0) {} - static const char* returnCodeToString(int) { return "stub"; } + public: + static const char* returnCodeToString(int) { return "stub"; } +}; +#include +using TaskHandle_t = void*; +struct HostTaskExit {}; +struct HostTask { + std::atomic cancelled{false}; + std::thread thread; }; +inline thread_local HostTask* currentHostTask = nullptr; +inline std::atomic hostTaskCount{0}; +inline std::atomic hostQueueCount{0}; +inline TaskHandle_t xTaskGetCurrentTaskHandle() { + thread_local int token; + return &token; +} +inline int xTaskCreate(void (*fn)(void*), const char*, uint32_t, void* arg, int, TaskHandle_t* handle) { + auto task = new HostTask; + *handle = task; + ++hostTaskCount; + task->thread = std::thread([=] { + currentHostTask = task; + try { fn(arg); } catch (const HostTaskExit&) {} + }); + return pdPASS; +} +inline void vTaskDelete(TaskHandle_t handle) { + auto task = static_cast(handle); + assert(task && task != currentHostTask); + task->cancelled = true; + task->thread.join(); + delete task; + --hostTaskCount; +} +struct HostQueue { + std::mutex mutex; + std::condition_variable cv; + size_t capacity, width; + std::deque> items; +}; +using QueueHandle_t = HostQueue*; +inline QueueHandle_t xQueueCreate(size_t n, size_t w) { + ++hostQueueCount; + auto q = new HostQueue; + q->capacity = n; + q->width = w; + return q; +} +inline void vQueueDelete(QueueHandle_t q) { + assert(q->items.empty()); + delete q; + --hostQueueCount; +} +inline size_t uxQueueSpacesAvailable(QueueHandle_t q) { + std::lock_guard l(q->mutex); + return q->capacity - q->items.size(); +} +inline int xQueueSend(QueueHandle_t q, const void* item, TickType_t) { + std::lock_guard l(q->mutex); + if (q->items.size() == q->capacity) return pdFALSE; + auto b = (const uint8_t*)item; + q->items.emplace_back(b, b + q->width); + q->cv.notify_one(); + return pdTRUE; +} +inline int xQueueReceive(QueueHandle_t q, void* item, TickType_t) { + std::unique_lock l(q->mutex); + while (q->items.empty()) { + if (currentHostTask && currentHostTask->cancelled.load()) { throw HostTaskExit{}; } + q->cv.wait_for(l, std::chrono::milliseconds(1)); + } + memcpy(item, q->items.front().data(), q->width); + q->items.pop_front(); + return pdTRUE; +} +struct ble_npl_event { + void (*fn)(ble_npl_event*) = nullptr; + void* arg = nullptr; +}; +inline void ble_npl_event_init(ble_npl_event* e, void (*fn)(ble_npl_event*), void* p) { + e->fn = fn; + e->arg = p; +} +inline void ble_npl_event_deinit(ble_npl_event*) {} +inline void* ble_npl_event_get_arg(ble_npl_event* e) { + return e->arg; +} +inline QueueHandle_t nimble_port_get_dflt_eventq() { + static auto q = [] { + auto q = xQueueCreate(128, sizeof(ble_npl_event*)); + std::thread([=] { + for (;;) { + ble_npl_event* e; + xQueueReceive(q, &e, portMAX_DELAY); + e->fn(e); + } + }).detach(); + return q; + }(); + return q; +} +inline void ble_npl_eventq_put(QueueHandle_t q, ble_npl_event* e) { + assert(xQueueSend(q, &e, 0) == pdTRUE); +} diff --git a/tests/l2cap/stubs/freertos/FreeRTOS.h b/tests/l2cap/stubs/freertos/FreeRTOS.h new file mode 100644 index 00000000..dcdc9c8a --- /dev/null +++ b/tests/l2cap/stubs/freertos/FreeRTOS.h @@ -0,0 +1 @@ +#include "Host.h" diff --git a/tests/l2cap/stubs/freertos/queue.h b/tests/l2cap/stubs/freertos/queue.h new file mode 100644 index 00000000..dcdc9c8a --- /dev/null +++ b/tests/l2cap/stubs/freertos/queue.h @@ -0,0 +1 @@ +#include "Host.h" diff --git a/tests/l2cap/stubs/freertos/semphr.h b/tests/l2cap/stubs/freertos/semphr.h new file mode 100644 index 00000000..dcdc9c8a --- /dev/null +++ b/tests/l2cap/stubs/freertos/semphr.h @@ -0,0 +1 @@ +#include "Host.h" diff --git a/tests/l2cap/stubs/freertos/task.h b/tests/l2cap/stubs/freertos/task.h new file mode 100644 index 00000000..dcdc9c8a --- /dev/null +++ b/tests/l2cap/stubs/freertos/task.h @@ -0,0 +1 @@ +#include "Host.h" diff --git a/tests/l2cap/stubs/host/ble_hs.h b/tests/l2cap/stubs/host/ble_hs.h new file mode 100644 index 00000000..dcdc9c8a --- /dev/null +++ b/tests/l2cap/stubs/host/ble_hs.h @@ -0,0 +1 @@ +#include "Host.h" diff --git a/tests/l2cap/stubs/nimble/nimble_npl.h b/tests/l2cap/stubs/nimble/nimble_npl.h new file mode 100644 index 00000000..dcdc9c8a --- /dev/null +++ b/tests/l2cap/stubs/nimble/nimble_npl.h @@ -0,0 +1 @@ +#include "Host.h" diff --git a/tests/l2cap/stubs/nimble/nimble_port.h b/tests/l2cap/stubs/nimble/nimble_port.h new file mode 100644 index 00000000..dcdc9c8a --- /dev/null +++ b/tests/l2cap/stubs/nimble/nimble_port.h @@ -0,0 +1 @@ +#include "Host.h" From 69e5747ecf9ab1de95e8d0273e6d625f5bc7f4c5 Mon Sep 17 00:00:00 2001 From: "Dr. Michael Lauer" Date: Fri, 16 Oct 2026 14:50:38 +0200 Subject: [PATCH 3/5] ci: restrict build workflow credentials to read access --- .github/workflows/l2cap-tests.yml | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/.github/workflows/l2cap-tests.yml b/.github/workflows/l2cap-tests.yml index 52badcec..05e72a35 100644 --- a/.github/workflows/l2cap-tests.yml +++ b/.github/workflows/l2cap-tests.yml @@ -4,6 +4,9 @@ on: pull_request: push: branches: [master] +permissions: + contents: read + jobs: native: runs-on: ubuntu-latest @@ -12,6 +15,8 @@ jobs: cxx_flags: ["", "-fsanitize=address,undefined -fno-omit-frame-pointer"] steps: - uses: actions/checkout@v5 + with: + persist-credentials: false - run: cmake -S tests/l2cap -B build/l2cap-tests -DCMAKE_CXX_FLAGS="${{ matrix.cxx_flags }}" - run: cmake --build build/l2cap-tests - run: ctest --test-dir build/l2cap-tests --output-on-failure From 5637932749beaa76d1db64055f6876982fc193a8 Mon Sep 17 00:00:00 2001 From: "Dr. Michael Lauer" Date: Fri, 16 Oct 2026 14:50:38 +0200 Subject: [PATCH 4/5] ci: limit CoC build token permissions --- .github/workflows/l2cap-build.yml | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/.github/workflows/l2cap-build.yml b/.github/workflows/l2cap-build.yml index f79911b0..5731bade 100644 --- a/.github/workflows/l2cap-build.yml +++ b/.github/workflows/l2cap-build.yml @@ -4,6 +4,9 @@ on: pull_request: push: branches: [master] +permissions: + contents: read + jobs: coc: runs-on: ubuntu-latest @@ -16,6 +19,7 @@ jobs: steps: - uses: actions/checkout@v5 with: + persist-credentials: false path: esp-nimble-cpp - name: Compile and link CoC implementation env: From 8bba85330d24c14d31b3079fe818c02a5f230c9c Mon Sep 17 00:00:00 2001 From: "Dr. Michael Lauer" Date: Fri, 16 Oct 2026 15:08:20 +0200 Subject: [PATCH 5/5] test: enqueue host events even when assertions are disabled --- tests/l2cap/stubs/Host.h | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/tests/l2cap/stubs/Host.h b/tests/l2cap/stubs/Host.h index aaa3d970..9ceea2d3 100644 --- a/tests/l2cap/stubs/Host.h +++ b/tests/l2cap/stubs/Host.h @@ -271,5 +271,7 @@ inline QueueHandle_t nimble_port_get_dflt_eventq() { return q; } inline void ble_npl_eventq_put(QueueHandle_t q, ble_npl_event* e) { - assert(xQueueSend(q, &e, 0) == pdTRUE); + const auto queued = xQueueSend(q, &e, 0); + assert(queued == pdTRUE); + (void)queued; }