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

Allow disabling WeMo Discovery #18079

Merged
merged 3 commits into from Nov 3, 2018

Conversation

Projects
None yet
5 participants
@sqldiablo
Contributor

sqldiablo commented Nov 1, 2018

Description:

Added a new optional configuration.yaml item for the WeMo platform called "disable_discovery", which allows a user to turn off the automatic discovery of WeMo devices.

Related issue (if applicable): fixes #16475

Pull request in home-assistant.io with documentation (if applicable): home-assistant/home-assistant.io#7314

Example entry for configuration.yaml (if applicable):

wemo:
  disable_disovery: true
  static:
    - 192.168.1.23
    - 192.168.52.172

Checklist:

  • The code change is tested and works locally.
  • Local tests pass with tox. Your PR cannot be merged unless tests pass

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

@wafflebot wafflebot bot added the in progress label Nov 1, 2018

@fabaff fabaff changed the title from WeMo - Disable Discovery - New config option to Allow disabling WeMo Discovery Nov 1, 2018

@sqldiablo

This comment has been minimized.

Contributor

sqldiablo commented Nov 2, 2018

@pvizeli, when you have a chance, could you give this another review, please?

@bachya bachya merged commit 5c99862 into home-assistant:dev Nov 3, 2018

5 checks passed

Hound No violations found. Woof!
WIP ready for review
Details
cla-bot Everyone involved has signed the CLA
continuous-integration/travis-ci/pr The Travis CI build passed
Details
coverage/coveralls Coverage decreased (-0.07%) to 93.072%
Details

@wafflebot wafflebot bot removed the in progress label Nov 3, 2018

@bachya

This comment has been minimized.

Contributor

bachya commented Nov 3, 2018

Thanks! 😎

@sqldiablo

This comment has been minimized.

Contributor

sqldiablo commented Nov 3, 2018

No problem, and thank you, @bachya & @pvizeli.

@sqldiablo sqldiablo deleted the sqldiablo:wemo_discovery branch Nov 3, 2018

@balloob balloob referenced this pull request Nov 9, 2018

Merged

0.82 #18335

@@ -58,12 +58,17 @@ def coerce_host_port(value):


CONF_STATIC = 'static'
CONF_DISABLE_DISCOVERY = 'disable_discovery'

DEFAULT_DISABLE_DISCOVERY = False

This comment has been minimized.

@balloob

balloob Nov 9, 2018

Member

So this is a breaking change and is also wrong. We always do automatic discovery. Going to change this for 0.82.

This comment has been minimized.

@balloob

balloob Nov 9, 2018

Member

Realized that it's a double negative and discovery is not disabled by default.

We should not have negative config options. It should have been an option called discovery which defaults to true.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment