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.

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

Jump to bottom

Add config flow for Ecovacs #108111

Merged
merged 9 commits into from Jan 16, 2024
Merged

Add config flow for Ecovacs #108111

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
8a01001
Add config flow for Ecovacs
edenhaus Jan 15, 2024
03f2c10
Implement suggestions
edenhaus Jan 15, 2024
9700ad0
Validate data on import
edenhaus Jan 15, 2024
c7b4155
assert_called
edenhaus Jan 15, 2024
27603e7
Apply suggestions from code review
edenhaus Jan 16, 2024
2d0f5b7
Implement suggestions
edenhaus Jan 16, 2024
4c4f16b
Add descriptions
edenhaus Jan 16, 2024
dca4f4b
Update homeassistant/components/ecovacs/strings.json
edenhaus Jan 16, 2024
c44dd21
Remove error handling for now
edenhaus Jan 16, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
62 changes: 35 additions & 27 deletions homeassistant/components/ecovacs/__init__.py
View file
Open in desktop
Expand Up @@ -13,6 +13,7 @@
Platform,
)
from homeassistant.core import HomeAssistant
from homeassistant.exceptions import ConfigEntryError, ConfigEntryNotReady
import homeassistant.helpers.config_validation as cv
from homeassistant.helpers.typing import ConfigType

Expand Down Expand Up @@ -56,35 +57,42 @@ async def async_setup_entry(hass: HomeAssistant, entry: ConfigEntry) -> bool:
"""Set up this integration using UI."""

def get_devices() -> list[VacBot]:
ecovacs_api = EcoVacsAPI(
get_client_device_id(),
entry.data[CONF_USERNAME],
EcoVacsAPI.md5(entry.data[CONF_PASSWORD]),
entry.data[CONF_COUNTRY],
entry.data[CONF_CONTINENT],
)
ecovacs_devices = ecovacs_api.devices()
_LOGGER.debug("Ecobot devices: %s", ecovacs_devices)

devices: list[VacBot] = []
for device in ecovacs_devices:
_LOGGER.info(
"Discovered Ecovacs device on account: %s with nickname %s",
device.get("did"),
device.get("nick"),
)
vacbot = VacBot(
ecovacs_api.uid,
ecovacs_api.REALM,
ecovacs_api.resource,
ecovacs_api.user_access_token,
device,
try:
ecovacs_api = EcoVacsAPI(
get_client_device_id(),
entry.data[CONF_USERNAME],
EcoVacsAPI.md5(entry.data[CONF_PASSWORD]),
entry.data[CONF_COUNTRY],
entry.data[CONF_CONTINENT],
monitor=True,
)

devices.append(vacbot)
return devices
ecovacs_devices = ecovacs_api.devices()
_LOGGER.debug("Ecobot devices: %s", ecovacs_devices)

devices: list[VacBot] = []
for device in ecovacs_devices:
_LOGGER.info(
edenhaus marked this conversation as resolved.
Show resolved Hide resolved
"Discovered Ecovacs device on account: %s with nickname %s",
device.get("did"),
device.get("nick"),
)
vacbot = VacBot(
ecovacs_api.uid,
ecovacs_api.REALM,
ecovacs_api.resource,
ecovacs_api.user_access_token,
device,
entry.data[CONF_CONTINENT],
monitor=True,
)

devices.append(vacbot)
return devices
except ValueError as ex:
edenhaus marked this conversation as resolved.
Show resolved Hide resolved
_LOGGER.error("Ecovacs login failed due to wrong username/password")
edenhaus marked this conversation as resolved.
Show resolved Hide resolved
raise ConfigEntryError("invalid username or password") from ex
except RuntimeError as ex:
_LOGGER.exception("Unexpected exception")
Copy link
Member

Choose a reason for hiding this comment

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

Normally we don't log when we try again to avoid spamming the log. We can pass the message as an exception argument, which will be logged.

It's always ok to log at debug level, besides performance considerations. ConfigEntryNotReady will log the message at debug level.

We also normally don't catch unknown exceptions outside of the config flow.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Will add the error handling in a follow-up including the new lib

raise ConfigEntryNotReady from ex

hass.data.setdefault(DOMAIN, {})[
entry.entry_id
Expand Down
67 changes: 47 additions & 20 deletions homeassistant/components/ecovacs/config_flow.py
View file
Open in desktop
Expand Up @@ -10,7 +10,7 @@
from homeassistant.config_entries import ConfigFlow
from homeassistant.const import CONF_COUNTRY, CONF_PASSWORD, CONF_USERNAME
from homeassistant.core import DOMAIN as HOMEASSISTANT_DOMAIN
from homeassistant.data_entry_flow import FlowResult
from homeassistant.data_entry_flow import AbortFlow, FlowResult
from homeassistant.helpers import selector
import homeassistant.helpers.config_validation as cv
from homeassistant.helpers.issue_registry import IssueSeverity, async_create_issue
Expand Down Expand Up @@ -53,12 +53,11 @@ async def async_step_user(
errors = {}

if user_input:
self._async_abort_entries_match({CONF_USERNAME: user_input[CONF_USERNAME]})

errors = await self.hass.async_add_executor_job(validate_input, user_input)

if not errors:
self._async_abort_entries_match(
{CONF_USERNAME: user_input[CONF_USERNAME]}
)
return self.async_create_entry(
title=user_input[CONF_USERNAME], data=user_input
)
Expand Down Expand Up @@ -89,20 +88,48 @@ async def async_step_user(

async def async_step_import(self, user_input: dict[str, Any]) -> FlowResult:
"""Import configuration from yaml."""
async_create_issue(
self.hass,
HOMEASSISTANT_DOMAIN,
f"deprecated_yaml_{DOMAIN}",
breaks_in_ha_version="2024.8.0",
is_fixable=False,
issue_domain=DOMAIN,
severity=IssueSeverity.WARNING,
translation_key="deprecated_yaml",
translation_placeholders={
"domain": DOMAIN,
"integration_title": "Ecovacs",
},
)

self._async_abort_entries_match({CONF_USERNAME: user_input[CONF_USERNAME]})
return self.async_create_entry(title=user_input[CONF_USERNAME], data=user_input)
def create_repair(error: str | None = None) -> None:
if error:
async_create_issue(
self.hass,
DOMAIN,
f"deprecated_yaml_import_issue_{error}",
breaks_in_ha_version="2024.8.0",
is_fixable=False,
issue_domain=DOMAIN,
severity=IssueSeverity.WARNING,
translation_key=f"deprecated_yaml_import_issue_{error}",
translation_placeholders={
"url": "/config/integrations/dashboard/add?domain=ecovacs"
},
)
else:
async_create_issue(
self.hass,
HOMEASSISTANT_DOMAIN,
f"deprecated_yaml_{DOMAIN}",
breaks_in_ha_version="2024.8.0",
is_fixable=False,
issue_domain=DOMAIN,
severity=IssueSeverity.WARNING,
translation_key="deprecated_yaml",
translation_placeholders={
"domain": DOMAIN,
"integration_title": "Ecovacs",
},
)

try:
result = await self.async_step_user(user_input)
except AbortFlow as ex:
if ex.reason == "already_configured":
create_repair()
raise ex

if errors := result.get("errors"):
create_repair(errors["base"])
else:
create_repair()

return result
11 changes: 11 additions & 0 deletions homeassistant/components/ecovacs/strings.json
View file
Open in desktop
Expand Up @@ -11,10 +11,21 @@
"user": {
"data": {
"continent": "Continent",
"country": "Country",
edenhaus marked this conversation as resolved.
Show resolved Hide resolved
"password": "[%key:common::config_flow::data::password%]",
"username": "E-mail or ShortID"
edenhaus marked this conversation as resolved.
Show resolved Hide resolved
}
}
}
},
"issues": {
"deprecated_yaml_import_issue_invalid_auth": {
"title": "The Ecovacs YAML configuration import failed",
"description": "Configuring Ecovacs using YAML is being removed but there was an authentication error importing your YAML configuration.\n\nCorrect the YAML configuration and restart Home Assistant to try again or remove the Ecovacs YAML configuration from your configuration.yaml file and continue to [set up the integration]({url}) manually."
edenhaus marked this conversation as resolved.
Show resolved Hide resolved
},
"deprecated_yaml_import_issue_unknown": {
"title": "The Ecovacs YAML configuration import failed",
"description": "Configuring Ecovacs using YAML is being removed but there was an unknown error when trying to import the YAML configuration.\n\nEnsure the imported configuration is correct and restart Home Assistant to try again or remove the Ecovacs YAML configuration from your configuration.yaml file and continue to [set up the integration]({url}) manually."
edenhaus marked this conversation as resolved.
Show resolved Hide resolved
}
}
}
72 changes: 55 additions & 17 deletions tests/components/ecovacs/test_config_flow.py
View file
Open in desktop
Expand Up @@ -8,7 +8,7 @@
from homeassistant.components.ecovacs.const import CONF_CONTINENT, DOMAIN
from homeassistant.config_entries import SOURCE_IMPORT, SOURCE_USER
from homeassistant.const import CONF_COUNTRY, CONF_PASSWORD, CONF_USERNAME
from homeassistant.core import HomeAssistant
from homeassistant.core import DOMAIN as HOMEASSISTANT_DOMAIN, HomeAssistant
from homeassistant.data_entry_flow import FlowResultType
from homeassistant.helpers import issue_registry as ir

Expand All @@ -22,7 +22,7 @@
}


async def _test_config_flow(hass: HomeAssistant) -> dict[str, Any]:
async def _test_user_flow(hass: HomeAssistant) -> dict[str, Any]:
"""Test config flow."""
result = await hass.config_entries.flow.async_init(
DOMAIN,
Expand All @@ -35,13 +35,13 @@ async def _test_config_flow(hass: HomeAssistant) -> dict[str, Any]:
)


async def test_full_user_flow(hass: HomeAssistant, mock_setup_entry) -> None:
"""Test the full user configuration flow."""
async def test_user_flow(hass: HomeAssistant, mock_setup_entry) -> None:
"""Test the user config flow."""
with patch(
"homeassistant.components.ecovacs.config_flow.EcoVacsAPI",
return_value=Mock(spec_set=EcoVacsAPI),
):
result = await _test_config_flow(hass)
result = await _test_user_flow(hass)
assert result["type"] == FlowResultType.CREATE_ENTRY
assert result["title"] == _USER_INPUT[CONF_USERNAME]
assert result["data"] == _USER_INPUT
Expand All @@ -54,8 +54,8 @@ async def test_full_user_flow(hass: HomeAssistant, mock_setup_entry) -> None:
(Exception, "unknown"),
],
)
async def test_flow_error(
hass: HomeAssistant, side_effect: Exception, reason: str, mock_setup_entry
async def test_user_flow_error(
hass: HomeAssistant, side_effect: Exception, reason: str
) -> None:
"""Test handling invalid connection."""
with patch(
Expand All @@ -64,7 +64,7 @@ async def test_flow_error(
) as mock_ecovacs:
mock_ecovacs.side_effect = side_effect

result = await _test_config_flow(hass)
result = await _test_user_flow(hass)
assert result["type"] == FlowResultType.FORM
assert result["step_id"] == "user"
assert result["errors"] == {"base": reason}
Expand All @@ -74,21 +74,25 @@ async def test_import_flow(
hass: HomeAssistant, issue_registry: ir.IssueRegistry, mock_setup_entry
) -> None:
"""Test importing yaml config."""
result = await hass.config_entries.flow.async_init(
DOMAIN,
context={"source": SOURCE_IMPORT},
data=_USER_INPUT,
)
with patch(
"homeassistant.components.ecovacs.config_flow.EcoVacsAPI",
return_value=Mock(spec_set=EcoVacsAPI),
):
result = await hass.config_entries.flow.async_init(
DOMAIN,
context={"source": SOURCE_IMPORT},
data=_USER_INPUT,
)
assert result["type"] == FlowResultType.CREATE_ENTRY
assert result["title"] == _USER_INPUT[CONF_USERNAME]
assert result["data"] == _USER_INPUT
assert len(issue_registry.issues) == 1
assert (HOMEASSISTANT_DOMAIN, f"deprecated_yaml_{DOMAIN}") in issue_registry.issues


async def test_import_flow_already_exists(
async def test_import_flow_already_configured(
hass: HomeAssistant, issue_registry: ir.IssueRegistry
) -> None:
"""Test importing yaml config where entry already exists."""
"""Test importing yaml config where entry already configured."""
entry = MockConfigEntry(domain=DOMAIN, data=_USER_INPUT)
entry.add_to_hass(hass)

Expand All @@ -99,4 +103,38 @@ async def test_import_flow_already_exists(
)
assert result["type"] == FlowResultType.ABORT
assert result["reason"] == "already_configured"
assert len(issue_registry.issues) == 1
assert (HOMEASSISTANT_DOMAIN, f"deprecated_yaml_{DOMAIN}") in issue_registry.issues


@pytest.mark.parametrize(
("side_effect", "reason"),
[
(ValueError, "invalid_auth"),
(Exception, "unknown"),
],
)
async def test_import_flow_error(
hass: HomeAssistant,
side_effect: Exception,
reason: str,
issue_registry: ir.IssueRegistry,
) -> None:
"""Test handling invalid connection."""
with patch(
"homeassistant.components.ecovacs.config_flow.EcoVacsAPI",
return_value=Mock(spec_set=EcoVacsAPI),
) as mock_ecovacs:
mock_ecovacs.side_effect = side_effect

result = await hass.config_entries.flow.async_init(
DOMAIN,
context={"source": SOURCE_IMPORT},
data=_USER_INPUT,
)
assert result["type"] == FlowResultType.FORM
assert result["step_id"] == "user"
edenhaus marked this conversation as resolved.
Show resolved Hide resolved
assert result["errors"] == {"base": reason}
assert (
DOMAIN,
f"deprecated_yaml_import_issue_{reason}",
) in issue_registry.issues