From d12300679eb9928b99c717a32f1488e19bafec9e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Edvard=20Filistovi=C4=8D?= Date: Fri, 24 Jul 2026 13:26:33 +0300 Subject: [PATCH] [bk72xx_ble] Scan primitives and main-task scan report queue (#17802) Co-authored-by: J. Nick Koston --- esphome/components/bk72xx_ble/bk72xx_ble.cpp | 134 ++++++++++++++++-- esphome/components/bk72xx_ble/bk72xx_ble.h | 54 +++++++ esphome/core/event_pool.h | 4 +- esphome/core/lock_free_queue.h | 56 +++++++- .../components/core/test_lock_free_queue.cpp | 96 +++++++++++++ 5 files changed, 330 insertions(+), 14 deletions(-) create mode 100644 tests/components/core/test_lock_free_queue.cpp diff --git a/esphome/components/bk72xx_ble/bk72xx_ble.cpp b/esphome/components/bk72xx_ble/bk72xx_ble.cpp index db6b665ba6..a5ecaf4abb 100644 --- a/esphome/components/bk72xx_ble/bk72xx_ble.cpp +++ b/esphome/components/bk72xx_ble/bk72xx_ble.cpp @@ -1,15 +1,22 @@ // bk72xx_ble.cpp // // BLE controller support for the BK72xx BLE-5.x chips (LibreTiny beken-72xx -// family) — the platform analog of esp32_ble / rp2040_ble. Owns the Beken BDK -// BLE stack bring-up (ble_entry()) and the controller BLE address. Consumers -// (bk72xx_ble_tracker) build on this component and contain no SDK calls of -// their own. +// family) — the platform analog of esp32_ble / rp2040_ble. Owns everything that +// talks to the Beken BDK BLE stack: +// - one-time stack bring-up (ble_set_notice_cb() + ble_entry()), +// - the controller BLE address, +// - the raw controller scan primitives (bk_ble_scan_start/stop), +// - the scan-report ring: the BDK notice callback (BLE task) takes a report +// from a fixed pool and pushes it on a lock-free SPSC queue; loop() drains, +// dispatches on the main task and returns reports to the pool — the same +// EventPool + LockFreeQueue handoff esp32_ble uses, zero allocation at +// steady state. +// Consumers contain no SDK calls of their own. // // NOTE: the Beken BDK BLE 5.x stack is compiled and linked by the LibreTiny // beken-72xx builder itself (prebuilt libble_.a + ble_5_x sources, gated // on CFG_SUPPORT_BLE / CFG_BLE_VERSION in sys_config.h). This component only -// calls into it via its public API — no framework patch is required. +// calls into it via the public ble_api.h — no framework patch is required. #include "bk72xx_ble.h" // pulls esphome/core/defines.h for USE_BK72XX_BLE @@ -44,10 +51,15 @@ #ifndef BK72XX_BLE_NO_SDK // --------------------------------------------------------------------------- -// Beken BDK BLE 5.x SDK surface used here. Wrapped in extern "C" because these -// are C symbols consumed from C++. +// Beken BDK BLE 5.x SDK — public API. +// Exposed on the include path by the LibreTiny beken-72xx builder +// (cores/.../ble_5_x_rw + driver/include). Wrapped in extern "C" because these +// are C headers consumed from C++ (a standard C-header-from-C++ pattern). // --------------------------------------------------------------------------- extern "C" { +#include "ble_api.h" // bk_ble_scan_start/stop, ble_entry, ble_set_notice_cb, + // app_ble_get_idle_actv_idx_handle, struct scan_param, + // recv_adv_t, ble_notice_t, BLE_5_REPORT_ADV, SCAN_ACTV #ifdef BK72XX_BLE_HAS_COMMON_BDADDR #include "common_bt_defines.h" // struct bd_addr // The controller's public BLE address, populated by the BDK during ble_entry(). @@ -64,11 +76,53 @@ namespace esphome::bk72xx_ble { static const char *const TAG = "bk72xx_ble"; +// The BDK notice callback is a plain C function pointer with no user argument, +// so it reaches the (single) component instance through a file-static pointer. +static BK72xxBLE *s_ble = nullptr; // NOLINT(cppcoreguidelines-avoid-non-const-global-variables) + +// --------------------------------------------------------------------------- +// BLE notice callback — runs in the BDK BLE task context. +// The BK controller reports every advertisement as a BLE_5_REPORT_ADV notice +// carrying a recv_adv_t. Copy it into the queue and return; all dispatch +// happens in loop() on the main task. +// --------------------------------------------------------------------------- +static void ble_notice_callback(ble_notice_t notice, void *param) { + if (s_ble == nullptr || param == nullptr) + return; + if (notice != BLE_5_REPORT_ADV) + return; + + const recv_adv_t *info = reinterpret_cast(param); + // rssi is a signed dBm carried in a uint8_t; cast through int8_t (standard for + // a signed dBm value packed in a uint8_t). + s_ble->enqueue_scan_report(info->adv_addr, static_cast(info->rssi), info->adv_addr_type, info->data, + info->data_len); +} + +void BK72xxBLE::enqueue_scan_report(const uint8_t *mac, int8_t rssi, uint8_t addr_type, const uint8_t *data, + uint16_t data_len) { + BLEScanReport *report = this->report_pool_.allocate(); + if (report == nullptr) { + // Pool exhausted — the queue is full; count and drop. + this->report_queue_.increment_dropped_count(); + return; + } + memcpy(report->mac, mac, 6); + report->rssi = rssi; + report->addr_type = addr_type; + report->data_len = + (data_len <= sizeof(report->data)) ? static_cast(data_len) : static_cast(sizeof(report->data)); + memcpy(report->data, data, report->data_len); + // Cannot fail: the pool is sized to the queue capacity. + this->report_queue_.push(report); +} + // --------------------------------------------------------------------------- // Component lifecycle // --------------------------------------------------------------------------- void BK72xxBLE::setup() { + s_ble = this; // Resolve the MAC early so get_mac_lsb_first() is valid for consumers before // the stack is up (it is re-read once ble_entry() has run). this->resolve_mac_(); @@ -86,7 +140,9 @@ void BK72xxBLE::enable() { return; this->state_ = BLEComponentState::ENABLING; - // One-time BLE stack init. The BDK has no teardown path — init happens at most once. + // One-time BLE stack init: register the notice callback, then bring up the + // BDK BLE stack. The BDK has no teardown path — init happens at most once. + ble_set_notice_cb(ble_notice_callback); ble_entry(); delay(100); // NOLINT — one-time BLE stack init; the SDK needs this settle time @@ -116,6 +172,25 @@ void BK72xxBLE::enable() { ESP_LOGD(TAG, "BLE stack initialised"); } +void BK72xxBLE::loop() { + // Drain the lock-free ring filled by the BLE task; all per-report work runs + // here on the main task, then the report returns to the pool. + BLEScanReport *report = this->report_queue_.pop(); + if (report == nullptr) + return; + do { + for (auto *listener : this->scan_listeners_) + listener->on_scan_report(*report); + this->report_pool_.release(report); + } while ((report = this->report_queue_.pop()) != nullptr); + + // Log dropped reports — only reachable when reports were processed; drops can + // only occur while the queue is full, and only this loop drains it. + uint16_t dropped = this->report_queue_.get_and_reset_dropped_count(); + if (dropped > 0) + ESP_LOGW(TAG, "Dropped %u scan reports due to queue overflow", dropped); +} + void BK72xxBLE::get_mac_lsb_first(uint8_t out[6]) const { for (int i = 0; i < 6; i++) out[i] = this->ble_mac_[i]; @@ -165,11 +240,52 @@ void BK72xxBLE::resolve_mac_() { get_mac_address_raw(wifi_mac); // MSB-first const uint8_t ble[6] = {wifi_mac[0], wifi_mac[1], wifi_mac[2], wifi_mac[3], wifi_mac[4], static_cast(wifi_mac[5] + 1)}; - // Store LSB-first to match the BLE controller's address ordering. + // Store LSB-first to match recv_adv_t adv_addr ordering. for (int i = 0; i < 6; i++) this->ble_mac_[i] = ble[5 - i]; } +// --------------------------------------------------------------------------- +// Controller scan primitives +// --------------------------------------------------------------------------- + +bool BK72xxBLE::scan_start(uint16_t interval, uint16_t window) { + if (!this->is_active()) + this->enable(); + + if (this->scan_actv_idx_ != 0xFF) { + // Already scanning — stop first so this call cleanly restarts with the new + // parameters (the BDK cannot start a second scan on a busy activity). + this->scan_stop(); + } + + struct scan_param sp; + memset(&sp, 0, sizeof(sp)); + sp.channel_map = 7; // advertising channels 37/38/39 + sp.interval = interval; + sp.window = window; + + this->scan_actv_idx_ = app_ble_get_idle_actv_idx_handle(SCAN_ACTV); + if (this->scan_actv_idx_ == 0xFF) { + ESP_LOGE(TAG, "Scan start failed: no idle activity handle"); + return false; + } + ble_err_t ret = bk_ble_scan_start(this->scan_actv_idx_, &sp, nullptr); + if (ret != ERR_SUCCESS) { + ESP_LOGE(TAG, "Scan start failed (err %d)", static_cast(ret)); + this->scan_actv_idx_ = 0xFF; + return false; + } + return true; +} + +void BK72xxBLE::scan_stop() { + if (this->scan_actv_idx_ != 0xFF) { + bk_ble_scan_stop(this->scan_actv_idx_, nullptr); + this->scan_actv_idx_ = 0xFF; + } +} + } // namespace esphome::bk72xx_ble #endif // BK72XX_BLE_NO_SDK diff --git a/esphome/components/bk72xx_ble/bk72xx_ble.h b/esphome/components/bk72xx_ble/bk72xx_ble.h index a327f7cbd8..2654f4e68e 100644 --- a/esphome/components/bk72xx_ble/bk72xx_ble.h +++ b/esphome/components/bk72xx_ble/bk72xx_ble.h @@ -5,8 +5,11 @@ #ifdef USE_BK72XX_BLE #include "esphome/core/component.h" +#include "esphome/core/event_pool.h" +#include "esphome/core/lock_free_queue.h" #include +#include namespace esphome::bk72xx_ble { @@ -16,9 +19,37 @@ enum class BLEComponentState : uint8_t { ACTIVE, }; +/// One advertisement report from the controller. +struct BLEScanReport { + uint8_t mac[6]; // LSB-first, as the controller delivers it + int8_t rssi; // signed dBm + uint8_t addr_type; + uint8_t data_len; // bytes valid in data[] + uint8_t data[62]; // legacy advertisement (31) + scan response (31) + + // EventPool contract: nothing is heap-allocated inside a report. + void release() {} +}; + +/// Consumer interface for controller scan reports. on_scan_report() always runs +/// on the ESPHome main task: reports are queued from the BDK BLE task and +/// drained by the controller's loop(), so consumers never deal with cross-task +/// state (the esp32_ble event-queue pattern). +class BLEScanListener { + public: + virtual void on_scan_report(const BLEScanReport &report) = 0; + + protected: + ~BLEScanListener() = default; // deletion via this interface is not part of the contract +}; + +// Maximum reports buffered between the BLE task and loop(). +static constexpr uint8_t MAX_SCAN_REPORT_QUEUE_SIZE = 64; + class BK72xxBLE final : public Component { public: void setup() override; + void loop() override; void dump_config() override; float get_setup_priority() const override; @@ -31,10 +62,33 @@ class BK72xxBLE final : public Component { /// Controller BLE address, least-significant octet first (BLE convention). void get_mac_lsb_first(uint8_t out[6]) const; + /// Register a consumer for scan reports (delivered on the main task via loop()). + void register_scan_listener(BLEScanListener *listener) { this->scan_listeners_.push_back(listener); } + + /// Start the controller scan. Interval/window are in BLE units (0.625 ms). + /// Enables the stack first if needed. Returns false on controller failure. + bool scan_start(uint16_t interval, uint16_t window); + /// Stop the controller scan (no-op when not scanning). + void scan_stop(); + + /// Internal: buffer one controller report (BDK notice callback, BLE task + /// context — bounded copy under the scheduler lock, nothing else). + void enqueue_scan_report(const uint8_t *mac, int8_t rssi, uint8_t addr_type, const uint8_t *data, uint16_t data_len); + protected: void resolve_mac_(); + std::vector scan_listeners_; + // Report ring: the BDK notice callback (BLE task) allocates a report from the + // pool, fills it and pushes the pointer; loop() pops, dispatches and releases. + // Lock-free SPSC, zero allocation at steady state — the esp32_ble pattern. + esphome::LockFreeQueue report_queue_; + // Pool sized to queue capacity (SIZE-1): the ring reserves one slot, so + // allocate() returns nullptr before push() can fail. This prevents leaking a + // pool slot on a failed push and keeps release() off the producer path. + esphome::EventPool report_pool_; uint8_t ble_mac_[6]{0}; // LSB-first (BLE convention) + uint8_t scan_actv_idx_{0xFF}; BLEComponentState state_{BLEComponentState::STATE_OFF}; bool enable_on_boot_{false}; }; diff --git a/esphome/core/event_pool.h b/esphome/core/event_pool.h index ee8e81225a..55c9254327 100644 --- a/esphome/core/event_pool.h +++ b/esphome/core/event_pool.h @@ -1,6 +1,6 @@ #pragma once -#if defined(USE_ESP32) || defined(USE_ZEPHYR) +#if defined(USE_ESP32) || defined(USE_ZEPHYR) || defined(USE_LIBRETINY) #include #include @@ -86,4 +86,4 @@ template class EventPool { } // namespace esphome -#endif // defined(USE_ESP32) +#endif // defined(USE_ESP32) || defined(USE_ZEPHYR) || defined(USE_LIBRETINY) diff --git a/esphome/core/lock_free_queue.h b/esphome/core/lock_free_queue.h index 316186ea54..ce54231137 100644 --- a/esphome/core/lock_free_queue.h +++ b/esphome/core/lock_free_queue.h @@ -1,5 +1,7 @@ #pragma once +#include "esphome/core/defines.h" + #include #include @@ -26,6 +28,54 @@ namespace esphome { +namespace lockfree_internal { +#ifdef ESPHOME_THREAD_MULTI_NO_ATOMICS +// Platforms whose cores lack atomic read-modify-write instructions (currently +// the ARMv5TE BK72xx SoCs — no LDREX/STREX, no libatomic; other LibreTiny +// chips such as LN882x/RTL87xx are ARMv7-M and keep std::atomic). For this +// queue's SPSC contract RMW atomics are not needed: aligned 8/16-bit loads and +// stores are single instructions on these cores, so torn reads cannot occur, +// and on a single in-order core a compiler barrier supplies all the +// acquire/release ordering the algorithm requires. Each index has exactly one +// writer (head_: consumer, tail_: producer). The dropped counter's +// increment/exchange pair is not atomic here — a concurrent reset can lose +// counts — which is acceptable for a diagnostic drop counter. +#define ESPHOME_LFQ_COMPILER_BARRIER() __asm__ __volatile__("" ::: "memory") +template class PlainAtomic { + public: + PlainAtomic() = default; + constexpr PlainAtomic(T value) : value_(value) {} + T load(std::memory_order order = std::memory_order_seq_cst) const { + T value = value_; + if (order != std::memory_order_relaxed) + ESPHOME_LFQ_COMPILER_BARRIER(); // acquire: later reads may not hoist above this load + return value; + } + void store(T value, std::memory_order order = std::memory_order_seq_cst) { + if (order != std::memory_order_relaxed) + ESPHOME_LFQ_COMPILER_BARRIER(); // release: earlier writes may not sink below this store + value_ = value; + } + T fetch_add(T amount, std::memory_order /*order*/ = std::memory_order_seq_cst) { + T value = value_; + value_ = value + amount; + return value; + } + T exchange(T desired, std::memory_order /*order*/ = std::memory_order_seq_cst) { + T value = value_; + value_ = desired; + return value; + } + + private: + volatile T value_{0}; +}; +template using AtomicIndex = PlainAtomic; +#else +template using AtomicIndex = std::atomic; +#endif +} // namespace lockfree_internal + // Base lock-free queue without task notification template class LockFreeQueue { public: @@ -126,13 +176,13 @@ template class LockFreeQueue { protected: T *buffer_[SIZE]{}; // Atomic: written by producer (push/increment), read+reset by consumer (get_and_reset) - std::atomic dropped_count_; // 65535 max - more than enough for drop tracking + lockfree_internal::AtomicIndex dropped_count_; // 65535 max - more than enough for drop tracking // Atomic: written by consumer (pop), read by producer (push) to check if full // Using uint8_t limits queue size to 255 elements but saves memory and ensures // atomic operations are efficient on all platforms - std::atomic head_; + lockfree_internal::AtomicIndex head_; // Atomic: written by producer (push), read by consumer (pop) to check if empty - std::atomic tail_; + lockfree_internal::AtomicIndex tail_; }; #ifdef USE_ESP32 diff --git a/tests/components/core/test_lock_free_queue.cpp b/tests/components/core/test_lock_free_queue.cpp new file mode 100644 index 0000000000..2f74278129 --- /dev/null +++ b/tests/components/core/test_lock_free_queue.cpp @@ -0,0 +1,96 @@ +// Exercises the no-atomics LockFreeQueue implementation (PlainAtomic indices — +// the path used on cores without atomic RMW instructions, currently BK72xx). +// The define is forced before the include so this TU deterministically compiles +// that path regardless of the host's default thread model; no other test TU +// instantiates this template, so the differing definition is confined here. +#define ESPHOME_THREAD_MULTI_NO_ATOMICS +#include "esphome/core/lock_free_queue.h" + +#include + +namespace esphome::core::testing { + +TEST(LockFreeQueueNoAtomics, EmptyPopReturnsNull) { + esphome::LockFreeQueue q; + EXPECT_EQ(q.pop(), nullptr); + EXPECT_TRUE(q.empty()); + EXPECT_FALSE(q.full()); + EXPECT_EQ(q.size(), 0u); +} + +TEST(LockFreeQueueNoAtomics, FifoOrder) { + esphome::LockFreeQueue q; + int a = 1, b = 2, c = 3; + EXPECT_TRUE(q.push(&a)); + EXPECT_TRUE(q.push(&b)); + EXPECT_TRUE(q.push(&c)); + EXPECT_EQ(q.size(), 3u); + EXPECT_EQ(q.pop(), &a); + EXPECT_EQ(q.pop(), &b); + EXPECT_EQ(q.pop(), &c); + EXPECT_EQ(q.pop(), nullptr); +} + +TEST(LockFreeQueueNoAtomics, CapacityIsSizeMinusOne) { + esphome::LockFreeQueue q; + int v[4] = {0, 1, 2, 3}; + EXPECT_TRUE(q.push(&v[0])); + EXPECT_TRUE(q.push(&v[1])); + EXPECT_TRUE(q.push(&v[2])); + EXPECT_TRUE(q.full()); + // Ring reserves one slot: the SIZEth push fails and is counted as dropped. + EXPECT_FALSE(q.push(&v[3])); + EXPECT_EQ(q.get_and_reset_dropped_count(), 1u); + EXPECT_EQ(q.get_and_reset_dropped_count(), 0u); // reset is sticky +} + +TEST(LockFreeQueueNoAtomics, NullPushRejected) { + esphome::LockFreeQueue q; + EXPECT_FALSE(q.push(nullptr)); + EXPECT_TRUE(q.empty()); +} + +TEST(LockFreeQueueNoAtomics, WrapAround) { + esphome::LockFreeQueue q; + int v[3] = {10, 20, 30}; + // Cycle several times the ring size to cross the wrap boundary repeatedly. + for (int cycle = 0; cycle < 10; cycle++) { + for (auto &value : v) + ASSERT_TRUE(q.push(&value)); + EXPECT_TRUE(q.full()); + for (auto &value : v) + ASSERT_EQ(q.pop(), &value); + EXPECT_TRUE(q.empty()); + } + EXPECT_EQ(q.get_and_reset_dropped_count(), 0u); +} + +TEST(LockFreeQueueNoAtomics, IncrementDroppedCount) { + esphome::LockFreeQueue q; + // Producer-side external drop accounting (pool exhausted before push). + q.increment_dropped_count(); + q.increment_dropped_count(); + EXPECT_EQ(q.get_and_reset_dropped_count(), 2u); +} + +TEST(LockFreeQueueNoAtomics, InterleavedPushPop) { + esphome::LockFreeQueue q; + int v[64]; + int popped = 0; + for (int i = 0; i < 64; i++) { + v[i] = i; + ASSERT_TRUE(q.push(&v[i])); + if (i % 2 == 1) { + int *first = q.pop(); + ASSERT_NE(first, nullptr); + EXPECT_EQ(*first, popped++); + int *second = q.pop(); + ASSERT_NE(second, nullptr); + EXPECT_EQ(*second, popped++); + } + } + EXPECT_TRUE(q.empty()); + EXPECT_EQ(popped, 64); +} + +} // namespace esphome::core::testing