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

Improve Ikea Idasen config flow error messages #101567

Merged
merged 1 commit into from
Oct 7, 2023
Merged
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
7 changes: 5 additions & 2 deletions homeassistant/components/idasen_desk/config_flow.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,9 +4,9 @@
import logging
from typing import Any

from bleak import BleakError
from bleak.exc import BleakError
from bluetooth_data_tools import human_readable_name
from idasen_ha import Desk
from idasen_ha import AuthFailedError, Desk
import voluptuous as vol

from homeassistant import config_entries
Expand Down Expand Up @@ -64,6 +64,9 @@ async def async_step_user(
desk = Desk(None)
try:
await desk.connect(discovery_info.device, monitor_height=False)
except AuthFailedError as err:
_LOGGER.exception("AuthFailedError", exc_info=err)
Copy link
Member

Choose a reason for hiding this comment

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

Using both the exception logging method and exc_info isn't needed.

We normally don't log the stack trace if the exception is known.

errors["base"] = "auth_failed"
frenck marked this conversation as resolved.
Show resolved Hide resolved
except TimeoutError as err:
_LOGGER.exception("TimeoutError", exc_info=err)
errors["base"] = "cannot_connect"
Expand Down
2 changes: 1 addition & 1 deletion homeassistant/components/idasen_desk/manifest.json
Original file line number Diff line number Diff line change
Expand Up @@ -11,5 +11,5 @@
"dependencies": ["bluetooth_adapters"],
"documentation": "https://www.home-assistant.io/integrations/idasen_desk",
"iot_class": "local_push",
"requirements": ["idasen-ha==1.4"]
"requirements": ["idasen-ha==1.4.1"]
}
3 changes: 2 additions & 1 deletion homeassistant/components/idasen_desk/strings.json
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,8 @@
}
},
"error": {
"cannot_connect": "[%key:common::config_flow::error::cannot_connect%]",
"auth_failed": "Unable to authenticate with the desk. This is usually solved by using an ESPHome Bluetooth Proxy. Please check the integration documentation for alternative workarounds.",
"cannot_connect": "Cannot connect. Make sure that the desk is in Bluetooth pairing mode. If not already, you can also use an ESPHome Bluetooth Proxy, as it provides a better connection.",
"unknown": "[%key:common::config_flow::error::unknown%]"
},
"abort": {
Expand Down
2 changes: 1 addition & 1 deletion requirements_all.txt
Original file line number Diff line number Diff line change
Expand Up @@ -1042,7 +1042,7 @@ ical==5.0.1
icmplib==3.0

# homeassistant.components.idasen_desk
idasen-ha==1.4
idasen-ha==1.4.1

# homeassistant.components.network
ifaddr==0.2.0
Expand Down
2 changes: 1 addition & 1 deletion requirements_test_all.txt
Original file line number Diff line number Diff line change
Expand Up @@ -822,7 +822,7 @@ ical==5.0.1
icmplib==3.0

# homeassistant.components.idasen_desk
idasen-ha==1.4
idasen-ha==1.4.1

# homeassistant.components.network
ifaddr==0.2.0
Expand Down
55 changes: 54 additions & 1 deletion tests/components/idasen_desk/test_config_flow.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
from unittest.mock import patch

from bleak import BleakError
from idasen_ha import AuthFailedError
import pytest

from homeassistant import config_entries
Expand Down Expand Up @@ -89,7 +90,7 @@ async def test_user_step_no_new_devices_found(hass: HomeAssistant) -> None:
async def test_user_step_cannot_connect(
hass: HomeAssistant, exception: Exception
) -> None:
"""Test user step and we cannot connect."""
"""Test user step with a cannot connect error."""
with patch(
"homeassistant.components.idasen_desk.config_flow.async_discovered_service_info",
return_value=[IDASEN_DISCOVERY_INFO],
Expand Down Expand Up @@ -140,6 +141,58 @@ async def test_user_step_cannot_connect(
assert len(mock_setup_entry.mock_calls) == 1


async def test_user_step_auth_failed(hass: HomeAssistant) -> None:
"""Test user step with an auth failed error."""
with patch(
"homeassistant.components.idasen_desk.config_flow.async_discovered_service_info",
return_value=[IDASEN_DISCOVERY_INFO],
):
result = await hass.config_entries.flow.async_init(
DOMAIN, context={"source": config_entries.SOURCE_USER}
)
assert result["type"] == FlowResultType.FORM
assert result["step_id"] == "user"
assert result["errors"] == {}

with patch(
"homeassistant.components.idasen_desk.config_flow.Desk.connect",
side_effect=AuthFailedError,
), patch("homeassistant.components.idasen_desk.config_flow.Desk.disconnect"):
result2 = await hass.config_entries.flow.async_configure(
result["flow_id"],
{
CONF_ADDRESS: IDASEN_DISCOVERY_INFO.address,
},
)
await hass.async_block_till_done()

assert result2["type"] == FlowResultType.FORM
assert result2["step_id"] == "user"
assert result2["errors"] == {"base": "auth_failed"}

with patch("homeassistant.components.idasen_desk.config_flow.Desk.connect"), patch(
"homeassistant.components.idasen_desk.config_flow.Desk.disconnect"
), patch(
"homeassistant.components.idasen_desk.async_setup_entry",
return_value=True,
) as mock_setup_entry:
result3 = await hass.config_entries.flow.async_configure(
result2["flow_id"],
{
CONF_ADDRESS: IDASEN_DISCOVERY_INFO.address,
},
)
await hass.async_block_till_done()

assert result3["type"] == FlowResultType.CREATE_ENTRY
assert result3["title"] == IDASEN_DISCOVERY_INFO.name
assert result3["data"] == {
CONF_ADDRESS: IDASEN_DISCOVERY_INFO.address,
}
assert result3["result"].unique_id == IDASEN_DISCOVERY_INFO.address
assert len(mock_setup_entry.mock_calls) == 1


async def test_user_step_unknown_exception(hass: HomeAssistant) -> None:
"""Test user step with an unknown exception."""
with patch(
Expand Down