Skip to content

Conversation

@RobertD502
Copy link

@RobertD502 RobertD502 commented Jan 8, 2025

Alright this is a big one, so I'll break down the changes.

Translations

  • Added the new keys needed for the reworked config flow, reauth flow, and options flow.
  • I didn't realize that there were that many translation files, so, I ended up creating a script that added any keys/values that were present in the new en.json file, but missing from the other translation files. Afterwards, the key counts were validated between the en.json file and every other translation.
  • If, for a translation file, a key:value pair is a new addition, the English value was used with (Needs Translation) appended to make it easier for contributors to ctrl + F their way through while translating.

config_flow.py

  • The config flow version has been bumped to version 2 and pretty much completely reworked.
  • Data schema has been reworked to use Home Assistant's config validator.
  • Polling interval (in seconds) has been added to the config flow so that users can set a value during initial setup, but can also change it, if already setup, from within the integration page by clicking on the Configure button. For new config flows, a default prefilled suggestion of 60 seconds is used.
  • The single instance check has been removed as I don't see why users shouldn't be able to add multiple accounts - the integration creates a new VeSync instance during a config flow/within the update coordinator, so, there is no session sharing going on that would cause problems (like when reusing an aiohttp session from HA). However, a check still remains to prevent users from trying to add multiple configs with the same account.
  • Reauth flow has been added, which is triggered when the pyvesync login method fails - I wish the backend was more granular and raised an exception for a specific code/error that is encountered when wrong credentials are used, but we have to work with what we got.
  • Options Flow has been added to allow users that have the integration set up to change the polling interval via the Configure button in the UI from the VeSync integration page.

Example of new Config Flow

image

Example of Options Flow

vesyn_options_flow.mov

init.py

  • When the integration is loaded, if login fails, the ConfigEntryAuthFailed exception gets raised, which allows the user to enter the reauth flow.
  • The coordinator now uses the polling_interval that has been defined by the user - if the polling interval is ever changed by the user (via the options flow), the coordinator will immediately start using the new polling_interval that the user set.
  • The update listener is now stored as we need to have one to listen to any option changes (polling interval changes) which then reloads the config entry in order to use the new option value set.
  • Changed the coordinator to use the correct async_config_entry_first_refresh() method to fetch initial data.
  • The loaded_platforms list is now also stored in Home Assistant data as we need to use it when unloading a config entry (more on that below).
  • async_unload_entry has been reworked to use the stored loaded_platforms list. Previously, the PLATFORMS constant's dict keys were used to create a list, but that causes issues. If a user doesn't have at least 1 entity of each type of platform defined in the PLATFORMS dict, an error is logged by Home Assistant during a config unload - aside from removing a config entry, a config unload is also done when an option has been changed in the options flow (polling interval) as part of the config reload. For example, I only have a humidifier that has no fan or button entities. When using the PLATFORMs dict keys, an error is raised about unloading the fan and button platforms since they were never loaded. By using the stored loaded_platforms list, HA will only try to unload platforms that are actually being used by a vesync config entry.
  • async_migrate_entry has been added. This will migrate the config entry for any user that set the integration up previously (who have a config entry version of 1) to the new config entry version 2, which adds a default polling_interval of 60 seconds to those users' config entries. Of course, with the new options flow added, they can change the polling interval to something else via the UI.

README

  • Added a HACS MY button for users to be able to open the repo directly in HACS without going through adding a custom repo, but, of course, adding the repo manually is still a valid option.
  • Added a new image of the Debug UI to replace the one that is missing.

Github Workflows

  • The linter workflow has all actions bumped to the latest version & the python-setup action now uses a .python-version file to specify the python version (3.13 at the moment to reflect the python version HA uses).
  • release.yml has been added, which will create a zip of custom_components/vesync as vesync.zip and, whenever you publish a release, add it as an asset to the release. By doing so, you can have a metric of how many times a release has been downloaded + the number of times downloaded stat will also show in HACS.

hacs.json

  • The render_readme key has been removed as it is no longer used (HACS now requires integrations to have a README file).
  • Keys have been added to let HACS know to look for a zip file and for what zip file to look for in your releases.
  • The homeassistant key has been added to specify a minimum HA version required (2024.12 in this case). HA introduced some changes to the options flow in 2024.12 and the newly added options flow follows those changes. By setting the min version of HA we can prevent users, with a lower version of HA, from trying to install a release of the integration that includes the option flow code. I could have the option flow use the older code and we not set the minimum HA version, but anyone on 2024.12 or greater will get deprecation warnings in their logs. It's a good idea to keep track of what HA code changes, that this integration ends up implementing, aren't backwards compatible and bump the version as it is needed.

Related issues

I know this is a lot, but I hope the rundown helped provide some insight into the code changes.

@ewelin34
Copy link

ewelin34 commented Jan 8, 2025

Wow! Great work!!

@RobertD502
Copy link
Author

@gdgib
I'll have some more improvements coming in a later PR:

  • I noticed that entities, which can be controlled (humidifier, switches, etc), revert back to the previous state until the coordinator returns new data on the next poll - the state will be written to Home Assistant if an action is successful so that the UI immediately reflects the new state (e.g., If you turn on a humidifier via HA, the humidifier should remain On in the HA UI and not immediately revert back to the previous off state.)
  • For my 300S, setting the mode of the humidifier entity to Auto fails....turns out the old code references the Home Assistant auto mode for both the vesync auto and humidity modes and it would end up sending humidity as the mode to vesync, which fails since it isn't a supported mode. Now, instead, I've added a check if Auto mode is selected to set the humidifier to auto mode if auto exists in the mist_modes else if humidity exists in the mist_modes, send that instead (similar to what the pyvesync library does).
  • I noticed that setting target humidity when the humidifier is off or in manual mode (Normal mode in Home Assistant), doesn't actually change the target humidity mode (isn't reflected on the VeSync app). A check will raise an exception to alert users if they attempt to set a target humidity when the humidifier is off or in manual (normal) mode.

@gdgib
Copy link
Collaborator

gdgib commented Jan 8, 2025

Excellent!! I'll go through this a bit to make sure I can follow it all in some detail, but I expect this to be the backbone of 1.4.0 to be released within a week.

Any sense of timeline on the other PR? The issues you noticed overlapped with ones I noticed, too, but as this isn't the only integration I'm working on right now it sounds like you'll beat me to the punch.

@RobertD502
Copy link
Author

RobertD502 commented Jan 8, 2025

I have all of my other integrations in a stable state (hopefully no breaking API changes in the near future), so I can probably shoot for within your week timeframe. Don't let that hold you off from dealing with this PR.

Edit:

@gdgib
Would you mind enabling discussions on the repo (or maybe I just don't see it on mobile)? There are a few entities I'd like to discuss before removing them. For example: After looking at the code, I saw that the Auto mode switch turns on Auto mode, but, when setting that switch to off, it serves to set the mode to manual mode with a mist level of 1. Not something that users would think would happen naturally. Also, seems to be kind of pointless since the mode can be controlled from the humidifier entity. The only thing I can think of is for HomeKit purposes since mode isn't exposed within HomeKit within the humidifier entity. However, in that case, it would make sense to put the mode into manual mode, when the switch is set to off, without adjusting the mist level.

@gdgib
Copy link
Collaborator

gdgib commented Jan 8, 2025

I'll review this in a few hours. Let me know if you want me to release 1.4.0 with or without the other PR. I'm not picky, and you have a few days to decide I figure.

And I'll get discussions turned on when I'm back at a computer myself.

@gdgib gdgib self-assigned this Jan 9, 2025
@gdgib gdgib added the main label Jan 9, 2025
@gdgib gdgib added this to the v1.4.0 milestone Jan 9, 2025
Copy link
Collaborator

@gdgib gdgib left a comment

Choose a reason for hiding this comment

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

Only one question, otherwise everything looks great.

@gdgib
Copy link
Collaborator

gdgib commented Jan 9, 2025

Oh, and @RobertD502 discussions are on.

@RobertD502
Copy link
Author

@gdgib Any chance of getting this pulled in? I'm hoping to finish up the next big PR tonight - having these changes merged in the repo will help make the next PR go smoothly.

Let me know if there are any other issues you wanted to discuss regarding this PR!

@gdgib gdgib merged commit cba97bd into haext:main Jan 14, 2025
3 checks passed
@gdgib
Copy link
Collaborator

gdgib commented Jan 14, 2025

Had some personal events this weekend that required my time. Thanks for explaining the change in the release process. I'd love to think about how to generalize some of that work so that we can add the same features to other integrations I maintain.

@gdgib
Copy link
Collaborator

gdgib commented Jan 14, 2025

FWIW, @RobertD502 you have an invitation to the team now.

@RobertD502
Copy link
Author

No problem at all. Completely understand personal time taking priority! I have essentially reworked every aspect of the integration for the next PR (still have to rework the current diagnostics.py file) and would love for you to take a look at it/take it for a spin and test it out. After having completed the work, I saw an issue opened about the superior6000, which will require adding some additional sensors and functionality (Levoit just haddddd to use a 1 or 0 for display status instead of keeping in line how they report for other devices).

I'll try to find some time to squeeze that in between a home reno and replacing fried components on my washer, dryer, mini split inverter, and ceiling fan control boards (Stupid snow/ice storm).

@RobertD502 RobertD502 deleted the rework_config_flow branch January 14, 2025 17:27
@gdgib
Copy link
Collaborator

gdgib commented Jan 14, 2025

I never thought I'd meet someone whose project list intimidated ME, but here you are.

I'm try to keep an eye on issues then, and leave the coding to you entirely for a while just to avoid creating merge conflicts later. If I don't hear from you in the next 2 weeks, I'll reach out to see how things are going.

In the meantime I'm testing these changes, and you have an invitation to the org so you'll get write access to the repo.

I'm trying to level up a few integrations right now, so my focus will be split. While I want to be involved more here, if you aren't hearing from me enough, use your judgement and write access as you see fit.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Update README and HACS [FR] Polling rate adjustment

3 participants