[Reactor] Variables not updating correctly in latest-25201-2aa18550
-
Build 25285 now up for docker 64-bit only once again, and intended only for users on this thread. This will be the last development/test build before a general release.
Author of Multi-system Reactor and Reactor, DelayLight, Switchboard, and about a dozen other plugins that run on Vera and openLuup.
-
Build 25285 now up for docker 64-bit only once again, and intended only for users on this thread. This will be the last development/test build before a general release.
@toggledbits updated to this build, and can report one bug (although it might have been present earlier, but I only noticed this behaviour with this build).
I have a rule with following local variables:
If I click "play" (run) in edit mode on any of these local expressions, I get a runtime error "request failed" (with no other information). Also, if MSR is restarted and expressions get re-evaluated, some of these will be incorrect. E.g. "night_prices" should be first 24 entries of "price_array", but instead it contains first 24 entries of of "sorted_array". Likewise "evening_prices" contains last entries of "sorted_array" when it should have last entries of "price_array".
"highs_array" seems to be okay for some reason.
If you want to test this, I've attached "price_array" below:
[0.125,0.124,0.122,0.062,0.001,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0.001,0.099,0.123,0.125,0.15,0.309,0.29,0.269,0.271,0.428,0.5,1.166,2.073,3.399,4.84,5,6.481,7.292,7.789,8.893,8.915,9.567,9.948,9.027,7.044,8.687,8.001,7.315,6.658,7.902,6.843,6.36,6.098,6.595,6.061,5.838,5.639,6.194,6.036,5.814,5.842,5.378,5.804,6.632,7.254,5.613,6.531,7.357,8.272,6.046,7.089,8.029,10.411,7.843,9.741,13.613,15.002,13.305,16.292,16.651,17.314,15,15.686,15.592,14.559,13.656,13.081,10.844,9.922,11.692,9.631,9.201,8.912,10.17,9.889,9.28,8.79] -
Wow. That error has been around a while, and I'm surprised nobody noticed and reported it. It's actually a bug in lexpjs, the parser/executor for expressions... it's sorting the array in place rather than sorting a copy that it then returns. That's not consistent with its spec, and I can easily fix that and get it into the next build. That bug causes other expression values to be modified when they should not be (the effect is pretty obvious in the image, for example, the first price is not 0, so the first night_price should not be 0 either).
If you need to work around it, clone your array before sorting it:
sorted_array = sort( clone( price_array ), $2 - $1 )I also added a little shortcut there for the sorting function. You don't specifically need to return +/-1 or 0, the sort will look at anything positive, negative, or zero, so simple subtraction can take the place of the more complex conditional expression.
The
clone()should not be necessary in future builds.Here's what it looks like for me now with the lexpjs fix in place (no
clone()needed):
Thank you for including a copy of the source array in text form. That was super-helpful and a big time-saver.
Author of Multi-system Reactor and Reactor, DelayLight, Switchboard, and about a dozen other plugins that run on Vera and openLuup.
-
Wow. That error has been around a while, and I'm surprised nobody noticed and reported it. It's actually a bug in lexpjs, the parser/executor for expressions... it's sorting the array in place rather than sorting a copy that it then returns. That's not consistent with its spec, and I can easily fix that and get it into the next build. That bug causes other expression values to be modified when they should not be (the effect is pretty obvious in the image, for example, the first price is not 0, so the first night_price should not be 0 either).
If you need to work around it, clone your array before sorting it:
sorted_array = sort( clone( price_array ), $2 - $1 )I also added a little shortcut there for the sorting function. You don't specifically need to return +/-1 or 0, the sort will look at anything positive, negative, or zero, so simple subtraction can take the place of the more complex conditional expression.
The
clone()should not be necessary in future builds.Here's what it looks like for me now with the lexpjs fix in place (no
clone()needed):Thank you for including a copy of the source array in text form. That was super-helpful and a big time-saver.
@toggledbits thanks for fixing! Another more cosmetic issue is that there seems to be a quite long delay until "Set Rules" widget gets populated? Using Chrome v141
Using MSR on Docker (Synology NAS), having InfluxDB, Grafana & Home Assistant, Hubitat C-8, Zigbee2MQTT & ZWA-2
-
@toggledbits thanks for fixing! Another more cosmetic issue is that there seems to be a quite long delay until "Set Rules" widget gets populated? Using Chrome v141
@tunnus said in [Reactor] Variables not updating correctly in latest-25201-2aa18550:
Another more cosmetic issue is that there seems to be a quite long delay until "Set Rules" widget gets populated? Using Chrome v141
Haven't observed anything like that. More detail needed. Proximate to a restart? Is the delay 3 seconds or 30? ...?
Author of Multi-system Reactor and Reactor, DelayLight, Switchboard, and about a dozen other plugins that run on Vera and openLuup.
-
@tunnus said in [Reactor] Variables not updating correctly in latest-25201-2aa18550:
Another more cosmetic issue is that there seems to be a quite long delay until "Set Rules" widget gets populated? Using Chrome v141
Haven't observed anything like that. More detail needed. Proximate to a restart? Is the delay 3 seconds or 30? ...?
@toggledbits I was using a mobile hotspot when observing this behaviour, and now that I'm back on a high speed connection, delay is 1-2 seconds after browser hard refresh, so nothing to worry about.
-
Oh that's not horrible, considering the amount of information it has to retrieve. But, I may be able to improve on it slightly. Stay tuned... the changelog is getting long for this release...
Author of Multi-system Reactor and Reactor, DelayLight, Switchboard, and about a dozen other plugins that run on Vera and openLuup.
-
@toggledbits updated to this build, and can report one bug (although it might have been present earlier, but I only noticed this behaviour with this build).
I have a rule with following local variables:
If I click "play" (run) in edit mode on any of these local expressions, I get a runtime error "request failed" (with no other information). Also, if MSR is restarted and expressions get re-evaluated, some of these will be incorrect. E.g. "night_prices" should be first 24 entries of "price_array", but instead it contains first 24 entries of of "sorted_array". Likewise "evening_prices" contains last entries of "sorted_array" when it should have last entries of "price_array".
"highs_array" seems to be okay for some reason.
If you want to test this, I've attached "price_array" below:
[0.125,0.124,0.122,0.062,0.001,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0.001,0.099,0.123,0.125,0.15,0.309,0.29,0.269,0.271,0.428,0.5,1.166,2.073,3.399,4.84,5,6.481,7.292,7.789,8.893,8.915,9.567,9.948,9.027,7.044,8.687,8.001,7.315,6.658,7.902,6.843,6.36,6.098,6.595,6.061,5.838,5.639,6.194,6.036,5.814,5.842,5.378,5.804,6.632,7.254,5.613,6.531,7.357,8.272,6.046,7.089,8.029,10.411,7.843,9.741,13.613,15.002,13.305,16.292,16.651,17.314,15,15.686,15.592,14.559,13.656,13.081,10.844,9.922,11.692,9.631,9.201,8.912,10.17,9.889,9.28,8.79]@toggledbits using your workaround (clone), but the problem remains with variables not updating correctly as price_array changes, e.g. sum variables do not update. If I (temporarily) modify one of the variables (e.g. change rounding & save), then all the rest changes correctly.
With this particular rule I cannot use Script Action workaround to force updates as I'm using those sum variables as triggers.
-
OK. Follow the instructions from this prior post to set up a log file just for this rule. Restart Reactor and just change the
price_array. Then capture that log file. I will send you a link when I get home tonight where you can upload it.Author of Multi-system Reactor and Reactor, DelayLight, Switchboard, and about a dozen other plugins that run on Vera and openLuup.
-
Got the log files; looking at them. Did you newly create this rule for the test?
Author of Multi-system Reactor and Reactor, DelayLight, Switchboard, and about a dozen other plugins that run on Vera and openLuup.
-
Got the log files; looking at them. Did you newly create this rule for the test?
@toggledbits no, it has been around for a while. Has been renamed though
-
OK. I just pushed 25291 for 64-bit Intel/AMD only (should be correct for your Synology NAS). Give that a try.
Author of Multi-system Reactor and Reactor, DelayLight, Switchboard, and about a dozen other plugins that run on Vera and openLuup.
-
OK. I just pushed 25291 for 64-bit Intel/AMD only (should be correct for your Synology NAS). Give that a try.
-
@toggledbits will report shortly (price_array updates at midnight), but wow, Set Rules widget is now lightning fast!
@tunnus said in [Reactor] Variables not updating correctly in latest-25201-2aa18550:
wow, Set Rules widget is now lightning fast!
Haha! Yeah, I gave it a specialized API endpoint. It's much faster to gather the necessary data on the server side and just push it through.
-
@tunnus said in [Reactor] Variables not updating correctly in latest-25201-2aa18550:
wow, Set Rules widget is now lightning fast!
Haha! Yeah, I gave it a specialized API endpoint. It's much faster to gather the necessary data on the server side and just push it through.
@toggledbits I have a couple of "known" issues left with my rules. One is where I have a VERY long rule, a lot of conditions, but even more stuff in reactions, especially groups. It works, but when editing this rule, and trying to scroll down to reactions, I'm constantly taken back to the beginning. After a while (maybe as the whole content is loaded or something?) I'm able to scroll up or down without UI "resisting" (for the lack of better word or description). Not a bug, but more of a usability issue.
I guess I could or should divide the contents to one or two other rules, so that there wouldn't be one long rule as it seems to cause problems. But if you want to look at this, I could send you a JSON file of the rule so that you get a better idea?
Using MSR on Docker (Synology NAS), having InfluxDB, Grafana & Home Assistant, Hubitat C-8, Zigbee2MQTT & ZWA-2
-
@toggledbits I have a couple of "known" issues left with my rules. One is where I have a VERY long rule, a lot of conditions, but even more stuff in reactions, especially groups. It works, but when editing this rule, and trying to scroll down to reactions, I'm constantly taken back to the beginning. After a while (maybe as the whole content is loaded or something?) I'm able to scroll up or down without UI "resisting" (for the lack of better word or description). Not a bug, but more of a usability issue.
I guess I could or should divide the contents to one or two other rules, so that there wouldn't be one long rule as it seems to cause problems. But if you want to look at this, I could send you a JSON file of the rule so that you get a better idea?
@tunnus said in [Reactor] Variables not updating correctly in latest-25201-2aa18550:
But if you want to look at this, I could send you a JSON file of the rule so that you get a better idea?
Yes, I'd be interested in looking at that. I think you can re-use the link I sent you previously.
Also, I pushed a 25292 build this morning. One of the two issues addressed therein could be related to what you are seeing, so if you haven't gone to that build yet, you might try it first.
Author of Multi-system Reactor and Reactor, DelayLight, Switchboard, and about a dozen other plugins that run on Vera and openLuup.
-
@tunnus said in [Reactor] Variables not updating correctly in latest-25201-2aa18550:
But if you want to look at this, I could send you a JSON file of the rule so that you get a better idea?
Yes, I'd be interested in looking at that. I think you can re-use the link I sent you previously.
Also, I pushed a 25292 build this morning. One of the two issues addressed therein could be related to what you are seeing, so if you haven't gone to that build yet, you might try it first.
-
@toggledbits just sent that rule as a JSON, and updated to 25292 which did not address this issue of scrolling a long rule
@tunnus said in [Reactor] Variables not updating correctly in latest-25201-2aa18550:
this issue of scrolling a long rule
That is a huge rule. I'm not going to address this now. I may look at it later, but I think this case is a true outlier and the juice may not be worth the squeeze. jQuery determines when to draw the objects on screen, and that's causing the scroll shifts as it adds new objects and the browser adds them to the display. I can't prevent the scroll shift when new objects are added; that's between jQuery and the browser. The only thing I could reasonably do is prevent jQuery drawing anything at all until the entire structure has been created off-screen, then draw it all at once. My desktop is an i9-9900K, so pretty fast even by today's standards, and it takes almost 15 seconds in Brave to create the entire page structure. That means you'd have 15 seconds of a blank page (or maybe a spinner and a "Please wait...") while it works at it. I don't really feel like much is gained by doing that, because your real goal is to be able to start looking through the rule while it's still drawing.
This rule is begging to be broken up. For what it's worth, I'm also working on changes to support parameterized rules and reactions that could make logic like this more modular and less "verbose" and repetitious in terms of their structure.
Author of Multi-system Reactor and Reactor, DelayLight, Switchboard, and about a dozen other plugins that run on Vera and openLuup.
-
@tunnus said in [Reactor] Variables not updating correctly in latest-25201-2aa18550:
this issue of scrolling a long rule
That is a huge rule. I'm not going to address this now. I may look at it later, but I think this case is a true outlier and the juice may not be worth the squeeze. jQuery determines when to draw the objects on screen, and that's causing the scroll shifts as it adds new objects and the browser adds them to the display. I can't prevent the scroll shift when new objects are added; that's between jQuery and the browser. The only thing I could reasonably do is prevent jQuery drawing anything at all until the entire structure has been created off-screen, then draw it all at once. My desktop is an i9-9900K, so pretty fast even by today's standards, and it takes almost 15 seconds in Brave to create the entire page structure. That means you'd have 15 seconds of a blank page (or maybe a spinner and a "Please wait...") while it works at it. I don't really feel like much is gained by doing that, because your real goal is to be able to start looking through the rule while it's still drawing.
This rule is begging to be broken up. For what it's worth, I'm also working on changes to support parameterized rules and reactions that could make logic like this more modular and less "verbose" and repetitious in terms of their structure.
@toggledbits understood. The other case I still have is two rules having partly the same logic, but those identical bits are not behaving in the same way. Just started logging those rules and the next time they behave differently I’ll send you logs along with description what was expected to happen.
