Oh, I’m glad you asked for help. This code is really bad in an OH context. There are a few things that the Rules DSL is not so friendly for helping new users and this is one of those cases.
The naive approach for most new coders is to work through the steps in their mind and then code each step in the Rule. Most of the time that’s not a problem. It may be a little ugly but it doesn’t cause any real harm to OH. But in this case you have a Rule that consumes a run time thread potentially indefinitely which is a really bad idea and at a minimum over 40 seconds!
The other thing I notice is you are using the sendCommand Action instead of the function. See this section of the docs for details.
Finally I see that the only difference between all those lines of code is the first value of the HSB being sent to the light. Thus this is a clear violation of DRY (Don’t Repeat Yourself) which is probably why you asked the question in the first place.
I notice some other undesirable side effects of this approach as well. For example, if the lights are turned OFF, the LED strip could potentially take 40+ seconds before the light actually turns OFF.
So let’s address these problems. First, let’s get rid of the duplicate code. We will tighten the loop to run every 1.5 seconds and recalculate the state on each time through the loop.
then
if(KitchenYeelightLEDStripTest_Power.state == ON){
val increment = 10
var currColor = 0
while(KitchenYeelightLEDStripTest_Power.state == ON && Kitchen_PartyMode.state == ON){
KitchenYeelightLEDStripTest.sendCommand(currColor+",100,100")
currColor = (currColor + increment) % 360 // remainder operation, when it gets over 360 currColor goes back to 0
Thread::sleep(1500)
}
KitchenYeelightLEDStripTest.sendCommand("0,100,100")
}
end
This is already WAY better. 10 lines of code compared to around 74 lines of code in the original. We also use the method on the Item instead of Action.
The real trick is the use of the modulo operator %. This is sometimes called the remainder operation. Essentially we divide (currColor + increment) by 360 and set currColor to the remainder of that operation. The remainder of 150 / 360 = 150. The remainder of 360/360 = 0. The remainder of 370/360 = 10. So the value will never get above 360 and it will continue to loop through the values until the loop is canceled.
Also, note that we are looping and therefore checking the state of the two Switches every 1.5 seconds, the light will remain on no more than 1.5 seconds after you turn one of them OFF. That’s better than over 40 seconds, but not yet perfect.
Now let’s deal with that sleep. We can avoid consuming a runtime thread for long period of time doing nothing by using Design Pattern: Looping Timers.
var Timer partyTimer = null
rule "Kitchen LED Party Mode"
when
Item Kitchen_Party_Mode changed from OFF to ON
then
partyTimer?.cancel // cancel the timer if one exists for some reason
val increment = 10
var currColor = 0
partyTimer = createTimer(now, [ |
if(KitchenYeelinghtLEDStripTest_Power.state == ON && Kitchen_PartyMode.state == ON) {
KitchenYeelightLEDStripTest.sendCommand(currColor+",100,100")
currColor = (currColor + increment) % 360
partyTimer.reschedule(now.plusMillis(1500))
}
else {
KitchenYeelightLEDStripTest.sendCommand("0,100,100")
partyTimer = null
}
])
In the above we create a looping Timer to execute immediately. If the two switches are ON, we set the Light to the calculated color. Then we reschedule the Timer to go off again in a 1.5 seconds. If not, we set the color to 0 and exit the Timer. Despite the warnings in the logs, this will work. The Timer will get it’s own copy of increment and currColor so the incrementing will work.
The big difference between this and using Thread::sleep is that the Timer doesn’t consume a thread doing nothing during that 1.5 second period waiting to run the next step. Instead it is sitting in the background not consuming a runtime thread, freeing that thread up for other Rules to use.
But, like the previous one, this version also will keep the light ON for up to 1.5 seconds after one of the Switches is turned OFF. To deal with that problem we need a separate Rule.
And because the canceling of the Timer is handled elsewhere we can move all the end of Timer loop stuff to that other Rule.
var Timer partyTimer = null
rule "Kitchen LED Party Mode"
when
Item Kitchen_Party_Mode changed from OFF to ON
then
partyTimer?.cancel // cancel the timer if one exists for some reason
val increment = 10
var currColor = 0
partyTimer = createTimer(now, [ |
KitchenYeelightLEDStripTest.sendCommand(currColor+",100,100")
currColor = (currColor + increment) % 360
partyTimer.reschedule(now.plusMillis(1500))
])
end
rule "Party mode turned off"
when
Item Kitchen_Party_Mode changed from ON to OFF or
Item KitchenYeellightLEDStripTest_Power.state changed from ON to OFF
then
partyTimer?.cancel
KitchenYeelightLEDStripTest.sendCommand("0,100,100")
partyTimer = null
end
NOTE: The partyTimer?.cancel is the same as if(partyTimer !== null) partyTimer.cancel
.