Problems with runIn method


(Jon Wallace) #1

Hi Folks,

I am writing a custom switch device handler and want to include an auto timeout which will turn the device off. I created a handler based off the Z-Wave Switch template and modified the on() and off() methods to be the following:

def on() {

	log.debug ("Switch Turn On")

	delayBetween([
	    zwave.basicV1.basicSet(value: 0xFF).format(),
		zwave.switchBinaryV1.switchBinaryGet().format(),
	    runIn(10, off)
        ])
    
}

and…

def off() {

	log.debug ("Switch Turn Off")

	delayBetween([
		zwave.basicV1.basicSet(value: 0x00).format(),
		zwave.switchBinaryV1.switchBinaryGet().format(),
	])
    
}

However this doesn’t appear to work. When I turn the device on, I get the log entry and indeed the device turns on. Then, 10 seconds later, I see the log entry from the off() method but the device does not turn off.

What am I doing wrong? The off() method is getting called from the timer but it doesn’t actually seem to turn the device off.

Thanks in advance for any help.

Jon


(Jon Wallace) #2

Doing some further research, and coming across this post, I modified the methods to the following:

def on() {

	log.debug "Turning Device On Programatically"

	def zwaveCmds = [
    	new physicalgraph.device.HubAction(zwave.basicV1.basicSet(value: 0xFF).format()),
		new physicalgraph.device.HubAction(zwave.basicV1.basicGet().format())
    ]
    
    sendHubCommand(zwaveCmds)
    
    if (( null != prefAutoTimeout ) && ( 0 != prefAutoTimeout )) {
    
    	log.debug "Activating Automatic Timeout At ${prefAutoTimeout} seconds"
        runIn(prefAutoTimeout, off)
    
    }
    
}

and

def off() {

	log.debug "Turning Device Off Programatically"

	def zwaveCmds = [
    	new physicalgraph.device.HubAction(zwave.basicV1.basicSet(value: 0x00).format()),
		new physicalgraph.device.HubAction(zwave.basicV1.basicGet().format())
    ]
    
    sendHubCommand(zwaveCmds)
    
}

Now, this does appear to work but it strays away so much from the code in the templates, and also the code that many of the other device handlers use that it doesn’t feel right…

So my follow up question is whether or not the re-writen methods are acceptable or am I doing something completely wrong?

Thanks again,
Jon


(ActionTiles.com co-founder Terry @ActionTiles; GitHub: @cosmicpuppy) #3

When it comes to SmartThings, my attitude is “if it works, don’t ask to find a way that doesn’t work”.

In other words, there may be known bugs as to why the first method doesn’t work (my brain has some vague recollection of reading something a year or more ago…), and there is so much different in the second method that you haven’t isolated the possible bug.

But in these situations, I’ve always found it best to just let it work and move on. Saves a lot of pain that way.


(Kevin) #4

I agree with @tgauchat that if it works, it’s acceptable, but I personally don’t like adding bug workarounds into a method that only needs it for a particular situation.

The only time the off command doesn’t work is when it’s executed from a schedule so I’d schedule a different method and put the workaround in that method instead.

The original on command might have worked, but I don’t recommend adding statements that aren’t commands inside of delayResponse.

I would have written it like:

def on() {
	log.debug ("Switch Turn On")
	
	if (prefAutoTimeout) { // This "if" does same thing as yours
	
		log.debug "Activating Automatic Timeout At ${prefAutoTimeout} seconds"
		runIn(prefAutoTimeout, autoOff)    
	}

	delayBetween([
		zwave.basicV1.basicSet(value: 0xFF).format(),
		zwave.switchBinaryV1.switchBinaryGet().format()
	])
}

void autoOff() {
	def cmds = []
	off().each {
		cmds << new physicalgraph.device.HubAction(it)
	}
	sendHubCommand(cmds)
}


def off() {
	log.debug ("Switch Turn Off")

	delayBetween([
		zwave.basicV1.basicSet(value: 0x00).format(),
		zwave.switchBinaryV1.switchBinaryGet().format(),
	])    
}

(ActionTiles.com co-founder Terry @ActionTiles; GitHub: @cosmicpuppy) #5

I kinda wondered about that and was going to suggest trying it outside; but @B69CA came up with a solution before I posted… :grin:


(Jon Wallace) #6

Hey Folks,

Firstly, thank you for your responses, the help is much appreciated. The tech in me couldn’t let this go as a simple bug or with a hacky workaround, I really needed to get to the bottom of it, and I believe I have. I don’t actually think this is a bug but in fact the way it works. I am putting the detail here in case anyone else comes across this issue, sorry if you already know this.

So firstly, the on() and off() methods within a device handler don’t seem to be treated strictly as methods in the true sense that they are called and expected to do something and then return. In fact, the purpose of these methods is simply to return either a single, or collection of commands in an array.

In the code example given here, the on() method is simply returning an array of zwave commands. Note the lack of a return() method which is not needed providing there is nothing after what is the desired return.

def on() {
        delayBetween([
                zwave.basicV1.basicSet(value: 0xFF).format(),
                zwave.basicV1.basicGet().format()
        ], 5000)  // 5 second delay for dimmers that change gradually, can be left out for immediate switches
}

If I modify that code slightly to print some debug messages, we can see what the output of the delayBetween() method is actually doing:

def on() {

	def cmds = delayBetween([
    	zwave.basicV1.basicSet(value: 0xFF).format(),
        zwave.basicV1.basicGet().format()
        ], 5000)  // 5 second delay for dimmers that change gradually, can be left out for immediate switches
        
        log.debug(cmds)
}

This gives me the following debug output:

8:05:36 PM: debug [2001FF, delay 5000, 2002]

So basically, what the delayBetween() method is doing, it pretty much the same thing as the physicalgraph.device.HubAction() class constructor does, expect it adds a delay in-between the commands. See the following code for an example:

def on() {

	def cmds = [
    	new physicalgraph.device.HubAction(zwave.basicV1.basicSet(value: 0xFF).format()),
    	new physicalgraph.device.HubAction(zwave.basicV1.basicGet().format())
    ]
    
    log.debug(cmds)

}

This gives me the following debug output:

8:09:25 PM: debug [2001FF, 2002]

As you can see, its the same commands except that the delayBetween() method simply adds the delay in the middle. In fact, this is noted in the developer documentation here.

There is a shorthand provided to create command objects: zwave.basicV1.basicSet(value: 0xFF) is the same as new physicalgraph.zwave.commands.basicv1.BasicSet(value: 0xFF). Note the different capitalization of the command name and the ‘V’ in the command class name.

So I now understand a couple of things. First, the reason that the default on() and off() methods work when called from the UI is because whatever code is calling the on() or off() method is simply using these methods to create an array of commands which I assume it is then processing using something like the sendHubCommand() method.

This also explains why calling the on() or off() method directly, from another method or something like the runIn() method doesn’t actually do anything, its because the function is doing nothing but returning an array; its not actually sending any commands to the switch.

This makes sense because the various zwave message formatting methods do the same exact thing.

With all of this in mind, I now understand why my solution worked, but also why it wasn’t desirable. If the on() method is supposed to just return an array, then its best to let it return that array as under the hood, whatever is calling it may now or at some point in the future do something more with the data.

As was suggested by the fine gentlemen @tgauchat and @krlaframboise in this post, its best to create a new method to turn on the switch programmatically, and another one that handles the sending the commands to the hub itself.

The way in which the device handlers are architected, in that they are an instance of a class which is then created by some other code, makes a lot of sense and is very similar to how iOS applications are architected with methods for example drawing tables.

Again, thanks for you help, and hopefully this long text helps someone else.

Cheers,
Jon


(ActionTiles.com co-founder Terry @ActionTiles; GitHub: @cosmicpuppy) #7

Excellent observations and analysis, Jon!


(Kevin) #8

I agree with your explanation about the on and off commands just being methods that returns zwave commands, but if those commands are returned from the first method called SmartThings is supposed to automatically send them to the device.

If the UI executes the refresh command and the refresh command returns the result of another method that returns the result of the off command, the result of the off command will get sent to the device without having to use sendHubAction.

When the UI or a SmartApp executes a command the result is automatically sent to the device.

When SmartThings automatically executes commands like poll and ping the result is automatically sent to the device.

The result of the updated method and the parse method doesn’t get sent to the device automatically, but it will if you wrap the returned zwave commands with the “response” method.

A method executed by a schedule should either send the commands automatically or require you to wrap them with response, but you shouldn’t have to manually use sendHubAction.

Not sending commands returned from a method executed by a schedule doesn’t fit the pattern of all the other entry points which is why I consider this a bug and manually using sendHubAction the workaround.