arjenhiemstra / ithowifi

Itho wifi add-on module (ESP32 wifi to itho I2C protocol)
GNU General Public License v3.0
194 stars 33 forks source link

Feature request - WPU heat-pump support #49

Closed jasperslits closed 2 years ago

jasperslits commented 3 years ago

Itho Daalderop also provides heat-pumps (water/water) which uses I2C communication as well similar to the HRU units.

Today the end-user can only control limited parameters (room temp, heating moment) but not read parameters. This is useful to get more information out of the device similar to what's used here: https://github.com/pommi/python-itho-wpu/tree/370209603b70a464821b8340ce9e07c4b6c08855

Why via this project if there are already solutions out there? 1) MQTT gives much more flexibility than storing it locally (in influx) 2) Soldering / Raspberry PI only adds complexity 3) Single PCB design for multiple devices made by Itho

I've created a simple script that reads the WPU database of Itho and converts it to a C++ header file to get a head start. It's here: https://github.com/jasperslits/ithowifi/blob/master/wpu.h

I was only able to compile test it (create a .cpp file and include the header) but would this be of any help as I cannot functionally test it?

Happy to assist with testing and on GoT there are more volunteers if needed.

arjenhiemstra commented 3 years ago

I've created a simple script that reads the WPU database of Itho and converts it to a C++ header file to get a head start. It's here: https://github.com/jasperslits/ithowifi/blob/master/wpu.h

Oh that's nice work!! Saves me hours of manual work!

arjenhiemstra commented 3 years ago

For status labels it might be a better idea to use longer names, are they available? And they need to be translated then from Dutch to English but that can be done quite easily with ie. https://www.deepl.com/translator

Also arrays that are the same could be squashed into 1, saving precious memory :)

jasperslits commented 3 years ago

I think the WPU devices are only used in NL but we have this for example:

130.0|FreeCoolingStabilizeTimer|Stabilisatie wachttijd vrijkoelen||sec. 131.0|PreHeatDHWMinOnTimer|Minimale tijd voorverwarmen tapwater||sec 132.0|PreHeatDHWMaxOnTimer|Maximale tijd voorverwarmen tapwater||sec. 133.0|RemainingTimeEelementReleased|Vrijgave elektrisch element||min

For preHeatDHWMinOnTimer we have the following labels available: 1) Voorverwarmen tapwater minimale aan tijd 2) Voorverwarmen tapwater is na activatie een minimale tijdsduur actief 3) Minimale tijd voorverwarmen tapwater

I guess you want to send preHeatDHWMinOnTimer in the MQTT pay load and either one of the 3 labels on the screen? I guess nr 3 (when translated) is more user friendly then?

And today this .h file is generated by a script so if multiple versions have the same labels, they can be squashed? I can add that logic, shouldn't be rocket science (compared to what you do here) ;)

arjenhiemstra commented 3 years ago

I think the WPU devices are only used in NL but we have this for example:

That might be the case, but still, I'd like to keep the interface in one language

I guess nr 3 (when translated) is more user friendly then?

Yes, sure is

Interesting to see that the repo you linked seems not to map labels based on itho fw versions, not sure if I missed something but that should not deliver the correct results in all cases.

arjenhiemstra commented 3 years ago

And today this .h file is generated by a script so if multiple versions have the same labels, they can be squashed? I can add that logic, shouldn't be rocket science (compared to what you do here) ;)

Yes, the squashed label arrays can be recognised by the start and end version number ie.: https://github.com/arjenhiemstra/ithowifi/blob/517c84ded1e9d2457fffdf496400ca92b07d6c7e/software/NRG_itho_wifi/IthoSystemConsts.h#L41

jasperslits commented 3 years ago

Yes, sure is

Interesting to see that the repo you linked seems not to map labels based on itho fw versions, not sure if I missed something but that should not deliver the correct results in all cases.

It does read the version from the WPU unit and then queries the database for the right labels using versiebeheer table. Each version has its own table (e.g. datalabels_v37) with some exceptions where multiple versions use the same datalabels table.

jasperslits commented 3 years ago

And today this .h file is generated by a script so if multiple versions have the same labels, they can be squashed? I can add that logic, shouldn't be rocket science (compared to what you do here) ;)

Yes, the squashed label arrays can be recognised by the start and end version number ie.:

https://github.com/arjenhiemstra/ithowifi/blob/517c84ded1e9d2457fffdf496400ca92b07d6c7e/software/NRG_itho_wifi/IthoSystemConsts.h#L41

Ah, now it makes sense. So I can adjust the naming convention. Didn't realise it was used for that squashing then

arjenhiemstra commented 3 years ago

This is where comments in code make sense I guess ;-)

jasperslits commented 3 years ago

So this then should work better?

const uint8_t itho_WPUset11_17[] = { 0,1,2,3,4,5,6,7,8,9,10,11,12,13,14,15,16,17,18,19,20,21,22,23,24,25,26,27,28,29,30,31,32,33,34,35,36,37,38,39,40,41,42,43,44,45,46,47,48,49,50,51,52,53,54,55,56,57,58,59,60,61,62,63,64,65,66,67,68,69,70,71,72,73,74,75,76,77,78,79,80,81,82,83,84,85,86,87,88,89,90,91,92,93,94,95,96,97,98 };

const uint8_t *ithoWPUSettingsMap[] =  { nullptr, itho_WPUset2,itho_WPUset4,itho_WPUset5,itho_WPUset6,itho_WPUset7,itho_WPUset8_9,itho_WPUset10,itho_WPUset11_17,itho_WPUset11_17,itho_WPUset11_17,itho_WPUset11_17,itho_WPUset11_17,itho_WPUset11_17,itho_WPUset18_19,itho_WPUset20,itho_WPUset21,itho_WPUset22,itho_WPUset23,itho_WPUset24,itho_WPUset25,itho_WPUset27,itho_WPUset31,itho_WPUset33,itho_WPUset34,itho_WPUset37};

Will tidy up the rest as well with some scripting.

arjenhiemstra commented 3 years ago

Yep, that saves memory, all those uint8_t values are loaded on the stack and thus consume memory during the lifetime of the program. Maybe it is better to write a file (or DB like system) to flash with all this info, not sure how though

jasperslits commented 3 years ago

I see. You don't want to put some if #defined (NON_CVE) around it? I've now a version for non-CVE and yet all other CVE values are defined and loaded?

Anyway, assume the squashed header file for settings and descriptions and Flash messages are provided. Is there anything else needed in terms of I2C output from debugging to glue to it all together?

arjenhiemstra commented 3 years ago

I see. You don't want to put some if #defined (NON_CVE) around it? I've now a version for non-CVE and yet all other CVE values are defined and loaded?

For now yes, I know some users that wired the CVE version to a non-CVE device. If I can keep the memory usage acceptable I would like to keep the firmware versions as much the same as possible.

Anyway, assume the squashed header file for settings and descriptions and Flash messages are provided. Is there anything else needed in terms of I2C output from debugging to glue to it all together?

Nope, with that status and settings should work when linked to the main struct here: https://github.com/arjenhiemstra/ithowifi/blob/517c84ded1e9d2457fffdf496400ca92b07d6c7e/software/NRG_itho_wifi/IthoSystem.cpp#L69-L90

All the other code should just wrap around it and work

jasperslits commented 3 years ago

Ok, new attempt of squashing:

https://github.com/jasperslits/ithowifi/blob/master/wpu.h

The Itho database is a bit weird: it has multiple versions pointing to the same datalabels. But it also has multiple versions with identical number of fields as others. Both are addressed.

Before spending time on the label translation, I'd like to get this tested first as part of an MVP / agile / whatever approach, just to see if the basics works. I can send a PR for it, but not sure if you want to have it as wpu.h (as-is for readability / testing) or merged with IthoSystemConstant.h and to see if it's a good enough shape?

Unit test shows this since I don't have an Arduino IDE and just plain GNU G++ compiler: me@playground:~/code$ g++ test.cpp

In file included from test.cpp:2:
wpu.h:41:7: error: ‘__FlashStringHelper’ does not name a type
   41 | const __FlashStringHelper *ithoWPUStatusLabels[] =  {
      |       ^~~~~~~~~~~~~~~~~~~
wpu.h:179:7: error: ‘__FlashStringHelper’ does not name a type
  179 | const __FlashStringHelper *ithoWPUSettingDescriptions[] = { 

me@playground:~/code$ cat test.cpp

#include <cstdint>
#include "wpu.h"
arjenhiemstra commented 3 years ago

__FlashStringHelper is a wrapper class defined here: https://github.com/espressif/arduino-esp32/blob/b94b38c9d11f3c4a85ac3e18a9ec22132351716d/cores/esp32/WString.h#L38

wpu.h or IthoSystemConstant.h are both fine, I'll pull the PR to a local dirty branch and test/compile the code. I can post the compiled version here

jasperslits commented 3 years ago

PR #50 is created for this

arjenhiemstra commented 3 years ago

nrgitho-noncve-v2.3-beta4-wputest.bin.zip Here's a compiled version, copied the code almost 1-on-1, only difference is that I use value 255 as a poor mans end indicator for the array so I've added these as last value in the array.

arjenhiemstra commented 3 years ago

Looking at this again, it seems that both the settings labels and the status labels are both status labels. It seems that the parameters are missing

basically, only test the status page, not the settings page cause changing settings will lead to undefined behaviour

jasperslits commented 3 years ago

Nice!

For pameters I have to see to translate it. There's another table called 'parameterlijst_v37' which has entries like:

sqlite> select * from parameterlijst_v37 where name = 'CO control enabled';
id|name|name_factory|min|max|def|title|description|unit
6.94683340033475e-310|CO control enabled|CO control enabled|2.12199579096527e-314|2.65249473870659e-314|2.12199579096527e-314|CO-klep regeling||

So you need just the 'name' field and not the min/max values? Or is that stored somewhere too?

It's connected now and returns 80 82 90 E0 01 07 00 01 00 0D 45 22 00 11 as device type.

Query status returns:

Result: 80 82 A4 01 01 D4 F4 48 0A A6 11 F0 07 EE 08 92 08 B2 08 B6 08 03 07 76 08 46 08 61 00 AA 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 32 32 00 00 00 01 00 00 00 07 CA 07 08 00 01 FF 00 07 BC 09 2B 07 BC 09 2B 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 01 F4 09 00 5A 94 91 19 2D 00 00 00 00 00 00 00 00 00 00 00 00 01 00 00 00 00 00 00 00 00 00 00 00 01 00 00 00 00 00 00 00 05 00 00 04 26 00 00 00 C5 00 00 00 56 00 01 25 1C 00 06 63 00 00 32 00 F5 10 00 96 00 03 89 00 00 00 00 00 FF 00 00 00 00 00 00 00 00 00 00 00 00 00 00 01 00 00 00 00 00 00 00 00 00 00 00 00 00 00 61

Status returns this but I don't know where temp, hum and ppmw are coming from as it's not applicable?: Some values are returned as 0 which is weird.

Can I see which version is used? Maybe there's something odd with the mapping.

Label Value
temp 0
hum 0
ppmw 0
buitentemp 0
boilertemp onder 0
boilertemp boven 0
verdamper temp 0
zuiggas temp 0
persgas temp 1
vloeistof temp 0
temp naar bron 0
temp uit bron 0
CV aanvoertemp 0
CV retourtemp 5
CV-druk 1062
stroom compressor 197
stroom e-element 86
drukschakelaar 0
tarief 1
condens beveiliging 95
spare ingang 0
vrijkoelklep 0
CV/DHW of koeltemp klep 9.05
compressor 0
element 0
trickle heating 0
fout 0
vrijkoelen 255
expansieventiel 0
kamertemp 0
gevr kamertemp 0
warmtevraag thermost. 0
status 0
sub_status 0
blokkade 0
berekende verdampings temp CV 1
berekende condensatie temp CV 0
berekende verdampings temp DHW 0
berekende condensatie temp DHW 0
fout_gevonden 0
fout_retry 0
taak actief 0
handbediening 0
compr blok 19.8
Elek blok 23.47
Correctie meting delta brontemperatuur 1635
vertraging cv start 1
vertraging cv stop 0
adaptief timer 0
adaptief oververhitting 0
adaptief fifo index 0
fout hoogste prioriteit 0
blokkeertijd vrijgave extern koelen 0
flowsensor 0
fase detectie 50
cv pomp 0
bron pomp 627.36
boiler pomp 150

Edit: looks like some values shifted. "stroom compressor" is measured in amps and a more realistic value is 5. So it looks like it's shifted a bit.

I am working on a new version of the labels and include the unit type (bar, degrees celcius, kWh etc) for the status so it's easier to validate. Mind sharing your new .h file so I can use it as a reference?

arjenhiemstra commented 3 years ago

the first three are always added in the current situation.

It's connected now and returns 80 82 90 E0 01 07 00 01 00 0D 45 22 00 11 as device type.

0D 45 22 0D = device type 22 = version (34 decimal)

Latest commit contains the info you provided, don't bother translation, let's first get the code ok, I'll translate it later again

jasperslits commented 3 years ago

the first three are always added in the current situation.

It's connected now and returns 80 82 90 E0 01 07 00 01 00 0D 45 22 00 11 as device type.

0D 45 22 0D = device type 22 = version (34 decimal)

Latest commit contains the info you provided, don't bother translation, let's first get the code ok, I'll translate it later again

Great! So the first 3 being added displayed doesn't cause the apparent down shift?

arjenhiemstra commented 3 years ago

No, it just this piece of code:

https://github.com/arjenhiemstra/ithowifi/blob/7ab30355dc46e3f4964ec645f011f5b85d0a0577/software/NRG_itho_wifi/10_Gen_func.ino#L2-L7

arjenhiemstra commented 3 years ago

Here the status message format command result is being parsed to determine the data length / type of the status message into a vector :

https://github.com/arjenhiemstra/ithowifi/blob/7ab30355dc46e3f4964ec645f011f5b85d0a0577/software/NRG_itho_wifi/IthoSystem.cpp#L532-L575

And with that info the status message itself is parsed and added to the vector created earlier: https://github.com/arjenhiemstra/ithowifi/blob/7ab30355dc46e3f4964ec645f011f5b85d0a0577/software/NRG_itho_wifi/IthoSystem.cpp#L620-L648

arjenhiemstra commented 3 years ago

So if the labels are not correct / or shifted, either te mapping is faulty or some cases are not handled correctly in the parsing of the status format

jasperslits commented 3 years ago

Ok, I am struggling a bit with debugging to see where it's coming from.

The debug page is empty and download the log (/curlog) gives a 404. So I cannot tell if that's related as /curlog is defined: server.on("/curlog", HTTP_GET, handleCurLogDownload);

Also the list of files is still empty when using /edit so could it be solved by a filesystem format? It was working OK before.

I also noticed this, not sure if it's telling anything:

1) ithoWPUStatusLabels has as 20th (starting at 1) element: F("flow sensor")

However under Status I see this: "flowsensor"

So the labels there are not defined in https://github.com/arjenhiemstra/ithowifi/blob/master/software/NRG_itho_wifi/IthoSystemConsts.h

arjenhiemstra commented 3 years ago

sorry, that has probably to do with the fact that I quickly added the translation yesterday but after the compilation. Here is the compiled version of the exact same code as is now on github. NRG_itho_wifi.ino.esp32.bin.zip

Also the list of files is still empty when using /edit so could it be solved by a filesystem format?

I did a format indeed and that solved it for me

jasperslits commented 3 years ago

Ok, to debug this. I can start with this Query Status?

" 80 82 A4 01 01 D4 F4 48 0B 38 10 FB 01 4A 03 6B 10 A1 09 BD 02 1A 03 50 09 8D 08 6A 00 E9 01 17 00 00 00 00 00 00 00 05 5A 00 5A 3D 00 00 00 01 00 00 01 00 01 F4 08 0E 08 66 64 02 07 00 01 18 0A 45 01 18 0A 45 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 09 00 00 00 00 00 00 00 00 00 00 00 00 2A 30 00 00 01 F4 20 00 5A 94 ED D3 2D 00 01 00 00 00 00 00 00 00 00 00 00 00 01 00 00 00 00 00 00 01 00 00 00 01 00 00 00 64 00 00 00 05 00 00 04 29 00 00 00 C5 00 00 00 56 00 01 27 10 00 06 63 00 00 00 00 F5 10 00 96 00 02 1A 05 78 3D 00 00 FF 00 00 00 00 0E 10 00 00 00 00 05 A0 00 00 01 00 00 00 00 00 00 00 00 00 00 00 00 00 00 C7"

I saw 1065 displayed as CV pressure which translates to 04 29 in hex and values are separated by 00? But will have a better look at your code this evening

Ah nvm, you explained it already.

jasperslits commented 3 years ago

I just realized now it's in English :) The data is now much better it seems but seems not 100% correct.

Label Value
temp 0
hum 0
ppmw 0
AirQuality (%) out of range low
AirQbased on 72
CO2level (ppm) 1250
Indoorhumidity (%) 5
Outdoorhumidity (%) 50
Exhausttemp (°C) 11.47
SupplyTemp (°C) 19.97
IndoorTemp (°C) 26.9
OutdoorTemp (°C) 22.3
SpeedCap 1124
BypassPos (%) 3
FanInfo unknown
ExhFanSpeed (%) 4
InFanSpeed (%) 31
RemainingTime (min) 2048
PostHeat (%) 0
PreHeat (%) 86
InFlow (l sec) 0
ExhFlow (l sec) 0
outdoor temp 0
boiler temp below 0
boiler temp up 0
evaporator temp 0
suction gas temp 0
compressed gas temp 1
liquid temp 0
temp to source 0
temp from source 0
CV supply temp 0
CV return temp 5
CV pressure 1069
flow compressor 197
flow e-element 86
pressure switch 0
rate 1
condensation protection 100
spare input 0
free cooling valve 0
CV/DHW or cooling temp valve 6.85
compressor 0
element 0
trickle heating 0
fault 0
free cooling 0
expansion valve 0
room temp 0
gevr room temp 0
heat demand thermost. 0
status 0
sub_status 0
blockage 0
calculated evaporation temp CV 21.06
calculated condensation temp CV 18
calculated evaporation temp DHW 0
calculated condensation temp DHW 1
error_found 255
error_retry 0
task active 11.25
manual operation 23.42
compr block 11.25
Elek block 23.42
correction measurement delta source temperature 1635
delay cv start 1
delay cv stop 0
adaptive timer 0
adaptive overheat 0
adaptive fifo index 0
error highest priority 0
Remaining_override_timer 0
flow sensor 0
phase detection 50
cv pump 0
source pump 627.36
boiler pump 150

I managed to hack the python script itho-wpu.py from Pommi and feed it with the "Query Status Format" and "Query Format" results (turning it into a hex array did the job) so it's not relying on I2C but even there the output seems not 100% reliable as -30.0 is just wrong:

Unknown data type for label In_P_switch: 0x0C
Buitentemp (°C) (t_out): -30.0
Boilertemp Onder (°C) (t_boildwn): 12.5
Boilertemp Boven (°C) (t_boiltop): 13.67
Verdamper Temp (°C) (t_evap): 11.29
Zuiggas Temp (°C) (t_suct): 19.97
Persgas Temp (°C) (t_disc): 27.07
Vloeistof Temp (°C) (t_cond): 22.3
Temp Naar Bron (°C) (t_source_r): 10.83
Temp Uit Bron (°C) (t_source_s): 18.18
Cv Aanvoertemp (°C) (t_ch_supp): 23.75
Cv Retourtemp (°C) (t_ch_ret): 20.3
Cv-Druk (Bar) (p_sens): 1.73
Stroom Compressor (A) (i_tr1): 0.0
Stroom E-Element (A) (i_tr2): 0.0
Drukschakelaar (in_p_switch): 0

Now "Boilertemp Onder" is reported at 12.5 for the Python script which makes sense. In add-on it's returning "boiler temp below" with 0.

"CV Pressure" is reported as 1069 but I dont see anywhere the 1.73 Bar returned in the Itho Status, so even if the label was wrong. I guess it should show up?

Edit 2: it seems that the values with type 0x92 are not showing. Could that provide a clue?

arjenhiemstra commented 3 years ago

The status labels are somehow not mapped correctly, left is what you have posted here, right is what should be the list if you have version 34.

ie. right your kamertemp is 21.06, sounds ok. same for the "berekende verdampings temp CV" values etc.

So it seems the status format is parsed ok (at least partly), but the status labels are not mapped correctly

<html xmlns:v="urn:schemas-microsoft-com:vml" xmlns:o="urn:schemas-microsoft-com:office:office" xmlns:x="urn:schemas-microsoft-com:office:excel" xmlns="http://www.w3.org/TR/REC-html40">

outdoor temp | 0 |   | 0 | buitentemp -- | -- | -- | -- | -- boiler temp below | 0 |   | 1 | boilertemp onder boiler temp up | 0 |   | 2 | boilertemp boven evaporator temp | 0 |   | 3 | verdamper temp suction gas temp | 0 |   | 4 | zuiggas temp compressed gas temp | 1 |   | 5 | persgas temp liquid temp | 0 |   | 6 | vloeistof temp temp to source | 0 |   | 7 | temp naar bron temp from source | 0 |   | 8 | temp uit bron CV supply temp | 0 |   | 9 | CV aanvoertemp CV return temp | 5 |   | 10 | CV retourtemp CV pressure | 1069 |   | 11 | CV-druk flow compressor | 197 |   | 12 | stroom compressor flow e-element | 86 |   | 13 | stroom e-element pressure switch | 0 |   | 14 | drukschakelaar rate | 1 |   | 15 | tarief condensation protection | 100 |   | 16 | condens beveiliging spare input | 0 |   | 17 | spare ingang free cooling valve | 0 |   | 18 | Reserve CV/DHW or cooling temp valve | 6.85 |   | 19 | flowsensor compressor | 0 |   | 20 | fase detectie element | 0 |   | 21 | cv pomp trickle heating | 0 |   | 22 | bron pomp fault | 0 |   | 23 | boiler pomp free cooling | 0 |   | 24 | vrijkoelklep expansion valve | 0 |   | 25 | CV/DHW of koeltemp klep room temp | 0 |   | 26 | compressor gevr room temp | 0 |   | 27 | element heat demand thermost. | 0 |   | 28 | trickle heating status | 0 |   | 29 | fout sub_status | 0 |   | 30 | vrijkoelen blockage | 0 |   | 31 | expansieventiel calculated evaporation temp CV | 21.06 |   | 32 | kamertemp calculated condensation temp CV | 18 |   | 33 | gevr kamertemp calculated evaporation temp DHW | 0 |   | 34 | warmtevraag thermost. calculated condensation temp DHW | 1 |   | 35 | status error_found | 255 |   | 36 | sub_status error_retry | 0 |   | 37 | blokkade task active | 11.25 |   | 38 | berekende verdampings temp CV manual operation | 23.42 |   | 39 | berekende condensatie temp CV compr block | 11.25 |   | 40 | berekende verdampings temp DHW Elek block | 23.42 |   | 41 | berekende condensatie temp DHW correction measurement delta source temperature | 1635 |   | 42 | fout_gevonden delay cv start | 1 |   | 43 | fout_retry delay cv stop | 0 |   | 44 | taak actief adaptive timer | 0 |   | 45 | handbediening adaptive overheat | 0 |   | 46 | compr blok adaptive fifo index | 0 |   | 47 | Elek blok error highest priority | 0 |   | 48 | cv voor / nadraai Remaining_override_timer | 0 |   | 49 | bron voor / nadraai flow sensor | 0 |   | 50 | boiler voor / nadraai phase detection | 50 |   | 51 | delay elektr cv pump | 0 |   | 52 | min draaitijd compr source pump | 627.36 |   | 53 | afpomp tijd boiler pump | 150 |   | 54 | EV drukvereffenen   |   |   | 55 | EV verstellen   |   |   | 56 | vrijkoel interval   |   |   | 57 | handbediening   |   |   | 58 | lage druk timer   |   |   | 59 | compressor start   |   |   | 60 | compr power on delay   |   |   | 61 | vertraging cv start   |   |   | 62 | vertraging cv stop   |   |   | 63 | adaptief timer   |   |   | 64 | adaptief oververhitting   |   |   | 65 | adaptief fifo index   |   |   | 66 | fout hoogste prioriteit   |   |   | 67 | UTC tijd   |   |   | 68 | Max cv retour temp   |   |   | 69 | bronpomp snelheid vrijkoel mode   |   |   | 70 | compressor in Cv mode   |   |   | 71 | compressor in boiler mode   |   |   | 72 | element in cv mode   |   |   | 73 | element in boiler mode   |   |   | 74 | CV mode geblokkeerd   |   |   | 75 | boiler mode geblokkeerd   |   |   | 76 | vrijkoel mode geblokkeerd   |   |   | 77 | ontlucht mode geblokkeerd   |   |   | 78 | elektr element geblokkeerd   |   |   | 79 | elektr element DHW geblokkeerd   |   |   | 80 | compressor geblokkeerd   |   |   | 81 | off mode actief   |   |   | 82 | CV mode actief   |   |   | 83 | boiler mode actief   |   |   | 84 | vrijkoel mode actief   |   |   | 85 | CV pomp voordraaien   |   |   | 86 | bron pomp voordraaien   |   |   | 87 | element vrijgegeven   |   |   | 88 | vrijgave extra koelen   |   |   | 89 | CV vrijgegeven   |   |   | 90 | vrijkoel vrijgegeven   |   |   | 91 | tarief laag vanuit thermostaat   |   |   | 92 | ontluchten vanuit thermostaat   |   |   | 93 | ECO geselecteerd op thermostaat   |   |   | 94 | comfort geselecteerd op thermostaat   |   |   | 95 | boiler blokkade vanuit thermostaat   |   |   | 96 | boiler boost vanuit thermostaat   |   |   | 97 | warmtevraag totaal   |   |   | 98 | E-verbuik tijdens stand-by   |   |   | 99 | E-verbuik tijdens verwarmen   |   |   | 100 | E-verbuik tijdens DHW   |   |   | 101 | E-verbuik tijdens koelen   |   |   | 102 | voorverwarmen tapwater   |   |   | 103 | element geblokkeerd tijdens retry   |   |   | 104 | Pi fout bronflow   |   |   | 105 | actuele bronklepstand   |   |   | 106 | utc Tijd offset   |   |   | 107 | vrijkoel blokkade tijd   |   |   | 108 | koeltemp regelklep setpoint   |   |   | 109 | CO-klep positie   |   |   | 110 | HRU uitblaas temp   |   |   | 111 | HRU uitblaas flow   |   |   | 112 | regeneratie actief   |   |   | 113 | laatst geldige brontoevoer temp.   |   |   | 114 | bronpomp flow setpoint   |   |   | 115 | bronpomp snelheid bij Compr   |   |   | 116 | bronpomp snelheid bij vrijkoel   |   |   | 117 | bronpomp snelheid bij luchtreg   |   |   | 118 | vrijkoelmodus   |   |   | 119 | tijd CO-klep start positie   |   |   | 120 | minimale vrijgave tijd extern koelen   |   |   | 121 | blokkeertijd vrijgave extern koelen   |   |   | 122 | blokkeertijd vrijgave extern verwarmen   |   |   | 123 | blokkeertijd trickle low na power-up   |   |   | 124 | blokkeertijd trickle low na CV-bedrijf   |   |   | 125 | blokkeertijd voorverwarmen tapwater   |   |   | 126 | UTC tijd :   |   |   | 127 | slow start bronpomp   |   |   | 128 | Bron retourtemperatuur te laag   |   |   | 129 | Regelperiode bronflow   |   |   | 130 | Stabilisatie wachttijd vrijkoelen   |   |   | 131 | Minimale tijd voorverwarmen tapwater   |   |   | 132 | Maximale tijd voorverwarmen tapwater   |   |   | 133 | Vrijgave elektrisch element

arjenhiemstra commented 3 years ago

The 0x92 is also part of the normal CVE status format and there it is parsed correctly. I would expect the same to happen here but there might be another reason. Could you post the status format result as well?

jasperslits commented 3 years ago

So I guess there are two problems: 1) Labels in wrong order 2) Missing values (e.g. reported as 0 by the add-on

Status format: 80 82 A4 00 01 86 92 92 92 92 92 92 92 92 92 92 92 92 92 92 0C 0C 0C 0C 00 10 0C 00 00 00 00 00 0C 0C 0C 0C 0C 10 92 92 00 00 00 00 92 92 92 92 00 00 10 0C 10 10 10 10 10 10 10 10 10 10 10 10 10 10 10 10 10 10 12 00 00 20 00 00 0C 0C 0C 0C 0C 0C 0C 0C 0C 0C 0C 0C 0C 0C 0C 0C 0C 0C 0C 0C 0C 0C 0C 0C 0C 0C 0C 00 20 20 20 20 0C 00 92 00 90 10 00 00 92 10 00 92 10 00 00 00 00 10 10 10 10 10 10 10 00 10 10 10 10 10 10 10 3F

Status: 80 82 A4 01 01 D4 F4 48 16 86 16 94 08 13 09 BB 0A 53 09 EF 08 59 09 68 09 CB 09 C5 00 A9 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 32 32 00 00 00 01 00 00 00 07 D9 07 08 00 01 FF 00 07 E1 0A 7B 07 E1 0A 7B 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 01 F4 1F 00 72 67 5B 97 2D 00 00 00 00 00 00 00 00 00 00 00 00 01 00 00 00 00 00 00 00 00 00 00 00 01 00 00 00 00 00 00 00 06 00 00 04 2D 00 00 00 C7 00 00 00 56 00 01 27 10 00 FF F1 00 00 32 00 F5 10 00 96 00 03 58 00 00 00 00 00 FF 00 00 00 00 00 00 00 00 00 00 02 6B 00 00 01 00 00 00 00 00 00 00 00 00 00 00 00 00 00 64

Not sure if this helps. It's the output from the Python tool using the above. (-30.0 is apparently the 'right' value upon checking). The labels are displayed in the right order :

Data type Label Value
0x92 Buitentemp (°C) (t_out) -30.0
0x92 Boilertemp Onder (°C) (t_boildwn) 12.63
0x92 Boilertemp Boven (°C) (t_boiltop) 14.25
0x92 Verdamper Temp (°C) (t_evap) 13.52
0x92 Zuiggas Temp (°C) (t_suct) 19.88
0x92 Persgas Temp (°C) (t_disc) 24.91
0x92 Vloeistof Temp (°C) (t_cond) 22.0
0x92 Temp Naar Bron (°C) (t_source_r) 13.51
0x92 Temp Uit Bron (°C) (t_source_s) 19.02
0x92 Cv Aanvoertemp (°C) (t_ch_supp) 22.69
0x92 Cv Retourtemp (°C) (t_ch_ret) 21.54
0x92 Cv-Druk (Bar) (p_sens) 1.71
0x92 Stroom Compressor (A) (i_tr1) 0.0
0x92 Stroom E-Element (A) (i_tr2) 0.0
0xc Drukschakelaar (in_p_switch) 0
0xc Tarief (in_hl) 0
0xc Condens Beveiliging (in_cond) 0
0xc Spare Ingang (sparein) 0
0x0 Reserve (reserve) 0
0x10 Flowsensor (lt/hr) (in_flow) 0
0xc Fase Detectie (in_phase) 0
0x0 Cv Pomp (%) (out_ch) 0
0x0 Bron Pomp (%) (out_src) 0
0x0 Boiler Pomp (%) (out_dhw) 0
0x0 Vrijkoelklep (%) (out_fc_valve) 50
0x0 Cv/Dhw Of Koeltemp Klep (%) (out_ch_valve) 50
0xc Compressor (out_c1) 0
0xc Element (out_ele) 0
0xc Trickle Heating (out_trickle) 0
0xc Fout (out_fault) 1
0xc Vrijkoelen (out_fc) 0
0x10 Expansieventiel (pls) (out_ev) 0
0x92 Kamertemp (°C) (ot_room) 21.02
0x92 Gevr Kamertemp (°C sp) (ot_setp) 18.0
0x0 Warmtevraag Thermost. (%) (ot_mod) 0
0x0 Status (state) 1
0x0 Sub_Status (sub_state) 255
0x0 Blokkade (ot_vrij) 0
0x92 Berekende Verdampings Temp Cv (°C) (c_sh_evap) 13.02
0x92 Berekende Condensatie Temp Cv (°C) (c_sh_con) 23.15
0x92 Berekende Verdampings Temp Dhw (°C) (c_sh_evap_dhw) 13.02
0x92 Berekende Condensatie Temp Dhw (°C) (c_sh_con_dhw) 23.15
0x0 Fout_Gevonden (fault_reported) 0
0x0 Fout_Retry (fault_retrying) 0
0x10 Taak Actief (task_active) 0
0xc Handbediening (c_hand) 0
0x10 Compr Blok (sec) (tr_compr) 0
0x10 Elek Blok (sec) (tr_eblock) 0
0x10 Cv Voor / Nadraai (sec) (tr_cv) 0
0x10 Bron Voor / Nadraai (sec) (tr_src) 0
0x10 Boiler Voor / Nadraai (sec) (tr_dhw) 0
0x10 Delay Elektr (sec) (tr_elek) 0
0x10 Min Draaitijd Compr (sec) (tr_mincompr) 0
0x10 Afpomp Tijd (sec) (tr_press) 0
0x10 Ev Drukvereffenen (sec) (tr_ev_bal) 0
0x10 Ev Verstellen (sec) (tr_ev_verstel) 0
0x10 Vrijkoel Interval (sec) (tr_fc) 0
0x10 Handbediening (sec) (tr_hand) 0
0x10 Lage Druk Timer (sec) (tr_ld) 0
0x10 Compressor Start (sec) (tr_strtcompr) 0
0x10 Compr Power On Delay (sec) (compr_powondelay) 0
0x10 Vertraging Cv Start (sec) (delaybefstartch) 2154
0x10 Vertraging Cv Stop (sec) (delaybefstopch) 0
0x10 Adaptief Timer (sec) (tr_adaptief) 0
0x12 Adaptief Oververhitting (K) (t_adoverheating) 258.44
0x0 Adaptief Fifo Index (adfifoindex) 29
0x0 Fout Hoogste Prioriteit (hipriorfault) 0
0x20 Utc Tijd (utc_time) 1519732976
0x0 Max Cv Retour Temp (°C) (maxcvtemp) 45
0x0 Bronpomp Snelheid Vrijkoel Mode (bronpompspeed) 0
0xc Compressor In Cv Mode (comprch) 0
0xc Compressor In Boiler Mode (comprdhw) 0
0xc Element In Cv Mode (elemch) 0
0xc Element In Boiler Mode (elemdhw) 0
0xc Cv Mode Geblokkeerd (chmodeblocked) 0
0xc Boiler Mode Geblokkeerd (dhwmodeblocked) 0
0xc Vrijkoel Mode Geblokkeerd (fcmodeblocked) 0
0xc Ontlucht Mode Geblokkeerd (ventmodeblocked) 0
0xc Elektr Element Geblokkeerd (elblocked) 0
0xc Elektr Element Dhw Geblokkeerd (eldhwblocked) 0
0xc Compressor Geblokkeerd (comprblocked) 0
0xc Off Mode Actief (offmode) 1
0xc Cv Mode Actief (chmode) 0
0xc Boiler Mode Actief (dhwmode) 0
0xc Vrijkoel Mode Actief (fcmode) 0
0xc Cv Pomp Voordraaien (chpmpprerun) 0
0xc Bron Pomp Voordraaien (srcpmpprerun) 0
0xc Element Vrijgegeven (elreluser) 0
0xc Vrijgave Extra Koelen (fclowrel) 0
0xc Cv Vrijgegeven (chrel) 0
0xc Vrijkoel Vrijgegeven (fcrel) 0
0xc Tarief Laag Vanuit Thermostaat (lttherm) 0
0xc Ontluchten Vanuit Thermostaat (venttherm) 0
0xc Eco Geselecteerd Op Thermostaat (ecotherm) 1
0xc Comfort Geselecteerd Op Thermostaat (comftherm) 0
0xc Boiler Blokkade Vanuit Thermostaat (dhwthermblocked) 0
0xc Boiler Boost Vanuit Thermostaat (dhwthermboost) 0
0x0 Warmtevraag Totaal (%) (maxrmod) 0
0x20 E-Verbuik Tijdens Stand-By (kWh) (kwhstandby) 5
0x20 E-Verbuik Tijdens Verwarmen (kWh) (kwhch) 1069
0x20 E-Verbuik Tijdens Dhw (kWh) (kwhdhw) 197
0x20 E-Verbuik Tijdens Koelen (kWh) (kwhcooling) 86
0xc Voorverwarmen Tapwater (dlpreheatdhwmodeactive) 0
0x0 Element Geblokkeerd Tijdens Retry (elementalleenvoordhw) 1
0x92 Pi Fout Bronflow (pibron) 100.0
0x0 Actuele Bronklepstand (%) (kleppos) 0
0x90 Utc Tijd Offset (min.) (utcoffset) 1635
0x10 Vrijkoel Blokkade Tijd (min.) (tr_fc_block) 0
0x0 Koeltemp Regelklep Setpoint (%) (fctcontrolperc) 50
0x0 Co-Klep Positie (%) (airregvalvepos) 0
0x92 Hru Uitblaas Temp (°C) (hruexhtemp) -28.0
0x10 Hru Uitblaas Flow (m3/h) (hruexhflow) 150
0x0 Regeneratie Actief (airregactive) 0
0x92 Laatst Geldige Brontoevoer Temp. (°C) (lstvalidsrcsuptmp) 6.85
0x10 Bronpomp Flow Setpoint (l/h) (srcpumpflowsetp) 0
0x0 Bronpomp Snelheid Bij Compr (%) (srcpumpspeedcompressor) 0
0x0 Bronpomp Snelheid Bij Vrijkoel (%) (srcpumpspeedfccontrol) 0
0x0 Bronpomp Snelheid Bij Luchtreg (%) (srcpumpspeedaircontrol) 0
0x0 Vrijkoelmodus (actfcsubstate) 255
0x10 Tijd Co-Klep Start Positie (sec) (co_valve_position) 0
0x10 Minimale Vrijgave Tijd Extern Koelen (sec.) (minruntimeexterncooltimer) 0
0x10 Blokkeertijd Vrijgave Extern Koelen (sec.) (externcoolingblockingtimer) 0
0x10 Blokkeertijd Vrijgave Extern Verwarmen (sec.) (externheatingblockingtimer) 0
0x10 Blokkeertijd Trickle Low Na Power-Up (sec.) (tmtrickleheatinglowblocking) 0
0x10 Blokkeertijd Trickle Low Na Cv-Bedrijf (sec.) (tmchmodeblocking) 1296
0x10 Blokkeertijd Voorverwarmen Tapwater (sec.) (tmpreheatdhwblock) 0
0x0 Utc Tijd : (utctimevalid) 1
0x10 Slow Start Bronpomp (sec.) (srcpumprampuptimer) 0
0x10 Bron Retourtemperatuur Te Laag (sec.) (sourcereturntemptoolow) 0
0x10 Regelperiode Bronflow (sec.) (srcpumpreturncontroltimer) 0
0x10 Stabilisatie Wachttijd Vrijkoelen (sec.) (freecoolingstabilizetimer) 0
0x10 Minimale Tijd Voorverwarmen Tapwater (sec) (preheatdhwminontimer) 0
0x10 Maximale Tijd Voorverwarmen Tapwater (sec.) (preheatdhwmaxontimer) 0
0x10 Vrijgave Elektrisch Element (min) (remainingtimeeelementreleased) 0
arjenhiemstra commented 3 years ago

Thanks for the info! I cannot explain why the first values are not parsed correctly but it seems it must be a bug somewhere. I'll draft some tests and dig into it somewhere this weekend probably.

jasperslits commented 3 years ago

Thanks for the effort so far and happy to help with testing.

arjenhiemstra commented 3 years ago

https://github.com/arjenhiemstra/ithowifi/blob/7ab30355dc46e3f4964ec645f011f5b85d0a0577/software/NRG_itho_wifi/IthoSystemConsts.h#L119

This explains the status mapping issue, there are less than 37 versions mapped (I see only 28) This means that version 34 will be out of bound.

jasperslits commented 3 years ago

That's because there are fewer than 37 versions in the Itho database:

sqlite> select count(*) from versiebeheer; 27 sqlite> select count(distinct(datalabel)) from versiebeheer; 20

So if you are at version 34, you still need the array to have 35 (nullptr + 34) elements?

arjenhiemstra commented 3 years ago

Ah, that explains! The array index = itho version So version 34 would translate to the pointer at *ithoWPUSstatusMap[34].

So the array must be something like this: const uint8_t *ithoWPUSstatusMap[] = { nullptr,nullptr,itho_WPUstatus2,nullptr,itho_WPUstatus4, itho_WPUstatus5, itho_WPUstatus6, itho_WPUstatus7, itho_WPUstatus8_9, itho_WPUstatus8_9, itho_WPUstatus10, itho_WPUstatus11_17, itho_WPUstatus11_17, itho_WPUstatus11_17, itho_WPUstatus11_17, itho_WPUstatus11_17, itho_WPUstatus11_17, itho_WPUstatus11_17, itho_WPUstatus18_19, itho_WPUstatus18_19, itho_WPUstatus20, itho_WPUstatus21, itho_WPUstatus22_25, itho_WPUstatus22_25, itho_WPUstatus22_25, itho_WPUstatus22_25,nullptr, itho_WPUstatus27,nullptr,nullptr,nullptr, itho_WPUstatus31,nullptr, itho_WPUstatus33, itho_WPUstatus34_37, itho_WPUstatus34_37, itho_WPUstatus34_37, itho_WPUstatus34_37};

jasperslits commented 3 years ago

Ah, that explains! The array index = itho

does it make sense to get a new bin file with the amended array or do you want to do debugging first?

arjenhiemstra commented 3 years ago

It probably does. In the current bin array position 34 does not exist but the code will happily return a memory address, which apparently (at least partly points) to readable strings but probably also points to junk. It is due to the resilience of the ArduinoJSON lib that this doesn't crash the program but it could definitely be the cause of the 0x92 glitches as well.

arjenhiemstra commented 3 years ago

Digging through my own code, now I'm pretty sure it could be the source of the issue. Here the status label gets added to the vector: https://github.com/arjenhiemstra/ithowifi/blob/7ab30355dc46e3f4964ec645f011f5b85d0a0577/software/NRG_itho_wifi/IthoSystem.cpp#L536

What happens if junk gets fed? Probably exactly what we see... I thought I'd built in protection for versions > supported (because itho always can start selling a newer version of their device) but clearly not.

jasperslits commented 3 years ago

Thanks for digging into this. And good to keep in mind when adding AutoTemp and WPU settings support. 😉

arjenhiemstra commented 3 years ago

NRG_itho_wifi.ino.esp32.bin.zip Ok, home again, quick copy paste of the array and a blind compile. Maybe you could test this and see if there are differences

jasperslits commented 3 years ago

Looks much better and values / descriptions seem to match upon first glance, will do a better comparison later on.

One observation: outdoor temp is apparently -30 but shown here as 625.36 Well both are logically wrong, the -30 is also what the Python tool shows so that's I expect here as well. Its data type is 0x92

arjenhiemstra commented 3 years ago

Great! Thanks for testing!

F448 is actually an error code: https://github.com/arjenhiemstra/ithowifi/blob/7ab30355dc46e3f4964ec645f011f5b85d0a0577/software/NRG_itho_wifi/IthoSystemConsts.h#L1184

But nonetheless, it is handled als unsigned float incorrectly

jasperslits commented 3 years ago

Everything else looks good OK now. Havent seen anything that looks completely off.

Some suggestions for next versions: 1) Add unit type (I can read it from Itho dbase and add it to the label) as otherwise it's difficult to interpret the values 2) Selection of fields for MQTT? I guess out of all status values available, not everything is interesting for a HA system. MQTT supports the large message size all together and filtering can be done on receiving end, but sending a select number of fields feels cleaner.

arjenhiemstra commented 3 years ago

Thanks, I was trying to load the data via MQTT in Home Assistant but it seems there's another fix needed?

This is not relevant anymore?

arjenhiemstra commented 3 years ago

Everything else looks good OK now. Havent seen anything that looks completely off.

Some suggestions for next versions:

  1. Add unit type (I can read it from Itho dbase and add it to the label) as otherwise it's difficult to interpret the values

Yes that would be great, the reason I did not include this (yet) is that it creates another layer of values and complexity. We then would have to register for every value the unit (or error code). In an DB this is easy, but the structure I chose (because of missing DB solution for arduino) is somewhat limited on this extend. Trade off between functionality and memory usage. But probably a lot more is possible but I lack the experience to know and use a different approach to be honest.

  1. Selection of fields for MQTT? I guess out of all status values available, not everything is interesting for a HA system. MQTT supports the large message size all together and filtering can be done on receiving end, but sending a select number of fields feels cleaner.

Yes that is sure possible and I thought about this as well. This would put the burden on a relatively small and limited system, whereas the receiving system could quite easily parse the received info. For now I (hopefully) made the buffer large enough to accommodate all the values and there is still headroom to extend on the ESP32 platform if needed.

jasperslits commented 3 years ago

Thanks, I was trying to load the data via MQTT in Home Assistant but it seems there's another fix needed?

This is not relevant anymore?

No, It was my fault. I didn’t turn on mqtt with the new firmware so initially looked at the old mqtt from the previous version. Rookie mistake

jasperslits commented 3 years ago

Everything else looks good OK now. Havent seen anything that looks completely off. Some suggestions for next versions:

  1. Add unit type (I can read it from Itho dbase and add it to the label) as otherwise it's difficult to interpret the values

Yes that would be great, the reason I did not include this (yet) is that it creates another layer of values and complexity.

Clear. So shall I just send a patch which adds the unit types to the labels? E.g. description (unit type).

I looked at the code to understand the parameter / setting processing. But while in the Itho database I see the min and max values, it looks like you read it from the device output itself? not much of a c++ coder myself, just seeing what needs to be provided to get the settings to show up.

arjenhiemstra commented 3 years ago

Clear. So shall I just send a patch which adds the unit types to the labels? E.g. description (unit type).

Ah, I misunderstood it. Adding it to the label itself is off-course easy and can be done without adding complexity.

jasperslits commented 3 years ago

My original idea was to have unit type separated from the description but after reading your comments on complexity, updating the description seems the easier way to go.

jasperslits commented 3 years ago

I've created #51 PR for the updated labels.