feat: share device name cache across scanners - #443
Conversation
Passive scanners rarely see device names because the local name lives in SCAN_RSP and is only returned during active scanning, so a passive scanner that wins the dispatch (better RSSI or fresher ad) used to forward a nameless advertisement even when an active scanner had already learned the name. The manager now owns a small address to name dict that every scanner contributes to. New names replace cached ones using a case folded prefix rule, so 'Onv' upgrades to 'Onvis XXX' (a SCAN_RSP extension) while a clearly different name like 'Donkey' is treated as a rename and replaces the cached value. Truncations such as 'Onvis XXX' then 'Onv' are rejected; the longer complete name wins.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #443 +/- ##
==========================================
+ Coverage 95.94% 96.05% +0.11%
==========================================
Files 14 14
Lines 1922 1976 +54
Branches 232 247 +15
==========================================
+ Hits 1844 1898 +54
Misses 42 42
Partials 36 36 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Merging this PR will degrade performance by 5.52%
|
| Benchmark | BASE |
HEAD |
Efficiency | |
|---|---|---|---|---|
| 🆕 | test_update_name_cache_address_fallback |
N/A | 349.8 µs | N/A |
| 🆕 | test_update_name_cache_cold_first_name |
N/A | 760.3 µs | N/A |
| 🆕 | test_update_name_cache_prefix_rule_paths |
N/A | 2.2 ms | N/A |
| 🆕 | test_update_name_cache_steady_state_equality |
N/A | 385.1 µs | N/A |
| 🆕 | test_update_name_cache_steady_state_identity |
N/A | 342.8 µs | N/A |
| 👁 | test_inject_100_raw_unchanged_advertisements |
144.3 µs | 152.8 µs | -5.52% |
Comparing feat/shared-name-cache (b85da55) with main (78504ba)
CodSpeed flagged 8-22% regressions on the per-advertisement benchmarks because the cpdef _update_name_cache call cost ~350 ns on their hardware, paid on every ad even when no work was needed. Inline the steady-state check (cached name is service_info.name) so the dominant case is a single dict.get plus a pointer compare with no function call at all. Drop _update_name_cache from cpdef to cdef so the slow path no longer pays the Python override check, and route the cold path through a new cdef _handle_name_cache_miss helper that keeps the hot method readable. Add a Python visible seed_name_cache wrapper so BaseHaScanner.restore_discovered_devices can still seed at startup.
|
@bluetoothbot review |
Codecov flagged 5 lines in manager.py uncovered by the previous commit: the cached==name no-op in _handle_name_cache_miss (different scanner objects, same value), the post-update patch branch when _update_name_cache keeps the longer cached name, the cached==name no-op in _update_name_cache (seed path), and the equal-length case-only-diff branch. Add four targeted tests; three unit and one cross-scanner integration.
PR Review — feat: share device name cache across scannersSolid implementation of a cross-scanner name cache. The prior Copilot concerns (identity-vs-equality for the address-fallback detection, and 🟢 Suggestions1. Cache update precedes source-preference rejection (`src/habluetooth/manager.py`, L737)The cross-scanner name cache is updated in 2. `name_len > cached_len` branch skips actual extension verification (`src/habluetooth/manager.py`, L681)The docstring lists 'extension' (cached='Onv', new='Onvis XXX' -> store) and 'rename' (cached='Onv', new='Donkey' -> store) as distinct cases, but the longer-side branch (line 678-682) stores unconditionally without verifying the prefix relationship. This is intentional — both extension and longer-rename end up storing the new name — but the asymmetry with the shorter branch (which does Checklist
SummarySolid implementation of a cross-scanner name cache. The prior Copilot concerns (identity-vs-equality for the address-fallback detection, and |
There was a problem hiding this comment.
Pull request overview
This PR introduces a cross-scanner device-name cache in BluetoothManager so that names learned by active scanners (from SCAN_RSP) can be reused when passive scanners “win” advertisement dispatch, ensuring downstream bleak callbacks observe a consistent canonical device name.
Changes:
- Add a shared
_name_cache(address → best name) with a case-folded prefix/rename update policy and integrate it into the advertisement receive path. - Seed the shared cache from each scanner’s restored/persisted discovered-device history and evict entries on history clear / device disappearance.
- Add unit + integration tests for name policy, cross-scanner propagation, eviction, restore seeding, plus microbenchmarks for the hot-path updater.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
src/habluetooth/manager.py |
Implements _name_cache, updates it on advertisements, patches service info names, and evicts on clear/unavailable. |
src/habluetooth/manager.pxd |
Exposes _name_cache and declares Cython-typed helpers for the new hot path. |
src/habluetooth/base_scanner.py |
Seeds the shared cache from restored per-scanner history on startup. |
tests/conftest.py |
Clears _name_cache during bluetooth fixture teardown to prevent test cross-contamination. |
tests/test_name_cache.py |
Adds policy/unit tests and cross-scanner integration tests for propagation, eviction, and restore seeding. |
tests/test_benchmark_base_scanner.py |
Adds codspeed/benchmark coverage for _update_name_cache hot-path scenarios. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
…ache HaScanner.on_advertisement (scanner.py) pre-sets service_info._advertisement to bleak's AdvertisementData; without invalidation the patched service_info.name would not propagate to advertisement.local_name and bleak callbacks from a local passive scanner would still see a missing name. Clear _advertisement on the two patch paths in _handle_name_cache_miss so the lazy rebuild in _advertisement_internal picks up the canonical name. Also accept name == address (not just identity) for the address-fallback short-circuit so a producer that hands us equal-value distinct str objects still skips cache pollution. Add a test exercising the local passive scanner shape.
| # dict.get plus a pointer compare; the function call to the | ||
| # helper only fires when the cached name and the incoming name | ||
| # are different str objects, which excludes the dominant case of | ||
| # the same scanner re-broadcasting the same name. | ||
| cached_name = self._name_cache.get(service_info.address) | ||
| if cached_name is not service_info.name: |
| @pytest.mark.usefixtures("enable_bluetooth") | ||
| def test_update_name_cache_steady_state_identity(benchmark: BenchmarkFixture) -> None: | ||
| """Hot path: same name object as cached. Should be a dict.get + pointer compare.""" | ||
| manager = get_manager() | ||
| address = "44:44:33:11:23:60" | ||
| name = "Onvis XXX" | ||
| manager.seed_name_cache(address, name) | ||
| assert manager._name_cache[address] is name | ||
|
|
||
| @benchmark | ||
| def run(): | ||
| for _ in range(1000): | ||
| manager.seed_name_cache(address, name) | ||
|
|
Passive scanners almost never see device names since the local name lives in SCAN_RSP, so whenever a passive scanner wins the per-ad dispatch the manager forwarded a nameless advertisement even though an active scanner had already learned the name.
The manager now keeps a small address to name dict that every scanner contributes to, and patches
service_info.name/device.namefrom it before dispatch so bleak callbacks always see the canonical name. New names replace cached ones using a case folded prefix rule:Onvupgrades toOnvis XXX(SCAN_RSP extension),Onvis XXXthenOnvis kept as the longer complete name, and a clearly different name likeDonkeyreplaces the cached value as a rename.The cache is seeded from each scanner's persisted history on restore so passive only setups inherit known names across restarts, and it is evicted alongside
_all_historyonasync_clear_advertisement_historyand the unavailable tracker.Hot path is cpdef with length dispatched casefold so we do at most one
startswithper call; identity short circuit on the cached name keeps the steady state at around 14 ns per call on my machine (new dedicated codspeed benchmarks added).