Skip to content

Import new device diagnostics - #922

Merged
puddly merged 7 commits into
zigpy:devfrom
puddly:puddly/diagnostics-import-20261005
Oct 28, 2026
Merged

puddly merged 7 commits into
zigpy:devfrom
puddly:puddly/diagnostics-import-20261005

Conversation

@puddly

@puddly puddly commented Oct 5, 2026

Copy link
Copy Markdown
Member

These new diagnostics encompass devices attached to PRs, earlier imports did not include them.

We now have over 1k devices in our test suite (1,141).

Copilot AI balanced review requested due to automatic review settings October 5, 2026 20:27

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot wasn't able to review this pull request because it exceeds the maximum number of lines (20,000). Try reducing the number of changed lines and requesting a review from Copilot again.

@zigpy-review-bot zigpy-review-bot left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The importer and decoder changes look good, but the 215 new snapshots fail test_devices_from_files on current dev. CI hasn't caught this yet: the Python 3.12 "Prepare base dependencies" job failed, so the pre-commit and test jobs were skipped.

Blocker: every new snapshot fails the round-trip. All 215 new files have "endpoint_id": null on their LQISensor and RSSISensor entities. On dev, those entities get their endpoint ID, so the full suite at this head gives 215 failed, 925 passed in test_devices_from_files, and the failures are exactly the 215 new files. The snapshots look like they were generated on a tree that includes #919: that PR changes exactly these fields to null across the existing corpus. Running python -m tools.regenerate_diagnostics on the 215 new files on top of this branch changes only those two lines per file (430 lines) and fixes all but one. If #919 is meant to land first, regenerating after rebasing onto it also works.

The last one is a test-helper issue, not the snapshot. After the regen, candeo-c-zb-rd1p-rem-0x0000000c.json still fails, deterministically, on the identify-on-join assertion. Details inline.

Verified (6 checks)
  • Full suite in a fresh worktree venv (zigpy 2.3.0, zha-quirks 2.2.2) at a6bfab4: the 215 new snapshots fail and the other 925 pass. After regenerating the 215 files, the only remaining failure is the Candeo remote. With the _get_identify_cluster change suggested inline, tests/test_discover.py passes (1159 tests).
  • #919 as the source: on #919's branch, the existing snapshots switch LQISensor/RSSISensor from the endpoint ID to null. With this branch merged into #919's head, 214 of the 215 new snapshots pass. The Candeo remote fails there too, for the identify reason below. The other failures in that tree are the 52 snapshots #920 added, which #919 hasn't regenerated yet.
  • Legacy/modern switch ("signature" in zha_data): Device.get_diagnostics_json has never written a signature key, from its introduction in #284 to now. After its nwk check, the legacy loader reads device_data["signature"] unconditionally. So a dump without a signature could only be skipped or raise a KeyError on that path before, and it can't lose a dump that used to import. Trimmed modern dumps without version are now skipped instead.
  • Stricter decode_json_value: the committed snapshots only ever carry "__type": "<class 'bytes'>" (14 occurrences). That's because ZhaJsonEncoder writes plain bytes, and decoded arrays come back out as plain JSON lists. The other listed types only show up in the raw HA dumps the importer reads. All 1140 snapshots load with the new decoder.
  • No identifying data: all 215 new snapshots have a faked ab:cd:ef:… IEEE and contain no leftover REDACTED strings.
  • Real mypy in the venv: clean. ruff check/ruff format --check on tests/common.py and tools/import_diagnostics.py: clean.

Comment thread tests/data/devices/a89g12c-arteco.json Resolved
Comment thread tests/common.py Resolved
@zigpy-review-bot zigpy-review-bot added the code quality Improvement to code quality label Oct 10, 2026
@zigpy-review-bot

Copy link
Copy Markdown
Collaborator

Pushed 5e5ef62210e08b8d0278ff7a7163ba1a6cb279e07 on request from zigpy/zha-device-handlers#5415: one new snapshot, tests/data/devices/tze2841000000-3yp57tby-ts0601-0x0000006b.json, imported from that PR's attached diagnostics. It was generated against current dev (zha-quirks 2.3.0), and its test_devices_from_files case passes there.

@codecov

codecov Bot commented Oct 28, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 97.58%. Comparing base (c552c7b) to head (7fc6ceb).

Additional details and impacted files
@@           Coverage Diff           @@
##              dev     #922   +/-   ##
=======================================
  Coverage   97.58%   97.58%           
=======================================
  Files          61       61           
  Lines       10870    10870           
=======================================
  Hits        10607    10607           
  Misses        263      263           

☔ 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.

@puddly
puddly merged commit d60052e into zigpy:dev Oct 28, 2026
10 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

code quality Improvement to code quality

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants