Loop rule (in OH2)

Come to think about, you cannot use an item object in such a comparator
Please note that almost everything in openHAB is object based programming, which means that items are complex objects and not just a number or string.
you have to use:

if (Avtivity.state == 5)

this should allow you to access the state of your object Activity and compare it against a number (your rules reads that the state is a simple number)

However, I am still not sure your rule would fully work as intended. As your rule sleeps for two times 2.5 sec, you may want to watch out whether there is a possibility that the state of Activity can change to another value and back to 5 again within these 5 secs, which would trigger your rule again, while it is already running and cause confusion. (just one use case that comes to mind where your rule would fail); you can have reentry locks (there are forum posts on this topic, that would guard against triggering a rule twice, still your rule locks up too much resources, I believe)
You may want to look up the Tutorials section, especially the ones with timers. It may make more sense to launch a timer (which runs in the background) that blinks your lamp using recursive timers:

That is some useful information. I think I will avoid triggering it again, it happens based on a door opening. If another door opens it is still set and can’t be changed to trigger it again. After I reset the status it could happen again. I will study up on the rest though. I’ll need to more before taking it on.

Now i’ve failed to add a counter to the loop. It seems to count once but then as the loop starts over so does the counter. I have declared the variable SirenTimer outside of the rule and loop, thinking that made it a global variable.

var Number SirenTimer = 0
rule "Warning"
when Item Warning changed to ON
then
	var Number tloop = 1
	logInfo("info", "Warning Activated")
	while (tloop>0)
		{
		logInfo("info", "Warning on. SirenTimer is " + SirenTimer)
		sendCommand(WeMoSwitch_Switch, ON)
		Thread::sleep(2500)
		sendCommand(WeMoSwitch_Switch, OFF)
		Thread::sleep(2500)
		if(Warning.state == ON) 
			{
			val SirenTimer = SirenTimer + 1
			logInfo("info", "Warning still active. SirenTimer is " + SirenTimer)
			}
		else{
			logInfo("info", "Warning no longer active. ")
			tloop = tloop - 1
			val SirenTimer = 0
			}
		}
end

The log result is:

Warning Activated
2017-01-21 12:06:09.511 [INFO ] [.eclipse.smarthome.model.script.info] - Warning on. SirenTimer is 0
2017-01-21 12:06:14.511 [INFO ] [.eclipse.smarthome.model.script.info] - Warning still active. SirenTimer is 1
2017-01-21 12:06:14.511 [INFO ] [.eclipse.smarthome.model.script.info] - Warning on. SirenTimer is 0
2017-01-21 12:06:19.511 [INFO ] [.eclipse.smarthome.model.script.info] - Warning still active. SirenTimer is 1
2017-01-21 12:06:19.512 [INFO ] [.eclipse.smarthome.model.script.info] - Warning on. SirenTimer is 0

I don’t see why SirenTimer won’t make it to 2.

alright we are reaching officially the limit of my knowledge…
outside your rule, you are defining a variable, which I believe is OK and valid but only within this specific file (keep that in mind if you were using “SirenTimer” elsewhere
… by this I mean you used

var Number SirenTimer = 0

before you started the rule definition
Yet in your rules you used

val SirenTimer = SirenTimer + 1

If I understand it correctly you would have overwritten the early assignment with a local static variable
Again, see disclaimer above, but it looks to me that you defined a variable using “var” outside the rule, and then you re-defined inside inside the rule, which would essentially overwrite everything you did outside the rule definition, and in addition you defined it as a constant (val is equivalent to what programming languages call a constant).
You may want to try to move

var Number SirenTimer = 0

after the “then” and drop the “val” to read in your if statement

SirenTimer = SirenTimer + 1

and in your else statement

SirenTimer = 0

Notice the absence of the “val” in both

Your knowledge was exactly what I needed. Having reviewed lots of other people I picked up on some things I didn’t fully understand. I didn’t realize I was re-declaring the variable from scratch and that I didn’t need anything to update its value.

It is working great now. Thank you for your help!

var Number SirenTimer = 0    
rule "Warning then Siren"
    when Item Warning changed to ON
    then
    	var Number tloop = 1
    	logInfo("info", "Warning Activated")
    	while (tloop>0)
    		{
    		logInfo("info", "Warning on. SirenTimer is " + SirenTimer)
    		sendCommand(WeMoSwitch_Switch, ON)
    		Thread::sleep(2500)
    		sendCommand(WeMoSwitch_Switch, OFF)
    		Thread::sleep(2500)
    		if(Warning.state == ON) 
    			{
    			SirenTimer = SirenTimer + 1
    			logInfo("info", "Warning still active. SirenTimer is " + SirenTimer)
    			if(SirenTimer == 8)
    				{
    					sendCommand(Siren_Switch, ON)
    				}
    			}
    		else{
    			logInfo("info", "Warning no longer active. ")
    			tloop = tloop - 1
    			SirenTimer = 0
    			}
    		}
    end

    rule "Alarm Off"
    when Item Alarm changed to OFF
    then
    	sendCommand(Warning, OFF)
    	sendCommand(Siren_Switch, OFF)
    	SirenTimer = 0
    end

That is incorrect. Thread::sleep only blocks the one instance of the rule. There is a case where sleeping too long on a rule that triggers a lot can cause OH to run out of threads to use but that is unlikely in this case.

Everything else said is correct.

1 Like

Thanks!! I have deleted this in post above, appreciate your input.

Just for fun, here is a version of the loop that flashes the international morse code signal “SOS” (… - - - …) when my alarm goes off. I am using a RPI GPIO pin, and the AlarmSystem Item is of the Type Contact.

rule “alarmsystem_set_off”

when
Item AlarmSystem changed

then
while (AlarmSystem.state== OPEN){
sendCommand(Node3_CtrDiningRoom_Dimmer, ON)
Thread::sleep(500)
sendCommand(Node3_CtrDiningRoom_Dimmer, OFF)
Thread::sleep(500)

            sendCommand(Node3_CtrDiningRoom_Dimmer, ON)
            Thread::sleep(500)
            sendCommand(Node3_CtrDiningRoom_Dimmer, OFF)
            Thread::sleep(500)

            sendCommand(Node3_CtrDiningRoom_Dimmer, ON)
            Thread::sleep(500)
            sendCommand(Node3_CtrDiningRoom_Dimmer, OFF)
            Thread::sleep(1500)

            sendCommand(Node3_CtrDiningRoom_Dimmer, ON)
            Thread::sleep(1500)
            sendCommand(Node3_CtrDiningRoom_Dimmer, OFF)
            Thread::sleep(500)

            sendCommand(Node3_CtrDiningRoom_Dimmer, ON)
            Thread::sleep(1500)
            sendCommand(Node3_CtrDiningRoom_Dimmer, OFF)
            Thread::sleep(500)

            sendCommand(Node3_CtrDiningRoom_Dimmer, ON)
            Thread::sleep(1500)
            sendCommand(Node3_CtrDiningRoom_Dimmer, OFF)
            Thread::sleep(1500)

            sendCommand(Node3_CtrDiningRoom_Dimmer, ON)
            Thread::sleep(500)
            sendCommand(Node3_CtrDiningRoom_Dimmer, OFF)
            Thread::sleep(500)

            sendCommand(Node3_CtrDiningRoom_Dimmer, ON)
            Thread::sleep(500)
            sendCommand(Node3_CtrDiningRoom_Dimmer, OFF)

            sendCommand(Node3_CtrDiningRoom_Dimmer, ON)
            Thread::sleep(500)
            sendCommand(Node3_CtrDiningRoom_Dimmer, OFF)
            Thread::sleep(3500)

    }

end

FWIW, the indentations look right when I posted this as text, but obviously the formatting was screwed up by the codefencing…

1 Like

That is because you didn’t use code fencing, you used quotes.

```
code goes here
```

One thing I will note is that any sleep of more than a few hundred milliseconds can cause problems in the long run because it ties up a Rule execution thread. This Thread ties up an execution thread for 11 seconds at a minimum, potentially and likely longer. If you have sleeps in other rules that are executing at this same time you can bring your OH to a halt as it runs out of Rule execution threads. Given this is an error case, that may not matter. But if it does, you would have to use Timers. Obviously, this makes it much more complex.

Note, I’m just typing this in, there are likely errors. Here is an attempt at a generic morse code set of rules and lambdas. I’m mainly doing this because of the challenge. I’m not sure I would recommend it as a valid approach.

import org.eclipse.xtext.xbase.lib.Functions
import org.eclipse.xtext.xbase.lib.Procedures

// Convert a String to Morse code with characters separated by commas
val Functions$Function1<String, String> toMorse = [message |
    val StringBuilder morse = new StringBuilder()
    var int charIndex = 0
    while(charIndex < message.length){
        morse.append(transform("MAP", "morse.map", ""+message.charAt(charIndex))
        morse.append(",")
    }
    morse.toString
]

val Procedures$Procedure4<String, Map<SwitchItem, String, Integer, Procedures$Procedure3> blinkMorse = [ light, message, index, recurse |

    // If the Alarm is OPEN, continue the message, otherwise the blinking will stop
    if(AlarmSystem.state == OPEN) {

        // reset the index to resend the message
        if(index >= message.length) index = 0

        // Initialize values for a dot
        var int onTime = 500 // stay on 500 msec for a dot
        var int offTime = 500 // stay off for 500 msec between morse symbols
        var lightState = ON

        switch(message.charAt[index]) {
//            case '.':  // do nothing, defaults are already set for dot
            case '-':  onTime = 1500 // stay on for 1500 for a dash
            case ',': {
                lightState = OFF
                onTime = 500 
                offTime = 500 // 500 msec from sleep after turning off light + onTime + offTime means a rest of 1500
            }
        }

        // Turn ON/OFF the light
        light.sendCommand(lightState)

        // Turn OFF the line after onTime msecs
        createTimer(now.plusMillis(onTime), [|
            light.sendCommand(OFF)

            // sleep for half a second before processing the next character
            createTimer(now.plusMillis(500), [| recurse.apply(light, message, index+1, recurse)]
        ])
    }
]

rule "Blink lights with morse code"
when
    Item AlarmSystem changed
then
    // Blink morse code
    if(AlarmSystem.state == OPEN) {
        val morse = toMorse.apply("SOS")
        blinkMorse.apply(Node3_CtrDiningRoom_Dimmer, morse, 0, blinkMorse)
    }
end

morse.map

A=.-
a=.-
B=-...
b=-...
C=-.-.
c=-.-.
// and so on

Theory of operation:

We use morse.map to map the letters to their Morse code equivalent represented with . and -.

When the AlarmSystem changes to OPEN we convert the String “SOS” to Morse with a comma, separating the characters.

Then we kick off the recursive timer procedure to start blinking the light. Note that we pass blinkMorse to itself. This lets us recursively call the procedure.

In the procedure we see if the AlarmSystem is still OPEN. If it is not the recursion ends and the lights blinking stops. It should end with the light in the OFF state.

If the AlarmSystem is OPEN, then we process the next character in the morse message. We keep track of what symbol in the message we are on using the index variable. When we get to the end of the Morse message we reset the index to zero so we can send the message again. Next, we initialize some variables with the on and off times and whether to turn ON or OFF the light. Then we turn ON/OFF the light, set a Timer for as long as the light should stay ON, when the Timer executes we turn off the light and recursively call the procedure with an incremented index to process the next symbol.

Not bad for around 60 lines of code and 64 some lines in a map file. Assuming this works.

3 Likes

I SO love the openHAB community!!! This is awesome! (Okay - maybe off-the-charts nerd factor, but hey…)

Yes @rlkoshak, I saw your comment about SLEEP and exceptions. I will take a look at your code and drop it on to my dev system to see what happens. Standby…

Oh - and for completeness, inter-word spacing would be 3500…

I think with the above two commas would get you to 3500, maybe only 3000, I’d have to think about it. I think it is only 3000 so another character would need to be added (space makes sense) :slight_smile:

    case ' ': {
        lightState = OFF
        onTime = 500
        offTime = 2500
    }

If using message.toUpperCase.chartAt(), you could omit the lower case in the map, as it’s redundancy.

:wink:

3 Likes

I try to adapt the code present into this topic to just make some lights flash during 100 cycles every 2 seconds if my alarm system trigger. After that, I want to let all the lights open until I take action. For sure the way I was doing it was clearly not correct after reading some post about issue cause by long Thread::sleep into a while statement.

old rule:

var Number i = 0
while (i<100) {
	if (SwMasterFlashLightOnAlarm.state==ON) {
		sendCommand(gAllLights, ON)
		Thread::sleep(2000)
		sendCommand(gAllLights, OFF)
		Thread::sleep(2000)
		i = i+1	
	}
	else
		i=100
}
sendCommand(gAllLights, ON)

now replaced by this (still not working correctly):

var Number i = 0
while (i<100) {
	if (SwMasterFlashLightOnAlarm.state==ON) {
		// Turn ON the lights
		gAllLights.sendCommand(ON)
		// Turn OFF the lights after 2000 msecs
    	createTimer(now.plusMillis(2000), [|
			gAllLights.sendCommand(OFF)
			// Sleep for 2000 msecs before next while
			createTimer(now.plusMillis(2000), [|
				// do nothing
			])
		])
		i = i+1	
	}
	else
		i=100
}
gAllLights.sendCommand(ON)

It still not work correctly and I don’t know how to replace the while statement by something else? That rule do not fire often but if it happen, I don’t want to block every other rule or crash OH. Any advice how to proceed?

Well, you can’t just use createTimer instead of Thread::sleep(), these two are very different. Thread::sleep() stops the code and waits, while createTimer schedules a timer which then will do the job in time. Solution would be a rule without while(!):

// define global vars on top of file!
var Timer tBlink = null  // Timer for light flashing
var int iBlink = 0       // counter for light flashing

rule "Alarm"
when
    Item Alarm changed to ON                                                // Alarm triggered
then
    if(SwMasterFlashLightOnAlarm.state==OFF) {                              // Flashlight disabled
        logInfo("alarm","Alarm, but Flashlight disabled")
        return;                                                             // so stop rule
    }
    if(tBlink !== null){                                                    // timer already exists
        logInfo("alarm","Seems there is already a running timer")
        return;                                                             // so stop rule
    }
    iBlink = 0                                                              // set initial counter
    tBlink = createTimer(now,[|                                             // create a timer which is executed immediately
        iBlink++                                                            // count up (only available with int)
        if(iBlink < 201) {                                                  // while less than 201 (i.e. 100 * 2 + 1)
            gAllLights.sendCommand(if(gAllLights.state != ON) ON else OFF)  // toggle gAllLights
            tBlink.reschedule(now.plusSeconds(2))                           // reschedule Timer in 2 Seconds
        } else {                                                            // otherwise
            gAllLights.sendCommand(ON)                                      // switch ON
            tBlink = null                                                   // and delete timer
        }
    ])
end

rule "End Alarm"
when
    Item SwMasterFlashLightOnAlarm changed to OFF                           // Flashlight disabled
then
    tBlink?.cancel                                                          // so cancel a timer (if scheduled)
    if(Alarm.state != OFF) gAllLights.sendCommand(OFF)                      // if active alarm, switch off gAllLights
end

Please be aware that the code in question is only this part:

    iBlink = 0                                                              // set initial counter
    tBlink = createTimer(now,[|                                             // create a timer which is executed immediately
        iBlink++                                                            // count up (only available with int)
        if(iBlink < 201) {                                                  // while less than 201 (i.e. 100 * 2 + 1)
            gAllLights.sendCommand(if(gAllLights.state != ON) ON else OFF)  // toggle gAllLights
            tBlink.reschedule(now.plusSeconds(2))                           // reschedule Timer in 2 Seconds
        } else {                                                            // otherwise
            gAllLights.sendCommand(ON)                                      // switch ON
            tBlink = null                                                   // and delete timer
        }
    ])

1 Like

I don’t know what to say… I asked for some help, at least some good advice and you came with that complete solution. Very appreciate! With that complete example I now understand better how timer work. It will help me to do some more rule avoiding long “Thread::sleep”.

I have however another quick question related to this example. I perfectly understand the concept you propose me. It still not working but I will figure out with some more test. My question is more about that line:

Why we must use " !== null " and not " != null ".
Currently, I get in the log file “Seems there is already a running timer”, so the rule detect the timer tBlink as different as null and exit without flashing the light, but I ran that rule only once. I have to figure out why that timer is not equal to null when the alarm activated.

Is there a way to test if a timer is active to troubleshoot that issue?

No, that is why we use the “test for null / set to null” trick in rules to see if the timer exists or not.

Bear in mind while developing rules, if your timer code executes but has an error it will never get to the variable=null assignment. The timer’s handle variable does not return to null by itself any other way.

The “fix” is to reload your rules file, which will also reinitialize your timer handle variable to null.

In OH 3.0, you will be able to use my_timer.isActive() to determine if a timer has completed or not.

OH2 offers myTimer.hasTerminated() , but it doesn’t work unless the timer actually exists.

Problem fixed and worked perfectly well! I had to change that section for the following:

tBlink = createTimer(now) [|                                             // create a timer which is executed immediately
    iBlink++                                                            // count up (only available with int)
    if(iBlink < 201) {                                                  // while less than 201 (i.e. 100 * 2 + 1)
        gAllLights.sendCommand(if(gAllLights.state != ON) ON else OFF)  // toggle gAllLights
        tBlink.reschedule(now.plusSeconds(2))                           // reschedule Timer in 2 Seconds
    } else {                                                            // otherwise
        gAllLights.sendCommand(ON)                                      // switch ON
        tBlink = null                                                   // and delete timer
    }
]

Maybe you can explained difference between " createTimer(now,[| " and " tBlink = createTimer(now) [| ".

In fact I used that way to create timer seen in another thread without knowing exactly the difference. With the first version I had some log error related to an unhandled Exception.

2020-06-24 07:12:34.743 [ERROR] [org.quartz.core.JobRunShell ] - Job DEFAULT.Timer 58 2020-06-24T07:12:34.721-04:00: Proxy for org.eclipse.xtext.xbase.lib.Procedures$Procedure0: [ | {
++
org.eclipse.xtext.xbase.impl.XIfExpressionImpl@13c28 (conditionalExpression: false)
} ] threw an unhandled Exception:
java.lang.NullPointerException: cannot invoke method public abstract boolean org.eclipse.smarthome.model.script.actions.Timer.reschedule(org.joda.time.base.AbstractInstant) on null

Well, there is no difference between

tTimer = createTimer(now, [|
    // foo
])

and

tTimer = createTimer(now) [|
    // foo
]

But the former version is more clear about the fact, that the lambda (the part between [ and ]) is a parameter of the function createTimer().