Skip to content

Remove Velux KLF disconnection on HA stop event #47465

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.

Sign up for GitHub

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

Closed
pawlizio wants to merge 2 commits into from
Closed

Remove Velux KLF disconnection on HA stop event #47465

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
9505c8d
Remove disconnection on HA stop event
pawlizio Mar 5, 2021
94768de
Remove unused import
pawlizio Mar 5, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 1 addition & 7 deletions homeassistant/components/velux/__init__.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
from pyvlx import PyVLX, PyVLXException
import voluptuous as vol

from homeassistant.const import CONF_HOST, CONF_PASSWORD, EVENT_HOMEASSISTANT_STOP
from homeassistant.const import CONF_HOST, CONF_PASSWORD
from homeassistant.helpers import discovery
import homeassistant.helpers.config_validation as cv

Expand Down Expand Up @@ -53,15 +53,9 @@ def __init__(self, hass, domain_config):
def setup(self):
"""Velux component setup."""

async def on_hass_stop(event):
"""Close connection when hass stops."""
_LOGGER.debug("Velux interface terminated")
await self.pyvlx.disconnect()
Copy link
Member

@balloob balloob Mar 5, 2021

Choose a reason for hiding this comment

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

It seems weird that we would not cleanly disconnect. Why is an automation even necessary to reboot. Seems like we're now asking each used to solve the same problem

Copy link
Contributor Author

@pawlizio pawlizio Mar 5, 2021

Choose a reason for hiding this comment

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

After some hours of operation the KLF200 Gateway refuses new connections. If HA restarts, the connection build-up run into timeout during SSL Handshake. So if I start a reboot of the KLF200 before HA shutdown I can establish a new connection after HA is rebooted. Finall it's just a workaround. A disconnect function is considered in the destructor of pyvlx.connection, so I think it will not change much to remove this functionality on HA component side.

Copy link
Contributor Author

@pawlizio pawlizio Mar 5, 2021

Choose a reason for hiding this comment

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

Yes of course it would be much simpler for the user if we hardcode that, so they will not need to create an automation for that. Only point to consider is that KLF200 starts a wifi access point after each reboot where you can perform configurations. It's usually protected by a password if the user does not remove that. In the standard configuration this access point will be closed after some minutes. Finally my safety concerns are the reason not to hardcode this automation.

Copy link
Contributor

@Julius2342 Julius2342 Mar 5, 2021

Choose a reason for hiding this comment

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

@pawlizio : IMO all explicit disconnects are preferred bc then the disconnects happen in a controlled way. I guess als hass-shutdown process waits untill the disconnect is finished.

But yes, this buggy behavior of the KLF 200 gateway is annoying. There is another PR from you #42773 - where you implemented an explicit service to reboot the gateway from time to time. Isn't this the better approach?

Copy link
Contributor

@Julius2342 Julius2342 Mar 5, 2021

Choose a reason for hiding this comment

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

Only point to consider is that KLF200 starts a wifi access point after each reboot where you can perform configurations. [...] Finally my safety concerns are the reason not to hardcode this automation.

Valid point m-/ .

Copy link
Contributor Author

@pawlizio pawlizio Mar 5, 2021

Choose a reason for hiding this comment

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

The reboot service is already available in the component and yes you can start them periodically. But my preferred way was to initiate the reboot in parallel to a HA restart. Main concern was to reduce the amount of unnecessary KLF reboots and the corresponding opening of the Wifi Access Point to a bare minimum.


async def async_reboot_gateway(service_call):
await self.pyvlx.reboot_gateway()

self._hass.bus.async_listen_once(EVENT_HOMEASSISTANT_STOP, on_hass_stop)
host = self._domain_config.get(CONF_HOST)
password = self._domain_config.get(CONF_PASSWORD)
self.pyvlx = PyVLX(host=host, password=password)
Expand Down