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
New Component: KIWI Door Lock #12304
Conversation
bugfixes improved attribute display flake8 more style adjustments
flake8
"""Unlock the device.""" | ||
if not self._client.open_door(self.id): | ||
_LOGGER.error("failed to open door") | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
blank line at end of file
self._device_attrs = None | ||
self._client = client | ||
self.id = kiwi_lock['sensor_id'] | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
blank line contains whitespace
looks like travis run into a timeout - that's why the build failed: Traceback (most recent call last):
File "/home/travis/build/home-assistant/home-assistant/tests/components/sensor/test_dsmr.py", line 85, in test_default_setup
assert power_consumption.state == '0.0'
AssertionError: assert 'unknown' == '0.0'
+ where 'unknown' = <state sensor.power_consumption=unknown; friendly_name=Power Consumption, icon=mdi:flash @ 2018-02-11T17:27:53.351542+00:00>.state can someone please re-run this build? |
reordered imports and flake8 attempt to pelase a very picky linter also pleasing pylint now :)
051447a
to
6db0fb2
Compare
@andrey-git can you re-run job https://travis-ci.org/home-assistant/home-assistant/jobs/340244800 please? It timed out and blocked the build. Cheers |
You can close and then re-open the PR to rerun the build. |
import requests | ||
|
||
import voluptuous as vol | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please keep the import grouped.
|
||
_LOGGER = logging.getLogger(__name__) | ||
|
||
ATTR_TYPE = "hardware type" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Set it to ATTR_TYPE = 'hardware_type'
.
from homeassistant.const import ( | ||
CONF_PASSWORD, CONF_USERNAME, ATTR_ID, ATTR_LONGITUDE, ATTR_LATITUDE) | ||
|
||
REQUIREMENTS = ['python-dateutil==2.6.1'] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Use homeassistant.util.dt
if you have to deal with time/date objects.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ah okay - didn't know about that - that would be nice because It would eliminate the REQUIREMENTS
|
||
ATTR_TYPE = "hardware type" | ||
ATTR_PERMISSION = 'permission' | ||
ATTR_CAN_INVITE = "can invite others" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ATTR_CAN_INVITE = 'can_invite_others'
add_devices([KiwiLock(lock, kiwi) for lock in kiwi.get_locks()], True) | ||
|
||
|
||
class KiwiClient: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This code should be placed in a module which can be installed from PyPI. This way other people can re-use it and we don't need to ship device-specific code with Home Assistant.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That would be ideal but If you look at it - it doesn't do much but and in my opinion it's not worth creating a PyPI package for it. I reference to the locktrion lock which does not use a site package as well. I'm pro moving to a PyPI once functionality grows.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
https://home-assistant.io/developers/code_review_platform/
See 6.1 and the note on top.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
OK I'll create a pip. Just give me some days to finish my vacation 🏄
@fabaff is there anything else? |
@@ -418,6 +418,7 @@ omit = | |||
homeassistant/components/lock/nello.py | |||
homeassistant/components/lock/nuki.py | |||
homeassistant/components/lock/sesame.py | |||
homeassistant/components/lock/kiwi.py |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🔡
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please create a standalone Python package that contains the logic.
This PR seems to have gone stale. Closing it. You can reopen it when you're ready to finish it. |
Description:
Added support for the KIWI Door lock platform (https://kiwi.ki).
Pull request in home-assistant.github.io with documentation (if applicable): home-assistant/home-assistant.io#4639
Example entry for
configuration.yaml
(if applicable):Checklist:
If user exposed functionality or configuration variables are added/changed:
If the code communicates with devices, web services, or third-party tools:
tox
run successfully. Your PR cannot be merged unless tests passREQUIREMENTS
variable (example).requirements_all.txt
by runningscript/gen_requirements_all.py
..coveragerc
.