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

Conversation

pdcastro
Copy link
Contributor

@pdcastro pdcastro commented Feb 9, 2024

Thanks for creating and maintaining this integration! 👍

I have observed that it is the last integration to finish loading whenever I (re)start my Home Assistant instance. I have found that, when the integration starts, it makes 6 calls to the WeConnect API update() method nearly at once — the same api.update() call that is normally made every 45 seconds. Each such call results in 6 HTTP GET requests to the following VW/Cariad endpoints:

  1. https://emea.bff.cariad.digital/vehicle/v1/vehicles
  2. https://emea.bff.cariad.digital/vehicle/v1/vehicles/<VIN>/selectivestatus?jobs=access,activeventilation,automation,auxiliaryheating,userCapabilities,charging,chargingProfiles,batteryChargingCare,climatisation,climatisationTimers,departureTimers,fuelStatus,vehicleLights,lvBattery,readiness,vehicleHealthInspection,vehicleHealthWarnings,oilLevel,measurements,batterySupport,trips
  3. https://emea.bff.cariad.digital/vehicle/v1/vehicles/<VIN>/parkingposition
  4. https://emea.bff.cariad.digital/vehicle/v1/trips/<VIN>/shortterm/last
  5. https://emea.bff.cariad.digital/vehicle/v1/trips/<VIN>/longterm/last
  6. https://emea.bff.cariad.digital/vehicle/v1/trips/<VIN>/cyclic/last

So 6 × 6 = 36 authenticated API endpoint requests within a few seconds from the integration being loaded (in my logs I observed all HTTP requests being started within 3 seconds of the integration loading). In addition, when the config flow executes during installation, an extra api.update() call is made for username and password validation, in which case it is 7 × 6 = 42 authenticated API endpoint requests just a few seconds apart.

2 of the 6 or 7 update() calls are explicit, and the others happen as a result of calls to coordinator.async_config_entry_first_refresh():

  1. __init__.py#L49
  2. __init__.py#L79
  3. binary_sensor.py#L139
  4. device_tracker.py#L29
  5. number.py#L29
  6. sensor.py#L448
  7. config_flow.py#L41

I have verified this finding by adding debug logging at the relevant WeConnect fetchData session.get() call.

A single WeConnect API update() call would be sufficient at start up. This PR proposes a relatively simple solution to achieve this, even in the config flow scenario when the integration is installed.

I tested my solution with HASS version 2024.1.6, including the case of the user entering incorrect username and password in the config flow. I have also exercised the code added to async_unload_entry() by deleting and re-adding the integration.

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.

@michaelarnauts
Copy link
Contributor

This would explain why I always get blocked for a few hours when I restart Home Assistant a few times!

@northalpha
Copy link
Contributor

hey @mitch-dc may i ask you to have a look at this PR in particular (as well as all other PR from @pdcastro) they greatly enhance the overall performance, reliability (read no api rate limit hitting anymore) and usage of your awesome integration

@mitch-dc
Copy link
Owner

If you fix the conflict i will merge your change! :)

@pdcastro
Copy link
Contributor Author

If you fix the conflict i will merge your change! :)

Done! 👍

@mitch-dc mitch-dc merged commit c7f95c4 into mitch-dc:main Feb 20, 2024
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.

4 participants