From 4692ea85b7e53ab21cefc9bfe4b73fc436d5238d Mon Sep 17 00:00:00 2001 From: Johann Kellerman Date: Wed, 4 Jan 2017 00:17:56 +0200 Subject: [PATCH] [mqtt] Embedded MQTT server fix (#5132) * Embedded MQTT server fix and schema * feedback --- homeassistant/components/mqtt/__init__.py | 29 +++++++------ homeassistant/components/mqtt/server.py | 16 ++++++- tests/components/mqtt/test_init.py | 52 ++++++++++++++--------- 3 files changed, 62 insertions(+), 35 deletions(-) diff --git a/homeassistant/components/mqtt/__init__.py b/homeassistant/components/mqtt/__init__.py index 87c1a783e6e..ad4cce15cf3 100644 --- a/homeassistant/components/mqtt/__init__.py +++ b/homeassistant/components/mqtt/__init__.py @@ -20,6 +20,7 @@ from homeassistant.helpers.event import threaded_listener_factory from homeassistant.const import ( EVENT_HOMEASSISTANT_START, EVENT_HOMEASSISTANT_STOP, CONF_VALUE_TEMPLATE, CONF_USERNAME, CONF_PASSWORD, CONF_PORT, CONF_PROTOCOL, CONF_PAYLOAD) +from homeassistant.components.mqtt.server import HBMQTT_CONFIG_SCHEMA _LOGGER = logging.getLogger(__name__) @@ -80,7 +81,6 @@ def valid_publish_topic(value): _VALID_QOS_SCHEMA = vol.All(vol.Coerce(int), vol.In([0, 1, 2])) -_HBMQTT_CONFIG_SCHEMA = vol.Schema(dict) CLIENT_KEY_AUTH_MSG = 'client_key and client_cert must both be present in ' \ 'the mqtt broker config' @@ -109,7 +109,7 @@ CONFIG_SCHEMA = vol.Schema({ vol.Optional(CONF_TLS_INSECURE): cv.boolean, vol.Optional(CONF_PROTOCOL, default=DEFAULT_PROTOCOL): vol.All(cv.string, vol.In([PROTOCOL_31, PROTOCOL_311])), - vol.Optional(CONF_EMBEDDED): _HBMQTT_CONFIG_SCHEMA, + vol.Optional(CONF_EMBEDDED): HBMQTT_CONFIG_SCHEMA, vol.Optional(CONF_WILL_MESSAGE): MQTT_WILL_BIRTH_SCHEMA, vol.Optional(CONF_BIRTH_MESSAGE): MQTT_WILL_BIRTH_SCHEMA }), @@ -222,18 +222,7 @@ def setup(hass, config): broker_config = _setup_server(hass, config) - broker_in_conf = CONF_BROKER in conf - - # Only auto config if no server config was passed in - if broker_config and CONF_EMBEDDED not in conf: - broker, port, username, password, certificate, protocol = broker_config - # Embedded broker doesn't have some ssl variables - client_key, client_cert, tls_insecure = None, None, None - elif not broker_config and not broker_in_conf: - _LOGGER.error("Unable to start broker and auto-configure MQTT") - return False - - if broker_in_conf: + if CONF_BROKER in conf: broker = conf[CONF_BROKER] port = conf[CONF_PORT] username = conf.get(CONF_USERNAME) @@ -243,6 +232,18 @@ def setup(hass, config): client_cert = conf.get(CONF_CLIENT_CERT) tls_insecure = conf.get(CONF_TLS_INSECURE) protocol = conf[CONF_PROTOCOL] + elif broker_config: + # If no broker passed in, auto config to internal server + broker, port, username, password, certificate, protocol = broker_config + # Embedded broker doesn't have some ssl variables + client_key, client_cert, tls_insecure = None, None, None + else: + err = "Unable to start MQTT broker." + if conf.get(CONF_EMBEDDED) is not None: + # Explicit embedded config, requires explicit broker config + err += " (Broker configuration required.)" + _LOGGER.error(err) + return False # For cloudmqtt.com, secured connection, auto fill in certificate if certificate is None and 19999 < port < 30000 and \ diff --git a/homeassistant/components/mqtt/server.py b/homeassistant/components/mqtt/server.py index 7910477c808..57ad04fd18d 100644 --- a/homeassistant/components/mqtt/server.py +++ b/homeassistant/components/mqtt/server.py @@ -7,14 +7,27 @@ https://home-assistant.io/components/mqtt/#use-the-embedded-broker import logging import tempfile +import voluptuous as vol + from homeassistant.core import callback -from homeassistant.components.mqtt import PROTOCOL_311 from homeassistant.const import EVENT_HOMEASSISTANT_STOP +import homeassistant.helpers.config_validation as cv from homeassistant.util.async import run_coroutine_threadsafe REQUIREMENTS = ['hbmqtt==0.8'] DEPENDENCIES = ['http'] +# None allows custom config to be created through generate_config +HBMQTT_CONFIG_SCHEMA = vol.Any(None, vol.Schema({ + vol.Optional('auth'): vol.Schema({ + vol.Optional('password-file'): cv.isfile, + }, extra=vol.ALLOW_EXTRA), + vol.Optional('listeners'): vol.Schema({ + vol.Required('default'): vol.Schema(dict), + str: vol.Schema(dict) + }) +}, extra=vol.ALLOW_EXTRA)) + def start(hass, server_config): """Initialize MQTT Server.""" @@ -48,6 +61,7 @@ def start(hass, server_config): def generate_config(hass, passwd): """Generate a configuration based on current Home Assistant instance.""" + from homeassistant.components.mqtt import PROTOCOL_311 config = { 'listeners': { 'default': { diff --git a/tests/components/mqtt/test_init.py b/tests/components/mqtt/test_init.py index ff74c070d85..54566a09f59 100644 --- a/tests/components/mqtt/test_init.py +++ b/tests/components/mqtt/test_init.py @@ -17,6 +17,7 @@ from tests.common import ( get_test_home_assistant, mock_mqtt_component, fire_mqtt_message) +# pylint: disable=invalid-name class TestMQTT(unittest.TestCase): """Test the MQTT component.""" @@ -49,27 +50,37 @@ class TestMQTT(unittest.TestCase): self.hass.block_till_done() self.assertTrue(mqtt.MQTT_CLIENT.stop.called) - def test_setup_fails_if_no_connect_broker(self): + @mock.patch('paho.mqtt.client.Client') + def test_setup_fails_if_no_connect_broker(self, _): """Test for setup failure if connection to broker is missing.""" + test_broker_cfg = {mqtt.DOMAIN: {mqtt.CONF_BROKER: 'test-broker'}} + with mock.patch('homeassistant.components.mqtt.MQTT', side_effect=socket.error()): self.hass.config.components = [] - assert not setup_component(self.hass, mqtt.DOMAIN, { - mqtt.DOMAIN: { - mqtt.CONF_BROKER: 'test-broker', - } - }) + assert not setup_component(self.hass, mqtt.DOMAIN, test_broker_cfg) - def test_setup_protocol_validation(self): - """Test for setup failure if connection to broker is missing.""" - with mock.patch('paho.mqtt.client.Client'): + # Ensure if we dont raise it sets up correctly + self.hass.config.components = [] + assert setup_component(self.hass, mqtt.DOMAIN, test_broker_cfg) + + @mock.patch('paho.mqtt.client.Client') + def test_setup_embedded(self, _): + """Test setting up embedded server with no config.""" + client_config = ('localhost', 1883, 'user', 'pass', None, '3.1.1') + + with mock.patch('homeassistant.components.mqtt.server.start', + return_value=(True, client_config)) as _start: self.hass.config.components = [] - assert setup_component(self.hass, mqtt.DOMAIN, { - mqtt.DOMAIN: { - mqtt.CONF_BROKER: 'test-broker', - mqtt.CONF_PROTOCOL: 3.1, - } - }) + assert setup_component(self.hass, mqtt.DOMAIN, + {mqtt.DOMAIN: {}}) + assert _start.call_count == 1 + + # Test with `embedded: None` + self.hass.config.components = [] + assert setup_component(self.hass, mqtt.DOMAIN, + {mqtt.DOMAIN: {'embedded': None}}) + assert _start.call_count == 2 # Another call def test_publish_calls_service(self): """Test the publishing of call to services.""" @@ -81,11 +92,11 @@ class TestMQTT(unittest.TestCase): self.assertEqual(1, len(self.calls)) self.assertEqual( - 'test-topic', - self.calls[0][0].data['service_data'][mqtt.ATTR_TOPIC]) + 'test-topic', + self.calls[0][0].data['service_data'][mqtt.ATTR_TOPIC]) self.assertEqual( - 'test-payload', - self.calls[0][0].data['service_data'][mqtt.ATTR_PAYLOAD]) + 'test-payload', + self.calls[0][0].data['service_data'][mqtt.ATTR_PAYLOAD]) def test_service_call_without_topic_does_not_publish(self): """Test the service call if topic is missing.""" @@ -293,7 +304,8 @@ class TestMQTTCallbacks(unittest.TestCase): 3: 'home/sensor', }, mqtt.MQTT_CLIENT.progress) - def test_mqtt_birth_message_on_connect(self): + def test_mqtt_birth_message_on_connect(self): \ + # pylint: disable=no-self-use """Test birth message on connect.""" mqtt.MQTT_CLIENT._mqtt_on_connect(None, None, 0, 0) mqtt.MQTT_CLIENT._mqttc.publish.assert_called_with('birth', 'birth', 0,