Add Zigbee Green Power (ZGP) protocol support#1814
Draft
nmingam wants to merge 71 commits into
Draft
Conversation
Add GP protocol constants (endpoint 242, cluster 0x0021, group ID, default ZigBeeAlliance09 link key) and a comprehensive GPDCommandID enum covering commissioning (0xE0-0xE4), on/off, level control, color control, door lock, and attribute reporting commands as defined in the ZGP specification Table 49.
Implement GP-specific cryptographic operations per ZGP spec A.1.5.4: - build_nonce(): 13-byte CCM nonce from sourceID and frame counter - encrypt/decrypt_security_key(): key protection during commissioning - encrypt/decrypt_payload(): frame payload encryption with variable security levels (NoSecurity, ShortFrameCounter, Full, Encrypted) Uses the cryptography library's AESCCM with 4-byte MIC tags. Includes 27 unit tests covering round-trips, tampering detection, wrong keys/counters, and edge cases.
Implement GPDF commissioning payload parsing per ZGP specification: - GPCommissioningPayload: full parser for commissioning command (0xE0) with options, extended options, security key, key MIC, outgoing counter, and application info (manufacturer/model/commands/clusters) - GPChannelRequestPayload: parser for channel request command (0xE3) - Bidirectional serialization with from_bytes()/to_bytes() Update zgp __init__.py to export crypto, frame, and type modules. Includes 24 unit tests covering all optional field combinations and round-trip serialization.
Implement GPDevice dataclass representing a commissioned GP device:
- sourceID-based identification with synthetic IEEE address generation
(source_id_to_ieee / ieee_to_source_id) matching zigbee2mqtt convention
- Frame counter with replay attack protection
- Security configuration (key, level, key type)
- Device capabilities from commissioning (RX-on, MAC seq, fixed location)
- model_identifier property (GreenPower_{device_id}) for quirks matching
- Full dict serialization/deserialization for persistence
GPDevice intentionally does not subclass zigpy's Device since GPDs have
no ZCL endpoints, binding tables, or standard attribute operations.
Includes 24 unit tests.
Implement the central GP controller (equivalent to zigbee-herdsman's greenPower.ts) that processes GP frames on endpoint 242, cluster 0x0021: - handle_packet(): synchronous entry point from packet_received() - GP Notification processing with device lookup and frame dispatch - Commissioning flow: parse commissioning payload, create GPDevice, decrypt security keys, send GP Pairing to proxies - Decommissioning: remove device, send unpair to proxies - Channel request handling - Commissioning window control via ProxyCommissioningMode broadcast - GP Pairing command construction and broadcast - Device persistence via load_devices()/get_devices_data() - Listener events: gp_device_joined, gp_device_left, gp_command_received Includes 30 unit tests covering packet routing, command dispatch, replay protection, commissioning lifecycle, and persistence.
Hook the GP manager into the application controller: - Instantiate GreenPowerManager as app.green_power in __init__ - Intercept GP packets (ep 242, cluster 0x0021) in packet_received() before the standard device lookup, so GP frames from unknown proxy NWK addresses are handled without triggering device discovery - Add permit_gp(time_s) method separate from standard permit() to avoid accidental GP commissioning during normal Zigbee pairing Includes 7 integration tests verifying GP routing, non-GP passthrough, unknown device handling, and permit_gp delegation.
Implement GPProxyTable to track which GP Proxy devices are forwarding frames for which GPDs from the coordinator (sink) perspective: - add_or_update(): register/update proxy forwarding entries - remove_by_source_id(): cleanup on GPD decommissioning - remove_by_proxy(): cleanup when proxy leaves network - get_proxies_for_device() / get_devices_for_proxy(): topology queries Integrate into GreenPowerManager: - Track proxy NWK on each GP Notification received - Clean up proxy entries on decommissioning Includes 17 unit tests for proxy table CRUD operations.
Comprehensive E2E tests exercising the full GP stack through the real ControllerApplication: - Full lifecycle: commissioning → command dispatch → decommissioning - Commissioning with security key - Commissioning rejection when window closed - GP Notification routing through packet_received() from unknown proxies - Replay protection across sequential and replayed frame counters - Proxy table tracking on notification and cleanup on decommission - Device persistence across save/load cycles 10 tests covering all critical paths.
All bits from 1 to 6 were shifted by +1 due to incorrectly treating MAC Sequence Number Capability as a 2-bit field (bits 0-1) instead of a single bit (bit 0). This caused every subsequent field to be read from the wrong bit position. Corrected layout per Wireshark zbee-nwk-gp dissector and zigbee-herdsman: - Bit 0: MAC Sequence Number Capability (unchanged) - Bit 1: RX On Capability (was bit 2) - Bit 2: Application Information present (was bit 3) - Bit 3: reserved - Bit 4: PAN ID request (was bit 5) - Bit 5: GP Security Key request (was bit 6) - Bit 6: Fixed Location (was bit 7, conflicted with Extended Options) - Bit 7: Extended Options field present (unchanged) Also adds missing test_fixed_location and test_extended_options_present test cases. All test byte values updated to match corrected bit positions.
Per ZGP spec Table 12, SecurityLevel 0b10 (FullFrameCounterAndMIC) provides authentication without encryption: the payload remains in cleartext and only a 4-byte MIC is appended for integrity. Previously, encrypt_payload/decrypt_payload used the same AES-CCM encryption path for all security levels, incorrectly encrypting the payload even for auth-only levels. This would break interoperability with real GPDs using SecurityLevel 0b10. Fix by using CCM* associated data (AAD) mode for auth-only levels: the payload is passed as associated_data with empty plaintext, producing a MIC that authenticates the cleartext without encrypting. Add _is_auth_only() helper to distinguish between encrypted and auth-only security levels. Add test asserting payload identity (output == input) for auth-only and a tamper detection test.
The comments incorrectly documented the bit positions: Direction was listed as bit 2 (should be bit 3, mask 0x08) and Disable Default Response as bit 3 (should be bit 4, mask 0x10). The code was correct, only the comments were wrong. Aligned comments with ZCL spec 2.4.1.1.
Add explicit cleanup for the GP manager, called during ControllerApplication.shutdown(). Cancels any running commissioning window timer and resets the commissioning state, consistent with how OTA, Backups and Topology managers handle their cleanup.
Per the current ZGP specification and all reference implementations (Silicon Labs EMBER_GP_SECURITY_LEVEL_RESERVED, ESP-IDF), SecurityLevel value 0b01 is reserved. The original ZGP 1.0 name "Short Frame Counter and MIC" was deprecated in subsequent revisions. Rename to SecurityLevel.Reserved to align with the current spec and avoid implying this is a functional security level.
The encrypt_security_key function was imported but never used. It would be needed for GP Commissioning Reply to RX-capable GPDs, which is not yet implemented.
Add missing test cases identified during code review: - ApplicationID.LPED enum value - GPDCommandID: identify, scenes, color, door lock, reporting, application description, any command, level control stop - ProxyCommissioningModeExitMode combined values - GPProxyTable.remove_by_proxy with nonexistent proxy - GPCommissioningOptions: fixed_location and extended_options bits
Change struct.pack format from 'b' (signed) to 'B' (unsigned) for the security control byte in build_nonce(). The current value 0x05 works with both, but unsigned is semantically correct for a bitfield and required for future GPP-to-GPD direction support (values >= 0x80).
The ZGP spec requires the GPDF header as CCM* associated data, but it is not available when GP frames arrive via ZCL GP Notification commands. Radio firmware (EZSP, Z-Stack) handles GP decryption natively, making this a non-issue in practice. Same approach as zigbee-herdsman.
Per ZGP spec A.3.6.1.2, the sink must maintain a duplicate filtering table to silently drop GP Notifications forwarded by multiple proxies for the same GPD frame. Without this, each proxy forwarding the same frame triggered a false "possible replay attack" warning. Implement a time-based cache keyed by (sourceID, frameCounter) with a 2-second timeout matching the spec recommendation. Duplicates are logged at DEBUG level instead of WARNING. The frame counter anti-replay check in GPDevice.update_frame_counter() remains as a security measure for genuine replay attacks outside the dedup window. Includes 5 unit tests covering first-pass, duplicate blocking, different sourceID/counter, and cache expiry.
RX-capable GPDs expect a GP Commissioning Reply (cmd 0xF0) containing the security key from the sink. This is not yet implemented. Log a warning so users know why their RX-capable device may not complete commissioning. Most consumer GPDs (EnOcean PTM 215Z, Hue Tap) are not RX-capable and are unaffected.
Add the two missing bit combinations (0b110, 0b111) so all valid 3-bit values are represented. Document that this is semantically a bitmask but uses enum3 due to zigpy's t.Struct serialization constraints (IntFlag is not compatible with bit-level packing).
Add a note to ieee_to_source_id() docstring clarifying that sourceID 0 is reserved/unspecified in the ZGP specification. Validation is left to callers rather than the conversion function itself.
Serialize last_seen as ISO 8601 string in as_dict() and restore it in from_dict(). Previously last_seen was lost on restart, showing "unknown" in the UI until the next GPD event. This improves UX for infrequently-used GP devices like door sensors. Backward compatible: from_dict gracefully handles missing last_seen field (defaults to None).
Fix all ruff violations: - D413: add blank lines after last docstring sections (Google style) - F401: remove unused imports (DeviceID, ApplicationID, FrameType, GPDCommandID in frame.py; source_id_to_ieee, DEFAULT_GP_LINK_KEY in manager.py; DeviceID in device.py) - F841: remove unused variable assignment in test_integration.py - Apply ruff format (line length 88, import ordering, string quotes)
Adjust style to match existing zigpy patterns:
- Use <ClassName ...> angle bracket format for __repr__ methods
instead of ClassName(...) constructor-like format
- Fix GPProxyTable.__repr__ singular/plural ("1 entry" vs "N entries")
- Use tests.async_mock imports instead of unittest.mock directly,
consistent with all other zigpy test files
Document two known differences compared to zigbee-herdsman: - Communication mode is always UnicastLightweight instead of dynamically choosing between Groupcast and Unicast based on the commissioning context - Security key is sent as-is in the Pairing instead of being re-encrypted via encryptSecurityKey() as zigbee-herdsman does Both are acceptable for most home networks but may cause interoperability issues in extended networks or with certain proxy firmware implementations.
Add GP Response (client command 0x06) infrastructure and implement two previously missing GP sink responses: GP Channel Configuration (0xF3): When a GPD sends a Channel Request (0xE3), the sink now responds with the coordinator's operational channel via GP Response. This allows GPDs that scan channels to lock onto the correct one without exhaustive multi-channel commissioning. GP Commissioning Reply (0xF0): When an RX-capable GPD commissions, the sink now sends a minimal Commissioning Reply (options=0x00, no key provisioning) via the forwarding proxy. This matches zigbee-herdsman's approach and allows RX-capable GPDs to complete commissioning without key exchange. Full key provisioning can be added later. Both responses use the new _send_gp_response() helper that constructs a GP Response frame and routes it through the temp master proxy. The proxy_nwk parameter is now propagated from notification handlers through to _process_commissioning and _process_channel_request.
Test coverage for the new GP sink responses: - Channel Config: response sent, correct channel, proxy fallback, invalid payload handling - Commissioning Reply: RX-capable GPD gets reply, non-RX skips it, proxy used as temp master - GP Response: packet structure (ep/cluster/profile), coordinator fallback, send failure handling Also adds network_info.channel to mock fixture for channel-dependent tests.
Add 8 tests with independently computed reference values for AES-CCM operations, replacing sole reliance on round-trip consistency tests. The vectors were generated using raw AESCCM calls with manually constructed nonces (bypassing build_nonce/encrypt_security_key), then cross-validated against the implementation. This ensures the crypto output matches specific expected ciphertext bytes, catching bugs that self-consistent round-trip tests would miss (e.g. wrong nonce structure, wrong CCM mode). Covers: key encryption/decryption, payload encryption/decryption (SecurityLevel.Encrypted), auth-only MIC (FullFrameCounterAndMIC), and nonce byte-level verification.
Fix three tests that validated implementation behavior without checking spec-defined outputs: - test_channel_config_contains_correct_channel: now verifies the GP Channel Configuration byte (channel 20 => offset 9 | basic=1 = 0x19) appears in the sent packet, per ZGP spec - test_commissioning_with_security_key: add assertion on security_key_type (extended byte 0x23 bits 2-4 = NoKey per Table 54), with spec reference in docstring - test_commissioning_reply_uses_proxy_as_temp_master: verify the sent packet is a GP Response (ZCL cmd 0x06) containing GP Commissioning Reply (gpd_cmd 0xF0), check packet count is exactly 2 (reply + pairing)
Fix outgoing_counter=0 being treated as absent due to Python falsy evaluation: - _process_commissioning: use `is not None` check instead of `or` - send_pairing: always include frame_counter in GP Pairing when adding a sink, since frame_counter=0 is a valid initial value per the ZGP spec Add test verifying that outgoing_counter=0 from the commissioning payload is preserved (not replaced by the notification frame_counter).
A.3.7.1.2.3 authenticates the key over the 4-byte SrcID, not an empty AAD. Without it, SrcID-mode devices fail commissioning with InvalidTag.
The captured 0xE0 frame now unwraps its key with the well-known link key, so drop the "OOB key, can't decrypt" fixture and assert against the real decrypted key and the join event.
Sort imports per the project ruff config and widen the link_key annotation to KeyData | bytes, since the default is a KeyData.
Release = 0x23 and the 0x60-0x68 multi-button press/release block from Table 49. Generic GP switches advertise these, so without them they show up as undefined_0xNN.
The options field is 16 bits, not 18. The requires= lambdas dereferenced top-level attributes that actually live under .options. Rename distance to gpp_gpd_link to match the spec.
- ResponseOptions gains transmit_on_endpoint_match (Figure 59) - temp_master_tx_channel is a 4-bit channel offset + 4 reserved bits - ProxyCommissioningModeOptions gains commissioning_window_present; its exit-mode sub-field is 2 bits, distinct from the 3-bit attribute enum - ProxyCommissioningModeSchema gates window on commissioning_window_present and gains the channel field - rename PairingSchema.forwarding_radius to groupcast_radius
Author
|
Status update + replies to the threads above. Applied from @puddly's review
Not done yet
Re the bigger items (BaseDevice split, SQLite persistence, quirks, splitting this PR): agreed on all of it. Suggested order from here: land the bellows fix, then carve out the types/parsing PR, then the manager on top of it. Works for you @puddly? And yes, I'll take you up on the test hardware, I'll email you. @dmatschekoThe crypto fix is in the branch now ( On the silent gaps: please retest with a router in between. Without an on-network GP proxy the coordinator does all the GPP work itself, and that path is the least reliable. @konistehrad's note above is exactly this. Any Hue / IKEA Tradfri / Sunricher / Nodon bulb in range should act as a proxy; with one forwarding the frames the gaps should go away. I'll get some ZGP devices myself so I can test it directly instead of putting all the testing work on you. @Hedda re precommissioningGood idea, and worth having on its own merits: migrating a switch off a Hue Bridge, restoring after a backup, devices that ship with an installation-code key. Not in this PR, but a clean follow-up: a small On the testing-blocker angle: commissioning isn't actually blocking anymore. The crypto fix landed and dmatscheko's Friends-of-Hue pairs over the air now. |
|
I added a Philips Hue White 800 bulb (LWW003 by Signify Netherlands B.V., 00:17:88:01:0e:c2:b7:bd) as a router updated to @nmingam's latest versions of zigpy and bellows and then repeated the pairing test twice (using There was no silent gap (see this comment) between the first two attempts. I also pressed a few buttons at random on the switch, after those two attempts (still without rebooting) but could not find the frames for them in the full log file home-assistant_2026-05-15T20-18-20.494Z.log. So the silent gap behavior is still there. Attempt 1UI logAttempt 2UI logI removed repetitive After those attempts I tried to remove the existing source_id = 0x0171F886
gp = app.green_power
ezsp = app._ezsp
addr = EmberGpAddress(
applicationId=0,
id=source_id.to_bytes(4, "little") + bytes(4),
endpoint=0,
)
(index,) = await ezsp.gpSinkTableLookup(addr=addr)
device = gp.get_device(source_id)
if device is not None:
gp.remove_device(source_id)
gp.proxy_table.remove_by_source_id(source_id)
await gp.send_pairing(device, add_sink=False)
if index != 0xFF:
await ezsp.gpSinkTableRemoveEntry(sinkIndex=index)Since then no frames from But my random button presses also don't show up in the log file, so it might not be the removal code. Another thing I saw were many log entries like that, without doing anything myself: Log entriesUI logThe Hue light regularly sends packets and then always a Could that interfere with the tests somehow? |
mineshaftgap
pushed a commit
to mineshaftgap/zigpy
that referenced
this pull request
Jun 11, 2026
Ports konistehrad's ~25-line registry pattern into zigpy.quirks without adopting his GreenPowerDevice(Device) model — keeps the zigpy#1814 GPDevice dataclass and SQLite persistence untouched (appendix §D Strategy 2). - zigpy/quirks/__init__.py: add _GP_REGISTRY list, CustomGreenPowerDevice base class with __init_subclass__(priority=…) auto-registration and match() classmethod, and get_green_power_quirk(gpd) lookup helper. The ZHA layer calls get_green_power_quirk() directly rather than hooking into get_device() (which is the Strategy-1 path we skip). - zigpy/zgp/device.py: GreenPowerDevice = GPDevice alias (konistehrad name). - zigpy/zgp/__init__.py: vocabulary aliases for cross-fork compat — GreenPowerDevice, GreenPowerDeviceData, GPSecurityLevel, GPSecurityKeyType, GREENPOWER_CLUSTER_ID. Canonical names unchanged (GPDevice, SecurityLevel, SecurityKeyType, GP_CLUSTER_ID). - zigpy/profiles/zgp.py: GREENPOWER_CLUSTER_ID = 0x0021 alias (siblings import from this module). - tests/zgp/test_gp_registry.py: 10 tests — registration mechanics, priority ordering, first-match-wins, vocabulary aliases. Acceptance: 253 zigpy GP tests pass (243 existing + 10 new). Next: P1 (huetap.py → CustomGreenPowerDevice subclass) or P0b (matcher).
mineshaftgap
pushed a commit
to mineshaftgap/bellows
that referenced
this pull request
Jun 11, 2026
Real Green Power frames emitted by a Friends of Hue class switch never reached the host: bellows dropped them with 'Data is too short' at deserialization time, as reported in zigpy/zigpy#1814 with a capture from a Busch-Jaeger 6716 U switch on a Silabs ZBT-1 stick. Three things were wrong: - The v4 schema for this callback, inherited up to v16, treated the GP address as five scattered fields (addrType, addr:uint32, applicationId, address:EUI64, endpoint). The NCP actually sends a single 10-byte EmberGpAddress struct: applicationId, an 8-byte id union, and endpoint. The existing EmberGpAddress type also assumed the wrong layout (14 bytes with separate gpdIeeeAddress and sourceId fields). - The v17 override uses sl_GpStatus, a strict enum that only accepts 0x00..0x07. Current ZBT-1 firmware returns higher status bytes for frames that were not matched in the proxy table (0x7C observed on the captured frame), which dropped the whole callback before the address was even read. - EZSP v16 appends an SlRxPacketInfo struct after the LVBytes payload. zigbee-herdsman's ember adapter gates the read on 'version >= 0x10', so v13 and v14 do not carry this trailer but v16 does. The previous override that flowed through from v13 was one field short on v16. Fix EmberGpAddress to the correct 10-byte layout, add a v13 override of gpepIncomingMessageHandler that uses a plain uint8_t for status so unexpected values reach the host, and add a dedicated v16 override that appends the SlRxPacketInfo trailer. v14 picks the v13 override up through the _REPLACEMENTS loop. v17+ keep their own schema which now also benefits from the struct fix. Add tests driven by the exact bytes captured by the community tester, a smoke test on v14 to verify the schema flows through inheritance, and a v16 test that appends a synthetic SlRxPacketInfo to the same capture to exercise the trailer parsing.
mineshaftgap
pushed a commit
to mineshaftgap/bellows
that referenced
this pull request
Jun 11, 2026
Two CI failures on this PR: 1. pytest on Python 3.14 (and all other versions) fails with `AttributeError: module 'zigpy.zgp.types' has no attribute 'GP_ENDPOINT'`. These constants (``GP_ENDPOINT``, ``GP_CLUSTER_ID``) are not exported by the current zigpy release -- they would be added by zigpy/zigpy#1814, which is still unreleased. Depending on them here introduces a hard coupling to an unreleased upstream. 2. pre-commit (black) rewrites the aligned hex-dump comments in the BJ6716U capture and the split `bellows.ezsp.v13.commands.COMMANDS` lookup. Fix both by: - Defining ``GP_ENDPOINT`` (242), ``GP_CLUSTER_ID`` (0x0021) and ``GP_PROFILE_ID`` (0xA1E0) as module-level constants in ``bellows.zigbee.application``, citing the ZGP 1.1b profile. These values are stable and do not need to travel across repos. - Dropping the now-unused ``zigpy.zgp.types`` import from ``tests/test_application.py`` and replacing the assertions with the same literals. - Running black to compact the inline comments so pre-commit is idempotent. No behaviour change. All 22 EZSP v13/v14/v16 tests pass against a venv with PyPI zigpy (which does not ship GP_ENDPOINT), confirming the fix removes the upstream dependency.
mineshaftgap
pushed a commit
to mineshaftgap/zigpy
that referenced
this pull request
Jun 11, 2026
Ports konistehrad's ~25-line registry pattern into zigpy.quirks without adopting his GreenPowerDevice(Device) model — keeps the zigpy#1814 GPDevice dataclass and SQLite persistence untouched (appendix §D Strategy 2). - zigpy/quirks/__init__.py: add _GP_REGISTRY list, CustomGreenPowerDevice base class with __init_subclass__(priority=…) auto-registration and match() classmethod, and get_green_power_quirk(gpd) lookup helper. The ZHA layer calls get_green_power_quirk() directly rather than hooking into get_device() (which is the Strategy-1 path we skip). - zigpy/zgp/device.py: GreenPowerDevice = GPDevice alias (konistehrad name). - zigpy/zgp/__init__.py: vocabulary aliases for cross-fork compat — GreenPowerDevice, GreenPowerDeviceData, GPSecurityLevel, GPSecurityKeyType, GREENPOWER_CLUSTER_ID. Canonical names unchanged (GPDevice, SecurityLevel, SecurityKeyType, GP_CLUSTER_ID). - zigpy/profiles/zgp.py: GREENPOWER_CLUSTER_ID = 0x0021 alias (siblings import from this module). - tests/zgp/test_gp_registry.py: 10 tests — registration mechanics, priority ordering, first-match-wins, vocabulary aliases. Acceptance: 253 zigpy GP tests pass (243 existing + 10 new). Next: P1 (huetap.py → CustomGreenPowerDevice subclass) or P0b (matcher).
mineshaftgap
pushed a commit
to mineshaftgap/bellows
that referenced
this pull request
Jun 12, 2026
Real Green Power frames emitted by a Friends of Hue class switch never reached the host: bellows dropped them with 'Data is too short' at deserialization time, as reported in zigpy/zigpy#1814 with a capture from a Busch-Jaeger 6716 U switch on a Silabs ZBT-1 stick. Three things were wrong: - The v4 schema for this callback, inherited up to v16, treated the GP address as five scattered fields (addrType, addr:uint32, applicationId, address:EUI64, endpoint). The NCP actually sends a single 10-byte EmberGpAddress struct: applicationId, an 8-byte id union, and endpoint. The existing EmberGpAddress type also assumed the wrong layout (14 bytes with separate gpdIeeeAddress and sourceId fields). - The v17 override uses sl_GpStatus, a strict enum that only accepts 0x00..0x07. Current ZBT-1 firmware returns higher status bytes for frames that were not matched in the proxy table (0x7C observed on the captured frame), which dropped the whole callback before the address was even read. - EZSP v16 appends an SlRxPacketInfo struct after the LVBytes payload. zigbee-herdsman's ember adapter gates the read on 'version >= 0x10', so v13 and v14 do not carry this trailer but v16 does. The previous override that flowed through from v13 was one field short on v16. Fix EmberGpAddress to the correct 10-byte layout, add a v13 override of gpepIncomingMessageHandler that uses a plain uint8_t for status so unexpected values reach the host, and add a dedicated v16 override that appends the SlRxPacketInfo trailer. v14 picks the v13 override up through the _REPLACEMENTS loop. v17+ keep their own schema which now also benefits from the struct fix. Add tests driven by the exact bytes captured by the community tester, a smoke test on v14 to verify the schema flows through inheritance, and a v16 test that appends a synthetic SlRxPacketInfo to the same capture to exercise the trailer parsing.
mineshaftgap
pushed a commit
to mineshaftgap/bellows
that referenced
this pull request
Jun 12, 2026
Two CI failures on this PR: 1. pytest on Python 3.14 (and all other versions) fails with `AttributeError: module 'zigpy.zgp.types' has no attribute 'GP_ENDPOINT'`. These constants (``GP_ENDPOINT``, ``GP_CLUSTER_ID``) are not exported by the current zigpy release -- they would be added by zigpy/zigpy#1814, which is still unreleased. Depending on them here introduces a hard coupling to an unreleased upstream. 2. pre-commit (black) rewrites the aligned hex-dump comments in the BJ6716U capture and the split `bellows.ezsp.v13.commands.COMMANDS` lookup. Fix both by: - Defining ``GP_ENDPOINT`` (242), ``GP_CLUSTER_ID`` (0x0021) and ``GP_PROFILE_ID`` (0xA1E0) as module-level constants in ``bellows.zigbee.application``, citing the ZGP 1.1b profile. These values are stable and do not need to travel across repos. - Dropping the now-unused ``zigpy.zgp.types`` import from ``tests/test_application.py`` and replacing the assertions with the same literals. - Running black to compact the inline comments so pre-commit is idempotent. No behaviour change. All 22 EZSP v13/v14/v16 tests pass against a venv with PyPI zigpy (which does not ship GP_ENDPOINT), confirming the fix removes the upstream dependency.
mineshaftgap
pushed a commit
to mineshaftgap/zigpy
that referenced
this pull request
Jun 12, 2026
Ports konistehrad's ~25-line registry pattern into zigpy.quirks without adopting his GreenPowerDevice(Device) model - keeps the zigpy#1814 GPDevice dataclass and SQLite persistence untouched (appendix sec.D Strategy 2). - zigpy/quirks/__init__.py: add _GP_REGISTRY list, CustomGreenPowerDevice base class with __init_subclass__(priority=...) auto-registration and match() classmethod, and get_green_power_quirk(gpd) lookup helper. The ZHA layer calls get_green_power_quirk() directly rather than hooking into get_device() (which is the Strategy-1 path we skip). - zigpy/zgp/device.py: GreenPowerDevice = GPDevice alias (konistehrad name). - zigpy/zgp/__init__.py: vocabulary aliases for cross-fork compat - GreenPowerDevice, GreenPowerDeviceData, GPSecurityLevel, GPSecurityKeyType, GREENPOWER_CLUSTER_ID. Canonical names unchanged (GPDevice, SecurityLevel, SecurityKeyType, GP_CLUSTER_ID). - zigpy/profiles/zgp.py: GREENPOWER_CLUSTER_ID = 0x0021 alias (siblings import from this module). - tests/zgp/test_gp_registry.py: 10 tests - registration mechanics, priority ordering, first-match-wins, vocabulary aliases. Acceptance: 253 zigpy GP tests pass (243 existing + 10 new). Next: P1 (huetap.py -> CustomGreenPowerDevice subclass) or P0b (matcher).
mineshaftgap
pushed a commit
to mineshaftgap/bellows
that referenced
this pull request
Jun 12, 2026
Real Green Power frames emitted by a Friends of Hue class switch never reached the host: bellows dropped them with 'Data is too short' at deserialization time, as reported in zigpy/zigpy#1814 with a capture from a Busch-Jaeger 6716 U switch on a Silabs ZBT-1 stick. Three things were wrong: - The v4 schema for this callback, inherited up to v16, treated the GP address as five scattered fields (addrType, addr:uint32, applicationId, address:EUI64, endpoint). The NCP actually sends a single 10-byte EmberGpAddress struct: applicationId, an 8-byte id union, and endpoint. The existing EmberGpAddress type also assumed the wrong layout (14 bytes with separate gpdIeeeAddress and sourceId fields). - The v17 override uses sl_GpStatus, a strict enum that only accepts 0x00..0x07. Current ZBT-1 firmware returns higher status bytes for frames that were not matched in the proxy table (0x7C observed on the captured frame), which dropped the whole callback before the address was even read. - EZSP v16 appends an SlRxPacketInfo struct after the LVBytes payload. zigbee-herdsman's ember adapter gates the read on 'version >= 0x10', so v13 and v14 do not carry this trailer but v16 does. The previous override that flowed through from v13 was one field short on v16. Fix EmberGpAddress to the correct 10-byte layout, add a v13 override of gpepIncomingMessageHandler that uses a plain uint8_t for status so unexpected values reach the host, and add a dedicated v16 override that appends the SlRxPacketInfo trailer. v14 picks the v13 override up through the _REPLACEMENTS loop. v17+ keep their own schema which now also benefits from the struct fix. Add tests driven by the exact bytes captured by the community tester, a smoke test on v14 to verify the schema flows through inheritance, and a v16 test that appends a synthetic SlRxPacketInfo to the same capture to exercise the trailer parsing.
mineshaftgap
pushed a commit
to mineshaftgap/bellows
that referenced
this pull request
Jun 12, 2026
Two CI failures on this PR: 1. pytest on Python 3.14 (and all other versions) fails with `AttributeError: module 'zigpy.zgp.types' has no attribute 'GP_ENDPOINT'`. These constants (``GP_ENDPOINT``, ``GP_CLUSTER_ID``) are not exported by the current zigpy release -- they would be added by zigpy/zigpy#1814, which is still unreleased. Depending on them here introduces a hard coupling to an unreleased upstream. 2. pre-commit (black) rewrites the aligned hex-dump comments in the BJ6716U capture and the split `bellows.ezsp.v13.commands.COMMANDS` lookup. Fix both by: - Defining ``GP_ENDPOINT`` (242), ``GP_CLUSTER_ID`` (0x0021) and ``GP_PROFILE_ID`` (0xA1E0) as module-level constants in ``bellows.zigbee.application``, citing the ZGP 1.1b profile. These values are stable and do not need to travel across repos. - Dropping the now-unused ``zigpy.zgp.types`` import from ``tests/test_application.py`` and replacing the assertions with the same literals. - Running black to compact the inline comments so pre-commit is idempotent. No behaviour change. All 22 EZSP v13/v14/v16 tests pass against a venv with PyPI zigpy (which does not ship GP_ENDPOINT), confirming the fix removes the upstream dependency.
mineshaftgap
pushed a commit
to mineshaftgap/zigpy
that referenced
this pull request
Jun 12, 2026
Ports konistehrad's ~25-line registry pattern into zigpy.quirks without adopting his GreenPowerDevice(Device) model - keeps the zigpy#1814 GPDevice dataclass and SQLite persistence untouched (appendix sec.D Strategy 2). - zigpy/quirks/__init__.py: add _GP_REGISTRY list, CustomGreenPowerDevice base class with __init_subclass__(priority=...) auto-registration and match() classmethod, and get_green_power_quirk(gpd) lookup helper. The ZHA layer calls get_green_power_quirk() directly rather than hooking into get_device() (which is the Strategy-1 path we skip). - zigpy/zgp/device.py: GreenPowerDevice = GPDevice alias (konistehrad name). - zigpy/zgp/__init__.py: vocabulary aliases for cross-fork compat - GreenPowerDevice, GreenPowerDeviceData, GPSecurityLevel, GPSecurityKeyType, GREENPOWER_CLUSTER_ID. Canonical names unchanged (GPDevice, SecurityLevel, SecurityKeyType, GP_CLUSTER_ID). - zigpy/profiles/zgp.py: GREENPOWER_CLUSTER_ID = 0x0021 alias (siblings import from this module). - tests/zgp/test_gp_registry.py: 10 tests - registration mechanics, priority ordering, first-match-wins, vocabulary aliases. Acceptance: 253 zigpy GP tests pass (243 existing + 10 new). Next: P1 (huetap.py -> CustomGreenPowerDevice subclass) or P0b (matcher).
mineshaftgap
pushed a commit
to mineshaftgap/zigpy
that referenced
this pull request
Jun 12, 2026
…docstring Remove comments naming konistehrad/nmingam/zigpy#1814/Strategy-2 from: - zgp/__init__.py: collapse verbose alias block header to "Convenience aliases." - zgp/__init__.py: remove cluster-id alias comment - profiles/zgp.py: remove "Vocabulary alias - konistehrad imports" comment - zgp/device.py: remove "konistehrad's fork" alias comment - tests/zgp/fixtures/busch_jaeger_6716u.py: remove "See zigpy#1814" ref Also add required blank line after GPDeviceType class docstring (ruff D204).
mineshaftgap
pushed a commit
to mineshaftgap/zigpy
that referenced
this pull request
Jun 13, 2026
Bring the Phase-0b Hue Tap tests into the fork from the offline zgp-lab (originally authored against d154a9f, lab commit 999cca9). They cover the SecurityLevel.NoSecurity / NoKey dispatch path that zigpy#1814's existing Busch-Jaeger fixture (FullFrameCounterAndMIC) does not exercise: - fixtures/hue_tap.py - SrcID 0x0040F4E4, device_id 0x02, four button frames (Toggle 0x22, RecallScene0-2 0x10/0x11/0x12) - test_hue_tap.py - command IDs are known to GPDCommandID (static), and each unencrypted button frame fires a CommandReceived via the manager Both pass against the current rebased branch unchanged.
|
Hi nmingam & puddly, I've been following your Green Power work in #1814 / bellows#713. Thank you for it, it's a I know commissioning isn't the blocker anymore, so really this is just a second real-hardware data A few things I built on top that seem to line up with puddly's review notes, in case they're useful:
I saw you're short on time until end of June, no rush at all. I'd much rather slot into your plan Would the SQLite persistence be useful to fold in? If so, how would you prefer I contribute? PRs Thanks again! mineshaftgap |
Author
|
Two replies in one. @mineshaftgapThis is great, thank you. A Hue Tap is a useful second data point: it's a scene controller (Toggle + RecallScene0/1/2), not a Friends-of-Hue rocker, so it exercises a different command set. Short answer: yes, I'd like to fold the SQLite persistence in. It's the headline gap right now, the manager keeps devices in memory only so everything is lost on restart. How I'd like to sequence it, to match the order agreed with @puddly:
So persistence sits on step 3. Cleanest for me would be a PR against my branch ( Two things to line up so we don't collide:
I'd defer proxy-table persistence for now (it self-heals within seconds of the first notification after restart). Device table only. Your v4 bellows schema fix: please send it to #713, we don't have it yet. On the HA integration: understood that your No rush on my side either, I'm short on time until end of June. @dmatschekoThanks for the retest, and get well, no rush. Good news first: with the Hue in range the silent gap is gone. That confirms the proxy path works. On commissioning "not completing": this isn't crypto anymore. Your switch already commissioned back on May 10. It's a TX-only device, so from its side it's paired and it won't re-send On the blackout after your removal script: To get back to a clean state now:
Heads up: it still won't survive a restart until the persistence PR lands. That's the one to wait for. |
|
Sounds good - the bellows -> types -> manager+persistence order works for me, and persistence on step 3 is right. I've opened the v4 bellows fix as a PR against your branch (nmingam/bellows#1). Here are the two things to line up so we don't end up with two v16 definitions.
CREATE TABLE gp_devices_v16 (
source_id INTEGER NOT NULL,
device_id INTEGER NOT NULL,
security_key TEXT,
security_level INTEGER NOT NULL,
security_key_type INTEGER NOT NULL,
frame_counter INTEGER NOT NULL,
manufacturer_id TEXT,
model_id TEXT,
gpd_commands TEXT NOT NULL,
server_clusters TEXT NOT NULL,
client_clusters TEXT NOT NULL,
mac_seq_num_capability INTEGER NOT NULL,
rx_on_capability INTEGER NOT NULL,
fixed_location INTEGER NOT NULL,
last_seen TEXT
);
CREATE UNIQUE INDEX gp_devices_idx_v16 ON gp_devices_v16(source_id);It rehydrates through your existing Frame-counter writes. Agreed, and that's exactly how it's wired - the counter is only persisted on the accepted path. The persistence listener subscribes to the manager's Device table only on my side too - I've left the proxy table out deliberately, since it self-heals from the first forwarded notification after restart and a live write path would fire per frame. And understood on the HA side - the |
Author
|
@mineshaftgap thanks, v4 fix is merged. For the rest, let's keep it on this PR rather than reopen, so we keep puddly's review thread, and he wanted PRs small and split. So:
Do you agree? |
|
Agreed on all three.
One thing so the two v16 defs don't collide: the |
Collaborator
|
@mineshaftgap For now, I think it would be best to get this PR merged in stages. It has already grown to over 5000 lines of added code and will continue to grow if new features are added in. We're refactoring quirks at the moment so I think those are best left untouched. IMO, these parts of this can already be merged as-is:
The parsing code and associated tests are the bulk of the PR and should be good to go. The manager, base device, database, and quirks side will need the most iteration and should be split out. Could you also adjust the comment style to match the rest of the repo? The docstrings currently follow the Google style guide but we don't really use this, it's very very verbose. If possible, propagate the comments inline with the relevant code instead of growing the docstrings to multiple paragraphs. |
|
Sounds good - staging the parsing core ( The database piece is already split out as a standalone, device-table-only PR against nmingam's branch On comment style: agreed. For what it's worth, that persistence PR already keeps to inline comments |
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
6 participants
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.
Summary
Add comprehensive Zigbee Green Power (ZGP) protocol support to zigpy, implementing the GP Sink role as defined in the ZGP specification. This is the most requested missing feature (#341, 175+ thumbs-up).
New modules (
zigpy/zgp/)types.pycrypto.pyframe.pydevice.pyproxy.pymanager.pyManager capabilities (equivalent to zigbee-herdsman's
greenPower.ts)Integration (
application.py)packet_received()before device lookup, so frames from unknown proxy NWK addresses don't trigger device discoverypermit_gp(time_s)method separate from standardpermit()green_power.shutdown()called during controller shutdownKnown limitations
ApplicationID.SrcID(0b000) supported — same as zigbee-herdsman; no known consumer GPD uses IEEE modeCommissioningNotificationSchemahas a bit-field alignment issue (18 bits) in the existing GP cluster definition from Green Power Clusters and supporting Schemas #1659 — documented in testsTesting
ruff check+ruff format: 0 violationsTest plan
pytest tests/zgp/— 216 GP tests passpytest tests/test_application.py— 102 existing tests pass (no regression)ruff check zigpy/zgp/ tests/zgp/— 0 violationsResolves #341