Help combining rules with the same trigger

I have two working rules that have the same trigger but do different things. I would like to combine them into 1 rule. I have tried over and over again but have trouble with the timer not resetting to null.
The scenario is- A Ring doorbell detects motion and turns on a virtual switch in openhab2 called “Motion”. Then depending on the state of another virtual switch called “Watchdog” and the time of day, then things happen.
Any advice appreciated.

var Timer timer = null
rule "Motion Warning"
when
	Item Motion received command ON
then
	if (Watch_Dog.state == ON) {
	sendCommand(HeyGoogle_Volume, 80)
	playSound("dog_barking.mp3")
	sendCommand(Motion, OFF)
	
	if (timer === null) {
		timer = createTimer(now.plusSeconds(20)) [|
  		sendCommand(HeyGoogle_Volume, 30)
	timer = null
		]
	
	}} else {
	sendCommand(HeyGoogle_Volume, 60)
	playSound("motionalarm.mp3")
	sendCommand(Motion, OFF)
	
	if (timer === null) {
		timer = createTimer(now.plusSeconds(5)) [|
  		sendCommand(HeyGoogle_Volume, 30)
	timer = null
		]
	}
    }
	
end

And this rule

var Timer timer = null
rule "Motion Light"
when
	Item Motion received command ON
then
	if (now.getMinuteOfDay > 1 && now.getMinuteOfDay < 1080) {
	sendCommand(SW2_01_1, ON)
	sendCommand(Motion, OFF)
	
	if (timer === null) {
		timer = createTimer(now.plusSeconds(120)) [|
  		sendCommand(SW2_01_1, OFF)
	timer = null
		]
	
	}
	}
end

You are using the same timer variable in both rules.
Declare 2 timers, timer1 and timer2
Use timer1 in the first rule and timer2 in the second
You should then be able to put the code into 1 rule

Out of curiosity, how did you integrate the Ring with OH? Is it just the IFTTT integration I’ve seen discussed elsewhere or have they release an API?

Yep Just IFTTT, thats about as clever as I get.
Cheers

Thanks for that, I know what you mean but I don’t know how to code that. I keep losing track with too many IF statements. I’ll have another crack.
Cheers mate

So this loaded without errors but looks messy. I would love to see another way to do this.

var Timer timer1 = null
var Timer timer2 = null
rule "Motion Detection"

when
	Item Motion received command ON

then
	if (Watch_Dog.state == ON) {
	sendCommand(HeyGoogle_Volume, 80)
	playSound("dog_barking.mp3")
	sendCommand(Motion, OFF)
	
	if (timer2 === null) {
		timer2 = createTimer(now.plusSeconds(30)) [|
  		sendCommand(HeyGoogle_Volume, 30)
	timer2 = null
		]
	
	}} else {
	sendCommand(HeyGoogle_Volume, 60)
	playSound("motionalarm.mp3")
	sendCommand(Motion, OFF)
	if (now.getMinuteOfDay > 1 && now.getMinuteOfDay < 1080) {
	sendCommand(SW2_01_1, ON)
	
	if (timer1 === null) {
		timer1 = createTimer(now.plusSeconds(120)) [|
  		sendCommand(SW2_01_1, OFF)
	timer1 = null
		]
	if (timer2 === null) {
		timer2 = createTimer(now.plusSeconds(5)) [|
  		sendCommand(HeyGoogle_Volume, 30)
	timer2 = null
		]
	}
    }
	}
	}
end

For one use proper indentation. Every time you have a { all the following lines should be indented until the }.

Avoid putting } on the same line and make them line up with the line for the matching }.

The same goes for [ and ].

This makes it MUCH easier to see which lines are inside which if statements and which lines are inside the Timer lambda.

Remember, your first goal should be to write code that is easy for humans to read. The computer doesn’t care.

Look at Design Pattern: How to Structure a Rule. Most of the code in the two clauses are the same with differing parameters. So first calculate the parameters then you only have to send the commands and create the timers once.

var Timer timer1 = null
var Timer timer2 = null

rule "Motion Detection"
when
    Item Motion received command ON
then
    // 1. Always run the rule

    // 2. Calculate what needs to be done
    var volume = 60
    var sound = "motionalarm.mp3"
    var volTimer = 5
    val nightTime = now.isAfter(now.withTimeAtStartOfDay) && now.isBefore(now.withTimeAtStartOfDay.plusHours(18)
    
    if(Watch_Dog.state == ON) {
        volume = 80
        sound = "dog_barking.mp3"
        volTimer = 30
    }

    // 3. Do it
    HeyGoogle_Volume.sendCommand(volume)
    playSound(sound)
    Motion.sendCommand(OFF)

    timer1?.cancel
    timer1 = createTimer(now.plusSeconds(volTimer), [ |
        HeyGoogle_Volume.sendCommand(30)
        timer1 = null
    ])

    if(Watch_Dog.state != ON && nightTime) {
       SW2_01_1.sendCommand(ON)
        timer2?.cancel
        timer2 = createTimer(now.plusSeconds(120), [|
            SW2_01_1.sendCommand(OFF)
            timer2 = null
        ])
    }
end
1 Like

Thanks for the advice, Ill certainly pay more attention to syntax and structure.
Ill see what I can make of your code, it will take me some time to get my head around it. The last code I posted seems to work and do what it’s meant to but the last “timer2 = null” throws lots of errors in the logs like this.
“[ERROR] [org.quartz.core.JobRunShell ] - Job DEFAULT.2018-10-11T16:00:54.391+11:00: Proxy for org.eclipse.xtext.xbase.lib.Procedures$Procedure0: [ | {”
If i comment out the timer2 = null, it all still works but I don’t know if the timer is reset.
Anyway I’ll have a look at your code.
Cheers mate

Hi Rich,
Could you please tell me how this line would look if “nighttime” was from 5pm to 5am the next day?

val nightTime = now.isAfter(now.withTimeAtStartOfDay) && now.isBefore(now.withTimeAtStartOfDay.plusHours(18)

Cheers

also, rule does not load with this error in log. “missing ‘)’ at ‘if’”
I cant see the problem though.

There’s a bracket problem in that nightTime line you just posted.

Note that setting timer2 to null does’t actually reset it. You need to call cancel to reset the Timer. Setting the variable to null is a convention often used as an easy way to tell whether you have a timer actively running and you can do something different if it is. Setting timer1 and timer2 to null is not strictly necessary in this code.

There are several ways. Probably the easiest would be:

val nightTime = now.isAfter(now.withTimeAtStartOfDay.plusHours(17) && now.isBefore(now.withTimeAtStartOfDay.plusHours(41)

Note that there might be a problem during the change to/from daylight savings which can be dealt with by jumping to tomorrow and subtracting hours.

val nightTime = now.isAfter(now.withTimeAtStartOfDay.plusDays(1).minusHours(24-17)) && now.isBefore(now.withTimeAtStartOfDay.plusDays(1).plusHours(5))

I don’t think this will work as expected, since the second condition will always be true. The way I do this, which handles daylight savings nicely, is as follows:

val nightTime = now.isAfter(now.withTimeAtStartOfDay.plusHours(17) || now.isBefore(now.withTimeAtStartOfDay.plusHours(5)

This just checks if it’s before 5am or after 5pm on the current day.

That’s right. I wasn’t thinking clearly this morning. You have to check before and after midnight separately.

Thanks for the Assist.
Cheers

Thanks Guys, I really appreciate the help. It’s all a bit of a mystery to me.
I still have a bracket error when the rule tries to load,
Here it is.

var Timer timer1 = null
var Timer timer2 = null

rule "Motion Detection"
when
    Item Motion received command ON
then
    // 1. Always run the rule

    // 2. Calculate what needs to be done
    var volume = 60
    var sound = "motionalarm.mp3"
    var volTimer = 5
    val nightTime = now.isAfter(now.withTimeAtStartOfDay.plusHours(17) || now.isBefore(now.withTimeAtStartOfDay.plusHours(5)
    
    if (Watch_Dog.state == ON) {
        volume = 80
        sound = "dog_barking.mp3"
        volTimer = 30
    }

    // 3. Do it
    HeyGoogle_Volume.sendCommand(volume)
    playSound(sound)
    Motion.sendCommand(OFF)

    timer1?.cancel
    timer1 = createTimer(now.plusSeconds(volTimer), [ |
        HeyGoogle_Volume.sendCommand(30)
        timer1 = null
    ])

    if (Watch_Dog.state != ON && nightTime) {
       SW2_01_1.sendCommand(ON)
        timer2?.cancel
        timer2 = createTimer(now.plusSeconds(120), [|
            SW2_01_1.sendCommand(OFF)
            timer2 = null
        ])
    }
end

should be

val nightTime = now.isAfter(now.withTimeAtStartOfDay.plusHours(17)) || now.isBefore(now.withTimeAtStartOfDay.plusHours(5))

But note the posting above, this won’t work. See Patrick’s post above.

Thanks again mate, I learned something new which is awesome. Just one last question about the watchdog,
“if (Watch_Dog.state != ON” is this the same as “if (Watch_Dog.state == ON)” or is it the opposite?
Edit…I think I found the answer, != does not equal and == means equal?

Cheers

Correct.