Skip to content

Add support for capturing renewals to dhcp discovery#48242

Merged
MartinHjelmare merged 2 commits into
home-assistant:devfrom
bdraco:dhcp_capture_renewals
Mar 30, 2021
Merged

Add support for capturing renewals to dhcp discovery#48242
MartinHjelmare merged 2 commits into
home-assistant:devfrom
bdraco:dhcp_capture_renewals

Conversation

@bdraco
Copy link
Copy Markdown
Member

@bdraco bdraco commented Mar 23, 2021

Proposed change

Add support for capturing renewals to dhcp discovery

This should speed things up quite a bit for more users as we don't have to wait for a power off/power on event.

Explains why #47696 could never see the roomba

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)
  • Breaking change (fix/feature causing existing functionality to break)
  • Code quality improvements to existing code or addition of tests

Example entry for configuration.yaml:

# Example configuration.yaml

Additional information

  • This PR fixes or closes issue: fixes #
  • 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
  • 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:

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.
  • Untested files have been added to .coveragerc.

The integration reached or maintains the following Integration Quality Scale:

  • No score or internal
  • 🥈 Silver
  • 🥇 Gold
  • 🏆 Platinum

To help with the load of incoming pull requests:

@bdraco
Copy link
Copy Markdown
Member Author

bdraco commented Mar 23, 2021

@scyto if you checkout this PR and run it, does it now find your roomba?

@scyto

This comment has been minimized.

@bdraco
Copy link
Copy Markdown
Member Author

bdraco commented Mar 23, 2021

My next step will be to repull my patch-1 branch finish the code changes (like running hass fest etc) and then reset my roombas networking).

Anything else i should do?

Sounds good to me. This changed solved the renewal case for one of my networks so it looks like it will improve the status quo even if it doesn't solve the case you are seeing.

Thanks for testing

@scyto
Copy link
Copy Markdown
Contributor

scyto commented Mar 23, 2021

Improvement is always good! I hate it at work when we say 'well that only solves an edge case so lets not do it' or worse 'we can't solved all edge cases so lets not do it' :-)

I know how to generate a broadcast packet on demand from both i7 and 980 models.
Basically hold the clean button for 20 seconds and release, i can do a docs change to clarify.
Did get one issue with roombapy - i will document that on the other thread.

I do need one tip. if you have the time.

This time rather than do anything with vscode on windows I cloned my fork to my debian box, checked out my branch. Setup the python virtual env. Ran hassfest, tweaked files.

Now when i deactivate the python virtual env and then if I check in the branch am i going to mess up all the files? I never did the upstream command.

@bdraco

This comment has been minimized.

@scyto

This comment has been minimized.

@bdraco

This comment has been minimized.

@bdraco bdraco mentioned this pull request Mar 27, 2021
21 tasks
@bdraco bdraco force-pushed the dhcp_capture_renewals branch from faa5414 to 2b25eb5 Compare March 28, 2021 17:38
@bdraco
Copy link
Copy Markdown
Member Author

bdraco commented Mar 30, 2021

Nothing wrong here. Marked a few comments about tool usage off-topic since it would be a distraction to reviewers.

Copy link
Copy Markdown
Member

@MartinHjelmare MartinHjelmare left a comment

Choose a reason for hiding this comment

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

Looks good!

@MartinHjelmare MartinHjelmare merged commit f91de1c into home-assistant:dev Mar 30, 2021
@bdraco
Copy link
Copy Markdown
Member Author

bdraco commented Mar 30, 2021

Thank you 🙏🏻

@github-actions github-actions Bot locked and limited conversation to collaborators Mar 31, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants