search

zwave-js / certification-backlog

Issues and discussions related to Z-Wave certification of HA + Z-Wave JS
1 stars 0 forks source link

Implement missing functionality for Door Locks #21

Closed marcelveldt closed 2 weeks ago

marcelveldt commented 1 year ago

From core created by AlCalzone: home-assistant/core#86955

The problem

grafik (the mentioned operation types mean constant 0x01 and timed 0x02, V4+ adds additional ones)

I could not find any way to configure the operation type of a lock, so this will need to be added before certification.

Also setting the lock mode (Locked, Unlocked, and several timed modes) must be available to the user if supported by the lock: grafik

What version of Home Assistant Core has the issue?

2023.1.7

What was the last working version of Home Assistant Core?

No response

What type of installation are you running?

Home Assistant OS

Integration causing the issue

Z-Wave JS

Link to integration documentation on our website

No response

Diagnostics information

No response

Example YAML snippet

No response

Anything in the logs that might be useful for us?

No response

Additional information

No response

marcelveldt commented 1 year ago

See the original issue in core for some discussion about this, the gist is that there are no configuration options missing but actual controls and sensors for door locks.

raman325 commented 1 year ago

@AlCalzone I started looking at this again. Can you review my assumptions and tell me which, if any are wrong?

  1. We need to set all DoorLock CC parameters at the same time which we will do using the setConfiguration CC command.
  2. outsideHandlesCanOpenDoor and insideHandlesCanOpenDoor are not required to be controllable by users.
  3. outsideHandlesCanOpenDoor and insideHandlesCanOpenDoor must be sent as part of the setConfiguration CC command, so presumably we can just retrieve the current value of these parameters and resend them.
  4. If the CC version is v4, only then will the following parameters be available: blockToBlock, holdAndReleaseTime, twistAssist, and blockToBlock
  5. If the CC version is v4, all four parameters listed in 4 need to be sent as part of the setConfiguration CC command.
  6. If the CC version < v3, we CANNOT include any of the parameters listed in 4 in the command.
AlCalzone commented 1 year ago
  1. correct
  2. correct
  3. correct, but I think this should be done in the driver. The serialize function already has some fallbacks if those are not arrays, so they should be just made optional and default to 0xF as recommended if no value has been set before.
  4. correct, but not all of them have to be supported by a lock. Z-Wave JS will only create the corresponding values if they are supported.
  5. No, they are still optional. They will default to 0/false though if not provided. Might make sense to read the cached values in the setConfiguration command though.
  6. You can pass them, but Z-Wave JS will not send them to the device.
raman325 commented 1 year ago

Might make sense to read the cached values in the setConfiguration command though.

Are you saying that if the v4 properties aren't included, the library should read the cached values or are you thinking about adding this to the driver?

AlCalzone commented 1 year ago

I'm not sure this is solved yet:

  1. I have no idea what to fix here:

Image

  1. Not sure how to do this:

    Setup Door Lock Mode to 'UnsecuredWithTimeout'

Image

AlCalzone commented 1 year ago

Related to 2. in the above comment, other states than locked and unlocked can also not be displayed:

Please check if current mode is set to 'UnsecuredWithTimeout'. Is the mode displayed correctly?

AlCalzone commented 7 months ago

So, I got further this time, but setting the lock configuration still fails. This is because lockTimeoutConfiguration is missing from the configuration passed to the driver, even though the checkbox is set:

Image

This field is required when the operation type is timed.

Also how is this supposed to be formatted? I can't get it to pass validation:

Image

marcelveldt commented 4 months ago

Shouldnt this be in the expert UI ?

AlCalzone commented 2 months ago

Very, very short term fix:

Better:

AlCalzone commented 2 months ago

Device diagnostics: zwave_js-01J2ZVFHVQG8V8J9D9360S4VCQ-Node 7-685335944bc32e1f4a03cdc5b3940b7d.json

MartinHjelmare commented 2 months ago

I think I've found the bug for missing lock timeout configuration. We don't include that attribute when building the dict from the dataclass that represents the lock configuration in the client library. So even though we pass the parameter from the integration it's not included in the parameters sent from the client to the server.

https://github.com/home-assistant-libs/zwave-js-server-python/blob/78c0ed02c5ffd0f57d991058eb099b82394d2a2f/zwave_js_server/const/command_class/lock.py#L177-L197

I'm working on a fix.

MartinHjelmare commented 2 months ago

Let's move the expert panel tasks to a new issue. I'll close this issue when merging the fix.

AlCalzone commented 2 months ago

Followup issue: https://github.com/zwave-js/certification-backlog/issues/48

AlCalzone commented 3 weeks ago

Still not fixed I'm afraid: Image

MartinHjelmare commented 3 weeks ago

Please paste the YAML from the YAML mode instead.

MartinHjelmare commented 3 weeks ago

We decided to remove those parameters (inside/outside handles) since they weren't required for certification as we understood it at that time. We forgot to remove it from the service description.

https://github.com/home-assistant/core/pull/103595#pullrequestreview-1725594543