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

TelldusLive config flow #18758

Merged
merged 11 commits into from Dec 10, 2018

Conversation

@fredrike
Copy link
Contributor

fredrike commented Nov 28, 2018

Description:

Add Config Flow to the TelldusLive component

Pull request in home-assistant.io with documentation (if applicable): home-assistant/home-assistant.io#<home-assistant.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
  • There is no commented out code in this PR.

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

If the code communicates with devices, web services, or third-party tools:

  • New dependencies have been added to the REQUIREMENTS variable (example).
  • New dependencies are only imported inside functions that use them (example).
  • New or updated dependencies have been added to requirements_all.txt by running script/gen_requirements_all.py.
  • New files were added to .coveragerc.

If the code does not interact with devices:

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

TODO:

  • Read hub device status from TelldusLive (update tellduslive-lib)
@MartinHjelmare
Copy link
Member

MartinHjelmare left a comment

Please make the refactor in a separate PR first, to move entities from a shared container to use our dispatch helper instead for communication between component and platforms. Also move the component to a package.

Then when that is done, we can return to this PR and rebase it.

Show resolved Hide resolved homeassistant/components/tellduslive/__init__.py Outdated

@fredrike fredrike force-pushed the fredrike:tellduslive-config_flow branch from d3e531a to 0a2562a Nov 29, 2018

@fredrike fredrike force-pushed the fredrike:tellduslive-config_flow branch 2 times, most recently from abe46b1 to ea0fdc1 Dec 4, 2018

@fredrike

This comment has been minimized.

Copy link
Contributor

fredrike commented Dec 4, 2018

@MartinHjelmare I've rebased against dev now, good idea as this pr is much cleaner.

Show resolved Hide resolved homeassistant/components/tellduslive/__init__.py
Show resolved Hide resolved homeassistant/components/tellduslive/__init__.py Outdated
Show resolved Hide resolved homeassistant/components/tellduslive/__init__.py Outdated
Show resolved Hide resolved homeassistant/components/tellduslive/__init__.py Outdated
Show resolved Hide resolved homeassistant/components/tellduslive/__init__.py Outdated
Show resolved Hide resolved homeassistant/components/tellduslive/config_flow.py Outdated
Show resolved Hide resolved homeassistant/components/tellduslive/config_flow.py Outdated
Show resolved Hide resolved homeassistant/components/tellduslive/config_flow.py Outdated
Show resolved Hide resolved homeassistant/components/tellduslive/config_flow.py Outdated
Show resolved Hide resolved homeassistant/components/tellduslive/config_flow.py

@fredrike fredrike force-pushed the fredrike:tellduslive-config_flow branch from 26960bd to 0dc8a11 Dec 5, 2018

@fredrike fredrike force-pushed the fredrike:tellduslive-config_flow branch from a8e1dce to e1b1ca1 Dec 5, 2018

fredrike and others added some commits Dec 4, 2018

Update homeassistant/components/tellduslive/config_flow.py
Co-Authored-By: fredrike <fredrik.e@gmail.com>
@MartinHjelmare
Copy link
Member

MartinHjelmare left a comment

Unload entry in the component package is missing. Almost done.

@fredrike fredrike force-pushed the fredrike:tellduslive-config_flow branch from b94b449 to d9fb053 Dec 9, 2018

@fredrike

This comment has been minimized.

Copy link
Contributor

fredrike commented Dec 9, 2018

I've added unload entry, not sure if I'm stopping the listener correctly though.

@fredrike fredrike force-pushed the fredrike:tellduslive-config_flow branch from 9a19eb5 to 9a574d1 Dec 9, 2018

"""Discover the component."""
device = self._client.device(device_id)
component = self.identify_device(device)
config_entries_key = '{}.{}'.format(component, DOMAIN)

This comment has been minimized.

@MartinHjelmare

MartinHjelmare Dec 9, 2018

Member

This is what's in hass.data at CONFIG_ENTRY_IS_SETUP. We can't use that directly when unloading the entries.

This comment has been minimized.

@fredrike

fredrike Dec 9, 2018

Contributor

It would make more sense to just store the component in config_entries_key..

This comment has been minimized.

@fredrike

fredrike Dec 9, 2018

Contributor

Or what do you think?

This comment has been minimized.

@MartinHjelmare

MartinHjelmare Dec 9, 2018

Member

Yeah, I guess that would be enough. 👍

@fredrike fredrike force-pushed the fredrike:tellduslive-config_flow branch from f2ae6c7 to ea86d0f Dec 9, 2018

@MartinHjelmare

This comment has been minimized.

Copy link
Member

MartinHjelmare commented Dec 10, 2018

There are still some parts of the flow that are missing test coverage:

Test session starts (platform: linux, Python 3.6.5, pytest 4.0.0, pytest-sugar 0.9.2)
rootdir: /home/martin/Dev/home-assistant, inifile: setup.cfg
plugins: requests-mock-1.5.2, timeout-1.3.2, sugar-0.9.2, cov-2.6.0, aiohttp-0.3.0
collecting ... 
 tests/components/tellduslive/test_config_flow.py ✓✓✓✓✓✓✓✓✓                                                                                                                                    100% ██████████

----------- coverage: platform linux, python 3.6.5-final-0 -----------
Name                                                  Stmts   Miss  Cover   Missing
-----------------------------------------------------------------------------------
homeassistant/components/tellduslive/config_flow.py      75     10    87%   38, 112-113, 130, 136-144


Results (0.51s):
       9 passed
@fredrike

This comment has been minimized.

Copy link
Contributor

fredrike commented Dec 10, 2018

How do I test this (L38)?

    def _get_auth_url(self):
        return self._session.authorize_url
@MartinHjelmare

This comment has been minimized.

Copy link
Member

MartinHjelmare commented Dec 10, 2018

Can we patch authorize_url on the session?

@fredrike

This comment has been minimized.

Copy link
Contributor

fredrike commented Dec 10, 2018

def test_auth_url(hass, mock_tellduslive):
    """Test _get_auth_url."""
    flow = config_flow.FlowHandler()
    with patch.object(flow, '_session') as session:
        session.return_value.authorize_url='url'
        result = flow._get_auth_url
    assert result == 'url'

Gives: AssertionError: assert <bound method FlowHandler._get_auth_url of <homeassistant.components.tellduslive.config_flow.FlowHandler object at 0x10c5c8d68>> == 'url'

Edit: found it!

def test_auth_url(hass, mock_tellduslive):
    """Test _get_auth_url."""
    flow = config_flow.FlowHandler()
    with patch.object(flow, '_session') as session:
        session.authorize_url = 'url'
        result = flow._get_auth_url()
    assert result == 'url'
@fredrike

This comment has been minimized.

Copy link
Contributor

fredrike commented Dec 10, 2018

pytest --cov=homeassistant/components/tellduslive/ --cov-report term-missing tests/components/tellduslive/test_config_flow.py
Test session starts (platform: darwin, Python 3.7.0, pytest 4.0.1, pytest-sugar 0.9.2)
rootdir: /Users/fer/Documents/github/home-assistant, inifile: setup.cfg
plugins: requests-mock-1.5.2, timeout-1.3.3, sugar-0.9.2, cov-2.6.0, aiohttp-0.3.0, pylama-7.6.5
collecting ...
 tests/components/tellduslive/test_config_flow.py ✓✓✓✓✓✓✓✓✓✓✓✓✓✓✓                                                                                                                                                                                 100% ██████████

---------- coverage: platform darwin, python 3.7.0-final-0 -----------
Name                                                  Stmts   Miss  Cover   Missing
-----------------------------------------------------------------------------------
homeassistant/components/tellduslive/config_flow.py      76      0   100%
homeassistant/components/tellduslive/const.py            20      0   100%
-----------------------------------------------------------------------------------
TOTAL                                                    96      0   100%


Results (0.74s):
      15 passed
@MartinHjelmare
Copy link
Member

MartinHjelmare left a comment

Looks great!

@MartinHjelmare MartinHjelmare merged commit 92e19f6 into home-assistant:dev Dec 10, 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 increased (+0.06%) to 92.989%
Details

@wafflebot wafflebot bot removed the in progress label Dec 10, 2018

@fredrike

This comment has been minimized.

Copy link
Contributor

fredrike commented Dec 10, 2018

Thank you @MartinHjelmare for keeping the high standards!

mxworm added a commit to mxworm/home-assistant that referenced this pull request Dec 11, 2018

Merge branch 'dev' into current
* dev: (26 commits)
  Move daikin to package (home-assistant#19187)
  Restore states for RFLink devices (home-assistant#18816)
  Add SCAN_INTERVAL (home-assistant#19186)
  Enable alarmdecoder to see open/close state of bypassed RF zones when armed (home-assistant#18477)
  Updated frontend to 20181211.0
  Fix cloud defaults (home-assistant#19172)
  TelldusLive config flow (home-assistant#18758)
  ZHA - Event foundation (home-assistant#19095)
  Add raw service data to event (home-assistant#19163)
  Updated frontend to 20181210.1
  Google assistant fix target temp for *F values. (home-assistant#19083)
  Fix lovelace save (home-assistant#19162)
  Drop OwnTracks bad packets (home-assistant#19161)
  Update translations
  Updated frontend to 20181210.0
  Lovelace using storage (home-assistant#19101)
  Update pygtfs to upstream's 0.1.5 (home-assistant#19151)
  Update radiotherm to 2.0.0 and handle change in tstat error detection (home-assistant#19107)
  Upgrade sphinx-autodoc-typehints to 1.5.2 (home-assistant#19140)
  Update geizhals dependency (home-assistant#19152)
  ...

@balloob balloob removed the new-platform label Jan 10, 2019

@balloob balloob referenced this pull request Jan 10, 2019

Merged

0.85.0 #19897

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