Skip to content

Ensure homekit_controller recieves zeroconf c# updates #35545

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

Merged
bdraco merged 4 commits into from
May 12, 2020
Merged

Ensure homekit_controller recieves zeroconf c# updates #35545

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
5d31f9f
Ensure homekit_controller recieves zeroconf c# updates
bdraco May 12, 2020
b508dfd
Only send updates to homekit controller if the device is paired
bdraco May 12, 2020
f788c11
remove debug
bdraco May 12, 2020
db660ea
fix refactor error
bdraco May 12, 2020
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
30 changes: 26 additions & 4 deletions homeassistant/components/zeroconf/__init__.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,10 @@
CONF_DEFAULT_INTERFACE = "default_interface"
DEFAULT_DEFAULT_INTERFACE = False

HOMEKIT_PROPERTIES = "properties"
HOMEKIT_PAIRED_STATUS_FLAG = "sf"
HOMEKIT_MODEL = "md"

CONFIG_SCHEMA = vol.Schema(
{
DOMAIN: vol.Schema(
Expand Down Expand Up @@ -178,8 +182,26 @@ def service_update(zeroconf, service_type, name, state_change):
_LOGGER.debug("Discovered new device %s %s", name, info)

# If we can handle it as a HomeKit discovery, we do that here.
if service_type == HOMEKIT_TYPE and handle_homekit(hass, info):
return
if service_type == HOMEKIT_TYPE:
handle_homekit(hass, info)
# Continue on here as homekit_controller
Copy link
Copy Markdown
Member

@balloob balloob May 12, 2020

Choose a reason for hiding this comment

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

I am wondering here if this should be the responsibility of the zeroconf integration or if homekit_controller should register with the zeroconf ServiceBrowser directly instead.

Copy link
Copy Markdown
Member

@balloob balloob May 12, 2020

Choose a reason for hiding this comment

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

I think it's fine in this PR. Let's keep it.

# still needs to get updates on devices
# so it can see when the 'c#' field is updated.
#
# We only send updates to homekit_controller
# if the device is already paired in order to avoid
# offering a second discovery for the same device
if (
HOMEKIT_PROPERTIES in info
and HOMEKIT_PAIRED_STATUS_FLAG in info[HOMEKIT_PROPERTIES]
):
try:
if not int(info[HOMEKIT_PROPERTIES][HOMEKIT_PAIRED_STATUS_FLAG]):
return
except ValueError:
# HomeKit pairing status unknown
# likely bad homekit data
return

for domain in ZEROCONF[service_type]:
hass.add_job(
Expand All @@ -203,10 +225,10 @@ def handle_homekit(hass, info) -> bool:
Return if discovery was forwarded.
"""
model = None
props = info.get("properties", {})
props = info.get(HOMEKIT_PROPERTIES, {})

for key in props:
if key.lower() == "md":
if key.lower() == HOMEKIT_MODEL:
model = props[key]
break

Expand Down
63 changes: 57 additions & 6 deletions tests/components/zeroconf/test_init.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,9 @@
NON_ASCII_KEY: None,
}

HOMEKIT_STATUS_UNPAIRED = b"0"
HOMEKIT_STATUS_PAIRED = b"1"


@pytest.fixture
def mock_zeroconf():
Expand Down Expand Up @@ -45,8 +48,8 @@ def get_service_info_mock(service_type, name):
)


def get_homekit_info_mock(model):
"""Return homekit info for get_service_info."""
def get_homekit_info_mock(model, pairing_status):
"""Return homekit info for get_service_info for an homekit device."""

def mock_homekit_info(service_type, name):
return ServiceInfo(
Expand All @@ -57,7 +60,7 @@ def mock_homekit_info(service_type, name):
weight=0,
priority=0,
server="name.local.",
properties={b"md": model.encode()},
properties={b"md": model.encode(), b"sf": pairing_status},
)

return mock_homekit_info
Expand Down Expand Up @@ -119,7 +122,9 @@ async def test_homekit_match_partial_space(hass, mock_zeroconf):
) as mock_config_flow, patch.object(
zeroconf, "HaServiceBrowser", side_effect=service_update_mock
) as mock_service_browser:
mock_zeroconf.get_service_info.side_effect = get_homekit_info_mock("LIFX bulb")
mock_zeroconf.get_service_info.side_effect = get_homekit_info_mock(
"LIFX bulb", HOMEKIT_STATUS_UNPAIRED
)
assert await async_setup_component(hass, zeroconf.DOMAIN, {zeroconf.DOMAIN: {}})

assert len(mock_service_browser.mock_calls) == 1
Expand All @@ -137,7 +142,7 @@ async def test_homekit_match_partial_dash(hass, mock_zeroconf):
zeroconf, "HaServiceBrowser", side_effect=service_update_mock
) as mock_service_browser:
mock_zeroconf.get_service_info.side_effect = get_homekit_info_mock(
"Rachio-fa46ba"
"Rachio-fa46ba", HOMEKIT_STATUS_UNPAIRED
)
assert await async_setup_component(hass, zeroconf.DOMAIN, {zeroconf.DOMAIN: {}})

Expand All @@ -155,14 +160,60 @@ async def test_homekit_match_full(hass, mock_zeroconf):
) as mock_config_flow, patch.object(
zeroconf, "HaServiceBrowser", side_effect=service_update_mock
) as mock_service_browser:
mock_zeroconf.get_service_info.side_effect = get_homekit_info_mock("BSB002")
mock_zeroconf.get_service_info.side_effect = get_homekit_info_mock(
"BSB002", HOMEKIT_STATUS_UNPAIRED
)
assert await async_setup_component(hass, zeroconf.DOMAIN, {zeroconf.DOMAIN: {}})

homekit_mock = get_homekit_info_mock("BSB002", HOMEKIT_STATUS_UNPAIRED)
info = homekit_mock("_hap._tcp.local.", "BSB002._hap._tcp.local.")
import pprint

pprint.pprint(["homekit", info])
assert len(mock_service_browser.mock_calls) == 1
assert len(mock_config_flow.mock_calls) == 1
assert mock_config_flow.mock_calls[0][1][0] == "hue"


async def test_homekit_already_paired(hass, mock_zeroconf):
"""Test that an already paired device is sent to homekit_controller."""
with patch.dict(
zc_gen.ZEROCONF, {zeroconf.HOMEKIT_TYPE: ["homekit_controller"]}, clear=True
), patch.object(
hass.config_entries.flow, "async_init"
) as mock_config_flow, patch.object(
zeroconf, "HaServiceBrowser", side_effect=service_update_mock
) as mock_service_browser:
mock_zeroconf.get_service_info.side_effect = get_homekit_info_mock(
"tado", HOMEKIT_STATUS_PAIRED
)
assert await async_setup_component(hass, zeroconf.DOMAIN, {zeroconf.DOMAIN: {}})

assert len(mock_service_browser.mock_calls) == 1
assert len(mock_config_flow.mock_calls) == 2
assert mock_config_flow.mock_calls[0][1][0] == "tado"
assert mock_config_flow.mock_calls[1][1][0] == "homekit_controller"


async def test_homekit_invalid_paring_status(hass, mock_zeroconf):
"""Test that missing paring data is not sent to homekit_controller."""
with patch.dict(
zc_gen.ZEROCONF, {zeroconf.HOMEKIT_TYPE: ["homekit_controller"]}, clear=True
), patch.object(
hass.config_entries.flow, "async_init"
) as mock_config_flow, patch.object(
zeroconf, "HaServiceBrowser", side_effect=service_update_mock
) as mock_service_browser:
mock_zeroconf.get_service_info.side_effect = get_homekit_info_mock(
"tado", b"invalid"
)
assert await async_setup_component(hass, zeroconf.DOMAIN, {zeroconf.DOMAIN: {}})

assert len(mock_service_browser.mock_calls) == 1
assert len(mock_config_flow.mock_calls) == 1
assert mock_config_flow.mock_calls[0][1][0] == "tado"


async def test_info_from_service_non_utf8(hass):
"""Test info_from_service handles non UTF-8 property keys and values correctly."""
service_type = "_test._tcp.local."
Expand Down