Skip to content

Commit 95b4e20

Browse files
authored
fix(ble): 3 connect attempts and drop half-open links on failed session setup (#173)
* fix(ble): reach more connection paths per connect and drop half-open links on failed session setup connect() now makes 4 attempts bounded to 10s each (was 2 x the connector's fixed 20s), keeping the ~40s worst case while letting habluetooth re-pick a path on each attempt. Failed session setup over an established link (notify setup, handshake timeout/rejection, pairing, wake_up(wait=True)) now disconnects before raising; an infotainment timeout on a car VCSEC reports asleep keeps the link. * fix(ble): drop the per-attempt timeout wrapper; default to 3 connect attempts Per review, the BleakClient subclass that pinned a shorter per-attempt timeout moves to a separate change. Keep link teardown on failed session setup. * test(session-info): read the captured write mock after a failed handshake drops the link
1 parent b849266 commit 95b4e20

5 files changed

Lines changed: 461 additions & 28 deletions

File tree

‎docs/bluetooth_vehicles.md‎

Lines changed: 22 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -74,21 +74,33 @@ disable keepalive or disconnect when you have no work for it.
7474

7575
## Connection Retry Budget
7676

77-
`connect()` and `connect_if_needed()` make at most two connection attempts by
78-
default. This allows one retry for a waking vehicle or weak signal while
79-
limiting a failed connection to roughly 40 seconds before raising
80-
`BluetoothTransportError`. In particular, this lets a `VehicleRouter` move to
81-
its cloud fallback promptly when the vehicle is discoverable but all of its BLE
82-
connection slots are occupied.
83-
84-
Pass `max_attempts` explicitly when an environment needs a larger retry budget:
77+
`connect()` and `connect_if_needed()` make up to three connection attempts by
78+
default. The underlying connector's per-attempt timeout is fixed at about 20
79+
seconds, so a connection that times out on every attempt raises
80+
`BluetoothTransportError` after roughly a minute, before a `VehicleRouter` moves
81+
to its cloud fallback.
82+
83+
Under Home Assistant, each attempt re-selects the best connection path (local
84+
adapter or ESPHome proxy), so several attempts let a failed adapter or proxy be
85+
followed by another path within one connect. Path selection itself belongs to
86+
Home Assistant's Bluetooth stack: it ranks paths by signal strength with only a
87+
small penalty for past failures, so a much weaker path that works is reached
88+
sooner across repeated connects, not necessarily within a single one.
89+
90+
Pass `max_attempts` explicitly when an environment needs a different budget;
91+
each extra attempt adds up to about 20 seconds to the worst case:
8592

8693
```python
8794
await vehicle.connect(max_attempts=4)
8895
```
8996

90-
The underlying connector's per-attempt timeout is fixed at about 20 seconds, so
91-
increasing this value increases the worst-case connection delay accordingly.
97+
A failed session setup over an established link - notification setup, a
98+
handshake timeout or rejection, a failed pairing, or a failed
99+
`wake_up(wait=True)` - disconnects the link before raising, so a retry starts on
100+
a fresh connection rather than a half-open one. An infotainment handshake that
101+
times out while VCSEC reports the car asleep keeps the link, since that read
102+
proved the link healthy and a sleeping car's infotainment is expected to be
103+
silent.
92104

93105
## Pair Vehicle
94106

‎tesla_fleet_api/tesla/vehicle/bluetooth.py‎

Lines changed: 113 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -131,9 +131,15 @@
131131
# wake of a sleeping car, while VCSEC reports awake after ~0.5s.
132132
DEFAULT_WAKE_TIMEOUT = 30.0
133133

134-
# The connector's per-attempt timeout is fixed and unexposed. Keep one retry for
135-
# transient failures without delaying Router fallback for its full default.
136-
DEFAULT_CONNECT_ATTEMPTS = 2
134+
# Under Home Assistant every connect attempt re-picks the best connection path
135+
# (local adapter or proxy), so the attempt count bounds how many paths one
136+
# connect can reach. The connector's per-attempt timeout is a fixed ~20s, so
137+
# three attempts keep an all-attempts-time-out failure to about a minute.
138+
DEFAULT_CONNECT_ATTEMPTS = 3
139+
140+
# Bound on tearing down a link after a failed session setup; the failure is
141+
# what gets reported, so a stuck disconnect must not hold it back.
142+
LINK_TEARDOWN_TIMEOUT = 5.0
137143

138144
if TYPE_CHECKING:
139145
# Resolved dynamically as ``bleak.BleakClient``/``bleak.BleakScanner`` at
@@ -659,7 +665,17 @@ def get_device(self) -> BLEDevice | None:
659665
return self.device
660666

661667
async def connect(self, max_attempts: int = DEFAULT_CONNECT_ATTEMPTS) -> None:
662-
"""Connect to the Tesla BLE device."""
668+
"""Connect to the Tesla BLE device.
669+
670+
Makes up to ``max_attempts`` connection attempts. Under Home Assistant
671+
each attempt re-selects the best available connection path, so a
672+
failed adapter or proxy is followed by the next one rather than ending
673+
the connect.
674+
675+
Any failure after a GATT link was established (notification setup, a
676+
dropped link) tears that link down before raising, so the vehicle is
677+
never left holding a half-open connection.
678+
"""
663679
if not self.device:
664680
raise ValueError(f"BLEDevice {self.ble_name} has not been found or set")
665681
# Resolve BleakClient dynamically: habluetooth replaces
@@ -695,14 +711,45 @@ async def connect(self, max_attempts: int = DEFAULT_CONNECT_ATTEMPTS) -> None:
695711
type(e).__name__,
696712
e,
697713
)
698-
client = self.client
699-
self.client = None
700-
if client:
701-
try:
702-
await client.disconnect()
703-
except (BleakError, TimeoutError):
704-
pass
714+
await self._drop_link(f"connect failed at {stage}")
705715
raise BluetoothTransportError from e
716+
except (Exception, TeslaFleetError):
717+
await self._drop_link(f"connect failed at {stage}")
718+
raise
719+
720+
async def _drop_link(self, reason: str) -> None:
721+
"""Tear down the current GATT link after a failed session setup.
722+
723+
Best effort and bounded: it never raises, so the caller's own failure
724+
is what surfaces. The next send reconnects via ``connect_if_needed``.
725+
"""
726+
await self._stop_keepalive()
727+
client = self.client
728+
self.client = None
729+
if client is None:
730+
return
731+
LOGGER.debug("BLE dropping link vin=%s: %s", self.vin, reason)
732+
try:
733+
async with asyncio.timeout(LINK_TEARDOWN_TIMEOUT):
734+
await client.disconnect()
735+
except Exception as e:
736+
LOGGER.debug(
737+
"BLE link teardown failed vin=%s %s: %s", self.vin, type(e).__name__, e
738+
)
739+
self._set_connected(False)
740+
741+
async def _drop_link_on_failure(self, reason: str, err: BaseException) -> None:
742+
"""Drop the link for a failed session setup unless ``err`` is exempt.
743+
744+
``SigningDisabled`` is raised before anything touches the link (a
745+
passive listener's held link must survive it), and cancellation or
746+
interpreter exit is not a session failure.
747+
"""
748+
if isinstance(err, SigningDisabled) or not isinstance(
749+
err, (Exception, TeslaFleetError)
750+
):
751+
return
752+
await self._drop_link(f"{reason}: {type(err).__name__}")
706753

707754
async def disconnect(self) -> bool:
708755
"""Disconnect from the Tesla BLE device."""
@@ -1338,6 +1385,35 @@ async def _command(
13381385
confirm_broadcast=confirm_broadcast,
13391386
)
13401387

1388+
async def handshakeVehicleSecurity(self) -> None:
1389+
"""Perform a handshake with the vehicle security domain.
1390+
1391+
A failed handshake drops the BLE link before raising, so a retry starts
1392+
on a fresh connection instead of a half-open one.
1393+
"""
1394+
try:
1395+
await super().handshakeVehicleSecurity()
1396+
except BaseException as err:
1397+
await self._drop_link_on_failure("vehicle security handshake failed", err)
1398+
raise
1399+
1400+
async def handshakeInfotainment(self) -> None:
1401+
"""Perform a handshake with the infotainment domain.
1402+
1403+
A failed handshake drops the BLE link before raising, unless VCSEC
1404+
reports the car asleep - a sleeping car's infotainment is expected to
1405+
be silent, and that read proves the link itself is alive.
1406+
"""
1407+
try:
1408+
await super().handshakeInfotainment()
1409+
except BluetoothTimeout:
1410+
if not await self._vehicle_asleep():
1411+
await self._drop_link("infotainment handshake failed")
1412+
raise
1413+
except BaseException as err:
1414+
await self._drop_link_on_failure("infotainment handshake failed", err)
1415+
raise
1416+
13411417
async def _ensure_handshake(self, domain: Domain) -> None:
13421418
if not self._sessions[domain].ready:
13431419
await self._handshake(domain)
@@ -1430,7 +1506,11 @@ async def _sendVehicleSecurity(
14301506
unresolved resolves as a best-effort success instead of raising - see
14311507
the class docstring.
14321508
"""
1433-
await self._ensure_handshake(Domain.DOMAIN_VEHICLE_SECURITY)
1509+
try:
1510+
await self._ensure_handshake(Domain.DOMAIN_VEHICLE_SECURITY)
1511+
except BaseException as err:
1512+
await self._drop_link_on_failure("vehicle security handshake failed", err)
1513+
raise
14341514
if self.confirmation == "optimistic":
14351515
return await self._send_optimistic(
14361516
Domain.DOMAIN_VEHICLE_SECURITY,
@@ -1490,6 +1570,8 @@ async def _send_infotainment_ladder(
14901570
# Nothing was sent yet, so waking and retrying cannot double-execute.
14911571
sleep_status = await self._sleep_status()
14921572
if sleep_status == VehicleSleepStatus_E.VEHICLE_SLEEP_STATUS_ASLEEP:
1573+
# A sleeping car's silent infotainment is expected, and the
1574+
# VCSEC read just proved the link alive: keep it.
14931575
if not wake:
14941576
raise
14951577
await self._wake_for_command(timeout)
@@ -1503,9 +1585,14 @@ async def _send_infotainment_ladder(
15031585
self._infotainment_boot_timeout, wake=False
15041586
)
15051587
except BluetoothTimeout:
1588+
await self._drop_link("infotainment handshake failed")
15061589
raise timeout from None
15071590
else:
1591+
await self._drop_link("infotainment handshake failed")
15081592
raise
1593+
except BaseException as err:
1594+
await self._drop_link_on_failure("infotainment handshake failed", err)
1595+
raise
15091596
if self.confirmation == "optimistic" and mutating:
15101597
return await self._send_optimistic(
15111598
Domain.DOMAIN_INFOTAINMENT,
@@ -1652,6 +1739,15 @@ async def pair(
16521739
uuid=randbytes(16),
16531740
)
16541741

1742+
try:
1743+
await self._pair(msg, timeout, poll_interval)
1744+
except BaseException as err:
1745+
await self._drop_link_on_failure("pairing failed", err)
1746+
raise
1747+
1748+
async def _pair(
1749+
self, msg: RoutableMessage, timeout: float, poll_interval: float
1750+
) -> None:
16551751
deadline = time.monotonic() + timeout
16561752

16571753
# Fast path: write the op once and wait one interval for its one-shot
@@ -1762,7 +1858,11 @@ async def wake_up(
17621858
return await self._sendVehicleSecurity(
17631859
UnsignedMessage(RKEAction=RKEAction_E.RKE_ACTION_WAKE_VEHICLE)
17641860
)
1765-
await self._await_infotainment(timeout, wake=True)
1861+
try:
1862+
await self._await_infotainment(timeout, wake=True)
1863+
except BaseException as err:
1864+
await self._drop_link_on_failure("wake failed", err)
1865+
raise
17661866
return {"response": {"result": True, "reason": ""}}
17671867

17681868
async def _await_infotainment(self, timeout: float, *, wake: bool) -> None:

0 commit comments

Comments
 (0)