Add more tests/support for VeSync device variants #138176
Conversation
|
Hey there @markperdue, @webdjoe, @TheGardenMonkey, @cdnninja, @iprak, mind taking a look at this pull request as it has been labeled with an integration ( Code owner commandsCode owners of
|
This improves the two VeSync static constants, `DEV_TYPE_TO_HA` and `SKU_TO_BASE_DEVICE`, to make them built automatically based on the supported devices within pyvesync. This should reduce the amount of changes necessary in the future to support new devices and makes the integration slightly more resilient to new skus released for devices that already exist. Since switches and bulbs use multiple device types, we need to detect the supported device based on the features that are exposed.
This adds tests for support of the following VeSync devices: * Humidifier Classic 300s * Air Purifier Core 300s * Outlet ESW01-EU (10A Indoor Round Plug) * Outlet ESO15-TB (Outdoor Plug) These devices were all either supported by the VeSync plugin previously or enabled within the plugin with the changes made in the previous commits to pull supported devices from the pyvesync module. This renames the previous "Outlet" test entity to "Outlet 7A" to reflect the fact that it is no longer the only outlet being tested, but it is the only 7A outlet currently supported. A custom API response for the outdoor plug details needed to be added because it supports sub-devices for the two individual sockets it controls which requires a new flavor of details response in order for the pyvesync module to correctly parse the details.
kevin-brown
force-pushed
the
vesync/more-variants
branch
from
650888e to
6d1ba9a
Compare
|
Interesting to see this approach. the Is humidifier approach also is checking the library data to see if it is a humidifier. It is the new approach each platform is moving to with the intent to move away from those lookup tables. I am interested to hear @iprak s thoughts on this. Two other thoughts, for this PR to get merged in my experience it would be better to shrink it, most likely the tests should be a separate PR from any other changes. Smaller the PRs the faster they move. As for #128183 it should be resolved as of 2025.2 when humidifier was released. The person commenting they still get an error posted a screenshot showing line 79 of fan.py with a log message that was removed in 2025.2 so I don't think they updated. |
|
This automatically means that all devices of a type are supported. Are we actually doing that verification? |
|
Documentation changes need to be included as well. |
I do think that is our goal as a whole. The is_humidifer approach does the same all inclusive approach as the two class definitions cover all humidifiers. My on concern of using this approach is it needs to be unique approach for each device type, vs the class approach will always exist. is_type also reads easier. |
According to the documentation for each device type (pyvesync readme), yes there is a stable API that we are making use of here so it should be safe to do.
Generally speaking I'm in favor of this. Switches, outlets, and bulbs all extend from a common type class within pyvesync so that should be fairly easy to work with. Unfortunately fans (and by extension, humidifiers) do not which makes that a moving target until pyvesync moves to a common base class for those as well. |
I think it all depends on how we can confirm that the unified class or separate classes perform in contrast to the device. A unified base class for humidifier would be neat. When I first looked at the device fixture, they felt to have been duplicated with invalid humidifier related data items. I am also in favor of the feature based approach since the library is the source of truth. I think the library can tell if a device is "humidifier" instead of maintaining 2 mappings |
The devices themselves provide most of this information for us already. As far as I can tell, |
|
#135744 does the alternate approach to remove these mapping tables. Only items left are the mode constants and speeds. I don't think speed ranges were in the library but could be added. |
Proposed change
The current VeSync plugin makes uses of long lists of hard-coded device SKUs and device types, which results in many issues and pull requests to support new devices after they are added to the underlying pyvesync library. This is further complicated by VeSync's love of adding new device SKUs for the same device, long after the original device was released.
In order to simplify handling within the Home Assistant integration for VeSync, this refactors the logic for detecting devices and SKUs to build the list based on the devices exposed by the pyvesync library. Additionally, this adds additional tests for newer VeSync devices that were previously supported by the integration but were not well-covered.
Type of change
Additional information
Checklist
ruff format homeassistant tests)If user exposed functionality or configuration variables are added/changed:
If the code communicates with devices, web services, or third-party tools:
Updated and included derived files by running:
python3 -m script.hassfest.requirements_all.txt.Updated by running
python3 -m script.gen_requirements_all.To help with the load of incoming pull requests: