Migrate Aqara pet feeder ACN001 to quirks v2 - #5296
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## dev #5296 +/- ##
==========================================
- Coverage 92.60% 92.60% -0.01%
==========================================
Files 424 424
Lines 14667 14665 -2
==========================================
- Hits 13582 13580 -2
Misses 1085 1085 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
🟡 Changes recommended
The migration drops the previously injected Time cluster behavior (and needs corresponding v2 handling), which risks a functional regression for devices relying on time sync for scheduling.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
This PR migrates the Aqara Pet Feeder ACN001 quirk from a legacy v1 CustomDevice implementation to a quirks v2 QuirkBuilder definition in zhaquirks, and updates the Xiaomi test suite to construct the device using the v2 quirk fixture.
Changes:
- Replace the v1
AqaraFeederAcn001device definition with a v2QuirkBuilderregistration and explicit entity exposures. - Update feeder enums to include
Scheduleand renameRemote→HomeAssistant. - Adjust tests to use
zigpy_device_from_v2_quirkand validate the updated enum name.
File summaries
| File | Description |
|---|---|
| zhaquirks/xiaomi/aqara/feeder_acn001.py | Migrates the feeder to quirks v2 and defines exposed entities via QuirkBuilder. |
| tests/test_xiaomi.py | Updates tests to instantiate the feeder via v2 quirk construction and align expected enum values. |
Review details
Suppressed comments (1)
zhaquirks/xiaomi/aqara/feeder_acn001.py:273
- This migration drops the previously injected Time cluster (the old v1 replacement explicitly added Time.cluster_id). If the feeder relies on the Time cluster for time sync (important for onboard scheduling), this will be a behavioral regression after migrating to v2.
.friendly_name(manufacturer="Aqara", model="aqara.feeder.acn001")
.removes(OnOff.cluster_id)
.replaces(OppleCluster)
.enum(
attribute_name=OppleCluster.AttributeDefs.last_feeding_source.name,
- Files reviewed: 2/2 changed files
- Comments generated: 2
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
There was a problem hiding this comment.
🟢 Approval recommended
The v2 migration is consistent with existing quirks v2 patterns and the updated tests cover the changed behavior and API surface used by this quirk.
Review details
- Files reviewed: 2/2 changed files
- Comments generated: 0 new
- Review effort level: Lite
There was a problem hiding this comment.
🟡 Changes recommended
The updated tests don’t currently exercise the real-world “manufacturer is missing” pairing scenario that the v2 quirk explicitly relies on.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review details
Suppressed comments (1)
tests/test_xiaomi.py:1230
- Same as above: this test uses manufacturer="Aqara", but the device/quirk pairing scenario described in the quirk is that manufacturer is missing during interview. Use
Nonehere to ensure quirk resolution continues to work in that case.
device = zigpy_device_from_v2_quirk(
"Aqara",
"aqara.feeder.acn001",
cluster_ids={
- Files reviewed: 2/2 changed files
- Comments generated: 1
- Review effort level: Lite
… during interview
There was a problem hiding this comment.
🟡 Changes recommended
A new v2 sensor is declared with TOTAL_INCREASING but without a unit/device_class, which is inconsistent with existing usage and may cause HA statistics/validation issues.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review details
- Files reviewed: 2/2 changed files
- Comments generated: 1
- Review effort level: Lite
| .sensor( | ||
| attribute_name=OppleCluster.AttributeDefs.portions_dispensed.name, | ||
| cluster_id=OppleCluster.cluster_id, | ||
| state_class=SensorStateClass.TOTAL_INCREASING, |
There was a problem hiding this comment.
state_class=SensorStateClass.TOTAL_INCREASING for portions_dispensed and weight_dispensed matches the existing legacy entities in ZHA.
There was a problem hiding this comment.
Thinking about this, I could add unit="portions" and keep the legacy state_class=SensorStateClass.TOTAL_INCREASING
.sensor(
attribute_name=OppleCluster.AttributeDefs.portions_dispensed.name,
cluster_id=OppleCluster.cluster_id,
state_class=SensorStateClass.TOTAL_INCREASING,
unit="portions",
unique_id_suffix=f"{OppleCluster.cluster_id}-portions_dispensed",
translation_key="portions_dispensed_today",
fallback_name="Portions dispensed today",
)
|
Wait it 🥹 Thanks 🙏🙏🙏 |
Proposed change
PR for the migration to quirks v2 for the Aqara Pet Feeder ACN001.
This will also allow for the feeder entities in ZHA to be removed, in PR zigpy/zha#883
Split from this PR: #5294
Additional information
This prepares for changes in ZHA mentioned here: zigpy/zha#705
Device diagnostics
Updated with the diagnostics from the existing quirk:
zha-diagnostics-Aqara_aqara.feeder.acn001.json
Checklist
pre-commitchecks pass / the code has been formatted using Black