Big radiator and temperature tends to overshoot

[Anycubic]

I'm testing a big radiator (1x2 meters), temperature tends to overshoot (i.e. set temperature was 21 and 22 reached + radiator was still pretty hot). I guess that maybe it's a matter of latency, which value can I try to play with (in RadiatorValve.cpp, right?).

[alexmohr]

All radiators overshoot at the moment. Try out the latest commit on the deepSleep branch. I changed the large temp diff to 3°C This has been implemented to heat up faster when a room is cold but this tends to overshoot a lot.

What was the baseline temperature when you activated the heating?

[Anycubic]

It was 22 degrees and measured was 21.6. I lowered to 21 to see if the reaction was going to be "let's close the valve like a crazy" but the reaction was "lazy" instead ;-)

Could it also help increasing this closeTime values?

if (predictDiff <= 1) {
        closeTime = 600;
      } else if (predictDiff <= 0.5) {
        closeTime = 300;

[alexmohr]

Good idea, take a look at https://github.com/alexmohr/open-heat/commit/697c1361aac4b35ccfb6d4fd467e018c7e52bfeb If this is still not closing fast enough using the timings from the "open branch" could be an option. At least for small differences the previous version worked quite well. But I turned it off completely after it overshoot but 1.5° and turned it back on after it cooled a bit.

Blue is set point and green means "heat"

Do you have the valve in home assistant or do you log temperatures? it could be quite useful for analyzing overshooting or other issues.

[Anycubic]

Good, I'll test the latest version. I have the valve in Home assistant and so I can check the history graph as well. 1.5° of overshoot I don't need the graph, I will just need a swimsuit :-D

[alexmohr]

At least for my "test radiator" the last change works pretty well

[Anycubic]

I'll test it in the next few days, temperatures are unusually mild this year (Emilia Romagna - Italy) hopefully it should get colder :-D

[Anycubic]

I made some tests, my big radiator was still overshooting a lot. Looking for ideas I saw ISTAtrol code, where MOT_OPEN_TIME is double than MOT_CLOSE_TIME while in your code is quite the opposite. So I changed the closeTime values accordingly:

if (predictDiff <= 2) {
        closeTime = 5000;
      } else if (predictDiff <= 1.5) {
        closeTime = 4000;
      } else if (predictDiff <= 1) {
        closeTime = 2500;
      } else if (predictDiff <= 0.5) {
        closeTime = 1500;

As per attached graph the result is awesome! I'll keep on testing also with my smaller radiators .

[alexmohr]

Awesome, I'll test with these values too

[Anycubic]

There must be something wrong in the code. After approx 90 minutes that the radiator was switched on the temperature started to raise and raise. I switched off but the valve didn't close. I had to reset the ESP and immediately the valve closed but it was pretty open. It's like it was behaving at opposite of it was supposed to do, opening instead of closing an then just didn't close at all

.

[alexmohr]

Make sure the wiring is according to the schematic. I fixed some inconsistentencies a while ago. Motor ground should be pin 12 and vin should be 14. I'll test it further tomorrow but its working fine so far for me

[Anycubic]

Yes I was aware at a certain point Vin and ground were swapped, that's not the case. Wires are fine, there must be something tricky which makes the valve get crazy, as I said reset and voilà it full closed. I'll try tounderstand why. At a certain point I restarted HA but I don't think this was the issue

[alexmohr]

Actually I had the same issue over night. I'll try to fix it

[Anycubic]

Good you could reproduce it. It's like if when the set temperature is close to the effective one something starts to go wrong. I wonder also if that "fail to close the valve on shut off" was due to the same bug

[alexmohr]

Well I increased the time again to 90s and it's working for the moment. But I'm pretty sure it will break again sooner or later. Reading the valve state back is not trivial and needs additional hardware, 7 resistors, 3 OPs and 2 diodes to realize at schmitt trigger based approach as I don't think the approach with another ADC is viable. I'm not sure yet if I'm going to implement this. For the moment the current solution is "good enough"

[Anycubic]

Of course being able to know when the valve is actually closed (or opened) is the best thing..So what does happen? The valve position goes out of synch with calculations? Out of curiosity: isn't going to harm the motor that the valve being closed and motor still trying to push? Real closing time is around 40 seconds with my valves

[alexmohr]

I'm still not sure what happens I probably have to write tests at some point to find bugs like these in a controlled environment. The calculations should be done is such way that they cannot run out of sync.

It's probably not too good for the motor. I can't really tell how this affects long longevity of it. Maybe it reduces the lifespan of the motor a bit.

I just ordered the necessary parts to implement a "stop detection" on a breadboard at some point. This also would make the voltage regulator you asked a couple of days ago mandatory because the circuit would depend on a constant Vcc. But I don't think I'll get around to try this any time soon.

[Anycubic]

What about issuing e a "off" command followed by a simulated ESP off/on sequence and "heet".command every on hour or so? I mean if the valve was stuck/out of synch this will restart it properly. Don't know how the swtich on/off could be done properly via software, I mean emulating the unplug/plug which at least closes the valve eventually if it got stuck.

[alexmohr]

No I won't implement something like this because it's bad for the battery life (and it's already bad). Either I'll fix the time based approach or the "stop detection" will fix this. The stop detection also should be using less battery.

But to be honest if I'm not getting it a battery lifetime of at least 30 days I'll drop this project anyways. I have some things in the pipeline to improve battery lifetime like the extended modem sleep over night you suggested. At the moment it's using about 5% battery in 24h (with heating enabled).

[Anycubic]

Which battery are tou talking about? 2 18650 in parallel? I can't believe 30 days only with such a batteries! Btw 15minutes wifi + very deep sleep during night time should be very helpful. As far as I'm concerned if you fix the stuck valve problem your work is pretty good and I'm going to use it in all of my radiators (approx 14...)

[alexmohr]

2x18650 but probably my batteries are not the best. I already implemented the option to set the modem sleep time via mqtt. Automating the through home assitant or a cron job must be done manually. I'll commit it as soon as it's tested. I haven't seen the stuck valve in a while so maybe you can test it again?

[Anycubic]

Today I will flash the latest available deep sleep release

[Anycubic]

I come to the conclusion that your default openTime, closeTime and checkIntervalMillis_ are too much aggressive considering my big radiator latency: before the valve could measure ANY change in temperature VALVE_FULL_ROTATE_TIME value will be exceeded quite easily (unless you set in many minutes) -> the valve will go out synch/range. So I'm testing with much lower values and 5 minutes checkIntervalMillis_, I'll let you know how it goes. I guess that the only real solution is being able to know when the valve is actually opened and closed

[alexmohr]

I don't think that is the issue because if the valve is opened for 90s the pin which pushes against the actual valve of the radiator will be fully retracted. The latency of your radiator does not matter at all. But feel free to experiment with other values.

As I already mentioned before I won't get around to implement the "stop detection" any time soon. The necessary operational amplifiers are scheduled for delivery in January

Fyi I pushed a change which allows you to configure the modem sleep time via mqtt but It's not exposed on the webinterface. Check the readme for details.

[Anycubic]

ok thank you!

[Anycubic]

Fyi I pushed a change which allows you to configure the modem sleep time via mqtt but It's not exposed on the webinterface. Check the readme for details.

I couldn't find anything about that in the readme, am I still sleeping? :-D

[alexmohr]

No you are not. I messed up merging the readme. I'll update it later today. modemsleep/set and modemsleep/get are the mqtt topics. Time is in milliseconds

[alexmohr]

But I have to say, that the modemsleep has no significant influence on battery life time. It still drops ~0.4% per hour.

[Anycubic]

But I have to say, that the modemsleep has no significant influence on battery life time. It still drops ~0.4% per hour.

So do you think that is actually moving the motor which is very power hungry? Maybe moving the motor the same amount you are doing is not good for batteries. I also noticed that %opening of the valve is not linear, i.e. opening it 1 second seems a lot compared to say 10 seconds (is it a matter of fluid pressure maybe?). I'm now working with these values, till now got NO crazy valve syndrome (still using VALVE_FULL_ROTATE_TIME = 60) but I need to test more:

unsigned short openTime = 200U;
unsigned short closeTime = 400U;

openTime = 1000;
      } else if (predictDiff >= 2) {
        openTime = 1000;
      } else if (predictDiff >= 1.5) {
        openTime = 500;
      } else if (predictDiff >= 1) {
        openTime = 500;
      } else if (predictDiff >= 0.5) {
        openTime = 500;

  closeTime = 1500;
      } else if (predictDiff <= 1.5) {
        closeTime = 800;
      } else if (predictDiff <= 1) {
        closeTime = 800;
      } else if (predictDiff <= 0.5) {
        closeTime = 800;

[alexmohr]

No it's the esp itself. Heating was turned off over night. The motor is even more power hungry. But the discharge curve of a battery is not linear anyways. I can make definite statements when I'm not updating the devices on a regular basis because that always uses a lot of power. I'll try out the values you've posted and see if they are working for me too. If they are you can either open a MR or I'll just commit it

[Anycubic]

Well it will be interesting to see how they work on a "normal" size radiator, btw need to test those values for more days, here is still not very cold so I can't test properly

[Anycubic]

Unfortunately the not closing bug it's still there. I will increase slowly VALVE_FULL_ROTATE_TIME value to try to find a sweet spot. I will also plug a full time serial monitor trying to understand when the error start ro build up. Please also note that those my closing numbers are too weak, I'll try with 2000 and 1000 just for now

[alexmohr]

The serial monitor is a great idea. Thanks

[Anycubic]

The serial monitor is a great idea. Thanks In debug mode the BME280 sensor is pretty overheated, it gives more than 1C then usual reading. Are you able to.make a special debug build which is behaving exactly like the deepsleep one but of course without going to sleep? I'd like to debug something which is as similar to reality as possible

[alexmohr]

replace ESP.deepSleep in https://github.com/alexmohr/open-heat/blob/dev/src/RTCMemory.cpp#L245 with delay

[Anycubic]

I actually thought the same thing but couldn't believe it was going to be so easy 🤣 But will serial monitor/debugging messages work without setting debug = true via MQTT?

[alexmohr]

yes debug mode is just used to disable sleep via mqtt i.e. to enable web interface

[Anycubic]

Ok I will test it in the next few days

[alexmohr]

I found a possible root cause.

Index: src/heating/RadiatorValve.cpp
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/src/heating/RadiatorValve.cpp b/src/heating/RadiatorValve.cpp
--- a/src/heating/RadiatorValve.cpp (revision d545fadc503d06a291dc5296d872434d0d487b4b)
+++ b/src/heating/RadiatorValve.cpp (date 1636707779581)
@@ -202,8 +202,11 @@
   }

   if (rtc::read().currentRotateTime < 0) {
-    const auto remainingTime
+    auto remainingTime
       = VALVE_FULL_ROTATE_TIME - std::abs(rtc::read().currentRotateTime);
+    if (remainingTime < 0) {
+      remainingTime = 0;
+    }
     rotateTime = std::min(rotateTime, static_cast<unsigned int>(remainingTime));
   }

@@ -237,8 +240,12 @@
   }

   if (rtc::read().currentRotateTime > 0) {
-    const auto remainingTime
+    auto remainingTime
       = VALVE_FULL_ROTATE_TIME - std::abs(rtc::read().currentRotateTime);
+    if (remainingTime < 0) {
+      remainingTime = 0;
+    }
+
     rotateTime = std::min(rotateTime, static_cast<unsigned int>(remainingTime));
   }

[Anycubic]

I'll try to understand later. Actually I'm logging the valve as we speak

[Anycubic]

Wait a minute, one says "<" and the other ">" 🤣

[alexmohr]

Because one is close and the other is open.

[Anycubic]

I see! I thought that during a merge you mistakenly inverted the sign. So I guess I'll think about all of the night long! :-D BTW, in my logs today I saw a pretty strange thing:

[1;37m[DEBUG] Updating RTCMemory [1;37m[DEBUG] Updating RTCMemory [1;32m[INFO] DEBUG: 0 [1;37m[DEBUG] Sleep times: valveSleep: 299999, mqttSleep: 295521 [1;37m[DEBUG] Updating RTCMemory [1;37m[DEBUG] Updating RTCMemory [1;37m[DEBUG] Closing valve for 0ms, currentRotateTime: -4464ms, vin: 14, ground: 12 [1;37m[DEBUG] Updating RTCMemory [1;37m[DEBUG] Updating RTCMemory [1;32m[INFO] DEBUG: 0 [1;37m[DEBUG] Sleep times: valveSleep: 299995, mqttSleep: 299978 Updating RTCMemory ........ [1;37m[DEBUG] `Opening valve for 1000ms, currentRotateTime: -2464ms, vin: 14, ground: 12 [1;37m[DEBUG] Updating RTCMemory [1;32m[INFO] DEBUG: 0 1;37m[DEBUG] Updating RTCMemory [1;37m[DEBUG] Updating RTCMemory ........ [1;37m[DEBUG]Closing valve for 4464ms, currentRotateTime: -4000ms, vin: 14, ground: 12 [1;37m[DEBUG] Updating RTCMemory [1;37m[DEBUG] Updating RTCMemory [1;32m[INFO] DEBUG: 0 Updating RTCMemory [1;37m[DEBUG] Updating RTCMemory [1;37m[DEBUG]Closing valve for 464ms, currentRotateTime: -4464ms`, vin: 14, ground: 12 [1;37m[DEBUG] Updating RTCMemory [1;37m[DEBUG] Updating RTCMemory

Those where taken from change to "heat" and to "off". There is that initial currentRotateTime: -4464ms which I can't understand where is coming from. The log was taken right after flashing the firmware. The first 4 1000ms commands opening where never performed because of that -4464ms and the fifth was done partially which isn't right at all. Also on closing the negative term appears again. From where is this fixed negative offset coming from?

[Anycubic]

Uhmmm is your fix going to avoid those negative terms? Just guessing here.

if (remainingTime < 0) {
+      remainingTime = 0;

[alexmohr]

Negative rotate times are on purpose. 0 would be half way between fully open and fully closed. Rotate time of = max rotate time = fully open. Negative full rotate time is fully closed. The patch I posted should prevent that the subtraction wizh remaining time yields strange results when casting into and unsigned int. Although as it selects the minimum and the value is limit in the RTC storage it should not matter much.

About your trace: I guess you've updated the device via OTA? In this case the data from ram is kept the restart. Only a power cycle with unplugging the esp will erase them. Also please post the non truncated log instead of snippets. Changing the mode to off should also close the valve immediately and fully so something does not add up

[alexmohr]

If the current rotate time is negative no range check if performed either ... https://github.com/alexmohr/open-heat/blob/dev/src/heating/RadiatorValve.cpp#L246

[Anycubic]

About your trace: I guess you've updated the device via OTA? In this case the data from ram is kept the restart. Only a power cycle with unplugging the esp will erase them. Also please post the non truncated log instead of snippets. Changing the mode to off should also close the valve immediately and fully so something does not add up

I made a power cycle before logging, I had to make it in order to connect the ESP to my PC...

[Anycubic]

Just noticed this one in the compiler window:

src/heating/RadiatorValve.cpp: In member function 'long unsigned int open_heat::heating::RadiatorValve::loop()':
src/heating/RadiatorValve.cpp:25:16: warning: unsigned conversion from 'int' to 'short unsigned int' changes value from '70000' to '4464' [-Woverflow]
   25 |     closeValve(VALVE_FULL_ROTATE_TIME);
      |                ^~~~~~~~~~~~~~~~~~~~~~
src/heating/RadiatorValve.cpp:31:15: warning: unsigned conversion from 'int' to 'short unsigned int' changes value from '70000' to '4464' [-Woverflow]
   31 |     openValve(VALVE_FULL_ROTATE_TIME);

You see the same 4464. So VALVE_FULL_ROTATE_TIME is always mistakenly converted in 4464?

[alexmohr]

If you changed the valuve above 2^16 you will see this. It's changed already on the dev branch

[Anycubic]

When did you fix it? I compiled dev branch last night and got the warning

[alexmohr]

I just checked and it's not pushed yet. I'll do it later.