From 2d33ee6258a90aa37cd208c5d8c260af3a08e2e5 Mon Sep 17 00:00:00 2001
From: Johan Bloemberg <github@ijohan.nl>
Date: Wed, 15 Feb 2017 16:10:19 +0100
Subject: [PATCH] Reconnect robustness, expose connection state. (#5869)

* Reconnect robustness, expose connection state.

- Expose connection status as rflink.connection_status state.
- Handle alternative timeout scenario.
- Explicitly set a timeout for connections.
- Error when trying to send commands if disconnected.
- Do not block component setup on gateway connection.

* Don't use coroutine where none is needed.

* Test disconnected behaviour.

* Use proper conventions for task creation.

* Possibly fix test race condition?

* Update hass import style
---
 homeassistant/components/rflink.py | 41 +++++++++++++++++++++++-------
 tests/components/test_rflink.py    | 41 ++++++++++++++++++++++++++++++
 2 files changed, 73 insertions(+), 9 deletions(-)

diff --git a/homeassistant/components/rflink.py b/homeassistant/components/rflink.py
index e809430d210..10ccf32068f 100644
--- a/homeassistant/components/rflink.py
+++ b/homeassistant/components/rflink.py
@@ -9,12 +9,14 @@ from collections import defaultdict
 import functools as ft
 import logging
 
+import async_timeout
 import voluptuous as vol
 
 from homeassistant.const import (
     ATTR_ENTITY_ID, CONF_HOST, CONF_PORT, EVENT_HOMEASSISTANT_STOP,
     STATE_UNKNOWN)
 from homeassistant.core import CoreState, callback
+from homeassistant.exceptions import HomeAssistantError
 import homeassistant.helpers.config_validation as cv
 from homeassistant.helpers.entity import Entity
 
@@ -39,6 +41,7 @@ DATA_DEVICE_REGISTER = 'rflink_device_register'
 DATA_ENTITY_LOOKUP = 'rflink_entity_lookup'
 DEFAULT_RECONNECT_INTERVAL = 10
 DEFAULT_SIGNAL_REPETITIONS = 1
+CONNECTION_TIMEOUT = 10
 
 EVENT_BUTTON_PRESSED = 'button_pressed'
 EVENT_KEY_COMMAND = 'command'
@@ -148,7 +151,10 @@ def async_setup(hass, config):
     @asyncio.coroutine
     def connect():
         """Set up connection and hook it into HA for reconnect/shutdown."""
-        _LOGGER.info("Initiating Rflink connection")
+        _LOGGER.info('Initiating Rflink connection')
+        hass.states.async_set(
+            '{domain}.connection_status'.format(
+                domain=DOMAIN), 'connecting')
 
         # Rflink create_rflink_connection decides based on the value of host
         # (string or None) if serial or tcp mode should be used
@@ -164,13 +170,19 @@ def async_setup(hass, config):
         )
 
         try:
-            transport, protocol = yield from connection
+            with async_timeout.timeout(CONNECTION_TIMEOUT,
+                                       loop=hass.loop):
+                transport, protocol = yield from connection
+
         except (serial.serialutil.SerialException, ConnectionRefusedError,
-                TimeoutError) as exc:
+                TimeoutError, OSError, asyncio.TimeoutError) as exc:
             reconnect_interval = config[DOMAIN][CONF_RECONNECT_INTERVAL]
             _LOGGER.exception(
                 "Error connecting to Rflink, reconnecting in %s",
                 reconnect_interval)
+            hass.states.async_set(
+                '{domain}.connection_status'.format(
+                    domain=DOMAIN), 'error')
             hass.loop.call_later(reconnect_interval, reconnect, exc)
             return
 
@@ -182,9 +194,12 @@ def async_setup(hass, config):
         hass.bus.async_listen_once(EVENT_HOMEASSISTANT_STOP,
                                    lambda x: transport.close())
 
-        _LOGGER.info("Connected to Rflink")
+        _LOGGER.info('Connected to Rflink')
+        hass.states.async_set(
+            '{domain}.connection_status'.format(
+                domain=DOMAIN), 'connected')
 
-    yield from connect()
+    hass.async_add_job(connect)
     return True
 
 
@@ -279,6 +294,8 @@ class RflinkCommand(RflinkDevice):
     # are sent
     _repetition_task = None
 
+    _protocol = None
+
     @classmethod
     def set_rflink_protocol(cls, protocol, wait_ack=None):
         """Set the Rflink asyncio protocol as a class variable."""
@@ -286,6 +303,11 @@ class RflinkCommand(RflinkDevice):
         if wait_ack is not None:
             cls._wait_ack = wait_ack
 
+    @classmethod
+    def is_connected(cls):
+        """Return connection status."""
+        return bool(cls._protocol)
+
     @asyncio.coroutine
     def _async_handle_command(self, command, *args):
         """Do bookkeeping for command, send it to rflink and update state."""
@@ -329,6 +351,9 @@ class RflinkCommand(RflinkDevice):
         _LOGGER.debug(
             "Sending command: %s to Rflink device: %s", cmd, self._device_id)
 
+        if not self.is_connected():
+            raise HomeAssistantError('Cannot send command, not connected!')
+
         if self._wait_ack:
             # Puts command on outgoing buffer then waits for Rflink to confirm
             # the command has been send out in the ether.
@@ -359,12 +384,10 @@ class SwitchableRflinkDevice(RflinkCommand):
         elif command == 'off':
             self._state = False
 
-    @asyncio.coroutine
     def async_turn_on(self, **kwargs):
         """Turn the device on."""
-        yield from self._async_handle_command('turn_on')
+        return self._async_handle_command("turn_on")
 
-    @asyncio.coroutine
     def async_turn_off(self, **kwargs):
         """Turn the device off."""
-        yield from self._async_handle_command('turn_off')
+        return self._async_handle_command("turn_off")
diff --git a/tests/components/test_rflink.py b/tests/components/test_rflink.py
index ad5e7f91b2f..555ec9372ba 100644
--- a/tests/components/test_rflink.py
+++ b/tests/components/test_rflink.py
@@ -176,3 +176,44 @@ def test_reconnecting_after_failure(hass, monkeypatch):
 
     # we expect 3 calls, the initial and 2 reconnects
     assert mock_create.call_count == 3
+
+
+@asyncio.coroutine
+def test_error_when_not_connected(hass, monkeypatch):
+    """Sending command should error when not connected."""
+    domain = 'switch'
+    config = {
+        'rflink': {
+            'port': '/dev/ttyABC0',
+            CONF_RECONNECT_INTERVAL: 0,
+        },
+        domain: {
+            'platform': 'rflink',
+            'devices': {
+                'protocol_0_0': {
+                        'name': 'test',
+                        'aliasses': ['test_alias_0_0'],
+                },
+            },
+        },
+    }
+
+    # success first time but fail second
+    failures = [False, True, False]
+
+    # setup mocking rflink module
+    _, mock_create, _, disconnect_callback = yield from mock_rflink(
+        hass, config, domain, monkeypatch, failures=failures)
+
+    assert hass.states.get('rflink.connection_status').state == 'connected'
+
+    # rflink initiated disconnect
+    disconnect_callback(None)
+
+    yield from asyncio.sleep(0, loop=hass.loop)
+
+    assert hass.states.get('rflink.connection_status').state == 'error'
+
+    success = yield from hass.services.async_call(
+        domain, SERVICE_TURN_OFF, {ATTR_ENTITY_ID: 'switch.test'})
+    assert not success, 'changing state should not succeed when disconnected'
-- 
GitLab