Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Prevent multiple simultaneous WeConnect API calls on start up #245

Merged
merged 1 commit into from
Feb 20, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
55 changes: 47 additions & 8 deletions custom_components/volkswagen_we_connect_id/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,9 +3,11 @@

from dataclasses import dataclass
from datetime import timedelta
import functools
import logging
import asyncio
import threading
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This may look alarmingly like I’m replacing asyncio with threading, but not really! 😀 asyncio was deleted simply because it was not being used in this module (even before this PR), and threading was added for the use of threading.Lock.

By the way, this PR does not cause any threads to be spawn or any new execution parallelism. The threads were already there before this PR, making multiple parallel calls to the WeConnect API. This PR simply adds some thread coordination to prevent unnecessary API calls.

import time

from weconnect import weconnect
from weconnect.elements.vehicle import Vehicle
from weconnect.elements.control_operation import ControlOperation
Expand Down Expand Up @@ -47,13 +49,9 @@ async def async_setup_entry(hass: HomeAssistant, entry: ConfigEntry) -> bool:
"""Set up Volkswagen We Connect ID from a config entry."""

hass.data.setdefault(DOMAIN, {})
_we_connect = weconnect.WeConnect(
_we_connect = get_we_connect_api(
username=entry.data["username"],
password=entry.data["password"],
updateAfterLogin=False,
loginOnInit=False,
timeout=10,
updatePictures=False,
)

await hass.async_add_executor_job(_we_connect.login)
Expand Down Expand Up @@ -183,11 +181,49 @@ async def volkswagen_id_set_ac_charge_speed(call: ServiceCall) -> None:
return True


@functools.lru_cache(maxsize=1)
def get_we_connect_api(username: str, password: str) -> weconnect.WeConnect:
"""Return a cached weconnect api object, shared with the config flow."""
return weconnect.WeConnect(
username=username,
password=password,
updateAfterLogin=False,
loginOnInit=False,
timeout=10,
updatePictures=False,
)


_last_successful_api_update_timestamp: float = 0.0
_last_we_connect_api: weconnect.WeConnect | None = None
_update_lock = threading.Lock()


def update(
api: weconnect.WeConnect
) -> None:
"""API call to update vehicle information."""
api.update(updatePictures=False)
"""API call to update vehicle information.

This function is called on its own thread and it is possible for multiple
threads to call it at the same time, before an earlier weconnect update()
call has finished. When the integration is loaded, multiple platforms
(binary_sensor, number...) may each call async_config_entry_first_refresh()
that in turn calls hass.async_add_executor_job() that creates a thread.
"""
# pylint: disable=global-statement
global _last_successful_api_update_timestamp, _last_we_connect_api

# Acquire a lock so that only one thread can call api.update() at a time.
with _update_lock:
# Skip the update() call altogether if it was last succesfully called
# in the past 24 seconds (80% of the minimum update interval of 30s).
elapsed = time.monotonic() - _last_successful_api_update_timestamp
if elapsed <= 24 and api is _last_we_connect_api:
return
api.update(updatePictures=False)
_last_successful_api_update_timestamp = time.monotonic()
_last_we_connect_api = api


def start_stop_charging(
call_data_vin, api: weconnect.WeConnect, operation: str
Expand Down Expand Up @@ -331,6 +367,9 @@ async def async_unload_entry(hass: HomeAssistant, entry: ConfigEntry) -> bool:
unload_ok = await hass.config_entries.async_unload_platforms(entry, PLATFORMS)
if unload_ok:
hass.data[DOMAIN].pop(entry.entry_id)
get_we_connect_api.cache_clear()
global _last_we_connect_api # pylint: disable=global-statement
_last_we_connect_api = None

return unload_ok

Expand Down
12 changes: 2 additions & 10 deletions custom_components/volkswagen_we_connect_id/config_flow.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,14 +5,14 @@
from typing import Any

import voluptuous as vol
from weconnect import weconnect
from weconnect.errors import AuthentificationError

from homeassistant import config_entries
from homeassistant.core import HomeAssistant
from homeassistant.data_entry_flow import FlowResult
from homeassistant.exceptions import HomeAssistantError

from . import get_we_connect_api, update
from .const import DOMAIN, DEFAULT_UPDATE_INTERVAL_SECONDS, MINIMUM_UPDATE_INTERVAL_SECONDS

_LOGGER = logging.getLogger(__name__)
Expand All @@ -31,12 +31,9 @@
async def validate_input(hass: HomeAssistant, data: dict[str, Any]) -> dict[str, Any]:
"""Validate the user input allows us to connect."""

we_connect = weconnect.WeConnect(
we_connect = get_we_connect_api(
username=data["username"],
password=data["password"],
updateAfterLogin=False,
loginOnInit=False,
updatePictures=False,
)

await hass.async_add_executor_job(we_connect.login)
Expand All @@ -46,11 +43,6 @@ async def validate_input(hass: HomeAssistant, data: dict[str, Any]) -> dict[str,

return {"title": "Volkswagen We Connect ID"}

def update(
api: weconnect.WeConnect
) -> None:
"""API call to update vehicle information."""
api.update(updatePictures=False)

class ConfigFlow(config_entries.ConfigFlow, domain=DOMAIN):
"""Handle a config flow for Volkswagen We Connect ID."""
Expand Down