Add per-cat feeding sensors and household activity timeline - #353
Veldkornet wants to merge 1 commit into
Conversation
b3299a4 to
8d63836
Compare
There was a problem hiding this comment.
🟡 Changes recommended
The new feeding-timeline coordinator currently ignores the existing user-configurable timeline polling interval option, so runtime behavior won’t match the integration’s configured polling setting.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
Adds new timeline-backed feeding and household-activity sensors to the SurePetCare Home Assistant integration, including an incremental polling coordinator and supporting fixtures/tests, to enable per-pet “today” stats and a combined household activity feed.
Changes:
- Introduces
feedings_today,food_today(per pet) andhousehold_activity_today(per household) sensors backed by a new feeding-timeline aggregation module and coordinator. - Implements cold-rebuild-at-midnight + incremental
since_idpolling with bounded deduplication for timeline events. - Adds fixtures, regression tests, snapshots, and a diagnostic script; updates
uv.locktopy-surepetcare==0.6.4.
File summaries
| File | Description |
|---|---|
| uv.lock | Switches py-surepetcare dependency to PyPI 0.6.4. |
| tests/test_sensor.py | Freezes time for stable snapshots; adds integration-level assertions for new sensors. |
| tests/test_feeding_timeline.py | Adds regression tests for timeline parsing/aggregation (incl. frames-based splits). |
| tests/test_feeding_timeline_coordinator.py | Adds tests for cold vs incremental polling and cursor-lag dedup behavior. |
| tests/snapshots/test_sensor.ambr | Updates entity/state snapshots for new sensors and reordered entities. |
| tests/snapshots/test_init.ambr | Adds device-registry snapshots for per-household “Household” devices. |
| tests/fixtures/feeding_timeline_frames.json | Adds fixture events with weights[].frames populated for gram-splitting tests. |
| tests/init.py | Extends mock API to serve per-household timeline events + household name lookups. |
| scripts/check_timeline_frames.py | Adds an interactive diagnostic script to inspect real timeline frames presence. |
| custom_components/surepcha/translations/en.json | Adds translations for the new sensor keys/attributes. |
| custom_components/surepcha/translations/de.json | Adds translations for the new sensor keys/attributes. |
| custom_components/surepcha/translations/sv.json | Adds translations for the new sensor keys/attributes. |
| custom_components/surepcha/sensor.py | Registers new sensors; wires per-household timeline coordinator into pet sensors + household sensor. |
| custom_components/surepcha/feeding_timeline.py | New module to aggregate per-day feeding stats + household activity feed from timeline events. |
| custom_components/surepcha/coordinator.py | Adds SurePetCareFeedingTimelineCoordinator with cold rebuild + incremental polling logic. |
Review details
- Files reviewed: 14/15 changed files
- Comments generated: 1
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
8d63836 to
8f95ce2
Compare
There was a problem hiding this comment.
🟡 Changes recommended
There are correctness issues in the new timeline paging logic and fixture consistency, plus an availability semantics deviation that should be aligned with existing entity behavior.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review details
- Files reviewed: 14/15 changed files
- Comments generated: 3
- Review effort level: Lite
New sensors per cat: feedings_today (visit count) and food_today (grams, wet/dry split), sourced from the household timeline API. New household-wide household_activity_today sensor: a chronological feed mixing feeding visits with bowl fill/zero maintenance events, with pet/device photos listed once each rather than repeated per event. feedings_today/food_today use state_class total_increasing so long-term statistics carry the sum across each midnight reset; household_activity_today has none, since its count is redundant with feedings_today's per-pet statistics. SurePetCareFeedingTimelineCoordinator rebuilds from scratch (a backward walk to local midnight) only on its first poll, after local midnight, or when there's no cursor yet; otherwise it polls incrementally via a single since_id request. The cursor lags one poll behind with a bounded seen-id set for dedup, mirroring SurePetCareHouseholdTimelineCoordinator's own polling, since timeline ids aren't strictly ordered by created_at. pet_photos/device_photos are scoped to the household they're attached to, not built from every coordinator in the config entry - otherwise a legacy multi-household entry would leak one household's photos into another's. Also: scripts/check_timeline_frames.py, a diagnostic for checking whether the timeline API's weights[].frames data is populated, and a py-surepetcare 0.6.4 lockfile sync. Test coverage: direct unit tests for the aggregation/pagination/incremental- fold logic, plus an end-to-end test through the real config-entry setup - the shared test mock now correctly answers the timeline/household-detail endpoints, rather than every feeding sensor silently reporting "unavailable" regardless of what the aggregation code did.
8f95ce2 to
b65a9c4
Compare
|
Hi! I closed it since the tests were not passing. I also think there are quite a few things that should be reworked or moved. For example, all API calls should be moved to py-surepetcare. I’m also sceptical about the custom sensor entities, as they add extra complexity that shouldn’t be necessary on top of the existing base-class entities. Another blocker was the separation of households, which is currently merged. Fixing that allowed me to add a basic timeline event, which was added in release 2.2.4. Could you review whether the latest release contains all the information you need from the events? I’m also looking into whether it would be possible to provide events alongside the pet entities, but this could cause confusion due to mismatched data. For now, the timeline is therefore provided only as events and not as actual sensors. |
|
Thanks for the context, appreciate the detail.
I'm not sure what API calls you're referring to... just to clarify, there aren't any direct calls to the Sure Petcare API here. Everything goes through
The new ones (
yes, this PR is built directly on |
|
I have taken some more time on the PR and I agree I did some wrong assumptions but its quite a big PR to cover. I apologize for that. I forgot I exposed a version of the timeline since I had no feedback on it so it never worked as expected. The new version should work much better. What is the reason for a dedicated Feeder coordinator? I assume a coordinator for all events is required but we only need one to manage the timeline (feeding,drinking,movement)? HouseholdTimelineData is a good idea though to be explicit of the data it manages. It feels like we add lots of logic in feeding_timeline to handle the weights etc but is nothttps://github.com/FredrikM97/hass-surepetcare/blob/main/custom_components/surepcha/timeline.py enough for that or what is missing? Can you elaborate? In my opinion the best would be if we did not even need the timeline.py and instead just parse the data from py-surepetcare but that will be a refactor for the future. The changes to sensor.py also changes a lot compared to existing logic. I prefer to have one base entity instead of creating a new entity per sensor since that reduce maintenance and testing. Can you elaborate why this is needed instead of using the existing setup and adding more sensors? I have been thinking to add a sensor for the timeline data, however I think the best approach would be to listen to the events (similar toSurePetCareTimelineSensorBase) but but reuse the existing SensorBase. If possible to extend it with support for the events. So your approach this is on the right track but needs to be more generic not just feeding. If we replace the sensor with event data it is important not to mix data. So sensors not show the data out of sync. This is one of the main obstacles at the moment and I don't think we should add extra sensors which show same data. You' mentioned a running total. This is better to use a helper (check wiki for example). Since Surepetcare does not natively support it we should avoid combining and keeping daily states in the codebase. Last thing is to avoid storing so much data in extra attributes. This is not recommended by HA. Since the photos never change it is better to keep them in a dedicated sensor like it is right now. Otherwise we keep a copy with each sensor update instead of just keeping one copy of it. I hope this is a better reply so we can add some of the features you requested and I apologize for the previous lack of proper response. |
|
Thanks — this is much more actionable, no need to apologise. I think you're right on most of it. Going through it in order: Dedicated feeding coordinator — agreed, one is enough.
"Is timeline.py not enough / what's missing?" — Not needing timeline.py — agreed, that's a py-surepetcare refactor. I'll open an issue there for typed New entity classes in sensor.py — the reason is they're fed by a different coordinator than Event-driven and generic — on board. Natural fit is an Running totals — agreed the Attributes / photos — I'd like to keep Listed keys are stripped before the recorder serialises the state, and before the 16 KiB check ( Its Revised shape:
Two things before I rework the branch: (a) do you want the existing disabled-by-default |
|
I would like smaller PRs if possible. Replacing the existing feeding (same goes for drinking and movement) if possible and decouple it from the existing coordinator data. I do not want to mix data that can be out of sync. I think better to have a field which describe the fields. Something like this because a field cant be wet and dry at the same time. One pain point will be to rewrite sensor tests to take data from events. They will otherwise be empty. I have checked a bit with logic similar to utility_meter but I don't think it is possible to apply on an integration? Depending on complexity it could be better to be a helper and let the integration only feed raw data to the user. This can be a separate PR to decide the proper course of action. |
Summary
feedings_today(visit count) andfood_today(total/wet/dry grams), sourced from the household timeline API. The existing (disabled-by-default)feedingsensor, which reports only the most recent feed event, is unchanged. Both usestate_class total_increasing(nottotal) so long-term statistics carry the sum correctly across each midnight reset.household_activity_todaysensor: a chronological feed mixing feeding visits with bowl fill/zero maintenance events, with pet/device photos listed once each inpet_photos/device_photosrather than repeated per event (keeps state attributes under the recorder's 16 KiB limit on an active day). Nostate_class, since its count is redundant withfeedings_today's own per-pet statistics.SurePetCareFeedingTimelineCoordinatorrebuilds from scratch (a backward walk to local midnight) only on its first poll, after local midnight, or when nothing's happened yet today; every other poll instead folds in new events via a single forwardsince_idrequest. The cursor lags one poll behind with a bounded seen-id set for dedup, mirroringSurePetCareHouseholdTimelineCoordinator's own polling, since timeline ids aren't strictly ordered bycreated_at.scripts/check_timeline_frames.py, a diagnostic for checking whether the timeline API'sweights[].framesdata is populated for a live account, and apy-surepetcare0.6.4 lockfile sync.An earlier version of this (#336) predates the current one-config-entry-per-household architecture and typed
TimelineEventAPI, and was closed unmerged and without comment; this is a rebuild against the current codebase, not a rebase of that branch.Test plan
uv run pytest— 172 passed, 1 skipped, 610 snapshots passeduv run ruff check ./uv run ruff format --check .cleanpre-commit run --all-files(ruff + mypy) cleanThis allows you to build a dashboard like the following:
