Fix timeout issue on Roomba integration when adding a new device #129230

AuroreVgn · 2024-10-26T16:40:10Z

Proposed change

Fix timeout issue on Roomba integration when adding a new device

const.py: DEFAULT_DELAY = 100 instead of DEFAULT_DELAY = 1
config_flow.py: continuous=True instead of continuous=False

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 [roomba] fails to set up #117071 Roomba 980 #117039 Roomba j7 still not integrating after 2024.3.0 #112544
This PR is related to issue: [roomba] fails to set up #117071 Roomba 980 #117039 Roomba j7 still not integrating after 2024.3.0 #112544

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 Ruff (ruff format 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.

To help with the load of incoming pull requests:

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

DEFAULT_DELAY = 1 to DEFAULT_DELAY = 100 to fix timeout when adding a new device

continuous=False to continuous=True to fix timeout when adding a new device

home-assistant

Hi @AuroreVgn

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 · 2024-10-26T16:40:14Z

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 · 2024-10-26T16:40:18Z

Hey there @pschmitt, @cyr-ius, @shenxn, @Orhideous, mind taking a look at this pull request as it has been labeled with an integration (roomba) you are listed as a code owner for? Thanks!

Code owner commands

Code owners of roomba 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 roomba Removes the current integration label and assignees on the pull request, add the integration domain after the command.
@home-assistant add-label needs-more-information Add a label (needs-more-information, problem in dependency, problem in custom component) to the pull request.
@home-assistant remove-label needs-more-information Remove a label (needs-more-information, problem in dependency, problem in custom component) on the pull request.

homeassistant/components/roomba/const.py

Co-authored-by: Jan Bouwhuis <[email protected]>

jbouwh · 2024-10-27T18:58:22Z

At a few spots in tests/components/roomba/test_config_flow.py:

core/tests/components/roomba/test_config_flow.py

Line 209 in 2888e57

CONF_DELAY: 1,

core/tests/components/roomba/test_config_flow.py

Line 334 in 2888e57

CONF_DELAY: 1,

core/tests/components/roomba/test_config_flow.py

Line 471 in 2888e57

CONF_DELAY: 1,

core/tests/components/roomba/test_config_flow.py

Line 544 in 2888e57

CONF_DELAY: 1,

core/tests/components/roomba/test_config_flow.py

Line 680 in 2888e57

CONF_DELAY: 1,

core/tests/components/roomba/test_config_flow.py

Line 741 in 2888e57

CONF_DELAY: 1,

core/tests/components/roomba/test_config_flow.py

Line 819 in 2888e57

CONF_DELAY: 1,

core/tests/components/roomba/test_config_flow.py

Line 889 in 2888e57

CONF_DELAY: 1,

core/tests/components/roomba/test_config_flow.py

Line 1122 in 2888e57

user_input={CONF_CONTINUOUS: True, CONF_DELAY: 1},

and

core/tests/components/roomba/test_config_flow.py

Lines 1127 to 1128 in 2888e57

    
           assert result["data"] == {CONF_CONTINUOUS: True, CONF_DELAY: 1} 
        
           assert config_entry.options == {CONF_CONTINUOUS: True, CONF_DELAY: 1}

the CONF_DELAY value is asserted. This seems to have a relation with the changed const.

I suggest you import changed the DEFAULT_DELAY constant into test_config_flow.py and use this instead of the literal.

I guess this should fix the failing checks.

Change CONF_DELAY to match DEFAULT_DELAY (30 sec instead of 1)

AuroreVgn · 2024-10-28T08:04:41Z

At a few spots in tests/components/roomba/test_config_flow.py:

core/tests/components/roomba/test_config_flow.py

Line 209 in 2888e57

CONF_DELAY: 1,

core/tests/components/roomba/test_config_flow.py

Line 334 in 2888e57

CONF_DELAY: 1,

core/tests/components/roomba/test_config_flow.py

Line 471 in 2888e57

CONF_DELAY: 1,

core/tests/components/roomba/test_config_flow.py

Line 544 in 2888e57

CONF_DELAY: 1,

core/tests/components/roomba/test_config_flow.py

Line 680 in 2888e57

CONF_DELAY: 1,

core/tests/components/roomba/test_config_flow.py

Line 741 in 2888e57

CONF_DELAY: 1,

core/tests/components/roomba/test_config_flow.py

Line 819 in 2888e57

CONF_DELAY: 1,

core/tests/components/roomba/test_config_flow.py

Line 889 in 2888e57

CONF_DELAY: 1,

core/tests/components/roomba/test_config_flow.py

Line 1122 in 2888e57

user_input={CONF_CONTINUOUS: True, CONF_DELAY: 1},

and

core/tests/components/roomba/test_config_flow.py

Lines 1127 to 1128 in 2888e57

assert result["data"] == {CONF_CONTINUOUS: True, CONF_DELAY: 1}

assert config_entry.options == {CONF_CONTINUOUS: True, CONF_DELAY: 1}

the CONF_DELAY value is asserted. This seems to have a relation with the changed const.

I suggest you import changed the DEFAULT_DELAY constant into test_config_flow.py and use this instead of the literal.

I guess this should fix the failing checks.

Done !

jbouwh · 2024-10-28T17:52:18Z

Thanks! Mark your PR ready if you are finished.

tests/components/roomba/test_config_flow.py

Co-authored-by: Jan Bouwhuis <[email protected]>

jbouwh · 2024-10-28T19:42:16Z

tests/components/roomba/test_config_flow.py

@@ -206,7 +206,7 @@ async def test_form_user_discovery_and_password_fetch(hass: HomeAssistant) -> No
    assert result3["data"] == {
        CONF_BLID: "BLID",
        CONF_CONTINUOUS: True,
-        CONF_DELAY: 1,
+        CONF_DELAY: DEFAULT_DELAY,


You also need to update the other occurrences and addd the import.

Do a git pull to make sure you include the applied suggestions.

I am not familiar to this process, could you please tell me how to do a git pull ?

Like you used git clone to clone a he HA core repository. But let me help to commit the changes for you.

You can commit for me, do I need do to something ?

AuroreVgn

continuous=False
DEFAULT_DELAY = 30
CONF_DELAY: 30,

Stale

jbouwh

Nice first contribution!
Thanks @AuroreVgn 👍

AuroreVgn · 2024-10-31T09:19:50Z

Nice first contribution! Thanks @AuroreVgn 👍

Thanks for your help :-)

…e-assistant#129230) * Update const.py DEFAULT_DELAY = 1 to DEFAULT_DELAY = 100 to fix timeout when adding a new device * Update config_flow.py continuous=False to continuous=True to fix timeout when adding a new device * Update homeassistant/components/roomba/const.py Co-authored-by: Jan Bouwhuis <[email protected]> * Update test_config_flow.py Change CONF_DELAY to match DEFAULT_DELAY (30 sec instead of 1) * Update tests/components/roomba/test_config_flow.py Co-authored-by: Jan Bouwhuis <[email protected]> * Use constant for DEFAULT_DELAY in tests --------- Co-authored-by: Jan Bouwhuis <[email protected]> Co-authored-by: jbouwh <[email protected]>

…9230) * Update const.py DEFAULT_DELAY = 1 to DEFAULT_DELAY = 100 to fix timeout when adding a new device * Update config_flow.py continuous=False to continuous=True to fix timeout when adding a new device * Update homeassistant/components/roomba/const.py Co-authored-by: Jan Bouwhuis <[email protected]> * Update test_config_flow.py Change CONF_DELAY to match DEFAULT_DELAY (30 sec instead of 1) * Update tests/components/roomba/test_config_flow.py Co-authored-by: Jan Bouwhuis <[email protected]> * Use constant for DEFAULT_DELAY in tests --------- Co-authored-by: Jan Bouwhuis <[email protected]> Co-authored-by: jbouwh <[email protected]>

AuroreVgn added 2 commits October 26, 2024 18:29

Update const.py

52881f8

DEFAULT_DELAY = 1 to DEFAULT_DELAY = 100 to fix timeout when adding a new device

Update config_flow.py

b8f097f

continuous=False to continuous=True to fix timeout when adding a new device

home-assistant bot added bugfix cla-needed Hacktoberfest integration: roomba small-pr PRs with less than 30 lines. labels Oct 26, 2024

home-assistant bot previously requested changes Oct 26, 2024

View reviewed changes

home-assistant bot marked this pull request as draft October 26, 2024 16:40

home-assistant bot added the Quality Scale: No score label Oct 26, 2024

home-assistant bot added cla-recheck cla-signed and removed cla-recheck cla-needed labels Oct 26, 2024

jbouwh reviewed Oct 26, 2024

View reviewed changes

homeassistant/components/roomba/const.py Outdated Show resolved Hide resolved

jbouwh reviewed Oct 26, 2024

View reviewed changes

homeassistant/components/roomba/const.py Outdated Show resolved Hide resolved

Update homeassistant/components/roomba/const.py

55e0b72

Co-authored-by: Jan Bouwhuis <[email protected]>

Update test_config_flow.py

6cac804

Change CONF_DELAY to match DEFAULT_DELAY (30 sec instead of 1)

jbouwh reviewed Oct 28, 2024

View reviewed changes

tests/components/roomba/test_config_flow.py Outdated Show resolved Hide resolved

jbouwh added this to the 2024.11.0b0 milestone Oct 28, 2024

Update tests/components/roomba/test_config_flow.py

ab082cc

Co-authored-by: Jan Bouwhuis <[email protected]>

jbouwh reviewed Oct 28, 2024

View reviewed changes

AuroreVgn commented Oct 29, 2024

View reviewed changes

Use constant for DEFAULT_DELAY in tests

57efd05

jbouwh marked this pull request as ready for review October 30, 2024 17:38

edenhaus modified the milestones: 2024.11.0b0, 2024.11.0 Oct 30, 2024

jbouwh approved these changes Oct 30, 2024

View reviewed changes

jbouwh merged commit 94f906b into home-assistant:dev Oct 30, 2024
28 checks passed

bramkragten added the cherry-picked label Oct 31, 2024

github-actions bot locked and limited conversation to collaborators Nov 1, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix timeout issue on Roomba integration when adding a new device #129230

Fix timeout issue on Roomba integration when adding a new device #129230

AuroreVgn commented Oct 26, 2024

home-assistant bot left a comment

home-assistant bot commented Oct 26, 2024

home-assistant bot commented Oct 26, 2024

jbouwh commented Oct 27, 2024

AuroreVgn commented Oct 28, 2024

jbouwh commented Oct 28, 2024

jbouwh Oct 28, 2024

AuroreVgn Oct 29, 2024

jbouwh Oct 29, 2024

AuroreVgn Oct 29, 2024

AuroreVgn left a comment

jbouwh left a comment

AuroreVgn commented Oct 31, 2024

Fix timeout issue on Roomba integration when adding a new device #129230

Fix timeout issue on Roomba integration when adding a new device #129230

Conversation

AuroreVgn commented Oct 26, 2024

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 Oct 26, 2024

home-assistant bot commented Oct 26, 2024

jbouwh commented Oct 27, 2024

AuroreVgn commented Oct 28, 2024

jbouwh commented Oct 28, 2024

jbouwh Oct 28, 2024

Choose a reason for hiding this comment

AuroreVgn Oct 29, 2024

Choose a reason for hiding this comment

jbouwh Oct 29, 2024

Choose a reason for hiding this comment

AuroreVgn Oct 29, 2024

Choose a reason for hiding this comment

AuroreVgn left a comment

Choose a reason for hiding this comment

jbouwh left a comment

Choose a reason for hiding this comment

AuroreVgn commented Oct 31, 2024