It have two problems.
First, I need to wait for an update of the netamo item, (ten minutes).
And I just looked at the logfile. There is something wrong with the alarm state… This is the error I get when temperature is firing the rule:
2018-08-08 23:09:25.437 [ERROR] [ntime.internal.engine.RuleEngineImpl] - Rule 'Automatic control of all skylight windows': Could not cast OFF to void; line 15, column 17, length 35
Line 15 is val alarm = alarm_totalalarm.state as OnOfftype?
Let’s leave off the cast. It usually isn’t needed in the first place.
val alarm = alarm_totalalarm.state
One way to test a Rule like this is to add an Unbound Switch Item as a trigger to the Rule and put that Switch on your sitemap so you can trigger the Rule by hand whenever you want without needing to wait for the other Items to update. Once the Rule works you can remove the Switch, or keep it around and use it to test future Rules.
I thought it did, untill I saw you code
Just forgot to think, I guess. I already have the val´s in the rule to use. So it makes sense using one line covering each case.
And it work likes a charm!
2018-08-09 00:32:24.398 [ome.event.ItemCommandEvent] - Item 'dummy1' received command ON
2018-08-09 00:32:24.409 [vent.ItemStateChangedEvent] - dummy1 changed from OFF to ON
2018-08-09 00:32:24.519 [INFO ] [ipse.smarthome.model.script.skylight] - Sending ON command to VeluxAlleVent because Temp = 22.1000003814697265625 ℃ Lux = 0 and Alarm = OFF
2018-08-09 00:32:24.524 [ome.event.ItemCommandEvent] - Item 'VeluxAlleVent' received command ON
2018-08-09 00:32:28.601 [INFO ] [ding.velux.bridge.VeluxBridgeExecute] - execute() finished successfully.
There is something wrong with the cases.
When I got up this morning, all windows was 100% open, even though the temperature isn´t high enough.
This is from the log where it opens the windows.
2018-08-09 06:03:32.706 [INFO ] [ipse.smarthome.model.script.skylight] - Sending ON command to VeluxAlleVent because Temp = 17.700000762939453125 ℃ Lux = 54 and Alarm = OFF
2018-08-09 06:03:36.957 [INFO ] [ding.velux.bridge.VeluxBridgeExecute] - execute() finished successfully.
2018-08-09 06:13:34.381 [INFO ] [ipse.smarthome.model.script.skylight] - Sending ON command to VeluxAlleVent because Temp = 17.6000003814697265625 ℃ Lux = 54 and Alarm = OFF
2018-08-09 06:13:38.568 [INFO ] [ding.velux.bridge.VeluxBridgeExecute] - execute() finished successfully.
2018-08-09 06:17:02.086 [INFO ] [ipse.smarthome.model.script.skylight] - Sending ON command to VeluxAlleAaben100 because Temp = 17.6000003814697265625 ℃ Lux = 109 and Alarm = OFF
2018-08-09 06:17:06.193 [INFO ] [ding.velux.bridge.VeluxBridgeExecute] - execute() finished successfully.
2018-08-09 06:31:44.901 [INFO ] [ipse.smarthome.model.script.skylight] - Sending ON command to VeluxAlleAaben100 because Temp = 17.6000003814697265625 ℃ Lux = 160 and Alarm = OFF
It seems like, as soon as the lux is above 100, then it sends ON to VeluxAlleAaben100 which is not what the rule is suppose to. This is probably cause because I changed the criteria for the cases last night before I went to bed, (stupid change).
Anyway… After I have been thinking about once again, and had some sleep. I´ve come up with this idea, which I hope make more sense.
rule "Automatic control of all skylight windows"
when
Item NetamoUdendoersTemperature changed or
Item Node13_SensorLuminance changed or
Item alarm_totalalarm changed or
Item dummy1 changed
then
// Exit the rule when there is nothing to do
if(alarm_totalalarm.state instanceof Switch ) return;
if(!(Node13_SensorLuminance.state instanceof Number)) return;
if(!(NetamoUdendoersTemperature.state instanceof Number)) return;
// Calculate which velux to send the ON command to
val Number fTemp = NetamoUdendoersTemperature.state as Number
val Number lux = Node13_SensorLuminance.state as Number
val alarm = alarm_totalalarm.state
var velux = VeluxAlleLuk
switch fTemp {
case fTemp >= 25 && lux >= 100: velux = VeluxAlleAaben100
case fTemp >= 23 && lux >= 100: velux = VeluxAlleAaben75
case fTemp >= 21 && lux >= 100: velux = VeluxAlleAaben50
case fTemp < 21 || lux < 100, && alarm == OFF,
case alarm == ON: velux = VeluxAlleVent
case fTemp <= 20 || lux < 100, && alarm == OFF,
case alarm == ON: velux = VeluxAlleLuk
}
// Send the command
logInfo("skylight", "Sending ON command to " + velux.name + " because Temp = " + NetamoUdendoersTemperature.state + " Lux = " + Node13_SensorLuminance.state + " and Alarm = " + alarm_totalalarm.state)
velux.sendCommand(ON)
end
case fTemp < 21 || lux < 100, && alarm == OFF,
^ here
case fTemp <= 20 || lux < 100, && alarm == OFF,
^ and here
I’m pretty sure the || statement will not work as intended in this case (at least you would have to do some brackets around)
Please keep in mind, that only one case will be executed, even when there are several cases which are true.
Indeed || means or. So let’s take case fTemp < 21 || lux < 100, && alarm == OFF,. The operators are evaluated left to right so if fTemp < 21, the expression will always evaluate to true regardless if whether alarm is OFF or not. If fTemp >= 21 then the expression will only evaluate to true if lux < 100 and the alarm is OFF. You need to add parens to force the fTemp and lux comparisons to occur first. case (fTemp < 21 || lux < 100) && alarm == OFF,. This way it checks both the fTemp and lux and if either condition is true then it checks the alarm and only if both evaluate to true does the case evaluate to true.
Even with the simplifications this is starting to get really complex and hard to keep in ones head. Try to see if there is a way to consider the three conditions in a different order that may make it takes more sense.
For example, you only care if lux is over it under 100, so perhaps evaluate that separately into a boolean. We can do the same with the alarm.
And perhaps it would help if we split it up a bit. Maybe handle the lux separately.
val Number fTemp = NetamoUdendoersTemperature.state as Number
val lux = Node13_SensorLuminance.state as Number >= 100
val alarm = alarm_totalalarm.state == ON
var velux = VeluxAlleLuk
if(lux) {
switch fTemp {
case fTemp >= 25: velux = VeluxAlleAaben100
case fTemp >= 23: velux = VeluxAlleAaben75
case fTemp >= 21: velux = VeluxAlleAaben50
}
}
else {
if(fTemp < 20 && !alarm) velux = VeluxAlleVent
if(alarm) velux = VeluxAlleVent
}
// Send the command
logInfo("skylight", "Sending ON command to " + velux.name + " because Temp = " + NetamoUdendoersTemperature.state + " Lux = " + Node13_SensorLuminance.state + " and Alarm = " + alarm_totalalarm.state)
velux.sendCommand(ON)
Ahh I see, it makes sense, cause ofcouse I needed fTemp and lux to evaluate first (or together).
My head was about to explode trying to make the conditions more simple. I simple couldn´t find a way, untill you just showed me.
Correct, rule need to evaluate Lux beeing over or under 100. This is to make sure the windows always will either closed to ventilation at night, OR windows should close totally, all depending on the temperature.
Been away for a few days with a trip to London. But now I´m back an ready to conitnue…
First - The rule still doesn´t work as it´s suppose to. It stil ignores criterias, which I simply cant figure.
This is the latest rule:
rule "Automatic control of all skylight windows"
when
Item NetamoUdendoersTemperature changed or
Item Node13_SensorLuminance changed or
Item alarm_totalalarm changed or
Item dummy1 changed
then
// Exit the rule when there is nothing to do
if(alarm_totalalarm.state instanceof Switch ) return;
if(!(Node13_SensorLuminance.state instanceof Number)) return;
if(!(NetamoUdendoersTemperature.state instanceof Number)) return;
// Calculate which velux to send the ON command to
val Number fTemp = NetamoUdendoersTemperature.state as Number
val Number lux = Node13_SensorLuminance.state as Number
val alarm = alarm_totalalarm.state
var velux = VeluxAlleLuk
switch fTemp {
case fTemp >= 25 && lux >= 100: velux = VeluxAlleAaben100
case fTemp >= 23 && lux >= 100: velux = VeluxAlleAaben75
case fTemp >= 21 && lux >= 100: velux = VeluxAlleAaben50
case fTemp < 21 || lux < 100 && alarm == OFF,
case alarm == ON: velux = VeluxAlleVent
case (fTemp <= 20 || lux < 100) && alarm == OFF,
case alarm == ON: velux = VeluxAlleLuk }
// Send the command
logInfo("skylight", "Sending ON command to " + velux.name + " because Temp = " + NetamoUdendoersTemperature.state + " Lux = " + Node13_SensorLuminance.state + " and Alarm = " + alarm_totalalarm.state)
velux.sendCommand(ON)
end
When i got up this morning, all windows were 100% open.
This is the log entry:
2018-08-15 08:01:14.723 [INFO ] [ipse.smarthome.model.script.skylight] - Sending ON command to VeluxAlleAaben100 because Temp = 16.6000003814697265625 ? Lux = 439 and Alarm = OFF
As you can see, the temperature is way below 20 degrees. Lux is fine and alarm is fine. But windows should not be able to open 100% (VeluxAlleAaben100) when temperature is 16 degrees, infact windows should be fully closed (VeluxAlleLuk).
Now lets go through the switch statement line by line and see what evaluates to true…
The first three cases evaluate to false because fTemp < 21.
The next case evaluates to true. fTemp < 21. It doesn’t matter what lux and alarm are set to because the boolean operations are evaluated left to right, unless you group them using parens.
fTemp < 21 || lux < 100 && alarm == OFF is the same as fTemp < 21 II (lux < 100 && alarm == OFF)
If you change the evaluation to fTemp < 21 && lux < 100 && alarm == OFF then it would evaluate to false because all three cases need to be true for the condition to be true.
If you change the evaluation to (fTemp < 21 || lux < 100) && alarm == OFF then it would evaluate to true because either fTemp < 21 or lux < 100 evaluates to true, then we check to see if alarm is also OFF. Since it is OFF that means the whole condition evaluates to true.
Thus, because fTemp < 21 VeluxAlleVent gets the command.
There is another problem here. You have two case alarm == ON. Only the first one will ever evaluate. Switch statements work in order and execute the first case whose condition evaluates to true. So if alarm==ON VeluxAlleVent will always get the command, never VeluxAlleLuk.
Well, as I suggested above, it is very hard for experienced programmers even to keep such complicated boolean logic in their mind all at the same time.
Split the boolean logic up to see if you can make it simpler to understand like the example I provided above that separates out the calculation of the lux.
You must explicitly test for this case. And you need to place this test in the proper order compared to the other tests because only the first one that evaluates to true will execute.
This is really really complicated logic. It’s not complicated to write, it’s complicated to understand and figure out. It’s like a test on the final from a college level Discrete Math course.
That is why I recommend splitting it up as I did in the post above.
I know you did, but I have difficulties understanding what exactly you mean by splitting it up. Thats why I took the other version of the rule, and trying to get a grib from it. But I really do wish to understand how things works.
Rather than putting all the conditions in one line with a bunch of || and &&, use separate if statements to separate the cases from each other.
Above I split the cases using lux. If the lux is 100 or over then there is the simple switch statement that chooses the right setting based on the temp.
If the lux is below 100 it chooses the setting based on the temp and the alarm.
The code above is based on the conditions you already wrote which apparently are not correct. But I’m not in a position where I can correct them because you have so many conditions and subconditions and special cases it is all but impossible for my to just say “do this”. Unfortunately this is going to require you to work at understanding both your conditions.
I don’t know how to tell you how to figure out the right conditions but I suspect that you might have contradictory conditions.
Maybe see if you can create a chart that lists all the combinations of the three conditions and maybe a contradiction will become apparent. But consider this, you have 522 (5 temps, 2 lux, and 2 alarm) = 20 separate conditions. It is all but impossible to reason through that many things all at the same time.
But, when you split it up a bit, like the way I split it on lux above, I can remote one of those factors which takes the total from 20 down to two separate cases of 10. That is getting closer to reasonable (5-7 is the rule of thumb for the number of different pieces of data you can reason about mentally).
This is the best I can come up with. I have simplified the temperature values a bit.
Maybe the solution is to code this in two different rules. One for Alarm ON, and one for Alarm OFF.
Also I wonder if it would be possible to have a switch turning this automatic controle rules ON/OFF. But this is just a ‘nice to heave feature’ for now.
if (Alarm.state == OFF && (LUX.state as Number) >= 100) {
first part of table
}
else {
if ((fTemp.state as Number) >= 22) VeluxAlleVent
else VeluxAlleLuk
}
I don’t think that is necessary. You can split it into three if statements with switch inside. This is what I was meaning by splitting up a bit. Rather than trying to handle Alarm, Lux, and Temp all in one expression.
To fill out Udo’s idea:
rule "Automatic control of all skylight windows"
when
Item NetamoUdendoersTemperature changed or
Item Node13_SensorLuminance changed or
Item alarm_totalalarm changed or
Item dummy1 changed
then
// Exit the rule when there is nothing to do
if(alarm_totalalarm.state instanceof Switch ) return;
if(!(Node13_SensorLuminance.state instanceof Number)) return;
if(!(NetamoUdendoersTemperature.state instanceof Number)) return;
// Calculate which velux to send the ON command to
val Number fTemp = NetamoUdendoersTemperature.state as Number
val Number lux = Node13_SensorLuminance.state as Number
val alarm = alarm_totalalarm.state
var velux = VeluxAlleLuk
// Third table
if(alarm == ON) velux = if(fTemp >= 22) VeluxAlleLuk else VeluxAlleLuk
// Second table, we already know alarm isn't ON so we don't have to test it for OFF here
else if(lux < 100) velux = if(fTemp >= 22) VeluxAlleVent else VelusAlleLuk
// First table, we know that alarm isn't ON and we know lux >= 100 so we don't have to test for it here
else {
switch fTemp {
case fTemp >= 25: velux = VelusAlleAaben100
case fTemp >= 24: velux = VeluxAlleAaben75
case fTemp >= 23: velux = VeluxAlleAaben50
case fTemp >= 22: velux = VeluxAlleVent
default: velux = VeluxAlleLuk
}
}
// Send the command
logInfo("skylight", "Sending ON command to " + velux.name + " because Temp = " + NetamoUdendoersTemperature.state + " Lux = " + Node13_SensorLuminance.state + " and Alarm = " + alarm_totalalarm.state)
velux.sendCommand(ON)
end
I notice that the behavior is the same if Alarm is ON or if Lux < 100 so we can combine those into one if statement.