Filter out invalid trackers in fressnapf_tracker (#161670)

homeassistant/components/fressnapf_tracker/__init__.py

@@ -1,11 +1,22 @@
 """The Fressnapf Tracker integration."""
 
-from fressnapftracker import AuthClient, FressnapfTrackerAuthenticationError
+import logging
+
+from fressnapftracker import (
+    ApiClient,
+    AuthClient,
+    Device,
+    FressnapfTrackerAuthenticationError,
+    FressnapfTrackerError,
+    FressnapfTrackerInvalidTrackerResponseError,
+    Tracker,
+)
 
 from homeassistant.const import CONF_ACCESS_TOKEN, Platform
 from homeassistant.core import HomeAssistant
-from homeassistant.exceptions import ConfigEntryAuthFailed
+from homeassistant.exceptions import ConfigEntryAuthFailed, ConfigEntryNotReady
 from homeassistant.helpers.httpx_client import get_async_client
+from homeassistant.helpers.issue_registry import IssueSeverity, async_create_issue
 
 from .const import CONF_USER_ID, DOMAIN
 from .coordinator import (
@@ -21,6 +32,43 @@ PLATFORMS: list[Platform] = [
     Platform.SWITCH,
 ]
 
+_LOGGER = logging.getLogger(__name__)
+
+
+async def _get_valid_tracker(hass: HomeAssistant, device: Device) -> Tracker | None:
+    """Test if the tracker returns valid data and return it.
+
+    Malformed data might indicate the tracker is broken or hasn't been properly registered with the app.
+    """
+    client = ApiClient(
+        serial_number=device.serialnumber,
+        device_token=device.token,
+        client=get_async_client(hass),
+    )
+    try:
+        return await client.get_tracker()
+    except FressnapfTrackerInvalidTrackerResponseError:
+        _LOGGER.warning(
+            "Tracker with serialnumber %s is invalid. Consider removing it via the App",
+            device.serialnumber,
+        )
+        async_create_issue(
+            hass,
+            DOMAIN,
+            f"invalid_fressnapf_tracker_{device.serialnumber}",
+            issue_domain=DOMAIN,
+            learn_more_url="https://www.home-assistant.io/integrations/fressnapf_tracker/",
+            is_fixable=False,
+            severity=IssueSeverity.WARNING,
+            translation_key="invalid_fressnapf_tracker",
+            translation_placeholders={
+                "tracker_id": device.serialnumber,
+            },
+        )
+        return None
+    except FressnapfTrackerError as err:
+        raise ConfigEntryNotReady(err) from err
+
 
 async def async_setup_entry(
     hass: HomeAssistant, entry: FressnapfTrackerConfigEntry
@@ -40,12 +88,15 @@ async def async_setup_entry(
 
     coordinators: list[FressnapfTrackerDataUpdateCoordinator] = []
     for device in devices:
+        tracker = await _get_valid_tracker(hass, device)
+        if tracker is None:
+            continue
         coordinator = FressnapfTrackerDataUpdateCoordinator(
             hass,
             entry,
             device,
+            initial_data=tracker,
         )
-        await coordinator.async_config_entry_first_refresh()
         coordinators.append(coordinator)
 
     entry.runtime_data = coordinators

homeassistant/components/fressnapf_tracker/coordinator.py

@@ -34,6 +34,7 @@ class FressnapfTrackerDataUpdateCoordinator(DataUpdateCoordinator[Tracker]):
         hass: HomeAssistant,
         config_entry: FressnapfTrackerConfigEntry,
         device: Device,
+        initial_data: Tracker,
     ) -> None:
         """Initialize."""
         super().__init__(
@@ -49,6 +50,7 @@ class FressnapfTrackerDataUpdateCoordinator(DataUpdateCoordinator[Tracker]):
             device_token=device.token,
             client=get_async_client(hass),
         )
+        self.data = initial_data
 
     async def _async_update_data(self) -> Tracker:
         try:

homeassistant/components/fressnapf_tracker/strings.json

@@ -92,5 +92,11 @@
     "not_seen_recently": {
       "message": "The flashlight cannot be activated when the tracker has not moved recently."
     }
+  },
+  "issues": {
+    "invalid_fressnapf_tracker": {
+      "description": "The Fressnapf GPS tracker with the serial number `{tracker_id}` is sending invalid data. This most likely indicates the tracker is broken or hasn't been properly registered with the app. Please remove the tracker via the Fressnapf app. You can then close this issue.",
+      "title": "Invalid Fressnapf GPS tracker detected"
+    }
   }
 }

tests/components/fressnapf_tracker/conftest.py

@@ -35,6 +35,38 @@ MOCK_SERIAL_NUMBER = "ABC123456"
 MOCK_DEVICE_TOKEN = "mock_device_token"
 
 
+def create_mock_tracker() -> Tracker:
+    """Create a fresh mock Tracker instance."""
+    return Tracker(
+        name="Fluffy",
+        battery=85,
+        charging=False,
+        position=Position(
+            lat=52.520008,
+            lng=13.404954,
+            accuracy=10,
+            timestamp="2024-01-15T12:00:00Z",
+        ),
+        tracker_settings=TrackerSettings(
+            generation="GPS Tracker 2.0",
+            features=TrackerFeatures(
+                flash_light=True, energy_saving_mode=True, live_tracking=True
+            ),
+        ),
+        led_brightness=LedBrightness(status="ok", value=50),
+        energy_saving=EnergySaving(status="ok", value=1),
+        deep_sleep=None,
+        led_activatable=LedActivatable(
+            has_led=True,
+            seen_recently=True,
+            nonempty_battery=True,
+            not_charging=True,
+            overall=True,
+        ),
+        icon="http://res.cloudinary.com/iot-venture/image/upload/v1717594357/kyaqq7nfitrdvaoakb8s.jpg",
+    )
+
+
 @pytest.fixture
 def mock_setup_entry() -> Generator[AsyncMock]:
     """Override async_setup_entry."""
@@ -102,42 +134,26 @@ def mock_auth_client(mock_device: Device) -> Generator[MagicMock]:
 
 
 @pytest.fixture
-def mock_api_client() -> Generator[MagicMock]:
-    """Mock the ApiClient."""
+def mock_api_client_init() -> Generator[MagicMock]:
+    """Mock the ApiClient used by _tracker_is_valid in __init__.py."""
     with patch(
-        "homeassistant.components.fressnapf_tracker.coordinator.ApiClient"
-    ) as mock_api_client:
-        client = mock_api_client.return_value
-        client.get_tracker = AsyncMock(
-            return_value=Tracker(
-                name="Fluffy",
-                battery=85,
-                charging=False,
-                position=Position(
-                    lat=52.520008,
-                    lng=13.404954,
-                    accuracy=10,
-                    timestamp="2024-01-15T12:00:00Z",
-                ),
-                tracker_settings=TrackerSettings(
-                    generation="GPS Tracker 2.0",
-                    features=TrackerFeatures(
-                        flash_light=True, energy_saving_mode=True, live_tracking=True
-                    ),
-                ),
-                led_brightness=LedBrightness(status="ok", value=50),
-                energy_saving=EnergySaving(status="ok", value=1),
-                deep_sleep=None,
-                led_activatable=LedActivatable(
-                    has_led=True,
-                    seen_recently=True,
-                    nonempty_battery=True,
-                    not_charging=True,
-                    overall=True,
-                ),
-                icon="http://res.cloudinary.com/iot-venture/image/upload/v1717594357/kyaqq7nfitrdvaoakb8s.jpg",
-            )
-        )
+        "homeassistant.components.fressnapf_tracker.ApiClient",
+        autospec=True,
+    ) as mock_client:
+        client = mock_client.return_value
+        client.get_tracker = AsyncMock(return_value=create_mock_tracker())
+        yield client
+
+
+@pytest.fixture
+def mock_api_client_coordinator() -> Generator[MagicMock]:
+    """Mock the ApiClient used by the coordinator."""
+    with patch(
+        "homeassistant.components.fressnapf_tracker.coordinator.ApiClient",
+        autospec=True,
+    ) as mock_client:
+        client = mock_client.return_value
+        client.get_tracker = AsyncMock(return_value=create_mock_tracker())
         client.set_led_brightness = AsyncMock(return_value=None)
         client.set_energy_saving = AsyncMock(return_value=None)
         yield client
@@ -162,7 +178,8 @@ def mock_config_entry() -> MockConfigEntry:
 async def init_integration(
     hass: HomeAssistant,
     mock_config_entry: MockConfigEntry,
-    mock_api_client: MagicMock,
+    mock_api_client_init: MagicMock,
+    mock_api_client_coordinator: MagicMock,
     mock_auth_client: MagicMock,
 ) -> MockConfigEntry:
     """Set up the integration for testing."""

tests/components/fressnapf_tracker/test_config_flow.py

@@ -216,7 +216,9 @@ async def test_user_flow_duplicate_phone_number(
         ),
     ],
 )
-@pytest.mark.usefixtures("mock_api_client", "mock_auth_client")
+@pytest.mark.usefixtures(
+    "mock_api_client_init", "mock_api_client_coordinator", "mock_auth_client"
+)
 async def test_reauth_reconfigure_flow(
     hass: HomeAssistant,
     mock_config_entry: MockConfigEntry,
@@ -270,7 +272,7 @@ async def test_reauth_reconfigure_flow(
         ),
     ],
 )
-@pytest.mark.usefixtures("mock_api_client")
+@pytest.mark.usefixtures("mock_api_client_init", "mock_api_client_coordinator")
 async def test_reauth_reconfigure_flow_invalid_phone_number(
     hass: HomeAssistant,
     mock_auth_client: MagicMock,
@@ -333,7 +335,7 @@ async def test_reauth_reconfigure_flow_invalid_phone_number(
         ),
     ],
 )
-@pytest.mark.usefixtures("mock_api_client")
+@pytest.mark.usefixtures("mock_api_client_init", "mock_api_client_coordinator")
 async def test_reauth_reconfigure_flow_invalid_sms_code(
     hass: HomeAssistant,
     mock_auth_client: MagicMock,
@@ -393,7 +395,7 @@ async def test_reauth_reconfigure_flow_invalid_sms_code(
         ),
     ],
 )
-@pytest.mark.usefixtures("mock_api_client")
+@pytest.mark.usefixtures("mock_api_client_init", "mock_api_client_coordinator")
 async def test_reauth_reconfigure_flow_invalid_user_id(
     hass: HomeAssistant,
     mock_auth_client: MagicMock,

tests/components/fressnapf_tracker/test_device_tracker.py

@@ -40,12 +40,12 @@ async def test_device_tracker_no_position(
     hass: HomeAssistant,
     mock_config_entry: MockConfigEntry,
     mock_tracker_no_position: Tracker,
-    mock_api_client: MagicMock,
+    mock_api_client_init: MagicMock,
 ) -> None:
     """Test device tracker is unavailable when position is None."""
     mock_config_entry.add_to_hass(hass)
 
-    mock_api_client.get_tracker = AsyncMock(return_value=mock_tracker_no_position)
+    mock_api_client_init.get_tracker = AsyncMock(return_value=mock_tracker_no_position)
     await hass.config_entries.async_setup(mock_config_entry.entry_id)
     await hass.async_block_till_done()
 

tests/components/fressnapf_tracker/test_init.py

@@ -1,19 +1,40 @@
 """Test the Fressnapf Tracker integration init."""
 
-from unittest.mock import AsyncMock, MagicMock
+from collections.abc import Generator
+from unittest.mock import AsyncMock, MagicMock, patch
 
+from fressnapftracker import (
+    FressnapfTrackerError,
+    FressnapfTrackerInvalidTrackerResponseError,
+)
 import pytest
 from syrupy.assertion import SnapshotAssertion
 
+from homeassistant.components.fressnapf_tracker.const import DOMAIN
 from homeassistant.config_entries import ConfigEntryState
 from homeassistant.core import HomeAssistant
-from homeassistant.helpers import device_registry as dr
+from homeassistant.helpers import device_registry as dr, issue_registry as ir
+
+from .conftest import MOCK_SERIAL_NUMBER
 
 from tests.common import MockConfigEntry
 
 
-@pytest.mark.usefixtures("mock_auth_client")
-@pytest.mark.usefixtures("mock_api_client")
+@pytest.fixture
+def mock_api_client_malformed_tracker() -> Generator[MagicMock]:
+    """Mock the ApiClient for a malformed tracker response in _tracker_is_valid."""
+    with patch(
+        "homeassistant.components.fressnapf_tracker.ApiClient",
+        autospec=True,
+    ) as mock_api_client:
+        client = mock_api_client.return_value
+        client.get_tracker = AsyncMock(
+            side_effect=FressnapfTrackerInvalidTrackerResponseError("Invalid tracker")
+        )
+        yield client
+
+
+@pytest.mark.usefixtures("mock_auth_client", "mock_api_client_init")
 async def test_setup_entry(
     hass: HomeAssistant,
     mock_config_entry: MockConfigEntry,
@@ -27,8 +48,7 @@ async def test_setup_entry(
     assert mock_config_entry.state is ConfigEntryState.LOADED
 
 
-@pytest.mark.usefixtures("mock_auth_client")
-@pytest.mark.usefixtures("mock_api_client")
+@pytest.mark.usefixtures("mock_auth_client", "mock_api_client_init")
 async def test_unload_entry(
     hass: HomeAssistant,
     mock_config_entry: MockConfigEntry,
@@ -48,15 +68,18 @@ async def test_unload_entry(
 
 
 @pytest.mark.usefixtures("mock_auth_client")
-async def test_setup_entry_api_error(
+async def test_setup_entry_tracker_is_valid_api_error(
     hass: HomeAssistant,
     mock_config_entry: MockConfigEntry,
-    mock_api_client: MagicMock,
+    mock_api_client_init: MagicMock,
 ) -> None:
-    """Test setup fails when API returns error."""
+    """Test setup retries when API returns error during _tracker_is_valid."""
     mock_config_entry.add_to_hass(hass)
 
-    mock_api_client.get_tracker = AsyncMock(side_effect=Exception("API Error"))
+    mock_api_client_init.get_tracker = AsyncMock(
+        side_effect=FressnapfTrackerError("API Error")
+    )
+
     await hass.config_entries.async_setup(mock_config_entry.entry_id)
     await hass.async_block_till_done()
 
@@ -78,3 +101,48 @@ async def test_state_entity_device_snapshots(
         assert device_entry == snapshot(name=f"{device_entry.name}-entry"), (
             f"device entry snapshot failed for {device_entry.name}"
         )
+
+
+@pytest.mark.usefixtures("mock_auth_client", "mock_api_client_malformed_tracker")
+async def test_invalid_tracker(
+    hass: HomeAssistant,
+    mock_config_entry: MockConfigEntry,
+    issue_registry: ir.IssueRegistry,
+) -> None:
+    """Test that an issue is created when an invalid tracker is detected."""
+    mock_config_entry.add_to_hass(hass)
+
+    await hass.config_entries.async_setup(mock_config_entry.entry_id)
+    await hass.async_block_till_done()
+
+    assert mock_config_entry.state is ConfigEntryState.LOADED
+    assert len(issue_registry.issues) == 1
+
+    issue_id = f"invalid_fressnapf_tracker_{MOCK_SERIAL_NUMBER}"
+    assert issue_registry.async_get_issue(DOMAIN, issue_id)
+
+
+@pytest.mark.usefixtures("mock_auth_client", "mock_api_client_malformed_tracker")
+async def test_invalid_tracker_already_exists(
+    hass: HomeAssistant,
+    mock_config_entry: MockConfigEntry,
+    issue_registry: ir.IssueRegistry,
+) -> None:
+    """Test that an existing issue is not duplicated."""
+    ir.async_create_issue(
+        hass,
+        DOMAIN,
+        f"invalid_fressnapf_tracker_{MOCK_SERIAL_NUMBER}",
+        is_fixable=False,
+        severity=ir.IssueSeverity.WARNING,
+        translation_key="invalid_fressnapf_tracker",
+        translation_placeholders={"tracker_id": MOCK_SERIAL_NUMBER},
+    )
+
+    mock_config_entry.add_to_hass(hass)
+
+    await hass.config_entries.async_setup(mock_config_entry.entry_id)
+    await hass.async_block_till_done()
+
+    assert mock_config_entry.state is ConfigEntryState.LOADED
+    assert len(issue_registry.issues) == 1

tests/components/fressnapf_tracker/test_light.py

@@ -63,10 +63,10 @@ async def test_not_added_when_no_led(
     hass: HomeAssistant,
     entity_registry: er.EntityRegistry,
     mock_config_entry: MockConfigEntry,
-    mock_api_client: MagicMock,
+    mock_api_client_init: MagicMock,
 ) -> None:
     """Test light entity is created correctly."""
-    mock_api_client.get_tracker.return_value = TRACKER_NO_LED
+    mock_api_client_init.get_tracker.return_value = TRACKER_NO_LED
 
     mock_config_entry.add_to_hass(hass)
     await hass.config_entries.async_setup(mock_config_entry.entry_id)
@@ -81,7 +81,7 @@ async def test_not_added_when_no_led(
 @pytest.mark.usefixtures("init_integration")
 async def test_turn_on(
     hass: HomeAssistant,
-    mock_api_client: MagicMock,
+    mock_api_client_coordinator: MagicMock,
 ) -> None:
     """Test turning the light on."""
     entity_id = "light.fluffy_flashlight"
@@ -97,13 +97,13 @@ async def test_turn_on(
         blocking=True,
     )
 
-    mock_api_client.set_led_brightness.assert_called_once_with(100)
+    mock_api_client_coordinator.set_led_brightness.assert_called_once_with(100)
 
 
 @pytest.mark.usefixtures("init_integration")
 async def test_turn_on_with_brightness(
     hass: HomeAssistant,
-    mock_api_client: MagicMock,
+    mock_api_client_coordinator: MagicMock,
 ) -> None:
     """Test turning the light on with brightness."""
     entity_id = "light.fluffy_flashlight"
@@ -116,13 +116,13 @@ async def test_turn_on_with_brightness(
     )
 
     # 128/255 * 100 = 50
-    mock_api_client.set_led_brightness.assert_called_once_with(50)
+    mock_api_client_coordinator.set_led_brightness.assert_called_once_with(50)
 
 
 @pytest.mark.usefixtures("init_integration")
 async def test_turn_off(
     hass: HomeAssistant,
-    mock_api_client: MagicMock,
+    mock_api_client_coordinator: MagicMock,
 ) -> None:
     """Test turning the light off."""
     entity_id = "light.fluffy_flashlight"
@@ -138,7 +138,7 @@ async def test_turn_off(
         blocking=True,
     )
 
-    mock_api_client.set_led_brightness.assert_called_once_with(0)
+    mock_api_client_coordinator.set_led_brightness.assert_called_once_with(0)
 
 
 @pytest.mark.parametrize(
@@ -153,12 +153,13 @@ async def test_turn_off(
 async def test_turn_on_led_not_activatable(
     hass: HomeAssistant,
     mock_config_entry: MockConfigEntry,
-    mock_api_client: MagicMock,
+    mock_api_client_init: MagicMock,
+    mock_api_client_coordinator: MagicMock,
     activatable_parameter: str,
 ) -> None:
     """Test turning on the light when LED is not activatable raises."""
     setattr(
-        mock_api_client.get_tracker.return_value.led_activatable,
+        mock_api_client_init.get_tracker.return_value.led_activatable,
         activatable_parameter,
         False,
     )
@@ -177,7 +178,7 @@ async def test_turn_on_led_not_activatable(
             blocking=True,
         )
 
-    mock_api_client.set_led_brightness.assert_not_called()
+    mock_api_client_coordinator.set_led_brightness.assert_not_called()
 
 
 @pytest.mark.parametrize(
@@ -191,11 +192,11 @@ async def test_turn_on_led_not_activatable(
     ],
 )
 @pytest.mark.parametrize("service", [SERVICE_TURN_ON, SERVICE_TURN_OFF])
-@pytest.mark.usefixtures("mock_auth_client")
+@pytest.mark.usefixtures("mock_auth_client", "mock_api_client_init")
 async def test_turn_on_off_error(
     hass: HomeAssistant,
     mock_config_entry: MockConfigEntry,
-    mock_api_client: MagicMock,
+    mock_api_client_coordinator: MagicMock,
     api_exception: FressnapfTrackerError,
     expected_exception: type[HomeAssistantError],
     service: str,
@@ -208,7 +209,7 @@ async def test_turn_on_off_error(
 
     entity_id = "light.fluffy_flashlight"
 
-    mock_api_client.set_led_brightness.side_effect = api_exception
+    mock_api_client_coordinator.set_led_brightness.side_effect = api_exception
     with pytest.raises(expected_exception):
         await hass.services.async_call(
             LIGHT_DOMAIN,

tests/components/fressnapf_tracker/test_switch.py

@@ -62,10 +62,10 @@ async def test_not_added_when_no_energy_saving_mode(
     hass: HomeAssistant,
     entity_registry: er.EntityRegistry,
     mock_config_entry: MockConfigEntry,
-    mock_api_client: MagicMock,
+    mock_api_client_init: MagicMock,
 ) -> None:
     """Test switch entity is created correctly."""
-    mock_api_client.get_tracker.return_value = TRACKER_NO_ENERGY_SAVING_MODE
+    mock_api_client_init.get_tracker.return_value = TRACKER_NO_ENERGY_SAVING_MODE
 
     mock_config_entry.add_to_hass(hass)
     await hass.config_entries.async_setup(mock_config_entry.entry_id)
@@ -80,7 +80,7 @@ async def test_not_added_when_no_energy_saving_mode(
 @pytest.mark.usefixtures("init_integration")
 async def test_turn_on(
     hass: HomeAssistant,
-    mock_api_client: MagicMock,
+    mock_api_client_coordinator: MagicMock,
 ) -> None:
     """Test turning the switch on."""
     entity_id = "switch.fluffy_sleep_mode"
@@ -96,13 +96,13 @@ async def test_turn_on(
         blocking=True,
     )
 
-    mock_api_client.set_energy_saving.assert_called_once_with(True)
+    mock_api_client_coordinator.set_energy_saving.assert_called_once_with(True)
 
 
 @pytest.mark.usefixtures("init_integration")
 async def test_turn_off(
     hass: HomeAssistant,
-    mock_api_client: MagicMock,
+    mock_api_client_coordinator: MagicMock,
 ) -> None:
     """Test turning the switch off."""
     entity_id = "switch.fluffy_sleep_mode"
@@ -118,7 +118,7 @@ async def test_turn_off(
         blocking=True,
     )
 
-    mock_api_client.set_energy_saving.assert_called_once_with(False)
+    mock_api_client_coordinator.set_energy_saving.assert_called_once_with(False)
 
 
 @pytest.mark.parametrize(
@@ -132,11 +132,11 @@ async def test_turn_off(
     ],
 )
 @pytest.mark.parametrize("service", [SERVICE_TURN_ON, SERVICE_TURN_OFF])
-@pytest.mark.usefixtures("mock_auth_client")
+@pytest.mark.usefixtures("mock_auth_client", "mock_api_client_init")
 async def test_turn_on_off_error(
     hass: HomeAssistant,
     mock_config_entry: MockConfigEntry,
-    mock_api_client: MagicMock,
+    mock_api_client_coordinator: MagicMock,
     api_exception: FressnapfTrackerError,
     expected_exception: type[HomeAssistantError],
     service: str,
@@ -149,7 +149,7 @@ async def test_turn_on_off_error(
 
     entity_id = "switch.fluffy_sleep_mode"
 
-    mock_api_client.set_energy_saving.side_effect = api_exception
+    mock_api_client_coordinator.set_energy_saving.side_effect = api_exception
     with pytest.raises(expected_exception):
         await hass.services.async_call(
             SWITCH_DOMAIN,