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

Disconnect velux on hass stop #26266

Merged
merged 15 commits into from Sep 17, 2019

Conversation

@gibman
Copy link
Contributor

commented Aug 29, 2019

Description:

Often when rebooting hass the KLF 200 device would not disconnect the previous connection which in turns locks out the use from connecting again.
In these situations one would have to powercycle the klf200 velux device, which could be a problem if not being at home.

Ive set up af listener for EVENT_HOMEASSISTANT_STOP, which would jsutm ake sure to call disconnect on the device.

more info here:
https://community.home-assistant.io/t/velux-component-for-klf-200-doesnt-support-the-new-api-with-firmware-2-0-0-71/75641/67

Pull request with documentation for home-assistant.io (if applicable): home-assistant/home-assistant.io#<home-assistant.io PR number goes here>

Checklist:

  • The code change is tested and works locally.
  • Local tests pass with tox. Your PR cannot be merged unless tests pass
  • There is no commented out code in this PR.
  • I have followed the development checklist

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

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

  • The manifest file has all fields filled out correctly. Update and include derived files by running python3 -m script.hassfest.
  • New or updated dependencies have been added to requirements_all.txt by running python3 -m script.gen_requirements_all.
  • Untested files have been added to .coveragerc.

If the code does not interact with devices:

  • Tests have been added to verify that the new code works.
gibman added 2 commits Aug 29, 2019
…ss device.

disconnect is now being called on the 'EVENT_HOMEASSISTANT_STOP' event
gibman added 5 commits Aug 29, 2019
@gibman

This comment has been minimized.

Copy link
Contributor Author

commented Aug 29, 2019

So Ive added the "unknown" email to my account .. how do I retrigger the bot in order to re-evaluate?
I tried doing an empty commit along with a dummy comment.
Doesnt seem to cut it...

sigh..

@balloob balloob changed the base branch from master to dev Aug 30, 2019
@home-assistant home-assistant deleted a comment from homeassistant Aug 31, 2019
homeassistant/components/velux/__init__.py Outdated Show resolved Hide resolved
homeassistant/components/velux/__init__.py Outdated Show resolved Hide resolved
Dev automation moved this from Needs review to Review in progress Aug 31, 2019
@MartinHjelmare MartinHjelmare changed the title velux KLF200 device did not disconnect properly when rebooting hass. Disconnect velux on hass stop Aug 31, 2019
homeassistant/components/velux/__init__.py Outdated Show resolved Hide resolved
homeassistant/components/velux/__init__.py Outdated Show resolved Hide resolved
homeassistant/components/velux/__init__.py Outdated Show resolved Hide resolved
homeassistant/components/velux/__init__.py Outdated Show resolved Hide resolved
homeassistant/components/velux/__init__.py Outdated Show resolved Hide resolved
homeassistant/components/velux/__init__.py Outdated Show resolved Hide resolved
homeassistant/components/velux/__init__.py Outdated Show resolved Hide resolved
logger level moved from info to debug
only config[DOMAIN] exposed to module
imports moved to top
Dev automation moved this from Review in progress to Reviewer approved Sep 6, 2019
Copy link
Member

left a comment

Good! Can be merged when lint and formatting is fixed and build passes.

@gibman

This comment has been minimized.

Copy link
Contributor Author

commented Sep 6, 2019

Good! Can be merged when lint and formatting is fixed and build passes.

No idea what to fix here ??
Not too many hints revealed in those error messages :/

@ppetru

This comment has been minimized.

Copy link

commented Sep 13, 2019

@gibman thanks for the PR, looking forward to having it in a release since it's quite annoying to keep powercycling my KLF 200 :)

I'm not too familiar with the automated checks involved here, but clicking through the details links brought me to this: https://dev.azure.com/home-assistant/Home%20Assistant/_build/results?buildId=8149&view=logs&jobId=2fa2c639-d05e-530d-034e-4fb413fcea4e&j=4e214fed-f310-53fa-0018-b38e7beb405b -- if you expand the "Run flake8" section you get a bunch of lint errors:

homeassistant/components/velux/init.py:27:38: W291 trailing whitespace
homeassistant/components/velux/init.py:50:26: W291 trailing whitespace
homeassistant/components/velux/init.py:52:1: W293 blank line contains whitespace
homeassistant/components/velux/init.py:54:1: D102 Missing docstring in public method
homeassistant/components/velux/init.py:54:5: E303 too many blank lines (2)
homeassistant/components/velux/init.py:59:1: W293 blank line contains whitespace
homeassistant/components/velux/init.py:60:81: W291 trailing whitespace
homeassistant/components/velux/init.py:66:5: E303 too many blank lines (2)
homeassistant/components/velux/init.py:68:49: W291 trailing whitespace
homeassistant/components/velux/init.py:71:1: W391 blank line at end of file
homeassistant/components/velux/init.py:71:1: W293 blank line contains whitespace

This might also be useful for checking locally so that you don't have to wait for the CI robots to point out your errors: https://developers.home-assistant.io/docs/en/development_testing.html#preventing-linter-errors

@MartinHjelmare

This comment has been minimized.

Copy link
Member

commented Sep 13, 2019

Please run black from the project root and commit the changes, to solve formatting.

black --fast homeassistant
@gibman

This comment has been minimized.

Copy link
Contributor Author

commented Sep 13, 2019

Please run black from the project root and commit the changes, to solve formatting.

black --fast homeassistant

I think you assume that I know what black is ? :)
I'm merely running notepad++ on a windows box.
I did try to get rid of those annoying trailing whitespaces and the latest commit

regards
gibman

@ppetru

This comment has been minimized.

Copy link

commented Sep 13, 2019

gibman added 2 commits Sep 13, 2019
@MartinHjelmare

This comment has been minimized.

Copy link
Member

commented Sep 13, 2019

gibman added 2 commits Sep 16, 2019
@MartinHjelmare

This comment has been minimized.

Copy link
Member

commented Sep 17, 2019

Thanks!

@MartinHjelmare MartinHjelmare merged commit ed13cab into home-assistant:dev Sep 17, 2019
11 checks passed
11 checks passed
CI Build #20190916.8 succeeded
Details
CI (FullCheck Mypy) FullCheck Mypy succeeded
Details
CI (FullCheck Pylint) FullCheck Pylint succeeded
Details
CI (Overview CheckFormat) Overview CheckFormat succeeded
Details
CI (Overview Lint) Overview Lint succeeded
Details
CI (Overview Validate) Overview Validate succeeded
Details
CI (Tests PyTest Python36) Tests PyTest Python36 succeeded
Details
CI (Tests PyTest Python37) Tests PyTest Python37 succeeded
Details
cla-bot Everyone involved has signed the CLA
codecov/patch Coverage not affected.
Details
codecov/project 93.99% (target 90%)
Details
Dev automation moved this from Reviewer approved to Done Sep 17, 2019
@lock lock bot locked and limited conversation to collaborators Sep 18, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
Dev
  
Done
5 participants
You can’t perform that action at this time.