Add bluetooth_gatt_stop_notify and auto-cleanup notify callbacks on disconnect

[bdraco]

What does this implement/fix?

Add synchronous bluetooth_gatt_stop_notify and bluetooth_gatt_stop_notify_for_address methods to safely clean up Bluetooth GATT notify callbacks, and automatically clean up notify callbacks when a Bluetooth device disconnects.

Previously, callers had to track and manage the two callables returned by bluetooth_gatt_start_notify, which made it easy to leak callbacks on disconnect. This change:

1. Tracks notify callbacks internally by (address, handle) in _notify_callbacks dict
2. Adds bluetooth_gatt_stop_notify(address, handle) - a sync method to stop a single notify session
3. Adds bluetooth_gatt_stop_notify_for_address(address) - a sync method to stop all notify sessions for an address
4. Wraps the connection state callback in bluetooth_device_connect to automatically call bluetooth_gatt_stop_notify_for_address when a device disconnects

The sync methods are safe to call from exception handlers without awaiting, which is needed for proper cleanup in bleak-esphome (see Bluetooth-Devices/bleak-esphome#259).

Types of changes

• Bugfix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)
• Breaking change (fix or feature that would cause existing functionality to not work as expected)
• Code quality improvements to existing code or addition of tests
• Other

Related issue or feature (if applicable):

• fixes Client design makes it too easy to leak bluetooth notify callbacks on disconnect #667

Pull request in esphome (if applicable):

• N/A

Checklist:

• The code change is tested and works locally.
• If api.proto was modified, a linked pull request has been made to esphome with the same changes.
• Tests have been added to verify that the new code works (under tests/ folder).

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 100.00%. Comparing base (e1dd002) to head (f9e90c8).
⚠️ Report is 4 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff            @@
##              main     #1487   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files           23        23           
  Lines         3488      3506   +18     
=========================================
+ Hits          3488      3506   +18     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[codspeed-hq]

Merging this PR will not alter performance

✅ 11 untouched benchmarks

---

_{Comparing bluetooth_gatt_stop_notify (f9e90c8) with main (e1dd002)¹}

Footnotes

1. No successful run was found on main (85ee1e2) during the generation of this report, so e1dd002 was used instead as the comparison base. There might be some changes unrelated to this pull request in this report. ↩

[Copilot]

Pull request overview

This pull request adds synchronous methods to stop Bluetooth GATT notify callbacks and implements automatic cleanup of notify callbacks when a Bluetooth device disconnects. This addresses issue #667 where the previous API design made it easy for callers to leak notify callbacks.

Changes:

• Added internal tracking of notify callbacks via _notify_callbacks dictionary in APIClientBase
• Introduced bluetooth_gatt_stop_notify() and bluetooth_gatt_stop_notify_for_address() synchronous methods for cleanup
• Modified bluetooth_device_connect() to automatically clean up notify callbacks on disconnect

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.

File	Description
aioesphomeapi/client_base.py	Added _notify_callbacks dictionary to __slots__ and initialized it in __init__ to track active notify sessions
aioesphomeapi/client_base.pxd	Added Cython declaration for _notify_callbacks dictionary
aioesphomeapi/client.py	Implemented notify cleanup methods, added auto-cleanup wrapper in bluetooth_device_connect(), and integrated with bluetooth_gatt_start_notify()
tests/test_client.py	Added comprehensive tests for the new stop methods and auto-cleanup on disconnect functionality

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[coderabbitai]

Note

Other AI code review bot(s) detected

CodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review.

Walkthrough

The changes implement a mechanism to prevent Bluetooth notify callback leaks on device disconnect by introducing an internal registry keyed by (address, handle) to track active notify callbacks, automatically cleaning them up on connection state changes and providing public methods to stop individual or address-wide notify sessions.

Changes

Cohort / File(s)	Summary
Callback Registry & GATT Notify Management aioesphomeapi/client_base.pxd, aioesphomeapi/client_base.py	Added _notify_callbacks dict to track active GATT notify callbacks by (address, handle) tuple.
Client GATT & Connection Cleanup aioesphomeapi/client.py	Modified bluetooth_device_connect with wrapper callback for cleanup on disconnect; updated bluetooth_gatt_start_notify to register removal callbacks; added bluetooth_gatt_stop_notify and bluetooth_gatt_stop_notify_for_address public methods; adjusted subscribe_home_assistant_states signature with on_state_sub and on_state_request parameters.
GATT & Subscription Tests tests/test_client.py	Added 221 lines of test coverage for Bluetooth GATT notify lifecycle, callback cleanup on disconnect/abort, address-specific cleanup, error handling in notify callbacks, and subscription path behaviors.

Sequence Diagram(s)

sequenceDiagram
    participant App as Application
    participant Client as APIClient
    participant Registry as Notify Registry<br/>(_notify_callbacks)
    participant GATT as GATT Layer
    participant ConnMgr as Connection Manager

    App->>Client: bluetooth_gatt_start_notify(address, handle)
    Client->>GATT: Enable notification
    GATT-->>Client: removal_callback
    Client->>Registry: Store removal_callback<br/>at (address, handle)
    Client-->>App: stop_fn(), wrapped_remove_fn()
    
    Note over Client,ConnMgr: Device disconnects...
    
    ConnMgr->>Client: _on_bluetooth_device_connection_response<br/>(disconnected)
    Client->>Client: on_bluetooth_connection_state_with_notify_cleanup()
    Client->>Client: bluetooth_gatt_stop_notify_for_address(address)
    Client->>Registry: Fetch all callbacks for address
    Registry-->>Client: List of removal_callbacks
    Client->>GATT: Invoke each removal_callback
    Client->>Registry: Clear callbacks for address
    
    Note over App,Client: Alternative: App stops notify
    App->>Client: stop_fn()
    Client->>Client: bluetooth_gatt_stop_notify(address, handle)
    Client->>Registry: Lookup callback at (address, handle)
    Client->>GATT: Invoke removal_callback
    Client->>Registry: Clear (address, handle) entry

Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related PRs

• PR #999: Both PRs modify bluetooth_device_connect in aioesphomeapi/client.py to improve cleanup on failed connections (one cleans up notify callbacks; the other forces disconnect on unhandled exceptions).

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 55.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title accurately and concisely describes the main change: adding bluetooth_gatt_stop_notify methods and automatic cleanup of notify callbacks on disconnect.
Description check	✅ Passed	The description is well-detailed and directly related to the changeset, explaining the problem, solution, and implementation approach with references to the specific issue.
Linked Issues check	✅ Passed	The PR comprehensively addresses issue #667 by implementing internal callback tracking, adding synchronous stop methods, and auto-cleaning up notify callbacks on disconnect as required.
Out of Scope Changes check	✅ Passed	All changes are directly scoped to the linked issue: internal _notify_callbacks tracking, stop_notify methods, and connection state wrapper for cleanup. The changes to subscribe_home_assistant_states appear narrowly focused.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing touches

• 📝 Generate docstrings

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}