Add aqara lock support by SchumyHao · Pull Request #14419 · home-assistant/core

SchumyHao · 2018-05-12T15:18:34Z

Signed-off-by: SchumyHao schumyhaojl@126.com

Description:

Add aqara lock support

Related issue (if applicable): fixes #

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

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:

Documentation added/updated in home-assistant.github.io

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 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.

houndci-bot · 2018-05-12T15:18:43Z

line too long (85 > 79 characters)

houndci-bot · 2018-05-12T15:18:43Z

statement ends with a semicolon

houndci-bot · 2018-05-12T15:18:43Z

expected 2 blank lines, found 1

houndci-bot · 2018-05-12T15:18:43Z

expected 2 blank lines, found 1

homeassistant · 2018-05-12T15:35:56Z

Hi @SchumyHao,

It seems you haven't yet signed a CLA. Please do so here.

Once you do that we will be able to review and accept this pull request.

Thanks!

houndci-bot · 2018-05-12T15:36:09Z

continuation line over-indented for visual indent

syssi · 2018-05-12T16:03:19Z

Please fix the lint errors. I'm very happy with your approach!

syssi · 2018-05-12T16:13:52Z

I've released PyXiaomiGateway 0.9.2. Please update the requirements!

SchumyHao · 2018-05-12T16:38:30Z

Hi @syssi
I have update the requirement.
Thank you for your review.

syssi · 2018-05-12T17:28:35Z

requirements_all.txt is not up to date
Please run script/gen_requirements_all.py

Signed-off-by: SchumyHao <schumyhaojl@126.com>

Aqara lock can not read lock state. But After verify successfully, lock will be unlock state for about 5s. After 5s, lock will auto reset to lock state. Signed-off-by: SchumyHao <schumyhaojl@126.com>

syssi

LGTM.

syssi · 2018-05-13T06:30:05Z

+                self._changed_by = int(value)
+                self._verified_wrong_times = 0
+                self._state = STATE_UNLOCKED
+                async_call_later(self.hass, UNLOCK_MAINTAIN_TIME_S,


Please remove the unit (_S).

MartinHjelmare · 2018-05-13T07:50:46Z

+        }
+        return attributes
+
+    @asyncio.coroutine


Make this a callback instead via the @callback decorator that can be imported from core.py.

Could you care about this one, too?

I'm not sure for this comment.
This function will be invoked in async_call_later, I need this function called after UNLOCK_MAINTAIN_TIME seconds. but callback function can not do IO operation, I think delay should not called in callback too.

This function will be run with async_run_job. It's safe to run callbacks.

I try to change this by using async_run_job, I tested this can work:

@@ -67,8 +68,10 @@ class XiaomiAqaraLock(LockDevice, XiaomiDevice): return attributes @asyncio.coroutine - def clear_unlock_state(self, _): + def clear_unlock_state(self): """Clear unlock state automatically.""" + yield from asyncio.sleep(UNLOCK_MAINTAIN_TIME) + self._state = STATE_LOCKED self.async_schedule_update_ha_state() @@ -85,8 +88,7 @@ class XiaomiAqaraLock(LockDevice, XiaomiDevice): self._changed_by = int(value) self._verified_wrong_times = 0 self._state = STATE_UNLOCKED - async_call_later(self.hass, UNLOCK_MAINTAIN_TIME, - self.clear_unlock_state) + self.hass.async_run_job(self.clear_unlock_state) return True return False

But I failed to using @callback
I have tried this

@@ -66,9 +67,12 @@ class XiaomiAqaraLock(LockDevice, XiaomiDevice): } return attributes + @callback @asyncio.coroutine - def clear_unlock_state(self, _): + def clear_unlock_state(self): """Clear unlock state automatically.""" + yield from asyncio.sleep(UNLOCK_MAINTAIN_TIME) + self._state = STATE_LOCKED self.async_schedule_update_ha_state() @@ -85,8 +89,7 @@ class XiaomiAqaraLock(LockDevice, XiaomiDevice): self._changed_by = int(value) self._verified_wrong_times = 0 self._state = STATE_UNLOCKED - async_call_later(self.hass, UNLOCK_MAINTAIN_TIME, - self.clear_unlock_state) + self.hass.async_run_job(self.clear_unlock_state) return True

But clear_unlock_state(self) will not invoked.

I'm sorry that how to use callback decorator, can you point me out how to do this?

Hi @MartinHjelmare @syssi
May you help me to improve this code?
I have checked the code in homeassistant, which used async_call_later and callback, but do not find a similar usage in my situation.

Just change this line to @callback.

Done.
Thank you for your advice

MartinHjelmare · 2018-05-13T08:03:26Z

+UNLOCK_MAINTAIN_TIME_S = 5
+
+
+def setup_platform(hass, config, add_devices, discovery_info=None):


This should be made a coroutine via async def to make the call to async_call_later in parse_data, which will be called during entity init, work. I assume the rest of the calls in the base entity XiaomiDevice can be done safely from an async context?

Signed-off-by: SchumyHao <schumyhaojl@126.com>

syssi · 2018-05-16T16:04:31Z

Could you add some docs?

SchumyHao · 2018-05-16T16:28:01Z

OK, I'll do this job.
I have forked https://github.com/SchumyHao/home-assistant.github.io I'll create a PR later.

syssi · 2018-05-17T18:32:01Z

I added some basic docs: home-assistant/home-assistant.io#5387

SchumyHao · 2018-05-18T16:21:14Z

Thank you @syssi
I add some more explanation on your PR

SchumyHao · 2018-05-22T06:31:24Z

I'm not sure what should I do in next step?
@syssi Can you show me what should I do to push this PR merged?

syssi · 2018-05-22T06:48:49Z

I'm waiting for an review/approval of the docs. As far as the docs are approved I will merge this PR.

SchumyHao · 2018-05-22T07:57:52Z

Thank you @syssi. Please let me know if I need do something more.

SchumyHao requested review from Danielhiversen and syssi as code owners May 12, 2018 15:18

homeassistant added integration: xiaomi_aqara new-platform platform: lock.xiaomi_aqara cla-error labels May 12, 2018

houndci-bot reviewed May 12, 2018

View reviewed changes

SchumyHao force-pushed the dev branch from 35c0466 to 8a16ff9 Compare May 12, 2018 15:35

homeassistant added the cla-needed label May 12, 2018

houndci-bot reviewed May 12, 2018

View reviewed changes

Comment thread homeassistant/components/xiaomi_aqara.py Outdated

Copy link
Copy Markdown

houndci-bot May 12, 2018

Choose a reason for hiding this comment

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

continuation line over-indented for visual indent

homeassistant added cla-signed and removed cla-needed labels May 12, 2018

This was referenced May 12, 2018

Add Xiaomi Aqara Smart Door Lock support #14415

Closed

Support for Aqara door lock #14125

Closed

SchumyHao force-pushed the dev branch from 8a16ff9 to 77a568b Compare May 12, 2018 16:12

homeassistant added the cla-signed label May 12, 2018

SchumyHao force-pushed the dev branch from 77a568b to 6e0f15e Compare May 12, 2018 16:25

homeassistant added the cla-signed label May 12, 2018

home-assistant deleted a comment from homeassistant May 12, 2018

MartinHjelmare added cla-recheck and removed cla-error labels May 12, 2018

homeassistant added cla-signed and removed cla-recheck labels May 12, 2018

syssi closed this May 12, 2018

syssi reopened this May 12, 2018

fabaff changed the title ~~xiaomi_aqara: add aqara lock support~~ Add aqara lock support May 12, 2018

SchumyHao added 4 commits May 13, 2018 09:31

xiaomi_aqara: add aqara lock support

5f18b7f

Signed-off-by: SchumyHao <schumyhaojl@126.com>

fix some format issue

10e2c22

Signed-off-by: SchumyHao <schumyhaojl@126.com>

xiaomi_aqara: change Gateway to Aqara

294038e

Signed-off-by: SchumyHao <schumyhaojl@126.com>

xiaomi_aqara: change lock state

2b52183

Aqara lock can not read lock state. But After verify successfully, lock will be unlock state for about 5s. After 5s, lock will auto reset to lock state. Signed-off-by: SchumyHao <schumyhaojl@126.com>

SchumyHao force-pushed the dev branch from c0a86b9 to 2b52183 Compare May 13, 2018 02:59

homeassistant added the cla-signed label May 13, 2018

syssi approved these changes May 13, 2018

View reviewed changes

syssi reviewed May 13, 2018

View reviewed changes

MartinHjelmare reviewed May 13, 2018

View reviewed changes

SchumyHao added 3 commits May 13, 2018 22:37

xiaomi_aqara: remove UNLOCK_MAINTAIN_TIME unit

bddb91f

Signed-off-by: SchumyHao <schumyhaojl@126.com>

xiaomi_aqara: setup_platform in coroutine

d004921

Signed-off-by: SchumyHao <schumyhaojl@126.com>

xiaomi_aqara: use callback instead of asyncio.coroutine

df0c524

Signed-off-by: SchumyHao <schumyhaojl@126.com>

homeassistant added the cla-signed label May 16, 2018

home-assistant deleted a comment from houndci-bot May 16, 2018

syssi mentioned this pull request May 17, 2018

Add Xiaomi Aqara Door Lock home-assistant/home-assistant.io#5387

Merged

syssi mentioned this pull request May 21, 2018

Add support for KIWI Door Locks #14485

Merged

7 tasks

syssi merged commit 82770fa into home-assistant:dev May 22, 2018

balloob mentioned this pull request Jun 8, 2018

0.71.0 #14876

Merged

home-assistant locked and limited conversation to collaborators Sep 5, 2018

ghost removed the platform: lock.xiaomi_aqara label Mar 21, 2019

		UNLOCK_MAINTAIN_TIME_S = 5


		def setup_platform(hass, config, add_devices, discovery_info=None):

Uh oh!

Conversation

SchumyHao commented May 12, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description:

Example entry for configuration.yaml (if applicable):

Checklist:

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

homeassistant commented May 12, 2018

Uh oh!

Choose a reason for hiding this comment

Uh oh!

syssi commented May 12, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

syssi commented May 12, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

SchumyHao commented May 12, 2018

Uh oh!

syssi commented May 12, 2018

Uh oh!

syssi left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

MartinHjelmare May 14, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

syssi commented May 16, 2018

Uh oh!

SchumyHao commented May 16, 2018

Uh oh!

syssi commented May 17, 2018

Uh oh!

SchumyHao commented May 18, 2018

Uh oh!

SchumyHao commented May 22, 2018

Uh oh!

syssi commented May 22, 2018

Uh oh!

SchumyHao commented May 22, 2018

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

SchumyHao commented May 12, 2018 •

edited

Loading

Example entry for `configuration.yaml` (if applicable):

syssi commented May 12, 2018 •

edited

Loading

syssi commented May 12, 2018 •

edited

Loading

MartinHjelmare May 14, 2018 •

edited

Loading