[SOLVED] Is there a better way to code this?

Hi everyone,

I have two ZWave water sensors installed in my home and have created the rule below, although the code works fine I don’t believe I’m being very efficient. So I’m looking for any advice on how I achieve the same but maybe with less code? If that’s possible or have do I just stick with this code?

rule "Water Alarms"
when
	Item Kitchen_ST812_Flood_Alarm changed or
	Item Study_ST812_Flood_Alarm changed
then
	var String Water_Alarm = null
	if(Kitchen_ST812_Flood_Alarm.state == ON) {
		Water_Alarm = "Your attention please! Water has been detected in the Kitchen."
		notifyMyAndroid("Water Alarm", "Water has been detected in the Kitchen")
	} else if(Kitchen_ST812_Flood_Alarm.state == OFF) {
		Water_Alarm = "Kitchen water alarm! has now cleared."
	} else if(Study_ST812_Flood_Alarm.state == ON) {
		Water_Alarm = "Attention please! Water has been detected in the Study."
		notifyMyAndroid("Water Alarm", "Water has been detected in the Study")
	} else if(Study_ST812_Flood_Alarm.state == OFF) {
		Water_Alarm = "Study water alarm! has now cleared."
	}
	setMasterVolume(new PercentType(50))
	say(Water_Alarm)
end

There are many different ways to do that. One solution

rule "Kitchen_ST812_Flood_Alarm ON"
when
	Item Kitchen_ST812_Flood_Alarm changed to ON
then
	setMasterVolume(new PercentType(50))
	say("Your attention please! Water has been detected in the Kitchen.")
	notifyMyAndroid("Water Alarm", "Water has been detected in the Kitchen")
end

rule "Kitchen_ST812_Flood_Alarm OFF"
when
	Item Kitchen_ST812_Flood_Alarm changed to OFF
then
	setMasterVolume(new PercentType(50))
	say("Kitchen water alarm! has now cleared.")
end

rule "Study_ST812_Flood_Alarm ON"
when
	Item Study_ST812_Flood_Alarm changed to ON
then
	setMasterVolume(new PercentType(50))
	say("Attention please! Water has been detected in the Study.")
	notifyMyAndroid("Water Alarm", "Water has been detected in the Study")
end

rule "Study_ST812_Flood_Alarm OFF"
when
	Item Study_ST812_Flood_Alarm changed to OFF
then
	setMasterVolume(new PercentType(50))
	say("Study water alarm! has now cleared.")
end

If you implement your code as you have it there you will never have a situation where you see the alarm for the study. Since the Kitchen alarm will always either be ON or OFF your if statement will never get to the bottom two cases. Unless the kitchen alarm state is NULL.

I would recommend structuring your Rule according to Design Pattern: How to Structure a Rule and some String manipulation which will result in something like:

rule "Water Alarms"
when
    Item Kitchen_ST812_Flood_Alarm changed or
    Item Study_ST812_Flood_Alarm changed
then
    // 1. Decide if the Rule needs to run
    if(previousState == NULL) return; // we don't run on changes from the NULL state

    // 2. Calculate what needs to be done
    val room = triggeringItem.name.split("_").get(0)
    val sayMsg = if(triggeringItem.state == ON) "Water has been detected in the " + room else room + " water alarm! has now cleared."
    val nma = if(triggeringItem.state == ON) "Attention please! Water has been detected in the " + room else ""

    // 3. Do it
    if(nma != "") notifyMyAndroid("Water Alarm", nma)
    setMasterVolume(new PercentType(50)
    say(sayMsg)
end

It isn’t really fewer lines of code, but the code is more generic so if you needed to add more alarms later all you have to do is add them as triggers to the Rule. Or even better, create a Group and change the Rule trigger to Member of WaterAlarms changed (assuming you name the Group “WaterAlarms”).

It is also more maintainable in the future because if you ever want to change from using NMA to something else, or you have other things you want to do when the alarm is on, you only have to do it on ine place rather than the 2-4 places you would have to make changes in the current Rule.

I’ll also point you to Design Pattern: Separation of Behaviors which you can use to centralize your NMA and alerting because I’m going to guess this isn’t the only Rule you have where you want say and NMA alerts.

Then your Rules become something like the following:

String InfoAlert
String AlarmAlert

Group:Switch WaterAlarms
Switch Kitchen_ST812_Flood_Alarm (WaterAlarms)
Switch Study_ST812_Flood_Alarm (WaterAlarms)
rule "Alert!"
when
    Item InfoAlert received command or
    Item AlarmAlert received command
then
    setMasterVolume(new PercentType(50))
    say("Attention please! " + receivedCommand)

    if(triggeringItem.name == "AlarmAlert") notifyMyAndroid(receivedCommand)
end

rule "Water Alarms"
when
    Member of WaterAlarms changed
then
    // 1. Decide if the Rule needs to run
    if(previousState == NULL) return; // we don't run on changes from the NULL state

    // 2. Calculate what needs to be done
    val room = triggeringItem.name.split("_").get(0)
    val alertMsg = if(triggeringItem.state == ON) "Water has been detected in the " + room else room + " water alarm! has now cleared."

    // 3. Do it
    if(triggeringItem.state == ON) aAlert.sendCommand(alertMsg)
    else aInfo.sendCommand(alertMsg)
end

Not a significant savings in code if you are only alerting in this one Rule. But if more Rules you have that alert the more savings you get. Also, if you decide to use some other service instead of NMA or want to have more complex rules (e.g. I use a one alerting service during the day and differnt one at night) you only have to code that in one place.

Thanks @rlkoshak,

This is exactly what I was looking for. Thanks for all your help.

Regards,

Garry