I have a strange probleme. I would like to switch devices in a decent order depending on a trigger. In principle it works fine however if all Trigger Switches are activated for some reason the switch schalter 2 goes on and off a couple times before trigger 3 starts. I looks like that OH 3 is having a time problem to work through the while loop. What can I do or where is my mistake.
use indentation to show the code that is part of that timer.
createTimer(now.plusSeconds(1), [ |
TestTrigger1.sendCommand("OFF")
// indent every line until the closing "])"
The same goes for if statements. Pretty much every time you have an opening block (i.e. [ or {) all the following lines should be indented until the closing ] or }.
Even for this relatively simple rule it’s really hard to tell what code is in what context.
If you have more than one line in a context, always start those lines on the next line, not on the same line. Instead of
Why all of these formatting suggestions? Because as written this code is exceptionally difficult to figure out what it does. I can’t even tell where your ttest1 timer ends.
Your code will create hundreds of thousands of timers (even in the short time of ten seconds for each timer) as the while loop, well… loops forever, in a millisecond. and as the first condition is not true for the first ten seconds (Test_Trigger1 is ON) but the second is, it will turn ON Test_Schalter1 and create a timer, then set the var ttest1 to the pointer for that timer, loop the while, condition 1 is false, but condition two is still true, so it will create another (additional!) timer aand set the var to the new pointer. at this moment, the pointer of the first timer gets lost, but the timer is still there, and it’s active.
Same is for all other timers, a few seconds later.
So, at least you have to check if ttest1 is null before creating the timer
//Switch Test_Scheife “Test_Scheife” [Equipment]
//Switch Test_Trigger1 “Test_Trigger1” [Equipment]
//Switch Test_Trigger2 “Test_Trigger2” [Equipment]
//Switch Test_Trigger3 “Test_Trigger3” [Equipment]
//Switch Test_Schalter1 “Test_Schalter1” [Equipment]
//Switch Test_Schalter2 “Test_Schalter2” [Equipment]
//Switch Test_Schalter3 “Test_Schalter3” [Equipment]
var Timer ttest1
var Timer ttest2
var Timer ttest3
rule “Bewaesserung in a row”
when
Item Test_Scheife changed to ON
then
ttest1 = null
ttest2 = null
ttest3 = null
var i = 1
while (i < 4) {
if(Test_Trigger1.state==OFF && i==1)
i = i + 1
if(Test_Trigger1.state==ON && i==1 && ttest1 === null) {
Test_Schalter1.sendCommand(ON)
ttest1 = createTimer(now.plusSeconds(10), [ |
Test_Trigger1.sendCommand(OFF)
createTimer(now.plusSeconds(1), [ |
Test_Schalter1.sendCommand(OFF)
])
])
}
if(Test_Trigger2.state==OFF && i==2)
i = i + 1
if(Test_Trigger2.state==ON && i==2 && ttest2 === null) {
Test_Schalter2.sendCommand(ON)
ttest2 = createTimer(now.plusSeconds(10), [ |
Test_Trigger2.sendCommand(OFF)
createTimer(now.plusSeconds(1), [ |
Test_Schalter2.sendCommand(OFF)
])
])
}
if(Test_Trigger3.state==OFF && i==3)
i = i + 1
if(Test_Trigger3.state==ON && i==3 && ttest3 === null) {
Test_Schalter3.sendCommand(ON)
ttest3 = createTimer(now.plusSeconds(10), [ |
Test_Trigger3.sendCommand(OFF)
createTimer(now.plusSeconds(1), [ |
Test_Schalter3.sendCommand(OFF)
])
])
}
}
Test_Scheife.sendCommand(OFF)
end
A more elegant solution without while and only one timer:
//Switch Test_Scheife “Test_Scheife” [Equipment]
//Switch Test_Trigger1 “Test_Trigger1” [Equipment]
//Switch Test_Trigger2 “Test_Trigger2” [Equipment]
//Switch Test_Trigger3 “Test_Trigger3” [Equipment]
//Switch Test_Schalter1 “Test_Schalter1” [Equipment]
//Switch Test_Schalter2 “Test_Schalter2” [Equipment]
//Switch Test_Schalter3 “Test_Schalter3” [Equipment]
var Timer tSprinkler = null // timer for sprinkler
var Integer iSprinkler = 0 // counter (like the i in the old while)
val Long lTenMillis = 10000000 // 10.000.000 Nanos = 10 Milliseconds
rule “Bewässerung in a row”
when
Item Test_Scheife changed to ON
then
tSprinkler?.cancel // cancel existing timer
iSprinkler = 0 // set counter to 0
tSprinkler = createTimer(now.plusNanos(nTenMillis),[| // create timer and start almost immediately
var sSwitch1 = OFF // new state for Switch 1
var sSwitch2 = OFF
var sSwitch3 = OFF
var Long lResched = nTenMillis // default time for reschedule
val Long lElevenSecs = 11000000000 // 11.000.000.000 Nanos = 11 Seconds
iSprinkler ++ // add 1
switch(iSprinkler) { // split to different cases
case 1: { // counter 1
if(Test_Trigger1.state == ON) { // if Triggersswitch ON
Test_Trigger1.postUpdate(OFF) // set to OFF
sSwitch1 = ON // change new state for Switch 1
lResched = lElevenSecs // change reschedule time
}
}
case 2: { // counter 2
if(Test_Trigger2.state == ON) {
Test_Trigger2.postUpdate(OFF)
sSwitch2 = ON
lResched = lElevenSecs
}
}
case 3: { // counter 3
if(Test_Trigger3.state == ON) {
Test_Trigger3.postUpdate(OFF)
sSwitch3 = ON
lResched = lElevenSecs
}
}
default: { // every other value (4 and above...)
lResched = 0 // set reschedule to 0
Test_Scheife.postUpdate(OFF) // switch off
}
}
if(Test_Schalter1.state != sSwitch1) // bring Switch to correct position
Test_Schalter1.sendCommand(sSwitch1.toString)
if(Test_Schalter2.state != sSwitch2)
Test_Schalter2.sendCommand(sSwitch2.toString)
if(Test_Schalter3.state != sSwitch3)
Test_Schalter3.sendCommand(sSwitch3.toString)
if(lResched != 0) // if reschedule other than 0, reschedule timer
tSprinkler.reschedule(now.plusNanos(lResched))
])
end
In fact, the code could be reduced remarkably by using Group Items.
The idea is, to set a default OFF for every switch. If counter is at the correct position and corresponding trigger is activ, deactivate trigger but set Value to ON
In addition, the default reschedule time for the timer is 10 millis (has to be set as nanos for openHAB3)
If a switch is set to ON, the reschedule time will be changed to the ON duration.
As a last step, every switch is turned to the correct position and the timer is rescheduled.
// Group gSprinkler_Enable
// Group gSprinkler
//Switch Sprinkler_Start "Start Sprinkler" [Equipment]
//Switch Sprinkler_Enable_1 "Enable Sprinkler 1" (gSprinkler_Enable) [Equipment]
//Switch Sprinkler_Enable_2 "Enable Sprinkler 2" (gSprinkler_Enable) [Equipment]
//Switch Sprinkler_Enable_3 "Enable Sprinkler 3" (gSprinkler_Enable) [Equipment]
//Switch Sprinkler_1 "Sprinkler 1" (gSprinkler) [Equipment]
//Switch Sprinkler_2 "Sprinkler 2" (gSprinkler) [Equipment]
//Switch Sprinkler_3 "Sprinkler 3" (gSprinkler) [Equipment]
var Timer tSprinkler = null // timer for sprinkler
rule “Bewässerung in a row”
when
Item Sprinkler_Start changed to ON
then
tSprinkler?.cancel
iSprinkler = 0
tSprinkler = createTimer(now.plusSeconds(1),[|
var nResched = 11
if(gSprinkler_Enable.members.filter[i|i.state == ON].size == 0)
nResched = 0
val myEnable = gSprinkler_Enable.members.filter[i|i.state == ON].sortBy[name].head // first enabled
val strCurrent = if(myEnable !== null) myEnable.name.split("_").get(2) else "end"
gSprinkler.members.filter[ i |
!i.name.endsWith(strCurrent) && i.state != OFF // all which should be off and aren't
].forEach[ j | j.sendCommand(OFF)] // switch off
gSprinkler.members.filter[ i |
i.name.endsWith(strCurrent) && i.state != ON // the one which should be on and isn't
].head.sendCommand(ON) // switch on
if(myEnable !== null)
myEnable.postUpdate(OFF)
if(nResched != 0) // if reschedule other than 0, reschedule timer
tSprinkler.reschedule(now.plusSeconds(nResched))
])
end
As you can see, it’s even more compact, thanks to no counter…
Within the timer, the list of Enable Switches is filtered to these which are ON. the first is taken as Item myEnable (in alphabetical order…) If there is none (so size of list is 0), resched is set to 0
Then the digit is extracted (last part of the item name). If there is none, String is set to “end” (there are no group members which name ends with “end”)
Each Switch which has another digit in its name and is on is witched off
The one Switch which has the digit in its name is switched on (if not already on)
The current Enable Switch is set to OFF, if there is any.
If at least one Enable switch was ON at start of timer execution, the timer will be rescheduled.
