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

Initial scene support for Fibaro hubs #18779

Merged
merged 9 commits into from Dec 3, 2018

Conversation

Projects
None yet
5 participants
@pbalogh77
Copy link
Contributor

pbalogh77 commented Nov 29, 2018

Added initial support for scenes to fibaro hub integration

Description:

Fibaro hub integration now imports and can activate scenes

Related issue (if applicable): fixes #

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

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.

pbalogh77 added some commits Nov 29, 2018

Initial scene support
Added initial support for fibaro scenes

@wafflebot wafflebot bot added the in progress label Nov 29, 2018

@pbalogh77 pbalogh77 changed the title Initial scene support for Fibaro hubs WIP: Initial scene support for Fibaro hubs Nov 29, 2018

@pbalogh77 pbalogh77 changed the title WIP: Initial scene support for Fibaro hubs Initial scene support for Fibaro hubs Dec 1, 2018

@balloob

This comment has been minimized.

Copy link
Member

balloob commented Dec 2, 2018

Just a note that this is going to get a breaking change in the future as I want to break Home Assistant scenes out of the scene component and have a scene_external (with things like Fibaro) and scene which is scenes in Home Assistant land.

You're also welcome to open a PR to break up the component and then do a PR to add Fibaro to scene_external.

@pbalogh77

This comment has been minimized.

Copy link
Contributor

pbalogh77 commented Dec 2, 2018

Just a note that this is going to get a breaking change in the future as I want to break Home Assistant scenes out of the scene component and have a scene_external (with things like Fibaro) and scene which is scenes in Home Assistant land.

You're also welcome to open a PR to break up the component and then do a PR to add Fibaro to scene_external.

I'm way too junior in this yet to do that :-) However, when it's done, I'll migrate my code to the new component.

grrr, my mistake.
My local pylint and flake8 are playing tricks with me
Show resolved Hide resolved homeassistant/components/fibaro.py Outdated
Show resolved Hide resolved homeassistant/components/fibaro.py Outdated
Show resolved Hide resolved homeassistant/components/scene/fibaro.py Outdated
Show resolved Hide resolved homeassistant/components/scene/fibaro.py
fixes based on code review
ABC ordered the list of platforms
changed setup platform to async
removed overloaded name property as the FibaroDevice parent class already provides this
Changed to new style string formatting
@MartinHjelmare
Copy link
Member

MartinHjelmare left a comment

That should be enough.

Show resolved Hide resolved homeassistant/components/scene/fibaro.py Outdated
Update homeassistant/components/scene/fibaro.py
Co-Authored-By: pbalogh77 <peter.balogh2@gmail.com>
@MartinHjelmare
Copy link
Member

MartinHjelmare left a comment

Looks good!

@MartinHjelmare

This comment has been minimized.

Copy link
Member

MartinHjelmare commented Dec 3, 2018

Can be merged when build passes and docs PR is linked above in this PR description.

@pbalogh77

This comment has been minimized.

Copy link
Contributor

pbalogh77 commented Dec 3, 2018

Documentation added to home-assistant/home-assistant.io#7715

@MartinHjelmare MartinHjelmare merged commit 149edda into home-assistant:dev Dec 3, 2018

5 checks passed

Hound No violations found. Woof!
WIP Legacy commit status override — see details
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.02%) to 92.935%
Details

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

@balloob balloob referenced this pull request Dec 12, 2018

Merged

0.84 #19215

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