Wow that’s a lot of nested if statements.
First, the validation issues.
Each of those variables listed in the message are defined but never used. It’s pointless to define a variable and never use that variable, hence the warning.
But this is only a symptom of a whole bunch of much more fundamental problems.
- There is no Timer_6 type. There is only a Timer type so you should have.
var Timer timer_6 = null // für Rollo 2 WZ
var Timer timer_7 = null // für Rollo 3 WZ
var Timer timer_8 = null // für Rollo 5 EZ
var Timer timer_9 = null // für Rollo SZ Rollo 14
var Timer timer_10 = null // für Rollo GZ Rollo 17
var Timer timer_11 = null // für Rollo 19 KiZi1 (Gaube)
-
You never look at these timers again, unless you have some other Rule you are not showing, so there is no reason for timer_6 through timer_11 to even exist. Replace
timer_6 = createTimer(now.plusSeconds(17), [|
with
createTimer(now.plusSeconds(17), [|
-
Why are you taking something that is a Number, converting it to a String and then parsing that String back into a Number (Integer is a Number) for comparisons? Just use
if(astro_azimuth.state > rolloautomatic_azimuth_ost_start.state)
or if that doesn’t work
if(astro_azimuth.state as Number > rolloautomatic_azimuth_ost_start.state as Number)
-
Be consistent with your indents. There are some of those if statements that are even further indented. Indentation is vital to help humans understand the code (the computer doesn’t care).
-
If the Fenster Items are Contact Items, you need to compare them to CLOSED, not the String “CLOSED”.
OK, now for simplifications. Honesty, this code is too complex for me to quickly scan and understand what it does (and I don’t read German). But I do see a number of things that might help simplify the Rules.
if ((WZ_Fenster_02.state!="CLOSED") && (WZ_Fenster_03.state!="CLOSED") && (WZ_Fenster_04.state!="CLOSED") && (EZ_Fenster_05.state!="CLOSED") && (AZ_Fenster_12.state!="CLOSED") && (AZ_Fenster_13.state!="CLOSED")) {
if (log) logInfo('rules', logPrefix_ost + 'Beende, da mindestens ein großes Fenster im WZ, EZ oder AZ offen')
return;
}
if (rolloautomatik_ost.state == ON) {
if (log) logInfo('rules', logPrefix_ost + 'Beende, da Automatik generell nicht aktiv')
return;
}
// and so on
If you do that you will end up with just one or two indents and have much easier to understand code.
-
Consider moving some of these calculations outside of this Rule. For example, if you put all those WZ_Fenster Items into a Group:Contact:OR(OPEN, CLOSED) Fensters you can replace that first really long if statement with
if(Fensters.state != CLOSED
Similarly, you can move some comparisons into their own Rule that saves the result in an Item. For example, have an astro_azimuth triggered Rule that all it does is determine whether the rollershutters need to be adjusted and if so send a command to an Item. Then you have another Rule triggered by this other Item to do the work of adjusting the Items.
-
There is a lot of duplicated code here. If you apply How to Structure a Rule (linked to in the DRY post above) you should be able to just determine what values you need to adjust the rollershutters or not and then use the same code to adjust them all.
-
The whole purpose of the first argument to logInfo is to get your logPrefix_ost value, why prepend it to the log statement?
-
You usually only have to specify the type of a variable only if you will initialize it to null.
-
The usual way to handle logging/not logging is to use logging levels. So instead of an if statement in front of every log statement, use logDebug and if you ever need to turn on the logging open the Karaf Console and change the logging level for org.eclipse.esh.Script.Rolloautomatik Ost Rollos ab - (or something like that, I don’t have access to my home network right now to check, to debug.
I’m just typing the following in to the browser, it likely has a bunch of typos.
rule "Rollos Ostseite abfahren"
when Item astro_azimuth changed
then
val logPrefix_ost = 'Rolloautomatik Ost Rollos ab - '
logDebug(logPrefix_ost, 'Regel wurde gestartet')
val timeLast_ost = if(rolloautomatik_ost_start_last.state == NULL) now.minusDays(1) else new DateTime(rolloautomatik_ost_start_last.state.toString())
if(Fensters.state != CLOSED){
logDebug(logPrefix_ost, 'Beende, da mindestens ein großes Fenster im WZ, EZ oder AZ offen')
return;
}
if(rolloautomatik_ost.state != ON) {
logDebug(logPrefix_ost, 'Beende, da Automatik generell nicht aktiv')
return;
}
if(astro_azimuth.state <= rolloautomatik_azimuth_ost_start.state && astro_azimuth.state >= rolloautomatik_azimuth_ost_ende.state) {
logDebug(logPrefix_ost, 'Automatik heute bereits einmal gelaufen (' + rolloautomatik_ost_start_last.state.toString().substring(11,19) + '), wird daher ignoriert')
return;
}
if(wetter_temperatur_2.state < rolloautomatik_temp_min.state) {
logDebug(logPrefix_ost, ' Sollwert Azimuth für Abfahren (' + rolloautomatik_azimuth_ost_start.state.toString() + ') wurde noch nicht erreicht, aktueller Azimuth ist (' + astro_azimuth.state.toString() + ')')
return;
}
if (wetter_bewoelkung_2.averageSince(now.minusMinutes(30)) > rolloautomatik_wolken_max.state){
logDebug(logPrefix_ost, 'Mindest-Temperatur (' + rolloautomatik_temp_min.state.toString() + ') wurde nicht erreicht durch aktuelle Temperatur (' + wetter_temperatur_2.state.toString() + ')')
return;
}
if (astro_elevation.state <= rolloautomatik_elevation_ost_start.state.toString()) {
logDebug(logPrefix_ost, 'Sollwert Elevation für Abfahren (' + rolloautomatik_elevation_ost_start.state.toString() + ') wurde noch nicht erreicht, aktuelle Elevation ist (' + astro_elevation.state.toString() + ')')
return;
}
// NOTE: Create a Group to hold all the Rollo_Pos
val Map<String, Integer> downSeconds = newHashMap("WZ" -> 17, "SZ" -> 17, "GZ" -> 18, "KiZi1" -> 16)
Rollo_Pos.members.filter[ pos | pos.state <= rolloautomatik_zielwert.state ].forEach[ pos | // Working with Groups in Rules Design Pattern
logDebug(logPrefix_ost, 'Fahre Rolladen Ost auf ' + rolloautomatik_zielwert.state.toString() + '%: ' + pos.name)
val rolloName = pos.name.replace("_Pos", "")
sendCommand(rolloName, "DOWN") // Associated Items Design Pattern
createTimer(now.plusSeconds(downSeconds.get(pos.name.split("_").get(0), [ |
logInfo(rolloName, "Timer expired for " + rolloName + " set to STOP")
sendCommand(rolloName, "STOP")
]
logInfo(rolloName, rolloName + " auf 90%")
pos.sendCommand(90)
]
// Letzte Ausführung mit entsprechendem Zeitstempel belegen
sendBroadcastNotification("Verschattung Ostseite aktiv") //Pushnachricht
rolloautomatik_ost_start_last.postUpdate(now.toString())
end
