From 95786459c19f84b8302a2ab6daefa72ebc6dda22 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Edvard=20Filistovi=C4=8D?= Date: Tue, 28 Jul 2026 12:54:58 +0300 Subject: [PATCH] [ble_device_base] Replace raw-advertisement std::function with a lightweight callback slot (#17902) --- .../bk72xx_ble_tracker/bk72xx_ble_tracker.cpp | 10 +- .../bk72xx_ble_tracker/bk72xx_ble_tracker.h | 7 +- esphome/components/ble_device_base/ble_hub.h | 35 ++++-- .../esp32_ble_tracker/esp32_ble_tracker.h | 6 +- .../ble_device_base/test_raw_callback.cpp | 101 ++++++++++++++++++ 5 files changed, 143 insertions(+), 16 deletions(-) create mode 100644 tests/components/ble_device_base/test_raw_callback.cpp diff --git a/esphome/components/bk72xx_ble_tracker/bk72xx_ble_tracker.cpp b/esphome/components/bk72xx_ble_tracker/bk72xx_ble_tracker.cpp index 8d7199bd0a..634285d530 100644 --- a/esphome/components/bk72xx_ble_tracker/bk72xx_ble_tracker.cpp +++ b/esphome/components/bk72xx_ble_tracker/bk72xx_ble_tracker.cpp @@ -123,8 +123,14 @@ void BK72xxBLETracker::dump_config() { void BK72xxBLETracker::on_scan_report(const bk72xx_ble::BLEScanReport &report) { // Raw callback (the raw-advertisement path). - if (this->raw_advertisement_callback_) - this->raw_advertisement_callback_(report.mac, report.rssi, report.addr_type, report.data, report.data_len); + if (this->raw_advertisement_callback_.is_set()) { + const ble_device_base::RawAdvertisement adv{.mac = report.mac, + .data = report.data, + .data_len = report.data_len, + .rssi = report.rssi, + .addr_type = report.addr_type}; + this->raw_advertisement_callback_.invoke(adv); + } #ifdef ESPHOME_BLE_DEVICE_BASE_LISTENER_COUNT ble_device_base::ESPBTDevice device; diff --git a/esphome/components/bk72xx_ble_tracker/bk72xx_ble_tracker.h b/esphome/components/bk72xx_ble_tracker/bk72xx_ble_tracker.h index b8d0b31e6a..9c70246f05 100644 --- a/esphome/components/bk72xx_ble_tracker/bk72xx_ble_tracker.h +++ b/esphome/components/bk72xx_ble_tracker/bk72xx_ble_tracker.h @@ -33,7 +33,6 @@ #include "esphome/core/helpers.h" #include -#include #ifdef USE_OTA_STATE_LISTENER #include "esphome/components/ota/ota_backend.h" @@ -84,8 +83,8 @@ class BK72xxBLETracker : public Component, this->listeners_.push_back(listener); #endif } - void set_raw_advertisement_callback(ble_device_base::RawAdvertisementCallback cb) override { - this->raw_advertisement_callback_ = std::move(cb); + void set_raw_advertisement_callback(ble_device_base::RawAdvertisementCallback callback) override { + this->raw_advertisement_callback_ = callback; } ble_device_base::HubCapabilities get_capabilities() const override { // The Beken BDK exposes no active-scan path (passive scanning only), so the @@ -131,7 +130,7 @@ class BK72xxBLETracker : public Component, uint32_t scan_period_start_{0}; // millis() at start of current scan period; used to rate-limit on_scan_end() bool scan_started_once_{false}; // true after first successful scan start; gates the period timer - ble_device_base::RawAdvertisementCallback raw_advertisement_callback_{nullptr}; + ble_device_base::RawAdvertisementCallback raw_advertisement_callback_{}; #ifdef ESPHOME_BLE_DEVICE_BASE_LISTENER_COUNT // Parsed-advertisement consumers registered through ble_device_base. // Codegen-sized: no heap allocation, no std::vector template instantiations. diff --git a/esphome/components/ble_device_base/ble_hub.h b/esphome/components/ble_device_base/ble_hub.h index 0f3f4de88d..c02b491237 100644 --- a/esphome/components/ble_device_base/ble_hub.h +++ b/esphome/components/ble_device_base/ble_hub.h @@ -17,15 +17,36 @@ #include "ble_device.h" #include -#include namespace esphome::ble_device_base { -/// Callback for raw advertisements (the bluetooth_proxy path). -/// mac[] is least-significant octet first (BLE controller convention); -/// the hub delivers on the ESPHome main loop. -using RawAdvertisementCallback = - std::function; +/// One raw advertisement as delivered by the controller — a borrowed view, +/// valid only for the duration of the invoke() callback. +struct RawAdvertisement { + /// Least-significant octet first (BLE controller convention). + const uint8_t *mac; + const uint8_t *data; + uint16_t data_len; + int8_t rssi; // signed dBm + uint8_t addr_type; +}; + +/// Subscriber slot for the raw-advertisement stream (the bluetooth_proxy +/// path). The hub delivers on the ESPHome main loop. Same shape as +/// logger.h's LogCallback: an instance pointer plus a plain function +/// pointer — no virtuals, no std::function. +/// +/// Usage: +/// hub->set_raw_advertisement_callback({this, [](void *self, const RawAdvertisement &adv) { +/// static_cast(self)->on_raw_advertisement(adv); +/// }}); +struct RawAdvertisementCallback { + void *instance{nullptr}; + void (*fn)(void *instance, const RawAdvertisement &adv){nullptr}; + /// A default-constructed slot is "no subscriber"; hubs must guard on this. + bool is_set() const { return this->fn != nullptr; } + void invoke(const RawAdvertisement &adv) const { this->fn(this->instance, adv); } +}; /// What a tracker's controller/SDK can do — consumers branch on data, not #ifdefs. struct HubCapabilities { @@ -48,7 +69,7 @@ class BLEHub { virtual void register_listener(ESPBTDeviceListener *listener) = 0; /// Wire the raw-advertisement stream (bluetooth_proxy). One consumer at a time. - virtual void set_raw_advertisement_callback(RawAdvertisementCallback cb) = 0; + virtual void set_raw_advertisement_callback(RawAdvertisementCallback callback) = 0; virtual HubCapabilities get_capabilities() const = 0; diff --git a/esphome/components/esp32_ble_tracker/esp32_ble_tracker.h b/esphome/components/esp32_ble_tracker/esp32_ble_tracker.h index 01ae22e710..b0357289f1 100644 --- a/esphome/components/esp32_ble_tracker/esp32_ble_tracker.h +++ b/esphome/components/esp32_ble_tracker/esp32_ble_tracker.h @@ -239,8 +239,8 @@ class ESP32BLETracker final : public Component, // ---- ble_device_base::BLEHub (the platform-neutral tracker contract) ---- void register_listener(ble_device_base::ESPBTDeviceListener *listener) override; - void set_raw_advertisement_callback(ble_device_base::RawAdvertisementCallback cb) override { - this->raw_advertisement_callback_ = std::move(cb); + void set_raw_advertisement_callback(ble_device_base::RawAdvertisementCallback callback) override { + this->raw_advertisement_callback_ = callback; } ble_device_base::HubCapabilities get_capabilities() const override { return {/* active_scan = */ true, /* merges_scan_response = */ true, /* gatt = */ true}; @@ -341,7 +341,7 @@ class ESP32BLETracker final : public Component, #ifdef ESPHOME_BLE_DEVICE_BASE_LISTENER_COUNT StaticVector neutral_listeners_; #endif - ble_device_base::RawAdvertisementCallback raw_advertisement_callback_{nullptr}; + ble_device_base::RawAdvertisementCallback raw_advertisement_callback_{}; #ifdef USE_ESP32_BLE_DEVICE /// Per-period "Found device" DEBUG log with MAC dedup (shared ble_device_base impl) ble_device_base::DiscoveredDeviceLog discovered_log_; diff --git a/tests/components/ble_device_base/test_raw_callback.cpp b/tests/components/ble_device_base/test_raw_callback.cpp new file mode 100644 index 0000000000..cd18c3db59 --- /dev/null +++ b/tests/components/ble_device_base/test_raw_callback.cpp @@ -0,0 +1,101 @@ +#include + +#include + +#include "esphome/components/ble_device_base/ble_hub.h" + +namespace esphome::ble_device_base::testing { + +// Exercises the hub contract around RawAdvertisementCallback, not just the +// struct: a hub stores one slot via set_raw_advertisement_callback(), fires it +// only when set ("no subscriber" is the default-constructed slot), and a new +// registration replaces the old ("one consumer at a time"). +// +// The in-tree emit site (BK72xxBLETracker::on_scan_report) compiles against +// the Beken SDK and cannot run host-side, so the guard-and-fire semantics are +// pinned here through a minimal host BLEHub implementation instead. +namespace { + +class FakeHub : public BLEHub { + public: + void register_listener(ESPBTDeviceListener *listener) override {} + void set_raw_advertisement_callback(RawAdvertisementCallback callback) override { this->callback_ = callback; } + HubCapabilities get_capabilities() const override { return {false, false, false}; } + void get_adapter_mac(uint8_t out[6]) override {} + bool scan_running() override { return false; } + bool scan_active() override { return false; } + + /// The emit path every tracker implements: fire only when a subscriber is set. + void emit(const RawAdvertisement &adv) { + if (this->callback_.is_set()) + this->callback_.invoke(adv); + } + + protected: + RawAdvertisementCallback callback_; // default-constructed: no subscriber +}; + +struct CapturingSubscriber { + RawAdvertisement last{}; + int calls{0}; + + static void trampoline(void *self, const RawAdvertisement &adv) { + auto *sub = static_cast(self); + sub->last = adv; + sub->calls++; + } +}; + +// Device AA:BB:CC:DD:EE:FF — controller order delivers FF first. +const uint8_t MAC_LSB_FIRST[6] = {0xff, 0xee, 0xdd, 0xcc, 0xbb, 0xaa}; +const uint8_t ADV_DATA[4] = {0x02, 0x01, 0x06, 0x00}; + +RawAdvertisement make_test_adv() { + return RawAdvertisement{ + .mac = MAC_LSB_FIRST, .data = ADV_DATA, .data_len = sizeof(ADV_DATA), .rssi = -63, .addr_type = 1}; +} + +} // namespace + +TEST(RawAdvertisementCallback, DefaultConstructedSlotIsNotSet) { + const RawAdvertisementCallback callback{}; + EXPECT_FALSE(callback.is_set()); +} + +TEST(RawAdvertisementCallback, SubscriberSeesFieldsUnchanged) { + FakeHub hub; + CapturingSubscriber subscriber; + hub.set_raw_advertisement_callback({&subscriber, CapturingSubscriber::trampoline}); + + hub.emit(make_test_adv()); + + ASSERT_EQ(subscriber.calls, 1); + EXPECT_EQ(subscriber.last.mac, MAC_LSB_FIRST); + EXPECT_EQ(subscriber.last.data, ADV_DATA); + EXPECT_EQ(subscriber.last.data_len, sizeof(ADV_DATA)); + EXPECT_EQ(subscriber.last.rssi, -63); + EXPECT_EQ(subscriber.last.addr_type, 1); +} + +TEST(RawAdvertisementCallback, NoSubscriberDoesNotFire) { + FakeHub hub; + // No set_raw_advertisement_callback(): emitting must be a guarded no-op, + // not a jump through a garbage pointer. + hub.emit(make_test_adv()); +} + +TEST(RawAdvertisementCallback, NewSubscriberReplacesOld) { + FakeHub hub; + CapturingSubscriber first; + CapturingSubscriber second; + hub.set_raw_advertisement_callback({&first, CapturingSubscriber::trampoline}); + hub.set_raw_advertisement_callback({&second, CapturingSubscriber::trampoline}); + + hub.emit(make_test_adv()); + + EXPECT_EQ(first.calls, 0); // one consumer at a time + ASSERT_EQ(second.calls, 1); + EXPECT_EQ(second.last.rssi, -63); +} + +} // namespace esphome::ble_device_base::testing