Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update rule library version #121

Merged
merged 44 commits into from
May 13, 2024
Merged

Update rule library version #121

merged 44 commits into from
May 13, 2024

Conversation

CurlyMoo
Copy link

This contains various improvements:

  • Async running of rules
  • 2nd heap safe without extra compiler arguments
  • Smaller rules footprint

@CurlyMoo
Copy link
Author

@blb4github can i ask you to test this PR next?

@blb4github
Copy link

I’ll test is. What can I expect from it?

@CurlyMoo
Copy link
Author

Not much at this point from a user point of view, but on the backend a lot has changed and made more performant. Also this version can be set to run fully async.

@CurlyMoo
Copy link
Author

Funny thing right. Nearly 5,921 line additions and 3,021 line removals and as an end user, you won't notice much 😄

@blb4github
Copy link

It's running now for some moments, I have rebooted it a couple of times and first look is the same, there is something wrong with Opentherm values on ght GUI:
Scherm­afbeelding 2023-10-16 om 20 21 08

I have a rule which sets ?maxTSet = #maxTa and #maxTa = 33.33
?chSetpoint shows 100, on console it's 100 as well? Now I switched back to 3.2 beta 3 ?chSetpoint goes back to the correct 30.00.

@blb4github
Copy link

anything more to test? I'm running 3.2.1. now

@CurlyMoo
Copy link
Author

Not at the moment.

@CurlyMoo
Copy link
Author

@blb4github
Copy link

Hi @CurlyMoo, I tried yesterday evening, it was not stable at all. I could only install other FW by deleting all rules and switch off OT.

@CurlyMoo
Copy link
Author

CurlyMoo commented Oct 19, 2023 via email

@blb4github
Copy link

it's a bit difficult to test it but I'll try to give a better description:
after install this FW or a reboot it crashes before the Web if loads; the rules don't function, OT values stays default.
Web if is very slow, 1W sensors stays on -127, console info stops after a couple of seconds.
I could load another firmware with switching off OT function.

@CurlyMoo
Copy link
Author

Can you try removing all rules and check how the rest runs?

@blb4github
Copy link

blb4github commented Oct 19, 2023

also without rules is was constantly crashing, approx every 10 seconds. Switching off OT did make it much more stable, it's running now for 25 minutes

@CurlyMoo
Copy link
Author

@blb4github
Copy link

blb4github commented Oct 23, 2023

I've loaded this last version:
it's crashing every approx 15 seconds, with and without rules
it's not picking up old S0 values;
1W sensors stay -127
I have to switch off OT to be able to load other firmware

@CurlyMoo
Copy link
Author

Can you load this firmware on your test HeishaMon and let me know a concrete trigger for the crash?

@blb4github
Copy link

I'll do this evening

@CurlyMoo
Copy link
Author

CurlyMoo commented Nov 6, 2023

I found it really hard to get it to crash.

@blb4github
Copy link

Hi @CurlyMoo, I have tried it once more last week on both my spare and live HeishaMon, the result for me (again):
On my live HeishaMon it's un-usable, it crashes constantly. If I take out the rules and opentherm it's better, I even see 1W temperatures and S0.
On my spare HeishaMon it stays up without rules, with rules is crashes as well.

@CurlyMoo
Copy link
Author

Thanks for testing!

I switched from a Wemos D1 to an actual OpenTherm Heishamon for additional debugging. I will get back soon with results.

@CurlyMoo
Copy link
Author

CurlyMoo commented Jun 8, 2024

You're right. My fixed introduced new issues.

@CurlyMoo
Copy link
Author

CurlyMoo commented Jun 8, 2024

@McMagellan Can you check the latest commit?

@McMagellan
Copy link

Version Alpha-0776a05, build #537 on Heishamon V4.

look how beautifully it works !!

A 22 minute test with my test rule and a real heating cycle is recorded in the attached log file.
I haven't seen any more malfunctions.

I think this is a big step towards better acceptance of rules. I will now change my main rule "CoPilot" to the new situation.
Winter can come. :-))

Thank you

Log trigger fail V4.txt

@CurlyMoo
Copy link
Author

CurlyMoo commented Jun 8, 2024

I think this is a big step towards better acceptance of rules. I will now change my main rule "CoPilot" to the new situation.

I'd wish more people would be so cooperative.

@McMagellan
Copy link

In reality, you provide the cooperative service.
Yesterday I gave you the idea of ​​looking at the wrong trigger value and today it has been implemented in an improved form without cyclic repetition.
.
.
.
Next idea will come next week. :-)

@CurlyMoo
Copy link
Author

CurlyMoo commented Jun 8, 2024 via email

@blb4github
Copy link

I think this is a big step towards better acceptance of rules. I will now change my main rule "CoPilot" to the new situation.

I'd wish more people would be so cooperative.

I'm sorry, I follow the conversation but not more than that on this moment. Thanks both for the progress you're making!!

I was a bit frustrated with all issues with larger rules sets on the esp8266 based HeishaMon, I'm very happy with the huge improvement (stability, rules size) of the latest HW (v5 and firmware 3.5 and above). I'm still updating to Alpha firmware, I'm now running Alpha-0776a05 and that's running stable (for 5 hours now). I did use the minify solution from @klaashoekstra94 but that's not necessary anymore as the rules size seems unlimited now.

I'm planning to improve my rules set, especially for cooling but I have not found time for that yet.

@McMagellan
Copy link

McMagellan commented Jun 10, 2024

Feedback: Heishamon crash during connecting webserver
Version Alpha-0776a05, build #537 on Heishamon V4.

@McMagellan Can you discuss this in the issues instead of in this PR.

done

@CurlyMoo
Copy link
Author

@McMagellan Can you discuss this in the issues instead of in this PR.

@McMagellan
Copy link

@CurlyMoo Can you give me a link? I´m not an expert in Github.

@CurlyMoo
Copy link
Author

@McMagellan
Copy link

@CurlyMoo:
seen issue Egyras#496 vor error:
ERROR: cannot compute * with a left char value

@CurlyMoo
Copy link
Author

Confirmed, working on a fix.

@CurlyMoo
Copy link
Author

In the meanwhile, you can enclose these formulas in if then blocks:

on System#Boot then
  settimer(1,10);
end

on timer=1 then
  #Leistung = 3800;
  #Ampere = 2.0;
   if 1 then
      $BK1COP = round(#Leistung / ((#Ampere * 230) + 50) * 10) / 10;
   end
  #Ampere = 2.1;
   if 1 then
       $BK2COP = round(#Leistung / ((#Ampere * 230) + 50) * 10) / 10;
   end
  #Ampere = 2.2;
    $BK3COP = round(#Leistung / ((#Ampere * 230) + 50) * 10) / 10;
  #Ampere = 2.3;
    $BK4COP = round(#Leistung / ((#Ampere * 230) + 50) * 10) / 10;
  #Ampere = 2.4;
    $BK5COP = round(#Leistung / ((#Ampere * 230) + 50) * 10) / 10;
  #Ampere = 2.5;
    $BK6COP = round(#Leistung / ((#Ampere * 230) + 50) * 10) / 10;
  #Ampere = 2.6;
    $BK7COP = round(#Leistung / ((#Ampere * 230) + 50) * 10) / 10;
  #Ampere = 2.7;
    $BK8COP = round(#Leistung / ((#Ampere * 230) + 50) * 10) / 10;
  #Ampere = 2.8;
    $BK9COP = round(#Leistung / ((#Ampere * 230) + 50) * 10) / 10;
end

@CurlyMoo
Copy link
Author

Fixed

@McMagellan
Copy link

Switch to Version: Alpha-8727660, build #539 on Heishamon V4.
I can confirm that the test rule with the error cannot compute * with a left char value now runs without errors.

After I loaded my CoPilot Rule it no longer works.
Error: SetCurves JSON decode failed! See image.
I suspect that the SetCurves command is not implemented or wrong in this firmware.
Therefore I switched back to firmware build #538.

You can also see that the variable #ATBackup = $HKTempUnten was supplied with the name string of a variable from the rules code instead of a numerical value (#ATBackup = @Outside_Temp;).
This may be a result of the failed SetCurve.

Bild 2024-06-14 09_57_19

I attached my not ready build CoPilot Rule so you can reproduce it.
CoPilot Ver 3.24.txt

@McMagellan
Copy link

Additional information about the malfunction in build #539.
Maybe that's the bug after we've been looking for a while in 3.5.
Sometimes calculations are wrong or Boolean decisions in the If command are illogical.
This looks like a case like that to me too.

What can be seen in the console log is a non-working SetCurves command and an incorrect assignment of a variable.
Everything comes from the same rule (on VLUpdate then) which is the last one in the ruleset. The variable $HKTempUnten exists as a local variable only in this rule and #ATBackup is only written once in the System#Boot and then exclusively in this rule.

It looks to me as if the correct internal memory allocation of the variables in this rule is incorrect when parsing.
It would be a stroke of luck to be able to identify this malfunction. The difference between build #538 and build #539 is also a clue.
See image with the VLUpdate Rule. In the log file above, the rule is called with the variable #VLBasis = 2.

Bild 2024-06-14 12_52_26

@CurlyMoo
Copy link
Author

CurlyMoo commented Jun 14, 2024 via email

@CurlyMoo
Copy link
Author

I again pushed a fix.

@McMagellan
Copy link

I switcht to Version: Alpha-8dab112, build #541 on Heishamon V4, 14.6.2024.
I can confirm that my CoPilot Rule has now gone into operation without any errors.

Further tests will follow.
I would be interested to know if I was correct in my assessment of memory allocation?

@CurlyMoo
Copy link
Author

I would be interested to know if I was correct in my assessment of memory allocation?

No. You can read about it here:
https://github.com/CurlyMoo/rules/?tab=readme-ov-file#Efficiency, specially Free registry slots.

Inside the code, it's this function:
https://github.com/CurlyMoo/rules/blob/main/src/rules/rules.cpp#L1939

And again, more specifically this part where it tries to determine the start/end boundary within the free registry slots are determined:
https://github.com/CurlyMoo/rules/blob/main/src/rules/rules.cpp#L1953-L2003

That was too wide resulting in hitting the internal int8_t boundaries.

@McMagellan
Copy link

OK. That's too deep in the source program for me to understand anything there.
I only have a user's perspective and understanding of things that don't work as expected.

Two more questions:

  1. Could this also be the reason why the initialization process sometimes fails when the WebGUI is called up again? The tab freezes while ..loading.. is displayed, Heishamon restarts and the uptime restarts but without activating the rules engine.
  2. You wrote that the print command uses a lot of resources, which is why I removed all print lines and Heishamon actually ran more stable afterwards. Should I leave it like this for the next tests or use print again?

@CurlyMoo
Copy link
Author

  1. No, that's probably something inside the webserver we haven't tracked down.
  2. In general the Arduino IDE implementation of Serial prints are expensive. The rules print however also prints to the websocket connection which doubles the load.

@McMagellan
Copy link

McMagellan commented Jun 16, 2024

Feedback: Malfunction in calculating an IF command.
I use Version: Alpha-8dab112, build #541 on Heishamon V4.

If I remember correctly, this error was already described by @stumbaumr.
In the line: if @DHW_Holiday_Shift_Temp > 6 || @DHW_Holiday_Shift_Temp < 0 then the second value is not included in the Boolean evaluation.

The peculiarity:
If this rule is loaded as the only rule for testing, it works correctly.

on @DHW_Holiday_Shift_Temp then
  if @DHW_Holiday_Shift_Temp > 6 || @DHW_Holiday_Shift_Temp < 0 then
    #AnzahlBonusgrade = 2;
  else
    #AnzahlBonusgrade = @DHW_Holiday_Shift_Temp;
  end
  if @Holiday_Mode_State != 0 then
    @SetHolidayMode = 0;
  end
end

The same code integrated into my CoPilot ruleset (16 rules) causes this malfunction to occur reproducibly.
This excerpt from the console log presents both variants. In both cases it is expected that TOP25 values changed by remote control ​​above 6 and below 0 will be corrected to a default value of 2.

Screenshot 2024-06-16 095215

Attached is the working rule and the CoPilot ruleset where the malfunction occurs. This ruleset may contain additional errors because a lot has been changed and is untested in order to maintain reproducibility.

DHW holiday rule.txt
CoPilot V3.26.txt
.
.
.
One more small request.
It would be helpful to insert a blank line beforehand in the log file when a timer or trigger starts for better readability.

@CurlyMoo
Copy link
Author

I see what's happening. The rule block inside the failing one and as a separate rule:

Failing one:

 0	OP_GETVAL	-4	24	0	
 1	OP_LT		-4	-4	-2	
 2	OP_GETVAL	-4	24	0	
 3	OP_GT		-4	-4	-1	
 4	OP_OR		-4	-4	-4	
 5	OP_JMP		8	0	0	
 6	OP_SETVAL	12	-3	0	
 7	OP_JMP		10	0	0	
 8	OP_GETVAL	-4	24	0	
 9	OP_SETVAL	12	-4	0	
10	OP_GETVAL	-4	22	0	
11	OP_NE		-4	-4	-2	
12	OP_JMP		14	0	0	
13	OP_SETVAL	23	-2	0	
14	OP_RET		0	0	0	

 1	VINTEGER	6
 2	VINTEGER	0
 3	VINTEGER	2
 4	VNULL

What it should be:

 0      OP_GETVAL       -4      0       0
 1      OP_LT           -4      -4      -2 
 2      OP_GETVAL       -5      0       0
 3      OP_GT           -5      -5      -1
 4      OP_OR           -4      -5      -4
 5      OP_JMP          8       0       0
 6      OP_SETVAL       1       -3      0
 7      OP_JMP          10      0       0
 8      OP_GETVAL       -4      0       0
 9      OP_SETVAL       1       -4      0
10      OP_GETVAL       -4      2       0
11      OP_NE           -4      -4      -2
12      OP_JMP          14      0       0
13      OP_SETVAL       3       -2      0
14      OP_RET          0       0       0

 1      VINTEGER        6
 2      VINTEGER        0
 3      VINTEGER        2
 4      VNULL
 5      VNULL

@CurlyMoo
Copy link
Author

The latest commit should fix the issue.

@McMagellan
Copy link

Feedback: Error in IF evaluation
Version: Alpha-e17aac1, build #544
I can confirm that the last reproducible error in the if calculation no longer occurs.

This morning, after about 12 hours of operation, unlike every day before, there was no freezing of the tab when opening the WebGUI. The console worked correctly with the rules and did not have to be restarted.

Testing in the next few days will be difficult or not possible at all due to the high outside temperatures.
Thanks

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
7 participants