Repository navigation
Add a ZGP quirk builder and EnOcean PTM 216Z + Hue Tap quirks - #5273
Conversation
This reverts commit aac9018.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## dev #5273 +/- ##
==========================================
+ Coverage 93.34% 93.41% +0.07%
==========================================
Files 427 430 +3
Lines 14912 15076 +164
==========================================
+ Hits 13919 14083 +164
Misses 993 993 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
|
(Again, let's see how good the added tests are. Looking at the missing test coverage, it seems fine to merge without them. Still want to glance at the PR itself though.) @zigpy-review-bot Can you push a new branch to this repo and link it here which adds more test coverage (for what's new/modified by this PR)? Push and reply after review rounds. EDIT: Looks like this may already be covered by |
|
Saw the edit. I didn't push a branch, because coverage at this PR's head (343a256) is already complete for what the PR adds. On the full suite with branch coverage:
Before I saw the edit, I had prepared a test-only commit. It adds 51 test cases that pin behaviour rather than raise coverage: builder matching edge cases (inclusive SrcID bounds, IEEE prefix order, specificity, duplicate registration, |
|
@zigpy-review-bot Actually, can you still push what you had just to peek at it? Seems like it may add more coverage that's useful? (even though lines are technically covered already?) |
|
Pushed as zigpy-bot/zgp-test-coverage: puddly/zha-device-handlers@puddly/zgp...zigpy:zha-device-handlers:zigpy-bot/zgp-test-coverage (one test-only commit on top of 343a256, 6897f0a). It adds 51 test cases in tests/test_green_power.py, tests/test_enocean.py and tests/test_philips.py, covering the builder matching edge cases, PTM 216Z per-contact state and Hue Tap matching/entities/ignored commands described above. The three test files pass with it (152 passed). It has no PR of its own, and applies cleanly on the PR head if you want to cherry-pick any of it. |
There was a problem hiding this comment.
🟡 Changes recommended
Missing from_quirk=True causes duplicate button unique IDs and ignored names, while lost release frames can leave stale state that suppresses later button presses.
2 open issues
What changed in this PR
Adds Zigbee Green Power quirk support to ZHA-Quirks, with EnOcean PTM 216Z and Philips Hue Tap handlers addressing the device requests in #478 and #477.
Changes:
- Introduces a Green Power builder for device matching, naming, and custom event entities.
- Adds per-button events for Hue Tap and press/release tracking for PTM 216Z.
- Shares entity-creation logic and adds builder and device tests.
| File | Description |
|---|---|
zhaquirks/philips/hue_tap.py |
Maps Hue Tap commands to four button event entities. |
zhaquirks/enocean/ptm216z.py |
Tracks rocker contacts and emits press/release events. |
zhaquirks/enocean/__init__.py |
Introduces the EnOcean quirks package. |
zhaquirks/builder/green_power.py |
Adds Green Power matching, registration, and entity support. |
zhaquirks/builder/device.py |
Extracts shared custom entity-creation helpers. |
zhaquirks/builder/__init__.py |
Exports the Green Power builder and event constants. |
tests/test_philips.py |
Tests Hue Tap command-to-button mappings. |
tests/test_green_power.py |
Tests builder matching, validation, entities, and lifecycle. |
tests/test_enocean.py |
Tests PTM 216Z matching and contact event sequences. |
Review effort: Balanced
| if pressed == self._pressed: | ||
| return |
There was a problem hiding this comment.
We can add this sort of logic if GP frames are being consistently lost and this is required. For now, I don't think the added complexity is warranted.
TheJulianJES
left a comment
There was a problem hiding this comment.
Looks good to me overall!
(I do want to check if the bot has anything valuable but otherwise, this seems good to go.)
zigpy-review-bot
left a comment
There was a problem hiding this comment.
Approve — no blockers or must-address items at 343a256. The two issues I had noted at bfeb279 are fixed at this head (from_quirk dropped from the Green Power entities, primary-entity assertions removed from the PTM 216Z test), and the refactor that shares create_added_entities between QuirkV2Device and QuirkGreenPowerDevice behaves the same as before for v2 quirks. The builder, PTM 216Z (press/release per contact, matching by device ID and SrcID block) and Hue Tap (press-only, SrcID block 0x004xxxxx) look right to me.
Open question, not a blocker: is a diagnostics path for GPDs planned? tools/import_diagnostics / tools/regenerate_diagnostics in ZHA don't handle GPDs yet, so these entities have no snapshot coverage like regular quirks do. If you have dumps for either device, attaching them here would make adding snapshots easy once ZHA can import them.
The remaining open thread (PTM 216Z _pressed staying set after a lost release frame) is answered by the author's reply; I agree it isn't worth the extra state handling until lost GP frames show up in practice.
Verified (6 checks)
- Full suite at 343a256 on the locked zha 2.4.0 / zigpy 2.4.0: 4975 passed, 2 xfailed. With branch coverage,
ptm216z.pyandhue_tap.pyare 100%,builder/green_power.py99% (only theapplies_to()-without-device_idbranch at line 129 is unexercised), and the uncovered lines inbuilder/device.pyare existing code this PR doesn't touch. create_added_entities/_create_added_entityare a straight move out ofQuirkV2Device; the addedassert isinstance(device, Device)only guards the ZCL branch, which Green Power devices can't reach becauseadds_entityrejectsZclPlatformEntitysubclasses.- Unique IDs: zha 2.4.0's
PlatformEntityappliesunique_id_suffixas{unique_id}-{suffix}unconditionally, so each button gets a distinct ID, which the tests assert (button_a0..button_b1,button_1..button_4). - PTM 216Z press/release uses
contact_status & contactfor both vector commands, which is consistent with the ZGP 1.1.2 contact-status semantics, and the partial-release sequence is tested. Hue Tap command mapping (Toggle / RecallScene0-2 for buttons 1-4) matches Z2M'sHUE_TAP_LOOKUP, and a press-only entity matches a device that sends nothing on release. - Static 2.0 checks: no
zigpy.quirksimports in the diff; merge-base containszhaquirks/builder. - Overlap: no other open PR touches
zhaquirks/enocean/,philips/hue_tap.pyorbuilder/green_power.py; the earlier-noted import-line conflicts inbuilder/__init__.py/builder/device.pywith other builder PRs are small fixes for whichever merges second.


Proposed change
Add support for writing quirks for Zigbee Green Power devices with an implementation for an EnOcean four-button rocker switch and the Hue Tap switch. Practically all ZGP devices are going to be remotes. Differing from many other quirks in this repo, this one directly uses the ZHA entity creation API to subscribe to events and maintain a bit of state per button.
Requires zigpy/zha#868 and zigpy/zigpy#1874 (and the other WIP zigpy PRs to add ZGP management).
Part of OpenHomeFoundation/roadmap#195
Additional information
Related: #477, #478
Device diagnostics
Checklist
pre-commitchecks pass / the code has been formatted using Black