Use rollershutter state in rule

KraxelHuber · April 20, 2018, 4:05pm

I am using a rule that closes a number of rollershutters to 85% if the temperature rises above 24 degrees. They will be closed completely if the temperature rises above 30 degrees. I am checking every 30 mins between 11am and 4pm. Once the rollershutter are (partly) closed, I would like to avoid a shutting down command to be sent again. However, the check whether the rollershutter group is (partly) closed doesn’t seem to work correctly.

Here’s the rule:

rule "Close rollershutter in first floor if too hot"
when
    Time cron "0 0/30 11-16 ? * * *"   // Every 30 minutes between 11h-16h
then
    if (vWeatherTemperature.state >= 30 && g_og_rollershutter.state != 100) {
        g_og_rollershutter.sendCommand(DOWN)
    } else if (vWeatherTemperature.state >= 24 && g_og_rollershutter.state == 0) {
        g_og_rollershutter.sendCommand(85)
    }
end

The command to close the group’s rollershutters to 85% will be sent every 30 minutes. Why is it this way?

vzorglub · April 20, 2018, 4:21pm

I don’t quite get what you want to do.
The first if statement is clear “IF (TEMP>=30 AND SHUTTER OPEN)”
Your second if statement reads: “IF (TEMP>=24 AND SHUTTER CLOSED)”
That will not be true unless it was 30+ BEFORE
But if it’s only 26 at 11am your rule won’t do anything
Check if the roller shutter is NOT partly closed (85%) instead
“IF (TEMP >= 24 AND SHUTTER NOT PARTLYCLOSED)”

} else if (vWeatherTemperature.state >= 24 && g-og-rollershutter.state != 85) {

Does that make sense? Does it solve your issue?

opus · April 20, 2018, 6:55pm

I would use logInfo statements in the rule in order to see which line of code gets hit and which not.
Secondly I would initially separate the checks in each if statement in order to get a clear picture which check does work ( make two if statements instead of a combined check).
I suspect the group state is not what you think it is. How did you define the group?

KraxelHuber · April 20, 2018, 7:24pm

That’s simple:

If it’s more than 24°C, I want the rollershutter to be partially closed (85%).
If the temperature rises further above 30°C, I want the rollershutter to be completely closed (100%):
This should only happen between 11am and 4pm.

Not quite, it should read: “IF (TEMP>=30 AND SHUTTER ARE NOT COMPLETELY CLOSED)"

No, in my view it reads: “IF (TEMP>=24 AND SHUTTER OPEN)”

Yes, it will!

KraxelHuber · April 20, 2018, 7:38pm

These days, the “else if” part hat been correctly executed as the temperature rose above 24°C but not above 30°C (between 11am and 4pm). However, this rule has been executed every 30 minutes (I originally placed a telegram msg into the code but omitted it when posting).

Good idea. Will do.

That’s exactly what I think, hence the title of this post I suspect that openHAB interpretes the rollershutter.state part always as TRUE even when it’s actually not.

g_og_rollershutter.state == 0 should be true if all rollershutters are completely open. However, after the first execution of the rule, the rollershutters are closed (85%), i.e. I would expect rollershutter.state == 85. I wonder why this rule still executes?

The group is defined as follows:

Group:Rollershutter:OR(100,0) g_og_rollershutter    "Rollläden Obergeschoss"            <blinds>

Maybe the error is located in this very definition.

rlkoshak · April 20, 2018, 8:11pm

I find that it is a little more clear when writing rules like this to structure it a little bit differently:slight_smile:

determine if you need to run at all and if not exit
calculate what needs to be done
do it

So the rule becomes:

rule "Close rollershutter in first floor if too hot"
when
    Time cron "0 0/30 11-16 ? * * *"   // Every 30 minutes between 11h-16h
then
    // 1. determine if you need to run at all and if not exit
    if(vWeatherTemperature.state <= 24) return;

    // 2. calculate what needs to be done
    var State newState = if(vWeatherTemperature.state > 30) DOWN else new DecimalType(85)

    if(newState == DOWN && g_og_rollershutter.state == 100) return;
    if(newState instance of DecimalType && g_og_rollershutter.state != 0) return;

    // 3. do it
    g_og_rollershutter.sendCommand(newState)
end

Admittedly this is a few more lines of code, but with this approach, you avoid the long run on if statement conditionals and therefore it is much easier to debug.

The theory of operation is:

if the temp is less than 24 just exit because there is nothing to do
calculate the new state to send as a command to the rollershutter. Because we already know the temp is over 24 we only need to test to see if the temp is over 30 and choose between DOWN or 85 as appropriate
calculate whether we should send newState as a command or not based on the current state of the rollershutter. If we should not send the command, exit the Rule here
finally, if we got this far we know what command to send and we know we need to send it so we do it

KraxelHuber · April 22, 2018, 5:53pm

Hi Rich, thanks a lot for your input. I do see the advantages of using your strategy which is clearly superior.

However, I am struggling a bit when using your code (copy & paste). Using VS Code, I get a number of errors.

Also, would you mind to elaborate a bit on the syntax?

new DecimalType(85)
instance of DecimalType

Is ‘newState’ a variable or is it an instance of an object? I don’t fully understand.

Thanks a lot.

rlkoshak · April 22, 2018, 7:49pm

Hmm. All the errors are related to the same problem. I guess State isn’t available. Try replacing State with Command.

var Command newState = if(vWeatherTemperature.state > 30) DOWN

If that doesn’t work try removing the type entirely.

var newState = if(vWeatherTemperature.state > 30) DOWN

If that doesn’t work try using 100 instead of DOWN.

var DecimalType newState = if(vWeatherTemperature.state > 30) 100

Or perhaps it needs to be a PercentType.

Yes. Anytime you call new it creates an Object. Anytime you see var or val you are defining a variable. So that line is creating an instance if an object and assigning it to a variable.

instance of returns true if there last operand is the toe of the right operand.

KraxelHuber · April 22, 2018, 8:17pm

This line of code is producing error messages no matter which of your suggestions are used. I now tried the following solution, but I am not sure if this is all programmatically correct.

rule "Close rollershutter in first floor if too hot"
when
    Time cron "0 0/30 11-16 ? * * *"   // Every 30 minutes between 11h-16h
then
    // 1. determine if you need to run at all and if not exit
    if(vWeatherTemperature.state <= 24) return;

    // 2. calculate what needs to be done
    var newState = if(vWeatherTemperature.state > 30) 100 else 85

    if(newState == 100 && g_og_rollershutter.state == 100) return;
    if(newState == 85 && g_og_rollershutter.state != 0) return;

    // 3. do it
    g_og_rollershutter.sendCommand(newState)
end

rlkoshak · April 22, 2018, 8:21pm

The best way to find out if it works is to run it.

It looks ok to me.

KraxelHuber · April 22, 2018, 8:26pm

I am now inserting telegram msgs after each line to learn what’s really going on. I’ll post the results the other day.

KraxelHuber · April 28, 2018, 7:15pm

Here’s my current code. I think it’s working, but I need some more tests.

rule "Close rollershutter in first floor if too hot"
when
    Time cron "0 0 10-16 ? * * *"   // Every 60 minutes between 10h-16h
then
    // 1. determine if you need to run at all and if not exit
    if (vWeatherTemperature.state < 24) {
        logInfo("test.rules", "Current temperature (" + vWeatherTemperature.state.toString + "°C) is less than 24°C. No further action.")
        return;
    }
    
    // 2. calculate what needs to be done
    var Number newState
    if (vWeatherTemperature.state > 28) {
        newState=100
    } else {
        newState=85
    }
    logInfo("test.rules", "The new closing level is " + newState + "%.")

    if (newState == 100 && og_hanna_aRollade.state == 100 && og_luisa_aRollade.state == 100 && og_sz_aRollade.state == 100) {
        logInfo("test.rules", "Rollershutters have already been closed. No further action")
        return;
    }
    if (newState == 85 && og_hanna_aRollade.state != 0 && og_luisa_aRollade.state != 0 && og_sz_aRollade.state != 0) {
        logInfo("test.rules", "Rollershutters have already been partly closed. No further action")
        return;
    }

    // 3. do it
    g_og_rollershutter.sendCommand(newState)
    logInfo("test.rules","Rollershutter have been closed to" + newState + "%.")
    sendTelegram("bot_andi", "Rollershutter have been closed to " + newState + "%.")
end

rlkoshak · April 29, 2018, 2:37am

If you define g_og_rollershutter as Group:Number in then you can just check the state of the Group instead of the state of each rollershutter separately.

When you see repeated code, there is usually a way to avoid the repetitions which results in cleaner coffee which is easier to read and understand and modify.