Skip to content

deCONZ config entry#13402

Merged
balloob merged 16 commits intohome-assistant:devfrom
Kane610:deconz_config_entry
Mar 30, 2018
Merged

deCONZ config entry#13402
balloob merged 16 commits intohome-assistant:devfrom
Kane610:deconz_config_entry

Conversation

@Kane610
Copy link
Copy Markdown
Member

@Kane610 Kane610 commented Mar 22, 2018

Description:

Basic config entry support for deCONZ component

Checklist:

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

errors = {}

if user_input is not None:
api_key = await async_get_api_key(self.hass.loop, **self.deconz_config)
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

line too long (83 > 79 characters)

return False


async def async_unload_entry(hass, entry):
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Remove this. We don't support unloading platforms yet and so you are not removing all the entities that this config entry has created.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Will do

@@ -186,3 +187,92 @@ async def async_configuration_callback(data):
entity_picture="/static/images/logo_deconz.jpeg",
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Please remove all the configurator code.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Is it ok that I do this when I've migrated the discovery part?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Sure

@balloob
Copy link
Copy Markdown
Member

balloob commented Mar 23, 2018

All config flows will need 100% test coverage.


async def async_setup_entry(hass, entry):
"""Set up a bridge for a config entry."""
result = await async_setup_deconz(hass, None, entry.data)
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

What if a user has a host configured and has a config entry for the same host?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Of course, I will make sure to check in both setups as well.

"""Handle a flow start."""
from pydeconz.utils import async_discovery

if DOMAIN in self.hass.data:
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

So you cannot create a config entry if you have one configured, but you will be able to configure a host after you have created a config entry?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

If there is an instance of deconz running, the config flow won't run. What do you mean by "but you will be able to configure a host after you have created a config entry"?

@Kane610
Copy link
Copy Markdown
Member Author

Kane610 commented Mar 23, 2018

Then I can start learning how to write tests 👍
@balloob 100% test coverage of the config flow or component?

@balloob
Copy link
Copy Markdown
Member

balloob commented Mar 23, 2018

100% of the config flow.

@balloob
Copy link
Copy Markdown
Member

balloob commented Mar 23, 2018

See #13034 for inspiration

@Kane610
Copy link
Copy Markdown
Member Author

Kane610 commented Mar 23, 2018

What is the quickest way to run only my tests?

@balloob
Copy link
Copy Markdown
Member

balloob commented Mar 23, 2018

pip3 install pytest
py.test test/components/test_deconz.py

result['data_schema']({'host': '1.2.3.4'})
result['data_schema']({'host': '5.6.7.8'})


Copy link
Copy Markdown

Choose a reason for hiding this comment

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

blank line at end of file

Comment thread tests/components/test_deconz.py Outdated

import homeassistant.components.deconz as deconz

from tests.common import MockConfigEntry, mock_coro
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

'tests.common.MockConfigEntry' imported but unused
'tests.common.mock_coro' imported but unused

Comment thread tests/components/test_deconz.py Outdated
@@ -0,0 +1,90 @@
"""Tests for deCONZ config flow."""
import asyncio
from unittest.mock import Mock, patch
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

'unittest.mock.Mock' imported but unused
'unittest.mock.patch' imported but unused

Comment thread tests/components/test_deconz.py Outdated
@@ -0,0 +1,90 @@
"""Tests for deCONZ config flow."""
import asyncio
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

'asyncio' imported but unused

@Kane610
Copy link
Copy Markdown
Member Author

Kane610 commented Mar 27, 2018

@balloob what do you think. Is this enough for a first iteration of configuration entries? I'm ready to start working on discovery and other parts when your PR has been merged 👍🏻


async def async_setup(hass, config):
"""Set up services and configuration for deCONZ component."""
if DOMAIN in hass.data:
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This will never happen. Home Assistant already takes care of this.

@balloob
Copy link
Copy Markdown
Member

balloob commented Mar 30, 2018

This looks good! Ok to merge after that one check has been removed.

@balloob balloob merged commit 931bcee into home-assistant:dev Mar 30, 2018
@balloob balloob mentioned this pull request Apr 13, 2018
@home-assistant home-assistant locked and limited conversation to collaborators Jul 26, 2018
@Kane610 Kane610 deleted the deconz_config_entry branch July 29, 2019 17:50
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.

5 participants