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

Reboot function added #43

Merged
merged 6 commits into from
Jul 5, 2020
Merged

Reboot function added #43

merged 6 commits into from
Jul 5, 2020

Conversation

pawlizio
Copy link
Collaborator

This pull request adds reboot functionality to pyvlx
and reboot will be performed each 2nd connection (after 1st connection, after 3rd, after 5th,...) in order to avoid that KLF200 freezes due to lost TLS sockets.

This is experimental only, not sure if this fix the SSL Handshake Issue #30 at all.

@coveralls
Copy link

coveralls commented Jun 26, 2020

Coverage Status

Coverage decreased (-0.2%) to 81.62% when pulling 8462e24 on pawlizio:master into 790ee7a on Julius2342:master.

@pawlizio
Copy link
Collaborator Author

pawlizio commented Jun 26, 2020

As a separate function it is now also possible to initiate a reboot when HA is rebooted.

pyvlx/pyvlx.py Outdated
@@ -47,6 +48,16 @@ async def connect(self):
await login.do_api_call()
if not login.success:
raise PyVLXException("Login to KLF 200 failed, check credentials")
if self.connection.connection_counter & 1:
Copy link
Owner

Choose a reason for hiding this comment

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

I think this logic should be handled within HASS.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I agree. At the moment it seems that this doesn't solve the problem at all. I still had a frozen KLF over the weekend even though I initiate a reboot via HASS at async def on_hass_stop(event):. So before HASS is stopped for a reboot.

However I'm not sure whether the KLF was also rebooting when I had rebooted HASS. Last time I only could observe one reboot, but at a timing when HASS was loaded, not when HASS was shutting down. I'll have to do some more tests with this.

Copy link
Owner

Choose a reason for hiding this comment

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

All approaches are somehow fishy ...

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I cleaned it up, so now only the reboot function is added and the connection counter, but no automatic reboot. On my custom component I have added an automatic reboot on hass stop event

@Julius2342
Copy link
Owner

@pawlizio : The change looks good. But may I ask you to add some tests for it? Just to make sure we dont break the functionality in any future release .. Thank you very much!

@pawlizio
Copy link
Collaborator Author

pawlizio commented Jul 2, 2020

I added tests for each frame.

Copy link
Owner

@Julius2342 Julius2342 left a comment

Choose a reason for hiding this comment

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

Looks valid :)

@Julius2342 Julius2342 merged commit c4ae11e into Julius2342:master Jul 5, 2020
@gaggio
Copy link

gaggio commented Jul 27, 2020

Sorry to write here to ask for guidance: I'd like to test rebooting the KLF on HASS restart, I didn't find how to call the service just implemented.
In my experience the KLF doesn't hang if I pull the KLF power cord, restart HASS, re-power the KLF, so I'm trying to do this on automations. I'm on HA 0.113.0.

@pawlizio
Copy link
Collaborator Author

@Julius2342 : Could you kindly release the version with implemented reboot functionality and update the HASS manifest?
pyvlx==0.2.16 does not support reboot functionality.

On my custom component I have modified as follows:
As you can see I've also registered a service for HASS, so it is also possible to reboot directly from HASS via automation or manually.

class VeluxModule:
   """Abstraction for velux component."""

    def __init__(self, hass, domain_config):
        """Initialize for velux component."""
        self.pyvlx = None
        self._hass = hass
        self._domain_config = domain_config

    def setup(self):
        """Velux component setup."""

        async def on_hass_stop(event):
            """Close connection when hass stops."""
            _LOGGER.debug("Velux interface terminated, Gateway will be rebooted")
            await self.pyvlx.reboot_gateway()
            self.pyvlx.connection.disconnect()

        async def reboot_gateway(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)
        self._hass.services.async_register(DOMAIN, "reboot_gateway", reboot_gateway)

    async def reboot(self):
        self.pyvlx.reboot_gateway()

    async def async_start(self):
        """Start velux component."""
        _LOGGER.debug("Velux interface started")
        await self.pyvlx.load_scenes()
        await self.pyvlx.load_nodes()

However to be hones, I've still lots of freezes on KLF. Sometimes KLF looses the connection without reboot on HASS.
I implemented a logger on TCPTransport class to get an overview how often this happens:

 def connection_lost(self, exc):
        """Handle lost connection."""
        PYVLXLOG.warning("KLF connection lost")
        self.connection_closed_cb()

Below you can see the result:
image

I'm thinking about registering an event on HASS in order to initiate a reboot each time a connection has been lost (hoping that a second connection to KLF could be established). However I'm not that experienced developer and have currently no idea how to realize it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants