Skip to content

Add Config Flow support, Device Registry support, available property to vizio component#30653

Merged
MartinHjelmare merged 34 commits into
home-assistant:devfrom
raman325:vizio_config_flow
Jan 15, 2020
Merged

Add Config Flow support, Device Registry support, available property to vizio component#30653
MartinHjelmare merged 34 commits into
home-assistant:devfrom
raman325:vizio_config_flow

Conversation

@raman325
Copy link
Copy Markdown
Contributor

@raman325 raman325 commented Jan 10, 2020

Breaking Change:

The vizio integration has moved from a platform level config entry to a top level config entry.
the name parameter is an optional parameter. Before config flow, if you didn't provide a name, a default name was set and then HA would auto assign entity ID's based on the name, eg vizio_smartcast and vizio_smartcast_2. Once config flow is available, the names have to be unique otherwise import will fail

Description:

  • Added user and import based Config Flow support (with tests)
  • Added Device Registry support
  • available property to handle disconnects

NOTE: I also removed a bunch of line breaks I added in a recent PR because pydocstyle didn't like it

Pull request in home-assistant.io: home-assistant/home-assistant.io#11704

Example entry for configuration.yaml (if applicable):

vizio:
  - host: IP_ADDRESS
    access_token: AUTH_TOKEN

Previously:

media_player:
  - platform: vizio
    host: IP_ADDRESS
    access_token: AUTH_TOKEN

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.
  • I have followed the development checklist

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. Update and include derived files by running python3 -m script.hassfest.
  • New or updated dependencies have been added to requirements_all.txt by running python3 -m script.gen_requirements_all.
  • Untested files have been added to .coveragerc.

If the code does not interact with devices:

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

Comment thread homeassistant/components/vizio/__init__.py Outdated
Comment thread homeassistant/components/vizio/__init__.py Outdated
Comment thread homeassistant/components/vizio/__init__.py Outdated
Comment thread homeassistant/components/vizio/config_flow.py Outdated
Comment thread homeassistant/components/vizio/config_flow.py Outdated
Comment thread homeassistant/components/vizio/media_player.py Outdated
Comment thread tests/components/vizio/test_config_flow.py Outdated
Comment thread tests/components/vizio/test_config_flow.py Outdated
Comment thread tests/components/vizio/test_config_flow.py Outdated
Comment thread tests/components/vizio/test_config_flow.py Outdated
@raman325
Copy link
Copy Markdown
Contributor Author

raman325 commented Jan 12, 2020

OK so I removed the OptionsConfigFlow for now and moved CONF_VOLUME_STEP back into the main ConfigFlow because I think I may need to make changes in the base component to support it properly as an option. I also kept the async_set_unique_id check in async_step_import so that I could abort if a flow was already in progress.

@MartinHjelmare
Copy link
Copy Markdown
Member

CONF_VOLUME_STEP should not be part of the main flow. Save it separately and merge it into entry data before creating the entry if imported from config yaml.

Copy link
Copy Markdown
Member

@MartinHjelmare MartinHjelmare left a comment

Choose a reason for hiding this comment

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

See eg how hue config flow does its checks. It uses the bridge id for unique_id but also uses host for checking same entry in the import step.

Comment thread homeassistant/components/vizio/config_flow.py Outdated
Comment thread homeassistant/components/vizio/config_flow.py Outdated
…ates, set unique ID to serial, fixes based on review
@raman325
Copy link
Copy Markdown
Contributor Author

figured out how to make options flow work so added that back in along with addressing your comments

… patches to pytest fixtures and fix patch scoping
@raman325
Copy link
Copy Markdown
Contributor Author

Thanks for all of the help and patience @MartinHjelmare , I'm learning a lot through this and hopefully this last push does it!

@raman325
Copy link
Copy Markdown
Contributor Author

raman325 commented Jan 14, 2020

I would like to submit a PR for a breaking change that changes the soundbar device type to speaker because speaker is defined in the media_player component and allows the device to be controlled by Alexa and Google Assistant. Should I submit that as a separate change or can I include that here? I'm trying to avoid introducing breaking changes in multiple releases, and it is ready to go.

@raman325
Copy link
Copy Markdown
Contributor Author

raman325 commented Jan 14, 2020

Also, I would request that this PR, as well as the changes to use speaker as a device class instead of soundbar, get released along with #30536 and #30605 so that all of the breaking changes happen in one release, so that may mean holding those two merged PRs up if this won't make 0.104. The device class change is the last anticipated breaking change.

Comment thread homeassistant/components/vizio/media_player.py Outdated
Comment thread homeassistant/components/vizio/media_player.py Outdated
Comment thread homeassistant/components/vizio/media_player.py Outdated
Comment thread homeassistant/components/vizio/media_player.py Outdated
Comment thread homeassistant/components/vizio/media_player.py
Comment thread homeassistant/components/vizio/media_player.py Outdated
Comment thread homeassistant/components/vizio/media_player.py Outdated
Comment thread homeassistant/components/vizio/media_player.py
Comment thread tests/components/vizio/test_config_flow.py Outdated
Comment thread tests/components/vizio/test_config_flow.py Outdated
@MartinHjelmare
Copy link
Copy Markdown
Member

It's just the PR that removed the warning suppression that is in the release branch. I think that's ok. The rest of the PRs will go out in the following release 0.105.

Comment thread homeassistant/components/vizio/media_player.py Outdated
Comment thread homeassistant/components/vizio/media_player.py Outdated
Comment thread homeassistant/components/vizio/media_player.py Outdated
Comment thread homeassistant/components/vizio/media_player.py Outdated
Comment thread homeassistant/components/vizio/media_player.py Outdated
Copy link
Copy Markdown
Member

@MartinHjelmare MartinHjelmare left a comment

Choose a reason for hiding this comment

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

Looks good! Let me know if you want to do the final improvement or save that for later.

Comment thread homeassistant/components/vizio/config_flow.py
@raman325
Copy link
Copy Markdown
Contributor Author

@MartinHjelmare can you review this last change and make sure it was implemented the way you would expect?

Comment thread homeassistant/components/vizio/config_flow.py Outdated
Comment thread homeassistant/components/vizio/config_flow.py Outdated
Comment thread homeassistant/components/vizio/config_flow.py Outdated
Comment thread homeassistant/components/vizio/config_flow.py Outdated
Comment thread homeassistant/components/vizio/config_flow.py Outdated
Copy link
Copy Markdown
Member

@MartinHjelmare MartinHjelmare left a comment

Choose a reason for hiding this comment

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

Looks good!

@raman325
Copy link
Copy Markdown
Contributor Author

This should go out with my next PR which will also be a breaking change

@MartinHjelmare MartinHjelmare merged commit ac771ad into home-assistant:dev Jan 15, 2020
raman325 added a commit to raman325/home-assistant that referenced this pull request Jan 15, 2020
* upstream/dev: (82 commits)
  Add support for vacuums to Alexa. (home-assistant#30764)
  Refactor Ring data handling (home-assistant#30777)
  Restore unit_of_measurement from entity registry (home-assistant#30780)
  Update pyubee to 0.8 (home-assistant#30785)
  Update emulated_roku to 0.1.9 (home-assistant#30791)
  Add Config Flow support, Device Registry support, available property to vizio component (home-assistant#30653)
  Allow input_* and timer component setup without config (home-assistant#30772)
  Search: Add search to default config and don't resolve area (home-assistant#30762)
  [ci skip] Translation update
  Use storage based collections for Timer platform (home-assistant#30765)
  Upgrade youtube_dl to version 2020.01.15 (home-assistant#30767)
  Whitelist Frenck for release
  Hass.io allow to reset password with CLI (home-assistant#30755)
  Revert home-assistant#29701 (home-assistant#30766)
  Add Safe Mode (home-assistant#30723)
  Update Ring to 0.6.0 (home-assistant#30748)
  Add support for the voltage sensor on the greeneye GEM (home-assistant#30484)
  Fix supported_features in MQTT fan (home-assistant#28680)
  Fix small typo in alarmdotcom component (home-assistant#30758)
  bump aiokef to 0.2.5 which uses locks (home-assistant#30753)
  ...
@lock lock Bot locked and limited conversation to collaborators Jan 16, 2020
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.

3 participants