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

Remove polling_interval_seconds option from wemo #95468

Merged
merged 1 commit into from Jun 28, 2023

Conversation

esev
Copy link
Contributor

@esev esev commented Jun 28, 2023

Proposed change

Remove the polling_interval_seconds option added in #56972 based on feedback after that PR was merged. Users can disable automatic polling and set up their own polling interval if needed.

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

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:

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:

@esev
Copy link
Contributor Author

esev commented Jun 28, 2023

Hi @marcelveldt please take a look when you get a chance.

Note: My availability is limited today. I'll try to respond to any feedback, but it's a travel day for me, so it may be several hours before I can respond.

@marcelveldt
Copy link
Member

Thanks for the very quick turnaround, appreciated!

I just remembered in your previous PR you mentioned you got some complaints about databases growing too fast due to too many state updates. A solid way to fix that could be to implement "significant_change" logic where you tell the HA recorder what data change is considered significant enough. Maybe also worth taking a look at ?

https://developers.home-assistant.io/docs/core/platform/significant_change

Copy link
Member

@marcelveldt marcelveldt left a comment

Choose a reason for hiding this comment

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

don't forget to update docs PR.

Thanks!

@esev
Copy link
Contributor Author

esev commented Jun 28, 2023

Thanks for the reminder. home-assistant/home-assistant.io#28001 is the docs update.

I just remembered in your previous PR you mentioned you got some complaints about databases growing too fast due to too many state updates. A solid way to fix that could be to implement "significant_change" logic where you tell the HA recorder what data change is considered significant enough. Maybe also worth taking a look at ?

Yes, good call. I'll send another PR to add significant_change logic. It's okay if this isn't in the 2023.7.0 release though, correct?

@marcelveldt
Copy link
Member

Yes, good call. I'll send another PR to add significant_change logic. It's okay if this isn't in the 2023.7.0 release though, correct?

Yes that is correct. It was just a tip after remembering you reported that issue.

@marcelveldt marcelveldt merged commit 03dac6e into home-assistant:dev Jun 28, 2023
23 checks passed
@marcelveldt marcelveldt added this to the 2023.7.0 milestone Jun 28, 2023
@balloob balloob modified the milestone: 2023.7.0 Jun 29, 2023
@balloob
Copy link
Member

balloob commented Jun 29, 2023

Clearing milestone as it was merged before beta cut.

@balloob balloob removed this from the 2023.7.0 milestone Jun 29, 2023
@github-actions github-actions bot locked and limited conversation to collaborators Jun 30, 2023
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.

None yet

3 participants