[bk72xx_ble] Scan primitives and main-task scan report queue (#17802)

Co-authored-by: J. Nick Koston <nick@koston.org>

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_<chip>.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<const recv_adv_t *>(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<int8_t>(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<uint8_t>(data_len) : static_cast<uint8_t>(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<uint8_t>(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<int>(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

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 <cstdint>
+#include <vector>
 
 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<BLEScanListener *> 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<BLEScanReport, MAX_SCAN_REPORT_QUEUE_SIZE> 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<BLEScanReport, MAX_SCAN_REPORT_QUEUE_SIZE - 1> 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};
 };

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 <atomic>
 #include <cstddef>
@@ -86,4 +86,4 @@ template<class T, uint8_t SIZE> class EventPool {
 
 }  // namespace esphome
 
-#endif  // defined(USE_ESP32)
+#endif  // defined(USE_ESP32) || defined(USE_ZEPHYR) || defined(USE_LIBRETINY)

esphome/core/lock_free_queue.h

@@ -1,5 +1,7 @@
 #pragma once
 
+#include "esphome/core/defines.h"
+
 #include <atomic>
 #include <cstddef>
 
@@ -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<typename T> 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<typename T> using AtomicIndex = PlainAtomic<T>;
+#else
+template<typename T> using AtomicIndex = std::atomic<T>;
+#endif
+}  // namespace lockfree_internal
+
 // Base lock-free queue without task notification
 template<class T, uint8_t SIZE> class LockFreeQueue {
  public:
@@ -126,13 +176,13 @@ template<class T, uint8_t SIZE> class LockFreeQueue {
  protected:
   T *buffer_[SIZE]{};
   // Atomic: written by producer (push/increment), read+reset by consumer (get_and_reset)
-  std::atomic<uint16_t> dropped_count_;  // 65535 max - more than enough for drop tracking
+  lockfree_internal::AtomicIndex<uint16_t> 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<uint8_t> head_;
+  lockfree_internal::AtomicIndex<uint8_t> head_;
   // Atomic: written by producer (push), read by consumer (pop) to check if empty
-  std::atomic<uint8_t> tail_;
+  lockfree_internal::AtomicIndex<uint8_t> tail_;
 };
 
 #ifdef USE_ESP32

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 <gtest/gtest.h>
+
+namespace esphome::core::testing {
+
+TEST(LockFreeQueueNoAtomics, EmptyPopReturnsNull) {
+  esphome::LockFreeQueue<int, 4> q;
+  EXPECT_EQ(q.pop(), nullptr);
+  EXPECT_TRUE(q.empty());
+  EXPECT_FALSE(q.full());
+  EXPECT_EQ(q.size(), 0u);
+}
+
+TEST(LockFreeQueueNoAtomics, FifoOrder) {
+  esphome::LockFreeQueue<int, 4> 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<int, 4> 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<int, 4> q;
+  EXPECT_FALSE(q.push(nullptr));
+  EXPECT_TRUE(q.empty());
+}
+
+TEST(LockFreeQueueNoAtomics, WrapAround) {
+  esphome::LockFreeQueue<int, 4> 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<int, 4> 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<int, 8> 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