Skip to content

docs: explain why bonding fails over a Bluetooth proxy - #409

Open
bluetoothbot wants to merge 3 commits into
Bluetooth-Devices:mainfrom
bluetoothbot:koan/document-insufficient-authentication
Open

bluetoothbot wants to merge 3 commits into
Bluetooth-Devices:mainfrom
bluetoothbot:koan/document-insufficient-authentication

Conversation

@bluetoothbot

@bluetoothbot bluetoothbot commented Jul 26, 2026

Copy link
Copy Markdown
Contributor

What: Documents error=5 Insufficient authentication and why bonding
behaves differently through an ESPHome Bluetooth proxy.

Why: This is the most common unattributable failure in downstream
trackers — users see GATT error 5, try bluetoothctl pair on the Home
Assistant host, and it changes nothing. Three separate integration
maintainers currently describe proxy pairing as "not supported" without
being able to say what the actual boundary is (BMS_BLE-HA#735,
ha-specialized-turbo#25). The docs covered the NotImplementedError case
but never the field symptom.

How: Two facts a downstream maintainer cannot derive from our API.
(1) The bond is between the ESP32 and the peripheral — the host never
participates, so a host-adapter bond is invisible to the proxy. (2) The
pairing protocol is address-in / paired+error-out; neither
BluetoothDeviceRequest nor BluetoothDevicePairingResponse has a passkey,
numeric-comparison or IO-capability field, so only Just Works pairing can
complete over a proxy. Plus an ordered remedy: connect(pair=True) first,
then fall back to excluding the proxies via active: false (a hard
exclusion — a non-ACTIVE_CONNECTIONS proxy is never a connection
candidate) and bonding on the host.

Testing: sphinx-build -b html docs clean; line length checked at 88.


Quality Report

Changes: 1 file changed, 53 insertions(+)

Code scan: clean

Tests: failed (FAILED)

Branch hygiene: clean

Generated by Kōan

@codecov

codecov Bot commented Jul 26, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 100.00%. Comparing base (41931e6) to head (ba32aaf).

Additional details and impacted files
@@            Coverage Diff            @@
##              main      #409   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files            8         8           
  Lines          659       659           
  Branches        68        68           
=========================================
  Hits           659       659           

☔ View full report in Codecov by Harness.
📢 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.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@codspeed

codspeed Bot commented Jul 26, 2026

Copy link
Copy Markdown

Merging this PR will degrade performance by 3.21%

⚠️ Different runtime environments detected

Some benchmarks with significant performance changes were compared across different runtime environments,
which may affect the accuracy of the results.

Open the report in CodSpeed to investigate

❌ 1 regressed benchmark
✅ 4 untouched benchmarks

Warning

Please fix the performance issues or acknowledge them on CodSpeed.

Performance Changes

Benchmark BASE HEAD Efficiency
test_scanner_async_on_advertisement 8.9 ms 9.2 ms -3.21%

Tip

Investigate this regression by commenting @codspeedbot fix this regression on this PR, or directly use the CodSpeed MCP with your agent.


Comparing bluetoothbot:koan/document-insufficient-authentication (ba32aaf) with main (36a75ed)1

Open in CodSpeed

Footnotes

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

@bdraco
bdraco marked this pull request as ready for review August 18, 2026 17:22
@bluetoothbot

Copy link
Copy Markdown
Contributor Author

PR Review — docs: explain why bonding fails over a Bluetooth proxy

Well-researched docs addition with one factually wrong API call in its main remedy.

Every non-obvious technical claim in this section checks out against the source, which is unusual for a docs PR and worth calling out specifically:

  • The symptom block is verbatim-accurate. aioesphomeapi/core.py:389 builds exactly Bluetooth GATT Error address=... handle=... error=... description=..., and core.py:183 maps 5 to "Insufficient authentication". client.py:94-110 re-raises it as BleakError(str(ex)), so the quoted text is what the caller actually sees.
  • The passkey claim is provable, not folklore. I decoded the protobuf descriptor in api_pb2.py: BluetoothDeviceRequest carries address, request_type, has_address_type, address_type; BluetoothDevicePairingResponse carries address, paired, error. No passkey, no numeric-comparison, no IO-capability field in either direction — the "Just Works only" conclusion follows directly.
  • The active: false mechanism is right. connect.py:60 and connect.py:85 confirm a proxy without ACTIVE_CONNECTIONS is registered as a non-connectable scanner, so "hard exclusion, not an RSSI preference" is a correct and genuinely useful distinction.
  • Formatting is clean: longest added line is 77 chars, no trailing whitespace.

Issues:

  • 🟡 Step 1 tells readers to pass pair=True to connect(). In bleak 3.0.2 (the pinned version) pair is a BleakClient constructor arg that connect() forwards positionally, so await client.connect(pair=True) raises TypeError: connect() got multiple values for argument 'pair' — I reproduced it. Use BleakClient(device, pair=True) or establish_connection(..., pair=True) instead. The follow-on sentence about pairing before service discovery is correct and should stay.
  • 🟢 The active: false remedy does not say it is per-proxy — it disables proxied connections for every other peripheral those proxies serve, and assumes the device is in host-adapter range.
  • 🟢 The ordered remedy omits unpair() (client.py:473), the only way to clear a stale proxy-side bond, which produces this exact symptom.

One note on the quality report: it lists tests as FAILED. I ran the suite on this tree — 173 passed in 1.32s. That flag looks like a harness artifact, not a real regression, and codecov agrees at 100% on the head commit. Fix the pair=True line and this is ready to merge.


🟡 Important

1. `connect(pair=True)` is not a valid call — it raises TypeError
docs/troubleshooting.md:129-132

The primary remedy in this section tells the reader to "pass pair=True to connect()". That is not how bleak exposes the flag, and following it raises a TypeError rather than pairing.

In bleak 3.0.2 (the version pinned in poetry.lock), pair is a constructor argument on BleakClient, and BleakClient.connect() forwards it positionally to the backend:

def __init__(self, ..., *, timeout: float = 30, pair: bool = False, ...)
    self._pair_before_connect = pair

async def connect(self, **kwargs: Any) -> None:
    await self._backend.connect(self._pair_before_connect, **kwargs)

So await client.connect(pair=True) resolves to ESPHomeClient.connect(False, pair=True) against this repo's signature (client.py:273-274, async def connect(self, pair: bool, ...)). I ran it:

TypeError: connect() got multiple values for argument 'pair'

Why it matters: this section exists precisely because downstream maintainers cannot derive the boundary from the API, and step 1 is the action they will actually try first. Handing them a call that dies with a confusing TypeError replaces one unattributable failure with another — and it will look like a bleak-esphome bug, which is the opposite of what the page is trying to establish.

Fix — show one of the two forms the rest of docs/ already uses (docs/usage.md:90, docs/usage.md:264):

# direct bleak
client = bleak.BleakClient(device, pair=True)
await client.connect()

# or via bleak_retry_connector
client = await establish_connection(..., pair=True)

bleak_retry_connector.establish_connection() takes pair: bool = False and forwards it to the client constructor, so that path is correct as a kwarg there.

The rest of the sentence is accurate and worth keeping — I verified client.py:378-382 does if pair: await self._pair() before await self._get_services(...), so "pairs immediately after link-up and before service discovery" is exactly right.

1. Bond explicitly instead of relying on the peripheral to trigger it: pass
   `pair=True` to `connect()`. This backend pairs immediately after link-up
   and before service discovery, which is what a peripheral that protects
   its whole GATT database expects.

🟢 Suggestions

1. `active: false` remedy omits its collateral cost
docs/troubleshooting.md:135-141

The mechanism described here is correct — I verified connect.py:60 (connectable = bool(feature_flags & BluetoothProxyFeature.ACTIVE_CONNECTIONS)) and connect.py:85 (ESPHomeScanner(source, name, connector, connectable)), so a proxy without the flag really is registered as a non-connectable scanner and is a hard exclusion rather than an RSSI preference. That distinction is the most useful sentence in the section.

What is missing is what the reader gives up. active: false is a per-proxy setting, not per-device: applying it to "every proxy that can hear the device" also removes proxied connections for every other peripheral those proxies serve. In a typical multi-proxy install that is a large blast radius for one misbehaving device.

The remedy also silently assumes the peripheral is within range of the host adapter — if it were, the user probably would not be routing it through a proxy in the first place.

Suggest one added sentence, e.g.: "This is per-proxy, not per-device — those proxies stop accepting connections for all peripherals, and the device must be in radio range of the host adapter for this to help. Prefer restricting it to proxies that serve only this device."

3. If pairing is attempted but returns an error, assume the peripheral needs
   interactive pairing and route it through the host's own adapter instead:
   set `active: false` on the `bluetooth_proxy` block of every proxy that can
   hear the device, then bond the host adapter normally. A proxy without
   `ACTIVE_CONNECTIONS` is registered as a non-connectable scanner, so it is
   never a connection candidate — this is a hard exclusion, not an RSSI
   preference.
2. Ordered remedy never mentions clearing a stale proxy-side bond
docs/troubleshooting.md:127-141

The section opens by establishing that the bond lives on the ESP32, then gives an exhaustive-looking "What to do, in order" list that never mentions the one API this library exposes for manipulating that bond: unpair() (client.py:473-489).

A proxy-side bond that the peripheral no longer honours (peripheral factory-reset, its bond table full and the entry evicted, or the ESP32 holding an LTK from a previous pairing) produces exactly the symptom this section documents — encryption setup fails, protected handles return error=5, and retrying never clears it, which is what the intro promises. In that state, steps 1-3 all fail: pair=True will not re-pair over an existing bond entry, and the reader is pushed to the heavyweight active: false remedy for something a single unpair() would fix.

Suggest inserting a step between the current 2 and 3: try await client.unpair() to drop the proxy-side bond, then reconnect with pairing enabled. Same PAIRING flag gate as pair(), so the NotImplementedError caveat in step 2 already covers it.

(Unverified: I could not confirm the field frequency of stale-bond error=5 from this repo — the API surface supports the remedy, but the ordering is a judgement call.)

What to do, in order:

Checklist

  • Documented error strings match the actual source
  • Protocol/capability claims verified against the wire format
  • Documented API calls are valid against the pinned dependency versions — warning #1
  • Remedies state their trade-offs and preconditions — suggestion #1, suggestion #2
  • Line length and formatting conform to repo conventions
  • Diff matches the PR description with no scope creep
  • Test suite passes

To rebase and address feedback, mention me: @bluetoothbot rebase critical (fixes 🔴 only), @bluetoothbot rebase important (fixes 🔴 + 🟡), or @bluetoothbot rebase --fix for all. (A bare @bluetoothbot rebase only rebases onto the base branch.)


Automated review by Kōan (Claude) HEAD=bc0792e 4 min 56s

@bluetoothbot
bluetoothbot force-pushed the koan/document-insufficient-authentication branch from bc0792e to 6dc958a Compare August 18, 2026 18:25
@bluetoothbot

Copy link
Copy Markdown
Contributor Author

Rebase with requested adjustments

Branch koan/document-insufficient-authentication was rebased onto main and review feedback was applied.

Changes applied

  • Doc edits applied. Line lengths under 88. (pre-commit unrunnable locally — config/version mismatch, unrelated to this change.)
  • Replaced the invalid connect(pair=True) remedy with the two correct forms — bleak.BleakClient(device, pair=True) and establish_connection(..., pair=True) — per warning #1. Verified against the pinned bleak 3.0.2: pair is a BleakClient.__init__ keyword-only arg, connect() forwards self._pair_before_connect positionally into ESPHomeClient.connect(self, pair, ...) (client.py:273), so the old kwarg form would raise TypeError. establish_connection does take pair: bool = False.
  • Added stale-proxy-side-bond step calling await client.unpair() between the old steps 2 and 3, per suggestion #2; noted it shares the PAIRING flag gate (client.py:473-489) and that it needs a connected client (_raise_if_not_connected).
  • Added the collateral cost of active: false to the final step per suggestion #1: per-proxy not per-device, blocks connections for all peripherals those proxies serve, requires host-adapter radio range, prefer proxies serving only this device.

Stats

1 file changed, 74 insertions(+)
Actions performed
  • Already-solved check: negative (confidence=high, reasoning=No commit on main mentions bonding or error=5 Insufficient authentication, and grep of `docs/troub)
  • Rebased koan/document-insufficient-authentication onto upstream/main
  • Applied review feedback
  • Pre-push CI check: previous run still pending
  • Force-pushed koan/document-insufficient-authentication to origin
  • Private review gate skipped: disabled by config
  • CI check enqueued in ## CI (async)

CI status

CI will be checked asynchronously.


Automated by Kōan

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant