Collapse ViCare coordinators to one per gateway (#176163)

This commit is contained in:
Christian Lackas
2026-08-30 17:21:38 +02:00
committed by GitHub
parent cb844ff8ef
commit 59738d0455
8 changed files with 499 additions and 222 deletions
+36 -9
View File
@@ -1,5 +1,6 @@
"""The ViCare integration."""
from collections import defaultdict
from contextlib import suppress
import logging
import os
@@ -170,11 +171,25 @@ async def async_setup_entry(hass: HomeAssistant, entry: ViCareConfigEntry) -> bo
) as err:
raise ConfigEntryAuthFailed("Authentication failed") from err
device_count = len(entry.runtime_data.devices)
coordinators: list[ViCareCoordinator] = []
# Group devices by gateway: in viaGateway mode one bulk fetch refreshes
# every device behind a gateway, so one coordinator serves the gateway.
devices_by_gateway: dict[str, list[ViCareDevice]] = defaultdict(list)
for device in entry.runtime_data.devices:
coordinator = ViCareCoordinator(hass, entry, device.api, device_count)
device.coordinator = coordinator
devices_by_gateway[device.config.getConfig().serial].append(device)
gateway_count = len(devices_by_gateway)
coordinators: list[ViCareCoordinator] = []
for gateway_devices in devices_by_gateway.values():
representative = gateway_devices[0]
coordinator = ViCareCoordinator(
hass,
entry,
representative.api,
representative.config.getConfig(),
gateway_count,
)
for device in gateway_devices:
device.coordinator = coordinator
coordinators.append(coordinator)
for device in entry.runtime_data.devices:
@@ -233,20 +248,32 @@ def _setup_vicare_api(
) -> ViCareData:
"""Set up PyVicare API."""
client = PyViCare()
client.loadViaGateway(True)
client.setCacheDuration(cache_duration)
client.initWithExternalOAuth(auth)
device_config_list = get_supported_devices(client.devices)
# increase cache duration to fit rate limit to number of devices
if (number_of_devices := len(device_config_list)) > 1:
cache_duration = DEFAULT_CACHE_DURATION * number_of_devices
# In viaGateway mode each gateway is one bulk fetch per cycle, so the rate
# limit scales with the number of gateways, not devices. Offline gateways
# are never fetched, and are skipped below, so they must not count here
# either; this has to match the grouping in async_setup_entry.
gateway_count = len(
{
config.getConfig().serial
for config in device_config_list
if config.isOnline()
}
)
if gateway_count > 1:
cache_duration = DEFAULT_CACHE_DURATION * gateway_count
_LOGGER.debug(
"Found %s devices, adjusting cache duration to %s",
number_of_devices,
"Found %s gateways, adjusting cache duration to %s",
gateway_count,
cache_duration,
)
client = PyViCare()
client.loadViaGateway(True)
client.setCacheDuration(cache_duration)
client.initWithExternalOAuth(auth)
device_config_list = get_supported_devices(client.devices)
+20 -11
View File
@@ -5,11 +5,13 @@ import logging
from typing import override
from PyViCare.PyViCareDevice import Device as PyViCareDevice
from PyViCare.PyViCareService import ViCareDeviceAccessor
from PyViCare.PyViCareUtils import (
PyViCareDeviceCommunicationError,
PyViCareInternalServerError,
PyViCareInvalidCredentialsError,
PyViCareInvalidDataError,
PyViCareNotSupportedFeatureError,
PyViCareRateLimitError,
)
import requests
@@ -25,12 +27,12 @@ _LOGGER = logging.getLogger(__name__)
class ViCareCoordinator(DataUpdateCoordinator[None]):
"""Coordinator for a single ViCare device.
"""Coordinator for a single ViCare gateway.
Triggers a fresh fetch of the device's full feature payload into
PyViCare's internal cache so entity ``value_getter`` lambdas read
fresh data on each tick. Carries no payload of its own; freshness
is signalled via ``last_update_success``.
In viaGateway mode all devices behind a gateway share one service, so a
single feature fetch refreshes every device on that gateway. The fetch takes
the accessor of a representative device. Carries no payload of its own;
freshness is signalled via ``last_update_success``.
"""
config_entry: ViCareConfigEntry
@@ -40,28 +42,35 @@ class ViCareCoordinator(DataUpdateCoordinator[None]):
hass: HomeAssistant,
config_entry: ViCareConfigEntry,
device: PyViCareDevice,
device_count: int,
accessor: ViCareDeviceAccessor,
gateway_count: int,
) -> None:
"""Initialise the coordinator for one device."""
"""Initialise the coordinator for one gateway."""
super().__init__(
hass,
_LOGGER,
config_entry=config_entry,
name=f"{DOMAIN}_{device.accessor.serial}_{device.accessor.device_id}",
update_interval=timedelta(seconds=DEFAULT_CACHE_DURATION * device_count),
name=f"{DOMAIN}_{accessor.serial}",
update_interval=timedelta(seconds=DEFAULT_CACHE_DURATION * gateway_count),
)
self._device = device
self._accessor = accessor
@override
async def _async_update_data(self) -> None:
"""Refresh the device's feature payload."""
"""Refresh the gateway's feature payload."""
await self.hass.async_add_executor_job(self._refresh)
def _refresh(self) -> None:
"""Force a fresh fetch from the Viessmann API."""
try:
self._device.service.clear_cache()
self._device.service.fetch_all_features(self._device.accessor)
self._device.service.fetch_all_features(self._accessor)
except PyViCareNotSupportedFeatureError:
# PACKAGE_NOT_PAID_FOR: load with no features instead of retrying setup.
_LOGGER.debug(
"No accessible features for gateway %s", self._accessor.serial
)
except PyViCareInvalidCredentialsError as err:
raise ConfigEntryAuthFailed from err
except (
@@ -3,6 +3,7 @@
import json
from typing import Any
from PyViCare.PyViCareServiceViaGateway import filter_features_for_device
from PyViCare.PyViCareUtils import PyViCareDeviceCommunicationError
from homeassistant.components.diagnostics import async_redact_data
@@ -36,7 +37,13 @@ async def async_get_config_entry_diagnostics(
devices: list[dict[str, Any]] = []
for device in entry.runtime_data.client.all_devices:
try:
devices.append(json.loads(device.dump_secure()))
dump = json.loads(device.dump_secure())
# In viaGateway mode dump_secure() returns the whole gateway's
# features, so scope them to the device the entry describes.
dump["data"] = filter_features_for_device(
dump["data"], device.device_id
)
devices.append(dump)
except PyViCareDeviceCommunicationError as err:
# One offline gateway must not abort the whole diagnostics dump.
devices.append(
+48 -27
View File
@@ -2,6 +2,7 @@
from collections.abc import AsyncGenerator, Generator
from dataclasses import dataclass
import re
import time
from unittest.mock import AsyncMock, Mock, patch
@@ -32,47 +33,48 @@ class Fixture:
data_file: str
# Opt-in shared gateway serial; defaults to a per-fixture gateway when unset.
gateway_id: str | None = None
online: bool = True
class MockPyViCare:
"""Mocked PyVicare class based on a json dump."""
def __init__(self, fixtures: list[Fixture]) -> None:
"""Init a single device from json dump."""
"""Init devices from json dumps, sharing one service per gateway."""
self.devices = []
self.services: dict[str, MockViCareService] = {}
for idx, fixture in enumerate(fixtures):
accessor = ViCareDeviceAccessor(
f"installation{idx}",
fixture.gateway_id or f"gateway{idx}",
f"deviceId{idx}",
gateway_id = fixture.gateway_id or f"gateway{idx}"
device_id = f"deviceId{idx}"
service = self.services.setdefault(
gateway_id, MockViCareService(fixture.roles)
)
service = MockViCareService(fixture)
service.add_device(device_id, fixture)
self.devices.append(
PyViCareDeviceConfig(
accessor,
ViCareDeviceAccessor(f"installation{idx}", gateway_id, device_id),
service,
"Vitovalor"
if fixture.data_file.endswith("VitoValor.json")
else f"model{idx}",
"Online",
"Online" if fixture.online else "Offline",
roles=list(fixture.roles),
)
)
# 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_accessor = ViCareDeviceAccessor(
"installation_unsupported",
"gateway_unsupported",
"deviceId_unsupported",
)
unsupported_service = MockViCareService(
Fixture(set(), "vicare/dummy-device-no-serial.json")
)
unsupported_fixture = Fixture(set(), "vicare/dummy-device-no-serial.json")
unsupported_service = MockViCareService(set())
unsupported_service.add_device("deviceId_unsupported", unsupported_fixture)
self.all_devices = [
*self.devices,
PyViCareDeviceConfig(
unsupported_accessor,
ViCareDeviceAccessor(
"installation_unsupported",
"gateway_unsupported",
"deviceId_unsupported",
),
unsupported_service,
"unsupported_model",
"Online",
@@ -92,25 +94,44 @@ class MockPyViCare:
class MockViCareService:
"""PyVicareService mock using a json dump."""
"""Mock of the gateway-wide service PyViCare shares in viaGateway mode.
def __init__(self, fixture: Fixture) -> None:
"""Initialize the mock from a json dump."""
self._test_data = load_json_object_fixture(fixture.data_file)
# 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)
One instance serves every device on the gateway: `fetch_all_features`
returns the bulk payload for all of them, and `getProperty` filters by
`accessor.device_id`, like `ViCareCachedServiceViaGateway` does.
"""
def __init__(self, roles: set[str]) -> None:
"""Initialize an empty gateway service."""
self._features: dict[str, list] = {}
self.fetch_all_features = Mock(side_effect=self._fetch_all_features)
self.setProperty = Mock()
self.clear_cache = Mock()
self.roles = fixture.roles
self.roles = roles
def add_device(self, device_id: str, fixture: Fixture) -> None:
"""Add a device's features to the gateway payload."""
features = load_json_object_fixture(fixture.data_file)["data"]
# In the real bulk payload every feature carries its own device in the
# uri, which is what consumers filter on. The fixtures all say device 0.
for feature in features:
if "uri" in feature:
feature["uri"] = re.sub(
r"/devices/[^/]+/", f"/devices/{device_id}/", feature["uri"]
)
self._features[device_id] = features
def _fetch_all_features(self, accessor: ViCareDeviceAccessor):
"""Return the features of every device on the gateway."""
return {"data": [f for features in self._features.values() for f in features]}
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):
"""Read a property from json dump."""
return readFeature(self._test_data["data"], property_name)
"""Read a property of one device from the gateway payload."""
return readFeature(self._features[accessor.device_id], property_name)
@pytest.fixture(autouse=True)
File diff suppressed because it is too large Load Diff
+38 -1
View File
@@ -1,6 +1,6 @@
"""Test ViCare diagnostics."""
from unittest.mock import MagicMock
from unittest.mock import MagicMock, patch
from PyViCare.PyViCareUtils import PyViCareDeviceCommunicationError
from syrupy.assertion import SnapshotAssertion
@@ -8,6 +8,10 @@ from syrupy.filters import props
from homeassistant.core import HomeAssistant
from . import MODULE, setup_integration
from .conftest import Fixture, MockPyViCare
from tests.common import MockConfigEntry
from tests.components.diagnostics import get_diagnostics_for_config_entry
from tests.typing import ClientSessionGenerator
@@ -51,3 +55,36 @@ async def test_diagnostics_with_offline_device(
assert "error" in error_entry
assert "GATEWAY_OFFLINE" in error_entry["error"]
assert error_entry["device"]["id"] == devices[0].device_id
async def test_diagnostics_scopes_features_to_their_device(
hass: HomeAssistant,
hass_client: ClientSessionGenerator,
mock_config_entry: MockConfigEntry,
) -> None:
"""Devices sharing a gateway must not each dump the whole gateway payload."""
fixtures: list[Fixture] = [
Fixture({"type:boiler"}, "vicare/Vitodens300W.json", gateway_id="gateway0"),
Fixture({"type:heatpump"}, "vicare/Vitocal250A.json", gateway_id="gateway0"),
]
with (
patch(
"homeassistant.helpers.config_entry_oauth2_flow.OAuth2Session.async_ensure_token_valid",
),
patch(
f"{MODULE}._setup_vicare_api",
return_value=MockPyViCare(fixtures).as_vicare_data(),
),
):
await setup_integration(hass, mock_config_entry)
diag = await get_diagnostics_for_config_entry(hass, hass_client, mock_config_entry)
dumps = {entry["device"]["id"]: entry["data"] for entry in diag["data"]}
# 167 and 325 features; without scoping both would dump all 492.
assert [len(dumps["deviceId0"]), len(dumps["deviceId1"])] == [167, 325]
for device_id in ("deviceId0", "deviceId1"):
assert {
feature["uri"].split("/devices/")[1].split("/")[0]
for feature in dumps[device_id]
} == {device_id}
+180 -5
View File
@@ -1,7 +1,7 @@
"""Test ViCare initialization and migration."""
from datetime import timedelta
from unittest.mock import Mock, patch
from unittest.mock import Mock, call, patch
from aiohttp import ClientError
from freezegun.api import FrozenDateTimeFactory
@@ -11,9 +11,10 @@ from PyViCare.PyViCareUtils import (
PyViCareInvalidConfigurationError,
PyViCareInvalidCredentialsError,
PyViCareInvalidDataError,
PyViCareNotSupportedFeatureError,
)
from homeassistant.components.vicare.const import DOMAIN
from homeassistant.components.vicare.const import DEFAULT_CACHE_DURATION, DOMAIN
from homeassistant.config_entries import SOURCE_REAUTH, ConfigEntryState
from homeassistant.const import (
CONF_CLIENT_ID,
@@ -542,12 +543,12 @@ async def test_coordinator_handles_invalid_data(
assert "Unexpected error fetching" not in caplog.text
async def test_per_device_failure_isolation(
async def test_per_gateway_failure_isolation(
hass: HomeAssistant,
freezer: FrozenDateTimeFactory,
mock_config_entry: MockConfigEntry,
) -> None:
"""A transient failure on one device must not affect the other device's sensors."""
"""A transient failure on one gateway must not affect another gateway's sensors."""
fixtures: list[Fixture] = [
Fixture({"type:climateSensor"}, "vicare/RoomSensor1.json"),
Fixture({"type:climateSensor"}, "vicare/RoomSensor2.json"),
@@ -586,7 +587,7 @@ async def test_per_device_failure_isolation(
}
)
# Coordinator interval scales by device count (60 * 2 = 120s); tick past it.
# Coordinator interval scales by gateway count (60 * 2 = 120s); tick past it.
freezer.tick(timedelta(seconds=300))
async_fire_time_changed(hass, fire_all=True)
await hass.async_block_till_done(wait_background_tasks=True)
@@ -595,6 +596,56 @@ async def test_per_device_failure_isolation(
assert hass.states.get(sensor_device1).state != STATE_UNAVAILABLE
async def test_devices_on_same_gateway_share_coordinator(
hass: HomeAssistant,
freezer: FrozenDateTimeFactory,
mock_config_entry: MockConfigEntry,
) -> None:
"""Two devices behind one gateway share one coordinator and one fetch."""
fixtures: list[Fixture] = [
Fixture({"type:climateSensor"}, "vicare/RoomSensor1.json", gateway_id="gwA"),
Fixture({"type:climateSensor"}, "vicare/RoomSensor2.json", gateway_id="gwA"),
]
mock_vicare = MockPyViCare(fixtures)
service0 = 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_device0 = "sensor.model0_temperature"
sensor_device1 = "sensor.model1_temperature"
assert hass.states.get(sensor_device0).state != STATE_UNAVAILABLE
assert hass.states.get(sensor_device1).state != STATE_UNAVAILABLE
# The gateway's single fetch failing takes every device on it offline.
service0.fetch_all_features.side_effect = PyViCareInternalServerError(
{
"statusCode": 500,
"errorType": "INTERNAL_SERVER_ERROR",
"message": "Internal Server Error",
"viErrorId": "0",
}
)
# One gateway -> interval 60s; tick past it.
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_device0).state == STATE_UNAVAILABLE
assert hass.states.get(sensor_device1).state == STATE_UNAVAILABLE
async def test_coordinator_auth_failure_triggers_reauth(
hass: HomeAssistant,
freezer: FrozenDateTimeFactory,
@@ -698,3 +749,127 @@ async def test_device_via_device_missing_gateway(
)
assert channel_device is not None
assert channel_device.via_device_id is None
async def test_setup_runs_pyvicare_init_and_fetches_once_per_gateway(
hass: HomeAssistant,
mock_config_entry: MockConfigEntry,
) -> None:
"""Set up through _setup_vicare_api instead of a prebuilt ViCareData.
Covers the via-gateway init, the gateway-based cache duration, and one
fetch per gateway.
"""
# Two devices behind gwA, one behind gwB: two gateways, three devices.
fixtures: list[Fixture] = [
Fixture({"type:climateSensor"}, "vicare/RoomSensor1.json", gateway_id="gwA"),
Fixture({"type:climateSensor"}, "vicare/RoomSensor2.json", gateway_id="gwA"),
Fixture({"type:climateSensor"}, "vicare/RoomSensor1.json", gateway_id="gwB"),
]
client = MockPyViCare(fixtures)
# viaGateway has to be set before init, the services are wired during init.
setup_calls: list[str] = []
client.loadViaGateway = Mock(side_effect=lambda _: setup_calls.append("gateway"))
client.setCacheDuration = Mock()
client.initWithExternalOAuth = Mock(
side_effect=lambda _: setup_calls.append("init")
)
with (
patch(
"homeassistant.helpers.config_entry_oauth2_flow.OAuth2Session.async_ensure_token_valid",
),
patch(f"{MODULE}.PyViCare", return_value=client),
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()
assert mock_config_entry.state is ConfigEntryState.LOADED
# viaGateway mode enabled, cache duration scaled to the gateway count.
client.loadViaGateway.assert_called_with(True)
# Setup re-inits once to apply the gateway-based cache duration.
assert setup_calls == ["gateway", "init", "gateway", "init"]
assert call(DEFAULT_CACHE_DURATION * 2) in client.setCacheDuration.call_args_list
# One refresh per gateway, and the two devices behind gwA share that one
# service, so the second device is served without a fetch of its own.
assert client.services["gwA"].fetch_all_features.call_count == 1
assert client.services["gwB"].fetch_all_features.call_count == 1
assert hass.states.get("sensor.model0_temperature").state == "17.5"
assert hass.states.get("sensor.model1_temperature").state == "16.9"
async def test_offline_gateway_does_not_stretch_the_cache(
hass: HomeAssistant,
mock_config_entry: MockConfigEntry,
) -> None:
"""An offline gateway is never fetched, so it must not size the cache."""
fixtures: list[Fixture] = [
Fixture({"type:climateSensor"}, "vicare/RoomSensor1.json", gateway_id="gwA"),
Fixture({"type:climateSensor"}, "vicare/RoomSensor2.json", gateway_id="gwB"),
Fixture(
{"type:climateSensor"},
"vicare/RoomSensor1.json",
gateway_id="gwC",
online=False,
),
]
client = MockPyViCare(fixtures)
client.loadViaGateway = Mock()
client.setCacheDuration = Mock()
client.initWithExternalOAuth = Mock()
with (
patch(
"homeassistant.helpers.config_entry_oauth2_flow.OAuth2Session.async_ensure_token_valid",
),
patch(f"{MODULE}.PyViCare", return_value=client),
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()
assert mock_config_entry.state is ConfigEntryState.LOADED
# Two online gateways out of three, so the cache matches the coordinator
# interval instead of being stretched to 3 x 60s.
assert call(DEFAULT_CACHE_DURATION * 2) in client.setCacheDuration.call_args_list
assert (
call(DEFAULT_CACHE_DURATION * 3) not in client.setCacheDuration.call_args_list
)
assert client.services["gwC"].fetch_all_features.call_count == 0
async def test_setup_loads_with_unpaid_package_gateway(
hass: HomeAssistant,
mock_config_entry: MockConfigEntry,
) -> None:
"""A gateway whose bulk fetch raises PACKAGE_NOT_PAID_FOR still loads."""
fixtures: list[Fixture] = [
Fixture({"type:climateSensor"}, "vicare/RoomSensor1.json")
]
mock_vicare = MockPyViCare(fixtures)
mock_vicare.devices[
0
].service.fetch_all_features.side_effect = PyViCareNotSupportedFeatureError(
"PACKAGE_NOT_PAID_FOR"
)
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()
assert mock_config_entry.state is ConfigEntryState.LOADED
+2 -1
View File
@@ -201,7 +201,8 @@ async def test_turn_on_refused_while_another_quickmode_runs(
def activate_quickmode(mock_vicare: MockPyViCare, device: int, quickmode: str) -> None:
"""Mark a quickmode as active in the fixture data of a mocked device."""
for feature in mock_vicare.devices[device].service._test_data["data"]:
config = mock_vicare.devices[device]
for feature in config.service._features[config.device_id]:
if feature["feature"] == f"ventilation.quickmodes.{quickmode}":
feature["properties"]["active"]["value"] = True
return