From 5855246decb0eb3e5c4d26fc23c2a372fcd096a3 Mon Sep 17 00:00:00 2001 From: Erik Date: Fri, 22 May 2020 17:33:19 +0200 Subject: [PATCH 1/3] Let PAHO client handle connection to MQTT server --- homeassistant/components/mqtt/__init__.py | 37 ++++------------------- 1 file changed, 6 insertions(+), 31 deletions(-) diff --git a/homeassistant/components/mqtt/__init__.py b/homeassistant/components/mqtt/__init__.py index 25b2b0381ea139..b55177c2392090 100644 --- a/homeassistant/components/mqtt/__init__.py +++ b/homeassistant/components/mqtt/__init__.py @@ -29,11 +29,7 @@ EVENT_HOMEASSISTANT_STOP, ) from homeassistant.core import Event, ServiceCall, callback -from homeassistant.exceptions import ( - ConfigEntryNotReady, - HomeAssistantError, - Unauthorized, -) +from homeassistant.exceptions import HomeAssistantError, Unauthorized from homeassistant.helpers import config_validation as cv, event, template from homeassistant.helpers.dispatcher import async_dispatcher_connect from homeassistant.helpers.entity import Entity @@ -649,13 +645,7 @@ async def async_setup_entry(hass, entry): tls_version=tls_version, ) - result: str = await hass.data[DATA_MQTT].async_connect() - - if result == CONNECTION_FAILED: - return False - - if result == CONNECTION_FAILED_RECOVERABLE: - raise ConfigEntryNotReady + hass.data[DATA_MQTT].async_connect() async def async_stop_mqtt(_event: Event): """Stop MQTT component.""" @@ -824,26 +814,10 @@ async def async_publish( self._mqttc.publish, topic, payload, qos, retain ) - async def async_connect(self) -> str: + def async_connect(self) -> str: """Connect to the host. Does process messages yet.""" - # pylint: disable=import-outside-toplevel - import paho.mqtt.client as mqtt - - result: int = None - try: - result = await self.hass.async_add_executor_job( - self._mqttc.connect, self.broker, self.port, self.keepalive - ) - except OSError as err: - _LOGGER.error("Failed to connect due to exception: %s", err) - return CONNECTION_FAILED_RECOVERABLE - - if result != 0: - _LOGGER.error("Failed to connect: %s", mqtt.error_string(result)) - return CONNECTION_FAILED - + self._mqttc.connect_async(self.broker, self.port, self.keepalive) self._mqttc.loop_start() - return CONNECTION_SUCCESS async def async_disconnect(self): """Stop the MQTT client.""" @@ -933,6 +907,7 @@ def _mqtt_on_connect(self, _mqttc, _userdata, _flags, result_code: int) -> None: return self.connected = True + _LOGGER.info("Connected to MQTT server (%s).", result_code) # Group subscriptions to only re-subscribe once for each topic. keyfunc = attrgetter("topic") @@ -999,7 +974,7 @@ def _mqtt_handle_message(self, msg) -> None: def _mqtt_on_disconnect(self, _mqttc, _userdata, result_code: int) -> None: """Disconnected callback.""" self.connected = False - _LOGGER.warning("Disconnected from MQTT (%s).", result_code) + _LOGGER.warning("Disconnected from MQTT server (%s).", result_code) def _raise_on_error(result_code: int) -> None: From 7290d363d7263bfde223362472105826802be35f Mon Sep 17 00:00:00 2001 From: Erik Montnemery Date: Sat, 23 May 2020 00:01:54 +0200 Subject: [PATCH 2/3] Apply suggestions from code review Co-authored-by: Martin Hjelmare --- homeassistant/components/mqtt/__init__.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/homeassistant/components/mqtt/__init__.py b/homeassistant/components/mqtt/__init__.py index b55177c2392090..75128ec93b7175 100644 --- a/homeassistant/components/mqtt/__init__.py +++ b/homeassistant/components/mqtt/__init__.py @@ -907,7 +907,7 @@ def _mqtt_on_connect(self, _mqttc, _userdata, _flags, result_code: int) -> None: return self.connected = True - _LOGGER.info("Connected to MQTT server (%s).", result_code) + _LOGGER.info("Connected to MQTT server (%s)", result_code) # Group subscriptions to only re-subscribe once for each topic. keyfunc = attrgetter("topic") @@ -974,7 +974,7 @@ def _mqtt_handle_message(self, msg) -> None: def _mqtt_on_disconnect(self, _mqttc, _userdata, result_code: int) -> None: """Disconnected callback.""" self.connected = False - _LOGGER.warning("Disconnected from MQTT server (%s).", result_code) + _LOGGER.warning("Disconnected from MQTT server (%s)", result_code) def _raise_on_error(result_code: int) -> None: From fcee6a3063360f1a82a164792c578f9f21539ee6 Mon Sep 17 00:00:00 2001 From: Erik Date: Sat, 23 May 2020 13:10:02 +0200 Subject: [PATCH 3/3] Call blocking connect when loading config entry --- homeassistant/components/mqtt/__init__.py | 21 ++++++++++++++++++--- tests/components/mqtt/test_init.py | 12 ++++++------ 2 files changed, 24 insertions(+), 9 deletions(-) diff --git a/homeassistant/components/mqtt/__init__.py b/homeassistant/components/mqtt/__init__.py index 75128ec93b7175..64b25ad9486c92 100644 --- a/homeassistant/components/mqtt/__init__.py +++ b/homeassistant/components/mqtt/__init__.py @@ -645,7 +645,7 @@ async def async_setup_entry(hass, entry): tls_version=tls_version, ) - hass.data[DATA_MQTT].async_connect() + await hass.data[DATA_MQTT].async_connect() async def async_stop_mqtt(_event: Event): """Stop MQTT component.""" @@ -814,9 +814,24 @@ async def async_publish( self._mqttc.publish, topic, payload, qos, retain ) - def async_connect(self) -> str: + async def async_connect(self) -> str: """Connect to the host. Does process messages yet.""" - self._mqttc.connect_async(self.broker, self.port, self.keepalive) + # pylint: disable=import-outside-toplevel + import paho.mqtt.client as mqtt + + result: int = None + try: + result = await self.hass.async_add_executor_job( + self._mqttc.connect, self.broker, self.port, self.keepalive + ) + except OSError as err: + _LOGGER.error("Failed to connect to MQTT server due to exception: %s", err) + + if result is not None and result != 0: + _LOGGER.error( + "Failed to connect to MQTT server: %s", mqtt.error_string(result) + ) + self._mqttc.loop_start() async def async_disconnect(self): diff --git a/tests/components/mqtt/test_init.py b/tests/components/mqtt/test_init.py index 9ec5e09f276516..3626c5a746c517 100644 --- a/tests/components/mqtt/test_init.py +++ b/tests/components/mqtt/test_init.py @@ -18,7 +18,6 @@ TEMP_CELSIUS, ) from homeassistant.core import callback -from homeassistant.exceptions import ConfigEntryNotReady from homeassistant.helpers import device_registry from homeassistant.setup import async_setup_component from homeassistant.util.dt import utcnow @@ -678,23 +677,24 @@ async def test_setup_embedded_with_embedded(hass): assert _start.call_count == 1 -async def test_setup_fails_if_no_connect_broker(hass): +async def test_setup_logs_error_if_no_connect_broker(hass, caplog): """Test for setup failure if connection to broker is missing.""" entry = MockConfigEntry(domain=mqtt.DOMAIN, data={mqtt.CONF_BROKER: "test-broker"}) with patch("paho.mqtt.client.Client") as mock_client: mock_client().connect = lambda *args: 1 - assert not await mqtt.async_setup_entry(hass, entry) + assert await mqtt.async_setup_entry(hass, entry) + assert "Failed to connect to MQTT server:" in caplog.text -async def test_setup_raises_ConfigEntryNotReady_if_no_connect_broker(hass): +async def test_setup_raises_ConfigEntryNotReady_if_no_connect_broker(hass, caplog): """Test for setup failure if connection to broker is missing.""" entry = MockConfigEntry(domain=mqtt.DOMAIN, data={mqtt.CONF_BROKER: "test-broker"}) with patch("paho.mqtt.client.Client") as mock_client: mock_client().connect = MagicMock(side_effect=OSError("Connection error")) - with pytest.raises(ConfigEntryNotReady): - await mqtt.async_setup_entry(hass, entry) + assert await mqtt.async_setup_entry(hass, entry) + assert "Failed to connect to MQTT server due to exception:" in caplog.text async def test_setup_uses_certificate_on_certificate_set_to_auto(hass, mock_mqtt):