refactor: move accessor from service to device#774
Open
lackas wants to merge 2 commits into
Open
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
b969615 to
277fc78
Compare
Contributor
Author
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
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