Skip to content

fix: dispatch service state change callbacks positionally #1768

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
bluetoothbot wants to merge 1 commit into from
Closed

fix: dispatch service state change callbacks positionally #1768

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions src/zeroconf/_services/__init__.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -54,9 +54,9 @@ class Signal:
def __init__(self) -> None:
self._handlers: list[Callable[..., None]] = []

def fire(self, **kwargs: Any) -> None:
def fire(self, *args: Any) -> None:
for h in self._handlers[:]:
h(**kwargs)
h(*args)

@property
def registration_interface(self) -> SignalRegistrationInterface:
Expand Down
8 changes: 4 additions & 4 deletions src/zeroconf/_services/browser.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -732,10 +732,10 @@ def _fire_service_state_changed_event(self, event: tuple[tuple[str, str], Servic
name_type = event[0]
state_change = event[1]
self._service_state_changed.fire(
zeroconf=self.zc,
service_type=name_type[1],
name=name_type[0],
state_change=state_change,
self.zc,
name_type[1],
name_type[0],
state_change,
)

def _async_cancel(self) -> None:
Expand Down
63 changes: 53 additions & 10 deletions tests/services/test_browser.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -533,7 +533,7 @@ def send(out, addr=const._MDNS_ADDR, port=const._MDNS_PORT):
# patch the zeroconf send
with patch.object(zeroconf_browser, "async_send", send):
# dummy service callback
def on_service_state_change(zeroconf, service_type, state_change, name):
def on_service_state_change(zeroconf, service_type, name, state_change):
pass

start_time = current_time_millis()
Expand All @@ -559,7 +559,7 @@ async def test_asking_default_is_asking_qm_questions_after_the_first_qu(quick_ti
type_ = "_http._tcp.local."
registration_name = f"xxxyyy.{type_}"

def on_service_state_change(zeroconf, service_type, state_change, name):
def on_service_state_change(zeroconf, service_type, name, state_change):
if name == registration_name:
if state_change is ServiceStateChange.Added:
service_added.set()
Expand Down Expand Up @@ -661,7 +661,7 @@ async def test_ttl_refresh_cancelled_rescue_query(quick_timing: None) -> None:
type_ = "_http._tcp.local."
registration_name = f"xxxyyy.{type_}"

def on_service_state_change(zeroconf, service_type, state_change, name):
def on_service_state_change(zeroconf, service_type, name, state_change):
if name == registration_name:
if state_change is ServiceStateChange.Added:
service_added.set()
Expand Down Expand Up @@ -782,7 +782,7 @@ def send(out, addr=const._MDNS_ADDR, port=const._MDNS_PORT):
# patch the zeroconf send
with patch.object(zeroconf_browser, "async_send", send):
# dummy service callback
def on_service_state_change(zeroconf, service_type, state_change, name):
def on_service_state_change(zeroconf, service_type, name, state_change):
pass

browser = AsyncServiceBrowser(
Expand Down Expand Up @@ -822,7 +822,7 @@ def send(out, addr=const._MDNS_ADDR, port=const._MDNS_PORT):
# patch the zeroconf send
with patch.object(zeroconf_browser, "async_send", send):
# dummy service callback
def on_service_state_change(zeroconf, service_type, state_change, name):
def on_service_state_change(zeroconf, service_type, name, state_change):
pass

browser = AsyncServiceBrowser(
Expand Down Expand Up @@ -865,7 +865,7 @@ def update_record(self, zc: Zeroconf, now: float, record: r.DNSRecord) -> None:
zc.add_listener(listener, None)

# dummy service callback
def on_service_state_change(zeroconf, service_type, state_change, name):
def on_service_state_change(zeroconf, service_type, name, state_change):
pass

# start a browser
Expand Down Expand Up @@ -912,7 +912,7 @@ def test_service_browser_is_aware_of_port_changes():
callbacks = []

# dummy service callback
def on_service_state_change(zeroconf, service_type, state_change, name):
def on_service_state_change(zeroconf, service_type, name, state_change):
"""Dummy callback."""
if name == registration_name:
callbacks.append((service_type, state_change, name))
Expand Down Expand Up @@ -963,6 +963,49 @@ def on_service_state_change(zeroconf, service_type, state_change, name):
zc.close()


def test_service_browser_handler_callback_arg_names_independent() -> None:
"""Handler callbacks receive arguments positionally; parameter names are user choice."""
zc = Zeroconf(interfaces=["127.0.0.1"])
type_ = "_hap._tcp.local."
registration_name = f"renamed.{type_}"

callbacks: list[tuple[Zeroconf, str, str, ServiceStateChange]] = []

def on_service_state_change(zc_arg, type_arg, name_arg, change_arg): # type: ignore[no-untyped-def]
if name_arg == registration_name:
callbacks.append((zc_arg, type_arg, name_arg, change_arg))

browser = ServiceBrowser(zc, type_, [on_service_state_change])

info = ServiceInfo(
type_,
registration_name,
80,
0,
0,
{"path": "/~paulsm/"},
"ash-2.local.",
addresses=[socket.inet_aton("10.0.1.2")],
)
_inject_response(
zc,
mock_incoming_msg(
[
info.dns_pointer(),
info.dns_service(),
info.dns_text(),
*info.dns_addresses(),
]
),
)
time.sleep(0.1)

browser.cancel()
zc.close()

assert callbacks == [(zc, type_, registration_name, ServiceStateChange.Added)]


def test_service_browser_listeners_update_service():
"""Test that the ServiceBrowser ServiceListener that implements update_service."""

Expand Down Expand Up @@ -1089,7 +1132,7 @@ def test_service_browser_uses_non_strict_names():
"""Verify we can look for technically invalid names as we cannot change what others do."""

# dummy service callback
def on_service_state_change(zeroconf, service_type, state_change, name):
def on_service_state_change(zeroconf, service_type, name, state_change):
pass

zc = r.Zeroconf(interfaces=["127.0.0.1"])
Expand Down Expand Up @@ -1585,7 +1628,7 @@ async def test_close_zeroconf_without_browser_before_start_up_queries(quick_timi
type_ = "_http._tcp.local."
registration_name = f"xxxyyy.{type_}"

def on_service_state_change(zeroconf, service_type, state_change, name):
def on_service_state_change(zeroconf, service_type, name, state_change):
if name == registration_name and state_change is ServiceStateChange.Added:
service_added.set()

Expand Down Expand Up @@ -1653,7 +1696,7 @@ async def test_close_zeroconf_without_browser_after_start_up_queries(quick_timin
type_ = "_http._tcp.local."
registration_name = f"xxxyyy.{type_}"

def on_service_state_change(zeroconf, service_type, state_change, name):
def on_service_state_change(zeroconf, service_type, name, state_change):
if name == registration_name and state_change is ServiceStateChange.Added:
service_added.set()

Expand Down
2 changes: 1 addition & 1 deletion tests/test_asyncio.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1006,7 +1006,7 @@ async def test_integration(quick_timing: None) -> None:
type_ = "_http._tcp.local."
registration_name = f"xxxyyy.{type_}"

def on_service_state_change(zeroconf, service_type, state_change, name):
def on_service_state_change(zeroconf, service_type, name, state_change):
if name == registration_name:
if state_change is ServiceStateChange.Added:
service_added.set()
Expand Down
2 changes: 1 addition & 1 deletion tests/test_updates.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ def update_record(self, zc: Zeroconf, now: float, record: r.DNSRecord) -> None:
zc.add_listener(listener, None)

# dummy service callback
def on_service_state_change(zeroconf, service_type, state_change, name):
def on_service_state_change(zeroconf, service_type, name, state_change):
pass

# start a browser
Expand Down
Loading