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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Config flow tradfri #16665

Merged
merged 6 commits into from
Sep 19, 2018
Merged

Config flow tradfri #16665

merged 6 commits into from
Sep 19, 2018

Conversation

balloob
Copy link
Member

@balloob balloob commented Sep 17, 2018

Description:

Add a config flow to IKEA Tradfri.

Imports old config files.
Works with discovery.

Will allow us to hook into the device registry soon 馃槑

WIP: still have to update the tests.

CC @lwis @ggravlingen

Related issue (if applicable): fixes #

Pull request in home-assistant.github.io with documentation (if applicable): home-assistant/home-assistant.github.io#<home-assistant.github.io PR number goes here>

Example entry for configuration.yaml (if applicable):

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:

If the code does not interact with devices:

  • Tests have been added to verify that the new code works.

@balloob balloob requested a review from a team as a code owner September 17, 2018 15:21
@ghost ghost assigned balloob Sep 17, 2018
@ghost ghost added the in progress label Sep 17, 2018
info[CONF_HOST] = host
info[CONF_IMPORT_GROUPS] = conf[CONF_ALLOW_TRADFRI_GROUPS]

hass.async_add_job(hass.config_entries.flow.async_init(
Copy link
Member

Choose a reason for hiding this comment

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

Use hass.async_create_task.

if conf is None:
return True

known_hosts = await hass.async_add_job(load_json,
Copy link
Member

Choose a reason for hiding this comment

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

Use hass.async_add_executor_job.

if host is None or host in known_hosts:
return True

hass.async_add_job(hass.config_entries.flow.async_init(
Copy link
Member

Choose a reason for hiding this comment

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

See above.

gateways = hass.data.setdefault(KEY_GATEWAY, {})
hass.data.setdefault(KEY_API, {})[entry.entry_id] = api

hass.data.setdefault(KEY_TRADFRI_GROUPS, {})
Copy link
Member

Choose a reason for hiding this comment

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

This doesn't seem to be used anymore.

hass.async_create_task(hass.config_entries.async_forward_entry_setup(
entry, 'light'
))
# hass.async_create_task(hass.config_entries.async_forward_entry_setup(
Copy link
Member

Choose a reason for hiding this comment

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

Remove comment before merge.

with async_timeout.timeout(5):
key = await api_factory.generate_psk(security_code)
except RequestError as err:
print(err)
Copy link
Member

Choose a reason for hiding this comment

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

Stale debug print.

return await self._entry_from_data(auth)

except AuthError as err:
errors['base'] = err.code
Copy link
Member

Choose a reason for hiding this comment

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

If it's the key that is wrong we could use a specific error key to mark the form for the user.

@pvizeli
Copy link
Member

pvizeli commented Sep 17, 2018

Can you remove the group feature? They don't work like it should. User need use the light group or they will be not happy

@awarecan
Copy link
Contributor

Should remove all translation json modification from PR


assert len(mock_gateway_info.mock_calls) == 1
assert len(mock_entry_setup.mock_calls) == 1

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

@@ -10,6 +10,8 @@
from homeassistant.components import tradfri
from homeassistant.setup import async_setup_component

from tests.common import MockConfigEntry, mock_coro

Choose a reason for hiding this comment

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

'tests.common.mock_coro' imported but unused


@pytest.fixture
def mock_auth():
"""Mock get_gateway_info."""
Copy link
Member

Choose a reason for hiding this comment

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

Stale docstring.



async def test_import_connection(hass, mock_gateway_info, mock_entry_setup):
"""Test a connection via discovery."""
Copy link
Member

Choose a reason for hiding this comment

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

Stale docstring.

return await self.async_step_auth()

async def async_step_auth(self, user_input=None):
"""Handle a flow initialized by the user."""
Copy link
Member

Choose a reason for hiding this comment

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

Stale docstring.

@balloob balloob merged commit a1c524d into dev Sep 19, 2018
@balloob balloob deleted the config-flow-tradfri branch September 19, 2018 19:21
@ghost ghost removed the in progress label Sep 19, 2018
@balloob balloob mentioned this pull request Sep 28, 2018
@home-assistant home-assistant locked and limited conversation to collaborators Feb 5, 2019
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

6 participants