fix: Fix issues with the cache clobbering information for each device#614
fix: Fix issues with the cache clobbering information for each device#614allenporter merged 5 commits intoPython-roborock:mainfrom
Conversation
There was a problem hiding this comment.
Pull request overview
This PR introduces a new per-device caching structure to fix issues where device information was being clobbered across multiple devices. The changes introduce DeviceCacheData to store device-specific information and migrate from global cache fields to per-device storage indexed by device DUID.
- Introduces
DeviceCacheDataclass to encapsulate per-device cached information - Adds
device_infofield toCacheDatafor storing per-device cache data indexed by device DUID - Deprecates existing top-level cache fields in favor of per-device storage
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Lash-L
left a comment
Lash-L
left a comment
There was a problem hiding this comment.
few small comments - I do not think this PR should be held up for these - maybe in a follow up later.
As well, what will happen right now for users who have a CacheData that is stored but now are have completely different typing? I don't think we have a versioning system or anything in place but maybe i have missed it
But that's more on the HA side, but just something I want to make sure we have covered
| if device_cache_data.home_map_content_base64 is None: | ||
| device_cache_data.home_map_content_base64 = {} |
There was a problem hiding this comment.
may be worth setting home_map_content_base64 to a default factory so we never have to worry about it being none.
There was a problem hiding this comment.
I realize we set the default value to dict and this can never be none. I'll drop this in a followup PR.
| """Home cache content for each map data indexed by map_flag. | ||
|
|
||
| This is deprecated in favor of `home_map_content_base64`. | ||
| This is deprecated. Use the per-device `home_map_content_base64` field instead. |
There was a problem hiding this comment.
do we need to hold onto the deprecated value anymore?
There was a problem hiding this comment.
I think i want to just let it die and we can drop the old field name in the next major release as a breaking change.
My plan was to bump the store version and blow away the old cache entirely. |
Perfect! |
allenporter
force-pushed
the
device-cache
branch
from
57900cf to
2424f72
Compare
2 participants
Update cache interface to add a device specific cache.
This fixes invalid use today where the device data is not stored based on device id in all cases.
Related to #615