Update rules library version - 2nd attempt

[IgorYbema]

This PR exists due to a revert of the merge of #144

A bug exists which is discussed in that PR. Need to discuss it further here.

[IgorYbema]

@CurlyMoo can you update the PR with the latest commits from your rules repo and help to understand why the global vars changed without any reason like mentioned in the #144 pr?
I was hoping to take this PR to v3.9 which I want to release soon.

[stumbaumr]

Thanks for reverting. I continue testing for now on https://github.com/IgorYbema/HeishaMon/actions/runs/12793342635

[CurlyMoo]

Have you pinpointed the troubling commit?

[IgorYbema]

I did not go further then this PR commit but to me it looks like the large f9aabd4

The other commits are just small ones and don't see the issue there.

The large commit is probably a merge of commits of the rules library commits but you would know better where to look I hope

[CurlyMoo]

Is it possible to further simplify the rules to trigger the bug. I've analyzed the bytecode output with the previous and current version of the rule lib, but that doesn't show any functional difference.

[CurlyMoo]

And to help debug this you can also add a #define DEBUG at the top of the rules.cpp of the rules library. That shows you each step it takes. That helps identify the failing condition.

[IgorYbema]

And to help debug this you can also add a #define DEBUG at the top of the rules.cpp of the rules library. That shows you each step it takes. That helps identify the failing condition.

You could that that also right? Just get a normal ESP8266 and upload the firmware to it. Doesn't need to be a heishamon board and doesn't need to be connected to a heatpump. Just load the rules and notice that after 40 seconds when the timer starts that the global vars are changed

[IgorYbema]

A small update. If I change the ruleset to this it still fails (it overwrites global vars with random numbers). But if I remove the "&& $minute == 0" part it works fine. Does this give a clue @CurlyMoo ?

on System#Boot then
   #stateBeforeDHW = 1;
   #OpModeBeforeDHW = 0;
   #targetLowBeforeDHW = 31;
   #targetHighBeforeDHW = 40;
   #PumpDutyBeforeDHW = 110;
   #offAtNight = 1;
   @SetPump = 0;
   setTimer(10, 10);
end

on timer=10 then
   setTimer(10, 10);
   $hour = %hour;
   $minute = %minute;
   if $hour == 6 && $minute == 0 then
       #PumpDutyBeforeDHW = 160;
   end
end

[IgorYbema]

Even funkier. If I strip it down to below it still fails on overwriting some global vars. But if I remove "#stateBeforeDHW = 1;" from the systemboot, heishamon crashes. I am not at my test station now so can't see the crash report. But this PR is too buggy I am afraid :(

on System#Boot then
   #stateBeforeDHW = 1;
   #BeforeDHW = 0;
   #targetLowBeforeDHW = 31;
   $test = 1;
   setTimer(10, 10);
end

on timer=10 then
   setTimer(10, 10);
   $hour = %hour;
   $minute = %minute;
   if $hour == 12 && $minute == 0 then
       $test = 1;
   end
end

[CurlyMoo]

Somehow the rules lib outputs something different on the ESP8266 than on a amd64:

ESP8266

 0      OP_PUSH         -1      0       0        // 1
 1      OP_PUSH         -1      0       0        // 1
 2      OP_CLEAR        1       1       1
 3      OP_GETVAL       -5      7       0
 4      OP_SETVAL       2       -5      1
 5      OP_GETVAL       -5      9       0
 6      OP_SETVAL       3       -5      1
 7      OP_EQ           -5      2       -2
 8      OP_GETVAL       -6      8       0
 9      OP_CALL         -7      4       1        // 1
10      OP_GETVAL       -8      6       0
11      OP_EQ           -6      -6      -3
12      OP_AND          -5      -5      -6
13      OP_JMP          15      0       0
14      OP_SETVAL       4       -4      0
15      OP_RET          0       0       0

 1      VINTEGER        10
 2      VINTEGER        12
 3      VINTEGER        0
 4      VINTEGER        1
 5      VNULL
 6      VNULL
 7      VNULL
 8      VNULL

1 0     System#Boot
1 0     #stateBeforeDHW
1 0     #BeforeDHW
1 0     #targetLowBeforeDHW
1 0     $test
1 0     timer=10
1 0     $hour
1 0     %hour
1 0     $minute
1 0     %minute

amd64

 0      OP_PUSH         -1      0       0        // 1
 1      OP_PUSH         -1      0       0        // 1
 2      OP_CALL         -5      8       0        // 1
 3      OP_CLEAR        0       0       0
 4      OP_GETVAL       -6      7       0
 5      OP_SETVAL       6       -6      7
 6      OP_GETVAL       -5      9       0
 7      OP_SETVAL       8       -5      0
 8      OP_EQ           -6      6       -2
 9      OP_GETVAL       -7      8       0
10      OP_GETVAL       -8      6       0
11      OP_EQ           -7      -7      -3
12      OP_AND          -6      -6      -7
13      OP_JMP          15      0       0
14      OP_SETVAL       4       -4      0
15      OP_RET          0       0       0

 1      VINTEGER        10
 2      VINTEGER        12
 3      VINTEGER        0
 4      VINTEGER        1
 5      VNULL
 6      VNULL
 7      VNULL
 8      VNULL

 0 1 0  System#Boot
 1 1 0  #stateBeforeDHW
 2 1 0  #BeforeDHW
 3 1 0  #targetLowBeforeDHW
 4 1 0  $test
 5 1 0  timer=10
 6 1 0  $hour
 7 1 0  %hour
 8 1 0  $minute
 9 1 0  %minute

I do not yet know why.

[IgorYbema]

Is there a good reason for this PR to be merged into v3.9? What does it improve? Or can it wait until the bug is found?
I really want to release v3.9 soon because of improvement in the other changes it has.

[CurlyMoo]

It fixes one of the issues filed in the Egyras repo.

[IgorYbema]

I see, this issue Egyras#291

[stumbaumr]

I believe @CurlyMoo meant to fix issues Egyras#573 and Egyras#557 .
Issue Egyras#291 has already been fixed with the previous release 3.8 - at least I am using it in my rules...

[IgorYbema]

I believe @CurlyMoo meant to fix issues Egyras#573 and Egyras#557 . Issue Egyras#291 has already been fixed with the previous release 3.8 - at least I am using it in my rules...

Ok thank you. Both are not blocking for a v3.9 release. And for now it doesn't seem to be that the issue with this PR is fixed soon

[CurlyMoo]

And for now it doesn't seem to be that the issue with this PR is fixed soon

What makes you think that?

[IgorYbema]

And for now it doesn't seem to be that the issue with this PR is fixed soon

What makes you think that?

Or maybe not :-)

[CurlyMoo]

Should be fixed.

[IgorYbema]

Just tried this PR build but get "FATAL: function call 'setTimer' failed" while loading the rules from the comment #153 (comment)

Did you test this yourself also?

[CurlyMoo]

Yes, here: CurlyMoo/rules@605a896

[CurlyMoo]

Hmm, i tested with an even more compact ruleset, which didn't contain setTimer but max. Let me see why it fails.

[CurlyMoo]

Wierd, i indeed see that setTimer is handled differently than max.

[CurlyMoo]

It also fails in my unittests when i change max to ceil, so it's somehow dependent on the function array index.

[CurlyMoo]

Fixed

[blb4github]

Great to see a lot of activities! Are there specific things I can help testing?

[stumbaumr]

I am running now on https://github.com/IgorYbema/HeishaMon/actions/runs/12837044720
First tests look ok.
Will have to wait for a DHW production process to be sure though...
@blb4github , you could try running the firmware from that page as well - it is in the HeishaMon.ino.bin by Artifacts on the bottom of that page...

[blb4github]

I’m on that same version as well.Op 18 jan 2025 om 00:16 heeft stumbaumr ***@***.***> het volgende geschreven: I am running now on https://github.com/IgorYbema/HeishaMon/actions/runs/12837044720 First tests look ok. Will have to wait for a DHW production process to be sure though... —Reply to this email directly, view it on GitHub, or unsubscribe.You are receiving this because you commented.Message ID: ***@***.***>

[IgorYbema]

I confirm it passed the test which I did also. Waiting for you @stumbaumr to call it ok after a DHW run

[IgorYbema]

I already, by mistake, merged this into the main release. Wanted to make a testbranch and merge it there but forgot to switch branches. Don't want to revert it as probably this is still needed anyway. But that means that https://github.com/IgorYbema/HeishaMon/actions/runs/12842473673 will containt this PR and all other new stuff from v3.9. Hopefully this will be the final v3.9 release.

[stumbaumr]

Works fine in my setup. My smelly teenage sons even triggered a second DHW production. All good now for me!

[blb4github]

I don't know what was wrong with DHW but this afternoon my rules did work as planned to produce DHW and a Sterilization run as well.