Timers in a rule

Setup, Configuration and Use Scripts & Rules
5

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
2 Likes