Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
58 changes: 47 additions & 11 deletions aiocomfoconnect/discovery.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,8 @@
import logging
from typing import Any, List, Union

import netifaces

from .bridge import Bridge
from .protobuf import zehnder_pb2

Expand All @@ -15,7 +17,7 @@
class BridgeDiscoveryProtocol(asyncio.DatagramProtocol):
"""UDP Protocol for the ComfoConnect LAN C bridge discovery."""

def __init__(self, target: str = None, timeout: int = 5):
def __init__(self, target: str | None = None, timeout: int = 5):
loop = asyncio.get_running_loop()

self._bridges: List[Bridge] = []
Expand All @@ -33,8 +35,17 @@ def connection_made(self, transport: asyncio.transports.DatagramTransport):
_LOGGER.debug("Sending discovery request to %s:%d", self._target, Bridge.PORT)
self.transport.sendto(b"\x0a\x00", (self._target, Bridge.PORT))
else:
_LOGGER.debug("Sending discovery request to broadcast:%d", Bridge.PORT)
self.transport.sendto(b"\x0a\x00", ("<broadcast>", Bridge.PORT))
# Determine broadcast address programmatically
try:
gws = netifaces.gateways()
default_iface = gws['default'][netifaces.AF_INET][1]
addrs = netifaces.ifaddresses(default_iface)
broadcast_addr = addrs[netifaces.AF_INET][0].get('broadcast', '255.255.255.255')
Comment on lines +41 to +43
Copy link

Copilot AI Oct 31, 2025

Choose a reason for hiding this comment

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

This code assumes the presence of keys in nested dictionaries without checking their existence, which can cause KeyError exceptions not caught by the generic Exception handler. Specifically, gws['default'], gws['default'][netifaces.AF_INET], and addrs[netifaces.AF_INET] could raise KeyError if the network doesn't have a default IPv4 gateway or IPv4 addresses configured. While the broad exception handler will catch this, it would be more explicit and maintainable to use .get() methods or check for key existence.

Suggested change
default_iface = gws['default'][netifaces.AF_INET][1]
addrs = netifaces.ifaddresses(default_iface)
broadcast_addr = addrs[netifaces.AF_INET][0].get('broadcast', '255.255.255.255')
default = gws.get('default')
default_inet = default.get(netifaces.AF_INET) if default else None
default_iface = default_inet[1] if default_inet and len(default_inet) > 1 else None
if default_iface:
addrs = netifaces.ifaddresses(default_iface)
inet_addrs = addrs.get(netifaces.AF_INET)
if inet_addrs and len(inet_addrs) > 0:
broadcast_addr = inet_addrs[0].get('broadcast', '255.255.255.255')
else:
broadcast_addr = '255.255.255.255'
else:
broadcast_addr = '255.255.255.255'

Copilot uses AI. Check for mistakes.
except Exception as e:
_LOGGER.warning("Could not determine broadcast address, using 255.255.255.255: %s", e)
broadcast_addr = '255.255.255.255'
Comment on lines +43 to +46
Copy link

Copilot AI Oct 31, 2025

Choose a reason for hiding this comment

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

The code indexes into addrs[netifaces.AF_INET][0] assuming the first address entry contains the broadcast address, but some network configurations may have multiple IPv4 addresses on an interface. While the fallback to '255.255.255.255' handles missing 'broadcast' keys, the indexing with [0] could still raise IndexError if the AF_INET list is empty. This is an edge case but worth addressing for robustness.

Suggested change
broadcast_addr = addrs[netifaces.AF_INET][0].get('broadcast', '255.255.255.255')
except Exception as e:
_LOGGER.warning("Could not determine broadcast address, using 255.255.255.255: %s", e)
broadcast_addr = '255.255.255.255'
inet_addrs = addrs.get(netifaces.AF_INET, [])
if inet_addrs and 'broadcast' in inet_addrs[0]:
broadcast_addr = inet_addrs[0]['broadcast']
else:
broadcast_addr = '255.255.255.255'

Copilot uses AI. Check for mistakes.
_LOGGER.debug("Sending discovery request to broadcast:%d (%s)", Bridge.PORT, broadcast_addr)
self.transport.sendto(b"\x0a\x00", (broadcast_addr, Bridge.PORT))

def datagram_received(self, data: Union[bytes, str], addr: tuple[str | Any, int]):
"""Called when some datagram is received."""
Expand All @@ -43,12 +54,15 @@ def datagram_received(self, data: Union[bytes, str], addr: tuple[str | Any, int]
return

_LOGGER.debug("Data received from %s: %s", addr, data)

# Decode the response
parser = zehnder_pb2.DiscoveryOperation() # pylint: disable=no-member
parser.ParseFromString(data)

self._bridges.append(Bridge(host=parser.searchGatewayResponse.ipaddress, uuid=parser.searchGatewayResponse.uuid.hex()))
try:
# Decode the response
parser = zehnder_pb2.DiscoveryOperation() # pylint: disable=no-member
parser.ParseFromString(data)

self._bridges.append(Bridge(host=parser.searchGatewayResponse.ipaddress, uuid=parser.searchGatewayResponse.uuid.hex()))
except Exception as exc:
_LOGGER.error("Failed to parse discovery response from %s: %s", addr, exc)
return

# When we have passed a target, we only want to listen for that one
if self._target:
Expand All @@ -66,8 +80,30 @@ def get_bridges(self):
return self._future


async def discover_bridges(host: str = None, timeout: int = 1, loop=None) -> List[Bridge]:
"""Discover a bridge by IP."""
async def discover_bridges(host: str | None = None, timeout: int = 1, loop=None) -> List[Bridge]:
"""
Discover ComfoConnect bridges on the local network or at a specified host.

This asynchronous function sends a UDP broadcast (or unicast if a host is specified)
to discover available ComfoConnect bridges. It returns a list of discovered Bridge
instances.

Args:
host (str | None): The IP address of a specific bridge to discover. If None,
a broadcast is sent to discover all available bridges. Defaults to None.
timeout (int): The time in seconds to wait for responses. Defaults to 1.
loop (asyncio.AbstractEventLoop, optional): The event loop to use. If None,
the default event loop is used.

Returns:
List[Bridge]: A list of discovered Bridge objects.

Raises:
Any exceptions raised by the underlying asyncio transport or protocol.
Copy link

Copilot AI Oct 31, 2025

Choose a reason for hiding this comment

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

The 'Raises' section is vague and unhelpful. It should document specific exceptions that callers might need to handle. Based on the code, this function doesn't raise exceptions from the protocol (they're caught internally), but could raise exceptions from loop.create_datagram_endpoint(). Consider either removing this section or documenting specific exceptions like OSError for network issues.

Suggested change
Any exceptions raised by the underlying asyncio transport or protocol.
OSError: If a network error occurs while creating the datagram endpoint.

Copilot uses AI. Check for mistakes.

Example:
bridges = await discover_bridges(timeout=2)
"""

if loop is None:
loop = asyncio.get_event_loop()
Expand Down
1 change: 1 addition & 0 deletions pyproject.toml
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ homepage = "https://github.com/michaelarnauts/aiocomfoconnect"
python = "^3.10"
aiohttp = "^3.8.0"
protobuf = "^6.31"
netifaces = "^0.11.0"

[tool.poetry.group.dev.dependencies]
grpcio-tools = "^1.73.0"
Expand Down