Skip to content

API to subscribe to multicast groups - #737

Merged
puddly merged 8 commits into
zigpy:devfrom
puddly:puddly/zigpy-multicast-api
Jul 28, 2026
Merged

API to subscribe to multicast groups#737
puddly merged 8 commits into
zigpy:devfrom
puddly:puddly/zigpy-multicast-api

Conversation

@puddly

@puddly puddly commented Jul 27, 2026

Copy link
Copy Markdown
Contributor

Implements zigpy/zigpy#1864. CI will fail until a new version of zigpy is released.

Copilot AI review requested due to automatic review settings July 27, 2026 16:57

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

Adds support for zigpy’s new coordinator multicast group subscription APIs by wiring bellows’ multicast table controller into the ControllerApplication hooks, and updating tests/dependencies accordingly.

Changes:

  • Implement _subscribe_to_multicast_group() / _unsubscribe_from_multicast_group() in bellows.zigbee.application.ControllerApplication.
  • Add application-level tests covering subscribe/unsubscribe behavior with and without XNCP extensions.
  • Update multicast table “missing entry” logging and bump the zigpy minimum version.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.

File Description
bellows/zigbee/application.py Adds multicast subscribe/unsubscribe hook implementations that delegate to bellows.multicast.Multicast when enabled.
bellows/multicast.py Lowers log severity when unsubscribing a group that is not in the multicast table.
tests/test_application.py Adds tests for the new multicast group subscription APIs under different firmware feature sets.
pyproject.toml Raises the minimum zigpy dependency version to pick up the new APIs.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread tests/test_application.py
Comment thread tests/test_application.py
@codecov

codecov Bot commented Jul 28, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 99.54%. Comparing base (89750d1) to head (f7a8520).
⚠️ Report is 3 commits behind head on dev.

Additional details and impacted files
@@           Coverage Diff           @@
##              dev     #737   +/-   ##
=======================================
  Coverage   99.54%   99.54%           
=======================================
  Files          64       64           
  Lines        4203     4213   +10     
=======================================
+ Hits         4184     4194   +10     
  Misses         19       19           

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

Comment-only — one small real bug (a broken debug log), the rest nits. This is the thin adapter over Multicast anticipated in zigpy/zigpy#1864: the hook signatures match the merged zigpy API exactly (including the optional endpoint_id=1), keying the table by (group_id, endpoint_id) keeps this path and the existing EZSPGroupEndpoint.add_to_group path deduplicated in one table, and with XNCP MEMBER_OF_ALL_GROUPS the hooks correctly no-op since _multicast stays None. Demoting the unsubscribe missing-entry log from error to debug also makes sense now that unsubscribing an unknown group is a normal API outcome.

  • zigpy 2.1.0 was released today, so the zigpy>=2.1.0 pin now resolves — CI should pass on a re-run. The full suite is green locally against zigpy 2.1.0: 430 passed.
  • The success-path debug log in _set_multicast_entry has five placeholders but four arguments, so with debug logging enabled every successful table write logs a --- Logging error --- traceback instead of the message — inline. That's exactly the log a #708 user would be asked to capture, so worth fixing before quirks start driving this path.
  • Carried over from the #1864 thread, not a defect of this diff: subscriptions made through this hook still don't survive a restart — the NCP table is wiped on reset and Multicast.startup() replays only the DB-backed ep.member_of, while zigpy/zha-device-handlers#5209 still calls only from async_configure. If replay ends up being the fix, startup() here is the natural place.
Verification
  • Full suite in a clean worktree venv against zigpy 2.1.0: 430 passed in 66s.
  • Log bug reproduced end-to-end: Multicast.subscribe() with level=DEBUGTypeError: not enough arguments for format string via the logging error handler.
  • typing.get_type_hints(ControllerApplication._subscribe_to_multicast_group)AttributeError: module 'bellows.types' has no attribute 'Group'.
  • Hook signatures diffed against merged zigpy dev zigpy/application.py:1677-1704 — exact match, public wrappers default endpoint_id=1.
  • Copilot's two comments checked: the skip-if-API-missing suggestion is moot with zigpy 2.1.0 released and pinned; the "XNCP test asserts nothing" claim is incorrect — it asserts setMulticastTableEntry.mock_calls is empty.
  • No reactive auto-subscribe on send_multicast (unlike zigbee-herdsman) — unchanged by this PR, consistent with the explicit-API design.

Comment thread bellows/multicast.py Outdated
Comment thread bellows/zigbee/application.py Outdated
Comment thread bellows/multicast.py Outdated
Comment thread tests/test_application.py
@puddly
puddly merged commit 3adb0e3 into zigpy:dev Jul 28, 2026
10 checks passed
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