GeezerVison

@bluetoothbot rr

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 99.77%. Comparing base (cb81e67) to head (6e25708).
⚠️ Report is 120 commits behind head on master.

@@           Coverage Diff           @@
##           master    #1767   +/-   ##
=======================================
  Coverage   99.77%   99.77%           
=======================================
  Files          33       33           
  Lines        3536     3562   +26     
  Branches      498      508   +10     
=======================================
+ Hits         3528     3554   +26     
  Misses          5        5           
  Partials        3        3           

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

Merging this PR will not alter performance

✅ 14 untouched benchmarks

_{Comparing bluetoothbot:koan/fix-issue-1520 (6e25708) with master (cb81e67)}

PR Review — feat: add multicast_addresses to bind/multicast independently

The feature itself is a clean, additive escape hatch with sensible behaviour: extras are validated up-front against ip_version, deduped against the existing interfaces set, and joined on the listen socket in both the Default fast path and the per-interface path. The unicast=True rejection is correct (there's no listen socket to join groups on). Three things to address before merging: (1) tuple entries in multicast_addresses are accepted by the type signature and the validator but silently dropped by normalize_interface_choice — this needs to either be supported or explicitly removed from the type; (2) @bdraco's DRY request — pull the validation, dedup, and extras-join into small helpers so create_sockets stays a coordinator; (3) the TC-deferral test rewrite is unrelated scope and should ship in its own PR. The test-coverage gap @bdraco flagged earlier appears to have been fixed per the latest Codecov run. No correctness or security concerns in the wire-protocol path.

🟡 Important

if multicast_addresses:
    extra_multicast_members = normalize_interface_choice(list(multicast_addresses), ip_version)
    # Strip entries already covered by ``interfaces`` so add_multicast_member
    # is not called twice for the same membership.
    interface_set = set(normalized_interfaces)
    extra_multicast_members = [m for m in extra_multicast_members if m not in interface_set]

    # Widen the per-packet delay so Windows scheduling jitter between
    # assertions cannot fire the timer mid-test, while leaving plenty of
    # headroom under the first-arrival deadline for timer-replacement to
    # remain deterministic.
    with patch.object(_listener, "_TC_DELAY_RANDOM_INTERVAL", (1500, 60_000)):

🟢 Suggestions

def _validate_multicast_addresses(
    multicast_addresses: Sequence[...] | None, ip_version: IPVersion, unicast: bool,
) -> None:
    if not multicast_addresses:
        return
    if unicast:
        raise ValueError("multicast_addresses is incompatible with unicast=True")
    if ip_version == IPVersion.V4Only and any(_entry_ip_version(e) == 6 for e in multicast_addresses):
        raise ValueError("multicast_addresses contains IPv6 entries but ip_version is V4Only")
    if ip_version == IPVersion.V6Only and any(_entry_ip_version(e) == 4 for e in multicast_addresses):
        raise ValueError("multicast_addresses contains IPv4 entries but ip_version is V6Only")


def _resolve_extra_multicast_members(
    multicast_addresses: Sequence[...] | None,
    normalized_interfaces: list[...],
    ip_version: IPVersion,
) -> list[...]:
    if not multicast_addresses:
        return []
    extras = normalize_interface_choice(list(multicast_addresses), ip_version)
    interface_set = set(normalized_interfaces)
    return [m for m in extras if m not in interface_set]


def _join_extra_multicast_members(listen_socket, extras):
    for member in extras:
        add_multicast_member(listen_socket, member)

Checklist

• Implementation matches the documented behaviour — warning ＃1
• Validation at the API boundary
• No add_multicast_member double-calls
• autodetect_ip_version covers all entry kinds
• PR is scoped to the stated feature — warning ＃3
• Maintainer feedback addressed (DRY) — suggestion ＃2
• Tests cover the new branches
• No hand-edited Cython output
• Docstring / comment style per CLAUDE.md

Summary

The feature itself is a clean, additive escape hatch with sensible behaviour: extras are validated up-front against ip_version, deduped against the existing interfaces set, and joined on the listen socket in both the Default fast path and the per-interface path. The unicast=True rejection is correct (there's no listen socket to join groups on). Three things to address before merging: (1) tuple entries in multicast_addresses are accepted by the type signature and the validator but silently dropped by normalize_interface_choice — this needs to either be supported or explicitly removed from the type; (2) @bdraco's DRY request — pull the validation, dedup, and extras-join into small helpers so create_sockets stays a coordinator; (3) the TC-deferral test rewrite is unrelated scope and should ship in its own PR. The test-coverage gap @bdraco flagged earlier appears to have been fixed per the latest Codecov run. No correctness or security concerns in the wire-protocol path.

To rebase specific severity levels, mention me: @bluetoothbot rebase critical (fixes 🔴 only), @bluetoothbot rebase important (fixes 🔴 + 🟡), or just @bluetoothbot rebase for all.

Automated review by Kōanf265d95
933a7c9
5fc8e96
83fadb1
56f00c5

Rebase with requested adjustments

Branch koan/fix-issue-1520 was rebased onto master and review feedback was applied.

Changes applied

• only interfaces list no longer silently resolves to V4Only (Copilot review on _core.py:216).
• _utils/net.py: Added explicit IP-version validation for multicast_addresses before any socket work — raises ValueError with a clear message when V4Only sees IPv6 entries (or V6Only sees IPv4) instead of failing later in adapter lookup or setsockopt (Copilot review on _utils/net.py:469).
• _utils/net.py: Deduplicated extra_multicast_members against normalized_interfaces so addresses appearing in both lists no longer trigger add_multicast_member() twice (Copilot review on _utils/net.py:477).
• tests/test_core.py: Dropped unicast=True from the forwarding test so it uses a combination the real create_sockets() accepts, keeping the test aligned with the public contract (Copilot review on tests/test_core.py:193).
• tests/utils/test_net.py: Extended autodetect_ip_version and added focused tests for the new validation paths and the dedup behavior.

Stats

5 files changed, 237 insertions(+), 10 deletions(-)

CI status

CI will be checked asynchronously.

Automated by Kōan

@bluetoothbot rr

Rebase with requested adjustments

Branch koan/fix-issue-1520 was rebased onto master and review feedback was applied.

Changes applied

• _non_list_interfaces_with_multicast_addressesto cover theisinstance(interfaces, list)False branch inautodetect_ip_versionwhenmulticast_addressesis also supplied (was partial:539->545`).
• Added test_create_sockets_multicast_addresses_ip_version_all to cover the ip_version == IPVersion.V6Only elif False branch in create_sockets validation, exercised by ip_version=IPVersion.All with multicast_addresses set (was partial: 470->475).
• Addresses @bdraco's "Need to complete coverage" review (src/zeroconf/_utils/net.py was at 96.66% with 1 partial; now 94% lines with only pre-existing partials remaining — both new partial branches introduced by the PR are now covered).
• Other inline Copilot comments were already addressed by commit f24fdaf (autodetect now considers multicast_addresses, dedup against interfaces, ip_version validation). The stale Copilot note about unicast=True in the wiring test no longer applies — the current test does not pass unicast=True.

Stats

5 files changed, 273 insertions(+), 10 deletions(-)

CI status

CI will be checked asynchronously.

Automated by Kōan

@bluetoothbot rr

Rebase with requested adjustments

Branch koan/fix-issue-1520 was rebased onto master and review feedback was applied.

Stats

5 files changed, 273 insertions(+), 10 deletions(-)

CI status

CI will be checked asynchronously.

Automated by Kōan

@bluetoothbot rr

Rebase with requested adjustments

Branch koan/fix-issue-1520 was rebased onto master and review feedback was applied.

Changes applied

• ny(_entry_ip_version(e) == N for e in ...)` guard clauses.
• Rewrote autodetect_ip_version to build one entries list from interfaces + multicast_addresses and classify via the helper, replacing two parallel any() branches per version that each repeated the isinstance/ipaddress dance.
• Behavior preserved for all existing tests (61 passed in tests/utils/test_net.py + tests/test_core.py). Tuple entries in interfaces now consistently classify as IPv6 in autodetect — previously only int was checked there, while tuple was already treated as IPv6 in the multicast_addresses path.
• Coverage on src/zeroconf/_utils/net.py is now 97% with the existing test suite; the codecov bot already reports "All modified and coverable lines are covered by tests" after the prior coverage commit. The Copilot inline comments (autodetect including multicast_addresses, dedup against interfaces, ip_version validation, test using unicast=True) were already addressed in prior commits on this branch.

Stats

5 files changed, 274 insertions(+), 15 deletions(-)

CI status

CI will be checked asynchronously.

Automated by Kōan

@bluetoothbot rr

Rebase with requested adjustments

Branch koan/fix-issue-1520 was rebased onto master and review feedback was applied.

Changes applied

• Coverage gap closed. Line 450 now hit; remaining missing lines/branches all pre-existing on master.

• Summary

• Added test_autodetect_ip_version_interface_index_treated_as_v6 covering _entry_ip_version's non-str branch (interface-index int → IPv6). Closes the codecov patch gap bdraco flagged ("Need to complete coverage" / _utils/net.py 93.54% → patch now fully covered).
• Other reviewer comments were already addressed in prior commits on this branch:
• Copilot autodetect / dedup / IP-version validation → commit 933a7c9.
• bdraco DRY refactor (extract _entry_ip_version) → commit 83fadb1.
• Copilot stale comment about unicast=True in test_multicast_addresses_forwarded_to_create_sockets — current test does not pass unicast=True.

Stats

5 files changed, 319 insertions(+), 52 deletions(-)

CI status

CI will be checked asynchronously.

Automated by Kōan

@bluetoothbot rr

Rebase with requested adjustments

Branch koan/fix-issue-1520 was rebased onto master and review feedback was applied.

Changes applied

• Changes complete.

• Summary

• Addresses the remaining unresolved Copilot review comment: pre-normalized IPv6 adapter tuples passed in multicast_addresses were silently dropped because ip6_addresses_to_indexes() only handled int and str entries.
• src/zeroconf/_utils/net.py: Extended ip6_addresses_to_indexes() with a tuple pass-through branch so already-normalized tuple[tuple[str, int, int], int] entries reach add_multicast_member() instead of being filtered out. Fixes Copilot's "silently ignored" concern on _utils/net.py:489 and applies uniformly to both interfaces and multicast_addresses (consistent with the type signature).
• tests/utils/test_net.py: Added test_create_sockets_multicast_addresses_preserves_pre_normalized_tuple pinning the new behaviour.
• Prior commits on this branch already resolved bdraco's coverage and DRY-helper requests and the other Copilot inline comments (autodetect, dedup, ip_version validation, test wiring).

Stats

5 files changed, 347 insertions(+), 52 deletions(-)

CI status

CI will be checked asynchronously.

Automated by Kōan