OH2 Z-Wave refactoring and testing... and SECURITY

chris · June 24, 2018, 4:25pm

Thanks for investigating and finding this commonality - I’ll see if I can see any issue here…

Done.

mhilbush · June 24, 2018, 4:27pm

Was this the change to deal with devices that don’t wake up?

chris · June 24, 2018, 4:35pm

Yes, I already had a quick look at this and I don’t think it’s relevant as it’s related to the FAILED check and I think this code is now gone with the change in how this works… I need to double check though as I don’t currently have this code loaded.

chris · June 24, 2018, 5:12pm

This code is still in play, but it clearly went past this as this code would set the state to DONE if it was already initialised, and from your log, we’re in state PING which is after FAILED_CHECK, and clearly before DONE…

chris · June 24, 2018, 5:13pm

Do you have a longer log of the initialisation? I’f like to see what’s happening before it gets into this state…

mhilbush · June 24, 2018, 5:14pm

No but I’ll create one.

chris · June 24, 2018, 5:14pm

Thanks. Ping it over by email if you like…

mhilbush · June 24, 2018, 5:24pm

Starting at 2018-06-24 13:14:58.382

deleted the thing for node 72
ran discovery
added node 72 from inbox

https://drive.google.com/open?id=1p31MNpx8AmuiPK31EfJm2Bk44P5TJgqU

mhilbush · June 25, 2018, 12:36pm

One other data point…

I excluded one of the Minimotes, did a factory reset, then included it into the network. Appears to have the same behavior as above. It’s node 93 in this log.
https://drive.google.com/open?id=17wZy49pAO59SmPpO-kz9IF3d2tyzJr8I

mhilbush · June 25, 2018, 12:41pm

Also wanted to add that I’ve successfully migrated 6 other types of battery powered devices (ST814, ST812, FGFS101, FGMS001, ZW100, ZW097) – about 13 devices in total. As soon as these devices were woken up, they successfully initialized.

Lolodomo · June 26, 2018, 6:04am

Interesting that you succeeded with your ST814. Mine never worked. Did you do something special ?
If I correctly remember the analysis, it was considered as failed by the controler.

chris · June 26, 2018, 6:45am

This will not be device specific - I’d suggest to try again with the new dead code version that @mhilbush is using.

mhilbush · June 26, 2018, 10:43am

I actually have 6 of ST814’s, all of which are working well. I didn’t do anything special. They were fully initialized by the binding the first time I woke them up.

Lolodomo · June 26, 2018, 8:03pm

How do you wake them up ?

mhilbush · June 26, 2018, 8:05pm

Triple press the C/F button (2nd button from the left). You will see a little symbol flash on the LCD display indicating it is awake.

mhilbush · June 26, 2018, 8:16pm

@chris Looking at the logs I provided… Could it be that the node doesn’t indicate it supports WAKE_UP in the NIF? Then when it gets to this bit of code, it doesn’t process it because it thinks the node doesn’t support WAKE_UP (even though it obviously generated a WAKE_UP)?

github.com

openhab/org.openhab.binding.zwave/blob/development/src/main/java/org/openhab/binding/zwave/internal/protocol/ZWaveNode.java#L1349


 *
 * @param awake boolean true if the device is currently awake
 */
public void setAwake(boolean awake) {
    // Don't do anything if this node is listening
    if (listening == true || frequentlyListening == true) {
        logger.debug("NODE {}: Node is listening - ignore wakeup", getNodeId());
        return;
    }


    ZWaveWakeUpCommandClass wakeUpCommandClass = (ZWaveWakeUpCommandClass) getEndpoint(0)
            .getCommandClass(ZWaveCommandClass.CommandClass.COMMAND_CLASS_WAKE_UP);
    if (wakeUpCommandClass == null) {
        logger.debug("NODE {}: Node doesn't support WAKEUP - ignore wakeup", getNodeId());
        return;
    }


    // Create the timer if this is our first call
    if (timer == null) {
        logger.debug("NODE {}: Creating wakeup timer", getNodeId());
        timer = new Timer();

I don’t recall this check being in the master version of the binding.

chris · June 26, 2018, 8:29pm

I’ll try and take a look at this tomorrow.

Looking at the log above though, this doesn’t look the same. In that log it just shows that the command class isn’t known. This is correct since the init stage is PING - as such we haven’t received the NIF, and therefore don’t know what command classes are supported yet.

I have an idea to skip this check for battery devices - PING shouldn’t be needed for battery devices because they need to explicitly send the wakeup. I think I did this in a previous version.

This is all completely new from the old binding. The old binding had separate queues for wakeup - the dev version doesn’t. It keeps everything in a single queue to simplify the transaction management, and then has to know if the device is awake.

mhilbush · June 26, 2018, 9:08pm

So there are two logs above. The first one shows the device being discovered (it already is included in the network). The second one shows the device being included into the network.

I was looking at the second one where it looked like the NIF had been received (and it didn’t include WAKE_UP).

Of course, I might be misreading this.

mhilbush · June 26, 2018, 9:11pm

It seems a little odd that this is just being exposed given how widely the dev version is being used. Is anyone else using any of these devices with the dev version? Have you been able to discover the device?
Aeon Minimote
Aeon Wallmote
Fibaro Button
Ecolink TILTZWAVE2 Garage Door Sensor

chris · June 26, 2018, 9:11pm

I’ve not looked at the detailed logs you posted yet - just the log entry above -: