ISS integration: better entity handling (#159050)

Co-authored-by: Ariel Ebersberger <ariel@ebersberger.io>

homeassistant/components/iss/__init__.py

@@ -2,66 +2,21 @@
 
 from __future__ import annotations
 
-from dataclasses import dataclass
-from datetime import timedelta
-import logging
-
-import pyiss
-import requests
-from requests.exceptions import HTTPError
-
-from homeassistant.config_entries import ConfigEntry
 from homeassistant.const import Platform
 from homeassistant.core import HomeAssistant
-from homeassistant.helpers.update_coordinator import DataUpdateCoordinator, UpdateFailed
 
-from .const import DOMAIN
-
-_LOGGER = logging.getLogger(__name__)
+from .coordinator import IssConfigEntry, IssDataUpdateCoordinator
 
 PLATFORMS = [Platform.SENSOR]
 
 
-@dataclass
-class IssData:
-    """Dataclass representation of data returned from pyiss."""
-
-    number_of_people_in_space: int
-    current_location: dict[str, str]
-
-
-def update(iss: pyiss.ISS) -> IssData:
-    """Retrieve data from the pyiss API."""
-    return IssData(
-        number_of_people_in_space=iss.number_of_people_in_space(),
-        current_location=iss.current_location(),
-    )
-
-
-async def async_setup_entry(hass: HomeAssistant, entry: ConfigEntry) -> bool:
+async def async_setup_entry(hass: HomeAssistant, entry: IssConfigEntry) -> bool:
     """Set up this integration using UI."""
-    hass.data.setdefault(DOMAIN, {})
-
-    iss = pyiss.ISS()
-
-    async def async_update() -> IssData:
-        try:
-            return await hass.async_add_executor_job(update, iss)
-        except (HTTPError, requests.exceptions.ConnectionError) as ex:
-            raise UpdateFailed("Unable to retrieve data") from ex
-
-    coordinator = DataUpdateCoordinator(
-        hass,
-        _LOGGER,
-        config_entry=entry,
-        name=DOMAIN,
-        update_method=async_update,
-        update_interval=timedelta(seconds=60),
-    )
+    coordinator = IssDataUpdateCoordinator(hass, entry)
 
     await coordinator.async_config_entry_first_refresh()
 
-    hass.data[DOMAIN] = coordinator
+    entry.runtime_data = coordinator
 
     entry.async_on_unload(entry.add_update_listener(update_listener))
 
@@ -70,13 +25,11 @@ async def async_setup_entry(hass: HomeAssistant, entry: ConfigEntry) -> bool:
     return True
 
 
-async def async_unload_entry(hass: HomeAssistant, entry: ConfigEntry) -> bool:
+async def async_unload_entry(hass: HomeAssistant, entry: IssConfigEntry) -> bool:
     """Handle removal of an entry."""
-    if unload_ok := await hass.config_entries.async_unload_platforms(entry, PLATFORMS):
-        del hass.data[DOMAIN]
-    return unload_ok
+    return await hass.config_entries.async_unload_platforms(entry, PLATFORMS)
 
 
-async def update_listener(hass: HomeAssistant, entry: ConfigEntry) -> None:
+async def update_listener(hass: HomeAssistant, entry: IssConfigEntry) -> None:
     """Handle options update."""
     await hass.config_entries.async_reload(entry.entry_id)

homeassistant/components/iss/config_flow.py

@@ -4,16 +4,12 @@ from __future__ import annotations
 
 import voluptuous as vol
 
-from homeassistant.config_entries import (
-    ConfigEntry,
-    ConfigFlow,
-    ConfigFlowResult,
-    OptionsFlow,
-)
+from homeassistant.config_entries import ConfigFlow, ConfigFlowResult, OptionsFlow
 from homeassistant.const import CONF_SHOW_ON_MAP
 from homeassistant.core import callback
 
 from .const import DEFAULT_NAME, DOMAIN
+from .coordinator import IssConfigEntry
 
 
 class ISSConfigFlow(ConfigFlow, domain=DOMAIN):
@@ -24,7 +20,7 @@ class ISSConfigFlow(ConfigFlow, domain=DOMAIN):
     @staticmethod
     @callback
     def async_get_options_flow(
-        config_entry: ConfigEntry,
+        config_entry: IssConfigEntry,
     ) -> OptionsFlowHandler:
         """Get the options flow for this handler."""
         return OptionsFlowHandler()

homeassistant/components/iss/const.py

@@ -3,3 +3,5 @@
 DOMAIN = "iss"
 
 DEFAULT_NAME = "ISS"
+
+MAX_CONSECUTIVE_FAILURES = 5

homeassistant/components/iss/coordinator.py

@@ -0,0 +1,76 @@
+"""DataUpdateCoordinator for the ISS integration."""
+
+from __future__ import annotations
+
+from dataclasses import dataclass
+from datetime import timedelta
+import logging
+
+import pyiss
+import requests
+from requests.exceptions import HTTPError
+
+from homeassistant.config_entries import ConfigEntry
+from homeassistant.core import HomeAssistant
+from homeassistant.helpers.update_coordinator import DataUpdateCoordinator, UpdateFailed
+
+from .const import DOMAIN, MAX_CONSECUTIVE_FAILURES
+
+type IssConfigEntry = ConfigEntry[IssDataUpdateCoordinator]
+
+_LOGGER = logging.getLogger(__name__)
+
+
+@dataclass
+class IssData:
+    """Dataclass representation of data returned from pyiss."""
+
+    number_of_people_in_space: int
+    current_location: dict[str, str]
+
+
+class IssDataUpdateCoordinator(DataUpdateCoordinator[IssData]):
+    """ISS coordinator that tolerates transient API failures."""
+
+    config_entry: IssConfigEntry
+
+    def __init__(self, hass: HomeAssistant, entry: IssConfigEntry) -> None:
+        """Initialize the ISS coordinator."""
+        super().__init__(
+            hass,
+            _LOGGER,
+            config_entry=entry,
+            name=DOMAIN,
+            update_interval=timedelta(seconds=60),
+        )
+        self._consecutive_failures = 0
+        self.iss = pyiss.ISS()
+
+    def _fetch_iss_data(self) -> IssData:
+        """Fetch data from ISS API (blocking)."""
+        return IssData(
+            number_of_people_in_space=self.iss.number_of_people_in_space(),
+            current_location=self.iss.current_location(),
+        )
+
+    async def _async_update_data(self) -> IssData:
+        """Fetch data from the ISS API, tolerating transient failures."""
+        try:
+            data = await self.hass.async_add_executor_job(self._fetch_iss_data)
+        except (HTTPError, requests.exceptions.ConnectionError) as err:
+            self._consecutive_failures += 1
+            if self.data is None:
+                raise UpdateFailed("Unable to retrieve data") from err
+            if self._consecutive_failures >= MAX_CONSECUTIVE_FAILURES:
+                raise UpdateFailed(
+                    f"Unable to retrieve data after {self._consecutive_failures} consecutive update failures"
+                ) from err
+            _LOGGER.debug(
+                "Transient API error (%s/%s), using cached data: %s",
+                self._consecutive_failures,
+                MAX_CONSECUTIVE_FAILURES,
+                err,
+            )
+            return self.data
+        self._consecutive_failures = 0
+        return data

homeassistant/components/iss/sensor.py

@@ -6,36 +6,32 @@ import logging
 from typing import Any
 
 from homeassistant.components.sensor import SensorEntity
-from homeassistant.config_entries import ConfigEntry
 from homeassistant.const import ATTR_LATITUDE, ATTR_LONGITUDE, CONF_SHOW_ON_MAP
 from homeassistant.core import HomeAssistant
 from homeassistant.helpers.device_registry import DeviceEntryType, DeviceInfo
 from homeassistant.helpers.entity_platform import AddConfigEntryEntitiesCallback
-from homeassistant.helpers.update_coordinator import (
-    CoordinatorEntity,
-    DataUpdateCoordinator,
-)
+from homeassistant.helpers.update_coordinator import CoordinatorEntity
 
-from . import IssData
 from .const import DEFAULT_NAME, DOMAIN
+from .coordinator import IssConfigEntry, IssDataUpdateCoordinator
 
 _LOGGER = logging.getLogger(__name__)
 
 
 async def async_setup_entry(
     hass: HomeAssistant,
-    entry: ConfigEntry,
+    entry: IssConfigEntry,
     async_add_entities: AddConfigEntryEntitiesCallback,
 ) -> None:
     """Set up the sensor platform."""
-    coordinator: DataUpdateCoordinator[IssData] = hass.data[DOMAIN]
+    coordinator = entry.runtime_data
 
     show_on_map = entry.options.get(CONF_SHOW_ON_MAP, False)
 
     async_add_entities([IssSensor(coordinator, entry, show_on_map)])
 
 
-class IssSensor(CoordinatorEntity[DataUpdateCoordinator[IssData]], SensorEntity):
+class IssSensor(CoordinatorEntity[IssDataUpdateCoordinator], SensorEntity):
     """Implementation of the ISS sensor."""
 
     _attr_has_entity_name = True
@@ -43,8 +39,8 @@ class IssSensor(CoordinatorEntity[DataUpdateCoordinator[IssData]], SensorEntity)
 
     def __init__(
         self,
-        coordinator: DataUpdateCoordinator[IssData],
-        entry: ConfigEntry,
+        coordinator: IssDataUpdateCoordinator,
+        entry: IssConfigEntry,
         show: bool,
     ) -> None:
         """Initialize the sensor."""

tests/components/iss/conftest.py

@@ -0,0 +1,47 @@
+"""Configuration for ISS tests."""
+
+from collections.abc import Generator
+from unittest.mock import MagicMock, patch
+
+import pytest
+
+from homeassistant.components.iss.const import DOMAIN
+from homeassistant.core import HomeAssistant
+
+from tests.common import MockConfigEntry
+
+
+@pytest.fixture
+def mock_config_entry() -> MockConfigEntry:
+    """Return a mock config entry."""
+    return MockConfigEntry(
+        domain=DOMAIN,
+        data={},
+        options={},
+        entry_id="test_entry_id",
+    )
+
+
+@pytest.fixture
+def mock_pyiss() -> Generator[MagicMock]:
+    """Mock the pyiss.ISS class."""
+    with patch("homeassistant.components.iss.coordinator.pyiss.ISS") as mock_iss_class:
+        mock_iss = MagicMock()
+        mock_iss.number_of_people_in_space.return_value = 7
+        mock_iss.current_location.return_value = {
+            "latitude": "40.271698",
+            "longitude": "15.619478",
+        }
+        mock_iss_class.return_value = mock_iss
+        yield mock_iss
+
+
+@pytest.fixture
+async def init_integration(
+    hass: HomeAssistant, mock_config_entry: MockConfigEntry, mock_pyiss: MagicMock
+) -> MockConfigEntry:
+    """Set up the ISS integration for testing."""
+    mock_config_entry.add_to_hass(hass)
+    await hass.config_entries.async_setup(mock_config_entry.entry_id)
+    await hass.async_block_till_done()
+    return mock_config_entry

tests/components/iss/test_init.py

@@ -0,0 +1,188 @@
+"""Test the ISS integration setup and coordinator."""
+
+from unittest.mock import MagicMock
+
+from requests.exceptions import ConnectionError as RequestsConnectionError, HTTPError
+
+from homeassistant.components.iss.const import MAX_CONSECUTIVE_FAILURES
+from homeassistant.config_entries import ConfigEntryState
+from homeassistant.const import ATTR_LATITUDE, ATTR_LONGITUDE, CONF_SHOW_ON_MAP
+from homeassistant.core import HomeAssistant
+from homeassistant.helpers.update_coordinator import UpdateFailed
+
+from tests.common import MockConfigEntry
+
+
+async def test_setup_entry(
+    hass: HomeAssistant, init_integration: MockConfigEntry
+) -> None:
+    """Test successful setup of config entry."""
+    assert init_integration.state is ConfigEntryState.LOADED
+    coordinator = init_integration.runtime_data
+    assert coordinator.data is not None
+    assert coordinator.data.number_of_people_in_space == 7
+    assert coordinator.data.current_location == {
+        "latitude": "40.271698",
+        "longitude": "15.619478",
+    }
+
+
+async def test_unload_entry(
+    hass: HomeAssistant, init_integration: MockConfigEntry
+) -> None:
+    """Test unload of config entry."""
+    assert init_integration.state is ConfigEntryState.LOADED
+
+    await hass.config_entries.async_unload(init_integration.entry_id)
+    await hass.async_block_till_done()
+
+    assert init_integration.state is ConfigEntryState.NOT_LOADED
+
+
+async def test_update_listener(
+    hass: HomeAssistant, init_integration: MockConfigEntry, mock_pyiss: MagicMock
+) -> None:
+    """Test options update triggers reload and applies new options."""
+    state = hass.states.get("sensor.iss")
+    assert state is not None
+    assert "lat" in state.attributes
+    assert "long" in state.attributes
+    assert ATTR_LATITUDE not in state.attributes
+    assert ATTR_LONGITUDE not in state.attributes
+
+    hass.config_entries.async_update_entry(
+        init_integration, options={CONF_SHOW_ON_MAP: True}
+    )
+    await hass.async_block_till_done()
+
+    # After reload with show_on_map=True, attributes should switch
+    state = hass.states.get("sensor.iss")
+    assert state is not None
+    assert ATTR_LATITUDE in state.attributes
+    assert ATTR_LONGITUDE in state.attributes
+    assert "lat" not in state.attributes
+    assert "long" not in state.attributes
+
+
+async def test_coordinator_single_failure_uses_cached_data(
+    hass: HomeAssistant, init_integration: MockConfigEntry, mock_pyiss: MagicMock
+) -> None:
+    """Test coordinator tolerates single API failure and uses cached data."""
+    coordinator = init_integration.runtime_data
+    original_data = coordinator.data
+
+    # Simulate API failure
+    mock_pyiss.number_of_people_in_space.side_effect = HTTPError("API Error")
+
+    await coordinator.async_refresh()
+    await hass.async_block_till_done()
+
+    # Should still have the cached data
+    assert coordinator.data == original_data
+    assert coordinator.last_update_success is True
+
+
+async def test_coordinator_multiple_failures_uses_cached_data(
+    hass: HomeAssistant, init_integration: MockConfigEntry, mock_pyiss: MagicMock
+) -> None:
+    """Test coordinator tolerates multiple failures below threshold."""
+    coordinator = init_integration.runtime_data
+    original_data = coordinator.data
+
+    # Simulate multiple API failures (below MAX_CONSECUTIVE_FAILURES)
+    mock_pyiss.number_of_people_in_space.side_effect = RequestsConnectionError(
+        "Connection failed"
+    )
+
+    for _ in range(MAX_CONSECUTIVE_FAILURES - 1):
+        await coordinator.async_refresh()
+        await hass.async_block_till_done()
+
+    # Should still have cached data and be successful
+    assert coordinator.data == original_data
+    assert coordinator.last_update_success is True
+
+
+async def test_coordinator_max_failures_marks_unavailable(
+    hass: HomeAssistant, init_integration: MockConfigEntry, mock_pyiss: MagicMock
+) -> None:
+    """Test coordinator marks update failed after MAX_CONSECUTIVE_FAILURES."""
+    coordinator = init_integration.runtime_data
+
+    # Simulate consecutive API failures reaching the threshold
+    mock_pyiss.number_of_people_in_space.side_effect = HTTPError("API Error")
+
+    for _ in range(MAX_CONSECUTIVE_FAILURES):
+        await coordinator.async_refresh()
+        await hass.async_block_till_done()
+
+    # After MAX_CONSECUTIVE_FAILURES, update should be marked as failed
+    assert coordinator.last_update_success is False
+    assert isinstance(coordinator.last_exception, UpdateFailed)
+
+
+async def test_coordinator_failure_counter_resets_on_success(
+    hass: HomeAssistant, init_integration: MockConfigEntry, mock_pyiss: MagicMock
+) -> None:
+    """Test coordinator resets failure counter after successful fetch."""
+    coordinator = init_integration.runtime_data
+
+    # Simulate some failures
+    mock_pyiss.number_of_people_in_space.side_effect = HTTPError("API Error")
+    for _ in range(2):
+        await coordinator.async_refresh()
+        await hass.async_block_till_done()
+
+    # Now simulate success
+    mock_pyiss.number_of_people_in_space.side_effect = None
+    mock_pyiss.number_of_people_in_space.return_value = 8
+    await coordinator.async_refresh()
+    await hass.async_block_till_done()
+
+    assert coordinator.last_update_success is True
+    assert coordinator.data.number_of_people_in_space == 8
+
+    # Failure counter should be reset, so we can tolerate failures again
+    mock_pyiss.number_of_people_in_space.side_effect = RequestsConnectionError(
+        "Connection failed"
+    )
+    for _ in range(MAX_CONSECUTIVE_FAILURES - 1):
+        await coordinator.async_refresh()
+        await hass.async_block_till_done()
+
+    # Should still be successful due to cached data
+    assert coordinator.last_update_success is True
+
+
+async def test_coordinator_initial_failure_no_cached_data(
+    hass: HomeAssistant, mock_config_entry: MockConfigEntry, mock_pyiss: MagicMock
+) -> None:
+    """Test coordinator fails immediately on initial setup with no cached data."""
+    mock_pyiss.number_of_people_in_space.side_effect = HTTPError("API Error")
+    mock_config_entry.add_to_hass(hass)
+
+    # Setup should fail because there's no cached data
+    await hass.config_entries.async_setup(mock_config_entry.entry_id)
+    await hass.async_block_till_done()
+
+    assert mock_config_entry.state is ConfigEntryState.SETUP_RETRY
+
+
+async def test_coordinator_handles_connection_error(
+    hass: HomeAssistant, init_integration: MockConfigEntry, mock_pyiss: MagicMock
+) -> None:
+    """Test coordinator handles ConnectionError exceptions."""
+    coordinator = init_integration.runtime_data
+    original_data = coordinator.data
+
+    # Simulate ConnectionError
+    mock_pyiss.current_location.side_effect = RequestsConnectionError(
+        "Network unreachable"
+    )
+
+    await coordinator.async_refresh()
+    await hass.async_block_till_done()
+
+    # Should use cached data
+    assert coordinator.data == original_data
+    assert coordinator.last_update_success is True

tests/components/iss/test_sensor.py

@@ -0,0 +1,99 @@
+"""Test the ISS sensor platform."""
+
+from unittest.mock import MagicMock
+
+from homeassistant.components.iss.const import DEFAULT_NAME, DOMAIN
+from homeassistant.const import ATTR_LATITUDE, ATTR_LONGITUDE, CONF_SHOW_ON_MAP
+from homeassistant.core import HomeAssistant
+from homeassistant.helpers import device_registry as dr, entity_registry as er
+
+from tests.common import MockConfigEntry
+
+
+async def test_sensor_created(
+    hass: HomeAssistant, init_integration: MockConfigEntry
+) -> None:
+    """Test sensor entity is created."""
+    state = hass.states.get("sensor.iss")
+    assert state is not None
+    assert state.state == "7"
+
+
+async def test_sensor_attributes_show_on_map_false(
+    hass: HomeAssistant, init_integration: MockConfigEntry
+) -> None:
+    """Test sensor attributes when show_on_map is False."""
+    state = hass.states.get("sensor.iss")
+    assert state is not None
+    assert state.state == "7"
+    assert state.attributes["lat"] == "40.271698"
+    assert state.attributes["long"] == "15.619478"
+    # Should NOT have ATTR_LATITUDE/ATTR_LONGITUDE when show_on_map is False
+    assert ATTR_LATITUDE not in state.attributes
+    assert ATTR_LONGITUDE not in state.attributes
+
+
+async def test_sensor_attributes_show_on_map_true(
+    hass: HomeAssistant, mock_config_entry: MockConfigEntry, mock_pyiss: MagicMock
+) -> None:
+    """Test sensor attributes when show_on_map is True."""
+    mock_config_entry.add_to_hass(hass)
+    hass.config_entries.async_update_entry(
+        mock_config_entry, options={CONF_SHOW_ON_MAP: True}
+    )
+    await hass.config_entries.async_setup(mock_config_entry.entry_id)
+    await hass.async_block_till_done()
+
+    state = hass.states.get("sensor.iss")
+    assert state is not None
+    assert state.state == "7"
+    # Should have ATTR_LATITUDE/ATTR_LONGITUDE when show_on_map is True
+    assert state.attributes[ATTR_LATITUDE] == "40.271698"
+    assert state.attributes[ATTR_LONGITUDE] == "15.619478"
+    # Should NOT have lat/long keys
+    assert "lat" not in state.attributes
+    assert "long" not in state.attributes
+
+
+async def test_sensor_device_info(
+    hass: HomeAssistant, init_integration: MockConfigEntry
+) -> None:
+    """Test sensor has correct device info."""
+    entity_registry = er.async_get(hass)
+    entity = entity_registry.async_get("sensor.iss")
+
+    assert entity is not None
+    assert entity.unique_id == f"{init_integration.entry_id}_people"
+
+    device_registry = dr.async_get(hass)
+    device = device_registry.async_get(entity.device_id)
+
+    assert device is not None
+    assert device.name == DEFAULT_NAME
+    assert (DOMAIN, init_integration.entry_id) in device.identifiers
+
+
+async def test_sensor_updates_with_coordinator(
+    hass: HomeAssistant, init_integration: MockConfigEntry, mock_pyiss: MagicMock
+) -> None:
+    """Test sensor updates when coordinator data changes."""
+    state = hass.states.get("sensor.iss")
+    assert state.state == "7"
+
+    # Update mock data
+    mock_pyiss.number_of_people_in_space.return_value = 10
+    mock_pyiss.current_location.return_value = {
+        "latitude": "50.0",
+        "longitude": "-100.0",
+    }
+
+    # Trigger coordinator refresh
+    coordinator = init_integration.runtime_data
+    await coordinator.async_refresh()
+    await hass.async_block_till_done()
+
+    # Check sensor updated
+    state = hass.states.get("sensor.iss")
+    assert state.state == "10"
+    assert state.attributes["lat"] == "50.0"
+    assert state.attributes["long"] == "-100.0"