Peer app review please

All,

I create a new app and I have shared it. The app waits for a double tap on a specific switch and if you double tap down it will change a mode(arm system or sleep mode). Then it will do the opposite if you double tap up(disarm or switch to home mode). This is only my second app and I’m wondering if someone could review it to determine if I have done anything incorrectly. Thanks in advance for the feedback.

-Jeremy

A quick note: What concerns me about this app is the fact that I’m seeing this error on the console log files:

10:07:28 PM: warn No event handler found for mode event 'At Home Night'
/**
 *  Change Mode On Double Tap
 *
 *  Author: Jeremy R. Whittaker
 *  Date: 2013-10-26
 *  This mode change works great if you use a switch in your room to arm/disarm your system. 
 *  When you go to bed you can double tap down and set mode to sleeping(arm).  
 *  When you wake up you can simply double tap up to set the mode to day time(disarm) to disable your alarm.
 */
preferences {
	section("When this switch is double-tapped...") {
		input "master", "capability.switch", title: "Where?"
	}
	section("Double tap up change to this mode") {
		input "upMode", "mode", title: "Up Mode?"
	}
	section("Double tap down change to this mode") {
		input "downMode", "mode", title: "Down Mode?"
	}
	section( "Notifications" ) {
		input "sendPushMessage", "enum", title: "Send a push notification?", metadata:[values:["Yes","No"]], required:false
		input "phoneNumber", "phone", title: "Send a Text Message?", required: false
	}
}

def installed() {
	log.debug "Current mode = ${location.mode}"
	subscribe(master, "switch", switchHandler, [filterEvents: false])
	subscribe(location, modeChangeHandler)
}

def updated() {
	log.debug "Current mode = ${location.mode}"
	unsubscribe()
	subscribe(master, "switch", switchHandler, [filterEvents: false])
	subscribe(location, modeChangeHandler)
}

def switchHandler(evt) {
	log.info evt.value

	// use Event rather than DeviceState because we may be changing DeviceState to only store changed values
	def recentStates = master.eventsSince(new Date(now() - 4000), [all:true, max: 10]).findAll{it.name == "switch"}
	log.debug "${recentStates?.size()} STATES FOUND, LAST AT ${recentStates ? recentStates[0].dateCreated : ''}"

	if (evt.isPhysical()) {
		if (evt.value == "on" && lastTwoStatesWere("on", recentStates, evt)) {
			log.debug "detected two taps, disarming system"
			takeUpActions()
		} else if (evt.value == "off" && lastTwoStatesWere("off", recentStates, evt)) {
			log.debug "detected two taps, arming system"
			takeDownActions()
		}
	}
	else {
		log.trace "Skipping digital on/off event"
	}
}

private send(msg) {
	if ( sendPushMessage != "No" ) {
		log.debug( "sending push message" )
		sendPush( msg )
	}

	if ( phoneNumber ) {
		log.debug( "sending text message" )
		sendSms( phoneNumber, msg )
	}

	log.debug msg
}

def takeUpActions() {
	log.debug "changeMode, location.mode = $location.mode, newMode = $newMode, location.modes = $location.modes"
	if (location.mode != upMode) {
		if (location.modes?.find{it.name == upMode}) {
			setLocationMode(upMode)
			send "Double tap up has changed mode to '${upMode}'"
		}
		else {
			send "Double tap up has tried to change to undefined mode '${upMode}'"
		}
	}
}

def takeDownActions() {
	log.debug "changeMode, location.mode = $location.mode, newMode = $newMode, location.modes = $location.modes"
	if (location.mode != downMode) {
		if (location.modes?.find{it.name == downMode}) {
			setLocationMode(downMode)
			send "Double tap down has changed the mode to '${downMode}'"
		}
		else {
			send "Double tap down has tried to change to undefined mode '${downMode}'"
		}
	}
}

private lastTwoStatesWere(value, states, evt) {
	def result = false
	if (states) {

		log.trace "unfiltered: [${states.collect{it.dateCreated + ':' + it.value}.join(', ')}]"
		def onOff = states.findAll { it.isPhysical() || !it.type }
		log.trace "filtered:   [${onOff.collect{it.dateCreated + ':' + it.value}.join(', ')}]"

		// This test was needed before the change to use Event rather than DeviceState. It should never pass now.
		if (onOff[0].date.before(evt.date)) {
			log.warn "Last state does not reflect current event, evt.date: ${evt.dateCreated}, state.date: ${onOff[0].dateCreated}"
			result = evt.value == value && onOff[0].value == value
		}
		else {
			result = onOff.size() > 1 && onOff[0].value == value && onOff[1].value == value
		}
	}
	result
}

You haven’t defined the modeChangeHandler function, which is the callback function you define for the location mode change events.

@minollo can you confirm this is correct?

I added this code:

def modeChangeHandler(evt) {
	state.modeStartTime = now()
}
/**
 *  Change Mode On Double Tap
 *
 *  Author: Jeremy R. Whittaker
 *  Date: 2013-10-26
 *  This mode change works great if you use a switch in your room to arm/disarm your system. 
 *  When you go to bed you can double tap down and set mode to sleeping(arm).  
 *  When you wake up you can simply double tap up to set the mode to day time(disarm) to disable your alarm.
 */
preferences {
	section("When this switch is double-tapped...") {
		input "master", "capability.switch", title: "Where?"
	}
	section("Double tap up change to this mode") {
		input "upMode", "mode", title: "Up Mode?"
	}
	section("Double tap down change to this mode") {
		input "downMode", "mode", title: "Down Mode?"
	}
	section( "Notifications" ) {
		input "sendPushMessage", "enum", title: "Send a push notification?", metadata:[values:["Yes","No"]], required:false
		input "phoneNumber", "phone", title: "Send a Text Message?", required: false
	}
}

def installed() {
	log.debug "Current mode = ${location.mode}"
	subscribe(master, "switch", switchHandler, [filterEvents: false])
	subscribe(location, modeChangeHandler)
}

def updated() {
	log.debug "Current mode = ${location.mode}"
	unsubscribe()
	subscribe(master, "switch", switchHandler, [filterEvents: false])
	subscribe(location, modeChangeHandler)
}

def switchHandler(evt) {
	log.info evt.value

	// use Event rather than DeviceState because we may be changing DeviceState to only store changed values
	def recentStates = master.eventsSince(new Date(now() - 4000), [all:true, max: 10]).findAll{it.name == "switch"}
	log.debug "${recentStates?.size()} STATES FOUND, LAST AT ${recentStates ? recentStates[0].dateCreated : ''}"

	if (evt.isPhysical()) {
		if (evt.value == "on" && lastTwoStatesWere("on", recentStates, evt)) {
			log.debug "detected two taps, disarming system"
			takeUpActions()
		} else if (evt.value == "off" && lastTwoStatesWere("off", recentStates, evt)) {
			log.debug "detected two taps, arming system"
			takeDownActions()
		}
	}
	else {
		log.trace "Skipping digital on/off event"
	}
}

private send(msg) {
	if ( sendPushMessage != "No" ) {
		log.debug( "sending push message" )
		sendPush( msg )
	}

	if ( phoneNumber ) {
		log.debug( "sending text message" )
		sendSms( phoneNumber, msg )
	}

	log.debug msg
}

def takeUpActions() {
	log.debug "changeMode, location.mode = $location.mode, newMode = $newMode, location.modes = $location.modes"
	if (location.mode != upMode) {
		if (location.modes?.find{it.name == upMode}) {
			setLocationMode(upMode)
			send "Double tap up has changed mode to '${upMode}'"
		}
		else {
			send "Double tap up has tried to change to undefined mode '${upMode}'"
		}
	}
}

def takeDownActions() {
	log.debug "changeMode, location.mode = $location.mode, newMode = $newMode, location.modes = $location.modes"
	if (location.mode != downMode) {
		if (location.modes?.find{it.name == downMode}) {
			setLocationMode(downMode)
			send "Double tap down has changed the mode to '${downMode}'"
		}
		else {
			send "Double tap down has tried to change to undefined mode '${downMode}'"
		}
	}
}

private lastTwoStatesWere(value, states, evt) {
	def result = false
	if (states) {

		log.trace "unfiltered: [${states.collect{it.dateCreated + ':' + it.value}.join(', ')}]"
		def onOff = states.findAll { it.isPhysical() || !it.type }
		log.trace "filtered:   [${onOff.collect{it.dateCreated + ':' + it.value}.join(', ')}]"

		// This test was needed before the change to use Event rather than DeviceState. It should never pass now.
		if (onOff[0].date.before(evt.date)) {
			log.warn "Last state does not reflect current event, evt.date: ${evt.dateCreated}, state.date: ${onOff[0].dateCreated}"
			result = evt.value == value && onOff[0].value == value
		}
		else {
			result = onOff.size() > 1 && onOff[0].value == value && onOff[1].value == value
		}
	}
	result
}

def modeChangeHandler(evt) {
	state.modeStartTime = now()
}

Well I got this code to work consistently. However, if I arm or disarm the system I cannot immediately do the inverse. My guess is something with this code has to allow time for the hub to poll the switch to figure out it’s new current position. I’m not 100% fluent in this language but it is working good enough for the time being :slight_smile:

I’m also seeing this error, which is new, weird.
11:41:50 PM: error No signature of method: physicalgraph.app.ApplicationState.modeStartTime() is applicable for argument types: (java.lang.Long) values: [1383115310797]

Not sure what you are trying to do with modeStartTime (and I’m not going through your smartapp carefufully), but if you are trying to deal with application state, make sure you read this: https://support.smartthings.com/entries/21601924-SmartApp-Application-State
I believe the error you are getting is caused by your use of modeStartTime.

…looking at your shared application, it’s different from what you pasted above; you do:
state.modeStartTime now()
…instead of…
state.modeStartTime = now()

That caused the error you are seeing.

@Minollo I really appreciate that catch. I copied and pasted it from another program so quite honestly I’m not sure how it lost the “=” sign. I again appreciate you taking the time to review.

Did you even make any more progress on this?

I would love to integrate this app! :slight_smile:

@jpardee I believe I published the app is shared so you should be able to find it in ide. It doesn’t work 100% correctly and quite honestly I never gave it the time to perfect. It does however work.