Skip to content
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.

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

Add zeroconf support for velux integration #112451

Closed
wants to merge 13 commits into from
Closed
Show file tree
Hide file tree
Changes from 12 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
4 changes: 2 additions & 2 deletions CODEOWNERS
Original file line number Diff line number Diff line change
Expand Up @@ -1463,8 +1463,8 @@ build.json @home-assistant/supervisor
/tests/components/valve/ @home-assistant/core
/homeassistant/components/velbus/ @Cereal2nd @brefra
/tests/components/velbus/ @Cereal2nd @brefra
/homeassistant/components/velux/ @Julius2342 @DeerMaximum
/tests/components/velux/ @Julius2342 @DeerMaximum
/homeassistant/components/velux/ @Julius2342 @DeerMaximum @pawlizio
/tests/components/velux/ @Julius2342 @DeerMaximum @pawlizio
/homeassistant/components/venstar/ @garbled1 @jhollowe
/tests/components/venstar/ @garbled1 @jhollowe
/homeassistant/components/versasense/ @imstevenxyz
Expand Down
78 changes: 71 additions & 7 deletions homeassistant/components/velux/config_flow.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,13 +3,21 @@
from typing import Any

from pyvlx import PyVLX, PyVLXException
from pyvlx.discovery import VeluxDiscovery, VeluxHost
import voluptuous as vol

from homeassistant import config_entries
from homeassistant.components import zeroconf
from homeassistant.components.zeroconf import ZeroconfServiceInfo
from homeassistant.config_entries import ConfigFlow, ConfigFlowResult
from homeassistant.const import CONF_HOST, CONF_PASSWORD
from homeassistant.core import DOMAIN as HOMEASSISTANT_DOMAIN
import homeassistant.helpers.config_validation as cv
from homeassistant.helpers.issue_registry import IssueSeverity, async_create_issue
from homeassistant.helpers.selector import (
SelectSelector,
SelectSelectorConfig,
SelectSelectorMode,
)

from .const import DOMAIN, LOGGER

Expand All @@ -21,12 +29,15 @@
)


class VeluxConfigFlow(config_entries.ConfigFlow, domain=DOMAIN):
class VeluxConfigFlow(ConfigFlow, domain=DOMAIN):
"""Handle a config flow for velux."""

async def async_step_import(
self, config: dict[str, Any]
) -> config_entries.ConfigFlowResult:
VERSION = 1
MINOR_VERSION = 2

hosts: list[VeluxHost] = []

async def async_step_import(self, config: dict[str, Any]) -> ConfigFlowResult:
"""Import a config entry."""

def create_repair(error: str | None = None) -> None:
Expand Down Expand Up @@ -81,12 +92,19 @@ def create_repair(error: str | None = None) -> None:

async def async_step_user(
self, user_input: dict[str, str] | None = None
) -> config_entries.ConfigFlowResult:
) -> ConfigFlowResult:
"""Handle the initial step."""
errors: dict[str, str] = {}

if user_input is not None:
self._async_abort_entries_match({CONF_HOST: user_input[CONF_HOST]})
if self.hosts:
for host in self.hosts:
if user_input[CONF_HOST] == host.ip_address:
await self.async_set_unique_id(host.hostname)
self._abort_if_unique_id_configured(
updates={CONF_HOST: host.ip_address}
)

pyvlx = PyVLX(
host=user_input[CONF_HOST], password=user_input[CONF_PASSWORD]
Expand All @@ -106,9 +124,55 @@ async def async_step_user(
data=user_input,
)

data_schema = self.add_suggested_values_to_schema(DATA_SCHEMA, user_input)
if not self.hosts:
aiozc = await zeroconf.async_get_async_instance(self.hass)
vd: VeluxDiscovery = VeluxDiscovery(zeroconf=aiozc)
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm not sure about this, do we really need both user triggered and automatic discovery?
Maybe we could add functionality to the Home Assistant zeroconf integration which allows fetching services already discovered for a services, similar to what we support for bluetooth:

for discovery in async_discovered_service_info(self.hass):

Copy link
Member

Choose a reason for hiding this comment

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

I think adding that to zeroconf would be helpful long term. Right now users tend to ignore discoveries and than can’t find the integration and than open issues if they have no way to add it manually once it’s ignored. If they do discover that and unignore it, a restart is generally required to find it again.

When we solve these issues we probably don’t need manual adding anymore but we are still a ways off from a solution there. In the mean time it makes sense to implement manual adding via a user flow

Copy link
Member

@frenck frenck Jul 7, 2024

Choose a reason for hiding this comment

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

IMHO, we shouldn't add this here; and instead solve the issue at hand in core instead.

Copy link
Contributor

Choose a reason for hiding this comment

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

I agree. This PR could drive the needed change in the zeroconf integration. @pawlizio Is this something you could look into? You can contact me on Discord if it's not clear where to start.

if await vd.async_discover_hosts(expected_hosts=1):
self.hosts = vd.hosts # type: ignore[assignment]

if self.hosts:
data_schema = vol.Schema(
{
vol.Required(CONF_HOST): SelectSelector(
SelectSelectorConfig(
options=[host.ip_address for host in self.hosts],
custom_value=True,
mode=SelectSelectorMode.DROPDOWN,
)
),
vol.Required(CONF_PASSWORD): cv.string,
}
)
else:
data_schema = self.add_suggested_values_to_schema(DATA_SCHEMA, user_input)

return self.async_show_form(
step_id="user",
data_schema=data_schema,
errors=errors,
)

async def async_step_unignore(self, user_input: dict[str, Any]) -> ConfigFlowResult:
"""Rediscover a previously ignored discover."""
unique_id = user_input["unique_id"]
await self.async_set_unique_id(unique_id)
return await self.async_step_user()

async def async_step_zeroconf(
self, discovery_info: ZeroconfServiceInfo
) -> ConfigFlowResult:
"""Handle discovery by zeroconf."""
hostname = discovery_info.hostname.replace(".local.", "")
await self.async_set_unique_id(hostname)
self._abort_if_unique_id_configured(updates={CONF_HOST: discovery_info.host})

# Check if config_entry exists already without unigue_id configured.
for entry in self.hass.config_entries.async_entries(DOMAIN):
if entry.data[CONF_HOST] == discovery_info.host and entry.unique_id is None:
self.hass.config_entries.async_update_entry(
entry=entry, unique_id=hostname
)
return self.async_abort(reason="already_configured")

self.hosts.append(VeluxHost(hostname=hostname, ip_address=discovery_info.host))
return await self.async_step_user()
12 changes: 10 additions & 2 deletions homeassistant/components/velux/manifest.json
Original file line number Diff line number Diff line change
@@ -1,10 +1,18 @@
{
"domain": "velux",
"name": "Velux",
"codeowners": ["@Julius2342", "@DeerMaximum"],
"codeowners": ["@Julius2342", "@DeerMaximum", "@pawlizio"],
"config_flow": true,
"documentation": "https://www.home-assistant.io/integrations/velux",
"integration_type": "hub",
"iot_class": "local_polling",
"issue_tracker": "https://github.com/Julius2342/pyvlx/issues",
pawlizio marked this conversation as resolved.
Show resolved Hide resolved
"loggers": ["pyvlx"],
"requirements": ["pyvlx==0.2.21"]
"requirements": ["pyvlx==0.2.23"],
pawlizio marked this conversation as resolved.
Show resolved Hide resolved
"zeroconf": [
{
"type": "_http._tcp.local.",
"name": "velux_klf_lan*"
}
]
}
4 changes: 4 additions & 0 deletions homeassistant/generated/zeroconf.py
Original file line number Diff line number Diff line change
Expand Up @@ -515,6 +515,10 @@
"vendor": "tailwind",
},
},
{
"domain": "velux",
"name": "velux_klf_lan*",
},
],
"_hue._tcp.local.": [
{
Expand Down
2 changes: 1 addition & 1 deletion requirements_all.txt
Original file line number Diff line number Diff line change
Expand Up @@ -2355,7 +2355,7 @@ pyvesync==2.1.10
pyvizio==0.1.61

# homeassistant.components.velux
pyvlx==0.2.21
pyvlx==0.2.23

# homeassistant.components.volumio
pyvolumio==0.1.5
Expand Down
2 changes: 1 addition & 1 deletion requirements_test_all.txt
Original file line number Diff line number Diff line change
Expand Up @@ -1813,7 +1813,7 @@ pyvesync==2.1.10
pyvizio==0.1.61

# homeassistant.components.velux
pyvlx==0.2.21
pyvlx==0.2.23

# homeassistant.components.volumio
pyvolumio==0.1.5
Expand Down
21 changes: 21 additions & 0 deletions tests/components/velux/conftest.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@

import pytest

from homeassistant.components.zeroconf import HaAsyncZeroconf


@pytest.fixture
def mock_setup_entry() -> Generator[AsyncMock, None, None]:
Expand All @@ -13,3 +15,22 @@ def mock_setup_entry() -> Generator[AsyncMock, None, None]:
"homeassistant.components.velux.async_setup_entry", return_value=True
) as mock_setup_entry:
yield mock_setup_entry


@pytest.fixture
def mock_velux_discovery() -> Generator[AsyncMock, None, None]:
"""Override async_setup_entry."""
with patch(
"pyvlx.discovery.VeluxDiscovery._async_discover_hosts",
autospec=True,
) as mock_velux_discovery:
yield mock_velux_discovery


@pytest.fixture
def mock_async_zeroconf(mock_zeroconf: None) -> Generator[None, None, None]:
"""Mock AsyncZeroconf."""
with patch(
"homeassistant.components.zeroconf.HaAsyncZeroconf", spec=HaAsyncZeroconf
) as mock_aiozc:
yield mock_aiozc
4 changes: 4 additions & 0 deletions tests/components/velux/const.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
"""Constants for the Velux tests."""
PASSWORD = "NotAStrongPassword"
HOST = "127.1.1.1"
HOSTNAME = "VELUX_KLF_LAN_ABCD"
Loading
Loading