[mqtt] Embedded MQTT server fix (home-assistant#5132)

* Embedded MQTT server fix and schema

* feedback

homeassistant/components/mqtt/__init__.py

@@ -20,6 +20,7 @@
 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 @@ def valid_publish_topic(value):
         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 \

homeassistant/components/mqtt/server.py

@@ -7,14 +7,27 @@
 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 shutdown_mqtt_server(event):
 
 def generate_config(hass, passwd):
     """Generate a configuration based on current Home Assistant instance."""
+    from homeassistant.components.mqtt import PROTOCOL_311
     config = {
         'listeners': {
             'default': {

tests/components/mqtt/test_init.py

@@ -17,6 +17,7 @@
     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 @@ def test_client_stops_on_home_assistant_start(self):
         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 @@ def test_publish_calls_service(self):
 
         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 @@ def test_mqtt_subscribes_topics_on_connect(self):
             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,