Fix integration failed when freebox is configured in bridge mode #103221

jflefebvre06 · 2023-11-02T08:27:31Z

Breaking change

Proposed change

Allows freebox integration to work for users who have configured their router in bridge mode.

The problem is due to the fact that in bridge mode, access to the Host list is not implemented.

freebox_api.exceptions.HttpRequestError: Request failed (APIResponse: {"msg": "Erreur lors de la r\u00e9cup\u00e9ration de la liste des h\u00f4tes : Interface invalide", "success": false, "error_code": "nodev"})

Type of change

Dependency upgrade
Bugfix (non-breaking change which fixes an issue)
New integration (thank you!)
New feature (which adds functionality to an existing integration)
Deprecation (breaking change to happen in the future)
Breaking change (fix/feature causing existing functionality to break)
Code quality improvements to existing code or addition of tests

Additional information

This PR fixes or closes issue: fixes Freebox : ClientConnectorError while setup when router is in bridge mode #50954
This PR is related to issue:
Link to documentation pull request:

Checklist

The code change is tested and works locally.
Local tests pass. Your PR cannot be merged unless tests pass
There is no commented out code in this PR.
I have followed the development checklist
I have followed the perfect PR recommendations
The code has been formatted using Black (black --fast homeassistant tests)
Tests have been added to verify that the new code works.

If user exposed functionality or configuration variables are added/changed:

Documentation added/updated for www.home-assistant.io

If the code communicates with devices, web services, or third-party tools:

The manifest file has all fields filled out correctly.
Updated and included derived files by running: python3 -m script.hassfest.
New or updated dependencies have been added to requirements_all.txt.
Updated by running python3 -m script.gen_requirements_all.
For the updated dependencies - a link to the changelog, or at minimum a diff between library versions is added to the PR description.
Untested files have been added to .coveragerc.

To help with the load of incoming pull requests:

I have reviewed two other open pull requests in this repository.

home-assistant

Hi @jflefebvre06

It seems you haven't yet signed a CLA. Please do so here.

Once you do that we will be able to review and accept this pull request.

Thanks!

home-assistant · 2023-11-02T08:27:35Z

Please take a look at the requested changes, and use the Ready for review button when you are done, thanks 👍

Learn more about our pull request process.

home-assistant · 2023-11-02T08:27:38Z

Hey there @hacf-fr, @Quentame, mind taking a look at this pull request as it has been labeled with an integration (freebox) you are listed as a code owner for? Thanks!

Code owner commands

Code owners of freebox can trigger bot actions by commenting:

@home-assistant close Closes the pull request.
@home-assistant rename Awesome new title Renames the pull request.
@home-assistant reopen Reopen the pull request.
@home-assistant unassign freebox Removes the current integration label and assignees on the pull request, add the integration domain after the command.

Stale

Quentame

Not linked to VMs access but hosts access.

Please update your comments and description accordingly.

homeassistant/components/freebox/router.py

OK thanks Co-authored-by: Quentame <polletquentin74@me.com>

jflefebvre06 · 2023-11-02T10:05:56Z

Comments updates

homeassistant/components/freebox/router.py

Quentame

You have to apply the same "try once and don't retry next" mechanism as supports_raid and home_granted do.

Otherwise, HA will try to update hosts everytime and will always get an error.
We better want to test if it succeed the first time to continue fetching next times.

Quentame

Just need to adjust the test, otherwise I'm good !

Quentame · 2023-11-18T10:07:46Z

tests/components/freebox/test_sensor.py

@@ -115,3 +115,21 @@ async def test_battery(
    assert hass.states.get("sensor.telecommande_niveau_de_batterie").state == "25"
    assert hass.states.get("sensor.ouverture_porte_niveau_de_batterie").state == "50"
    assert hass.states.get("sensor.detecteur_niveau_de_batterie").state == "75"
+
+
+async def test_get_hosts_list(


Not resolved.

Move this test to test_device_tracker.py

And you have to count number of devices linked to the router (--> router.devices):

1 if in bridge mode : the router itself

more than 1 if not

jflefebvre06 · 2023-11-18T12:19:09Z

@Quentame if I add assert router_bridge_mode.call_count == 1 test failed " AssertionError: assert 2 == 1"
I think it is bebause acced durirng mock_router_get_hosts_list_error_not_json

    router().lan.get_hosts_list = AsyncMock(
        side_effect=HttpRequestError(
            "Request failed (APIResponse: %s)"
            % json.dumps(DATA_LAN_GET_HOSTS_LIST_MODE_BRIDGE)
        )
    )

Quentame · 2023-11-18T12:32:19Z

@jflefebvre06 you did not understand my comment

Do not count the call to the router instance but to the amount of devices generated by the function call

The function call should normally be 1 at any test.

My Discord ID : quentame
We could talk if French there 😉

jflefebvre06 · 2023-11-18T13:31:10Z

Is there an example how to get the router ?

…test_device_tracker.py

Quentame

Test suggestions

tests/components/freebox/test_utils.py

tests/components/freebox/test_device_tracker.py

Quentame

Should use device tracker platform for tests

tests/components/freebox/test_device_tracker.py

Quentame

Nice, thanks 🥳

Comments addressed

Fix integration failed when freebox is configured in bridge mode

e1210f4

jflefebvre06 requested a review from Quentame as a code owner November 2, 2023 08:27

home-assistant bot previously requested changes Nov 2, 2023

View reviewed changes

home-assistant bot added bugfix cla-needed integration: freebox small-pr PRs with less than 30 lines. labels Nov 2, 2023

home-assistant bot marked this pull request as draft November 2, 2023 08:27

home-assistant bot assigned Quentame Nov 2, 2023

home-assistant bot added Quality Scale: No score cla-recheck cla-signed and removed cla-recheck cla-needed labels Nov 2, 2023

jflefebvre06 marked this pull request as ready for review November 2, 2023 08:34

Merge branch 'dev' into fix-freebox-bridge-mode

9cd3e5b

Quentame requested changes Nov 2, 2023

View reviewed changes

homeassistant/components/freebox/router.py Outdated Show resolved Hide resolved

homeassistant/components/freebox/router.py Outdated Show resolved Hide resolved

home-assistant bot marked this pull request as draft November 2, 2023 09:05

jflefebvre06 and others added 2 commits November 2, 2023 11:05

Update homeassistant/components/freebox/router.py

e84e624

OK thanks Co-authored-by: Quentame <polletquentin74@me.com>

Update homeassistant/components/freebox/router.py

ecd24ea

OK thanks Co-authored-by: Quentame <polletquentin74@me.com>

jflefebvre06 requested a review from Quentame November 2, 2023 10:08

jflefebvre06 marked this pull request as ready for review November 2, 2023 10:09

Merge branch 'dev' into fix-freebox-bridge-mode

745f935

Quentame reviewed Nov 2, 2023

View reviewed changes

homeassistant/components/freebox/router.py Outdated Show resolved Hide resolved

Quentame reviewed Nov 2, 2023

View reviewed changes

homeassistant/components/freebox/router.py Outdated Show resolved Hide resolved

Quentame requested changes Nov 2, 2023

View reviewed changes

home-assistant bot requested a review from Quentame November 17, 2023 17:48

Quentame requested changes Nov 18, 2023

View reviewed changes

home-assistant bot marked this pull request as draft November 18, 2023 10:08

Merge branch 'dev' into fix-freebox-bridge-mode

e9c1b48

jflefebvre06 and others added 3 commits November 18, 2023 15:16

Merge branch 'dev' into fix-freebox-bridge-mode

5d6bd03

Add method is_json to clean the code and move test_get_hosts_list to …

e45b494

…test_device_tracker.py

is_json parameter only string

0329b5c

Quentame requested changes Nov 18, 2023

View reviewed changes

tests/components/freebox/test_device_tracker.py Outdated Show resolved Hide resolved

tests/components/freebox/test_device_tracker.py Outdated Show resolved Hide resolved

tests/components/freebox/test_device_tracker.py Outdated Show resolved Hide resolved

jflefebvre06 added 2 commits November 18, 2023 21:11

Improve router code in bridge maod and add test test_router_mode method

13bfdef

Improve router code in bridge maod and add test test_router_mode method

2c372b3

Quentame requested changes Nov 18, 2023

View reviewed changes

tests/components/freebox/test_device_tracker.py Show resolved Hide resolved

Merge branch 'dev' into fix-freebox-bridge-mode

c7dca36

Quentame requested changes Nov 18, 2023

View reviewed changes

tests/components/freebox/test_device_tracker.py Show resolved Hide resolved

Change comment

aaf015b

jflefebvre06 marked this pull request as ready for review November 18, 2023 22:12

home-assistant bot requested a review from Quentame November 18, 2023 22:12

jflefebvre06 and others added 3 commits November 18, 2023 22:16

Format

4148c32

Merge branch 'dev' into fix-freebox-bridge-mode

e280b8c

Update test_device_tracker.py

953235e

Quentame reviewed Nov 18, 2023

View reviewed changes

tests/components/freebox/test_device_tracker.py Show resolved Hide resolved

Quentame and others added 2 commits November 18, 2023 23:38

Update test_device_tracker.py

50cb008

Untested files have been added to .coveragerc

4c67907

Quentame approved these changes Nov 18, 2023

View reviewed changes

Quentame merged commit 9c86adf into home-assistant:dev Nov 18, 2023
22 of 23 checks passed

github-actions bot locked and limited conversation to collaborators Nov 20, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix integration failed when freebox is configured in bridge mode #103221

Fix integration failed when freebox is configured in bridge mode #103221

jflefebvre06 commented Nov 2, 2023 •

edited

home-assistant bot left a comment

home-assistant bot commented Nov 2, 2023

home-assistant bot commented Nov 2, 2023

Quentame left a comment

jflefebvre06 commented Nov 2, 2023

Quentame left a comment •

edited

Quentame left a comment

Quentame Nov 18, 2023

jflefebvre06 commented Nov 18, 2023 •

edited

Quentame commented Nov 18, 2023

jflefebvre06 commented Nov 18, 2023

Quentame left a comment

Quentame left a comment

Quentame left a comment

Fix integration failed when freebox is configured in bridge mode #103221

Fix integration failed when freebox is configured in bridge mode #103221

Conversation

jflefebvre06 commented Nov 2, 2023 • edited

Breaking change

Proposed change

Type of change

Additional information

Checklist

home-assistant bot left a comment

Choose a reason for hiding this comment

home-assistant bot commented Nov 2, 2023

home-assistant bot commented Nov 2, 2023

Quentame left a comment

Choose a reason for hiding this comment

jflefebvre06 commented Nov 2, 2023

Quentame left a comment • edited

Choose a reason for hiding this comment

Quentame left a comment

Choose a reason for hiding this comment

Quentame Nov 18, 2023

Choose a reason for hiding this comment

jflefebvre06 commented Nov 18, 2023 • edited

Quentame commented Nov 18, 2023

jflefebvre06 commented Nov 18, 2023

Quentame left a comment

Choose a reason for hiding this comment

Quentame left a comment

Choose a reason for hiding this comment

Quentame left a comment

Choose a reason for hiding this comment

jflefebvre06 commented Nov 2, 2023 •

edited

Quentame left a comment •

edited

jflefebvre06 commented Nov 18, 2023 •

edited