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

Add seperate setup flow to coordinator #447

Merged
merged 9 commits into from
Jan 4, 2025

Conversation

WebSpider
Copy link
Contributor

@WebSpider WebSpider commented Dec 13, 2024

This alters the coordinator setup as follows:

  • When doing a data update (poll), we make a distinction between a regular update and one being made while the MySkoda integration is being setup
  • During setup, we request only the minimal amount of data required to start. We do not fetch all sensor data anymore.
  • During setup, we tell HA to schedule the MQTT connection for when we are done setting up
  • During regular polling, we check if MQTT connection is still present, and try to reconnect if needed

This should allow the integration to succesfully set-up, even if MQTT is unavailable, and the REST API is being wonky instead of blocking HA startup or failing to initialise.

- Seperate flow for setup phase
- Schedule MQTT connection only after integration has initialized
- Wrap MQTT startup in try/except for proper unlocking
@WebSpider WebSpider added the bugfix This PR contains a bugfix label Dec 13, 2024
@WebSpider WebSpider marked this pull request as draft December 13, 2024 10:30
@WebSpider
Copy link
Contributor Author

Converted to draft since this probably needs a different approach

@dvx76
Copy link
Member

dvx76 commented Dec 23, 2024

Why does this need a different approach?

I'm trying to understand how the coordinator is involved during the setup (ConfigFlow?) i.e. how does _async_update_data end up being called during the setup?

@dvx76 dvx76 mentioned this pull request Dec 25, 2024
@WebSpider
Copy link
Contributor Author

Why does this need a different approach?

The approach that is currently in this draft is mostly incomplete and has corner cases for multiple vehicles, integration reloading

I'm trying to understand how the coordinator is involved during the setup (ConfigFlow?) i.e. how does _async_update_data end up being called during the setup?

ConfigFlow is the configuration of the integration. Once the integration is configure, it can be setup. Those are two different things.

In setup, async_setup is called in __init__.py, which instantiates and inits the coordinator(s).

Have a look at the coordinators parent class, they are partially overriden by us only

@WebSpider
Copy link
Contributor Author

My idea is to only request data during first setup of coordinator that is needed for the actual initialisation, and then do a normal data pull.

This should hopefully make integration setup a lot more robust

@WebSpider
Copy link
Contributor Author

Fixes after running some tests while MQTT is down

prvakt
prvakt previously requested changes Jan 3, 2025
Copy link
Collaborator

@prvakt prvakt left a comment

Choose a reason for hiding this comment

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

lgtm, just one minor change

custom_components/myskoda/coordinator.py Outdated Show resolved Hide resolved
@WebSpider
Copy link
Contributor Author

Good catch, will update the PR

@WebSpider WebSpider merged commit 4cec5c8 into skodaconnect:main Jan 4, 2025
3 checks passed
@WebSpider WebSpider deleted the fix-mqtt-locking-init branch January 4, 2025 01:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bugfix This PR contains a bugfix
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants