Add quality scale file to vesync integration by cdnninja · Pull Request #156663 · home-assistant/core

cdnninja · 2025-11-15T17:23:33Z

Proposed change

Adding the quality score file and initial evaluation.

Type of change

Dependency upgrade
Bugfix (non-breaking change which fixes an issue)
New integration (thank you!)
New feature (which adds functionality to an existing integration)
Deprecation (breaking change to happen in the future)
Breaking change (fix/feature causing existing functionality to break)
Code quality improvements to existing code or addition of tests

Additional information

This PR fixes or closes issue: fixes #
This PR is related to issue:
Link to documentation pull request:
Link to developer documentation pull request:
Link to frontend pull request:

Checklist

I understand the code I am submitting and can explain how it works.
The code change is tested and works locally.
Local tests pass. Your PR cannot be merged unless tests pass
There is no commented out code in this PR.
I have followed the development checklist
I have followed the perfect PR recommendations
The code has been formatted using Ruff (ruff format homeassistant tests)
Tests have been added to verify that the new code works.
Any generated code has been carefully reviewed for correctness and compliance with project standards.

If user exposed functionality or configuration variables are added/changed:

Documentation added/updated for www.home-assistant.io

If the code communicates with devices, web services, or third-party tools:

The manifest file has all fields filled out correctly.
Updated and included derived files by running: python3 -m script.hassfest.
New or updated dependencies have been added to requirements_all.txt.
Updated by running python3 -m script.gen_requirements_all.
For the updated dependencies - a link to the changelog, or at minimum a diff between library versions is added to the PR description.

To help with the load of incoming pull requests:

I have reviewed two other open pull requests in this repository.

home-assistant · 2025-11-15T17:23:42Z

Hey there @markperdue, @webdjoe, @TheGardenMonkey, @iprak, @SapuSeven, mind taking a look at this pull request as it has been labeled with an integration (vesync) you are listed as a code owner for? Thanks!

Code owner commands

Code owners of vesync can trigger bot actions by commenting:

@home-assistant close Closes the pull request.
@home-assistant rename Awesome new title Renames the pull request.
@home-assistant reopen Reopen the pull request.
@home-assistant unassign vesync Removes the current integration label and assignees on the pull request, add the integration domain after the command.
@home-assistant add-label needs-more-information Add a label (needs-more-information, problem in dependency, problem in custom component) to the pull request.
@home-assistant remove-label needs-more-information Remove a label (needs-more-information, problem in dependency, problem in custom component) on the pull request.

cdnninja · 2025-11-15T17:24:11Z

Creating as draft to show the progress. I will spend more time to run through the blank items. Once that is done will set to ready for review and start adding PRs for to do items.

cdnninja · 2025-11-15T19:08:53Z

I have reviewed the items. A few items I put as todo for future levels as needs more code review to confirm. As well docs while in okay shape I think need updates.

joostlek

Let's put the things we agree on fixing in the comments in the file and merge it and improve it from there

joostlek · 2025-11-27T16:30:52Z

+  icon-translations: todo
+  reconfiguration-flow: todo
+  repair-issues: done
+  stale-devices: todo


You did implement async_remove_config_entry_device though, which also makes me wonder, when a device is offline, is it removed from the API? Or can we safely assume when a device is actually gone? Because in the latter I would say that we can just remove the device automatically and remove the manual method to do so

Yes, from reading the rule my understanding is auto remove if possible. Two APIs exist in vesync. One gets a list of registered devices, another gets the status of a device. The code I added will update the device list, and compare. If missing it removes it. If offline it will still be in that list. So I listed this as to-do as I automate that on a schedule. I only want to call it from time to time though as it uses API calls. Say once a day or hour, when a device is removed it would auto remove from HA.

joostlek · 2025-11-27T16:32:35Z

@@ -0,0 +1,66 @@
+rules:


Would it make sense to at some point split the energy from the normal device coordinator?

To clarify would this be a second coordinator?

joostlek · 2025-11-27T17:17:14Z

@@ -0,0 +1,66 @@
+rules:


_attr_available_modes can be used in the humidifer

I am actually not sure why we collect a separate map here, as in, there are no duplicates so I think we can just create a global humdifier mode map instead of an entity specific one

Are you able to point me to where you see that? I only see a single humidifier mode map. A different one exists for fan though.

# Populate maps once. for vs_mode in self.device.mist_modes: ha_mode = _get_ha_mode(vs_mode) if ha_mode: self._available_modes.append(ha_mode) self._ha_to_vs_mode_map[ha_mode] = vs_mode self._available_modes.sort()

More like, instead of using the generic map, we build a new map based on the existing values. I only think there's a benefit for this if you have humidifier modes that map to the same thing (so let's say device A has mode X and device B has mode Y and they're mapped to the same value in HA, then this would make sense as this would make sure that we don't ask for mode Y on device A)

joostlek · 2025-11-27T17:18:37Z

@@ -0,0 +1,66 @@
+rules:


Why does the display turn off when we move to sleep mode?

This matches the behavior of the native app. We could decouple if preferred.

Ideally we do that, the less abstractions we have on the HA side the better (as in, you might have someone who doesn't want that to happen and then you'd have to add a configuration, and that can become a slippery slope)

joostlek · 2025-11-27T17:21:03Z

@@ -0,0 +1,66 @@
+rules:


Is getting a non-int light brightness usual?

Not sure! I believe that was carry over so haven't touched it recently. How would we track this in the quality score?

No real place

joostlek · 2025-11-27T17:25:17Z

@@ -0,0 +1,66 @@
+rules:


I think we need to get rid of the is_<type> methods at some point because they use isinstance and that's relatively expensive

This was an effort to start moving away from rgettattr. I either need to safely detect if the data exists, or confirm the class where they exist is used. I can think on this and see if the is_supported would work but I don't think that applies across all classes.

home-assistant · 2025-11-27T17:30:17Z

Please take a look at the requested changes, and use the Ready for review button when you are done, thanks 👍

Learn more about our pull request process.

joostlek · 2025-12-29T13:11:52Z

@@ -0,0 +1,66 @@
+rules:


There's no place for that or a rule to explicitly deny it, but I would consider this a thing that makes the integration of good quality. You could put it under strict-typing

joostlek · 2025-12-29T13:14:13Z

@@ -0,0 +1,66 @@
+rules:


# Populate maps once. for vs_mode in self.device.mist_modes: ha_mode = _get_ha_mode(vs_mode) if ha_mode: self._available_modes.append(ha_mode) self._ha_to_vs_mode_map[ha_mode] = vs_mode self._available_modes.sort()

More like, instead of using the generic map, we build a new map based on the existing values. I only think there's a benefit for this if you have humidifier modes that map to the same thing (so let's say device A has mode X and device B has mode Y and they're mapped to the same value in HA, then this would make sense as this would make sure that we don't ask for mode Y on device A)

joostlek · 2025-12-29T13:15:15Z

@@ -0,0 +1,66 @@
+rules:


Ideally we do that, the less abstractions we have on the HA side the better (as in, you might have someone who doesn't want that to happen and then you'd have to add a configuration, and that can become a slippery slope)

joostlek · 2025-12-29T13:15:34Z

@@ -0,0 +1,66 @@
+rules:


No real place

Base quality Scale

6935431

home-assistant Bot added cla-signed code-quality integration: vesync quality-scale by-code-owner Quality Scale: No score labels Nov 15, 2025

cdnninja added 4 commits November 15, 2025 17:42

All but docs

bbee9ae

fix indent

281eb5c

events

3388392

FInal review

27b7835

cdnninja marked this pull request as ready for review November 15, 2025 19:08

cdnninja requested a review from a team as a code owner November 15, 2025 19:08

Switch discovery to do

cdbc63b

joostlek requested changes Nov 27, 2025

View reviewed changes

home-assistant Bot marked this pull request as draft November 27, 2025 17:30

Change scores

ce8d100

cdnninja marked this pull request as ready for review November 28, 2025 03:20

home-assistant Bot requested a review from joostlek November 28, 2025 03:20

cdnninja mentioned this pull request Dec 3, 2025

Adjust vesync to follow action-setup #157795

Merged

21 tasks

Update config-flow-test-coverage status to done

b178bcb

This was referenced Dec 3, 2025

VeSync strict-typing #157797

Draft

Improve VeSync startup error handling #158126

Merged

Update Vesync Docs to meet Bronze Standard home-assistant/home-assistant.io#42361

Merged

cdnninja added 2 commits December 6, 2025 11:22

Update installation instructions status to done

85fc062

Update docs bronze items

a9619c8

Action setup is now complete

c2efcec

cdnninja mentioned this pull request Dec 24, 2025

Use runtime_data in VeSync #159720

Merged

21 tasks

test-before-setup complete

1087d95

joostlek approved these changes Dec 29, 2025

View reviewed changes

joostlek added 3 commits December 29, 2025 14:40

Fix

eddbae7

Merge branch 'dev' into qualityScoreVesync

b31ddda

Fix

f6e91b3

joostlek merged commit 8c9a6cc into home-assistant:dev Dec 29, 2025
35 of 36 checks passed

github-actions Bot locked and limited conversation to collaborators Dec 30, 2025

Uh oh!

Conversation

cdnninja commented Nov 15, 2025

Proposed change

Type of change

Additional information

Checklist

Uh oh!

home-assistant Bot commented Nov 15, 2025

Uh oh!

cdnninja commented Nov 15, 2025

Uh oh!

cdnninja commented Nov 15, 2025

Uh oh!

joostlek left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

cdnninja Nov 28, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

home-assistant Bot commented Nov 27, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

cdnninja Nov 28, 2025 •

edited

Loading