Skip to content

Commit

Permalink
fix: keep alive being rescheduled too often
Browse files Browse the repository at this point in the history
  • Loading branch information
bdraco committed Jun 13, 2023
1 parent d1009e6 commit 817fb13
Showing 1 changed file with 42 additions and 32 deletions.
74 changes: 42 additions & 32 deletions src/yalexs_ble/push.py
Original file line number Diff line number Diff line change
Expand Up @@ -259,7 +259,8 @@ def __init__(
self._seen_this_session: set[
type[LockStatus] | type[DoorStatus] | type[BatteryState] | type[AuthState]
] = set()
self._disconnect_or_keep_alive_timer: asyncio.TimerHandle | None = None
self._disconnect_timer: asyncio.TimerHandle | None = None
self._keep_alive_timer: asyncio.TimerHandle | None = None

Check warning on line 263 in src/yalexs_ble/push.py

View check run for this annotation

Codecov / codecov/patch

src/yalexs_ble/push.py#L262-L263

Added lines #L262 - L263 were not covered by tests
self._idle_disconnect_delay = idle_disconnect_delay
self._next_disconnect_delay = idle_disconnect_delay
self._first_update_future: asyncio.Future[None] | None = None
Expand Down Expand Up @@ -397,34 +398,37 @@ def _disconnected_callback(self) -> None:
def _keep_alive(self) -> None:
"""Keep the lock connection alive."""
self._schedule_future_update_with_debounce(0)
self._reset_disconnect_or_keep_alive_timer()
self._schedule_next_keep_alive()

Check warning on line 401 in src/yalexs_ble/push.py

View check run for this annotation

Codecov / codecov/patch

src/yalexs_ble/push.py#L401

Added line #L401 was not covered by tests

def _time_since_last_operation(self) -> float:
"""Return the time since the last operation."""
return time.monotonic() - self._last_operation_complete_time

Check warning on line 405 in src/yalexs_ble/push.py

View check run for this annotation

Codecov / codecov/patch

src/yalexs_ble/push.py#L405

Added line #L405 was not covered by tests

def _reset_disconnect_or_keep_alive_timer(self) -> None:
def _schedule_next_keep_alive(self) -> None:
"""Schedule the next keep alive."""
self._cancel_keepalive_timer()
self._keep_alive_timer = self.loop.call_later(

Check warning on line 410 in src/yalexs_ble/push.py

View check run for this annotation

Codecov / codecov/patch

src/yalexs_ble/push.py#L409-L410

Added lines #L409 - L410 were not covered by tests
max(0, KEEP_ALIVE_TIME - self._time_since_last_operation()),
self._keep_alive,
)

def _reset_disconnect_timer(self) -> None:
"""Reset disconnect timer."""
self._cancel_disconnect_or_keep_alive_timer()
self._expected_disconnect = False
if self._always_connected:
self._disconnect_or_keep_alive_timer = self.loop.call_later(
max(0, KEEP_ALIVE_TIME - self._time_since_last_operation()),
self._keep_alive,
)
return

self._cancel_disconnect_timer()
self._expected_disconnect = False

Check warning on line 420 in src/yalexs_ble/push.py

View check run for this annotation

Codecov / codecov/patch

src/yalexs_ble/push.py#L419-L420

Added lines #L419 - L420 were not covered by tests
timeout = self._next_disconnect_delay
_LOGGER.debug(
"%s: Resetting disconnect timer to %s seconds", self.name, timeout
)
self._disconnect_or_keep_alive_timer = self.loop.call_later(
self._disconnect_timer = self.loop.call_later(

Check warning on line 425 in src/yalexs_ble/push.py

View check run for this annotation

Codecov / codecov/patch

src/yalexs_ble/push.py#L425

Added line #L425 was not covered by tests
timeout, self._disconnect_with_timer, timeout
)

async def _execute_forced_disconnect(self, reason: str) -> None:
"""Execute forced disconnection."""
self._cancel_disconnect_or_keep_alive_timer()
self._cancel_disconnect_timer()

Check warning on line 431 in src/yalexs_ble/push.py

View check run for this annotation

Codecov / codecov/patch

src/yalexs_ble/push.py#L431

Added line #L431 was not covered by tests
_LOGGER.debug("%s: Executing forced disconnect: %s", self.name, reason)
if (update_task := self._update_task) and not update_task.done():
self._update_task = None
Expand All @@ -436,28 +440,34 @@ async def _execute_forced_disconnect(self, reason: str) -> None:
def _disconnect_with_timer(self, timeout: float) -> None:
"""Disconnect from device.
This should only ever be called from _reset_disconnect_or_keep_alive_timer
This should only ever be called from _reset_disconnect_timer
"""
if self._operation_lock.locked():
_LOGGER.debug("%s: Disconnect timer reset due to operation lock", self.name)
self._reset_disconnect_or_keep_alive_timer()
self._reset_disconnect_timer()

Check warning on line 447 in src/yalexs_ble/push.py

View check run for this annotation

Codecov / codecov/patch

src/yalexs_ble/push.py#L447

Added line #L447 was not covered by tests
return
if self._cancel_deferred_update:
_LOGGER.debug(
"%s: Disconnect timer fired while we were waiting to update", self.name
)
self._reset_disconnect_or_keep_alive_timer()
self._reset_disconnect_timer()

Check warning on line 453 in src/yalexs_ble/push.py

View check run for this annotation

Codecov / codecov/patch

src/yalexs_ble/push.py#L453

Added line #L453 was not covered by tests
self._cancel_future_update()
self._deferred_update()
return
self._cancel_disconnect_or_keep_alive_timer()
self._cancel_disconnect_timer()

Check warning on line 457 in src/yalexs_ble/push.py

View check run for this annotation

Codecov / codecov/patch

src/yalexs_ble/push.py#L457

Added line #L457 was not covered by tests
self.background_task(self._execute_timed_disconnect(timeout))

def _cancel_disconnect_or_keep_alive_timer(self) -> None:
def _cancel_disconnect_timer(self) -> None:
"""Cancel disconnect timer."""
if self._disconnect_or_keep_alive_timer:
self._disconnect_or_keep_alive_timer.cancel()
self._disconnect_or_keep_alive_timer = None
if self._disconnect_timer:
self._disconnect_timer.cancel()
self._disconnect_timer = None

Check warning on line 464 in src/yalexs_ble/push.py

View check run for this annotation

Codecov / codecov/patch

src/yalexs_ble/push.py#L463-L464

Added lines #L463 - L464 were not covered by tests

def _cancel_keepalive_timer(self) -> None:
"""Cancel keep alive timer."""
if self._keep_alive_timer:
self._keep_alive_timer.cancel()
self._keep_alive_timer = None

Check warning on line 470 in src/yalexs_ble/push.py

View check run for this annotation

Codecov / codecov/patch

src/yalexs_ble/push.py#L469-L470

Added lines #L469 - L470 were not covered by tests

async def _execute_timed_disconnect(self, timeout: float) -> None:
"""Execute timed disconnection."""
Expand All @@ -477,15 +487,13 @@ async def _async_handle_disconnected(self, exc: Exception) -> None:
self.name,
)
return
self._cancel_disconnect_or_keep_alive_timer()
self._cancel_disconnect_timer()

Check warning on line 490 in src/yalexs_ble/push.py

View check run for this annotation

Codecov / codecov/patch

src/yalexs_ble/push.py#L490

Added line #L490 was not covered by tests
await self._execute_disconnect()

async def _execute_disconnect(self) -> None:
"""Execute disconnection."""
async with self._connect_lock:
if (
self._disconnect_or_keep_alive_timer
): # If the timer was reset, don't disconnect
if self._disconnect_timer: # If the timer was reset, don't disconnect
return
client = self._client
self._client = None
Expand All @@ -497,20 +505,20 @@ async def _execute_disconnect(self) -> None:
async def _ensure_connected(self) -> Lock:
"""Ensure connection to device is established."""
if self._connect_lock.locked():
self._reset_disconnect_or_keep_alive_timer()
self._reset_disconnect_timer()

Check warning on line 508 in src/yalexs_ble/push.py

View check run for this annotation

Codecov / codecov/patch

src/yalexs_ble/push.py#L508

Added line #L508 was not covered by tests
_LOGGER.debug(
"%s: Connection already in progress, waiting for it to complete",
self.name,
)
if self.is_connected:
assert self._client is not None # nosec
self._reset_disconnect_or_keep_alive_timer()
self._reset_disconnect_timer()

Check warning on line 515 in src/yalexs_ble/push.py

View check run for this annotation

Codecov / codecov/patch

src/yalexs_ble/push.py#L514-L515

Added lines #L514 - L515 were not covered by tests
return self._client
async with self._connect_lock:
# Check again while holding the lock
if self.is_connected:
assert self._client is not None # type: ignore[unreachable] # nosec
self._reset_disconnect_or_keep_alive_timer()
self._reset_disconnect_timer()

Check warning on line 521 in src/yalexs_ble/push.py

View check run for this annotation

Codecov / codecov/patch

src/yalexs_ble/push.py#L520-L521

Added lines #L520 - L521 were not covered by tests
return self._client
self._client = self._get_lock_instance()
try:
Expand All @@ -522,7 +530,7 @@ async def _ensure_connected(self) -> Lock:
await self._client.disconnect()
raise
self._next_disconnect_delay = self._idle_disconnect_delay
self._reset_disconnect_or_keep_alive_timer()
self._reset_disconnect_timer()

Check warning on line 533 in src/yalexs_ble/push.py

View check run for this annotation

Codecov / codecov/patch

src/yalexs_ble/push.py#L533

Added line #L533 was not covered by tests
self._seen_this_session.clear()
return self._client

Expand Down Expand Up @@ -567,13 +575,14 @@ async def _execute_lock_operation(
now = time.monotonic()
self._last_lock_operation_complete_time = now
self._last_operation_complete_time = now
self._reset_disconnect_or_keep_alive_timer()
self._reset_disconnect_timer()
self._schedule_next_keep_alive()

Check warning on line 579 in src/yalexs_ble/push.py

View check run for this annotation

Codecov / codecov/patch

src/yalexs_ble/push.py#L575-L579

Added lines #L575 - L579 were not covered by tests

def _state_callback(
self, states: Iterable[LockStatus | DoorStatus | BatteryState]
) -> None:
"""Handle state change."""
self._reset_disconnect_or_keep_alive_timer()
self._reset_disconnect_timer()

Check warning on line 585 in src/yalexs_ble/push.py

View check run for this annotation

Codecov / codecov/patch

src/yalexs_ble/push.py#L585

Added line #L585 was not covered by tests
self._update_any_state(states)

def _get_current_state(self) -> LockState:
Expand Down Expand Up @@ -706,10 +715,11 @@ async def _update(self) -> LockState:
# so that if another update request is pending
# we do not disconnect until it completes.
self._next_disconnect_delay = FIRST_CONNECTION_DISCONNECT_TIME
self._reset_disconnect_or_keep_alive_timer()
self._reset_disconnect_timer()

Check warning on line 718 in src/yalexs_ble/push.py

View check run for this annotation

Codecov / codecov/patch

src/yalexs_ble/push.py#L718

Added line #L718 was not covered by tests

if made_request:
self._last_operation_complete_time = time.monotonic()
self._schedule_next_keep_alive()

Check warning on line 722 in src/yalexs_ble/push.py

View check run for this annotation

Codecov / codecov/patch

src/yalexs_ble/push.py#L721-L722

Added lines #L721 - L722 were not covered by tests
return state

def _callback_state(self, lock_state: LockState) -> None:
Expand Down Expand Up @@ -821,7 +831,7 @@ def update_advertisement(
# would keep the connection idle for too long and
# get us disconnected anyways.
self._next_disconnect_delay = self._idle_disconnect_delay * 2
self._reset_disconnect_or_keep_alive_timer()
self._reset_disconnect_timer()
return
self._schedule_future_update_with_debounce(next_update)

Check warning on line 836 in src/yalexs_ble/push.py

View check run for this annotation

Codecov / codecov/patch

src/yalexs_ble/push.py#L833-L836

Added lines #L833 - L836 were not covered by tests

Expand Down

0 comments on commit 817fb13

Please sign in to comment.