The block that reads “store value samples into index to private cache” needs to read “store value samples into samples to private cache”.
But I want to make sure you understand why this was the problem. You need to understand this code to be able to maintain and expand it in the long run and you need to know how to debug problems like these because you’re not going to always have @justaoldman and me too do this for you.
We’ve provided examples and are happy to help get those working, but if you don’t understand enough to do a little debugging on your own, eventually this code will break and you won’t be able to fix it, you won’t be able to expand it in the future, and you won’t be able to write your own rules from scratch.
var NUM_SAMPLES, WIND_THREHOLD, WIND_THREHOLD_HIGH, WIND_HIGH_ROLLERSHUTTER, WIND_ROLLERSHUTTER, curSpeed, shutterItem, shutterState, samples, index, sum, i, avg;
NUM_SAMPLES = 6;
WIND_THREHOLD = 14;
WIND_THREHOLD_HIGH = 25;
WIND_HIGH_ROLLERSHUTTER = 100;
WIND_ROLLERSHUTTER = 70;
curSpeed = items.getItem('Vento_vetrata').numericState;
shutterItem = items.getItem('Tende');
shutterState = shutterItem.numericState;
if (cache.private.exists('index') === false) {
cache.private.put('index', (-1));
}
if (cache.private.exists('samples') === false) {
samples = [];
cache.private.put('index', samples);
}
index = ((cache.private.get('index')) + 1) % NUM_SAMPLES;
samples = (cache.private.get('samples'));
samples[(index - 1)] = curSpeed;
sum = 0;
for (var i_index in samples) {
i = samples[i_index];
sum = sum + 1;
}
avg = sum / samples.length;
cache.private.put('index', index);
cache.private.put('samples', samples);
console.info(('Windspeed avarage is' + String(avg)));
if (avg > WIND_THREHOLD_HIGH && shutterState < WIND_HIGH_ROLLERSHUTTER) {
shutterItem.sendCommand(WIND_HIGH_ROLLERSHUTTER);
console.warn('Avarage windspeed is very high adjusting shutters');
} else if (avg > WIND_THREHOLD && shutterState < WIND_ROLLERSHUTTER) {
shutterItem.sendCommand(WIND_ROLLERSHUTTER);
console.warn('Avarage windspeed is high adjusting shutters');
} else {
console.info('Avarage windspeed is not high');
}
2024-05-05 17:36:20.228 [ERROR] [b.automation.script.javascript.stack] - Failed to execute script:
org.graalvm.polyglot.PolyglotException: TypeError: Cannot set property "0" of null
at <js>.:program(<eval>:21) ~[?:?]
at org.graalvm.polyglot.Context.eval(Context.java:399) ~[?:?]
at com.oracle.truffle.js.scriptengine.GraalJSScriptEngine.eval(GraalJSScriptEngine.java:458) ~[?:?]
at com.oracle.truffle.js.scriptengine.GraalJSScriptEngine.eval(GraalJSScriptEngine.java:426) ~[?:?]
at javax.script.AbstractScriptEngine.eval(AbstractScriptEngine.java:262) ~[java.scripting:?]
at org.openhab.automation.jsscripting.internal.scriptengine.DelegatingScriptEngineWithInvocableAndAutocloseable.eval(DelegatingScriptEngineWithInvocableAndAutocloseable.java:53) ~[?:?]
at org.openhab.automation.jsscripting.internal.scriptengine.InvocationInterceptingScriptEngineWithInvocableAndAutoCloseable.eval(InvocationInterceptingScriptEngineWithInvocableAndAutoCloseable.java:78) ~[?:?]
at org.openhab.automation.jsscripting.internal.scriptengine.DelegatingScriptEngineWithInvocableAndAutocloseable.eval(DelegatingScriptEngineWithInvocableAndAutocloseable.java:53) ~[?:?]
at org.openhab.automation.jsscripting.internal.scriptengine.InvocationInterceptingScriptEngineWithInvocableAndAutoCloseable.eval(InvocationInterceptingScriptEngineWithInvocableAndAutoCloseable.java:78) ~[?:?]
at org.openhab.core.automation.module.script.internal.handler.ScriptActionHandler.lambda$0(ScriptActionHandler.java:71) ~[?:?]
at java.util.Optional.ifPresent(Optional.java:178) [?:?]
at org.openhab.core.automation.module.script.internal.handler.ScriptActionHandler.execute(ScriptActionHandler.java:68) [bundleFile:?]
at org.openhab.core.automation.internal.RuleEngineImpl.executeActions(RuleEngineImpl.java:1188) [bundleFile:?]
at org.openhab.core.automation.internal.RuleEngineImpl.runRule(RuleEngineImpl.java:997) [bundleFile:?]
at org.openhab.core.automation.internal.TriggerHandlerCallbackImpl$TriggerData.run(TriggerHandlerCallbackImpl.java:87) [bundleFile:?]
at java.util.concurrent.Executors$RunnableAdapter.call(Executors.java:539) [?:?]
at java.util.concurrent.FutureTask.run(FutureTask.java:264) [?:?]
at java.util.concurrent.ScheduledThreadPoolExecutor$ScheduledFutureTask.run(ScheduledThreadPoolExecutor.java:304) [?:?]
at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1136) [?:?]
at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:635) [?:?]
at java.lang.Thread.run(Thread.java:833) [?:?]
2024-05-05 17:36:20.238 [ERROR] [internal.handler.ScriptActionHandler] - Script execution of rule with UID '97ba6d2f35' failed: org.graalvm.polyglot.PolyglotException: TypeError: Cannot set property "0" of null
Hi Rich I have corrected I doble check everything error still in the logs..
You haven’t corrected the problem. The block still says “store value samples into index to private cache” in the screenshot of the blocks. The key we store the samples under in the cache needs to be “samples”, not “index”.
But it calculate always the same average even if move manually the sensor more than 1.2 average so the roller doesn’t go up
2024-05-05 18:33:02.234 [INFO ] [openhab.event.ItemStateChangedEvent ] - Item 'Vento_vetrata' changed from 36.29 km/h to 27.29 km/h
==> /var/log/openhab/openhab.log <==
2024-05-05 18:33:02.237 [INFO ] [nhab.automation.script.ui.97ba6d2f35] - Windspeed avarage is1.2
2024-05-05 18:33:02.238 [INFO ] [nhab.automation.script.ui.97ba6d2f35] - Avarage windspeed is not high
2024-05-05 18:58:42.473 [INFO ] [nhab.automation.script.ui.97ba6d2f35] - Windspeed avarage is1.2
2024-05-05 18:58:42.475 [INFO ] [nhab.automation.script.ui.97ba6d2f35] - Avarage windspeed is not high
==> /var/log/openhab/events.log <==
2024-05-05 18:58:43.471 [INFO ] [openhab.event.ItemStateChangedEvent ] - Item 'Vento_vetrata' changed from 6.61 km/h to 3.89 km/h
==> /var/log/openhab/openhab.log <==
2024-05-05 18:58:43.475 [INFO ] [nhab.automation.script.ui.97ba6d2f35] - Windspeed avarage is1.2
2024-05-05 18:58:43.476 [INFO ] [nhab.automation.script.ui.97ba6d2f35] - Avarage windspeed is not high
Avarage is caluculated any second now
This suppose to be the high wind case and the roller supposed to roll up
It seems 6 samples is very sensitive and avg is calculated immediately
Do I have to increase the samples?
The first time the rule runs after it’s loaded, there is only one sample so that becomes the average.
The second time there are two samples so the average is the average of those two samples.
This continues until there are NUM_SAMPLES samples. From that point the oldest sample gets replaced with the new sample, keeping a moving average of the most recent NUM_SAMPLES samples. A new average is calculated every second.
If you want to change this behavior this is a great opportunity to see if you understand the code. You can make it wait until it has NUM_SAMPLES samples before it tries to calculate the average. Add an if after curr speed is added to the samples list to only run the rest of the rule when the length of the samples list is NUM_SAMPLES. There are example of all the blocks you need already in the rule.
I haven’t understood the code completely it is over my knowledge
Obviously it is not the behaviour i needed as the roller never goes up now as the avg is always very low
To debug that, do the math by hand and compare to what the rule is calculating. You’ll need to add logging statements to the rule to see what numbers it’s working with (i.e. what are the windspeed values it’s working with) and what the sum and avg variables (i.e. the results of the calculations) are. Maybe the calculation isn’t right? Maybe the numbers it’s getting are different from what you expect them to be? Maybe there’s a bug in the logic?
Unless there are pretty sustained winds, I would expect the average over ten seconds to be pretty low so maybe the rule is working correctly and your overall approach isn’t right or maybe you need an average over less time than ten seconds. For example, given these samples: 5,5,20,30,10,5,5,1,1,5 the average is only 8.2. Even though you had three readings over that ten second period above your threshold the average wouldn’t have caused the roller shutter to move. However, with only three samples, those three high values would have resulted in much higher averages.
They call it “computer science” because often, programmers have to solve problems by running experiments. That’s how we learn what’s wrong and how to fix it.
The best way to answer that question is to try it and see. You don’t need us to tell if the rule does or does not do what you expect it to.
From what I can see it should work. At least you added the if statement exactly like I described. Whether that works…I’m not a computer. I can say that it should work but actually running the rule and observing will tell you whether it does work.
Blocky is great and helps you avoid syntax errors and also prevent you from combining wrong methods, and structures but you still have to understand all the objects you are combining together in order to be successful at creating scripts that work exactly how you want them to.
That’s the rule trigger. You must set the trigger to a time based trigger using the cron expression from above as described above. That runs the rule every second.
That’s not part of the Blockly code at all. That’s the rule trigger that does that.
Hi,
Regardless of you using the sample approach I gave or the approach @rlkoshak offered this statement remains the same.
What I am saying is identify each piece of the script and what it is doing I keep linking you back to a site that offers clear and easy explanations and samples as well as try it yourself. It is one of many sites that explain how all these script pieces work I am sure there are others that may be in a different language or teaching approach that may suit you better but I really recommend you spend some time on one of them to avoid getting frustrated and giving up.
“Nothing is too difficult” if you are willing to put real effort into learning it.
Till you understand how all the pieces fit together and work by their selves you will struggle trying to change anyone’s examples to fit your needs.
Total bone headed mistake on my part. It’s supposed to be the variable i, not the number 1. The color difference should have been clue enough to me but apparently it wasn’t and my brain was just inserting the correct answer instead of what’s actually there.
Thanks again
If I can ask do you in the code insert to store the number of sample index and samples to store the value from the sensor isn’t?
That because we use two lists?
Finally changed math and set as trigger the cron condition of every second it seems works as expected!! My logs are flooded
Does it will be always running?
Is it an issue running a rule every second?
It’s not two lists, “index” is just a number and it points to the most recently saved sample in the list.
index is the index into the list. The 5th element of the list is 4. I save the index on each run of the rule so that I use the next slot the next time.
Let’s say NUM_SAMPLES is set to 3 to make things simple,
The first time the rule runs there won’t be any value stored int eh cache for “index” so we initialize it to -1. There won’t be the “samples” list either so we create an empty list and store that in the private cache. That’s what these blocks do:
And now that I look at that this might be a problem. Those two blocks need to run every time the rule runs. They should be put above the if statement and after the “in list samples set # as currSpeed”. Hmmm, but if you are getting some calculations done maybe that doesn’t need to be done.
I would move the blocks just to be safe though.
The next time the rule runs there already is a value in the cache for “index” and “samples” so those initialization blocks are skipped. Now the tricky line runs again, only this time “index” is 0 since that’s what was stored in the cache.
pull the value of “index” from the private cache. So we have 0 right now.
Add one to the value, so now we have 1.
Finally we take the remainder of 1/3 which is 1.
And now we store the current windspeed in the “index” slot of the list. The list now has two samples stored, one at 0 and one at 1.
The rule runs again and just like before the initialization blocks are skipped and the tricky line of code runs.
pull the value of “index” from the private cache. So we have 1 right now.
Add one to the value, so now we have 2.
Finally we take the remainder of 2/3 which is 2.
Now we store the current windspeed in the “index” slot of the list. The list now has three samples stored, one at 0, one at 1, and one at 2.
The rule runs to completion and triggers again. Again we skip the initialization blocks because we already have values stored for “index” and “samples”. The tricky block runs again.
pull the value of “index” from the private cache. So we have 2 right now.
Add one to the value, so now we have 3.
Finally we take the remainder of 3/3 which is 0. We have circled back to 0.
Now we store the current windspeed in the “index” slot of the list. This causes the oldest sample to get replaced with the current sample. That means that the sum that happens later will always be the sum of only the NUM_SAMPLES most recent windspeed samples (in this example 3).
Hopefully you can see why we therefore need to keep both the “index” and the “samples” in the cache. One is to keep track of where the oldest sample is in the list so we can replace it and the other is to keep the list of samples itself.
What do you mean by “flooded”? Which logs? Given the most recently blocks posted above, you should get no more than one log statement per second in openhab.log.
Unless you delete or disable the rule (the circle with ll in the middle next to the triangle play button) it will run every second as long as OH is running.
This rule probably takes no more than 100 msec to run, most likely closer to 20 msec. And what it does in that small fraction of a second is neither RAM nor CPU intensive. This rule will have negligible impact on your system. You could have dozens of these rules running and probably couldn’t measure the impact they all combined would have on your system.
Running this rule is not going to be an issue.
But let’s say you want to be super efficient for the practice. You could add another rule that triggers when the rollershutter changes. You can add a Blockly script action here that tests to see if the rollershutter is > 10 or not. If it’s less than 10 we don’t care what the windspeed is so disable the cron triggered rule (there’s a block for that). If it’s greater than or equal to 10 enable the cron triggered rule. Then the rule will only be calculating the windspeed average when it matters.
There’s really no compelling reason to do that but it could be a good learning exercise.
(Note: you wouldn’t want to add a condition to check the rollershutter to the cron triggered rule because then the values in the cache will get stale. By disabling and reenabling the rule, that causes the cache to get cleared so when it’s reenabled it starts with “index” == -1 again and an empty samples list.)
