[puddly]

The adapter firmware doesn't do any sort of filtering but does hide the MAC header so unprotected frames cannot be told apart from one another, requiring debouncing.

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 99.15%. Comparing base (7547a5e) to head (f931178).

Additional details and impacted files

@@            Coverage Diff             @@
##              dev     #279      +/-   ##
==========================================
+ Coverage   99.14%   99.15%   +0.01%     
==========================================
  Files           7        7              
  Lines         933      949      +16     
==========================================
+ Hits          925      941      +16     
  Misses          8        8              

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[zigpy-review-bot]

Clean, well-tested change: the GPDF is handed to zigpy unprocessed (MIC included), the unprotected-repeat debouncing is a sensible answer to the stripped MAC header, and the alias NotImplementedError is exactly what zigpy's GP manager already catches and falls back from. I checked it out against zigpy 2.4.0: all 145 tests pass.

One thing to fix before merging, plus two non-blocking notes (inline).

[zigpy-review-bot]

Post-merge follow-up to my earlier review (both points from that one were addressed before merge, thanks). I went through the merged head (f931178) against zigpy 2.4.0, including probes through this repo's app fixture. For protected GPDs everything checks out, and the frame layout is right: zigbee-herdsman's deCONZ parser also reads the NWK frame control directly after the two length fields, so the old reserved: LongOctetString schema was the wrong one.

Unprotected GPDs are where deCONZ ends up worse than bellows. Since the firmware strips the MAC header, unprotected GPDFs reach zigpy with frame_counter=None, while bellows passes the MAC sequence number there. deCONZ is the first radio to do this, and zigpy 2.4.0 doesn't handle None everywhere. Two of the items below are zigpy-side.

1. Commissioning an unprotected GPD with MACSeqNumCap crashes (zigpy)

GreenPowerDevice.from_commissioning copies packet.frame_counter (here None) into device.frame_counter for NoSecurity GPDs. send_pairing then sets frame_counter_present from mac_seq_num_capability and does t.uint32_t(device.frame_counter), which raises TypeError. By that point _finalize_commissioning has already registered and announced the GPD, so it shows up as a device, but the GP Pairing broadcast never goes out and proxies are never told about it. Probe: Commissioning GPDF 0C 44 33 22 11 E0 02 01 → task fails with TypeError, no pairing sent; the same packet with a MAC seq of 7 pairs normally. Treating a None counter as "not present" in send_pairing would fix it (the field is optional for SecurityLevel 0, A.3.3.5.2).

2. Commands run twice when a proxy also hears the GPD (zigpy)

The direct copy arrives with frame_counter=None, while a proxy's tunneled GP Notification carries the MAC sequence number in that field (A.3.3.4.1). GreenPowerDevice._is_packet_duplicate returns False as soon as either counter is None, so both copies are processed. zigpy does make proxies forward (send_pairing pairs groupcast to the GP group), so any proxy in range triggers this. A probe with a registered unprotected GPD gave 2 command events in either arrival order, versus 1 with a MAC seq as on bellows: a Toggle toggles twice, Step/Move commands double. The radio library can't fix this alone since it never sees the tunneled copy; zigpy would need to match counterless packets by content against recent tunneled ones.

3. Commissioning retry within 2 s is dropped (here and in zigpy)

The debounce key here (src_id, command_id, endpoint, payload) ignores RxAfterTx, and the GP manager's own debouncer keys on (src_id, frame_counter), i.e. (src_id, None). A bidirectional Commissioning followed within 2 s by a unidirectional retry with the same payload is therefore dropped, and the GPD stays pending until the window closes. On deCONZ that retry is the only path that can succeed, because the Commissioning Reply can't be transmitted for a directly heard GPD. How often real GPDs retry this quickly I don't know.

4. The 2 s window is wider than needed (here)

In my earlier review I called collapsing two quick presses unavoidable. That was wrong: spec A.1.6.3.3 bounds a GPFS (the repetitions the firmware delivers) at 5 ms, or 7 ms with RxAfterTx set. A window of a few hundred milliseconds would still absorb the repetitions plus host latency, and would keep deliberate re-presses that GP_DUPLICATE_TIMEOUT currently swallows.

Minor, not from this PR's code

• add_endpoint never adds the slot it just wrote to _written_endpoints, so the overwrite guard is dead. With zigpy 2.4.0 registering endpoints 1, 2 and 242, 2-slot firmware ends up with {0: 1, 1: 242} on every start (the ZLL endpoint is evicted); 3- and 4-slot firmware is fine. Probably harmless, but accidental.
• requires-python = ">=3.8" and the async-timeout marker are stale now that zigpy 2.4.0 requires Python 3.11.

[TheJulianJES]

Hmm, looks like I may need to tune what the bot considers a "trivial PR", as it skipped some things in the initial review. More detailed one is above now, but I've not checked any of it.

[puddly]

Unprotected GPDs are a bit tricky here because of the MAC header stripping. Unfortunately, I do not own a single GP device that fully disables encryption so the actual impact of this is hard to gauge. It's technically permitted but do any devices even exist?

[TheJulianJES]

I guess we'll find out at some point 😄