Code Cleanup HELP! ;)

Ok,

got it working. My only issue now is “sendNotification("alerts@cidcomm.com”, Notification_Proxy.state.toString)" isn’t working. I do not use my.openhab which i think this is needed for? If so can i do something like “sendMail("alerts@cidcomm.com”, Notification_Proxy.state.toString)" instead?

ok, figured that out also… Grasshopper is learning lol

sendMail("alerts@cidcomm.com", Notification_Proxy.state.toString, Notification_Proxy.state.toString)

Run it and see but so far it looks good.

A switch statement is an alternate way to do a bunch of if/else statements. So

switch(name.get(2)) {
    case "Battery": Notification...
    case "Minimote": Notification...
}

is the exact same functionally as

if(name.get(2) == "Battery") Notification...
else if(name.get(2) == "Minimote") Notification...

In short, it is looking for an exact match.

You would have to do it individually for each device.

Correct, that is how you send a notification through my.openhab. You need to change it to your preferred method of sending alerts (e.g. sendmail).

1 Like

Thanks! This is what i did so far and it seems to work perfectly…

rule "Process Battery Level Updates"
when
        Item gBattery received update
then
    gBattery.members.forEach[ battery |
        if((battery.state as DecimalType) < 10) {
            val name = battery.name.split("_")
            switch(name.get(1)){
                case "Kitchen":  Notification_Proxy.postUpdate("Replace the " + name.get(1) + " " + name.get(2) + " " + name.get(3) + " " + "Battery, " + battery.state.toString + "% left!")
                case "LivingRoom":  Notification_Proxy.postUpdate("Replace the " + name.get(1) + " " + name.get(2) + " " + name.get(3) + " " + "Battery, " + battery.state.toString + "% left!")
                case "BedRoom":  Notification_Proxy.postUpdate("Replace the " + name.get(1) + " " + name.get(2) + " " + name.get(3) + " " + "Battery, " + battery.state.toString + "% left!")
                case "Hall":  Notification_Proxy.postUpdate("Replace the " + name.get(1) + " " + name.get(2) + " " + name.get(3) + " " + "Battery, " + battery.state.toString + "% left!")
 }
         }
    ]
end





rule "Process Tamper Updates"
when
        Item gTamper changed from CLOSED to OPEN
then
    gTamper.members.forEach[ tamper |
        if((tamper.state == OPEN) {
            val name = tamper.name.split("_")
            switch(name.get(1)){
                case "Kitchen":  Notification_Proxy.postUpdate("" + name.get(1) + " " + name.get(2) + " " + name.get(3) + " " + "Tamper Alert!, " + tamper.state.toString + "")
                case "LivingRoom":  Notification_Proxy.postUpdate("" + name.get(1) + " " + name.get(2) + " " + name.get(3) + " " + "Tamper Alert!, " + tamper.state.toString + "")
                case "BedRoom":  Notification_Proxy.postUpdate("" + name.get(1) + " " + name.get(2) + " " + name.get(3) + " " + "Tamper Alert!, " + tamper.state.toString + "")
                case "Hall":  Notification_Proxy.postUpdate("" + name.get(1) + " " + name.get(2) + " " + name.get(3) + " " + "Tamper Alert!, " + tamper.state.toString + "")
 }
         }
    ]
end

I’m working on this now, like you said i have multiple groups so i may need to make two of these if this don’t work out… So far not working at all lol

rule "Process Intrusion Updates"
when
//        Item gMotionSensors changed from CLOSED to OPEN or
        Item gDoorSensors changed from CLOSED to OPEN
then
//    gMotionSensors.members.forEach[ sensor |
    gDoorSensors.members.forEach[ sensor |
        if((sensor.state == OPEN) {
//         if (AlarmSysStatus.state == 1)
            val name = sensor.name.split("_")
            switch(name.get(1)){
                case "Kitchen":  Notification_Proxy.postUpdate("" + name.get(1) + " " + name.get(2) + " " + name.get(3) + " " + "Intrusion Alert!, " + sensor.state.toString + "")
                case "LivingRoom":  Notification_Proxy.postUpdate("" + name.get(1) + " " + name.get(2) + " " + name.get(3) + " " + "Intrusion Alert!, " + sensor.state.toString + "")
                case "BedRoom":  Notification_Proxy.postUpdate("" + name.get(1) + " " + name.get(2) + " " + name.get(3) + " " + "Intrusion Alert!, " + sensor.state.toString + "")
                case "Hall":  Notification_Proxy.postUpdate("" + name.get(1) + " " + name.get(2) + " " + name.get(3) + " " + "Intrusion Alert!, " + sensor.state.toString + "")
 }
         }}
    ]
end
rule "Process Intrusion Updates"
when
    Item gMotionSensors changed from CLOSED to OPEN or
    Item gDoorSensors changed from CLOSED to OPEN
then
    val motionLatest = gMotionSensors.members.sortBy[lastUpdate].last
    val doorLatest = gDoorSensors.members.sortBy[lastUpdate].last

    val latest = if(motionLatest.lastUpdate.after(doorLatest.lastUpdate) motionLatest else doorLatest

    val name = latest.name.split("_")
    Notification_Proxy.postUpdate("" + name.get(1) + " " + name.get(2) + " " + name.get(3) + " Intrusion Alert!, " + latest.state.toString)
end

I needed the switch statement in my example above because I had a different message I wanted to send if the battery for the Minimote was low verses the ZCOMBO. Since the static parts of all of your messages are the same you don’t need the switch.

Yea this wont work if any doors are already open on nice days and such. Have to employ your previous code you posted on this :wink:

Awesome. Yea the only time i need the alerts is when all doors etc are closed and the system is armed so i think the groups are fine cause all there states will be “CLOSED”

How can i add

if (AlarmSysStatus.state != 1

To your last post as i want it to only work if the alarm system state is NOT set to 1 or “DISARMED” which will cover my other two states 2 and 3 for away and stay.

actually looking at the sequence i would need to do:

if (AlarmSysStatus.state == 2 || if (AlarmSysStatus.state == 3

for away mode, and

if (AlarmSysStatus.state == 3

for stay mode.

so Essentially:

rule "Process Away Mode Intrusion Updates"
when
//    Item gMotionSensors changed from CLOSED to OPEN or
//    Item gDoorSensors changed from CLOSED to OPEN
then
    **if (AlarmSysStatus.state == 2 || if (AlarmSysStatus.state == 3)**
    val motionLatest = gMotionSensors.members.sortBy[lastUpdate].last
    val doorLatest = gDoorSensors.members.sortBy[lastUpdate].last

    val latest = if(motionLatest.lastUpdate.after(doorLatest.lastUpdate) motionLatest else doorLatest

    val name = latest.name.split("_")
    Notification_Proxy.postUpdate("" + name.get(1) + " " + name.get(2) + " " + name.get(3) + " Intrusion Alert!, " + latest.state.toString)
end

and

 rule "Process Stay Mode Intrusion Updates"
when
//    Item gMotionSensors changed from CLOSED to OPEN or
//    Item gDoorSensors changed from CLOSED to OPEN
then
    **if (AlarmSysStatus.state == 2)**
    val motionLatest = gMotionSensors.members.sortBy[lastUpdate].last
    val doorLatest = gDoorSensors.members.sortBy[lastUpdate].last

    val latest = if(motionLatest.lastUpdate.after(doorLatest.lastUpdate) motionLatest else doorLatest

    val name = latest.name.split("_")
    Notification_Proxy.postUpdate("" + name.get(1) + " " + name.get(2) + " " + name.get(3) + " Intrusion Alert!, " + latest.state.toString)
end

Just add if statements as described in my original response to this thread.

The code in this rule goes inside the if’s brackets. NOTE: I noticed a typo in my original posting so I’m including the whole rule.

then
    if((AlarmSysStatus.state as DecimalType) == 2 || (AlarmSysStatus.state as DecimalType) == 3) {
        val motionLatest = gMotionSensors.members.sortBy[lastUpdate].last
        val doorLatest = gDoorSensors.members.sortBy[lastUpdate].last

        val latest = if(motionLatest.lastUpdate.after(doorLatest.lastUpdate)) motionLatest else doorLatest

        val name = latest.name.split("_")
        Notification_Proxy.postUpdate("" + name.get(1) + " " + name.get(2) + " " + name.get(3) + " Intrusion Alert!, " + latest.state.toString)
    }
end

If statements are VERY basic to writing rules. Please review the Rule’s wiki page, the Script’s wiki page and Xtend documentation and look at examples.

However, in your original code you only want door sensors to trigger when the Alarm is in state 3. So the rule needs to be something like:

then
    val alarm = AlarmSysStatus.state as DecimalType

    // Ignore the event unless alarm is in state 2 or 3
    if(alarm == 2 || alarm == 3) {

        // Get the latest triggered contact from both groups
        val motionLatest = gMotionSensors.members.sortBy[lastUpdate].last
        val doorLatest = gDoorSensors.members.sortBy[lastUpdate].last

        // Determine which of the two is the most recent (i.e. which Item triggered the rule)
        val latest = if(motionLatest.lastUpdate.after(doorLatest.lastUpdate)) motionLatest else doorLatest

        // Only send the alert if the latest is a door or if alarm == 2 and latest is a motion detector
        // In other words always send an alert if the door opens, only send an alert for motion detectors if alarm is 2
        if(latest == doorLatest || (alarm == 2 && latest == motionLatest)) {
            val name = latest.name.split("_")
            Notification_Proxy.postUpdate("" + name.get(1) + " " + name.get(2) + " " + name.get(3) + " Intrusion Alert!, " + latest.state.toString)
        }
    }
end

Personally I would probably simplify this by splitting it into two separate rules and maybe use a lambda. It will be simpler to understand and not really use more code to write.

import org.openhab.core.items.*
import org.eclipse.xtext.xbase.lib.*

val Functions$Function1 intrusion = [GroupItem g |
    val latest = g.members.sortBy[lastUpdate].last
    val name = latest.name.split("_")
    Notification_Proxy.postUpdate(name.get(1) + " " + name.get(2) + " " + name.get(3) + " Intrusion Alert!, " + latest.state.toString)
]

rule "Process door intrusion updates"
    Item gDoorSensors changed from CLOSED to OPEN
then
    val aState = AlarmSysStatus.state as DecimalType
    if(aState == 2 || aState == 3) {
        intrusion.apply(gDoorSensors)
    }
end

rule "Process motion intrusion updates"
    Item gMotionSensors changed from CLOSED to OPEN
then
    if((AlarmSysStatus.state as DecimalType) == 2) {
        intrusion.apply(gMotionSensors)
    }
end

With this approach all that messy nested if business goes away and it is very clear what will happen when.

You my friend have way too many tricks up your sleeve lol… I like that last part alot. I replicated it like this,

val Functions$Function1 intrusion = [GroupItem g |
    val latest = g.members.sortBy[lastUpdate].last
    val name = latest.name.split("_")
    Notification_Proxy.postUpdate(name.get(1) + " " + name.get(2) + " " + name.get(3) + " Intrusion Alert!, " + latest.state.toString)
]
val Functions$Function1 tamper = [GroupItem g |
    val latest = g.members.sortBy[lastUpdate].last
    val name = latest.name.split("_")
    Notification_Proxy.postUpdate(name.get(1) + " " + name.get(2) + " " + name.get(3) + " Tamper Alert!, " + latest.state.toString)
]
val Functions$Function1 smoke = [GroupItem g |
    val latest = g.members.sortBy[lastUpdate].last
    val name = latest.name.split("_")
    Notification_Proxy.postUpdate(name.get(1) + " " + name.get(2) + " " + name.get(3) + " Co/Smoke Alert!, " + latest.state.toString)
]

Works very well and way easier.

Try:

val Functions@Function2 alert = [GroupItem g, String type |
    val latest = g.members.sortBy[lastUpdate].last
    val name = latest.name.split("_")
    Notification_Proxy.postUpdate(name.get(1) + " " + name.get(2) + " " + name.get(3) + " " + type + " Alert!, " + latest.state.toString)
]

...
    alert.apply(gMotionSensors, "Intrusion")
...
    alert.apply(gTamperSensors, "Tamper")
...
    alert.apply(gSmoke, "CO/Smoke")

When possible, DRY: Don’t Repeat Yourself.

1 Like

Done!, Thanks. Very nice and clean! That’s one thing i’ve come to love about OH is the ability to write this stuff a million different ways. :grin:

So here’s a question, I added your updated code right under my imports and when i do it stopped all my rules from working. When i remove it the other rules work fine. Is their a typo in it or am i putting it in the wrong place?

That’s where i had the previous 3 vals we replaced and they all worked fine…

import org.openhab.core.library.types.*
import org.openhab.core.persistence.*
import org.openhab.model.script.actions.*
import org.openhab.core.library.types.*
import org.joda.time.DateTime
import org.openhab.core.items.*
import org.eclipse.xtext.xbase.lib.*
import org.openhab.model.script.actions.Timer
var Timer timer = null
val Functions@Function2 alert = [GroupItem g, String type |
    val latest = g.members.sortBy[lastUpdate].last
    val name = latest.name.split("_")
    Notification_Proxy.postUpdate(name.get(1) + " " + name.get(2) + " " + name.get(3) + " " + type + " Alert!, " + latest.state.toString)
]


What do your logs say?

Open it in Designer and see if it is highlighting something as being an error.

A lambda is a global variable so it is in the right place.

I don’t use designer and all that. I use the cli for everything.

Logs don’t say anything in debug, items and sitemaps still work just the entire rules file locks up and nothing in it works. If I comment out that val then were back to normal.

found it! changed the @ sign to $ like the other var’s were and all is good!

You are doing a disservice to yourself by not using Designer. The poorly described errors that OH generates coupled with the uneven coverage of the documentation makes Designer a must have tool for rules development. You can solve errors like these in seconds or even better, before you even save the file. You can also explore and discover the language through the <ctrl><space> shortcut which will give you the valid completions for the current line which comes in handy when trying to remember or figure out what methods a Class has.

As a case in point, OH throws a Null exception. That exception could be caused by:

  • typo in the Item name in your rule
  • the Item’s state is undefined
  • you are trying to postUpdate or sendCommand with data the Item cannot convert to its internally supported state
  • you need to cast the state to DecimalType or the like in order to use it like you want to

I’m sure there are more. One exception, at least four potential causes. But if you use Designer it will tell you immediately, as you type, if you have most of these errors before you even save the file.

Using Designer will save you many hours of time. And there are tons of ways you can set it up on a desktop to edit or share the configs with a headless server.

  • Samba share the configurations folder
  • check the config folder into git and push and pull the changes
  • rsync

Yea i hear ya. Makes sense. Once I migrate over to my new host device ill just share the config folders and edit them with designer on windows and check it out.

Thanks a lot for all your help. I really do appreciate it!! :wink: I’ll stop bothering ya for awhile haha :wink:

very intresting topic as i’m looking forward to do the same.
would you be so kind to share your complete setup for this ?
i would like to take a look at the rules and the item files.
Currently i allrady have an alarm setup with openhab but i would like to improve the detection as to know what triggerd the alarm.
Inspecting your code this seems possible.