Skip to content

Fix ViCare setup by passing the accessor to fetch_all_features #180315

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

Merged
TheJulianJES merged 2 commits into from
Aug 27, 2026
+14 −13
Merged

Fix ViCare setup by passing the accessor to fetch_all_features #180315

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 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
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 homeassistant/components/vicare/coordinator.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,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,7 +60,7 @@ def _refresh(self) -> None:
"""Force a fresh fetch from the Viessmann API."""
try:
self._device.service.clear_cache()

@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.

Not introduced by this PR, but this pair does not do what the class docstring claims ("into PyViCare's internal cache so entity value_getter lambdas read fresh data on each tick") on the code path HA actually uses.

HA never enables viaGateway and always passes cacheDuration >= 60, so the service is a ViCareCachedService. Its MRO is ViCareCachedService -> ViCareCachedServiceBase -> ViCareService, and neither subclass overrides fetch_all_features — it resolves to ViCareService.fetch_all_features, which is just return self.oauth_manager.get(url) and never writes self._cache. ViCareCachedServiceViaGateway does override it to go through _get_or_update_cache; the non-gateway class is the asymmetric one.

Measured by running this exact body against real objects with a request-counting oauth manager:

service _cache populated afterwards GETs after the first entity read
ViCareCachedService (what HA gets) False 2
ViCareCachedServiceViaGateway True 1

So the cache is cleared, the fetched payload is discarded, and the next entity read fetches again: 2 requests per device per interval where the design intends 1. That directly undercuts the __init__.py:241 logic that scales cache duration by device count "to fit rate limit to number of devices".

Also worth noting: native_value is now a property read in the event loop (via CoordinatorEntity._handle_coordinator_update -> async_write_ha_state()), where the old code used a sync update() that HA ran in an executor. With the cache left empty here, that second blocking requests GET happens in the event loop, which block_async_io guards against via HTTPConnection.putrequest (strict_core=True).

The clean fix looks like it belongs upstream in PyViCare — hoisting the override into ViCareCachedServiceBase:

def fetch_all_features(self, accessor: ViCareDeviceAccessor) -> Any:
    return self._get_or_update_cache(accessor)

Fine to split that into a follow-up rather than hold up this hotfix, but the docstring shouldn't keep claiming the cache is warmed meanwhile.

self._device.service.fetch_all_features()
self._device.service.fetch_all_features(self._device.accessor)
Comment thread
TheJulianJES marked this conversation as resolved.
except PyViCareInvalidCredentialsError as err:
raise ConfigEntryAuthFailed from err
except (
Expand Down
23 changes: 12 additions & 11 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,16 +94,15 @@ 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)
Comment thread
TheJulianJES marked this conversation as resolved.
self.setProperty = Mock()
self.clear_cache = Mock()
self.roles = fixture.roles
Comment thread
TheJulianJES marked this conversation as resolved.
self.accessor = ViCareDeviceAccessor(installation_id, gateway_id, device_id)

def hasRoles(self, requested_roles: list[str]) -> bool:
"""Return true if requested roles are assigned."""
Expand Down
Loading