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

Fix freebox pairing in bridge mode #106131

Merged
merged 1 commit into from Feb 15, 2024
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.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
4 changes: 2 additions & 2 deletions homeassistant/components/freebox/config_flow.py
Expand Up @@ -11,7 +11,7 @@
from homeassistant.data_entry_flow import FlowResult

from .const import DOMAIN
from .router import get_api
from .router import get_api, get_hosts_list_if_supported

_LOGGER = logging.getLogger(__name__)

Expand Down Expand Up @@ -69,7 +69,7 @@ async def async_step_link(

# Check permissions
await fbx.system.get_config()
await fbx.lan.get_hosts_list()
await get_hosts_list_if_supported(fbx)

# Close connection
await fbx.close()
Expand Down
51 changes: 30 additions & 21 deletions homeassistant/components/freebox/router.py
Expand Up @@ -64,6 +64,33 @@ async def get_api(hass: HomeAssistant, host: str) -> Freepybox:
return Freepybox(APP_DESC, token_file, API_VERSION)


async def get_hosts_list_if_supported(
fbx_api: Freepybox,
) -> tuple[bool, list[dict[str, Any]]]:
"""Hosts list is not supported when freebox is configured in bridge mode."""
supports_hosts: bool = True
fbx_devices: list[dict[str, Any]] = []
try:
fbx_devices = await fbx_api.lan.get_hosts_list() or []
except HttpRequestError as err:
if (
(matcher := re.search(r"Request failed \(APIResponse: (.+)\)", str(err)))
and is_json(json_str := matcher.group(1))
and (json_resp := json.loads(json_str)).get("error_code") == "nodev"
):
# No need to retry, Host list not available
supports_hosts = False
_LOGGER.debug(
"Host list is not available using bridge mode (%s)",
json_resp.get("msg"),
)

else:
raise err

return supports_hosts, fbx_devices


class FreeboxRouter:
"""Representation of a Freebox router."""

Expand Down Expand Up @@ -111,27 +138,9 @@ async def update_device_trackers(self) -> None:

# Access to Host list not available in bridge mode, API return error_code 'nodev'
if self.supports_hosts:
try:
fbx_devices = await self._api.lan.get_hosts_list()
except HttpRequestError as err:
if (
(
matcher := re.search(
r"Request failed \(APIResponse: (.+)\)", str(err)
)
)
and is_json(json_str := matcher.group(1))
and (json_resp := json.loads(json_str)).get("error_code") == "nodev"
):
# No need to retry, Host list not available
self.supports_hosts = False
_LOGGER.debug(
"Host list is not available using bridge mode (%s)",
json_resp.get("msg"),
)

else:
raise err
self.supports_hosts, fbx_devices = await get_hosts_list_if_supported(
self._api
)

# Adds the Freebox itself
fbx_devices.append(
Expand Down
11 changes: 11 additions & 0 deletions tests/components/freebox/conftest.py
Expand Up @@ -112,3 +112,14 @@ def mock_router_bridge_mode(mock_device_registry_devices, router):
)

return router


@pytest.fixture
def mock_router_bridge_mode_error(mock_device_registry_devices, router):
"""Mock a failed connection to Freebox Bridge mode."""

router().lan.get_hosts_list = AsyncMock(
side_effect=HttpRequestError("Request failed (APIResponse: some unknown error)")
)

return router
28 changes: 26 additions & 2 deletions tests/components/freebox/test_config_flow.py
Expand Up @@ -69,8 +69,8 @@ async def test_zeroconf(hass: HomeAssistant) -> None:
assert result["step_id"] == "link"


async def test_link(hass: HomeAssistant, router: Mock) -> None:
"""Test linking."""
async def internal_test_link(hass: HomeAssistant) -> None:
"""Test linking internal, common to both router modes."""
with patch(
"homeassistant.components.freebox.async_setup_entry",
return_value=True,
Expand All @@ -91,6 +91,30 @@ async def test_link(hass: HomeAssistant, router: Mock) -> None:
assert len(mock_setup_entry.mock_calls) == 1


async def test_link(hass: HomeAssistant, router: Mock) -> None:
"""Test link with standard router mode."""
await internal_test_link(hass)


async def test_link_bridge_mode(hass: HomeAssistant, router_bridge_mode: Mock) -> None:
"""Test linking for a freebox in bridge mode."""
await internal_test_link(hass)


async def test_link_bridge_mode_error(
hass: HomeAssistant, mock_router_bridge_mode_error: Mock
) -> None:
"""Test linking for a freebox in bridge mode, unknown error received from API."""
result = await hass.config_entries.flow.async_init(
DOMAIN,
context={"source": SOURCE_USER},
data={CONF_HOST: MOCK_HOST, CONF_PORT: MOCK_PORT},
)
result = await hass.config_entries.flow.async_configure(result["flow_id"], {})
assert result["type"] == data_entry_flow.FlowResultType.FORM
assert result["errors"] == {"base": "cannot_connect"}


async def test_abort_if_already_setup(hass: HomeAssistant) -> None:
"""Test we abort if component is already setup."""
MockConfigEntry(
Expand Down
36 changes: 35 additions & 1 deletion tests/components/freebox/test_router.py
@@ -1,7 +1,11 @@
"""Tests for the Freebox utility methods."""
import json
from unittest.mock import Mock

from homeassistant.components.freebox.router import is_json
from freebox_api.exceptions import HttpRequestError
import pytest

from homeassistant.components.freebox.router import get_hosts_list_if_supported, is_json

from .const import DATA_LAN_GET_HOSTS_LIST_MODE_BRIDGE, DATA_WIFI_GET_GLOBAL_CONFIG

Expand All @@ -20,3 +24,33 @@ async def test_is_json() -> None:
assert not is_json("")
assert not is_json("XXX")
assert not is_json("{XXX}")


async def test_get_hosts_list_if_supported(
router: Mock,
) -> None:
"""In router mode, get_hosts_list is supported and list is filled."""
supports_hosts, fbx_devices = await get_hosts_list_if_supported(router())
assert supports_hosts is True
# List must not be empty; but it's content depends on how many unit tests are executed...
assert fbx_devices
assert "d633d0c8-958c-43cc-e807-d881b076924b" in str(fbx_devices)


async def test_get_hosts_list_if_supported_bridge(
router_bridge_mode: Mock,
) -> None:
"""In bridge mode, get_hosts_list is NOT supported and list is empty."""
supports_hosts, fbx_devices = await get_hosts_list_if_supported(
router_bridge_mode()
)
assert supports_hosts is False
assert fbx_devices == []


async def test_get_hosts_list_if_supported_bridge_error(
mock_router_bridge_mode_error: Mock,
) -> None:
"""Other exceptions must be propagated."""
with pytest.raises(HttpRequestError):
await get_hosts_list_if_supported(mock_router_bridge_mode_error())