Arguments to createTimer

Is there a way to pass a variable to a timer? I have code that loops and I would like to send the current index as a argument to a timer. I started with the code below, which of course doesn’t work because the current state of index changes before the timer runs.

var int index = 0 
//For each pump speed serving 0-7
gProgramSettings.members.forEach[GroupItem gPS|
     //Do stuff
     //If pumpspeed(index) is enabled, schedule start time
     //Set pumpSpeedSetting to current speed index 0-7
     timers.add((createTimer(startTime) [| 
     //Move to next pump speed
     index = index + 1

Try defining a local variable (indexLoc) inside the forEach loop, copy over the index value and use it at createTimer.

You probably don’t want to use the index because the order of the Items in the members List is not fixed. If you actually care which pump get’s which setting you need to do something different like encoding the “index” as part of the Item name and parsing it out, or using a .map file mapping the Item name to the setting and call the transform Action.

But you can get the index in the forEach loop itself.

forEach[ GroupItem gPS, Number index |

The index may need to be an int. I forget which.

Are you sure the code as written works? You should be getting a “cannot reference a non-final variable in this context” or something like that because only final external variables (val) can be used inside a forEach lambda.

With the method above, won’t the value of index from the forEach loop change as the loop iterates? That’s the problem that I have now, index always = 8 by the time the timer executes as it’s value changes with each loop iteration. I need a way to get an actual number value to pass. .

You’re right though, the code doesn’t work. It iterates in order of the items in the file and send a correct value to the timer but it doesn’t execute the sendCommand properly. I hadn’t gotten to fixing the sendCommand portion yet, I’ve been trying to just pass the right value to the timer. There is alot more to the code though, I stripped it to isolate the question a bit.

For reference, more detailed code is below. Any suggestions would be appreciated!

/*Items */

//Number 0 - 7 representing speed 1 - 8
Number	filterPumpSpeedSetting	"Pump Speed Preset" <pump> 

/* Speed 0 Master Settings */	
Switch	pumpSpeed_0_Master	"Speed 0 [%s]"	<switch>	(gPumpSpeed_0_Settings)
String	pumpSpeed_0_StartTime	"Start Time [%s]"	<calendar>	(gPumpSpeed_0_Settings)
Number	pumpSpeed_0_StartTime_Hours	"Hours [%d]"	<settings>	(gPumpSpeed_0_Settings)
Number	pumpSpeed_0_StartTime_Minutes	"Minutes [%d]"	<settings>	(gPumpSpeed_0_Settings)
Number	pumpSpeed_0_Duration	"Speed 0 Duration [%d mins]"	<clock>	(gPumpSpeed_0_Settings, gPumpSpeed_0_Duration, gInitializeZero)	
String	pumpSpeed_0_EndTime	"Start Time [%s]"	<calendar>	(gPumpSpeed_0_Settings)	

Switch	Monday_pumpSpeed_0 "Monday [%s]" <switch>	(gPumpSpeed_0_Settings, gPumpSpeed_0_DayOfWeek)
Switch	Tuesday_pumpSpeed_0 "Tuesday [%s]" <switch>	(gPumpSpeed_0_Settings, gPumpSpeed_0_DayOfWeek)
// similar items follow for speeds 0 - 7, with index changed as required.

//*******Rules File********

//Day of week string array.  Used to evaluate day of week
val ArrayList<String> dayOfWeekArray = newArrayList(
//Timer array to hold scheduled timers
var List<Timer> timers = newArrayList	

rule "Schedule Run Times"
	//Item testSwitch changed or
	//Time cron "0 0 0 * * ?" or //Run at 12:00 daily
	Item pumpSpeed_0_Master received command //or
	// clear out the old timers if there are any
	logInfo("Timers","--------------------Clearing Timers.  Qty:  " + timers.size)
	    timers.forEach[ t | 
	    	logInfo("TimerCancel", t.toString + "  Cancelled")
	// If pump speed preset is enabled, get durations and schedule timers	
		logInfo("Pool Schedule","------------------------gProgramSettings------------------------")	
		//Index used to reference pump speed setting
		var int index = 0
		//For each speed group (gPumpSpeed_X_Settings)
		gProgramSettings.members.forEach[GroupItem gPS|
			logInfo("Pool Schedule",
			var logMessage = ""
			if (gPS.members.filter[t|"Master")].head.state == ON){
				logInfo("Pool Test", gPS.label + " - Master Switch is " + gPS.members.filter[t|"Master")].head.state)
				//If pump speed is scheduled for today
				if (gPS.members.filter[t|].head.state == ON){
					logInfo("Pool Test","Day of Week Switch is " + gPS.members.filter[t|].head.state)
					//Obtain Program X startTime.  This happens at midnight, and whenever a change is made to the speed schedule.
					var int progStartHours = (gPS.members.filter[t|"StartTime_Hours")].head.state as DecimalType).intValue()
					var int progStartMinutes = (gPS.members.filter[t|"StartTime_Minutes")].head.state as DecimalType).intValue()
					var DateTime startTime = parse(now.getYear() + "-" + now.getMonthOfYear() + "-" + now.getDayOfMonth() + "T" + progStartHours + ":" + progStartMinutes)
					//Check if start time has passed for the day
						logMessage = gPS.label + " Start Time= " + startTime.toString("EEE MMM dd, yyyy hh:mm:ss a")
						logInfo("Pool Schedule",logMessage)	
						//Why do we do this?  Only one duration, could pull directly with progStartHours
						for (t : gPS.members.filter[t|"Duration") && t.type == "Number" && t.state>0]){
						 	var int speedDuration = (t.state as DecimalType).intValue
						 	//val zoneName = (,"_"))) //Don't think I need this
						 	var DateTime endTime = startTime.plusMinutes(speedDuration)
						 	//startTimeTest used for logging only?  Seems unnecessary
							val startTimeTest = startTime
						 	val endTimeTest = endTime
						 	logInfo("Pool Schedule", " Start  " + startTime.toString("EEE MMM dd, yyyy hh:mm:ss a") + " End  " + endTime.toString("EEE MMM dd, yyyy hh:mm:ss a"))
							timers.add((createTimer(startTime) [| 
								sendCommand(filterPumpSpeedSetting.state, index) //pump speed = index
								logInfo("Pool Schedule", "Pump Speed Index set to (" + index + ")" + startTimeTest.toString("EEE MMM dd, yyyy hh:mm:ss a"))
							logInfo("Pool Schedule", "Pump Speed Index set to (" + index + ")" + startTimeTest.toString("EEE MMM dd, yyyy hh:mm:ss a"))
							//Check to see if pump is still at the indexed speed setting.  If it is not, another speed has taken control.
							timers.add((createTimer(endTime) [| 
								if (filterPumpSpeedSetting.state == index){
									//Set speed to default low speed.  
									//Designed to have pump run continuously at low speed unless scheduled otherwise.
									//Currently speed 0, this can be altered per pump performance.
									sendCommand(filterPumpSpeedSetting.state, 0)
									logInfo("Pool Schedule", "Pump Speed set to default low speed" + endTimeTest.toString("EEE MMM dd, yyyy hh:mm:ss a"))
							logInfo("Pool Schedule", "Pump Speed Index set to (" + index + ")" + startTimeTest.toString("EEE MMM dd, yyyy hh:mm:ss a"))
							startTime = endTime.plusMinutes(1)
					else logMessage = gPS.label + " Start Time has already passed.  Start Time = " + startTime.toString("EEE MMM dd, yyyy hh:mm:ss a") 
				else logMessage = gPS.label + " - Pump Speed is not scheduled for today"
			else logMessage = gPS.label + " - Master Switch is off, Pump Speed is disabled"
			if (logMessage != ""){
				logInfo("Pool Schedule", logMessage)
			index = index + 1

	timers.forEach[ t | 
	    	logInfo("Scheduled Timers", t.toString)

Yes, just as index changes in your original code for each iteration.That’s what index = index + 1 does.

But the difference here is that the forEach will be calling the lambda with the index on each iteration through the loop. It will therefore exist as a separate variable and will not be impacted by subsequent iterations of the loop.

I don’t have time to go too far into detail. But at a high level, every time you see a [ | ] you are seeing a lambda. A lambda is a function that you can pass around like any other Object.

The forEach method takes one of these lambdas as it’s argument. It will take a lambda that requires one argument (forEach[GroupItem gPS|) or it takes one that requires two arguments (forEach[GroupItem gPS, int index |). Behind the scenes forEach calls this lambda with the one or two arguments for each Item in the list in turn. The stuff before the | are the function arguments.

In your original code, you created a variable outside the forEach lambda. Then you created the forEach lambda. When you create a lambda it inherits all the variables in it’s current context. This means that it inherits index. This is why you can increment index from inside the lambda. (Since you claim this works this must be a recent change as this used to not work and only vals could be references from inside the lambda.)

But, only one lambda is created for use by forEach, not a separate one for each member. Therefore there is only one index variable and all the Timers created inside the forEach lambda end up pointing to the exact same index variable. So obviously, by the time the timer goes off, index is set to 8.

You need each timer to have it’s own copy of index instead of sharing the index variable defined outside the lambda.

Lukics provided one way to achieve this. It should work.

But the language has a way to do this too which is what I suggest. When you add the int index as an argument to the forEach lambda you are being passed a copy of the index that the forEach function is managing. So that value won’t change for subsequent calls of the forEach call to the lambda.

I don’t have time to really review the code, but it strikes me as being significantly more complicated than it probably needs to be. You might review the following to see if there is anything you can apply:

At the quick scan level, use findFirst instead of filter .head.

Avoid using primitives. Use Number where ever possible. Where not possible, avoid converting a Number to an int (for example) by calling .intValue until the last possible moment.

I’d split this up some. For example, I’d move the time/day of week calculations into their own set of Rules (see the time of day design pattern above).

I’d not rely on the index of a Pump Item in the members list for anything. The order of that list is not guaranteed to be the same every time.

That worked! I’ll try rlkoshak’s solution next

Thanks, rlkoshak, for the very thorough explanation. The openhab rule language is so alien to me, it takes forever to do things that seem to be simple.

I tried your solution out as well and it works like a charm.