Hi @dbadia,
I’ve tried your recommendations without success.
I’ll try a couple of more times and get the logs again.
An other question I have is if there has been any new work on the security module?
Hi @wificordon
Good timing. I’ve been working with a yale real living door lock the past few days and have squashed a few more bugs with the secure inclusion process. With the latest code, pairing has worked 4 of the last 5 times I’ve tried.
I made a couple updates to Secure Testing page:
-
The lock may chime, etc indicating that pairing is complete, but it really isn’t and you need to keep waiting.
-
If you see the “Secure inclusion complete” message, keep waiting until you see “Node advancer: loop - DONE” in the logs too. This is the critical indication that openhab is done interrogating the device to see which commands it supports.
Go ahead and pull my latest changes and give it a shot.
WIth my latest commit, I’ve also added support for setting user codes (still need to write up the instructions on this) and the protection command class.
Happy pairing
Dave
Hey @dbadia,
Awesome news!
I just bought the Yale Real Living 220 so I can help you with any testing!
I’m not in my country right now, but I’ll be back on saturday. So I’ll download the latest version of the code and test it as soon as I get back!
cheers!
W.
Hi @chris -
Have a question for you about ZWaveNodeStageAdvancer → DETAILS
Been trying to figure out a race condition in the secure pairing process for some time now.
When we enter the DETAILS stage and send RequestNodeInfo, one of my locks responds with multiple ApplicationUpdate reply messages. The mutliple replies is throwing the logic off because after the first it advances to the SECURITY stage which the starts firing off those messages.
I guess my main question is… is it normal/expected to get multiple ApplicationUpdate replies?
Full log is at https://pastebin.com/Yt22vhJ5
Here’s a visual of what’s happening:
554 20:59:25.312 N02 IdentifyNode →
599 20:59:25.377 N02 PROTOINFO ↔ PROTOINFO
607 20:59:25.384 N02 RoutingInfo →
633 20:59:25.408 N02 NEIGHBORS ↔ NEIGHBORS
662 20:59:25.422 N02 FAILED_CHECK ↔ FAILED_CHECK
674 20:59:25.426 N02 NoOperation →
684 20:59:25.430 N02 NOOP →
704 20:59:26.615 N02 PING ↔ PING
712 20:59:26.621 N02 RequestNodeInfo →
737 20:59:27.820 N02 ← ApplicationUpdate
748 20:59:27.826 N02 DETAILS ↔ DETAILS
769 20:59:27.840 N02 SECURITY_SCHEME_GET →
793 20:59:30.205 N02 ← ApplicationUpdate
808 20:59:30.256 N02 ← ApplicationUpdate
Hi Dave,
If the two updates are the same (which they seem to be), then it shouldn’t matter (should it?) since in general the application update can be sent at any time anyway. The first update should get processed and the advancer moves on to the next stage (which I think you’re saying it does?) and the second application update gets ignored (as far as the init code goes anyway).
Why is this causing the security code a problem?
Cheers
Chris
@chris
Heh, should have explained why I think I was a problem. I noticed the node was ending up with multiple copies of the same command class objects as ApplicationUpdateMessageClass.handleRequest would add them on each response. In this case, the application updates were identical and adding multiple copies of the security classes could be a problem (although since secure inclusion wasn’t complete it didn’t matter). I added some logic in handleRequest to see if the command class object already exists and only add it if it doesn’t.
As to why I think the multiple application updates are a problem… it’s just a guess as the device never responds to the very first security init message of SECURITY_SCHEME_GET. We send that, then get the 2nd and 3rd application update responses instead of SECURITY_SCHEME_RESPONSE.
The SECURITY_SCHEME_GET does get resent after the additional application updates are received, so one would think the device would just response then, but no lock so far
Thanks for your help
Dave
Hi Dave,
This shouldn’t happen - the system checks if a command class already exists, and doesn’t add it again - if it didn’t, this would be a major problem since the NIF is sent regularly by most devices - not just during initialisation. Many devices also use the NIF as a wakeup, or general notification, so it’s really quite common to have this sent…
See below - the code checks to see if the class exists, and only adds it if it doesn’t exist…
ZWaveCommandClass commandClass = ZWaveCommandClass.getInstance(data, node, zController);
if (commandClass != null) {
node.addCommandClass(commandClass);
}
If you’re seeing multiple entries, then something must be broken - have you changed how command classes are added / stored within the binding to support security?
Chris
This one still looks like a bug to me. ZWaveCommandClass.getInstance creates a new instance blindly, I see no check on the give node to see if it’s already added:
public static ZWaveCommandClass getInstance(int classId, ZWaveNode node, ZWaveController controller, ZWaveEndpoint endpoint) {
try {
CommandClass commandClass = CommandClass.getCommandClass(classId);
if (commandClass != null && commandClass.equals(CommandClass.MANUFACTURER_PROPRIETARY)){
commandClass = CommandClass.getCommandClass(node.getManufacturer(), node.getDeviceType());
}
if (commandClass == null) {
logger.warn(String.format(“NODE %d: Unknown command class 0x%02x”, node.getNodeId(), classId));
return null;
}
Class<? extends ZWaveCommandClass> commandClassClass = commandClass.getCommandClassClass();
if (commandClassClass == null) { logger.warn("NODE {}: Unsupported command class {}", node.getNodeId(), commandClass.getLabel(), classId); return null; } logger.debug("NODE {}: Creating new instance of command class {}", node.getNodeId(), commandClass.getLabel());
Constructor<? extends ZWaveCommandClass> constructor = commandClassClass.getConstructor(ZWaveNode.class, ZWaveController.class, ZWaveEndpoint.class); return constructor.newInstance(new Object[] {node, controller, endpoint}); } catch (Exception e) { logger.error(String.format("NODE %d: Error instantiating command class 0x%02x", node.getNodeId(), classId), e); }; return null; }
The only way it would return null is if the CommandClass doesn’t specify the corresponding ZWaveCommandClass. Did a diff against head, I haven’t changed that logic at all (I’m trying to only change what I need to, although you wouldn’t know it from the diffs 
)
I started looking at this because I noticed repeated “Creating new instance of command class…” log entries for the same node which was a problem for the security class.
I can submit a PR for the fix if you’d like
Dave
PS. Going back to my original issue, you were right, the multiple application updates aren’t causing any issues
Hi @dbadia,
I have good news! I just compiled and ran the latest version of the Z-Wave Binding and it worked with the Yale 220 Real Living.
I’ll try to check if the Schlage BE469 works now for me and I’ll let you know.
Do you need any logs from the tests I have done so far?
Cheers,
W.
Hey @dbadia,
I can confirm that this last build works awesome with Schlage BE468 (Edited)!
Let me know if you need help with any testing!
Cheers,
W.
You’re right of course… The only check is if initialisation has completed, so this doesn’t solve your problem… I’m actually surprised this hasn’t come up before to be honest!
Yes please
Hi @Nicholas_Waterton -
I have a Yale lock but I am not seeing the alarms as you did. Did you do any particular setup to get the Yale to send them?
Thanks
Dave
Yes,
You have to add the alarms to the binding code and recompile it. The Yale alarms aren’t in there normally. It’s described earlier in the thread (it’s easy – if you can compile a binding).
Nick Waterton P.Eng.
National Support Leader - Nuclear Medicine, PET and RP
GE Healthcare
Thanks Dave - I’ll try and merge this tomorrow…
it would be great to implement security 2 framework (S2) and OTA upgrade 
you can see the big security flaw of zwave here : http://www.networkworld.com/article/3024217/security/ez-wave-z-wave-hacking-tool-capable-of-breaking-bulbs-and-abusing-z-wave-devices.html
And thanks to work on the security feature!!!
I’ve added user code support to the security branch. Since it’s new you will have to exclude the lock, then run the secure inclusion process again for OH to recognize that the lock supports it. The secure inclusion process takes longer now, but succeeds most of the time.
After that, just follow the user code instructions
Dave
@Nicholas_Waterton the part I was missing was that I had to register the controller i the association as explained here: Z Wave Configuration · cdjackson/HABmin Wiki · GitHub
Now I am seeing the alarms appearing in my logs during lock events as you did.
@chris As Nicholas discovered, the alarm values for the Yale lock are different from those in ZWaveAlarmCommandClass.
Much to my surprise, I read that this is normal and the values can be defined by each manufacturer.
Do we currently have a section in the product database to support this? Perhaps a hackish code based solution short term and database support for OH2?
Dave
Yes, and no - but I’m not sure you need this…
Ultimately, it’s the name on the front that matters, not the smallprint on the back 
. By this I mean that even if we say that FIRE_ALARM is really DOOR_BROKEN alarm - or anything you want - the definition of the alarms in the database is effectively just telling the binding what number to look at. What people see is the label and the options etc - and we can call this whatever we want (ish!)…
The label is completely free-form within the database. The options are linked to the channel - this is currently hard coded in an XML file in the binding - we can add new channels with new options very easily, or I can do what I’ve done for thermostats where the options are also defined into the database and we make the whole thing configurable.
Does that make sense? I think it does what you want, but if not I’m sure we can find a way…
Those both sound like good approaches although I have to admit I don’t fully follow. I suppose I should look at the thermostat code and xml in OH1 to get a feel for that. Haven’t played with OH2 yet so I need to brush up on those concepts, especially channels since I think that is new. Sounds like either one would give us what we need so that’s good.
Dave