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

Add new component fritzbox binary_sensor #17057

Merged
merged 7 commits into from Oct 4, 2018

Conversation

Projects
None yet
5 participants
@hthiery
Contributor

hthiery commented Oct 1, 2018

Signed-off-by: Heiko Thiery heiko.thiery@gmail.com

Description:

Related issue (if applicable): fixes #

Pull request in home-assistant.io with documentation (if applicable): home-assistant/home-assistant.io#6423

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 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.
Add new component fritzbox binary_sensor
Signed-off-by: Heiko Thiery <heiko.thiery@gmail.com>

hthiery added some commits Oct 2, 2018

fix failed flake8 test
Signed-off-by: Heiko Thiery <heiko.thiery@gmail.com>
add new file to .coveragerc
Signed-off-by: Heiko Thiery <heiko.thiery@gmail.com>
.coveragerc Outdated
@@ -110,6 +110,7 @@ omit =

homeassistant/components/fritzbox.py
homeassistant/components/switch/fritzbox.py
homeassistant/components/binary_sensor/fritzbox.py

This comment has been minimized.

@fabaff

fabaff Oct 2, 2018

Member

Use

    homeassistant/components/*/fritzbox.py

to cover the platforms.

Show resolved Hide resolved homeassistant/components/binary_sensor/fritzbox.py
return self._device.name

@property
def should_poll(self):

This comment has been minimized.

@fabaff

fabaff Oct 2, 2018

Member

Not needed polling is true by default.

@fabaff

This comment has been minimized.

Member

fabaff commented Oct 2, 2018

Just three small comment, rest looks good.

hthiery and others added some commits Oct 2, 2018

use wildcard to cover all platforms
Signed-off-by: Heiko Thiery <heiko.thiery@gmail.com>
remove polling because polling is true by default
Signed-off-by: Heiko Thiery <heiko.thiery@gmail.com>
add blank line to keep imports ordered
Signed-off-by: Heiko Thiery <heiko.thiery@gmail.com>
@fabaff

fabaff approved these changes Oct 4, 2018

Thanks 🐦

@fabaff fabaff merged commit 178bf73 into home-assistant:dev Oct 4, 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 decreased (-0.1%) to 93.624%
Details

@wafflebot wafflebot bot removed the in progress label Oct 4, 2018

@balloob balloob referenced this pull request Oct 12, 2018

Merged

0.80.0 #17361

@VDRainer

This comment has been minimized.

VDRainer commented Oct 12, 2018

For what is this good for?
Not much info in the docs:

@hthiery

This comment has been minimized.

Contributor

hthiery commented Oct 13, 2018

Moin, currently only a window/door sensor is supported/tested. But in the Fritzbox it is implemeted as alarm sensor. So other devices should also bei possible.

@hthiery

This comment has been minimized.

Contributor

hthiery commented Oct 13, 2018

If you have other types of HANFUN sensors we could integrate these too.

@home-assistant home-assistant locked as resolved and limited conversation to collaborators Oct 13, 2018

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