[SOLVED] Combining 3 rules into 1 / Rule optimization

I suppose this is less OH related, but being a novice programmer at best I’m failing to wrap my mind around how I could make this into one rule, instead of triggered 3 separate ones. Or maybe it’s not needed at all, in any case, I’d welcome any pointers to streamline the rules.

rule "Gang Lys"
	when
		Item FrontDoorContact changed from CLOSED to OPEN
	then

		if(LocalSun_Position_Elevation.state < 0|°) { //Chekcs if the sun has gone down.
        GangLys.sendCommand(ON)
		}


end

	rule "Døren åbnes - ingen hjemme"
	when
		Item FrontDoorContact changed from CLOSED to OPEN
	then
		Thread::sleep(45000)
		if(KlingeTLF_Online.state == OFF && LottesTLF_Online.state == OFF) { //checks if anyones phone is currently online
       		sendNotification("my@mail.com", "Hovedøren er blevet åbnet!")  //sends notificatioon that the door has been opend and noone is home
		}
end


	
		rule " Døren holdes åben - Sluk varmen"
	when
		Item FrontDoorContact changed from CLOSED to OPEN
	then
		Thread::sleep(60000) //waits for 1 minute
		val Number pHeat = GangTermostat_SetpointHeat.state	 //Stores the current temperature in a variable.
		if(	FrontDoorContact.state == OPEN)
			GangTermostat_SetpointHeat.sendCommand(8) //Sets temperature to 8 degrees
			while(FrontDoorContact.state == OPEN){ //Continues to check if the door has been closed.
			Thread::sleep(1000) // sleep for 1 second
			}
			GangTermostat_SetpointHeat.sendCommand(pHeat) //Resets the temperature to what it was before the door opend.
			

end

From top to bottom the 3 rules does this:

When the front door opens, check if the sun has gone down, if yes, then turn on the lights in the hallway.

When the front door opens, wait for 45 seconds, then check if anyone’s phone is on the network, if they aren’t notify us both.

When the front door opens, wait for 1 minute. then save the current temperature setting of the radiator to a value. Then check if the door is still open, if yes, set the temperature to 8 degrees. continue to check if the door has been closed every second, if it is, reset the old temperature.

First thing: Avoid Treed::sleep. The number of threads openHAB can use are limited and you would block one for the time.
Better to use Timers or the Expire Binding: https://community.openhab.org/search?q=design%20pattern%20timer%20%23tutorials-examples

Other than that I see no trouble with simply trowing these tree rule bodies into one rule?

You might want a second rule to react on a closing front door to cancel your timers and - if needed - reset your temperature.

Yeah I know I shouldn’t use thread:sleep but it was the easiest solution at the time, I’ll try and rework that.

The way I see it, and why its an issue might stem from the use of sleep, but according to me the issue is that I have 2 timers with different duration, and 1 without, so anyway I set it up, one if going to have to wait on the other before it can complete, which seems unnecessary.

But considering you mention cancelling timers on the door closing, I assume I’m going to want to look into alternative to understand it.

This is very bad practice. Your are locking the thread these rules run in for a very long time
See:

To put your three rules into one:

var Number pHeat = null // AT TOP OF RULE FILE!!

rule "Front door CLOSED TO OPEN"
when
    Item FrontDoorContact changed from CLOSED to OPEN
then
    if(LocalSun_Position_Elevation.state < 0|°) { //Chekcs if the sun has gone down.
        GangLys.sendCommand(ON)
    }
    createTimer(now.plusSeconds(45), [ |
        if(KlingeTLF_Online.state == OFF && LottesTLF_Online.state == OFF) { //checks if anyones phone is currently online
            sendNotification("my@mail.com", "Hovedøren er blevet åbnet!")  //sends notificatioon that the door has been opend and noone is home
        }
    ])
    createTimer(now.plusSeconds(60) , [ |
        pHeat = GangTermostat_SetpointHeat.state as Number //Stores the current temperature in a variable.
        if(FrontDoorContact.state == OPEN) {
            GangTermostat_SetpointHeat.sendCommand(8) //Sets temperature to 8 degrees
        }
    ])
end

rule "Front Door OPEN TO CLOSED"
when
    Item FrontDoorContact changed from OPEN to CLOSED
then
    if (pHeat !==null) GangTermostat_SetpointHeat.sendCommand(pHeat)
    pHeat = null //Resets pHeat for next time the door is opened
end

Doesn’t work like that. You can set up as many timers as like at one time, all scheduled for different future times. Like @vzorglub example.

Thank you.

As I said, I know thread:sleep is bad practice, it’s part of the reason I made the topic.

I’ll look over your example, thank you very much.

Most people would have searched for alternatives then. I read that early on and chose the expire binding. I hope to replicate functionality in Jython and avoid that version 1 binding.

Most people I know using hacks and workarounds, then work out a proper solution down the line, different folks and all that. Which is exactly what I’m doing right now.

But please use different names for the rules :wink:

EDIT: Done :smiley:

Thanks I chaged the trigger but not the rule title…

1 Like

Let me know if this requires a new thread, but I’m having some issues that I can’t quite get my head around.

It seems that on occasion (mostly in the early morning when my SO leaves the house) it’ll disregard the timers, and save incorrect values to the pHeat variable.

var Number pHeat = null

rule "Front door CLOSED TO OPEN"
when
Item FrontDoorContact changed from CLOSED to OPEN
then
if(LocalSun_Position_Elevation.state < 0|°) { //Chekcs if the sun has gone down.
    GangLys.sendCommand(ON)
}
createTimer(now.plusSeconds(45), [ |
    if(KlingeTLF_Online.state == OFF && LottesTLF_Online.state == OFF) { //checks if anyones phone is currently online
        sendNotification("my@mail.com", "Hovedøren er blevet åbnet!")  //sends notificatioon that the door has been opend and noone is home
    }
])
createTimer(now.plusSeconds(60) , [ |
    pHeat = GangTermostat_SetpointHeat.state as Number //Stores the current temperature in a variable.
		logDebug("Heating", "Previous temperature saved as " + pHeat)
    if(FrontDoorContact.state == OPEN) {
        GangTermostat_SetpointHeat.sendCommand(8) //Sets temperature to 8 degrees
    }
])
end

rule "Front Door Open TO Closed"
when
Item FrontDoorContact changed from OPEN to CLOSED
then
logDebug("Heating", "Previous temperature logged as " + pHeat)
if (pHeat !==null){
	GangTermostat_SetpointHeat.sendCommand(pHeat)
	logDebug("Heating", "Temp set at " + pHeat)
pHeat = null //Resets pHeat for next time the door is opened
	logDebug("Heating", "Pheat reset to  " + pHeat)
	}
end

Ive added some debug messages, but they don’t really help me as much as I’d like, as I can’t explain why it happens, for instance:

Morning 8:14 thermostat is set to 8 degrees:

2019-10-25 08:14:02.800 [ome.event.ItemCommandEvent] - Item 'GangTermostat_SetpointHeat' received command 8
2019-10-25 08:14:02.814 [nt.ItemStatePredictedEvent] - GangTermostat_SetpointHeat predicted to become 8
2019-10-25 08:14:02.846 [vent.ItemStateChangedEvent] - GangTermostat_SetpointHeat changed from 21 °C to 8.0 °C

This is correct, I’ve confirmed that setpoint heat’s value is 8, both via the API and looking at the thermostat.

8:25 the door opens, which should start the 60 second timer.

2019-10-25 08:25:33.838 [vent.ItemStateChangedEvent] - FrontDoorContact changed from CLOSED to OPEN
2019-10-25 08:25:37.394 [vent.ItemStateChangedEvent] - FrontDoorContact changed from OPEN to CLOSED

But since the door closes within the 60 second timer, nothing should change, however, it seems the rule disregarded the timer, and, on top of that, saved the variable as 21, rather than 8, for some reason I can’t quite grasp.

2019-10-25 08:25:37.447 [DEBUG] [lipse.smarthome.model.script.Heating] - Previous temperature logged as 21 °C
2019-10-25 08:25:37.478 [DEBUG] [lipse.smarthome.model.script.Heating] - Temp set at 21 °C
2019-10-25 08:25:37.503 [DEBUG] [lipse.smarthome.model.script.Heating] - Pheat reset to  null
2019-10-25 08:26:33.901 [DEBUG] [lipse.smarthome.model.script.Heating] - Previous temperature saved as 21 °C

Any keen minds able to see what might go wrong here?

Well, that is the problem.
The 60-sec timer starts off when the door is opened.
Whether the door is ever closed or not, in 60 secs we store to pheat.

If the door is closed within the 60 secs AND pheat is non null, your thermostat thing happens.
So, would pheat be non-null? Yes - every time the door is opened, 60 secs later it gets populated.
In the morning, it will probably have a value from the evening before.

Why isn’t the ‘closed’ rule setting pheat to null? Well, it is. But as the door is usually closed within 60 secs, in a few secs the timer will populate it again.

The missing action is to cancel the 60 sec timer if the door is closed while it is running (so allowing pheat to remain null)
or
only populate pheat if the door is still open after 60 secs.

Ah of course I see now.

I’ve moved the populating of the variable inside the if statement, and I’ll test it again when I get home, thanks!

I had the impression, that the 8°C was not caused by an opened door, but on purpose through the thermostate.
But of course it would be a more straight forward to ensure the timer cancelled. You would only need one rule:

var Number pHeat = null
var Timer tDoor1 = null
var Timer tDoor2 = null

rule "Front door contact"
when
    Item FrontDoorContact changed
then
    if(FrontDoorContact.state == OPEN) {                                              // door was opened!
        if(LocalSun_Position_Elevation.state < 0|° && GangLys.state != ON)            // sun under horizon?
             GangLys.sendCommand(ON)
        if(tDoor1 === null)                                                           // timer 1 already started?
            tDoor1 = createTimer(now.plusSeconds(45), [ |                             // timer 1: delayed silent alarm 
                if(KlingeTLF_Online.state == OFF && LottesTLF_Online.state == OFF)    // anybody home?
                    sendNotification("my@mail.com", "Hovedøren er blevet åbnet!")     // send notification
                tDoor1 = null                                                         // delete timer 1
            ] )
        if(tDoor2 === null)                                                           // timer 2 already started?
            tDoor2 = createTimer(now.plusSeconds(60), [ |                             // timer 2: save heating energy
                pHeat = GangTermostat_SetpointHeat.state as Number                    // store current temperature
                logDebug("Heating", "Previous temperature saved as {}°C ", pHeat)
                GangTermostat_SetpointHeat.sendCommand(8)                             // set temperature to 8°C
                tDoor2 = null                                                         // delete timer 2
            ])
    } else {                                                                          // door was closed
        tDoor?.cancel
        tDoor2 = null                                                                 // delete timer 2
        if(pHeat !==null && (GangTermostat_SetpointHeat.state as Number) != pHeat) {  // thermostate was set through timer 2
            GangTermostat_SetpointHeat.sendCommand(pHeat)
            logDebug("Heating", "Temp set to {}°C", pHeat)
        }
    }
end

Thank you for responding.

Moving the storing of the value to Pheat inside the if that only executes after the timer has completed solves this nice.

I need to buy myself a rubber duck, however.