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’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Introduce wiz integration for the WiZ Platform #44779

Merged
merged 56 commits into from
Feb 5, 2022

Conversation

sbidy
Copy link
Contributor

@sbidy sbidy commented Jan 2, 2021

Proposed change

A HA integration for WiZ Platform - mainly for bulbs (Phillips, SLV and more). The integration extents HA to communicate directly with this WiFi connected WiZ devices via async UDP calls. There is no Cloud or Web API needed for interaction with this lights.

The integration is designed async, including the UDP calls via pywizlight extension.

Supported Features (Bulbs):

  • Brightness
  • Color (RGB)
  • White Color Temperature
  • On/Off, Toggle
  • Effects
  • Bulb feature detection. Mapping up to 16 different bulb feature sets.

Type of change

  • Dependency upgrade
  • Bugfix (non-breaking change which fixes an issue)
  • New integration (thank you!)
  • New feature (which adds functionality to an existing integration)
  • Breaking change (fix/feature causing existing functionality to break)
  • Code quality improvements to existing code or addition of tests

Example entry for configuration.yaml:

# Example configuration.yaml
light:
  - platform: wiz_light
    name: Name_Bulb_1
    host: 127.0.0.1
  - platform: wiz_light
    name: Name_Bulb_2
    host: 127.0.0.2

Additional information

Checklist

  • The code change is tested and works locally.
  • Local tests pass. Your PR cannot be merged unless tests pass
  • There is no commented out code in this PR.
  • I have followed the development checklist
  • The code has been formatted using Black (black --fast homeassistant tests)
  • Tests have been added to verify that the new code works.

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

The integration reached or maintains the following Integration Quality Scale:

  • No score or internal
  • 🥈 Silver
  • 🥇 Gold
  • 🏆 Platinum

To help with the load of incoming pull requests:

@sbidy
Copy link
Contributor Author

sbidy commented Jan 3, 2021

Error ModuleNotFoundError: No module named 'tests.async_mock' shows up in the pytest. I don't know why.

My run:

Test session starts (platform: linux, Python 3.8.6, pytest 6.1.2, pytest-sugar 0.9.4)
rootdir: /workspaces/core, configfile: pyproject.toml
plugins: sugar-0.9.4, forked-1.3.0, xdist-2.1.0, cov-2.10.1, test-groups-1.0.3, timeout-1.4.2, requests-mock-1.8.0, aiohttp-0.3.0
collecting ... 
 tests/components/wiz_light/test_config_flow.py ✓✓✓✓

Copy link
Contributor

@esev esev left a comment

Choose a reason for hiding this comment

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

It looks like only the coverage check is failing now. There are three untested lines in config_flow.py.
https://github.com/home-assistant/core/pull/44779/checks?check_run_id=1659541458

tests/components/wiz_light/test_config_flow.py Outdated Show resolved Hide resolved
tests/components/wiz_light/test_config_flow.py Outdated Show resolved Hide resolved
@sbidy
Copy link
Contributor Author

sbidy commented Jan 7, 2021

@esev Thank you for the feedback - I'll try to improve the test coverage. Can I made a dry-run in the devcontainer to check the coverage?

@esev
Copy link
Contributor

esev commented Jan 7, 2021

@esev Thank you for the feedback - I'll try to improve the test coverage.

You're welcome. Hope this helps getting it reviewed faster by one of the core members. Looks like everything is green now. Nice work on this integration.

Can I made a dry-run in the devcontainer to check the coverage?

I've been using the last example: In case you want to check the code coverage for your new component
https://developers.home-assistant.io/docs/development_testing#testing-outside-of-tox
I'm not sure if it works in the devcontainer, but if pytest works I assume the coverage report would also.

@RockPaperPencil
Copy link

I have been running the sbidy/wiz_light integration for a while and thought I would take this PR for a spin.
So I pulled sbidy/core and built myself a Docker image of the current wiz_light branch on top of armv7-homeassistant-base:2021.01.0 which is now running in my home.

A couple of observations/thoughts:

Missing label/explanation for host address input

missing-field-description

Input validation error message

If triggering an input validation error with a non-supported address, the Name field will reset.
This leads to users not just having to input the host address one more time, but their desired name as well.
error-handling
Also, typo with the enterd IP.

Limited to adding lights by IP through GUI

My router runs OpenWRT and will happily register and resolve DHCP client hostnames. This means that I don't have to worry about static DHCP address entries, and have defined my lights in the following manner from previously:

light:
  - platform: wiz_light
    name: spisebord
    host: wiz_7395e0

Should it work the same way through the GUI as through configuration.yaml?

Thumbs up

Thumbs up and thank you @sbidy for moving this forward 😄

@sbidy
Copy link
Contributor Author

sbidy commented Jan 14, 2021

I added some enhancements to the integration. Now it is possible to use hostnames and ips to add bulbs.
The labels are also now correct. Thanks @RockPaperPencil for the feedback!!

Copy link
Member

@balloob balloob left a comment

Choose a reason for hiding this comment

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

This looks good! Looks like some cosmetic changes left, otherwise ok to merge

@frenck frenck self-requested a review February 4, 2022 23:09
tests/components/wiz/test_config_flow.py Outdated Show resolved Hide resolved
homeassistant/components/wiz/light.py Outdated Show resolved Hide resolved
@frenck
Copy link
Member

frenck commented Feb 4, 2022

Tested it, works fine!
There are some improvements still possible, and also, it's coded centric around lights right now. But nothing that can't be improved on in the future. Couple of the small things above to resolve and ready to go 👍

@balloob balloob merged commit 432d9a8 into home-assistant:dev Feb 5, 2022
@frenck frenck mentioned this pull request Feb 5, 2022
11 tasks
@bdraco bdraco mentioned this pull request Feb 5, 2022
22 tasks
Copy link
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.

Please address the comments in a new PR. Thanks!

homeassistant/components/wiz/__init__.py Show resolved Hide resolved
homeassistant/components/wiz/light.py Show resolved Hide resolved
homeassistant/components/wiz/light.py Show resolved Hide resolved
homeassistant/components/wiz/light.py Show resolved Hide resolved
homeassistant/components/wiz/light.py Show resolved Hide resolved
homeassistant/components/wiz/light.py Show resolved Hide resolved
homeassistant/components/wiz/light.py Show resolved Hide resolved
homeassistant/components/wiz/strings.json Show resolved Hide resolved
tests/components/wiz/test_config_flow.py Show resolved Hide resolved
homeassistant/components/wiz/config_flow.py Show resolved Hide resolved
@sbidy
Copy link
Contributor Author

sbidy commented Feb 5, 2022

Woow - insane things happen here :) !!! Thank you !!!

@bram-pkg
Copy link

bram-pkg commented Feb 5, 2022

Hype! Thanks for all the work everyone! Will be using this a lot in my home.

@bdraco
Copy link
Member

bdraco commented Feb 5, 2022

This should be the last cleanup on the HASS side from me.

I'm going to take a look and see what we can do to make the lib less chatty when the bulb is offline

@sbidy
Copy link
Contributor Author

sbidy commented Feb 6, 2022

This should be the last cleanup on the HASS side from me.

I'm going to take a look and see what we can do to make the lib less chatty when the bulb is offline

I merged the PR on the lib side. I'll test it and create a new release for that.

@github-actions github-actions bot locked and limited conversation to collaborators Feb 7, 2022
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.