refactor: move accessor from service to device#774
Merged
Conversation
Splits the device-identity concern (installation/gateway/device IDs) from the HTTP/cache concern (OAuth, request building, caching). Service instances no longer hold an accessor — it lives on Device and PyViCareDeviceConfig and is passed at each call site. Prep work for a future gateway-scoped service that can serve multiple devices from a single bulk fetch. Picked up from CFenner's openviess#628 on current master since the 8-month drift (notably openviess#764's PACKAGE_NOT_PAID_FOR exception handling in ViCareCachedService) made a fresh implementation cleaner than the rebase. Same design and API shape — public surface (device.getProperty/setProperty, DeviceConfig.as*) is unchanged.
- ViCareDeviceAccessor takes int as first arg per its annotation; pass 0 instead of "[id]" in the typed setUp of test_PyViCareDeviceConfig - Rename intentionally-unused mock params to _-prefix (accessor, requested_roles) so pylint doesn't flag unused-argument
lackas
force-pushed
the
viacare-accessor-on-device
branch
from
June 15, 2026 20:42
b969615 to
277fc78
Compare
Contributor
Author
CFenner
added
new-feature
enhancement
CFenner
pushed a commit
that referenced
this pull request
Jul 14, 2026
* refactor: move roles + isGateway from Service to Device/DeviceConfig
Preparation for the viaGateway shared-service mode (next commit): when one
ViCareServiceViaGateway instance serves all devices on a gateway, the service
cannot answer per-device role questions because its roles list would be
whichever device was processed first. Move role-based decisions onto the
device-identity side of the architecture, where they belong.
- DeviceConfig.hasRoles(roles) + DeviceConfig.isGateway() use self.roles
instead of self.service.hasRoles / self.service._isGateway.
- Device base class now takes optional roles list; all DeviceConfig.as*
factories pass it through. Device.isLegacyDevice / isE3Device use
self.roles.
- HeatingDevice.get_heat_curve_formular uses self.roles for the
heatpump/E3 check.
- Service-side hasRoles / _isGateway are kept untouched for backwards
compatibility (still used internally by the per-device service for URL
shape selection).
No public API removals. Test mocks updated to pass roles via the
PyViCareDeviceConfig constructor instead of patching service.hasRoles.
* feat: add viaGateway mode for bulk per-gateway feature fetching
Adds an opt-in service mode where ONE ViCareCachedServiceViaGateway
instance serves all devices on a gateway from a single bulk API call,
instead of N per-device calls per refresh.
Endpoint used (already verified byte-identical for overlapping features):
GET /features/installations/{id}/gateways/{serial}/features?includeDevicesFeatures=true
Public API:
vicare = PyViCare()
vicare.loadViaGateway(True) # default False; must precede initWith*
vicare.initWithCredentials(...)
Per-device entry points (device.getProperty, DeviceConfig.as*) are
unchanged. The shared service is wired in PyViCare.__extract_all_devices:
one service per gateway in viaGateway mode, one per device otherwise.
Architecture:
- ViCareServiceViaGateway: transport, hits the bulk URL for fetch_all_features
and getProperty; setProperty still uses the per-device URL (writes target
one specific feature on one specific device).
- ViCareCachedServiceViaGateway: caches both getProperty and
fetch_all_features against the same cached payload. The fetch_all_features
cache is the key optimization for HA Core's DataUpdateCoordinator pattern,
where each per-device coordinator calls service.clear_cache +
service.fetch_all_features per refresh -- without it, N coordinators on a
shared service would still trigger N bulk fetches per cycle.
- Defensive error handling (PACKAGE_NOT_PAID_FOR, DeviceCommunicationError,
InternalServerError) mirrors ViCareCachedService: serve stale cache on
transient failure, raise on first-fetch failure.
Carries the design intent of CFenner's stalled #626 forward. The bulk
response test fixtures (heatbox1/heatbox2/tcu1) are lifted from that draft.
Implementation is fresh on current master because #774 made the previous
sketch incompatible (accessor lives on Device now, service is stateless),
and the original draft did not actually share the service instance across
devices.
Tests: 20 new ones across three files covering URL building, per-device
filtering of the bulk response, shared-cache behavior across devices,
TTL respect, setProperty cache invalidation, stale-cache fallback on
transient errors, and end-to-end PyViCare wiring with mocked OAuth.
Refs: #626 (CFenner draft)
Refs: home-assistant/core#173776 (raised the question)
* test(via-gateway): move ViCareCachedService import to module top
CI pylint failed with C0415 (import-outside-toplevel).
* refactor: dedupe cached-service caching and gateway role check
- extract ViCareCachedServiceBase (TTL, locking, stale-cache fallback,
write invalidation) shared by ViCareCachedService and
ViCareCachedServiceViaGateway via _fetch_uncached/_extract_entities hooks
- replace ViCareService._isGateway with module-level GATEWAY_ROLES /
is_gateway_role, also used by PyViCareDeviceConfig.isGateway
- move PyViCare.viaGateway to a class attribute
- annotate filter_features_for_device
Addresses review feedback on #777.
* fix: make cached base a ViCareService so setProperty resolves cleanly
Avoids the mixin super() lint/type noise; the concrete gateway __init__
keeps its explicit parent init (pylint super-init-not-called silenced).
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Splits the device-identity concern (installation/gateway/device IDs) from the HTTP/cache concern (OAuth, request building, caching). Service instances no longer hold an accessor — it lives on
DeviceandPyViCareDeviceConfigand is passed at each call site.Picked up from @CFenner's #628 on current master. The 8-month drift, especially #764's
PACKAGE_NOT_PAID_FOR/DeviceCommunicationError/InternalServerErrorexception handling added toViCareCachedService, made a fresh implementation cleaner than a rebase. Same design intent and API shape as #628 — closes #628.Motivation
This is prep work for a gateway-scoped service that can serve multiple devices from a single bulk fetch (
/features/installations/{id}/gateways/{serial}/features?includeDevicesFeatures=true). Today's per-devicefetch_all_features()is called N times per coordinator refresh; with a gateway service it could collapse to 1.Measured on my installation: the bulk endpoint covers every
isEnabled=truefeature on every device (deactivated features are excluded by default, but PyViCare's existing@handleNotSupported/isEnabledchecks already make missing and disabled features behaviourally equivalent). So a per-gateway architecture is functionally lossless for HA's use case — see also home-assistant/core#173776 where this came up.What changes
Device.__init__(accessor, service)— storesself.accessor, threads it throughgetProperty/setPropertyPyViCareDeviceConfig.__init__(accessor, service, ...)—device_idderived fromaccessor.device_id; allas*()constructors passself.accessorthroughViCareServiceandViCareCachedServicedrop theaccessorinstance attribute;getProperty/setProperty/fetch_all_features/buildGetPropertyUrl/reboot_gatewaytake accessor as a parameterget_available_burners(device)(free function inPyViCareHeatingDevice) takes a device instead of a service — its 4 callers updated accordinglyPyViCareRoomControland 1 inPyViCareHeatingDevicethat calledself.service.getProperty/setPropertydirectly now go through the Device delegation (self.getProperty/setProperty), which was already the established pattern in the base classViCareCachedServicekeeps the full defensive try/except forPyViCareNotPaidForError,PyViCareDeviceCommunicationError,PyViCareInternalServerErrorintroduced in fix: catch PyViCareNotPaidForError in auto-detection and diagnostics #764What stays the same
Public API surface used by consumers (HA Core, custom integrations):
device.getProperty(name)/device.setProperty(name, action, data)— unchanged signaturesDeviceConfig.as*()accessors — unchanged signatures, same return typesDeviceConfig.getConfig()still returns the accessorCode that constructs
ViCareService/ViCareCachedService/PyViCareDeviceConfigdirectly will need the new signature. ThePyViCareorchestrator itself handles the wiring.Verification
mypy PyViCare/clean (the 3 missing-stub warnings are pre-existing env-only issues that CI handles)ruff check PyViCare/ tests/— all checks passedCredit
Originally proposed by @CFenner in #628 — this PR carries the same design forward on current master.
closes #628