Pls, rule code review needed - noob here ;)

Hi all,

I’m on OH2.5. I would like to ask sombody with some better knowledge to give me a code review here.

Target: read heating schedule from CUL (Homegear/BidCos) and apply the scheduled temperature. Why that? Another topic…

I’m trying to read the actual configuration of my heating system. I’m using a HTTP query HG_Url which results in

{result=success, value={ENDTIME_FRIDAY_1=470, ENDTIME_FRIDAY_10=1440, ENDTIME_FRIDAY_11=1440, ENDTIME_FRIDAY_12=1440, ENDTIME_FRIDAY_13=1440, ENDTIME_FRIDAY_2=490, ENDTIME_FRIDAY_3=1050, ENDTIME_FRIDAY_4=1410, ENDTIME_FRIDAY_5=1440, ENDTIME_FRIDAY_6=1440, ENDTIME_FRIDAY_7=1440, ENDTIME_FRIDAY_8=1440, ENDTIME_FRIDAY_9=1440, ENDTIME_MONDAY_1=470, ENDTIME_MONDAY_10=1440, ENDTIME_MONDAY_11=1440, ENDTIME_MONDAY_12=1440, ENDTIME_MONDAY_13=1440, ENDTIME_MONDAY_2=490, ENDTIME_MONDAY_3=1050, ENDTIME_MONDAY_4=1410, ENDTIME_MONDAY_5=1440, ENDTIME_MONDAY_6=1440, ENDTIME_MONDAY_7=1440, ENDTIME_MONDAY_8=1440, ENDTIME_MONDAY_9=1440, ENDTIME_SATURDAY_1=490, ENDTIME_SATURDAY_10=1440, ENDTIME_SATURDAY_11=1440, ENDTIME_SATURDAY_12=1440, ENDTIME_SATURDAY_13=1440, ENDTIME_SATURDAY_2=510, ENDTIME_SATURDAY_3=1050, ENDTIME_SATURDAY_4=1410, ENDTIME_SATURDAY_5=1440, ENDTIME_SATURDAY_6=1440, ENDTIME_SATURDAY_7=1440, ENDTIME_SATURDAY_8=1440, ENDTIME_SATURDAY_9=1440, ENDTIME_SUNDAY_1=490, ENDTIME_SUNDAY_10=1440, ENDTIME_SUNDAY_11=1440, ENDTIME_SUNDAY_12=1440, ENDTIME_SUNDAY_13=1440, ENDTIME_SUNDAY_2=510, ENDTIME_SUNDAY_3=1050, ENDTIME_SUNDAY_4=1410, ENDTIME_SUNDAY_5=1440, ENDTIME_SUNDAY_6=1440, ENDTIME_SUNDAY_7=1440, ENDTIME_SUNDAY_8=1440, ENDTIME_SUNDAY_9=1440, ENDTIME_THURSDAY_1=470, ENDTIME_THURSDAY_10=1440, ENDTIME_THURSDAY_11=1440, ENDTIME_THURSDAY_12=1440, ENDTIME_THURSDAY_13=1440, ENDTIME_THURSDAY_2=490, ENDTIME_THURSDAY_3=1050, ENDTIME_THURSDAY_4=1410, ENDTIME_THURSDAY_5=1440, ENDTIME_THURSDAY_6=1440, ENDTIME_THURSDAY_7=1440, ENDTIME_THURSDAY_8=1440, ENDTIME_THURSDAY_9=1440, ENDTIME_TUESDAY_1=470, ENDTIME_TUESDAY_10=1440, ENDTIME_TUESDAY_11=1440, ENDTIME_TUESDAY_12=1440, ENDTIME_TUESDAY_13=1440, ENDTIME_TUESDAY_2=490, ENDTIME_TUESDAY_3=1050, ENDTIME_TUESDAY_4=1410, ENDTIME_TUESDAY_5=1440, ENDTIME_TUESDAY_6=1440, ENDTIME_TUESDAY_7=1440, ENDTIME_TUESDAY_8=1440, ENDTIME_TUESDAY_9=1440, ENDTIME_WEDNESDAY_1=470, ENDTIME_WEDNESDAY_10=1440, ENDTIME_WEDNESDAY_11=1440, ENDTIME_WEDNESDAY_12=1440, ENDTIME_WEDNESDAY_13=1440, ENDTIME_WEDNESDAY_2=490, ENDTIME_WEDNESDAY_3=1050, ENDTIME_WEDNESDAY_4=1410, ENDTIME_WEDNESDAY_5=1440, ENDTIME_WEDNESDAY_6=1440, ENDTIME_WEDNESDAY_7=1440, ENDTIME_WEDNESDAY_8=1440, ENDTIME_WEDNESDAY_9=1440, TEMPERATURE_FRIDAY_1=17.0, TEMPERATURE_FRIDAY_10=17.0, TEMPERATURE_FRIDAY_11=17.0, TEMPERATURE_FRIDAY_12=17.0, TEMPERATURE_FRIDAY_13=17.0, TEMPERATURE_FRIDAY_2=20.5, TEMPERATURE_FRIDAY_3=19.0, TEMPERATURE_FRIDAY_4=20.5, TEMPERATURE_FRIDAY_5=17.0, TEMPERATURE_FRIDAY_6=17.0, TEMPERATURE_FRIDAY_7=17.0, TEMPERATURE_FRIDAY_8=17.0, TEMPERATURE_FRIDAY_9=17.0, TEMPERATURE_MONDAY_1=17.0, TEMPERATURE_MONDAY_10=17.0, TEMPERATURE_MONDAY_11=17.0, TEMPERATURE_MONDAY_12=17.0, TEMPERATURE_MONDAY_13=17.0, TEMPERATURE_MONDAY_2=20.5, TEMPERATURE_MONDAY_3=19.0, TEMPERATURE_MONDAY_4=20.5, TEMPERATURE_MONDAY_5=17.0, TEMPERATURE_MONDAY_6=17.0, TEMPERATURE_MONDAY_7=17.0, TEMPERATURE_MONDAY_8=17.0, TEMPERATURE_MONDAY_9=17.0, TEMPERATURE_SATURDAY_1=17.0, TEMPERATURE_SATURDAY_10=17.0, TEMPERATURE_SATURDAY_11=17.0, TEMPERATURE_SATURDAY_12=17.0, TEMPERATURE_SATURDAY_13=17.0, TEMPERATURE_SATURDAY_2=20.5, TEMPERATURE_SATURDAY_3=19.0, TEMPERATURE_SATURDAY_4=20.5, TEMPERATURE_SATURDAY_5=17.0, TEMPERATURE_SATURDAY_6=17.0, TEMPERATURE_SATURDAY_7=17.0, TEMPERATURE_SATURDAY_8=17.0, TEMPERATURE_SATURDAY_9=17.0, TEMPERATURE_SUNDAY_1=17.0, TEMPERATURE_SUNDAY_10=17.0, TEMPERATURE_SUNDAY_11=17.0, TEMPERATURE_SUNDAY_12=17.0, TEMPERATURE_SUNDAY_13=17.0, TEMPERATURE_SUNDAY_2=20.5, TEMPERATURE_SUNDAY_3=19.0, TEMPERATURE_SUNDAY_4=20.5, TEMPERATURE_SUNDAY_5=17.0, TEMPERATURE_SUNDAY_6=17.0, TEMPERATURE_SUNDAY_7=17.0, TEMPERATURE_SUNDAY_8=17.0, TEMPERATURE_SUNDAY_9=17.0, TEMPERATURE_THURSDAY_1=17.0, TEMPERATURE_THURSDAY_10=17.0, TEMPERATURE_THURSDAY_11=17.0, TEMPERATURE_THURSDAY_12=17.0, TEMPERATURE_THURSDAY_13=17.0, TEMPERATURE_THURSDAY_2=20.5, TEMPERATURE_THURSDAY_3=19.0, TEMPERATURE_THURSDAY_4=20.5, TEMPERATURE_THURSDAY_5=17.0, TEMPERATURE_THURSDAY_6=17.0, TEMPERATURE_THURSDAY_7=17.0, TEMPERATURE_THURSDAY_8=17.0, TEMPERATURE_THURSDAY_9=17.0, TEMPERATURE_TUESDAY_1=17.0, TEMPERATURE_TUESDAY_10=17.0, TEMPERATURE_TUESDAY_11=17.0, TEMPERATURE_TUESDAY_12=17.0, TEMPERATURE_TUESDAY_13=17.0, TEMPERATURE_TUESDAY_2=20.5, TEMPERATURE_TUESDAY_3=19.0, TEMPERATURE_TUESDAY_4=20.5, TEMPERATURE_TUESDAY_5=17.0, TEMPERATURE_TUESDAY_6=17.0, TEMPERATURE_TUESDAY_7=17.0, TEMPERATURE_TUESDAY_8=17.0, TEMPERATURE_TUESDAY_9=17.0, TEMPERATURE_WEDNESDAY_1=17.0, TEMPERATURE_WEDNESDAY_10=17.0, TEMPERATURE_WEDNESDAY_11=17.0, TEMPERATURE_WEDNESDAY_12=17.0, TEMPERATURE_WEDNESDAY_13=17.0, TEMPERATURE_WEDNESDAY_2=20.5, TEMPERATURE_WEDNESDAY_3=19.0, TEMPERATURE_WEDNESDAY_4=20.5, TEMPERATURE_WEDNESDAY_5=17.0, TEMPERATURE_WEDNESDAY_6=17.0, TEMPERATURE_WEDNESDAY_7=17.0, TEMPERATURE_WEDNESDAY_8=17.0, TEMPERATURE_WEDNESDAY_9=17.0}} 

From this input, I wish to identify in which time range I am now (e.g., between ENDTIME_FRIDAY_1 and ENDTIME_FRIDAY_2. The time range is in minutes (one day = 1440 min). Based on this information, I select the temperature to apply via item max_KZ_WT_settemp and max_KZ_T_settemp accordingly.

Now, could somebody help me in a peer review on this rule? Datatypes are correct? Which function shall I use for ActlDay (weekday today) and ActlTime (time in minutes)?
Do you see code improvements needed? This rule will be replicated on multiple items like max_XX_YY_settemp.

Code here:

rule
 when xxxxxxx
then

Calendar calendar = Calendar.getInstance();
var ActlDay = calendar.get(Calendar.DAY_OF_WEEK); //Actual Weekday
var ActlTime = ?????? //Actual time in minutes 
var MAX_id = 1 //Kinderzimmer
var MAX_channel = 0 //
var HG_Url = "http://localhost:2001/api/v1/config/" + MAX_id + "/" + MAX_channel + "/MASTER"
var String MAX_config = sendHttpGetRequest(HG_Url) //retrieve schedule from Homegear for id = MAX_id, e.g., {"result":"success","value":{"ENDTIME_FRIDAY_1":470,...}}

if(MAX_config!=null){

var ENDTIME_1 = transform("JSONPATH", "$.value.ENDTIME_" + day "_1", MAX_config)
var ENDTIME_2 = transform("JSONPATH", "$.value.ENDTIME_" + day "_2", MAX_config)
var ENDTIME_3 = transform("JSONPATH", "$.value.ENDTIME_" + day "_3", MAX_config)
var ENDTIME_4 = transform("JSONPATH", "$.value.ENDTIME_" + day "_4", MAX_config)
var ENDTIME_5 = transform("JSONPATH", "$.value.ENDTIME_" + day "_5", MAX_config)
var ENDTIME_6 = transform("JSONPATH", "$.value.ENDTIME_" + day "_6", MAX_config)
var ENDTIME_7 = transform("JSONPATH", "$.value.ENDTIME_" + day "_7", MAX_config)
var ENDTIME_8 = transform("JSONPATH", "$.value.ENDTIME_" + day "_8", MAX_config)
var ENDTIME_9 = transform("JSONPATH", "$.value.ENDTIME_" + day "_9", MAX_config)
var ENDTIME_10 = transform("JSONPATH", "$.value.ENDTIME_" + day "_10", MAX_config)
var ENDTIME_11 = transform("JSONPATH", "$.value.ENDTIME_" + day "_11", MAX_config)
var ENDTIME_12 = transform("JSONPATH", "$.value.ENDTIME_" + day "_12", MAX_config)
var ENDTIME_13 = transform("JSONPATH", "$.value.ENDTIME_" + day "_13", MAX_config)

var TEMPERATURE_1 = transform("JSONPATH", "$.value.TEMPERATURE_" + day "_1", MAX_config)
var TEMPERATURE_2 = transform("JSONPATH", "$.value.TEMPERATURE_" + day "_2", MAX_config)
var TEMPERATURE_3 = transform("JSONPATH", "$.value.TEMPERATURE_" + day "_3", MAX_config)
var TEMPERATURE_4 = transform("JSONPATH", "$.value.TEMPERATURE_" + day "_4", MAX_config)
var TEMPERATURE_5 = transform("JSONPATH", "$.value.TEMPERATURE_" + day "_5", MAX_config)
var TEMPERATURE_6 = transform("JSONPATH", "$.value.TEMPERATURE_" + day "_6", MAX_config)
var TEMPERATURE_7 = transform("JSONPATH", "$.value.TEMPERATURE_" + day "_7", MAX_config)
var TEMPERATURE_8 = transform("JSONPATH", "$.value.TEMPERATURE_" + day "_8", MAX_config)
var TEMPERATURE_9 = transform("JSONPATH", "$.value.TEMPERATURE_" + day "_9", MAX_config)
var TEMPERATURE_10 = transform("JSONPATH", "$.value.TEMPERATURE_" + day "_10", MAX_config)
var TEMPERATURE_11 = transform("JSONPATH", "$.value.TEMPERATURE_" + day "_11", MAX_config)
var TEMPERATURE_12 = transform("JSONPATH", "$.value.TEMPERATURE_" + day "_12", MAX_config)
var TEMPERATURE_13 = transform("JSONPATH", "$.value.TEMPERATURE_" + day "_13", MAX_config)

if(ENDTIME_1 <= ActlTime && ActlTime < ENDTIME_2){
max_KZ_WT_settemp.sendCommand(TEMPERATURE_1)
max_KZ_T_settemp.sendCommand(TEMPERATURE_1)
}
else if(ENDTIME_2 <= ActlTime && ActlTime < ENDTIME_3){
max_KZ_WT_settemp.sendCommand(TEMPERATURE_2)
max_KZ_T_settemp.sendCommand(TEMPERATURE_2)
}
else if(ENDTIME_3 <= ActlTime && ActlTime < ENDTIME_4){
max_KZ_WT_settemp.sendCommand(TEMPERATURE_3)
max_KZ_T_settemp.sendCommand(TEMPERATURE_3)
}
else if(ENDTIME_4 <= ActlTime && ActlTime < ENDTIME_5){
max_KZ_WT_settemp.sendCommand(TEMPERATURE_4)
max_KZ_T_settemp.sendCommand(TEMPERATURE_4)
}
else if(ENDTIME_5 <= ActlTime && ActlTime < ENDTIME_6){
max_KZ_WT_settemp.sendCommand(TEMPERATURE_5)
max_KZ_T_settemp.sendCommand(TEMPERATURE_5)
}
else if(ENDTIME_6 <= ActlTime && ActlTime < ENDTIME_7){
max_KZ_WT_settemp.sendCommand(TEMPERATURE_6)
max_KZ_T_settemp.sendCommand(TEMPERATURE_6)
}
else if(ENDTIME_7 <= ActlTime && ActlTime < ENDTIME_8){
max_KZ_WT_settemp.sendCommand(TEMPERATURE_7)
max_KZ_T_settemp.sendCommand(TEMPERATURE_7)
}
else if(ENDTIME_8 <= ActlTime && ActlTime < ENDTIME_9){
max_KZ_WT_settemp.sendCommand(TEMPERATURE_8)
max_KZ_T_settemp.sendCommand(TEMPERATURE_8)
}
else if(ENDTIME_9 <= ActlTime && ActlTime < ENDTIME_10){
max_KZ_WT_settemp.sendCommand(TEMPERATURE_9)
max_KZ_T_settemp.sendCommand(TEMPERATURE_9)
}
else if(ENDTIME_10 <= ActlTime && ActlTime < ENDTIME_11){
max_KZ_WT_settemp.sendCommand(TEMPERATURE_10)
max_KZ_T_settemp.sendCommand(TEMPERATURE_10)
}
else if(ENDTIME_11 <= ActlTime && ActlTime < ENDTIME_12){
max_KZ_WT_settemp.sendCommand(TEMPERATURE_11)
max_KZ_T_settemp.sendCommand(TEMPERATURE_11)
}
else if(ENDTIME_12 <= ActlTime && ActlTime < ENDTIME_13){
max_KZ_WT_settemp.sendCommand(TEMPERATURE_12)
max_KZ_T_settemp.sendCommand(TEMPERATURE_12)
}
else if(ENDTIME_13 <= ActlTime && ActlTime < 1440){
max_KZ_WT_settemp.sendCommand(TEMPERATURE_13)
max_KZ_T_settemp.sendCommand(TEMPERATURE_13)
}
}

else logDebug("Rule","No connection")

I would thank anybody giving a hand here.

Have a pleasant day.

Can you not just ask it for the current scheduled temperature?

https://doc.homegear.eu/homegear/communication_protocols.html#get-variable

What do you get from

curl -X GET http://localhost:2001/api/v1/channels

If there is a way to ask it what you want the set temp to be at this current moment it would be much easier to implement

The used temperature when in AUTO mode in Homegear is wrong… Actually, I’m trying to solve this issue: MAX from ECO to AUTO does not take the week schedule - Devices - Homegear Forum

That’s why I need to go either " @job way " (see his post) or “brute force” the set temperature via its schedule. My complain here is that I also have a ECO Switch, which needs some rules to set AUTO or ECO. When going to AUTO, the schedule will be respected from the next setpoint onwards.

The variable to read would be this one: Homegear Reference - Device
It’s however wrongly set (till next setpoint change). That’s why I’m overwriting with max_KZ_WT_settemp.sendCommand

Hey Sergio,

I’ve done a similar thing in openHAB to schedule my heating actors. The approach I’ve used is to define a schedule plan in an init rule.

The script can be taken from here: Rules fail at startup of openHAB

Every single schedule is set a a timer, that when fired, changes the actors to reflect temperature changes, or more accurate the HVAC mode in my case.

Each passing of midnight is also a rule, that sets the timers for the upcoming day.

You could use a similar approach with reading the schedule from CUL and set timers in openHAB accordingly.

One question that comes to my mind also is whether CUL can send push events or call an REST API when a temperature has to be changed due to firing a schedule? In that case I would setup an item in openHAB and let it trigger via REST from CUL. You can than implement a rule that runs when the item changes and delegates the temperature change.

First improvement. Never duplicate coffee like this. you can use the same rule for both. look in the docs for the triggeringItem variable and the forum for the associated items design pattern. Also look at the DRY design pattern. You should never duplicate complicated code like this.

why need with calendar? now gives you a DateTime set to the instant you called now.

val dayOfWeek = now.dayOfWeek

Put the JSONPATH lines into a for loop.

Use the comparisons built into DateTime. Assuming the below from json.is in iso8601 format

val endtime1 = new DateTime(

var ENDTIME_1 = transform(“JSONPATH”, “$.value.ENDTIME_” + day “_1”, MAX_config))

if(endtime1.isBefore(now) && now.isBefore(endtime1)) {

Why not figure out which time period you are in inside the aforementioned loop and I send command to the two items just once at the end? See the How to Structure a Rule design pattern of for more details.

I’m not able to provide code right now but this is probably a 15 line rule, tops, if you apply the above.

As a general rule, if you find yourself copying and editing the same line of code over and over, there a better way.

Give it a try and come back if you have problems with your code.

1 Like

I’m at a computer today so here is some more code to help explain my suggestions.

Another suggestion is to switch to the HTTP binding instead of the action.

Note, I’m just typing this into the forum. There will likely be typos and errors.

rule "Give every rule a unique name"
when
    Item HG_Url received update
then
    // HG_Url will contain the JSON 
    var temp = ""
    val DAY = now.getDayOfWeek

    // Loop through the JSON until we get past 13 or find the current one
    var index = 1
    while(index < 13 && temp == ""){
        val endTimeStr = transform("JSONPATH", "$.value.ENDTIME_" + DAY + "_" + index, HG_Url.state.toString)
        val endTime = new DateTime(endTimeStr)
        val nextEndTimeStr = transform("JSONPATH", "$.value.ENDTIME_" + DAY + "_" + index+1, HG_Url.state.toString)
        val nextEndTime = if(index < 13) new DateTime(nextEndTime) else now.withHourOfDay(14).withMinuteOfHour(40)
        if(endTime.isBefore(now) && now.isBefore(nextEndTime)) { // We found it!
            temp = transform("JSONPATH", "$.value.TEMPERATURE_" + DAY + "_" + index, HG_Url.state.toString)
        }        
    }

    if(temp != "") {
        max_KZ_WT_settemp.sendCommand(temp)
        max_KZ_T_settemp.sendCommand(temp)      
    }
    else{
       logDebug("Rule","No connection")
    }
end 

There are no duplicated lines. It’s about 20 lines of code so I was a bit off on my original estimate.

1 Like

@rlkoshak, @franks
first of all, thank you!
I would need to have a deeper look at your posts. I have some posts to digest during my weekend :slight_smile: I really appreciate your efforts here.