Skip to content

Warm the ViCare service cache on refresh instead of bypassing it #180344

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
lackas wants to merge 4 commits into from
Closed

Warm the ViCare service cache on refresh instead of bypassing it #180344

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
c9d7c77
Pass the device accessor when the ViCare coordinator refreshes
lackas Aug 26, 2026
e380550
Build the ViCare coordinator name from the device accessor
lackas Aug 26, 2026
5f0ee61
Warm the ViCare service cache on refresh instead of bypassing it
lackas Aug 26, 2026
715972c
Map malformed ViCare payloads to UpdateFailed
lackas Aug 27, 2026
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
14 changes: 11 additions & 3 deletions homeassistant/components/vicare/coordinator.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
"""DataUpdateCoordinator for the ViCare integration."""

from contextlib import suppress
from datetime import timedelta
import logging
from typing import override
Expand All @@ -9,6 +10,8 @@
PyViCareDeviceCommunicationError,
PyViCareInternalServerError,
PyViCareInvalidCredentialsError,
PyViCareInvalidDataError,
PyViCareNotSupportedFeatureError,
PyViCareRateLimitError,
)
import requests
Expand Down Expand Up @@ -46,7 +49,7 @@ def __init__(
hass,
_LOGGER,
config_entry=config_entry,
name=f"{DOMAIN}_{device.service.accessor.id}",
name=f"{DOMAIN}_{device.accessor.serial}_{device.accessor.device_id}",
update_interval=timedelta(seconds=DEFAULT_CACHE_DURATION * device_count),
)
self._device = device
Expand All @@ -60,13 +63,18 @@ def _refresh(self) -> None:
"""Force a fresh fetch from the Viessmann API."""
try:
self._device.service.clear_cache()
self._device.service.fetch_all_features()
# Read one property instead of calling fetch_all_features(): on the
# cached service the latter bypasses the cache, so every entity read
# would hit the API again, from the event loop.
with suppress(PyViCareNotSupportedFeatureError):

@zigpy-review-bot zigpy-review-bot Aug 26, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

This suppress is broader than the device.serial-is-optional case it is aimed at. After clear_cache(), _get_or_update_cache converts PyViCareNotPaidForError into PyViCareNotSupportedFeatureError("PACKAGE_NOT_PAID_FOR"), which this swallows — so a wholly unpaid account gets a refresh that reports success with an empty cache:

this PR  -> suppressed -> last_update_success True -> entities AVAILABLE
   cache populated? False
   entity read 1: PyViCareNotSupportedFeatureError | cumulative GETs: 2
   entity read 2: PyViCareNotSupportedFeatureError | cumulative GETs: 3
previous -> PyViCareNotPaidForError escapes -> last_update_success False -> entities UNAVAILABLE

Two consequences. The condition becomes undiagnosable — entities stay available reading unknown, with nothing logged, where before they went unavailable. And because the cache never populates, _stringify_state does read self.state, so every entity read re-enters _get_or_update_cache and hits the API from the event loop — the blocking-call problem this PR fixes, reappearing on the unpaid path.

Checking that the cache actually warmed after the suppressed probe distinguishes the benign case from this one.

self._device.getProperty("device.serial")

@zigpy-review-bot zigpy-review-bot Aug 26, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

_get_or_update_cache raises PyViCareInvalidDataError when the response has no data key, and that is in neither except clause, so a malformed payload surfaces as Unexpected error fetching vicare_… data with a traceback on every refresh instead of a clean UpdateFailed. The old raw fetch_all_features() never validated the payload, so this path is new here.

Malformed payload (no 'data'):    this PR  -> PyViCareInvalidDataError, mapped? False
                                  previous -> no raise
Rate limit / invalid credentials: both     -> mapped? True

Still better than the old behaviour, where the same payload detonated at the entity read inside the event loop — it just wants adding to the UpdateFailed tuple.

except PyViCareInvalidCredentialsError as err:
raise ConfigEntryAuthFailed from err
except (
PyViCareDeviceCommunicationError,
PyViCareRateLimitError,
PyViCareInternalServerError,
PyViCareInvalidDataError,
PyViCareRateLimitError,
requests.RequestException,
) as err:
raise UpdateFailed(str(err)) from err
27 changes: 15 additions & 12 deletions tests/components/vicare/conftest.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,15 +41,15 @@ def __init__(self, fixtures: list[Fixture]) -> None:
"""Init a single device from json dump."""
self.devices = []
for idx, fixture in enumerate(fixtures):
service = MockViCareService(
accessor = ViCareDeviceAccessor(
f"installation{idx}",
fixture.gateway_id or f"gateway{idx}",
f"deviceId{idx}",
fixture,
)
service = MockViCareService(fixture)
self.devices.append(
PyViCareDeviceConfig(
service.accessor,
accessor,
service,
"Vitovalor"
if fixture.data_file.endswith("VitoValor.json")
Expand All @@ -61,16 +61,18 @@ def __init__(self, fixtures: list[Fixture]) -> None:
# Simulate a device with an unsupported deviceType that PyViCare's
# `devices` filter would drop but should still appear in `all_devices`
# (used by diagnostics).
unsupported_service = MockViCareService(
unsupported_accessor = ViCareDeviceAccessor(
"installation_unsupported",
"gateway_unsupported",
"deviceId_unsupported",
Fixture(set(), "vicare/dummy-device-no-serial.json"),
)
unsupported_service = MockViCareService(
Fixture(set(), "vicare/dummy-device-no-serial.json")
)
self.all_devices = [
*self.devices,
PyViCareDeviceConfig(
unsupported_service.accessor,
unsupported_accessor,
unsupported_service,
"unsupported_model",
"Online",
Expand All @@ -92,22 +94,23 @@ def as_vicare_data(self) -> ViCareData:
class MockViCareService:
"""PyVicareService mock using a json dump."""

def __init__(
self, installation_id: str, gateway_id: str, device_id: str, fixture: Fixture
) -> None:
def __init__(self, fixture: Fixture) -> None:
"""Initialize the mock from a json dump."""
self._test_data = load_json_object_fixture(fixture.data_file)
self.fetch_all_features = Mock(return_value=self._test_data)
# Mirror the real signature: fetch_all_features() requires an accessor,
# and no real service carries one.
self.fetch_all_features = Mock(side_effect=lambda accessor: self._test_data)
self.setProperty = Mock()
self.clear_cache = Mock()
self.roles = fixture.roles
self.accessor = ViCareDeviceAccessor(installation_id, gateway_id, device_id)
# A Mock so tests can inject API errors on the read path.
self.getProperty = Mock(side_effect=self._read_property)

def hasRoles(self, requested_roles: list[str]) -> bool:
"""Return true if requested roles are assigned."""
return requested_roles and set(requested_roles).issubset(self.roles)

def getProperty(self, accessor: ViCareDeviceAccessor, property_name: str):
def _read_property(self, accessor: ViCareDeviceAccessor, property_name: str):
"""Read a property from json dump."""
return readFeature(self._test_data["data"], property_name)

Expand Down
45 changes: 41 additions & 4 deletions tests/components/vicare/test_init.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
PyViCareInternalServerError,
PyViCareInvalidConfigurationError,
PyViCareInvalidCredentialsError,
PyViCareInvalidDataError,
)

from homeassistant.components.vicare.const import DOMAIN
Expand Down Expand Up @@ -479,7 +480,7 @@ async def test_coordinator_recovers_after_transient_failure(
assert state is not None, f"{sensor_id} not found in states"
assert state.state != STATE_UNAVAILABLE

service.fetch_all_features.side_effect = PyViCareInternalServerError(
service.getProperty.side_effect = PyViCareInternalServerError(
{
"statusCode": 500,
"errorType": "INTERNAL_SERVER_ERROR",
Expand All @@ -494,7 +495,7 @@ async def test_coordinator_recovers_after_transient_failure(
state = hass.states.get(sensor_id)
assert state.state == STATE_UNAVAILABLE

service.fetch_all_features.side_effect = None
service.getProperty.side_effect = service._read_property
freezer.tick(timedelta(seconds=120))
async_fire_time_changed(hass, fire_all=True)
await hass.async_block_till_done(wait_background_tasks=True)
Expand All @@ -503,6 +504,42 @@ async def test_coordinator_recovers_after_transient_failure(
assert state.state != STATE_UNAVAILABLE


async def test_coordinator_invalid_data_is_handled(
hass: HomeAssistant,
freezer: FrozenDateTimeFactory,
mock_config_entry: MockConfigEntry,
caplog: pytest.LogCaptureFixture,
) -> None:
"""A malformed payload fails the refresh without an unexpected error."""
fixtures: list[Fixture] = [Fixture({"type:boiler"}, "vicare/Vitodens300W.json")]
mock_vicare = MockPyViCare(fixtures)
service = mock_vicare.devices[0].service

with (
patch(
"homeassistant.helpers.config_entry_oauth2_flow.OAuth2Session.async_ensure_token_valid",
),
patch(
f"{MODULE}._setup_vicare_api",
return_value=mock_vicare.as_vicare_data(),
),
patch(f"{MODULE}.PLATFORMS", [Platform.SENSOR]),
):
mock_config_entry.add_to_hass(hass)
await hass.config_entries.async_setup(mock_config_entry.entry_id)
await hass.async_block_till_done()

sensor_id = "sensor.model0_outside_temperature"
service.getProperty.side_effect = PyViCareInvalidDataError({"error": "no data"})
caplog.clear()
freezer.tick(timedelta(seconds=120))
async_fire_time_changed(hass, fire_all=True)
await hass.async_block_till_done(wait_background_tasks=True)

assert hass.states.get(sensor_id).state == STATE_UNAVAILABLE
assert "Unexpected error fetching" not in caplog.text


async def test_per_device_failure_isolation(
hass: HomeAssistant,
freezer: FrozenDateTimeFactory,
Expand Down Expand Up @@ -538,7 +575,7 @@ async def test_per_device_failure_isolation(
assert hass.states.get(sensor_device0).state != STATE_UNAVAILABLE
assert hass.states.get(sensor_device1).state != STATE_UNAVAILABLE

service0.fetch_all_features.side_effect = PyViCareInternalServerError(
service0.getProperty.side_effect = PyViCareInternalServerError(
{
"statusCode": 500,
"errorType": "INTERNAL_SERVER_ERROR",
Expand Down Expand Up @@ -586,7 +623,7 @@ async def test_coordinator_auth_failure_triggers_reauth(
if flow["context"]["source"] == SOURCE_REAUTH
]

service.fetch_all_features.side_effect = PyViCareInvalidCredentialsError(
service.getProperty.side_effect = PyViCareInvalidCredentialsError(
"invalid_grant"
)
freezer.tick(timedelta(seconds=120))
Expand Down
Loading