ZHA lock code services and events by jcam · Pull Request #47208 · home-assistant/core

jcam · 2021-03-01T02:55:43Z

Proposed change

Add zha_events for zigbee lock notifications (including the sending device and user slot)
Add set_user_code and clear_user_code services to zigbee locks

Type of change

Dependency upgrade
Bugfix (non-breaking change which fixes an issue)
New integration (thank you!)
New feature (which adds functionality to an existing integration)
Breaking change (fix/feature causing existing functionality to break)
Code quality improvements to existing code or addition of tests

Example entry for `configuration.yaml`:

# Example configuration.yaml

Additional information

This PR fixes or closes issue: fixes #
This PR is related to issue: https://community.home-assistant.io/t/zha-user-code-management-on-zigbee-door-locks/225594
Link to documentation pull request: New services for managing lock codes on ZHA Zigbee locks home-assistant.io#16947

Checklist

The code change is tested and works locally.
Local tests pass. Your PR cannot be merged unless tests pass
There is no commented out code in this PR.
I have followed the development checklist
The code has been formatted using Black (black --fast homeassistant tests)
Tests have been added to verify that the new code works.

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

Documentation added/updated for www.home-assistant.io

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

The manifest file has all fields filled out correctly.
Updated and included derived files by running: python3 -m script.hassfest.
New or updated dependencies have been added to requirements_all.txt.
Updated by running python3 -m script.gen_requirements_all.
Untested files have been added to .coveragerc.

The integration reached or maintains the following Integration Quality Scale:

No score or internal
🥈 Silver
🥇 Gold
🏆 Platinum

To help with the load of incoming pull requests:

I have reviewed two other open pull requests in this repository.

jcam · 2021-03-01T02:59:01Z

Still need to add tests and documentation and clean up some things, possibly add a couple more function calls to be ready for keymaster support

jcam · 2021-03-01T03:38:57Z

@Adminiuga @dmulcahey looking for a little discussion on this if you have a minute.
The feature request (https://community.home-assistant.io/t/zha-user-code-management-on-zigbee-door-locks/225594) asks for get, set and clear for user codes.
I can write a get function pretty easily, could add a get service as well and have it send the user code to a zha_event, for example.
Or, I can make a function for completeness (and for ease of use by add-on tools like keymaster)
Or, I can skip get entirely.
Any thoughts on that? And also on the rest of it? :)

Adminiuga · 2021-03-01T04:28:23Z

Yeah, there's no good way to return a result of a service call to the user. So making a service call to set/clear the codes is easy, but for reading the code, the solution is going to be cumbersome, like issuing a service call and then generating an event???
But this also means, that a somewhat sensitive information like pin codes could be logged in the recorder.

Maybe, should not allow reading the pin codes at all?

jcam · 2021-03-01T12:02:54Z

That's what I was thinking as well. Looking at the zwavejs homeassistant library code, it looks like the read codes functions were included (so custom add-ons like Keymaster can read them), without any service calls.
I'm thinking of mirroring that approach.

Adminiuga · 2021-03-01T12:39:54Z

Does add-on read the code through home assistant api or directly through zwave j's server?

dmulcahey · 2021-03-01T12:41:04Z

If you add the functionality I can find a way to expose it via the websocket API to the front end

Adminiuga · 2021-03-01T12:52:12Z

Yeah, it probably makes most sense to be exposed via ws commands, but i have no ideas iv lovelace cards support any. Haven't looked frankly.

dmulcahey · 2021-03-01T12:59:07Z

My guess is I’ll have to build custom elements for the device page... we’ll see

jcam · 2021-03-01T13:15:41Z

Okay yeah that makes sense. I'll add a few more of the functions this evening. Understanding enough to get one to work took a day, but now that I got this far, adding get, clear all codes, enable/disable code slot etc will only be a few minutes each

raman325 · 2021-03-01T17:13:28Z

Does add-on read the code through home assistant api or directly through zwave j's server?

We use the client library to read the codes. As you mentioned, there's no way to do this through the homeassistant API using existing constructs. We can create a helper method in the component code in lieu of having it in the client library but that's about as far as we can go as far as the custom component (keymaster) is concerned

Vlad-Star · 2021-03-01T20:58:31Z

@Adminiuga @dmulcahey looking for a little discussion on this if you have a minute.
The feature request (https://community.home-assistant.io/t/zha-user-code-management-on-zigbee-door-locks/225594) asks for get, set and clear for user codes.
[...]

@Adminiuga @jcam @dmulcahey
OMG guys... I am the OP on this feature request and wanted to let you know what I truly appreciate for picking it up. Obviously, my request for those functions was intended for further integration with the key lock managers. If there is a standard API call for those, that would abstract them from specific integration (ZHA/Zwave/ZWave JS) - this would be even better.

Let me know if I can help anyhow. I am a tech person (sysadmin) with some dev skills in a past (perl, php, C) , but unfortunately not in Python.

jcam · 2021-03-01T22:48:14Z

I picked it up because I have a zigbee lock :D
And because I wanted to learn a little python, heh.

There is no standard API calls for the key lock managers, they were mostly all custom for zwave, but since raman325 and firstof9 both just went through the steps of adding zwave-js to keymanager, everyone's pretty familiar with what'll need to be in place for zigbee. The way they interact will be different because of how each integration stores things and presents things, but underneath it all, it's all just a lock with code slots and pin codes, so a translation function in keymanager won't be hard.

dmulcahey · 2021-03-08T12:41:24Z

+            self.zha_send_event(
+                command_name,
+                {
+                    "source": args[0].name,


@Adminiuga is there a better way to parse the args out? Or is there a better way to do this?

not really, cause args would change depending on the command_id

dmulcahey · 2021-03-08T12:43:30Z

+        set_pin_code = self.__getattr__("set_pin_code")
+        await set_pin_code(
+            *(
+                code_slot - 1,  # start code slots at 1


how come all of these are shifted by 1? Just curious

I shifted by one in part because I want this to be "people" friendly as opposed to "machine" friendly, so start at 1 instead of 0; and in part because z-wave locks start with slot 1, and I wanted to have consistency with them.

dmulcahey · 2021-03-08T12:44:55Z

This still needs more coverage in the tests. The uncovered lines are tagged in the PR by CI

jcam · 2021-03-08T14:06:15Z

This still needs more coverage in the tests. The uncovered lines are tagged in the PR by CI

Yup definitely still need to build some tests.
Hoping to have that done in time for the 2021.4 beta :)

Adminiuga · 2021-03-10T20:37:36Z

+        set_pin_code = self.__getattr__("set_pin_code")
+        await set_pin_code(


why not just await self.set_pin_code() ?

is that a thing that will work? If so I can update to that easily

definitely test it, but it should work.

I'm still new to python :)
running tox now, will try with my lock in a bit

Work got crazy for a couple weeks, but I finally was able to test with my door locks. As you suggested, it worked just fine. Updated commit pushed.
There is more that could be done... we can add more API tests and add UI to the device page, but this should be ready for the beta as an MVP.

Adminiuga · 2021-03-10T20:39:45Z

+        set_pin_code = self.__getattr__("set_pin_code")
+        await set_pin_code(
+            *(
+                code_slot - 1,  # start code slots at 1, Zigbee internals use 0
+                closures.DoorLock.UserStatus.Enabled,
+                closures.DoorLock.UserType.Unrestricted,
+                user_code,
+            ),
+        )


Suggested change

set_pin_code = self.__getattr__("set_pin_code")

await set_pin_code(

*(

code_slot - 1, # start code slots at 1, Zigbee internals use 0

closures.DoorLock.UserStatus.Enabled,

closures.DoorLock.UserType.Unrestricted,

user_code,

),

)

await self.set_pin_code(

code_slot - 1, # start code slots at 1, Zigbee internals use 0

closures.DoorLock.UserStatus.Enabled,

closures.DoorLock.UserType.Unrestricted,

user_code,

)

flyize · 2021-03-17T17:53:17Z

If there is anything I can do, short of coding, to help here - I'd love to. So excited for this.

jcam · 2021-03-17T19:00:47Z

Hi! If you're up for it, I'm aiming to have this in place for the beta, and you could test it out in the first beta :)

flyize · 2021-03-18T12:52:49Z

I'm happy to help in any way that I can. Thanks for taking on this project.

Vlad-Star · 2021-03-18T13:02:30Z

@jcam l'd love to participate in beta testing as well - hopefully without breaking my whole system :)

I am thinking about cloning the SD card, switching HA repository to "beta" (how?) on the cloned card. testing and then swapping the card back to the original at the end. Will it work?

flyize · 2021-03-18T13:26:50Z

You are already doing scheduled backups, right? 😄

Adminiuga

Lgtm

homeassistant added the cla-signed label Mar 1, 2021

probot-home-assistant Bot added integration: zha new-feature labels Mar 1, 2021

probot-home-assistant Bot assigned Adminiuga and dmulcahey Mar 1, 2021

jcam force-pushed the zha_lock_codes branch from a1d5f72 to d64e9be Compare March 1, 2021 03:41

Adminiuga reviewed Mar 1, 2021

View reviewed changes

Comment thread homeassistant/components/zha/core/channels/closures.py Outdated

jcam force-pushed the zha_lock_codes branch 2 times, most recently from 23d5f22 to 8b32f8c Compare March 2, 2021 03:37

dmulcahey reviewed Mar 8, 2021

View reviewed changes

jcam added 4 commits March 10, 2021 07:07

ZHA lock code services and events

28237c0

ZHA Locks: A few more services, use the library functions

96d84e1

Catch exception when command id is not in command list

9dc9e09

Add tests for lock code services

fdff804

jcam force-pushed the zha_lock_codes branch from 8b32f8c to fdff804 Compare March 10, 2021 18:24

Add tests for enable/disable

37f3ab4

jcam mentioned this pull request Mar 10, 2021

New services for managing lock codes on ZHA Zigbee locks home-assistant/home-assistant.io#16947

Merged

8 tasks

jcam marked this pull request as ready for review March 10, 2021 19:15

Better document code slot ID shifting

5f90fe1

Adminiuga reviewed Mar 10, 2021

View reviewed changes

Simplify cluster commands

77a41a5

Adminiuga approved these changes Mar 26, 2021

View reviewed changes

dmulcahey merged commit 67791fa into home-assistant:dev Mar 27, 2021

github-actions Bot locked and limited conversation to collaborators Mar 28, 2021

		set_pin_code = self.__getattr__("set_pin_code")
		await set_pin_code(

Uh oh!

Conversation

jcam commented Mar 1, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Proposed change

Type of change

Example entry for configuration.yaml:

Additional information

Checklist

Uh oh!

jcam commented Mar 1, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jcam commented Mar 1, 2021

Uh oh!

Adminiuga commented Mar 1, 2021

Uh oh!

jcam commented Mar 1, 2021

Uh oh!

Adminiuga commented Mar 1, 2021

Uh oh!

dmulcahey commented Mar 1, 2021

Uh oh!

Adminiuga commented Mar 1, 2021

Uh oh!

dmulcahey commented Mar 1, 2021

Uh oh!

jcam commented Mar 1, 2021

Uh oh!

Uh oh!

raman325 commented Mar 1, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Vlad-Star commented Mar 1, 2021

Uh oh!

jcam commented Mar 1, 2021

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!

dmulcahey commented Mar 8, 2021

Uh oh!

jcam commented Mar 8, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

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!

flyize commented Mar 17, 2021

Uh oh!

jcam commented Mar 17, 2021

Uh oh!

flyize commented Mar 18, 2021

Uh oh!

Vlad-Star commented Mar 18, 2021

Uh oh!

flyize commented Mar 18, 2021

Uh oh!

Adminiuga left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

jcam commented Mar 1, 2021 •

edited

Loading

Example entry for `configuration.yaml`:

jcam commented Mar 1, 2021 •

edited

Loading

raman325 commented Mar 1, 2021 •

edited

Loading

jcam commented Mar 8, 2021 •

edited

Loading