Improve ESPHome update platform error reporting (#96455)

homeassistant/components/esphome/update.py

@@ -14,6 +14,7 @@ from homeassistant.components.update import (
 )
 from homeassistant.config_entries import ConfigEntry
 from homeassistant.core import CALLBACK_TYPE, HomeAssistant, callback
+from homeassistant.exceptions import HomeAssistantError
 from homeassistant.helpers import device_registry as dr
 from homeassistant.helpers.dispatcher import async_dispatcher_connect
 from homeassistant.helpers.entity import DeviceInfo
@@ -27,6 +28,9 @@ from .entry_data import RuntimeEntryData
 KEY_UPDATE_LOCK = "esphome_update_lock"
 
 
+_LOGGER = logging.getLogger(__name__)
+
+
 async def async_setup_entry(
     hass: HomeAssistant,
     entry: ConfigEntry,
@@ -109,14 +113,10 @@ class ESPHomeUpdateEntity(CoordinatorEntity[ESPHomeDashboard], UpdateEntity):
         During deep sleep the ESP will not be connectable (by design)
         and thus, even when unavailable, we'll show it as available.
         """
-        return (
+        return super().available and (
-            super().available
+            self._entry_data.available
-            and (
+            or self._entry_data.expected_disconnect
-                self._entry_data.available
+            or self._device_info.has_deep_sleep
-                or self._entry_data.expected_disconnect
-                or self._device_info.has_deep_sleep
-            )
-            and self._device_info.name in self.coordinator.data
         )
 
     @property
@@ -137,33 +137,26 @@ class ESPHomeUpdateEntity(CoordinatorEntity[ESPHomeDashboard], UpdateEntity):
         """URL to the full release notes of the latest version available."""
         return "https://esphome.io/changelog/"
 
+    @callback
+    def _async_static_info_updated(self, _: list[EntityInfo]) -> None:
+        """Handle static info update."""
+        self.async_write_ha_state()
+
     async def async_added_to_hass(self) -> None:
         """Handle entity added to Home Assistant."""
         await super().async_added_to_hass()
-
-        @callback
-        def _static_info_updated(infos: list[EntityInfo]) -> None:
-            """Handle static info update."""
-            self.async_write_ha_state()
-
         self.async_on_remove(
             async_dispatcher_connect(
                 self.hass,
                 self._entry_data.signal_static_info_updated,
-                _static_info_updated,
+                self._async_static_info_updated,
             )
         )
-
-        @callback
-        def _on_device_update() -> None:
-            """Handle update of device state, like availability."""
-            self.async_write_ha_state()
-
         self.async_on_remove(
             async_dispatcher_connect(
                 self.hass,
                 self._entry_data.signal_device_updated,
-                _on_device_update,
+                self.async_write_ha_state,
             )
         )
 
@@ -172,16 +165,20 @@ class ESPHomeUpdateEntity(CoordinatorEntity[ESPHomeDashboard], UpdateEntity):
     ) -> None:
         """Install an update."""
         async with self.hass.data.setdefault(KEY_UPDATE_LOCK, asyncio.Lock()):
-            device = self.coordinator.data.get(self._device_info.name)
+            coordinator = self.coordinator
+            api = coordinator.api
+            device = coordinator.data.get(self._device_info.name)
             assert device is not None
-            if not await self.coordinator.api.compile(device["configuration"]):
+            try:
-                logging.getLogger(__name__).error(
+                if not await api.compile(device["configuration"]):
-                    "Error compiling %s. Try again in ESPHome dashboard for error",
+                    raise HomeAssistantError(
-                    device["configuration"],
+                        f"Error compiling {device['configuration']}; "
-                )
+                        "Try again in ESPHome dashboard for more information."
-            if not await self.coordinator.api.upload(device["configuration"], "OTA"):
+                    )
-                logging.getLogger(__name__).error(
+                if not await api.upload(device["configuration"], "OTA"):
-                    "Error OTA updating %s. Try again in ESPHome dashboard for error",
+                    raise HomeAssistantError(
-                    device["configuration"],
+                        f"Error updating {device['configuration']} via OTA; "
-                )
+                        "Try again in ESPHome dashboard for more information."
-            await self.coordinator.async_request_refresh()
+                    )
+            finally:
+                await self.coordinator.async_request_refresh()

tests/components/esphome/conftest.py

@@ -15,6 +15,7 @@ from aioesphomeapi import (
     ReconnectLogic,
     UserService,
 )
+import async_timeout
 import pytest
 from zeroconf import Zeroconf
 
@@ -53,6 +54,11 @@ async def load_homeassistant(hass) -> None:
     assert await async_setup_component(hass, "homeassistant", {})
 
 
+@pytest.fixture(autouse=True)
+def mock_tts(mock_tts_cache_dir):
+    """Auto mock the tts cache."""
+
+
 @pytest.fixture
 def mock_config_entry(hass) -> MockConfigEntry:
     """Return the default mocked config entry."""
@@ -248,10 +254,10 @@ async def _mock_generic_device_entry(
         "homeassistant.components.esphome.manager.ReconnectLogic", MockReconnectLogic
     ):
         assert await hass.config_entries.async_setup(entry.entry_id)
-        await try_connect_done.wait()
+        async with async_timeout.timeout(2):
+            await try_connect_done.wait()
 
     await hass.async_block_till_done()
-
     return mock_device
 
 

tests/components/esphome/test_update.py

@@ -1,18 +1,35 @@
 """Test ESPHome update entities."""
 import asyncio
+from collections.abc import Awaitable, Callable
 import dataclasses
 from unittest.mock import Mock, patch
 
+from aioesphomeapi import (
+    APIClient,
+    EntityInfo,
+    EntityState,
+    UserService,
+)
 import pytest
 
-from homeassistant.components.esphome.dashboard import async_get_dashboard
+from homeassistant.components.esphome.dashboard import (
+    async_get_dashboard,
+)
 from homeassistant.components.update import UpdateEntityFeature
-from homeassistant.const import STATE_ON, STATE_UNAVAILABLE
+from homeassistant.const import (
+    STATE_OFF,
+    STATE_ON,
+    STATE_UNAVAILABLE,
+    STATE_UNKNOWN,
+)
 from homeassistant.core import HomeAssistant
+from homeassistant.exceptions import HomeAssistantError
 from homeassistant.helpers.dispatcher import async_dispatcher_send
 
+from .conftest import MockESPHomeDevice
 
-@pytest.fixture(autouse=True)
+
+@pytest.fixture
 def stub_reconnect():
     """Stub reconnect."""
     with patch("homeassistant.components.esphome.manager.ReconnectLogic.start"):
@@ -30,7 +47,7 @@ def stub_reconnect():
                     "configuration": "test.yaml",
                 }
             ],
-            "on",
+            STATE_ON,
             {
                 "latest_version": "2023.2.0-dev",
                 "installed_version": "1.0.0",
@@ -44,7 +61,7 @@ def stub_reconnect():
                     "current_version": "1.0.0",
                 },
             ],
-            "off",
+            STATE_OFF,
             {
                 "latest_version": "1.0.0",
                 "installed_version": "1.0.0",
@@ -53,13 +70,14 @@ def stub_reconnect():
         ),
         (
             [],
-            "unavailable",
+            STATE_UNKNOWN,  # dashboard is available but device is unknown
             {"supported_features": 0},
         ),
     ],
 )
 async def test_update_entity(
     hass: HomeAssistant,
+    stub_reconnect,
     mock_config_entry,
     mock_device_info,
     mock_dashboard,
@@ -88,6 +106,48 @@ async def test_update_entity(
     if expected_state != "on":
         return
 
+    # Compile failed, don't try to upload
+    with patch(
+        "esphome_dashboard_api.ESPHomeDashboardAPI.compile", return_value=False
+    ) as mock_compile, patch(
+        "esphome_dashboard_api.ESPHomeDashboardAPI.upload", return_value=True
+    ) as mock_upload, pytest.raises(
+        HomeAssistantError, match="compiling"
+    ):
+        await hass.services.async_call(
+            "update",
+            "install",
+            {"entity_id": "update.none_firmware"},
+            blocking=True,
+        )
+
+    assert len(mock_compile.mock_calls) == 1
+    assert mock_compile.mock_calls[0][1][0] == "test.yaml"
+
+    assert len(mock_upload.mock_calls) == 0
+
+    # Compile success, upload fails
+    with patch(
+        "esphome_dashboard_api.ESPHomeDashboardAPI.compile", return_value=True
+    ) as mock_compile, patch(
+        "esphome_dashboard_api.ESPHomeDashboardAPI.upload", return_value=False
+    ) as mock_upload, pytest.raises(
+        HomeAssistantError, match="OTA"
+    ):
+        await hass.services.async_call(
+            "update",
+            "install",
+            {"entity_id": "update.none_firmware"},
+            blocking=True,
+        )
+
+    assert len(mock_compile.mock_calls) == 1
+    assert mock_compile.mock_calls[0][1][0] == "test.yaml"
+
+    assert len(mock_upload.mock_calls) == 1
+    assert mock_upload.mock_calls[0][1][0] == "test.yaml"
+
+    # Everything works
     with patch(
         "esphome_dashboard_api.ESPHomeDashboardAPI.compile", return_value=True
     ) as mock_compile, patch(
@@ -109,6 +169,7 @@ async def test_update_entity(
 
 async def test_update_static_info(
     hass: HomeAssistant,
+    stub_reconnect,
     mock_config_entry,
     mock_device_info,
     mock_dashboard,
@@ -155,6 +216,7 @@ async def test_update_static_info(
 )
 async def test_update_device_state_for_availability(
     hass: HomeAssistant,
+    stub_reconnect,
     expected_disconnect_state: tuple[bool, str],
     mock_config_entry,
     mock_device_info,
@@ -210,7 +272,11 @@ async def test_update_device_state_for_availability(
 
 
 async def test_update_entity_dashboard_not_available_startup(
-    hass: HomeAssistant, mock_config_entry, mock_device_info, mock_dashboard
+    hass: HomeAssistant,
+    stub_reconnect,
+    mock_config_entry,
+    mock_device_info,
+    mock_dashboard,
 ) -> None:
     """Test ESPHome update entity when dashboard is not available at startup."""
     with patch(
@@ -225,6 +291,7 @@ async def test_update_entity_dashboard_not_available_startup(
             mock_config_entry, "update"
         )
 
+    # We have a dashboard but it is not available
     state = hass.states.get("update.none_firmware")
     assert state is None
 
@@ -239,7 +306,7 @@ async def test_update_entity_dashboard_not_available_startup(
     await hass.async_block_till_done()
 
     state = hass.states.get("update.none_firmware")
-    assert state.state == "on"
+    assert state.state == STATE_ON
     expected_attributes = {
         "latest_version": "2023.2.0-dev",
         "installed_version": "1.0.0",
@@ -247,3 +314,69 @@ async def test_update_entity_dashboard_not_available_startup(
     }
     for key, expected_value in expected_attributes.items():
         assert state.attributes.get(key) == expected_value
+
+
+async def test_update_entity_dashboard_discovered_after_startup_but_update_failed(
+    hass: HomeAssistant,
+    mock_client: APIClient,
+    mock_esphome_device: Callable[
+        [APIClient, list[EntityInfo], list[UserService], list[EntityState]],
+        Awaitable[MockESPHomeDevice],
+    ],
+    mock_dashboard,
+) -> None:
+    """Test ESPHome update entity when dashboard is discovered after startup and the first update fails."""
+    with patch(
+        "esphome_dashboard_api.ESPHomeDashboardAPI.get_devices",
+        side_effect=asyncio.TimeoutError,
+    ):
+        await async_get_dashboard(hass).async_refresh()
+        await hass.async_block_till_done()
+        mock_device = await mock_esphome_device(
+            mock_client=mock_client,
+            entity_info=[],
+            user_service=[],
+            states=[],
+        )
+        await hass.async_block_till_done()
+    state = hass.states.get("update.test_firmware")
+    assert state is None
+
+    await mock_device.mock_disconnect(False)
+
+    mock_dashboard["configured"] = [
+        {
+            "name": "test",
+            "current_version": "2023.2.0-dev",
+            "configuration": "test.yaml",
+        }
+    ]
+    # Device goes unavailable, and dashboard becomes available
+    await async_get_dashboard(hass).async_refresh()
+    await hass.async_block_till_done()
+
+    state = hass.states.get("update.test_firmware")
+    assert state is None
+
+    # Finally both are available
+    await mock_device.mock_connect()
+    await async_get_dashboard(hass).async_refresh()
+    await hass.async_block_till_done()
+    state = hass.states.get("update.test_firmware")
+    assert state is not None
+
+
+async def test_update_entity_not_present_without_dashboard(
+    hass: HomeAssistant, stub_reconnect, mock_config_entry, mock_device_info
+) -> None:
+    """Test ESPHome update entity does not get created if there is no dashboard."""
+    with patch(
+        "homeassistant.components.esphome.update.DomainData.get_entry_data",
+        return_value=Mock(available=True, device_info=mock_device_info),
+    ):
+        assert await hass.config_entries.async_forward_entry_setup(
+            mock_config_entry, "update"
+        )
+
+    state = hass.states.get("update.none_firmware")
+    assert state is None