Issue with final parameters when using lambda to create "function"

I noticed I was copying rules to perform the same logic but on different items.
So I went looking for methods/functions to use rather then having duplicate code.
This wiki article gave me a good start. https://github.com/openhab/openhab1-addons/wiki/Reusable-Rules-via-Functions
However I can’t quite seem to get the details right. The lambda appears to have trouble referring back to a Boolean causing and error at run-time.

2018-05-14 01:24:39.375 [INFO ] [el.core.internal.ModelRepositoryImpl] - Validation issues found in configuration model 'thermostats.rules', using it anyway:
Function4 is a raw type. References to generic type Function4<P1, P2, P3, P4, Result> should be parameterized
Assignment to final parameter
Assignment to final parameter
Assignment to final parameter
Assignment to final parameter
2018-05-14 01:24:40.560 [INFO ] [el.core.internal.ModelRepositoryImpl] - Refreshing model 'thermostats.rules'
2018-05-14 01:25:48.830 [ERROR] [ntime.internal.engine.RuleEngineImpl] - Rule 'test thermostat2': Couldn't invoke 'assignValueTo' for feature param frozen
2018-05-14 01:25:49.835 [ERROR] [ntime.internal.engine.RuleEngineImpl] - Rule 'test thermostat2': Couldn't invoke 'assignValueTo' for feature param frozen
2018-05-14 01:25:50.836 [ERROR] [ntime.internal.engine.RuleEngineImpl] - Rule 'test thermostat2': Couldn't invoke 'assignValueTo' for feature param frozen
2018-05-14 01:25:51.850 [ERROR] [ntime.internal.engine.RuleEngineImpl] - Rule 'test thermostat2': Couldn't invoke 'assignValueTo' for feature param frozen
2018-05-14 01:25:52.843 [ERROR] [ntime.internal.engine.RuleEngineImpl] - Rule 'test thermostat2': Couldn't invoke 'assignValueTo' for feature param frozen

In addition to this I updated my Visual Studio Code to 1.23.1 since it said there was a new version.
But now it is giving me loads of errors on files where it previously saw no errors and which have been working just fine.
Any one ran into that as well?

Here is the .rules in question.
The Boolean’s are to prevent rapid switching on and off which would be detrimental to the heating system. This rule consists of two main parts one is the room thermostat which measures the temperature and opens the radiator valve. The second part is the switch that turns on the central heating.

The “function” is executed, but when it comes to setting the Boolean it fails as it is interpreted as a final parameter.
The same code (which is there but commented out. works just fine as it is referencing the objects directly.

// Imports

// Global Variables
var Boolean HVAC_CM_Target_Temp_frozen = false
var Boolean HVAC_MB_Target_Temp_frozen = false
var Boolean central_heater_frozen = false
val org.eclipse.xtext.xbase.lib.Functions$Function4 themostatlogic = [
	Boolean frozen, 
  NumberItem current_temperature,
  NumberItem target_temperature,
  SwitchItem heater |
	if (!frozen) {
    if ((current_temperature.state as DecimalType) < (target_temperature.state as DecimalType)) {
      if (heater.state.toString == "OFF") {
        frozen = true
        heater.sendCommand("ON")
        CENTRAL_GAS_HEATING.sendCommand("ON")
        createTimer(now.plusMinutes(1), [| frozen = false ] ) 
      }
    } else {
      if (heater.state.toString == "ON") { 
        frozen = true
        heater.sendCommand("OFF")
        CENTRAL_GAS_HEATING.sendCommand("OFF")
        createTimer(now.plusMinutes(1), [| frozen = false ] )
      }
    }
  } else {
    createTimer(now.plusSeconds(30), [| target_temperature.postUpdate((target_temperature.state as DecimalType)) ] )
  }
]

rule "central heater"
when
  Item CENTRAL_GAS_HEATING received update
then
  if (!central_heater_frozen) {
    if (CENTRAL_GAS_HEATING.state.toString == "ON") {
      if (DO4.state.toString == "OFF") {
        central_heater_frozen = true
        createTimer(now.plusSeconds(30), [| DO4.sendCommand("ON") ] )
        createTimer(now.plusMinutes(2), [| central_heater_frozen = false ] ) 
      }
    } else {
      if ((DO4.state.toString == "ON") && (Heaters.state.toString == "OFF")) { 
        central_heater_frozen = true
        DO4.sendCommand("OFF")
        createTimer(now.plusMinutes(2), [| central_heater_frozen = false ] )
      }
    }
  } else {
    if (CENTRAL_GAS_HEATING.state.toString == "ON") {
      createTimer(now.plusSeconds(30), [| CENTRAL_GAS_HEATING.sendCommand("OFF") ] )
    } else {
      createTimer(now.plusSeconds(30), [| CENTRAL_GAS_HEATING.sendCommand("ON") ] )
    }
  }
end

rule "test thermostat"
when
  Item HVAC_CM_Temp received update or
  Item HVAC_CM_Target_Temp received update
then
  if (!HVAC_CM_Target_Temp_frozen) {
    if ((HVAC_CM_Temp.state as DecimalType) < (HVAC_CM_Target_Temp.state as DecimalType)) {
      if (DO2.state.toString == "OFF") {
        HVAC_CM_Target_Temp_frozen = true
        DO2.sendCommand("ON")
        CENTRAL_GAS_HEATING.sendCommand("ON")
        createTimer(now.plusMinutes(1), [| HVAC_CM_Target_Temp_frozen = false ] ) 
      }
    } else {
      if (DO2.state.toString == "ON") { 
        HVAC_CM_Target_Temp_frozen = true
        DO2.sendCommand("OFF")
        CENTRAL_GAS_HEATING.sendCommand("OFF")
        createTimer(now.plusMinutes(1), [| HVAC_CM_Target_Temp_frozen = false ] )
      }
    }
  } else {
    createTimer(now.plusSeconds(30), [| HVAC_CM_Target_Temp.postUpdate((HVAC_CM_Target_Temp.state as DecimalType)) ] )
  }
end

rule "test thermostat2"
when
  Item HVAC_MB_Temp  received update or
  Item HVAC_MB_Target_Temp received update
then
  themostatlogic.apply(HVAC_MB_Target_Temp_frozen, HVAC_MB_Temp, HVAC_MB_Target_Temp, DO5)
  // The bit I would like to not repeat for every single room
  // if (!HVAC_MB_Target_Temp_frozen) {
  //   if ((HVAC_MB_Temp .state as DecimalType) < (HVAC_MB_Target_Temp.state as DecimalType)) {
  //     if (DO5.state.toString == "OFF") {
  //       HVAC_MB_Target_Temp_frozen = true
  //       DO5.sendCommand("ON")
  //       CENTRAL_GAS_HEATING.sendCommand("ON")
  //       createTimer(now.plusMinutes(1), [| HVAC_MB_Target_Temp_frozen = false ] ) 
  //     }
  //   } else {
  //     if (DO5.state.toString == "ON") { 
  //       HVAC_MB_Target_Temp_frozen = true
  //       DO5.sendCommand("OFF")
  //       CENTRAL_GAS_HEATING.sendCommand("OFF")
  //       createTimer(now.plusMinutes(1), [| HVAC_MB_Target_Temp_frozen = false ] )
  //     }
  //   }
  // } else {
  //   createTimer(now.plusSeconds(30), [| HVAC_MB_Target_Temp.postUpdate((HVAC_MB_Target_Temp.state as DecimalType)) ] )
  // }
end

If i understand correctly this is something java lambdas cannot do.
A workaround to this may be using persistence to figure out when it’s last state change was.
Any one have experience with this approach?

using the persistence to check whether there has been an update to a switch recently I have been able to work around the issue. See the below code.

Though according to Visual Studio Code there are still errors in there. Any idea how to fix that or how to stop VSC from throwing these?

Ambiguous binary operation.
The operator declarations
operator_lessThan(Type, Number) in NumberExtensions and
operator_lessThan(Number, Number) in NumberExtensions
both match.

Ambiguous feature call.
The extension methods
postUpdate(Item, State) in BusEvent and
postUpdate(Item, Number) in BusEvent
both match.

Function3 is a raw type. References to generic type Function3<P1, P2, P3, Result> should be parameterized

// Imports

// Global Variables
val org.eclipse.xtext.xbase.lib.Functions$Function3 themostatlogic = [
  NumberItem current_temperature,
  NumberItem target_temperature,
  SwitchItem heater |
	if (!heater.updatedSince(now.minusMinutes(1))) {
    if ((current_temperature.state as DecimalType) < (target_temperature.state as DecimalType)) {
      if (heater.state.toString == "OFF") {
        heater.sendCommand("ON")
        CENTRAL_GAS_HEATING.sendCommand("ON")
      }
    } else {
      if (heater.state.toString == "ON") { 
        heater.sendCommand("OFF")
        CENTRAL_GAS_HEATING.sendCommand("OFF")
      }
    }
  } else {
    createTimer(now.plusSeconds(30), [| target_temperature.postUpdate((target_temperature.state as DecimalType)) ] )
  }
]

rule "central heater"
when
  Item CENTRAL_GAS_HEATING received update
then
  if (!DO4.updatedSince(now.minusMinutes(2))) {
    if (CENTRAL_GAS_HEATING.state.toString == "ON") {
      if ((DO4.state.toString == "OFF") && (Heaters.state.toString == "ON")) {
        createTimer(now.plusSeconds(30), [| DO4.sendCommand("ON") ] )
      }
    } else {
      if ((DO4.state.toString == "ON") && (Heaters.state.toString == "OFF")) { 
        DO4.sendCommand("OFF")
      }
    }
  } else {
    if (CENTRAL_GAS_HEATING.state.toString == "ON") {
      createTimer(now.plusSeconds(30), [| CENTRAL_GAS_HEATING.sendCommand("ON") ] )
    } else {
      createTimer(now.plusSeconds(30), [| CENTRAL_GAS_HEATING.sendCommand("OFF") ] )
    }
  }
end

rule "test thermostat"
when
  Item HVAC_CM_Temp received update or Item HVAC_CM_Target_Temp received update
then
  themostatlogic.apply(HVAC_CM_Temp, HVAC_CM_Target_Temp, DO2)
end

rule "test thermostat2"
when
  Item HVAC_MB_Temp  received update or Item HVAC_MB_Target_Temp received update
then
  themostatlogic.apply(HVAC_MB_Temp, HVAC_MB_Target_Temp, DO5)
end

Did you look at:

or

That is correct. The variables passed to a lambda are passed as vals, not vars so you cannot change them. One sort of work around is to use a collection (Map, List, etc) as the variable itself is a val but you can setill change the contents of the collection from inside your lambda.

One thing to know about the way VSCode works is it connects to your actual OH server and runs the code through the exact same compiler that OH uses to parse and load the rules files. So errors that show up in VSCode are real errors and will almost certainly show up in openhab.log as well. What probably happened was you were using a version of the openHAB extension from before the connection to the language server in OH was added.

The problems you are seeing have always been problems. And you should have been seeing them in openhab.log as well.

Cast both sides of the < operation to Number. The problem is that the first argument is a Type and it is also a Number so the Rules Engine cannot tell which version of the < operator you intend to call.

The same goes for the postUpdate. This is usually caused when you cast the state of a Number Item to DecimalType instead of Number, or fail to cast the state of a Number Item at all. This is why I always recommend casting to Number and never casting to DecimalType.

As shown in the link Vincent posted, you should (this is just a warning so it isn’t an error that stops anything from working) provide the types of the arguments to the lambda:

val Functions$Function3 <NumberItem, NumberItem, SwitchItem, Timer> themostatlogic = [ current_temperature, target_temperature, heater |

NOTE, is it not clear what your lambda is supposed to be returning. The last argument type (Timer) defines the return type of the lambda. The return value of the last line of the lambda is what gets returned. In the else at least that is a Timer.

If you don’t need a return value then use Procedures$Procedure and omit the “Timer” between the < >.

Having said all of that, it really looks like this is a good candidate for using Design Pattern: Associated Items and the triggeringItem implicit variable which would let you use just one rule and avoid the lambda entirely.

The Associated Items pattern is indeed a great idea to further simplify this repeating logic without even having to change the rules when adding rooms, and avoid mishaps while creating/managing rules per room.
I will be changing it over to use that pattern and update this post with the results.
. . .
So here is the result.
It is considerably neater and takes better advantage of the groups so additional rooms don’t need additional rules. I am missing the triggering item withing a group though (yes looking forward to seeing that in OH 2.3). Right now I have to resort to another loop thru the group. basically updating all of them on every change.

What this still could benefit a lot from is a more elegant way of dealing with a group filter not returning an item (configuration fault yes but it would be way better to log what went wrong then get a rather ambiguous null reference error.

I would also like to use another method then the timer for dealing with the switching cool-down. What you’d ideally want is a way to store the trigger and have that stored trigger be overwritten by any subsequent triggers of the same item, until the cool-down expires and then deal with the last trigger.
This because some of the temperature sensing can be jumping back and forth as it wibble wobbels on the barrier between two values. This results in a whole bunch of those timers being created.

// Functional perspective
Group:Number Sensors
Group:Switch Actuators
Group:Switch:OR(ON, OFF) Ringers (Actuators)
Group:Switch:OR(ON, OFF) Heaters (Actuators)
Group:Number:SUM ThermostatsTemperatures (Sensors)
Group:Number:SUM Temperatures5min (Sensors)
Group:Number:SUM Thermostats (Sensors)
Group:Number:SUM DoorButtons (Sensors)

Switch CENTRAL_GAS_HEATING "Central Gas Heating"

// itemtype itemname               "labeltext [stateformat]"              <iconname>    (group1,group2,...)     ["tag1", "tag2", ...] {bindingconfig}
// ---------------------------------------------------------------------------------------------------------------------------------------------------------- //

// -- Thermostat Items -- //
String      HVAC_Mode              "HVAC Mode [%s]"                       <switch>                             ["HVACMode"]
String      HVAC_Schedule          "HVAC Schedule [%s]"                   <calendar>                           ["HVACSchedule"]
String      HVAC_Queue             "HVAC Queue [%s]"                                                           ["HVACQueue"]
DateTime    HVAC_Next_Change_Time  "HVAC Next Change Time [%1$ta %1$tR]"  <calendar>                           ["HVACNextChange"]
Number      HVAC_CR_Temp           "Computer Room Temp [%.0f °C]"         <temperature> (ThermostatsTemperatures) ["CRTemp"]
Number      HVAC_CR_Target_Temp    "Computer Room Target Temp [%.0f °C]"  <temperature> (Thermostats)          ["CRTargetTemp"]
Number      HVAC_MB_Temp           "Master Bedroom Temp [%.0f °C]"        <temperature> (ThermostatsTemperatures) ["MBTemp"]
Number      HVAC_MB_Target_Temp    "Master Bedroom Target Temp [%.0f °C]" <temperature> (Thermostats)          ["MBTargetTemp"]

Switch HVAC_CR_HEATER "DO2 HVAC_CM_Heater" (Heaters) {snmp="<[xxx.xxx.xxx.xxx:redacted:.1.3.6.1.4.1.32111.1.1.2.2:1000:MAP(daenetip4.map)] >[OFF:xxx.xxx.xxx.xxx:redacted:.1.3.6.1.4.1.32111.1.1.2.2:1] >[ON:xxx.xxx.xxx.xxx:redacted:.1.3.6.1.4.1.32111.1.1.2.2:0]"}
Switch HVAC_MB_HEATER "DO5 HVAC_MB_HEATER" (Heaters) {snmp="<[xxx.xxx.xxx.xxx:redacted:.1.3.6.1.4.1.32111.1.1.2.5:1000:MAP(daenetip4.map)] >[OFF:xxx.xxx.xxx.xxx:redacted:.1.3.6.1.4.1.32111.1.1.2.5:1] >[ON:xxx.xxx.xxx.xxx:redacted:.1.3.6.1.4.1.32111.1.1.2.5:0]"}

// Imports

// Global Variables

rule "central heater"
when
  Item CENTRAL_GAS_HEATING received update
then
  if (!DO4.updatedSince(now.minusMinutes(2))) {
    if (CENTRAL_GAS_HEATING.state.toString == "ON") {
      if ((DO4.state.toString == "OFF") && (Heaters.state.toString == "ON")) {
        logInfo("central heater", "switching on central heating")
        createTimer(now.plusSeconds(30), [| DO4.sendCommand("ON") ] )
      }
    } else {
      if ((DO4.state.toString == "ON") && (Heaters.state.toString == "OFF")) { 
        logInfo("central heater", "switching off central heating")
        DO4.sendCommand("OFF")
      }
    }
  } else {
    logInfo("central heater", "unable to switch central heating, recently switched")
    if (CENTRAL_GAS_HEATING.state.toString == "ON") {
      createTimer(now.plusSeconds(30), [| CENTRAL_GAS_HEATING.sendCommand("ON") ] )
    } else {
      createTimer(now.plusSeconds(30), [| CENTRAL_GAS_HEATING.sendCommand("OFF") ] )
    }
  }
end

rule "Generic Thermostat"
when
  Item Thermostats received update or
  Item ThermostatsTemperatures received update
then
  Thermostats.members.forEach[thermostat|
    // until triggeringItem has the functionality to point ot the trigering item within a group we will need to loop thru all of them
    val thermostatNameParts = thermostat.name.split("_")
    val thermostatNameStart = thermostatNameParts.get(0) + "_" + thermostatNameParts.get(1)
    val thermostatTemperatureName = thermostatNameStart + "_Temp"
    val thermostatName = thermostatNameStart + "_Target_Temp"
    val thermostatHeaterName = thermostatNameStart + "_HEATER"
    val target_temperature = Thermostats.members.filter[ th|th.name == thermostatName ].head as NumberItem
    val current_temperature = ThermostatsTemperatures.members.filter[ tt| tt.name == thermostatTemperatureName ].head as NumberItem
    val heater = Heaters.members.filter[ h|h.name == thermostatHeaterName ].head as SwitchItem
	  if (!heater.updatedSince(now.minusMinutes(1))) {
      if ((current_temperature.state as Number) < (target_temperature.state as Number)) {
        if (heater.state.toString == "OFF") {
          heater.sendCommand("ON")
          CENTRAL_GAS_HEATING.sendCommand("ON")
        }
      } else {
        if (heater.state.toString == "ON") { 
          heater.sendCommand("OFF")
          CENTRAL_GAS_HEATING.sendCommand("OFF")
        }
      }
    } else {
      createTimer(now.plusSeconds(30), [| target_temperature.postUpdate((target_temperature.state as Number)) ] )
    }
  ]
end

The above rules still produce a lot of the following errors then the rule is triggered.
I figure it is in part to do with the use of timers.
Not so sure what “Rule ‘Generic Thermostat’: null” is referring to though

2018-05-15 01:11:53.316 [ERROR] [ntime.internal.engine.RuleEngineImpl] - Rule 'Generic Thermostat': null
2018-05-15 01:11:53.192 [ERROR] [model.script.actions.ScriptExecution] - Failed to schedule code for execution.
org.quartz.ObjectAlreadyExistsException: Unable to store Job : 'DEFAULT.2018-05-15T01:12:23.190+02:00: Proxy for org.eclipse.xtext.xbase.lib.Procedures$Procedure0: [ | {
  <XFeatureCallImplCustom>.postUpdate(<XCastedExpressionImpl>)
} ]', because one already exists with this identification.

This is usually handled using hysteresis. At a high level you create a buffer between the temp where you turn it in and where you turn it off where you do nothing. For example:

if( CurrTemp.state > TargetTemp.state) turn on cooler
else if (CurrTemp.state < TargetTemp.state - 2) turn off the cooler

The buffer keeps it from rapidly switching between the two states because it has to be at least two degrees different (in this case). Just make sure to set the buffer larger than the natural noise in the data.

The null error is probably caused by a type missmatch. Add logging statements to the role to find the line it is failing on and then check the items and variables in user in that line.

The second sorry is a new one for me. It is clearly happening on one of the createTimer lines, but added logging to see which one.

I will indeed need to build in a buffer but this current sensor is only accurate to a degree Celsius so it makes sense to use one that is more accurate so the noise itself is smaller.

I have been unable to find the cause of the “[ntime.internal.engine.RuleEngineImpl] - Rule ‘Generic Thermostat’: null” messages. Any more tip on how to catch those (other then more logging? Still working on that.)

However looking at the code and how I would like to reduce the use of those timers I went about using another approach which also reduces the workload for the updates that don’t require any action being taken. It does slightly decrease the response time because if the 1 min release.
Now there is only one timer created per thermostat and once it is released it useses the temperature value of that moment rather then the one at the time the timer was set.

I still have a warning “The constructor DateTimeType(Calendar) is deprecated” though I have not found a good example of what would be the new, correct way of using it.

rule "Generic Thermostat"
when
  Item Thermostats received update or
  Item ThermostatsTemperatures received update or
  Item ThermostatsReleases received update
then
  Thermostats.members.forEach[thermostat|
    // until triggeringItem has the functionality to point ot the trigering item within a group we will need to loop thru all of them
    val thermostatNameParts = thermostat.name.split("_")
    val thermostatNameStart = thermostatNameParts.get(0) + "_" + thermostatNameParts.get(1)
    val thermostatReleaseName = thermostatNameStart + "_release"
    val release = ThermostatsReleases.members.filter[ r|r.name == thermostatReleaseName ].head as DateTimeItem
    if(release.state === NULL || (release.state as DateTimeType).zonedDateTime.toInstant.toEpochMilli < now.millis) {
      val thermostatTemperatureName = thermostatNameStart + "_Temp"
      val thermostatName = thermostatNameStart + "_Target_Temp"
      val thermostatHeaterName = thermostatNameStart + "_HEATER"
      val target_temperature = Thermostats.members.filter[ th|th.name == thermostatName ].head as NumberItem
      val current_temperature = ThermostatsTemperatures.members.filter[ tt| tt.name == thermostatTemperatureName ].head as NumberItem
      val heater = Heaters.members.filter[ h|h.name == thermostatHeaterName ].head as SwitchItem
      if ((current_temperature.state as Number) < (target_temperature.state as Number)) {
        if ((heater.state.toString == "OFF") && !heater.updatedSince(now.minusMinutes(1))) {
          heater.sendCommand("ON")
          CENTRAL_GAS_HEATING.sendCommand("ON")
          return;
        }
      } else {
        if ((heater.state.toString == "ON") && !heater.updatedSince(now.minusMinutes(1))) {
          heater.sendCommand("OFF")
          CENTRAL_GAS_HEATING.sendCommand("OFF")
          return;
        }
      }
      val calendar = java.util.Calendar::getInstance
      calendar.timeInMillis = now.plusMinutes(1).millis
      release.state = new DateTimeType(calendar)
      //logInfo("central heater", "creating timer to release.postUpdate(now.minusSeconds(1))")
      createTimer(now.plusMinutes(1), [| release.postUpdate(release.state) ] )
    }
  ]
end

Found the cause of the null errors.
As I was adding logging lines trying to find the line where it failed the results where conflicting.
So it had to be a concurrency issue. Adding a ReentrantLock around the body of the rule fixed the problem.

Just use

release.postUpdate(now.plusMinutes(1).toString)

Cameras had been deprecated and you never really needed to populate a DateTimeType with it anyway. And you can’t just set the state of an item. You must call postUpdate or sendCommand to change the state of an item.

Personally, I would list all the thermostats separately as triggers rather than connect the role like this. Having a lot of role triggers use tedious but it will be much easier to manage than looping. It also might eliminate that concurrency error. And when you move to 2.3 all you have to change is the rule trigger.

You only use === for all lowercase null. NUlL and null are not the same thing. I wish they had kept UNDEF rather than changing it to NULL.

It might be easier to use

(release.state as DateTimeType).isAfter(now.millis)

unfortunately that produces an error “The method isAfter(long) is undefined for the type DateTimeType”

That does look better though.

It is probably just after instead of isAfter.

If you use VSCode you can just type up to the . And it will pop up all the methods on the object.

Of course a tried that but VSC has no suggestions for methods that resemble that.

The otehr sugestions where good though.

Currently looking into how I can give some feedback when a heater is active or not in the applicable thermostat. HABPanel widget: Virtual Thermostat/OnOff Appliance scheduling
A great widget just lacks that feedback of “is it actually doing something right now”.

OK, it looks like before is on the calendar member.

Something must be missing in my VSC, the suggestions it shows you are actually useful.
At the dot I get no suggestions until i type at least one letter.
This works but it introduces another warning about getCalendar being deprecated.