Skip to content
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

Fix for issue #997. #1576

Closed
wants to merge 1 commit into from
Closed

Fix for issue #997. #1576

wants to merge 1 commit into from

Conversation

osenft
Copy link

@osenft osenft commented Aug 10, 2018

Add support to clear UserCode by writing empty value or value consisting of NULLs only.

Add support to clear UserCode by writing empty value or value consisting of NULLs only.
@osenft
Copy link
Author

osenft commented Aug 10, 2018

Tested against a Schlage BE469 Touchscreen Deadbolt as follows:

  • Built python-openzwave with Python 3.6 using patched openzwave code.
  • Copied into Home Assistant and restarted HASS.
  • Executed lock.get_usercode on populated and unpopulated slots.
  • Executed lock.set_usercode on unpopulated slot. Checked correct result via lock.get_usercode.
  • Executed lock.clear_usercode on populated slot. Checked correct result via lock.get_usercode.
  • Manually confirmed on physical lock that code from cleared slot no longer unlocks the deadbolt lock.

@Fishwaldo
Copy link
Member

Please see my reply to #997

@osenft
Copy link
Author

osenft commented Aug 15, 2018

See comments on #997. Are we sure the proposed solution is not sufficient?

@edif30
Copy link

edif30 commented Aug 31, 2018

I have tested this PR and seems to work well on my Schlage BE469. Slot shows (Available) after clearing.

@edif30
Copy link

edif30 commented Nov 10, 2018

@Fishwaldo based on your comments...

#1576 makes the assumption that the average joe knows that 0000 or '' or NULL would clear the the code. With the above approach (as most applications iterate over available ValueID's for a command class) they would expose a new Input Box or whatnot that is called "Reset Code" or whateverand wouldn't be required to know the "special" code to clear a code. (other than the index value of what they want to clear).

I don't know that we are going to solve this problem from a standpoint of who knows about special codes or not. However I do think that anyone with knowledge of the 0000 or NULL can use this patch to their advantage. This would be the case more so than not. Local clearing of codes still work as well. So I don't think this would break anything either. If anything, it at least improves things for certain users.

edif30 added a commit to edif30/open-zwave that referenced this pull request Nov 10, 2018
@Fishwaldo
Copy link
Member

Exactly my point. If we just create a new ValueInt called "Clear User Code" then an application just has to present a user with an Input Box (or drop down list or whatever). When we get a Set on that ValueID, we clear the code internally in the class.

Then the user doesn't have to care if its ascii 0000, decimal 0000, Null etc to clear the code. Devices with quirks, we can handle with a config option inside the class.

Will applications need to be updated? Yes - But there are lots of updates in the Dev branch, so this is just another change applications will need to take care of over 1.4

@Fishwaldo
Copy link
Member

I'm working on refactoring this already. So Closing this. Watch #997

@Fishwaldo Fishwaldo closed this Dec 3, 2018
mpenick added a commit to mpenick/open-zwave that referenced this pull request May 17, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants