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 support for KIWI Door Locks #14485

Merged
merged 20 commits into from Jun 12, 2018

Conversation

Projects
None yet
6 participants
@c7h
Contributor

c7h commented May 16, 2018

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#5381

Example entry for configuration.yaml (if applicable):

lock:
  - platform: kiwi
    username: email@example.com
    password: supersecretpassword

Checklist:

  • The code change is tested and works locally.

If user exposed functionality or configuration variables are added/changed:

If the code communicates with devices, web services, or third-party tools:

  • Local tests with tox run successfully. Your PR cannot be merged unless tests pass
  • New dependencies have been added to the REQUIREMENTS variable (example).
  • New dependencies are only imported inside functions that use them (example).
  • New dependencies have been added to requirements_all.txt by running script/gen_requirements_all.py.
  • New files were added to .coveragerc.

c7h added some commits Feb 9, 2018

initial commit for kiwi door locks
bugfixes

improved attribute display

flake8

more style adjustments
added requirements_all
reordered imports and flake8

attempt to pelase a very picky linter

also pleasing pylint now :)

@fabaff fabaff changed the title from New Component: KIWI Door Lock to Add support for KIWI Door Locks May 16, 2018

@fabaff

This comment has been minimized.

Member

fabaff commented May 16, 2018

Successor of #12304

@c7h c7h referenced this pull request May 16, 2018

Merged

Add documentation for KIWI.KI smartlock #5381

2 of 2 tasks complete
def setup_platform(hass, config, add_devices, discovery_info=None):
"""Set up the KIWI lock platform."""
from kiwiki import KiwiClient
kiwi = KiwiClient(config.get(CONF_USERNAME), config.get(CONF_PASSWORD))

This comment has been minimized.

@Kane610

Kane610 May 18, 2018

Contributor

No need for get, you already require those parameters in the schema

def __init__(self, kiwi_lock, client):
"""Initialize the lock."""
self._sensor = kiwi_lock
self._device_attrs = None

This comment has been minimized.

@Kane610

Kane610 May 18, 2018

Contributor

No need to set it to None if you assign it four lines down

@property
def is_locked(self):
"""Return true if lock is locked."""
return True

This comment has been minimized.

@Kane610

Kane610 May 18, 2018

Contributor

Can it never be unlocked?

This comment has been minimized.

@c7h

c7h May 18, 2018

Contributor

No, there's no way to detect the actual locked state. Therefore, the assumed state is locked (devices automatically lock after unlock within 5 seconds)

This comment has been minimized.

@syssi

syssi May 21, 2018

Member

You could use this approach: #14419 Just change the state for some seconds.

This comment has been minimized.

@syssi

syssi May 24, 2018

Member

Please implement the suggested approach. It changes the state of the lock for a short period of time and makes automations easier.

This comment has been minimized.

@c7h

c7h May 24, 2018

Contributor

Ok

ATTR_CAN_INVITE: kiwi_lock.get('can_invite')}
self._device_attrs.update(address)
self._device_attrs.update({

This comment has been minimized.

@syssi

syssi May 24, 2018

Member

Why do extract lat/lng and add it again separately? You could update the content of address and add it to the list by **address:

self._device_attrs {
      ATTR_ID: self.lock_I'd,
      **address
}

This comment has been minimized.

@c7h

c7h May 29, 2018

Contributor

ok this can be nested structure? cool - i'll change it 👍

def setup_platform(hass, config, add_devices, discovery_info=None):
"""Set up the KIWI lock platform."""
from kiwiki import KiwiClient
kiwi = KiwiClient(config[CONF_USERNAME], config[CONF_PASSWORD])

This comment has been minimized.

@fabaff

fabaff Jun 1, 2018

Member

I guess that there will be a traceback if the credentials are wrong. The setup should fails otherwise the users end up with a non-working platform.

@fabaff

This comment has been minimized.

Member

fabaff commented Jun 2, 2018

c7h/kiwiki_client#1 should be addressed upstream and the requirement updated.

"""Set up the KIWI lock platform."""
from kiwiki import KiwiClient
kiwi = KiwiClient(config[CONF_USERNAME], config[CONF_PASSWORD])
add_devices([KiwiLock(lock, kiwi) for lock in kiwi.get_locks()], True)

This comment has been minimized.

@fabaff

fabaff Jun 2, 2018

Member

A log entry that there are no locks found would help the users. Also, the setup can be terminated if there are no locks.

@property
def is_locked(self):
"""Return true if lock is locked."""
if self._state is not None:

This comment has been minimized.

@MartinHjelmare

MartinHjelmare Jun 2, 2018

Member

Remove this check. It doesn't add anything.

kiwi = KiwiClient(config[CONF_USERNAME], config[CONF_PASSWORD])
except KiwiException as e:
_LOGGER.error(e.msg)
return False

This comment has been minimized.

@MartinHjelmare

MartinHjelmare Jun 2, 2018

Member

Don't return False. Just return.

This comment has been minimized.

@c7h

c7h Jun 2, 2018

Contributor

That is horrible to read. I prefer return False - there is no room for misinterpretation.

This comment has been minimized.

@MartinHjelmare

MartinHjelmare Jun 2, 2018

Member

Nothing is checking the return value.

This comment has been minimized.

@c7h

c7h Jun 2, 2018

Contributor

Check the platform example -
False is returned here on login failed. It makes sense even if its not checked because it improves the readability. https://developers.home-assistant.io/docs/en/creating_platform_example_light.html

This comment has been minimized.

@MartinHjelmare

MartinHjelmare Jun 2, 2018

Member

That example needs updating.

It doesn't improve readability. It gives the wrong impression what happens when the function returns.

It also makes the return inconsistent.

This comment has been minimized.

@c7h

c7h Jun 4, 2018

Contributor

Ok, I created an Issue (home-assistant/developers.home-assistant#21) and will change it to return

if not available_locks:
# No locks found; abort setup routine.
_LOGGER.info("No KIWI locks found in your account.")
return False

This comment has been minimized.

@MartinHjelmare

MartinHjelmare Jun 2, 2018

Member

Same as above.

@c7h c7h referenced this pull request Jun 4, 2018

Closed

Update Platform Example Code #21

@MartinHjelmare

Sorry I missed this one.

_LOGGER.error("failed to open door")
else:
self._state = STATE_UNLOCKED
async_call_later(self.hass, UNLOCK_MAINTAIN_TIME,

This comment has been minimized.

@MartinHjelmare

MartinHjelmare Jun 4, 2018

Member

This is a callback so have to be run from within the event loop. Use hass.add_job to schedule async_call_later on the event loop.

This comment has been minimized.

@c7h

c7h Jun 4, 2018

Contributor

doesn't async_call_later already do that for you?

This comment has been minimized.

@MartinHjelmare

MartinHjelmare Jun 4, 2018

Member

async_call_later schedules the passed in callback yes, but async_call_later itself needs to be called from within the event loop. The unlock method here will not be run within the event loop but in a worker thread. So to make sure that async_call_later is called from the event loop, we need to use hass.add_job.

self.hass.add_job(
    async_call_later, self.hass, UNLOCK_MAINTAIN_TIME, self.clear_unlock_state)

@wafflebot wafflebot bot added the in progress label Jun 7, 2018

from kiwiki import KiwiClient, KiwiException
try:
kiwi = KiwiClient(config[CONF_USERNAME], config[CONF_PASSWORD])
except KiwiException as e:

This comment has been minimized.

@MartinHjelmare

MartinHjelmare Jun 12, 2018

Member

Linter is failing here. Please use eg exc instead of e.

This comment has been minimized.

@c7h

c7h Jun 12, 2018

Contributor

that's sad.... 😢

@MartinHjelmare MartinHjelmare merged commit 6755ae2 into home-assistant:dev Jun 12, 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 increased (+0.03%) to 94.011%
Details

@wafflebot wafflebot bot removed the in progress label Jun 12, 2018

@balloob balloob referenced this pull request Jun 22, 2018

Merged

0.72 #15088

MizterB added a commit to MizterB/home-assistant that referenced this pull request Aug 11, 2018

Add support for KIWI Door Locks (home-assistant#14485)
* initial commit for kiwi door locks

bugfixes

improved attribute display

flake8

more style adjustments

* added session handling

flake8

* added requirements_all

reordered imports and flake8

attempt to pelase a very picky linter

also pleasing pylint now :)

* re-try the build

* added kiwi.py to .coveragerc

* reorganized datetime handling and attribute naming

* created pypi package for door lock library

* updated requirements_all.txt

* code review changes

* added async lock state reset for locking state

* refactored lat/lon attribute updates

* initial locked state changed from undefined to locked

* refactored is_locked property check

* handling authentication exception in setup_platform

* added more check in setup_platform

* code review changes: return type in setup_platform

* fixed logging issue

* event handling in main thread

* updated kiwiki-client to version 0.1.1

* renamed alias e to exc

cyberjacob pushed a commit to cyberjacob/home-assistant that referenced this pull request Sep 4, 2018

Add support for KIWI Door Locks (home-assistant#14485)
* initial commit for kiwi door locks

bugfixes

improved attribute display

flake8

more style adjustments

* added session handling

flake8

* added requirements_all

reordered imports and flake8

attempt to pelase a very picky linter

also pleasing pylint now :)

* re-try the build

* added kiwi.py to .coveragerc

* reorganized datetime handling and attribute naming

* created pypi package for door lock library

* updated requirements_all.txt

* code review changes

* added async lock state reset for locking state

* refactored lat/lon attribute updates

* initial locked state changed from undefined to locked

* refactored is_locked property check

* handling authentication exception in setup_platform

* added more check in setup_platform

* code review changes: return type in setup_platform

* fixed logging issue

* event handling in main thread

* updated kiwiki-client to version 0.1.1

* renamed alias e to exc
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment