Add support for VeSync Fans #36132

TheGardenMonkey · 2020-05-25T18:26:09Z

Proposed change

Adds support to the VeSync component to include "fans" that are supported by the VeSync App / eco-system. Currently this includes the Leviot Smart Air Purifier LV-PUR131S. The added code is essentially a duplication of what is done in the current switch component.

Type of change

Dependency upgrade
Bugfix (non-breaking change which fixes an issue)
New integration (thank you!)
[ x ] New feature (which adds functionality to an existing integration)
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:
Add support for VeSync Fans home-assistant.io#13590

Checklist

[ x ] The code change is tested and works locally.
[ x ] Local tests pass. Your PR cannot be merged unless tests pass
[ x ] There is no commented out code in this PR.
[ x ] I have followed the development checklist
[ x ] The code has been formatted using Black (black --fast homeassistant tests)
Tests have been added to verify that the new code works.

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:

[ x ] 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.
Untested files have been added to .coveragerc.

The integration reached or maintains the following Integration Quality Scale:

No score or internal
🥈 Silver
🥇 Gold
🏆 Platinum

TheGardenMonkey · 2020-05-25T18:44:03Z

Created documentation pull request to accompany this

home-assistant/home-assistant.io#13590

SirDeadlystrike · 2020-06-01T05:24:43Z

Looks Good! Thanks for adding fan support from VeSync. Was looking into this as well recently.

Tests are failing, Flake8 is complaining. Need to fix that.

If you need help testing let me know!

TheGardenMonkey · 2020-06-01T21:41:24Z

Looks Good! Thanks for adding fan support from VeSync. Was looking into this as well recently.

Tests are failing, Flake8 is complaining. Need to fix that.

If you need help testing let me know!

thanks, I don't understand why it is failing. I believe what I have is correct, so not clear what is wrong...it is complaining about the "mood" on the first line but the language matches other integrations.

homeassistant/components/vesync/fan.py

SirDeadlystrike · 2020-06-02T00:41:05Z

Flake8 passed now! Nice!

iSort is blowing up now. I think you need to install and run the command it references local to see the full verbose reason/error. If you are seeing this message in CI, reproduce locally with: pre-commit run --all-files.

TheGardenMonkey · 2020-06-02T03:21:50Z

Should I just move the turn_on from common into the switch.py?

theaquarium · 2020-06-16T22:02:09Z

Thanks for adding this! I was just about to add it myself but I noticed that this PR already existed. I'd love to see this merged soon.

theaquarium · 2020-06-16T22:10:35Z

By the way, this integration should probably be renamed to just VeSync since it no longer only supports Etekcity VeSync products.

TheGardenMonkey · 2020-06-17T15:37:10Z

Thanks for the feedback, I was wondering if that was OK to do as part of this pull request. I have removed from here and from the docs pull request.

stale · 2020-07-18T06:27:14Z

There hasn't been any activity on this pull request recently. This pull request has been automatically marked as stale because of that and will be closed if no further activity occurs within 7 days.
Thank you for your contributions.

TheGardenMonkey · 2020-07-21T18:42:00Z

Adding a comment to prevent from closing.

bdraco

Thank you for your contribution thus far! 🎖 Since this is a significant contribution, we would appreciate you'd added yourself to the list of code owners for this integration. ❤️

Please, add your GitHub username to the manifest.json of this integration.

For more information about "code owners", see: Architecture Decision Record 0008: Code owners.

homeassistant/components/vesync/fan.py

homeassistant/components/vesync/__init__.py

Co-authored-by: J. Nick Koston <nick@koston.org>

homeassistant/components/vesync/fan.py

homeassistant/components/vesync/switch.py

homeassistant/components/vesync/__init__.py

bdraco

Looks good, just a few small comments.

bdraco · 2020-08-31T18:53:21Z

When its ready again for me to take a look, please request a review

bdraco · 2020-09-03T14:31:48Z

homeassistant/components/vesync/fan.py

+    def speed(self):
+        """Return the current speed."""
+        if self.smartfan.mode == SPEED_AUTO:
+            return SPEED_AUTO


Since the fan can still return SPEED_AUTO, and we have taken it out of FAN_SPEEDS this puts us in an invalid state. Unfortunately it doesn't look like we can tell what speed the fan is actually running it so it leaves us in a position where violating the spec might make more sense since auto can be set from outside Home Assistant.

I think its ok to add back in as long as we refactor after home-assistant/architecture#127 resolves this.

Sounds good and yes I will refactor when a decision is made.

It's not ok to violate our architecture design until the architecture has been changed. Please remove the auto speed.

We can return None in that case.

homeassistant/components/vesync/fan.py

MartinHjelmare

Please address the comments in a new PR. Thanks!

MartinHjelmare · 2020-09-04T05:59:18Z

homeassistant/components/vesync/fan.py

+async def async_setup_entry(hass, config_entry, async_add_entities):
+    """Set up the VeSync fan platform."""
+
+    async def async_discover(devices):


async_discover could be a callback since we don't await inside.

MartinHjelmare · 2020-09-04T05:59:36Z

homeassistant/components/vesync/fan.py

+    hass.data[DOMAIN][VS_DISPATCHERS].append(disp)
+
+    _async_setup_entities(hass.data[DOMAIN][VS_FANS], async_add_entities)
+    return True


Nothing is checking this return value.

MartinHjelmare · 2020-09-04T06:08:45Z

homeassistant/components/vesync/fan.py

+        if not self.smartfan.is_on:
+            self.smartfan.turn_on()
+
+        if speed is None or speed == SPEED_AUTO:


Remove this case.

MartinHjelmare · 2020-09-04T06:09:23Z

homeassistant/components/vesync/fan.py

+            "mode": self.smartfan.mode,
+            "active_time": self.smartfan.active_time,
+            "filter_life": self.smartfan.filter_life,
+            "air_quality": self.smartfan.air_quality,


Air quality should be a separate sensor entity if it means a measurement of air quality and not a fan setting.

The 'fan' reports an air quality which ranges between bad to excellent, so just want to clarify if that makes sense and is still OK to keep as a sensor. I am assuming yes.

It makes sense to make it a sensor in this case 👍

Co-authored-by: J. Nick Koston <nick@koston.org>

Adds support for VeSync Fan - LV-PUR131S

79cbada

homeassistant added the cla-signed label May 25, 2020

probot-home-assistant bot added integration: vesync new-feature new-platform labels May 25, 2020

Adds missing docstring data

9dacf26

TheGardenMonkey mentioned this pull request May 25, 2020

Add support for VeSync Fans home-assistant/home-assistant.io#13590

Merged

8 tasks

SirDeadlystrike reviewed Jun 1, 2020

View reviewed changes

homeassistant/components/vesync/fan.py Outdated Show resolved Hide resolved

Corrects docstring

24b0cba

TheGardenMonkey added 3 commits June 2, 2020 21:36

Runs through isort and updates turn_on signature

297656c

Moves turn_on to switch from common

1df6daa

Updates coverage file for vesync fan

f08e003

Removes Etekcity from integration name

1dce26f

stale bot added the stale label Jul 18, 2020

stale bot removed the stale label Jul 21, 2020

bdraco requested changes Aug 22, 2020

View reviewed changes

TheGardenMonkey and others added 2 commits August 22, 2020 20:34

Update homeassistant/components/vesync/fan.py

a760325

Co-authored-by: J. Nick Koston <nick@koston.org>

Adds self to codeowners

ac77241

TheGardenMonkey added 5 commits August 22, 2020 21:14

Uses async unload

5dbf5ec

Uses constants for FAN_SPEEDS

c982180

Updates CODEOWNERS to add self

f3123b1

Adds missing imports

cebf990

Adds auto to the FAN_SPEEDS array

a807b25

TheGardenMonkey requested a review from bdraco August 28, 2020 01:00

bdraco reviewed Aug 29, 2020

View reviewed changes

homeassistant/components/vesync/fan.py Show resolved Hide resolved

bdraco reviewed Aug 29, 2020

View reviewed changes

homeassistant/components/vesync/switch.py Outdated Show resolved Hide resolved

bdraco reviewed Aug 29, 2020

View reviewed changes

homeassistant/components/vesync/__init__.py Show resolved Hide resolved

bdraco requested changes Aug 29, 2020

View reviewed changes

Removes Auto and creates base switch class

33f56fb

TheGardenMonkey requested a review from bdraco August 31, 2020 19:24

bdraco reviewed Sep 3, 2020

View reviewed changes

Adds Auto back to the speed list

9d0ca91

TheGardenMonkey requested a review from bdraco September 4, 2020 01:47

bdraco reviewed Sep 4, 2020

View reviewed changes

homeassistant/components/vesync/fan.py Show resolved Hide resolved

bdraco approved these changes Sep 4, 2020

View reviewed changes

bdraco merged commit eb7742e into home-assistant:dev Sep 4, 2020

MartinHjelmare reviewed Sep 4, 2020

View reviewed changes

leikoilja pushed a commit to leikoilja/core that referenced this pull request Sep 6, 2020

Add support for VeSync Fans (home-assistant#36132)

dcc4ea2

Co-authored-by: J. Nick Koston <nick@koston.org>

MartinHjelmare mentioned this pull request Sep 24, 2020

Remove auto from the fan speed modes for VeSync #40559

Merged

21 tasks

bdraco mentioned this pull request Jan 22, 2021

Separate fan speeds into percentages and presets modes #45407

Merged

23 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add support for VeSync Fans #36132

Add support for VeSync Fans #36132

TheGardenMonkey commented May 25, 2020 •

edited

Loading

TheGardenMonkey commented May 25, 2020

SirDeadlystrike commented Jun 1, 2020

TheGardenMonkey commented Jun 1, 2020

SirDeadlystrike commented Jun 2, 2020

TheGardenMonkey commented Jun 2, 2020

theaquarium commented Jun 16, 2020

theaquarium commented Jun 16, 2020

TheGardenMonkey commented Jun 17, 2020

stale bot commented Jul 18, 2020

TheGardenMonkey commented Jul 21, 2020

bdraco left a comment

bdraco left a comment

bdraco commented Aug 31, 2020

bdraco Sep 3, 2020

TheGardenMonkey Sep 4, 2020

MartinHjelmare Sep 4, 2020 •

edited

Loading

MartinHjelmare left a comment

MartinHjelmare Sep 4, 2020 •

edited

Loading

MartinHjelmare Sep 4, 2020

MartinHjelmare Sep 4, 2020

MartinHjelmare Sep 4, 2020

TheGardenMonkey Sep 4, 2020

bdraco Sep 4, 2020

Add support for VeSync Fans #36132

Add support for VeSync Fans #36132

Conversation

TheGardenMonkey commented May 25, 2020 • edited Loading

Proposed change

Type of change

Additional information

Checklist

TheGardenMonkey commented May 25, 2020

SirDeadlystrike commented Jun 1, 2020

TheGardenMonkey commented Jun 1, 2020

SirDeadlystrike commented Jun 2, 2020

TheGardenMonkey commented Jun 2, 2020

theaquarium commented Jun 16, 2020

theaquarium commented Jun 16, 2020

TheGardenMonkey commented Jun 17, 2020

stale bot commented Jul 18, 2020

TheGardenMonkey commented Jul 21, 2020

bdraco left a comment

Choose a reason for hiding this comment

bdraco left a comment

Choose a reason for hiding this comment

bdraco commented Aug 31, 2020

bdraco Sep 3, 2020

Choose a reason for hiding this comment

TheGardenMonkey Sep 4, 2020

Choose a reason for hiding this comment

MartinHjelmare Sep 4, 2020 • edited Loading

Choose a reason for hiding this comment

MartinHjelmare left a comment

Choose a reason for hiding this comment

MartinHjelmare Sep 4, 2020 • edited Loading

Choose a reason for hiding this comment

MartinHjelmare Sep 4, 2020

Choose a reason for hiding this comment

MartinHjelmare Sep 4, 2020

Choose a reason for hiding this comment

MartinHjelmare Sep 4, 2020

Choose a reason for hiding this comment

TheGardenMonkey Sep 4, 2020

Choose a reason for hiding this comment

bdraco Sep 4, 2020

Choose a reason for hiding this comment

TheGardenMonkey commented May 25, 2020 •

edited

Loading

MartinHjelmare Sep 4, 2020 •

edited

Loading

MartinHjelmare Sep 4, 2020 •

edited

Loading