Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions CODEOWNERS

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion homeassistant/components/surepetcare/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@

_LOGGER = logging.getLogger(__name__)

PLATFORMS = [Platform.BINARY_SENSOR, Platform.LOCK, Platform.SENSOR]
PLATFORMS = [Platform.BINARY_SENSOR, Platform.LOCK, Platform.SENSOR, Platform.SWITCH]
SCAN_INTERVAL = timedelta(minutes=3)


Expand Down
4 changes: 4 additions & 0 deletions homeassistant/components/surepetcare/const.py
Original file line number Diff line number Diff line change
Expand Up @@ -21,3 +21,7 @@
ATTR_LOCATION = "location"
ATTR_LOCK_STATE = "lock_state"
ATTR_PET_NAME = "pet_name"

# pet profile modes
PROFILE_INDOOR = 3 # indoor mode - pet can enter but cannot exit
PROFILE_OUTDOOR = 2 # outdoor/normal mode - pet can enter and exit
Comment on lines +25 to +27
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  1. are there more profiles?
  2. How does the app call this?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The best API docs that exist for this vendor is from their Swagger docs for their Beta env. There is a schema for the profile (SpecialProfiles) however that just gives an array of ints with no description as to what these are ([ 0, 1, 2, 3, 4, 5, 6 ]]).

When viewing the requests made on their web app I cannot see any usage other than 2 (outdoor) or 3 (indoor) when you are marking a pet to be "inside only" for a given flap

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm, do we have a way to discover other modes? Because if we find more we should make it a select entity instead

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I suspect that the remaining profiles are related to their other products (devices) such as their smart feeder and water fountain. They appear to use "tags" on each "device" to control certain features, one of which is the indoor only mode for their cat flap as implemented here.

If I can find some batteries for our feeder I can help confirm that. Though going through their app again for everything catflap related, it's only ever "2" or "3"

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What if you use their API to set it to something different?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So long as you pass in an int from 0 to 6, you can successfully set that profile on the device. However, none of the other values have any impact on the Indoor Only mode. I have reached out to their support to see if they are able to provide any more info here, though I do suspect these other profile values are for other devices (not flaps)

2 changes: 1 addition & 1 deletion homeassistant/components/surepetcare/manifest.json
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
{
"domain": "surepetcare",
"name": "Sure Petcare",
"codeowners": ["@benleb", "@danielhiversen"],
"codeowners": ["@benleb", "@danielhiversen", "@AlexC"],
"config_flow": true,
"documentation": "https://www.home-assistant.io/integrations/surepetcare",
"iot_class": "cloud_polling",
Expand Down
7 changes: 7 additions & 0 deletions homeassistant/components/surepetcare/strings.json
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,13 @@
}
}
},
"entity": {
"switch": {
"indoor_mode": {
"name": "{pet_name} indoor-only mode"
}
}
},
"services": {
"set_lock_state": {
"description": "Sets lock state.",
Expand Down
143 changes: 143 additions & 0 deletions homeassistant/components/surepetcare/switch.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,143 @@
"""Support for Sure PetCare Flaps switches."""

from __future__ import annotations

from typing import Any, cast

from surepy.const import BASE_RESOURCE
from surepy.entities import SurepyEntity
from surepy.entities.pet import Pet as SurepyPet
from surepy.enums import EntityType
from surepy.exceptions import SurePetcareError

from homeassistant.components.switch import SwitchEntity
from homeassistant.config_entries import ConfigEntry
from homeassistant.core import HomeAssistant, callback
from homeassistant.exceptions import HomeAssistantError
from homeassistant.helpers.entity_platform import AddConfigEntryEntitiesCallback

from .const import DOMAIN, PROFILE_INDOOR, PROFILE_OUTDOOR
from .coordinator import SurePetcareDataCoordinator
from .entity import SurePetcareEntity


async def async_setup_entry(
hass: HomeAssistant,
entry: ConfigEntry,
async_add_entities: AddConfigEntryEntitiesCallback,
) -> None:
"""Set up Sure PetCare switches on a config entry."""

coordinator: SurePetcareDataCoordinator = hass.data[DOMAIN][entry.entry_id]

pets_by_tag_id: dict[int, SurepyPet] = {}
for entity in coordinator.data.values():
if entity.type == EntityType.PET:
pet = cast(SurepyPet, entity)
if pet.tag_id is not None:
pets_by_tag_id[pet.tag_id] = pet

entities: list[SurePetcareIndoorModeSwitch] = []
for entity in coordinator.data.values():
if entity.type in [EntityType.CAT_FLAP, EntityType.PET_FLAP]:
for tag_data in entity.raw_data().get("tags", []):
tag_id = tag_data.get("id")
if tag_id in pets_by_tag_id:
entities.append(
SurePetcareIndoorModeSwitch(
pet=pets_by_tag_id[tag_id],
flap=entity,
coordinator=coordinator,
)
)

async_add_entities(entities)


class SurePetcareIndoorModeSwitch(SurePetcareEntity, SwitchEntity):
"""A switch implementation for Sure Petcare pet indoor mode."""

_attr_has_entity_name = True
_attr_translation_key = "indoor_mode"
_attr_entity_registry_enabled_default = False

def __init__(
self,
pet: SurepyPet,
flap: SurepyEntity,
coordinator: SurePetcareDataCoordinator,
) -> None:
"""Initialize a Sure Petcare indoor mode switch."""
self._pet = pet
self._flap = flap
self._profile_id: int | None = None
self._available = False

# Initialize with flap_id so the entity is attached to the flap device
super().__init__(flap.id, coordinator)

self._attr_unique_id = f"{self._device_id}-{pet.id}-indoor_mode"
self._attr_translation_placeholders = {"pet_name": pet.name}

@property
def available(self) -> bool:
"""Return true if entity is available."""
return self._available and super().available

@callback
def _update_attr(self, surepy_entity: SurepyEntity) -> None:
"""Update the state from the flap's tag data."""
tags_by_id = {
tag.get("id"): tag for tag in surepy_entity.raw_data().get("tags", [])
}
Comment on lines +90 to +92
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Future improvement for the library, we should add this in the model in an easy way, as we're now looping over the tags for every door

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree with you here, see my comment around the general state of the library below


pet_tag = tags_by_id.get(self._pet.tag_id)
if pet_tag is None:
# Pet no longer configured for this flap
self._available = False
self._profile_id = None
return

self._available = True
self._profile_id = pet_tag.get("profile", PROFILE_OUTDOOR)
self._attr_is_on = self._profile_id == PROFILE_INDOOR

async def _async_update_tag_profile(self, profile: int) -> None:
"""Call the API to set the pet's profile on this flap."""
result = await self.coordinator.surepy.sac.call(
method="PUT",
resource=f"{BASE_RESOURCE}/device/{self._flap.id}/tag/{self._pet.tag_id}",
json={"profile": profile},
)
if result is None:
raise SurePetcareError("Device tag API call returned None")

async def _async_set_profile(self, profile: int) -> None:
"""Set the pet's profile on this flap."""
try:
await self._async_update_tag_profile(profile)
except SurePetcareError as err:
await self.coordinator.async_request_refresh()
mode = "indoor" if profile == PROFILE_INDOOR else "outdoor"
raise HomeAssistantError(
f"Failed to set {self._pet.name} {mode} mode on {self._flap.name}"
) from err

# Update state immediately after successful API call
self._profile_id = profile
self._attr_is_on = profile == PROFILE_INDOOR
self.async_write_ha_state()
Comment on lines +120 to +129
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why do we only refresh when it failed?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My thinking was that if the API request is successful, then there is no need to make another API request to the data that we already know. A refresh will happen on the next polling interval (every 3 minutes for this integration).

On the failure state, I feel a refresh is required because we don't know what happened to the request, so fetching the latest data right away would bring it back in sync.

Happy to take your advice here though

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Did you ever get it to error out?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not in a real example, only via the tests. However I do see an issue as the surepy library doesn't raise any exception when a HTTP 5xx occurs for example. See https://github.com/benleb/surepy/blob/d4af3694b4201d05afc06ff46e61e586b7cc2fed/surepy/client.py#L244-L246

From what I can see, None would get returned. Looks like other calls would treat None as "something went wrong" and then throw SurePetcareError e.g. https://github.com/benleb/surepy/blob/d4af3694b4201d05afc06ff46e61e586b7cc2fed/surepy/client.py#L381

I can update this to throw SurePetcareError if the response to .call is None - just as the other methods do, if you agree? This feels like an area that could do with some attention in the actual surepy lib to address more cleanly

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we shouldn't throw exceptions other than ServiceValidationError (when the user has a skill issue) and HomeAssistantError (aka, we had a reason that it failed)

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I mean within the try block its self, if the response is None then throw SurePetcareError - that then gets caught by the existing except SurePetcareError which will ultimately raise a HomeAssistantError

This is what existing methods do as linked to above

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've made the above change to align with how the other SureAPIClient methods handle this


async def async_turn_on(self, **kwargs: Any) -> None:
"""Turn on the switch (set indoor mode)."""
if self._attr_is_on:
return

await self._async_set_profile(PROFILE_INDOOR)

async def async_turn_off(self, **kwargs: Any) -> None:
"""Turn off the switch (set outdoor mode)."""
if not self._attr_is_on:
return

await self._async_set_profile(PROFILE_OUTDOOR)
29 changes: 29 additions & 0 deletions tests/components/surepetcare/__init__.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,13 @@
"""Tests for Sure Petcare integration."""

from unittest.mock import AsyncMock

from homeassistant.components.surepetcare.const import DOMAIN
from homeassistant.components.surepetcare.coordinator import SurePetcareDataCoordinator
from homeassistant.core import HomeAssistant

from tests.common import MockConfigEntry

HOUSEHOLD_ID = 987654321
HUB_ID = 123456789

Expand Down Expand Up @@ -56,6 +64,10 @@
"signal": {"device_rssi": 65, "hub_rssi": 64},
"online": True,
},
# id: tag ID (matches pet's tag_id), profile: 3 = indoor only (HA switch ON)
"tags": [
{"id": 246801, "profile": 3},
],
}

MOCK_PET_FLAP = {
Expand All @@ -71,10 +83,15 @@
"signal": {"device_rssi": 70, "hub_rssi": 65},
"online": True,
},
# id: tag ID (matches pet's tag_id), profile: 2 = outdoor (HA switch OFF)
"tags": [
{"id": 246801, "profile": 2},
],
}

MOCK_PET = {
"id": 24680,
"tag_id": 246801, # Tag ID used to match against flap tags
"household_id": HOUSEHOLD_ID,
"name": "Pet",
"position": {"since": "2020-08-23T23:10:50", "where": 1},
Expand All @@ -85,3 +102,15 @@
"devices": [MOCK_HUB, MOCK_CAT_FLAP, MOCK_PET_FLAP, MOCK_FEEDER, MOCK_FELAQUA],
"pets": [MOCK_PET],
}


async def setup_integration(
hass: HomeAssistant,
config_entry: MockConfigEntry,
surepetcare_mock: AsyncMock,
) -> SurePetcareDataCoordinator:
"""Fixture for setting up the component."""
config_entry.add_to_hass(hass)
await hass.config_entries.async_setup(config_entry.entry_id)
await hass.async_block_till_done()
return hass.data[DOMAIN][config_entry.entry_id]
31 changes: 15 additions & 16 deletions tests/components/surepetcare/conftest.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,18 +3,18 @@
from unittest.mock import patch

import pytest
from surepy import MESTART_RESOURCE
from surepy.const import MESTART_RESOURCE

from homeassistant.components.surepetcare.const import DOMAIN
from homeassistant.const import CONF_PASSWORD, CONF_TOKEN, CONF_USERNAME
from homeassistant.core import HomeAssistant

from . import MOCK_API_DATA

from tests.common import MockConfigEntry


async def _mock_call(method, resource):
"""Mock API call that only handles the initial GET request."""
if method == "GET" and resource == MESTART_RESOURCE:
return {"data": MOCK_API_DATA}
return None
Expand All @@ -32,17 +32,16 @@ async def surepetcare():


@pytest.fixture
async def mock_config_entry_setup(hass: HomeAssistant) -> MockConfigEntry:
"""Help setting up a mocked config entry."""
data = {
CONF_USERNAME: "test-username",
CONF_PASSWORD: "test-password",
CONF_TOKEN: "token",
"feeders": [12345],
"flaps": [13579, 13576],
"pets": [24680],
}
entry = MockConfigEntry(domain=DOMAIN, data=data)
entry.add_to_hass(hass)
assert await hass.config_entries.async_setup(entry.entry_id)
return entry
def mock_config_entry() -> MockConfigEntry:
"""Return the default mocked config entry."""
return MockConfigEntry(
domain=DOMAIN,
data={
CONF_USERNAME: "test-username",
CONF_PASSWORD: "test-password",
CONF_TOKEN: "token",
"feeders": [12345],
"flaps": [13579, 13576],
"pets": [24680],
},
)
Loading