Skip to content

Support zigpy 2.0.0 - #179

Draft
TheJulianJES wants to merge 8 commits into
zigpy:devfrom
TheJulianJES:tjj/zigpy-2.0-compat
Draft

Support zigpy 2.0.0#179
TheJulianJES wants to merge 8 commits into
zigpy:devfrom
TheJulianJES:tjj/zigpy-2.0-compat

Conversation

@TheJulianJES

@TheJulianJES TheJulianJES commented Jun 29, 2026

Copy link
Copy Markdown
Contributor

This PR updates zigpy-xbee to be more compatible with zigpy 2.0.0, without relying on the presence of zha-quirks.

As zha-quirks now depends on ZHA, which pulls in zigpy-xbee, I noticed some test failures of the deprecated CustomCluster usage in all quirks tests. These are addressed by this.

Do note this PR was not tested.

AI summary

Why

  • zigpy 2.0.0 moved the quirks API out of zigpy (into zha-device-handlers). zigpy.quirks is now only a deprecation shim that lazily re-imports CustomDevice/CustomCluster from zhaquirks.legacy. This gives zigpy-xbee an implicit, undeclared dependency on zhaquirks: under Home Assistant it kept working (ZHA installs zha-quirks, so the shim resolves — only DeprecationWarnings were emitted), but a standalone install fails at import with ModuleNotFoundError: No module named 'zhaquirks'. zigpy-xbee should not depend on zha-device-handlers at all.
  • zigpy recently switched its serial layer from pyserial-asyncio-fast to serialx, whose Serial class has no BAUDRATES attribute. This breaks the baudrate setter on the path that switches baud to auto-enter API mode (init_api_mode); a coordinator already in API mode does not hit it.

Changes

  • Reimplement XBeeGroup/XBeeGroupResponse as plain Groups subclasses kept out of the global cluster registry (_skip_registry), and XBeeCoordinator as a zigpy.device.Device that builds its endpoint and clusters directly instead of via a quirks CustomDevice. add_endpoint registers endpoints directly rather than mutating a quirks replacement dict.
  • Replace the deprecated Device.update_last_seen() call.
  • Define the standard baudrate list on Gateway so validation no longer depends on the serial backend.
  • Bump minimums: zigpy>=2.0.0 (was 0.70.0), requires-python>=3.11 (was 3.8); bump the pyupgrade target to --py311-plus; drop the unused, unmaintained asynctest test dependency.
  • Remove tox.ini, which is no longer used (CI runs via the shared zigpy/workflows workflow).
  • Apply the resulting Python 3.11+ modernizations: rewrite the deprecated asyncio.TimeoutError alias to the builtin TimeoutError, and replace typing.Dict/Optional with builtin generics and PEP 604 unions (unused imports dropped).

Testing

  • pytest: 105 passed, 100% coverage, with DeprecationWarning treated as errors.
  • CI green on Python 3.11, 3.12, 3.13, and 3.14; pre-commit (black, flake8, isort, ruff, mypy, pyupgrade, autoflake, codespell) all pass.
  • Verified against both zigpy 2.0.0 and 1.5.1.

zigpy 2.0.0 moved the quirks API out of zigpy into zha-device-handlers,
so importing CustomDevice/CustomCluster from zigpy.quirks now fails with
ModuleNotFoundError (zhaquirks is not, and must not be, a dependency).

- Reimplement XBeeGroup/XBeeGroupResponse as plain Groups subclasses kept
  out of the global cluster registry via _skip_registry.
- Reimplement XBeeCoordinator as a zigpy.device.Device that builds its
  coordinator endpoint and clusters directly instead of via a quirks
  CustomDevice 'replacement' dict.
- Rewrite ControllerApplication.add_endpoint to register endpoints and
  clusters directly rather than mutating the quirks replacement dict.
- Replace the deprecated Device.update_last_seen() call with a direct
  last_seen assignment.
zigpy 2.0.0 switched its serial layer from pyserial-asyncio-fast to
serialx, whose Serial class does not expose a BAUDRATES attribute. The
baudrate setter (used when entering AT command mode) relied on
self._transport.serial.BAUDRATES and would raise AttributeError.

- Define the standard baudrate list on Gateway and validate against it.
- Drop the stale 'import serial_asyncio_fast' from the uart tests and
  patch zigpy.serial.create_serial_connection (what uart.connect actually
  calls) instead.
- Require zigpy>=2.0.0 (where the quirks API was moved out) instead of the
  long-outdated 0.70.0 floor.
- Require Python >=3.11 to match zigpy 2.0.0, and update the tox env list
  (py311-py313) and pyupgrade target (--py311-plus) accordingly.
- Drop the unused, unmaintained asynctest test dependency; the tests use
  unittest.mock.
The --py311-plus pyupgrade target rewrites the deprecated asyncio.TimeoutError
alias to the builtin TimeoutError and replaces typing.Dict/Optional with
builtin generics and PEP 604 unions; autoflake drops the now-unused imports.
CI runs via the shared zigpy/workflows workflow, not tox; the file is no
longer referenced.
@codecov

codecov Bot commented Jun 29, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 100.00%. Comparing base (f5f9d6f) to head (7187a17).

Additional details and impacted files
@@            Coverage Diff            @@
##               dev      #179   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files            7         7           
  Lines          718       733   +15     
=========================================
+ Hits           718       733   +15     

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

{"status": foundation.Status},
direction=foundation.Direction.Client_to_Server,
),
}

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.

This construct looks really odd but it apparently is the correct way 😓

Image

The ZCL header does not flip the manufacturer specific bit either.

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.

I'm not actually seeing where this is used. #46 introduced the change.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

@zigpy-review-bot Also post inline reply here for what/if this is used, or not.

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.

Answering the "I'm not actually seeing where this is used" — checked against current zigpy (2.1.0) by probing it, not just by reading. Short version: the request half is still on a live path, the response half is unreachable, so the feature as a whole cannot complete today.

XBeeGroup (0x0006) is live on the transmit side. It inherits ep_attribute = "groups", so Endpoint.add_to_group() resolves self.groups to it (zigpy/endpoint.py:139) and Device.add_to_group() walks every non-ZDO endpoint. ZHA's one coordinator-side caller is LightLinkGroupJoin.async_configure_cluster (zha/application/platforms/virtual.py:223-225), which adds the coordinator to a LightLink device's groups, or to 0x0000 as a fallback. Driving that against a freshly built XBeeCoordinator, the frame really does go out the way the Digi Group Table API expects — profile 0xC105, cluster 0x0006, endpoint 0xE6.

XBeeGroupResponse (0x8006), and its remove_all_response, is dead code on current zigpy. Two independent reasons, both in zigpy and both long after #46:

  • Since zigpy 0.82.0 (zigpy#1628, 2025-07-30) Device.packet_received() only parses ZCL for the ZHA and ZLL profiles; everything else goes to custom_profile_packet_received(), which logs and drops it. Endpoint 0xE6 is profile 0xC105, so every reply on it is discarded before any cluster is consulted — whether the module answers on 0x8006 or on 0x0006.
  • Since zigpy 0.81.0 (zigpy#1622, 2025-07-29) replies are matched on (endpoint, cluster, direction, tsn), with the ^ 0x8000 flip applied only to ZDO endpoints — so a 0x8006 reply cannot match a 0x0006 request regardless. On 0.81.0 the frame still reached XBeeGroupResponse and parsed correctly, but the pending request stayed unresolved. It last worked end to end on ≤ 0.80.x, where pending requests were keyed by TSN alone — which is what XBee Group subscription. #46 was written against.

Net effect today: coordinator.add_to_group(...) still transmits, and the module presumably does register the group, but the caller waits out the APS reply timeout and then raises.

None of this is caused by this PR, and nothing here blocks it — the two clusters carry over unchanged and cost nothing to keep. It does mean neither XBeeGroupResponse nor the remove_all_response definition is exercised by anything at the moment, on hardware or in tests. If XBee group support is worth reviving, zigpy 2.1.0's new _subscribe_to_multicast_group() / _unsubscribe_from_multicast_group() hooks (zigpy#1864) are the natural seam, and it would additionally need zigpy to stop dropping Digi-profile replies.

What I ran

Coordinator built from this branch, zigpy 2.1.0, no zhaquirks installed.

Transmit side — ep.add_to_group(0x1234, "grp") with the radio stubbed out:

TX: profile 0xc105  cluster 0x6  src_ep 0xe6  tsn 1  data 010100341203677270
pending request keys: [ResponseKey(endpoint_id=230, cluster_id=6, direction=Direction.Server_to_Client, tsn=1)]

(010100341203677270 = cluster-specific Groups.add(group_id=0x1234, group_name='grp').)

Receive side — feeding a synthetic add_response back in, three ways:

reply profile 0xc105 / cluster 0x8006  -> "Received packet with custom profile 0xc105, ignoring"; add_to_group unresolved
reply profile 0xc105 / cluster 0x0006  -> same, unresolved            (so the response cluster ID is not what breaks it)
reply profile 0x0104 / cluster 0x0006  -> resolved, Status.SUCCESS    (what zigpy expects today)

The same probe on zigpy 0.81.0 (the release between the two changes above): the frame is deserialized by XBeeGroupResponseXBeeGroupResponse:add_response(status=Status.SUCCESS, group_id=0x1234) reaches its handle_message — while the pending 0x0006 request still goes unresolved.

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

Reviewed at 1a14981. No blockers — the migration is faithful, and I verified the load-bearing parts empirically rather than by reading.

What I ran

Fresh worktree + venv, uv pip install -e ".[testing]" -r requirements_test.txt, without zha-quirks installed (the whole point of the PR):

  • pytest: 105 passed against zigpy 2.1.0, and 105 passed against 2.0.0 — the declared floor in pyproject.toml actually holds.
  • pre-commit run --all-files: pyupgrade / autoflake / black / flake8 / isort / codespell / mypy / ruff all pass.
  • Premise confirmed: on the base branch, zigpy.quirks.CustomDevice raises ModuleNotFoundError: No module named 'zhaquirks' — so zigpy_xbee.zigbee.application currently fails at class-definition time on a standalone zigpy 2.x install. This PR is a genuine fix, not just a tidy-up.
  • Constructed an XBeeCoordinator directly and diffed the resulting object against what the old CustomDevice path produced: status, manufacturer="Digi", model="XBee", node_desc, skip_configuration=False, original_signature=None, endpoint 0xE6 (status=ZDO_INIT, profile_id=0xC105, device_type=0x0050), in_clusters={0x0006: XBeeGroup, 0x8006: XBeeGroupResponse}, ep.groups / ep.xbee_groups_response both resolving — identical, and no DeprecationWarning with -W error::DeprecationWarning.
  • Checked the attributes zhaquirks' BaseCustomDevice.__init__ copies that the new __init__ does not (lqi, rssi, last_seen, relays, original_signature): all of them are still at their defaults on the freshly-built replaces device at the call site, so nothing is actually lost.
  • apply_custom_configuration is gone from the coordinator now that it isn't a CustomDevice, but ZHA's only caller is hasattr-guarded (zha/zigbee/device.py:1001), so a plain Device is fine there.

The add_endpoint rewrite fixes more than it looks like

Worth calling out explicitly, because it's not in the PR description: the old add_endpoint was already silently broken on zigpy 2.x, and this PR repairs it.

Current zhaquirks gained a replace_device is None early-return in BaseCustomDevice.add_endpoint, so self._device.add_endpoint(descriptor.endpoint) (called without replace_device) fell through to plain Device.add_endpoint and the freshly-mutated replacement["endpoints"] entry was ignored. I reproduced the old construct against current zigpy + zha-quirks and got a bare endpoint back for zigpy's endpoint 1:

OLD PATH -> ep1.profile_id: None  device_type: None  status: 0
OLD PATH -> ep1.in_clusters:  {}
OLD PATH -> ep1.out_clusters: {}

versus this PR:

NEW PATH -> ep1.profile_id: 0x104  device_type: 0x400  status: 1
NEW PATH -> ep1.in:  {0x0: Basic, 0x6: OnOff, 0xa: Time, 0x19: Ota, 0x25: KeepAlive, 0x501: IasAce}
NEW PATH -> ep1.out: {0x1: PowerConfiguration, 0x20: PollControl, 0x500: IasZone, 0x502: IasWd}

So on current zigpy the coordinator's endpoints 1 and 2 had no Basic / Ota / IasAce / … clusters at all. Might be worth a line in the changelog or PR body — it's a user-visible fix.

Side note that falls out of the same probe: 0x6 on endpoint 1 correctly resolves to OnOff, not XBeeGroup — i.e. _skip_registry = True is doing its job. That matters more than usual here, since XBeeGroup overrides cluster_id to 0x0006, which is OnOff's real ID; without it the global registry entry for OnOff would be clobbered process-wide. I checked zigpy.zcl.Cluster._registry after importing zigpy_xbee and it's clean (_registry[0x0006] is OnOff, 0x8006 absent).

Re: where XBeeGroupResponse is used (puddly's open thread)

Answering rather than re-raising, since the thread is still open and the code is carried over unchanged.

Both clusters are used, but only implicitly — which is why grepping for them finds nothing:

  • XBeeGroup (0x0006 on endpoint 0xE6) inherits ep_attribute = "groups", and zigpy.endpoint.Endpoint.add_to_group / remove_from_group dispatch through exactly that attribute (res = await self.groups.add(grp_id, name)). Device.add_to_group iterates every non-ZDO endpoint, so subscribing the XBee coordinator to a group sends the ZCL Groups command to cluster 0x0006 on endpoint 0xE6 — the Digi XBee Group Table API that #46 was implementing.
  • XBeeGroupResponse (0x8006) is purely a receive-side handler: XBee replies come back on 0x8000 | cluster, and zigpy dispatches an incoming frame to ep.in_clusters[0x8006], which is this class. It is never called by name, only matched against. Its remove_all_response addition exists because plain ZCL Groups has no response for remove_all — the Digi profile adds one, so without it the reply would fail to parse.

So removing either would break XBee group subscription, but only at runtime on real hardware — nothing in the test suite exercises it.

Suggestions (all optional)

  1. Test the migrated structure, not just its line coverage. Coverage is 100%, but no test asserts the shape of what's built — XBeeCoordinator isn't referenced in tests/ at all. That's precisely the gap that let the old add_endpoint degenerate to a bare endpoint without CI noticing. Two small assertions would close it for good: one that app._device.endpoints[0xE6] has profile_id == 0xC105, device_type == 0x0050 and both group clusters, and one that after register_endpoints() endpoint 1 has its profile/device_type and the expected in/out clusters.
  2. The dev built at zigpy_xbee/zigbee/application.py:99-104 is now vestigial — dev.add_endpoint(XBEE_ENDPOINT_ID) has no effect (the new XBeeCoordinator builds its own endpoint and never reads replaces' endpoints), and replaces is consumed only for .status, which is set to a constant two lines earlier. See the inline note.
  3. Minor: datetime.now(timezone.utc) can be datetime.now(UTC) on 3.11+, which is what zigpy's own (now-deprecated) update_last_seen uses.

Given the PR says it wasn't tested on hardware, the two paths I'd smoke-test on a real XBee are init_api_mode (the baudrate-sweep path, the only caller of the changed BAUDRATES) and group subscription (XBeeGroup / XBeeGroupResponse) — everything else is covered by the suite.

Second opinion from GitHub Copilot (GPT-5.6 Sol, read-only) returned no findings.

"output_clusters": descriptor.output_clusters,
}
self._device.add_endpoint(descriptor.endpoint)
ep = self._device.add_endpoint(descriptor.endpoint)

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.

This rewrite is correct, and it also repairs behaviour that was already broken on zigpy 2.x — see the review body for the before/after probe.

Short version: current zhaquirks' BaseCustomDevice.add_endpoint early-returns to the plain Device.add_endpoint when replace_device is None, which is how the old code called it. The replacement["endpoints"][...] entry set on the line above was therefore ignored, and endpoints 1 and 2 came out with profile_id=None, device_type=None and zero clusters. Building the endpoint explicitly like this is both the right post-quirks API and a fix.

Comment thread zigpy_xbee/zigbee/application.py Outdated
Comment thread zigpy_xbee/zigbee/application.py Outdated
Comment thread zigpy_xbee/uart.py
@TheJulianJES

Copy link
Copy Markdown
Contributor Author

@zigpy-review-bot Make the changes, do review rounds before pushing. Also check if anything else should be changed (e.g., require even newer zigpy version? Everything still fine with latest zigpy/ZHA?).

Matches the spelling of zigpy's own (deprecated) `update_last_seen()` and
drops the `timezone` import; `datetime.UTC` is available on Python 3.11+.
`XBeeCoordinator` builds its own `0xE6` endpoint and only read `replaces`
for `.status`, which the call site assigned two lines earlier, so the plain
`Device` it was handed — and the `add_endpoint()` call on it — had no effect.
Coverage was already 100%, but nothing asserted the shape of what
`start_network()` builds, which is what let the old quirks-based
`add_endpoint()` silently degenerate to bare endpoints.
@TheJulianJES

TheJulianJES commented Aug 22, 2026

Copy link
Copy Markdown
Contributor Author

@zigpy-review-bot You can push to this PR/repo now. Check my above comment and pending/parked changes. Push when ready, or do review rounds.

@TheJulianJES

Copy link
Copy Markdown
Contributor Author

@zigpy-review-bot Oops, didn't realize this was opened from my fork. Accept the invite and push / see instructions above.

@zigpy-review-bot

Copy link
Copy Markdown
Collaborator

Pushed to tjj/zigpy-2.0-compat. The fork invite turned out not to be needed — the bot has write on zigpy/zigpy-xbee and this PR has "allow edits by maintainers", so the push went through that route; I left the invite unaccepted, feel free to revoke it.

Made the three changes agreed in the threads above, and ran review rounds before pushing — round 2 came back clean from both reviewers.

What changed (47c4de77187a17):

  • datetime.now(UTC), dropping the timezone import — the nit on handle_rx.
  • Dropped the replaces scaffold: XBeeCoordinator.__init__ no longer takes a replaces device, start_network() no longer builds the throwaway Device (or calls add_endpoint() on it), and status = ENDPOINTS_INIT is assigned at the call site. One side effect worth naming: on zigpy 2.x every Device.__init__ registers a PollControl check-in callback on the application, so the throwaway device was leaving a dangling listener behind for a device that never entered app.devices.
  • Added the two structural tests from suggestion 1 of my review: one asserting the 0xE6 endpoint's profile, device type and both group clusters, one asserting that zigpy's endpoints 1 and 2 come out with a profile, a device type and their clusters — the exact regression the old quirks-based add_endpoint() silently had. Mutation-checked rather than assumed: reverting add_endpoint to the old bare-endpoint behaviour, removing _skip_registry from XBeeGroup, skipping register_endpoints(), or dropping the call-site status assignment each fail at least one of them.

Left alone on purpose: the hardcoded BAUDRATES list, per your "nothing uses this anyways".

On "anything else / require a newer zigpy?"zigpy>=2.0.0 is exactly right, and nothing else needs changing:

  • serialx landed in 2.0.0 (serialx>=1.4.0 is already a 2.0.0 dependency), which is what the BAUDRATES list works around, so the floor can't go lower; nothing on the branch uses an API newer than 2.0.0. The suite is green on 2.0.0 and on 2.1.0, and requires-python >= 3.11 matches zigpy's own floor.
  • Checked against the released stack rather than in isolation: with zha 2.1.0 + zha-quirks 2.2.0 + zigpy 2.1.0 installed, import zigpy_xbee.zigbee.application is clean under -W error::DeprecationWarning, the global cluster registry stays unpolluted (0x0006 still resolves to OnOff, 0x8006 absent), and all 107 tests pass in that environment too. For contrast, released 0.21.1 in the same env fails with ModuleNotFoundError: No module named 'zhaquirks' as soon as quirks isn't installed.
  • Follow-up for a different repo: ZHA pins zigpy-xbee==0.21.1, so it needs a bump once this is released.
  • Unrelated to this PR but found while answering puddly's thread: XBee group support is broken on current zigpy — the transmit side still works, but replies on the Digi profile endpoint are dropped by zigpy, so add_to_group() on the coordinator always times out. Details in that thread.

Still not tested on hardware — the two paths I'd smoke-test remain init_api_mode's baudrate sweep and group subscription.

Review rounds

Two rounds, each an adversarial Claude reviewer (Fable 5) reviewing the delta in a worktree plus GitHub Copilot (GPT-5.6 Sol), both read-only and independent.

Round 1 — Copilot: no findings. Fable: verdict "ready", with two test-quality notes, both folded in: a comment claiming xbee_groups_response was a dispatch path (it is only an ep_attribute alias — incoming frames are matched by cluster ID), and assertions pinning zigpy's default endpoint descriptors (device_type == 0x0400, an exact out_clusters set) too tightly, which would break on a benign zigpy change.

Round 2 (the amended tests) — Copilot: no findings. Fable: "ready", with a mutation matrix confirming the relaxed assertions still kill every regression the exact ones did.

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

Fresh re-review at 7187a17. Rebuilt the branch in a clean worktree with no zhaquirks installed and verified the migration by running it rather than by reading: 107 tests pass on zigpy 2.1.0 / Python 3.13 and on zigpy 2.0.0 / Python 3.11. An independent second opinion (Copilot, GPT-5.6 Sol) came back with no findings.

The three changes agreed in the earlier threads all landed as discussed, and the migration itself looks right to me. One non-blocking ordering suggestion inline; everything else below is verification, not objection.

What I checked empirically

  • The global cluster registry stays clean, which is the main risk of a _skip_registry cluster overriding cluster_id: Cluster._registry[0x0006] is still OnOff, 0x8006 is absent entirely, and 0x0004 is still Groups.
  • add_endpoint() really does produce fully-populated endpoints now — ep 1 → profile 0x0104, in-clusters {0x0000, 0x0006, 0x000A, 0x0019, 0x0025, 0x0501}; ep 2 → profile 0xC05E, in-clusters {0x0000}; ep 0xE6 → profile 0xC105, in-clusters {0x0006, 0x8006}.
  • XBeeGroupResponse.remove_all_response survives the move out of the quirks API: zigpy 2.x rebuilds ClientCommandDefs from the legacy dict-form client_commands, so commands_by_name still resolves remove_all_response and client_commands still has ids 0–4. (Whether anything can reach it is the separate matter in puddly's thread, unchanged by this PR.)
  • The serial side works with the new backend, not just the validation list: serialx's SerialTransport.serial property exists and BaseSerial.baudrate still has a setter that calls _configure_port(), so init_api_mode()'s baud sweep (api.py:547) can still retune an open port. serialx's docstring labels that setter "(deprecated)" but it emits no warning, so nothing here trips -W error::DeprecationWarning. Worth remembering if serialx ever removes it, since that sweep is the only caller that needs runtime retuning.
  • On "should this require an even newer zigpy?" — no. zigpy>=2.0.0 is the correct floor: the whole suite is green against exactly 2.0.0, and nothing on the branch uses an API introduced after it. requires-python >= 3.11 matches zigpy's own floor and is required by datetime.UTC.
  • All nine BAUDRATE_TO_BD keys (1200 … 230400) are present in the new Gateway.BAUDRATES tuple, so the sweep's candidate set is unchanged.

Still unverified: hardware. The two paths worth a smoke test remain init_api_mode()'s baudrate sweep and coordinator group subscription.

self, self.state.node_info.ieee, self.state.node_info.nwk
)
xbee_dev.status = zigpy.device.Status.ENDPOINTS_INIT
self.listener_event("raw_device_initialized", xbee_dev)

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.

Not a regression from this PR, but this PR is what makes it matter: the event fires three lines above register_endpoints(), so every listener sees a coordinator that still only has endpoint 0xE6.

Two concrete consequences:

  • zigpy.appdb.PersistingListener.raw_device_initialized() takes device.clone() synchronously inside the event, and _raw_device_initialized_internal() is the only code path that ever writes endpoints/clusters rows (PersistingListener.device_initialized() is a pass). So endpoints 1 and 2 — which this PR gives real profiles, device types and clusters for the first time — never reach the database.
  • ZHA's handler (zha/application/gateway.py:416) puts device.get_signature() into the event it emits, so that signature is missing endpoints 1 and 2 as well.

Probed against a real sqlite DB driving the exact start_network() sequence. In memory the device ends up with endpoints [0, 1, 2, 230], but the DB gets only:

endpoints_v15: [(ieee, 230, 49413, 80, 1)]
clusters_v15:  [(ieee, 230, 0, 6), (ieee, 230, 0, 32774)]

Moving the event below register_endpoints() fixes both:

xbee_dev.status = zigpy.device.Status.ENDPOINTS_INIT
self.devices[xbee_dev.ieee] = xbee_dev

await self.register_endpoints()

self.listener_event("raw_device_initialized", xbee_dev)

The self.devices[...] insertion has to stay above register_endpoints(), since add_endpoint() resolves self._device out of self.devices. With that reorder the same probe records all three endpoints and their 13 clusters, and the suite still passes 107/107.

Low impact in practice: start_network() rebuilds the coordinator device on every startup, so the incomplete DB rows self-heal and nothing user-visible reads them today. Entirely reasonable to leave for a follow-up rather than grow this PR.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

@zigpy-review-bot Create issue about this or directly create a PR, possibly stacked on top of this for now, later rebased after this one is merged.

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.

Opened #180 with the reorder, stacked on this PR's head (7187a175) — so its diff currently includes this PR's commits, and only 1711d8f belongs to it. I'll rebase it onto dev once this one merges; re-mention me when that happens.

It carries a regression test that asserts the listener sees all endpoints at event time (it fails on the current ordering), and I re-ran the sqlite probe at this head: endpoints_v15 goes from [(230, 49413)] / 2 clusters to [(1, 260), (2, 49246), (230, 49413)] / 13 clusters. Full suite 108 passed, pre-commit clean.

Nothing here needs a change in this PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants