Create a UUID from given LG soundbar device name (#81918)

Co-authored-by: Martin Hjelmare <marhje52@gmail.com>
Fixes https://github.com/home-assistant/core/issues/77524
fixes undefined

.coveragerc

@@ -672,6 +672,7 @@ omit =
     homeassistant/components/led_ble/__init__.py
     homeassistant/components/led_ble/light.py
     homeassistant/components/lg_netcast/media_player.py
+    homeassistant/components/lg_soundbar/__init__.py
     homeassistant/components/lg_soundbar/media_player.py
     homeassistant/components/lidarr/__init__.py
     homeassistant/components/lidarr/coordinator.py

homeassistant/components/lg_soundbar/config_flow.py

@@ -1,5 +1,6 @@
 """Config flow to configure the LG Soundbar integration."""
-from queue import Full, Queue
+import logging
+from queue import Empty, Full, Queue
 import socket
 
 import temescal
@@ -7,6 +8,7 @@ import voluptuous as vol
 
 from homeassistant import config_entries
 from homeassistant.const import CONF_HOST, CONF_PORT
+from homeassistant.data_entry_flow import FlowResult
 
 from .const import DEFAULT_PORT, DOMAIN
 
@@ -14,50 +16,64 @@ DATA_SCHEMA = {
     vol.Required(CONF_HOST): str,
 }
 
+_LOGGER = logging.getLogger(__name__)
+
+QUEUE_TIMEOUT = 10
+
 
 def test_connect(host, port):
     """LG Soundbar config flow test_connect."""
     uuid_q = Queue(maxsize=1)
     name_q = Queue(maxsize=1)
 
+    def check_msg_response(response, msgs, attr):
+        msg = response["msg"]
+        if msg == msgs or msg in msgs:
+            if "data" in response and attr in response["data"]:
+                return True
+            _LOGGER.debug(
+                "[%s] msg did not contain expected attr [%s]: %s", msg, attr, response
+            )
+        return False
+
     def queue_add(attr_q, data):
         try:
             attr_q.put_nowait(data)
         except Full:
-            pass
+            _LOGGER.debug("attempted to add [%s] to full queue", data)
 
     def msg_callback(response):
-        if (
+        if check_msg_response(response, ["MAC_INFO_DEV", "PRODUCT_INFO"], "s_uuid"):
-            response["msg"] in ["MAC_INFO_DEV", "PRODUCT_INFO"]
-            and "s_uuid" in response["data"]
-        ):
             queue_add(uuid_q, response["data"]["s_uuid"])
-        if (
+        if check_msg_response(response, "SPK_LIST_VIEW_INFO", "s_user_name"):
-            response["msg"] == "SPK_LIST_VIEW_INFO"
-            and "s_user_name" in response["data"]
-        ):
             queue_add(name_q, response["data"]["s_user_name"])
 
+    details = {}
+
     try:
         connection = temescal.temescal(host, port=port, callback=msg_callback)
+        connection.get_info()
         connection.get_mac_info()
         if uuid_q.empty():
             connection.get_product_info()
-        connection.get_info()
+        details["name"] = name_q.get(timeout=QUEUE_TIMEOUT)
-        details = {"name": name_q.get(timeout=10), "uuid": uuid_q.get(timeout=10)}
+        details["uuid"] = uuid_q.get(timeout=QUEUE_TIMEOUT)
-        return details
+    except Empty:
+        pass
     except socket.timeout as err:
         raise ConnectionError(f"Connection timeout with server: {host}:{port}") from err
     except OSError as err:
         raise ConnectionError(f"Cannot resolve hostname: {host}") from err
 
+    return details
+
 
 class LGSoundbarConfigFlow(config_entries.ConfigFlow, domain=DOMAIN):
     """LG Soundbar config flow."""
 
     VERSION = 1
 
-    async def async_step_user(self, user_input=None):
+    async def async_step_user(self, user_input=None) -> FlowResult:
         """Handle a flow initiated by the user."""
         if user_input is None:
             return self._show_form()
@@ -70,13 +86,19 @@ class LGSoundbarConfigFlow(config_entries.ConfigFlow, domain=DOMAIN):
         except ConnectionError:
             errors["base"] = "cannot_connect"
         else:
-            await self.async_set_unique_id(details["uuid"])
+            if len(details) != 0:
-            self._abort_if_unique_id_configured()
+                info = {
-            info = {
+                    CONF_HOST: user_input[CONF_HOST],
-                CONF_HOST: user_input[CONF_HOST],
+                    CONF_PORT: DEFAULT_PORT,
-                CONF_PORT: DEFAULT_PORT,
+                }
-            }
+                if "uuid" in details:
-            return self.async_create_entry(title=details["name"], data=info)
+                    unique_id = details["uuid"]
+                    await self.async_set_unique_id(unique_id)
+                    self._abort_if_unique_id_configured()
+                else:
+                    self._async_abort_entries_match(info)
+                return self.async_create_entry(title=details["name"], data=info)
+            errors["base"] = "no_data"
 
         return self._show_form(errors)
 

homeassistant/components/lg_soundbar/media_player.py

@@ -25,7 +25,7 @@ async def async_setup_entry(
             LGDevice(
                 config_entry.data[CONF_HOST],
                 config_entry.data[CONF_PORT],
-                config_entry.unique_id,
+                config_entry.unique_id or config_entry.entry_id,
             )
         ]
     )
@@ -82,7 +82,7 @@ class LGDevice(MediaPlayerEntity):
 
     def handle_event(self, response):
         """Handle responses from the speakers."""
-        data = response["data"]
+        data = response["data"] if "data" in response else {}
         if response["msg"] == "EQ_VIEW_INFO":
             if "i_bass" in data:
                 self._bass = data["i_bass"]

homeassistant/components/lg_soundbar/strings.json

@@ -8,7 +8,8 @@
       }
     },
     "error": {
-      "cannot_connect": "[%key:common::config_flow::error::cannot_connect%]"
+      "cannot_connect": "[%key:common::config_flow::error::cannot_connect%]",
+      "no_data": "Device did not return any data required to an entry."
     },
     "abort": {
       "already_configured": "[%key:common::config_flow::abort::already_configured_device%]"

homeassistant/components/lg_soundbar/translations/en.json

@@ -1,7 +1,8 @@
 {
     "config": {
         "abort": {
-            "already_configured": "Device is already configured"
+            "already_configured": "Device is already configured",
+            "no_uuid": "Device missing unique identification required for discovery."
         },
         "error": {
             "cannot_connect": "Failed to connect"
@@ -14,4 +15,4 @@
             }
         }
     }
-}
+}

tests/components/lg_soundbar/test_config_flow.py

@@ -1,5 +1,10 @@
 """Test the lg_soundbar config flow."""
-from unittest.mock import DEFAULT, MagicMock, Mock, call, patch
+from __future__ import annotations
+
+from collections.abc import Callable
+import socket
+from typing import Any
+from unittest.mock import DEFAULT, patch
 
 from homeassistant import config_entries
 from homeassistant.components.lg_soundbar.const import DEFAULT_PORT, DOMAIN
@@ -8,6 +13,43 @@ from homeassistant.const import CONF_HOST, CONF_PORT
 from tests.common import MockConfigEntry
 
 
+def setup_mock_temescal(
+    hass, mock_temescal, mac_info_dev=None, product_info=None, info=None
+):
+    """Set up a mock of the temescal object to craft our expected responses."""
+    tmock = mock_temescal.temescal
+    instance = tmock.return_value
+
+    def create_temescal_response(msg: str, data: dict | None = None) -> dict[str, Any]:
+        response: dict[str, Any] = {"msg": msg}
+        if data is not None:
+            response["data"] = data
+        return response
+
+    def temescal_side_effect(
+        addr: str, port: int, callback: Callable[[dict[str, Any]], None]
+    ):
+        mac_info_response = create_temescal_response(
+            msg="MAC_INFO_DEV", data=mac_info_dev
+        )
+        product_info_response = create_temescal_response(
+            msg="PRODUCT_INFO", data=product_info
+        )
+        info_response = create_temescal_response(msg="SPK_LIST_VIEW_INFO", data=info)
+
+        instance.get_mac_info.side_effect = lambda: hass.add_job(
+            callback, mac_info_response
+        )
+        instance.get_product_info.side_effect = lambda: hass.add_job(
+            callback, product_info_response
+        )
+        instance.get_info.side_effect = lambda: hass.add_job(callback, info_response)
+
+        return DEFAULT
+
+    tmock.side_effect = temescal_side_effect
+
+
 async def test_form(hass):
     """Test we get the form."""
 
@@ -18,14 +60,16 @@ async def test_form(hass):
     assert result["errors"] == {}
 
     with patch(
-        "homeassistant.components.lg_soundbar.config_flow.temescal",
+        "homeassistant.components.lg_soundbar.config_flow.temescal"
-        return_value=MagicMock(),
+    ) as mock_temescal, patch(
-    ), patch(
-        "homeassistant.components.lg_soundbar.config_flow.test_connect",
-        return_value={"uuid": "uuid", "name": "name"},
-    ), patch(
         "homeassistant.components.lg_soundbar.async_setup_entry", return_value=True
     ) as mock_setup_entry:
+        setup_mock_temescal(
+            hass=hass,
+            mock_temescal=mock_temescal,
+            mac_info_dev={"s_uuid": "uuid"},
+            info={"s_user_name": "name"},
+        )
         result2 = await hass.config_entries.flow.async_configure(
             result["flow_id"],
             {
@@ -36,6 +80,7 @@ async def test_form(hass):
 
     assert result2["type"] == "create_entry"
     assert result2["title"] == "name"
+    assert result2["result"].unique_id == "uuid"
     assert result2["data"] == {
         CONF_HOST: "1.1.1.1",
         CONF_PORT: DEFAULT_PORT,
@@ -43,8 +88,8 @@ async def test_form(hass):
     assert len(mock_setup_entry.mock_calls) == 1
 
 
-async def test_form_uuid_missing_from_mac_info(hass):
+async def test_form_mac_info_response_empty(hass):
-    """Test we get the form, but uuid is missing from the initial get_mac_info function call."""
+    """Test we get the form, but response from the initial get_mac_info function call is empty."""
 
     result = await hass.config_entries.flow.async_init(
         DOMAIN, context={"source": config_entries.SOURCE_USER}
@@ -53,23 +98,16 @@ async def test_form_uuid_missing_from_mac_info(hass):
     assert result["errors"] == {}
 
     with patch(
-        "homeassistant.components.lg_soundbar.config_flow.temescal", return_value=Mock()
+        "homeassistant.components.lg_soundbar.config_flow.temescal"
     ) as mock_temescal, patch(
         "homeassistant.components.lg_soundbar.async_setup_entry", return_value=True
     ) as mock_setup_entry:
-        tmock = mock_temescal.temescal
+        setup_mock_temescal(
-        tmock.return_value = Mock()
+            hass=hass,
-        instance = tmock.return_value
+            mock_temescal=mock_temescal,
-
+            mac_info_dev={"s_uuid": "uuid"},
-        def temescal_side_effect(addr, port, callback):
+            info={"s_user_name": "name"},
-            product_info = {"msg": "PRODUCT_INFO", "data": {"s_uuid": "uuid"}}
+        )
-            instance.get_product_info.side_effect = lambda: callback(product_info)
-            info = {"msg": "SPK_LIST_VIEW_INFO", "data": {"s_user_name": "name"}}
-            instance.get_info.side_effect = lambda: callback(info)
-            return DEFAULT
-
-        tmock.side_effect = temescal_side_effect
-
         result2 = await hass.config_entries.flow.async_configure(
             result["flow_id"],
             {
@@ -80,6 +118,7 @@ async def test_form_uuid_missing_from_mac_info(hass):
 
     assert result2["type"] == "create_entry"
     assert result2["title"] == "name"
+    assert result2["result"].unique_id == "uuid"
     assert result2["data"] == {
         CONF_HOST: "1.1.1.1",
         CONF_PORT: DEFAULT_PORT,
@@ -99,35 +138,18 @@ async def test_form_uuid_present_in_both_functions_uuid_q_empty(hass):
     assert result["type"] == "form"
     assert result["errors"] == {}
 
-    mock_uuid_q = MagicMock()
-    mock_name_q = MagicMock()
-
     with patch(
-        "homeassistant.components.lg_soundbar.config_flow.temescal", return_value=Mock()
+        "homeassistant.components.lg_soundbar.config_flow.temescal"
     ) as mock_temescal, patch(
-        "homeassistant.components.lg_soundbar.config_flow.Queue",
-        return_value=MagicMock(),
-    ) as mock_q, patch(
         "homeassistant.components.lg_soundbar.async_setup_entry", return_value=True
     ) as mock_setup_entry:
-        mock_q.side_effect = [mock_uuid_q, mock_name_q]
+        setup_mock_temescal(
-        mock_uuid_q.empty.return_value = True
+            hass=hass,
-        mock_uuid_q.get.return_value = "uuid"
+            mock_temescal=mock_temescal,
-        mock_name_q.get.return_value = "name"
+            mac_info_dev={"s_uuid": "uuid"},
-        tmock = mock_temescal.temescal
+            product_info={"s_uuid": "uuid"},
-        tmock.return_value = Mock()
+            info={"s_user_name": "name"},
-        instance = tmock.return_value
+        )
-
-        def temescal_side_effect(addr, port, callback):
-            mac_info = {"msg": "MAC_INFO_DEV", "data": {"s_uuid": "uuid"}}
-            instance.get_mac_info.side_effect = lambda: callback(mac_info)
-            product_info = {"msg": "PRODUCT_INFO", "data": {"s_uuid": "uuid"}}
-            instance.get_product_info.side_effect = lambda: callback(product_info)
-            info = {"msg": "SPK_LIST_VIEW_INFO", "data": {"s_user_name": "name"}}
-            instance.get_info.side_effect = lambda: callback(info)
-            return DEFAULT
-
-        tmock.side_effect = temescal_side_effect
 
         result2 = await hass.config_entries.flow.async_configure(
             result["flow_id"],
@@ -139,14 +161,12 @@ async def test_form_uuid_present_in_both_functions_uuid_q_empty(hass):
 
     assert result2["type"] == "create_entry"
     assert result2["title"] == "name"
+    assert result2["result"].unique_id == "uuid"
     assert result2["data"] == {
         CONF_HOST: "1.1.1.1",
         CONF_PORT: DEFAULT_PORT,
     }
     assert len(mock_setup_entry.mock_calls) == 1
-    mock_uuid_q.empty.assert_called_once()
-    mock_uuid_q.put_nowait.has_calls([call("uuid"), call("uuid")])
-    mock_uuid_q.get.assert_called_once()
 
 
 async def test_form_uuid_present_in_both_functions_uuid_q_not_empty(hass):
@@ -161,33 +181,21 @@ async def test_form_uuid_present_in_both_functions_uuid_q_not_empty(hass):
     assert result["type"] == "form"
     assert result["errors"] == {}
 
-    mock_uuid_q = MagicMock()
-    mock_name_q = MagicMock()
-
     with patch(
-        "homeassistant.components.lg_soundbar.config_flow.temescal", return_value=Mock()
+        "homeassistant.components.lg_soundbar.config_flow.QUEUE_TIMEOUT",
+        new=0.1,
+    ), patch(
+        "homeassistant.components.lg_soundbar.config_flow.temescal"
     ) as mock_temescal, patch(
-        "homeassistant.components.lg_soundbar.config_flow.Queue",
-        return_value=MagicMock(),
-    ) as mock_q, patch(
         "homeassistant.components.lg_soundbar.async_setup_entry", return_value=True
     ) as mock_setup_entry:
-        mock_q.side_effect = [mock_uuid_q, mock_name_q]
+        setup_mock_temescal(
-        mock_uuid_q.empty.return_value = False
+            hass=hass,
-        mock_uuid_q.get.return_value = "uuid"
+            mock_temescal=mock_temescal,
-        mock_name_q.get.return_value = "name"
+            mac_info_dev={"s_uuid": "uuid"},
-        tmock = mock_temescal.temescal
+            product_info={"s_uuid": "uuid"},
-        tmock.return_value = Mock()
+            info={"s_user_name": "name"},
-        instance = tmock.return_value
+        )
-
-        def temescal_side_effect(addr, port, callback):
-            mac_info = {"msg": "MAC_INFO_DEV", "data": {"s_uuid": "uuid"}}
-            instance.get_mac_info.side_effect = lambda: callback(mac_info)
-            info = {"msg": "SPK_LIST_VIEW_INFO", "data": {"s_user_name": "name"}}
-            instance.get_info.side_effect = lambda: callback(info)
-            return DEFAULT
-
-        tmock.side_effect = temescal_side_effect
 
         result2 = await hass.config_entries.flow.async_configure(
             result["flow_id"],
@@ -199,26 +207,196 @@ async def test_form_uuid_present_in_both_functions_uuid_q_not_empty(hass):
 
     assert result2["type"] == "create_entry"
     assert result2["title"] == "name"
+    assert result2["result"].unique_id == "uuid"
     assert result2["data"] == {
         CONF_HOST: "1.1.1.1",
         CONF_PORT: DEFAULT_PORT,
     }
     assert len(mock_setup_entry.mock_calls) == 1
-    mock_uuid_q.empty.assert_called_once()
-    mock_uuid_q.put_nowait.assert_called_once()
-    mock_uuid_q.get.assert_called_once()
 
 
-async def test_form_cannot_connect(hass):
+async def test_form_uuid_missing_from_mac_info(hass):
-    """Test we handle cannot connect error."""
+    """Test we get the form, but uuid is missing from the initial get_mac_info function call."""
+
+    result = await hass.config_entries.flow.async_init(
+        DOMAIN, context={"source": config_entries.SOURCE_USER}
+    )
+    assert result["type"] == "form"
+    assert result["errors"] == {}
+
+    with patch(
+        "homeassistant.components.lg_soundbar.config_flow.temescal"
+    ) as mock_temescal, patch(
+        "homeassistant.components.lg_soundbar.async_setup_entry", return_value=True
+    ) as mock_setup_entry:
+        setup_mock_temescal(
+            hass=hass,
+            mock_temescal=mock_temescal,
+            product_info={"s_uuid": "uuid"},
+            info={"s_user_name": "name"},
+        )
+
+        result2 = await hass.config_entries.flow.async_configure(
+            result["flow_id"],
+            {
+                CONF_HOST: "1.1.1.1",
+            },
+        )
+        await hass.async_block_till_done()
+
+    assert result2["type"] == "create_entry"
+    assert result2["title"] == "name"
+    assert result2["result"].unique_id == "uuid"
+    assert result2["data"] == {
+        CONF_HOST: "1.1.1.1",
+        CONF_PORT: DEFAULT_PORT,
+    }
+    assert len(mock_setup_entry.mock_calls) == 1
+
+
+async def test_form_uuid_not_provided_by_api(hass):
+    """Test we get the form, but uuid is missing from the all API messages."""
+
+    result = await hass.config_entries.flow.async_init(
+        DOMAIN, context={"source": config_entries.SOURCE_USER}
+    )
+    assert result["type"] == "form"
+    assert result["errors"] == {}
+
+    with patch(
+        "homeassistant.components.lg_soundbar.config_flow.QUEUE_TIMEOUT",
+        new=0.1,
+    ), patch(
+        "homeassistant.components.lg_soundbar.config_flow.temescal"
+    ) as mock_temescal, patch(
+        "homeassistant.components.lg_soundbar.async_setup_entry", return_value=True
+    ) as mock_setup_entry:
+        setup_mock_temescal(
+            hass=hass,
+            mock_temescal=mock_temescal,
+            product_info={"i_model_no": "8", "i_model_type": 0},
+            info={"s_user_name": "name"},
+        )
+        result2 = await hass.config_entries.flow.async_configure(
+            result["flow_id"],
+            {
+                CONF_HOST: "1.1.1.1",
+            },
+        )
+        await hass.async_block_till_done()
+
+    assert result2["type"] == "create_entry"
+    assert result2["title"] == "name"
+    assert result2["result"].unique_id is None
+    assert result2["data"] == {
+        CONF_HOST: "1.1.1.1",
+        CONF_PORT: DEFAULT_PORT,
+    }
+    assert len(mock_setup_entry.mock_calls) == 1
+
+
+async def test_form_both_queues_empty(hass):
+    """Test we get the form, but none of the data we want is provided by the API."""
+
+    result = await hass.config_entries.flow.async_init(
+        DOMAIN, context={"source": config_entries.SOURCE_USER}
+    )
+    assert result["type"] == "form"
+    assert result["errors"] == {}
+
+    with patch(
+        "homeassistant.components.lg_soundbar.config_flow.QUEUE_TIMEOUT",
+        new=0.1,
+    ), patch(
+        "homeassistant.components.lg_soundbar.config_flow.temescal"
+    ) as mock_temescal, patch(
+        "homeassistant.components.lg_soundbar.async_setup_entry", return_value=True
+    ) as mock_setup_entry:
+        setup_mock_temescal(hass=hass, mock_temescal=mock_temescal)
+
+        result2 = await hass.config_entries.flow.async_configure(
+            result["flow_id"],
+            {
+                CONF_HOST: "1.1.1.1",
+            },
+        )
+        await hass.async_block_till_done()
+
+    assert result2["type"] == "form"
+    assert result2["errors"] == {"base": "no_data"}
+    assert len(mock_setup_entry.mock_calls) == 0
+
+
+async def test_no_uuid_host_already_configured(hass):
+    """Test we handle if the device has no UUID and the host has already been configured."""
+
+    mock_entry = MockConfigEntry(
+        domain=DOMAIN,
+        data={
+            CONF_HOST: "1.1.1.1",
+            CONF_PORT: DEFAULT_PORT,
+        },
+    )
+    mock_entry.add_to_hass(hass)
+
+    result = await hass.config_entries.flow.async_init(
+        DOMAIN, context={"source": config_entries.SOURCE_USER}
+    )
+
+    assert result["type"] == "form"
+    assert result["errors"] == {}
+
+    with patch(
+        "homeassistant.components.lg_soundbar.config_flow.QUEUE_TIMEOUT",
+        new=0.1,
+    ), patch(
+        "homeassistant.components.lg_soundbar.config_flow.temescal"
+    ) as mock_temescal:
+        setup_mock_temescal(
+            hass=hass, mock_temescal=mock_temescal, info={"s_user_name": "name"}
+        )
+        result2 = await hass.config_entries.flow.async_configure(
+            result["flow_id"],
+            {
+                CONF_HOST: "1.1.1.1",
+            },
+        )
+
+    assert result2["type"] == "abort"
+    assert result2["reason"] == "already_configured"
+
+
+async def test_form_socket_timeout(hass):
+    """Test we handle socket.timeout error."""
     result = await hass.config_entries.flow.async_init(
         DOMAIN, context={"source": config_entries.SOURCE_USER}
     )
 
     with patch(
-        "homeassistant.components.lg_soundbar.config_flow.test_connect",
+        "homeassistant.components.lg_soundbar.config_flow.temescal"
-        side_effect=ConnectionError,
+    ) as mock_temescal:
-    ):
+        mock_temescal.temescal.side_effect = socket.timeout
+        result2 = await hass.config_entries.flow.async_configure(
+            result["flow_id"],
+            {
+                CONF_HOST: "1.1.1.1",
+            },
+        )
+
+    assert result2["type"] == "form"
+    assert result2["errors"] == {"base": "cannot_connect"}
+
+
+async def test_form_os_error(hass):
+    """Test we handle OSError."""
+    result = await hass.config_entries.flow.async_init(
+        DOMAIN, context={"source": config_entries.SOURCE_USER}
+    )
+
+    with patch(
+        "homeassistant.components.lg_soundbar.config_flow.temescal"
+    ) as mock_temescal:
+        mock_temescal.temescal.side_effect = OSError
         result2 = await hass.config_entries.flow.async_configure(
             result["flow_id"],
             {
@@ -247,9 +425,15 @@ async def test_form_already_configured(hass):
     )
 
     with patch(
-        "homeassistant.components.lg_soundbar.config_flow.test_connect",
+        "homeassistant.components.lg_soundbar.config_flow.temescal"
-        return_value={"uuid": "uuid", "name": "name"},
+    ) as mock_temescal:
-    ):
+        setup_mock_temescal(
+            hass=hass,
+            mock_temescal=mock_temescal,
+            mac_info_dev={"s_uuid": "uuid"},
+            info={"s_user_name": "name"},
+        )
+
         result2 = await hass.config_entries.flow.async_configure(
             result["flow_id"],
             {