[core] Shared slot count factory for codegen sized listener storage - #18057
Conversation
|
@bluetoothbot review |
1 similar comment
|
@bluetoothbot review |
|
To use the changes in this PR: # Clone the repository:
git clone https://github.com/esphome/esphome
cd esphome
# Checkout the PR branch:
git fetch origin pull/18057/head:ble_listener_slot_factory
git checkout ble_listener_slot_factory
# Install the development version:
script/setup
# Activate the development version:
source venv/bin/activateNow you can run (Added by the PR bot) |
|
@bluetoothbot review |
1 similar comment
|
@bluetoothbot review |
|
|
There was a problem hiding this comment.
Tip
No blocking issues found — ready to merge.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## dev #18057 +/- ##
==========================================
+ Coverage 86.91% 87.10% +0.18%
==========================================
Files 61 62 +1
Lines 14534 14599 +65
Branches 2189 2200 +11
==========================================
+ Hits 12632 12716 +84
+ Misses 1593 1575 -18
+ Partials 309 308 -1
🚀 New features to boost your workflow:
|
|
@bluetoothbot review |
5e62c66 to
b0ef638
Compare
Memory Impact AnalysisComponents:
This analysis runs automatically when components change. Memory usage is measured from a representative test configuration. |
b0ef638 to
1047b01
Compare
|
@bluetoothbot review |
|
|
There was a problem hiding this comment.
Tip
No blocking issues found — ready to merge.
|
|
There was a problem hiding this comment.
Tip
No blocking issues found — ready to merge.
1047b01 to
9b37520
Compare
|
|
There was a problem hiding this comment.
Tip
No blocking issues found — ready to merge.
|
👋 Hi there! This PR modifies 8 file(s) with codeowners. @Bl00d-B0b, @esphome/core - As codeowner(s) of the affected files, your review would be appreciated! 🙏 Note: Automatic review request may have failed, but you're still welcome to review. |
There was a problem hiding this comment.
Pull request overview
This PR centralizes the “count slots during codegen, then emit a #define at FINAL priority” pattern into a shared cg.slot_counter(DEFINE) factory, reducing duplicated listener-count plumbing across BLE components while preserving existing define names and public request function names.
Changes:
- Added
slot_counter()/get_slot_count()toesphome/cpp_helpers.pyand re-exported them viaesphome/codegen.py. - Migrated LibreTiny BLE controllers/trackers and
ble_device_baseaway from bespoke CORE.data + FINAL emit jobs to the shared factory. - Added unit + component tests validating define emission behavior (including FINAL-time request edge cases) and end-to-end behavior for BK72xx configs.
Reviewed changes
Copilot reviewed 13 out of 13 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| tests/unit_tests/test_cpp_helpers.py | Adds unit tests covering slot_counter behavior (emit, no-emit, FINAL-time request edge cases, late-request error). |
| tests/component_tests/helpers.py | Adds a helper to read rendered CORE define values for component tests. |
| tests/component_tests/ble_device_base/test_slot_counter.py | End-to-end component tests asserting listener-count define emission/absence via real configs and a mock BLEHub path. |
| tests/component_tests/ble_device_base/config/bk72xx_tracker.yaml | Minimal tracker config used to assert count define is emitted. |
| tests/component_tests/ble_device_base/config/bk72xx_controller_only.yaml | Minimal controller-only config used to assert count define is not emitted. |
| esphome/cpp_helpers.py | Implements the shared slot_counter factory and get_slot_count backed by CORE.data. |
| esphome/components/rp2_ble_tracker/init.py | Removes the tracker-local FINAL emit job in favor of the shared slot-counter mechanism. |
| esphome/components/ln882h_ble/init.py | Replaces hand-rolled scan listener counting/emission with cg.slot_counter(...). |
| esphome/components/esp32_ble_tracker/init.py | Switches neutral listener feature-coupling to rely on cg.get_slot_count(...) (define emission handled elsewhere). |
| esphome/components/ble_device_base/init.py | Converts listener counting to cg.slot_counter(...) and defines a shared LISTENER_COUNT_DEFINE. |
| esphome/components/bk72xx_ble/init.py | Replaces hand-rolled scan listener counting/emission with cg.slot_counter(...). |
| esphome/components/bk72xx_ble_tracker/init.py | Removes the tracker-local FINAL emit job in favor of the shared slot-counter mechanism. |
| esphome/codegen.py | Re-exports slot_counter and get_slot_count for component use via cg.*. |
|
@bluetoothbot review |
PR Review — [core] Shared slot count factory for codegen sized listener storageApprove — re-verification against the What holds up under checking this round:
Checklist
Automated review by Kōan (Claude) |
There was a problem hiding this comment.
Tip
No blocking issues found — ready to merge.
|
Thanks |
The shared cg.slot_counter factory now owns emitting ESPHOME_BLE_DEVICE_BASE_LISTENER_COUNT from ble_device_base itself, and get_listener_count() no longer exists, so the tracker's local FINAL coroutine and its CORE.add_job call are gone (same migration esphome#18057 applied to the bk72xx and rp2 trackers).
…anual listener-count emission
Rebased onto esphome#18057: listener counting via the core slot-counter factory; tracker carries no counting code.
Rebased onto esphome#18057: listener counting via the core slot-counter factory; tracker carries no counting code.
Rebased onto esphome#18057: listener counting via the core slot-counter factory; tracker carries no counting code.
Rebased onto esphome#18057: listener counting via the core slot-counter factory; tracker carries no counting code.
Rebased onto esphome#18057: listener counting via the core slot-counter factory; tracker carries no counting code.
Rebased onto esphome#18057: listener counting via the core slot-counter factory; tracker carries no counting code.
Rebased onto esphome#18057: listener counting via the core slot-counter factory; tracker carries no counting code.
What does this implement/fix?
Lifts the duplicated slot count pattern behind the codegen sized StaticVector listener storage into one shared factory. cg.slot_counter(define) returns a single request function; a consumer's to_code calls it once per slot, and the first request schedules a FINAL priority job that emits the define with the requested count. No requests, no define, so the guarded storage compiles out. There is no emit job for callers to wire up or forget.
The factory lives in esphome/cpp_helpers.py (exported through codegen) because nothing in it is BLE specific; bk72xx_ble and ln882h_ble drop their hand rolled copies and call it, keeping their public request_scan_listener_slot names and define names unchanged. ble_device_base uses the same factory for the neutral ESPHOME_BLE_DEVICE_BASE_LISTENER_COUNT, replacing the per tracker emit jobs in bk72xx_ble_tracker and rp2_ble_tracker.
wifi and ethernet carry the same hand rolled count machinery with a paired USE_* define; migrating them is left as a follow up since it also touches their C++ guards.
Codegen tests build real tracker and controller only configs and assert on the emitted defines; unit tests pin the factory itself, including a slot requested while FINAL jobs are already draining.
Chained on #18056.
Types of changes
Related issue or feature (if applicable):
Pull request in esphome.io with documentation (if applicable):
Pull request in developers.esphome.io with developer documentation (if applicable):
Test Environment
Example entry for
config.yaml:Checklist:
tests/folder).If user exposed functionality or configuration variables are added/changed: