Added zwave lock state from alarm type workaround

[adrum]

Description:

#17386 Did not seem to fix the issues I was experiencing with my YRD220. The access_control value was set to none on my lock when events were occurring. I noticed the alarm_type value was being update on my lock. This PR adds a workaround to set lock state from the alarm_type value.

This will also probably have a conflict with #18737. I can resolve the conflict, should that PR get accepted before this one. This PR is based on the workaround implementation of both of the aforementioned pull requests.

CC: @Juggler00

Checklist:

• The code change is tested and works locally.
• Local tests pass with tox. Your PR cannot be merged unless tests pass
• There is no commented out code in this PR.

[ahayworth]

So - it looks like there are times when we expect WORKAROUND_DEVICE_STATE to fail - because the value of access_control is None?

I think you can actually simplify this PR quite a bit, if that's so. Here's what I was thinking:

1. You don't actually have to track did_use_state_workaround at lines 259 and 267. If access_control is really None, then this test at line 262 should fail, and we'll never get around to setting did_use_state_workaround = True at line 267. So those could be removed.

2. I noticed that in your tests, you sent events with codes [18, 19, 21, 22, 24, 25, 27]. All of those codes are already listed in LOCK_STATUS and ALARM_TYPE_STD, at lines 94 and 111. We can use those to simplify further. You could actually delete lines 289-299 entirely, and instead extend the alarm checks further on. I think at line 300 we could restructure the _lock_status like so:

        if str(alarm_type) in ALARM_TYPE_STD:
          if self._alarm_type_workaround:
            self._state = LOCK_STATUS.get(str(alarm_type))
            _LOGGER.debug("workaround: lock state set to %s -- alarm type: %s",
                          self._state, str(alarm_type))

          if alarm_type == 21:
            alarm_type_str = MANUAL_LOCK_ALARM_LEVEL.get(str(alarm_level))
          else: 
            alarm_type_str = str(alarm_level)

          self._lock_status = '{}{}'.format(LOCK_ALARM_TYPE.get(str(alarm_type)),
                                                           alarm_type_str)
          return

        if alarm_type == 161:
...etc

The really nice side effect of streamlining it all like this, is that our PRs should merge more cleanly. :)

(edit: to be clear though, I don't think you have to - this looks like it works as-is to me. 😄 )

[adrum]

@ahayworth Thanks for the input. I implemented 1, but I think implementing 2 as is would cause issues with my lock, since the codes in LOCK_STATUS are more inclusive. Some states would be dropped since they are not in ALARM_TYPE_STD. It would also prevent the _lock_status property from being set around 307.

The conflicts I imagine the 2 PRs to have are more with the setup of the workarounds, rather than their specific implementations. Specifically line 34 and probably a few more between lines 233 to 250ish.

[Juggler00]

It doens’t look like you’ve added the code to handle the YRD210. Should I do some testing first before you add?

[Juggler00]

Just wondering how you got HA to identify your "Vision" YRD220 as a lock? Looking through /lib/python3.6/site-packages/python_openzwave/ozw_configmanufacturer_specific.xml, the only references to both the YRD210 and 220 are:

<Manufacturer id="0129" name="Assa Abloy">
  <Product type="0001" id="0000" name="Yale Touchscreen Lever (YRL220)" config="assa_abloy/TouchLever.xml"/>
  <Product type="0002" id="0000" name="Yale Touchscreen Deadbolt (YRD220)" config="assa_abloy/TouchDeadbolt.xml" />
  <Product type="0004" id="0000" name="Yale Push Button Deadbolt (YRD210)" config="assa_abloy/PushButtonDeadbolt.xml"/>
  <Product type="0004" id="0209" name="Yale Push Button Deadbolt (YRD210)" config="assa_abloy/PushButtonDeadbolt.xml"/>
  <Product type="0004" id="aa00" name="Yale Push Button Deadbolt (YRD210)" config="assa_abloy/PushButtonDeadbolt.xml"/>

Under the Vision section, there are no locks:
<Manufacturer id="0109" name="Vision">

[adrum]

@Juggler00 Honestly, I have no idea. I paired the locked with my Z-Wave network, and thats what the manufacturer was. I tried removing the node and adding it again, but it still showed up as a Vision device. If I recall, it didn't show up as a lock initially. I think each node communicates its capabilities to the controller on the initial pair sequence. I also know you need the network_key key in your configuration.yaml to work properly with secure devices. I also added the network key in options.xml as well. If you post what's in your zwcfg for your lock, I can add the workaround for your device.

[Juggler00]

@adrum Very odd... here is what is in my zwcfg for my lock:

<Node id="2" name="" location="" basic="4" generic="64" specific="3" type="Secure Keypad Door Lock" listening="false" frequentListening="true" beaming="true" routing="true" max_baud_rate="40000" version="4" secured="true" query_stage="Complete">
    <Manufacturer id="109" name="Vision">
        <Product type="4" id="0" name="Yale Push Button Deadbolt (YRD210)" />
    </Manufacturer>

Let me know if you need anything further or any testing done.

[cdkonecny]

There are additional Kwikset IDs that have not been covered.
For instance the 914 Converts are ID 0446. These had been working with @mtreinish workaround at one point.

[mtreinish]

@cdkonecny hmm, the conversion kit has as a separate id? Well, it's easy enough to add that to the workaround list so those locks are covered by the device state workaround. (maybe in a separate PR since it's sorta unrelated to what @adrum is fixing here)

[cdkonecny]

Yes, I tested by adding ID 0446 to the list as a custom component and it fixed them right up. I apologize for not posting in the best spot. The more applicable thread had been shut down.

…

On December 6, 2018 9:41:56 AM Matthew Treinish ***@***.***> wrote: @cdkonecny hmm, the conversion kit has as a separate id? Well, it's easy enough to add that to the workaround list so those locks are covered by the device state workaround. (maybe in a separate PR since it's sorta unrelated to what @adrum is fixing here) — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or mute the thread.

[adrum]

@Juggler00 I believe your device matches mine based on your zwcfg, so feel free to pull this down and test it.

[adrum]

@turbokongen After resolving all the merge conflicts from #18737, I believe this PR is ready for review. I know you and @ahayworth were discussing a refactor, but I'm hoping this could get merged and released before the refactor happens.

[ahayworth]

I know you and @ahayworth were discussing a refactor, but I'm hoping this could get merged and released before the refactor happens.

I had volunteered for it but won't realistically have time to work on it until after the new year. Don't let it stop you. 😄

[adrum]

@turbokongen Any chance you can review this?

[EionRobb]

Is it possible to modify configuration.yaml to enable other devices to use this workaround? My YRD240 (manuf. id 0x129, prod. id 0x209) suffers from the same problem, and I'd like to be able to use the workaround without modifying the hassio docker image

[MartinHjelmare]

If you need help please use our help channels:
https://home-assistant.io/help/#communication-channels

Merged PRs should not be used for support or bug reports. Thanks!