arjenhiemstra / ithowifi

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

[Feature Request] Support for WPU Version 41 #238

Open RobBie1221 opened 4 months ago

RobBie1221 commented 4 months ago

First of all, kudos for the very nice prints and code, I use them on my WPU as well as all my AutoTemp units!

Today, they exchanged my heatpump. The print used to work perfectly, but they bumped me to hw version 83, fw version 41. It seems from wpu.h that up to fw version 37 is currently supported.

Obviously, I'd like to get support in for fw version 41. I can add some effort, but need some guidance. It seems I need to extend wpu.h? Could I test this by locally running build_script.py from Python to generate a binary? How would I be able to determine if they added / changed some settings / status registers? Do I need to enable sniffing?

arjenhiemstra commented 4 months ago

Thanks for you this issue. I see that there is indeed a new version in the servicetool.

How would I be able to determine if they added / changed some settings / status registers? Do I need to enable sniffing?

Unfortunately there is no easy way to get these labels out of the tool, for starters because all labels are in Dutch... I'll work on it; maybe @tomkooij has an idea how to create this file in an easier way?

RobBie1221 commented 4 months ago

Dutch is not an issue ;-) Do you need an account for the service tool to be able to see the labels?

Theoretically, if I would build a new version for myself to test where I copy the status registers from version 37, could that work? Then at least I can have monitoring again.

arjenhiemstra commented 4 months ago

Ok, the status label are easy, not a lot of changes, commit https://github.com/arjenhiemstra/ithowifi/commit/aa65d7bf8259f7d7a1e6d1f7f024ba8f8bdd4f69

settings labels need a bit more work

arjenhiemstra commented 4 months ago

Dutch is not an issue ;-) Do you need an account for the service tool to be able to see the labels?

Theoretically, if I would build a new version for myself to test where I copy the status registers from version 37, could that work? Then at least I can have monitoring again.

if you pull this repo and build it you should have status labels the service tool is free and downloadable from the itho website

arjenhiemstra commented 4 months ago

I theory the settings should be an easy fix as well. There is one change on an existing index (228) and there are 10 additions. But unfortunately I have some issues with the script to generate the .h header files, I' getting a totally different result than currently is in de firmware...

RobBie1221 commented 4 months ago

Thanks for the fast response!

Took me some time to build, commit https://github.com/arjenhiemstra/ithowifi/commit/aa65d7bf8259f7d7a1e6d1f7f024ba8f8bdd4f69 doesn't actually build with me (gives 2 errors regarding type conversion).

I was able to build the change of this commit on top of 2.8.0. I now see status data again!

arjenhiemstra commented 4 months ago

Thanks for the fast response!

Took me some time to build, commit aa65d7b doesn't actually build with me (gives 2 errors regarding type conversion).

I was able to build the change of this commit on top of 2.8.0. I now see status data again!

Hmm, your correct. I accidentally checked in two files extra... trying to do too many things at once...

I'll fix that

here is a reference to the script in use to generate the files https://github.com/arjenhiemstra/ithowifi/issues/49#issuecomment-971422213

But somehow its not working anymore, need to check this one into version control as well cleary

RobBie1221 commented 4 months ago

Thanks for the hint towards the script. I just manually opened the par file from the tool, still trying to understand how it's all built together.

For now at least my monitoring is running again. I'll see if I can get the script working to also get parameters in for version 41.

arjenhiemstra commented 4 months ago

Here is some useful info about the par files and how to download them: https://github.com/pommi/python-itho-wpu

and here is the "latest" version of the script. I'm far from an PHP expert and as said, should have better kept track of things using vc. If you can help out, that would be great!

<?php

$lang    = 'NL';
//$lang  = 'GB';
//$lang  = 'other';
$product = 'WPU'; //(cve14, cve1B, demandflow, autotemp, hrueco, hru200, hru250-300, hru350, WPU)
$dbname  = 'wpu.db';
$pwlevel = 500;

//use versionlist instead of DB query
//cve14
//$versions = [1,2,3,4,5,6];
//cve1B
//$versions = [6,7,8,9,10,11,17,18,20,21,22,24,25,26,27];
//demandflow
//$versions = [1,3,4,6,7,8,9,10,11,12,13,14,15,16,17,18,19,20,21];
//autotemp
//$versions = [1,2,3,4,5,6,7,8,9,10,12,13];
//hrueco
//$versions = [3,4,6,7,8,10,11,12];
//hru200
//$versions = [12,13,14,15,17,18,19,20,21,25,26,27];
//hru250-300, $lang  = 'GB';
//$product = 'HRU250_300';
//$versions = [1,2,3,4,6,7,8,9,10];
//hru350
//$versions = [1,3,4];
//wpu & set $verlist to TRUE
$versions = [];
$verlist = TRUE;

$db = new SQLite3($dbname);

$dl = [];

if ($verlist == TRUE) {
    $q = 'select DataLabel from VersieBeheer';
    $res = $db->query($q);

    //  All datalabel numbers in an array

    while ($row = $res->fetchArray()) {
        $dl[$row['DataLabel']] = [];
    }
}
else {

    foreach ($versions as $value) {
        $dl[$value] = [];
    }

}

if($lang == 'NL') {
    $col_unit = 'Eenheid_NL';
    $col_title = 'Tekst_NL';
    $col_name = 'Naam';
}
else if($lang == 'GB') {
    $col_unit = 'Eenheid_GB';
    $col_title = 'Tooltip_GB';
    $col_name = 'Naam';
}
else {
    $col_unit = 'unit';
    $col_title = 'title';
    $col_name = 'name';
}

// Collect names per datalabel and the flash label text
foreach($dl as $k=>$v) {
    $q = "select * from Datalabel_V$k ORDER BY `Index` ASC";

    $res = $db->query($q);
    while($row = $res->fetchArray()) {

//      if (empty($row[$col_title])) {
//              $col_title_save = $col_title;
//              $col_title = 'Naam';
//      }   
    echo print_r($row);
        $fullname = $row[$col_title];
        if (!empty($row[$col_unit])) {
            $row[$col_unit] = preg_replace('/[\s]+/','',$row[$col_unit]);
            //Strip off spaces and non-alpha-numeric

            $fullname .= " (".$row[$col_unit].")";
        }
        if (!empty($row[$col_unit])) {

            $unit = "";
            if (strcmp($row[$col_unit], "%") == 0) $unit = 'perc';
            else if (strcmp($row[$col_unit], "% ") == 0) $unit = 'perc';
            else if (strcmp($row[$col_unit], "&deg;c") == 0) $unit = 'degc';
            else {
                $unit = $row[$col_unit];
            }
            $unit = preg_replace('/[\W]+/','',$unit);

            $row[$col_title] .= "_".$unit;
            $row[$col_title] = preg_replace('/[[:space:]]+/', '-', $row[$col_title] ?? '');
        }
        //$dl[$k][$row[$col_name]] = strtolower($row[$col_title]);
        $dl[$k][$row[$col_name]] = array(ucfirst($fullname ?? ''), strtolower($row[$col_title] ?? ''));
//      $col_title = $col_title_save;
        //echo print_r($dl[$k]);
        echo "\n";

    }

}

// Build array of labels with their associated datalabel number
$labels = [];
foreach($dl as $k=>$v) {
    foreach($v as $label=>$id) {
        if (! isset($labels[$label])) {
            $labels[$label] = [];
        }
        $labels[$label][$k] = $id;
    }
    //p rint_r($labels);
    echo "\n";
}
$nr = 0;
$f = [];

// Store label sequences
$labelsequence = [];
foreach($labels as $label => $version) {
    $keys = array_keys($version);
    $labelsequence[$label][$nr] = $keys;
    //  Flash message position

    $f[$nr] = $version[$keys[array_key_last($keys)]];

    //p rint array_key_last($keys)."\n";
    //p rint_r($keys)."\n";
    $nr++;
}

print("\n");

// Display the relevant labels numbers per version
foreach($dl as $labelnr=>$nameArr)
{
    $fieldseq = [];
    foreach($nameArr as $name=>$id) {
        $n = $labelsequence[$name];
        $fieldseq[] = array_keys($n)[0];
    }
    printf("const uint8_t itho_%sstatus%d[] { %s,255};\n",$product,$labelnr,join(",",$fieldseq));
}

print("\n");

print("const struct ithoLabels itho{$product}StatusLabels[] {\n");
//print("const __FlashStringHelper *itho{$product}StatusLabels[] =  {\n");
// Flash messages
foreach($f as $pos => $flashlabel) {
    $flashlabel[1] = preg_replace('/[[:space:]]+/', '-', $flashlabel[1] ?? '');
    $flashlabel[1] = preg_replace('/^[\-]+/','',$flashlabel[1] ?? '');
    //  Strip off the starting hyphens

    $flashlabel[1] = preg_replace('/[\-]+$/','',$flashlabel[1] ?? '');
    //  //  Strip off the ending hyphens

    //$ flashlabel = preg_replace('/[\s\W]+/','-',$flashlabel);
    //  Strip off spaces and non-alpha-numeric

    printf("    { \"%s\",   \"%s\" },\n",$flashlabel[0],$flashlabel[1]);
}
print("};\n");

$dl = [];

if ($verlist == TRUE) {
    $q = 'select ParameterLijst from VersieBeheer';
    $res = $db->query($q);

    //  All datalabel numbers in an array

    while ($row = $res->fetchArray()) {
        $dl[$row['ParameterLijst']] = [];
    }
}
else {

    foreach ($versions as $value) {
        $dl[$value] = [];
    }

}

if($lang == 'NL') {
    $col_unit = 'Eenheid_NL';
    $col_title = 'Tekst_NL';
    $col_name = 'Naam';
    if($product == 'DemandFlow') {
        $col_title = 'Naam_fabriek';
    }
    if($product == 'WPU') {
        $col_name = 'Tekst_NL';
    }
}
else if($lang == 'GB') {
    $col_unit = 'Eenheid_GB';
    $col_title = 'Tekst_GB';
    $col_name = 'Naam';
    if($product == 'WPU') {
        $col_name = 'Tekst_NL';
    }   
}
else {
    $col_unit = 'unit';
    $col_title = 'title';
    $col_name = 'name';
}

// Collect names per settingslabel and the flash label text
foreach($dl as $k=>$v) {

    $q = "select * from Parameterlijst_V$k where Paswoordnivo<$pwlevel ORDER BY `Index` ASC";

    $res = $db->query($q);
    while($row = $res->fetchArray()) {
        if (empty($row[$col_title])) {

            if($product == 'AutoTemp') {
                $col_title = 'Naam';
            }
            if($product == 'WPU') {
                $col_title = 'Naam_fabriek';
            }           

        }
        if(strcmp($row[$col_title] ?? '', "Minimum ventilation speed during Auto Night mode") == 0 || strcmp($row[$col_title] ?? '', "Minimum ventilation speed during Auto mode") == 0) {
            $col_title = 'Naam';
        }
        if (!empty($row[$col_unit])) {
            $row[$col_unit] = preg_replace('/[\s]+/','',$row[$col_unit]);
            //Strip off spaces and non-alpha-numeric

            $row[$col_title] .= " (".$row[$col_unit].")";

        }

        $dl[$k][$row[$col_name]] = ucfirst($row[$col_title] ?? '');
        if($product == 'AutoTemp') { //check if correct
            $col_title = 'Tekst_GB';
        }
        if($product == 'WPU') { //check if correct
            $col_title = 'Tekst_GB';
        }       
    }

}

// Build array of labels with their associated datalabel number
$labels = [];
foreach($dl as $k=>$v) {
    foreach($v as $label=>$id) {
        if (! isset($labels[$label])) {
            $labels[$label] = [];
        }
        $labels[$label][$k] = $id;

    }

}

$nr = 0;
$f = [];

// Store label sequences
$labelsequence = [];
foreach($labels as $label => $version) {
    $keys = array_keys($version);
    $labelsequence[$label][$nr] = $keys;
    //  Flash message position

    $f[$nr] = $version[$keys[array_key_last($keys)]];
    $nr++;
}

print("\n");

// Display the relevant labels numbers per version
foreach($dl as $labelnr=>$nameArr)
{
    $fieldseq = [];
    foreach($nameArr as $name=>$id) {
        $n = $labelsequence[$name];
        $fieldseq[] = array_keys($n)[0];
    }
    printf("const uint16_t itho_%ssetting%d[] { %s,999};\n",$product,$labelnr,join(",",$fieldseq));
}

print("\n");

print("const char* itho{$product}SettingsLabels[] =  {\n");
// Flash messages
foreach($f as $pos => $flashlabel) {

    printf("    \"%s\",\n",$flashlabel);
}
print("};\n");
arjenhiemstra commented 4 months ago

Looking at my own earlier reply in the mentioned issue:

Hmm, in the DB apparently not all data info is sorted the same way...

It probably has something to do with the data alignment (rows in the DB do not appear to be stored in index order), the script does not correct for that

tomkooij commented 4 months ago

I have a tool that needs cleaning up that does just this...

Shall I dig into this and add the setting labels for v41, or is this already fixed by @RobBie1221 ?

RobBie1221 commented 4 months ago

I didn't fix anything yet, I'm still in a learning curve ;)

If you can do this that would be perfect.

tomkooij commented 4 months ago

Unfortunately my (spaghetti) code just reads all the StatusLabels and exports a list. However, the order does not seem to match wpu.h. So it has the same issue that is described by @arjenhiemstra above :-(

I guess last time I just manually edited/added some stuff. (Just like I manually editted the double 'utc-time' key).

I'll try to write the records to sqllite and try with the PHP tool above.

RobBie1221 commented 4 months ago

If I look in the database, parameterlijst_v41 is inserted in the database between v4 and v5, which probably means scripts parse v41 before v5, v6 etc. This is I think also what @arjenhiemstra means by correction for ordering?

If this is the case, I wonder how parsing has been done with scripts before, because e.g. v37 is also inserted before v4...

I can make a script which parses from v1 to v41 (first to last), but that would also maybe change wpu.h quite a bit?

RobBie1221 commented 4 months ago

I'm debuggingconvert-itho-db.py script, but something goes seriously wrong with this script. From Parameterlijst_v17 onwards, it is not able to parse the Index. Some get a strange number, most get nan

image

tomkooij commented 4 months ago

@RobBie1221 I just read the tables from the Access database (parameter file). Using code copied from convert-itho-db.py.

I have two problems which prevent me from auto-generating much of wpu.h:

The StatusLabels seems to autogen just fine (except from different descriptions, but the order is an exact match)

I'll upload my python notebook to GitHub so you can have a look: https://github.com/tomkooij/itho_parameters/blob/master/parse_parameters.ipynb

(This is a "work in progress" notebook, I have not cleaned it up)

RobBie1221 commented 4 months ago

I also noticed StatusLabels are different in sense of descriptions, this probably has to do with different descriptions. Some StatusLabels have among versions the same Naam but different Tekst_NL and Tekst_GB. It seems to depend on order of parsing which label ends up in the header.

I'm trying to wrap my head around this: https://github.com/pommi/python-itho-wpu/blob/7048014a223431d9024f0c5fca820f5ad146e19c/convert-itho-db.py#L55-L58

That should simply fetch the Index (among other keys) but somehow for Parameterlijst 17 and up this does not result in indices. Can't figure out what is wrong here.

arjenhiemstra commented 4 months ago

If I look in the database, parameterlijst_v41 is inserted in the database between v4 and v5, which probably means scripts parse v41 before v5, v6 etc. This is I think also what @arjenhiemstra means by correction for ordering?

If this is the case, I wonder how parsing has been done with scripts before, because e.g. v37 is also inserted before v4...

I can make a script which parses from v1 to v41 (first to last), but that would also maybe change wpu.h quite a bit?

Also if I order the versions ASC the result is the same, also with old db files unfortunately

arjenhiemstra commented 4 months ago

@RobBie1221 I just read the tables from the Access database (parameter file). Using code copied from convert-itho-db.py.

I have two problems which prevent me from auto-generating much of wpu.h:

The StatusLabels seems to autogen just fine (except from different descriptions, but the order is an exact match)

I'll upload my python notebook to GitHub so you can have a look: https://github.com/tomkooij/itho_parameters/blob/master/parse_parameters.ipynb

(This is a "work in progress" notebook, I have not cleaned it up)

I think I manually translated the labels because no english version was available. Some other devices have english labels and some luckily don't get updated as often :)

arjenhiemstra commented 4 months ago

I also noticed StatusLabels are different in sense of descriptions, this probably has to do with different descriptions. Some StatusLabels have among versions the same Naam but different Tekst_NL and Tekst_GB. It seems to depend on order of parsing which label ends up in the header.

Thats correct, the goal is to store labels with as little memory as possible. User a certain labels (Naam, Tekst_NL etc.) as keys. The keys need to be unique in the sense that they generate enough unique labels for all settings across all firmware versions to be covered. This sometimes means a value can be repeated, keys were different in that case.

Unfortunately, the db files of the service tool are a just as much spaghetti as our code :) It differs between devices what the best to use as keys, that is why there are all kind of exceptions in my adapted script.

What would be great is if we could generate the .h file and with that output check agains the db if all labels line up with device fw version x and index no y etc.

RobBie1221 commented 4 months ago

Just checking here, I'm now manually reading and trying to interpret the v37 parameter list. Via Access I can see the following:

image

Here:

https://github.com/arjenhiemstra/ithowifi/blob/23b57b28fe74c5ad1da1fd70d228a89984759f52/software/NRG_itho_wifi/main/devices/wpu.h#L31

It seems that index 228 refers to position 409 from ithoWPUSettingsLabels.

Position 409 should be:

https://github.com/arjenhiemstra/ithowifi/blob/23b57b28fe74c5ad1da1fd70d228a89984759f52/software/NRG_itho_wifi/main/devices/wpu.h#L433

Right? Or am I seeing this wrong? Seems this should point to 9?

https://github.com/arjenhiemstra/ithowifi/blob/23b57b28fe74c5ad1da1fd70d228a89984759f52/software/NRG_itho_wifi/main/devices/wpu.h#L43

arjenhiemstra commented 4 months ago

Just checking here, I'm now manually reading and trying to interpret the v37 parameter list. Via Access I can see the following:

image

Here:

https://github.com/arjenhiemstra/ithowifi/blob/23b57b28fe74c5ad1da1fd70d228a89984759f52/software/NRG_itho_wifi/main/devices/wpu.h#L31

It seems that index 238 refers to position 409 from ithoWPUSettingsLabels.

Position 409 should be:

https://github.com/arjenhiemstra/ithowifi/blob/23b57b28fe74c5ad1da1fd70d228a89984759f52/software/NRG_itho_wifi/main/devices/wpu.h#L433

Right? Or am I seeing this wrong? Seems this should point to 9?

https://github.com/arjenhiemstra/ithowifi/blob/23b57b28fe74c5ad1da1fd70d228a89984759f52/software/NRG_itho_wifi/main/devices/wpu.h#L43

You should read it as follows: setting in database for version 37 column index value: 228 then itho_WPUsetting37[228] = 409

ithoWPUSettingsLabels[409] = "Log interval (sec)"

which indeed is in the db:

228.0 228.0 P_Data_Interval dataLogInterval 5.0 0x10 300.0 5.0 loginterval tijd tijd tussen 2 metingen

RobBie1221 commented 4 months ago

Ok, thanks. My reasoning was correct, my indexing in ithoWPUSettingsLabel not...

Nevertheless, this overlaps with "9" in the tabel. Somehow it generated a new key in ithoWPUSettingsLabel. For example v2:

image

There is potential to save memory :)

tomkooij commented 4 months ago

Okay, My understanding of this as of now:

My proposed solution: Auto-generate wpu.h from the current parameter files without breaking everything (so keep hand translated statuslabels). I can tidy up my python code to do this. That's a bit of work now, but would save a lot of work when a new firmware appears. It would especially save (me) from trying to figure this out all over again.

The solution above would ofcourse introduce the chance of broken settings (mis-aligned labels) but I can at least check it for version 37 myself and this should give a higher confidence in the correctness.

@arjenhiemstra Looking at your php-script above, I don't think the script can do the above mentioned things, right? (I cannot save some work by trying to figure out PHP?). Would regenerating SettingLabels indices be acceptable?

tomkooij commented 4 months ago

O... I've thought a little bit about this, but the solution above is not a real solution: It would work for wpu.h, but it it should work for all of `devices/' (It's easy enough to add that, but perhaps just fixing the PHP code is easier)

Anyway, I'll finish the python script. Propose a new wpu.h with the duplicates removed and we can work from there. If you (Arjen) decide not to use it and stick with the PHP code, that's fine as well.

arjenhiemstra commented 4 months ago

Okay, My understanding of this as of now:

  • wpu.h was generated from the parameter files but with hand-made translations of the names/labels. These cannot be auto generated and will change when autogenerated. This is not really a problem for settinglabels but it is a concern for statuslabels as these appear as keys in MQTT messages. They cannot change without breaking this for users. (This would be unacceptable)

If the generator script works correctly, new version additions should only result in labels being added to the end of the array. Only these new labels need translation if no english version is available. This worked up until now with the wpu settings. The datalables I checked in have been created in exact that way, there is a bit of manual work involved but this is minimal.

  • I do not have a database with the hand translated stuff

You do have a "database", this is the current wpu.h ;-)

  • Somewhere in conversion the ordering of tables is messed up for settings. Parameterlijst_V31 was inserted/used after 2 and before 4 resulting in many duplicate keys (with different translations/slugs) starting from firmware 31.

Proper sorting of the tables before generating the label should fix this (also for future releases). Because then the order in wich labels are placed in the array with the script are always the same and new additions should end up at the bottom.

My proposed solution: Auto-generate wpu.h from the current parameter files without breaking everything (so keep hand translated statuslabels). I can tidy up my python code to do this. That's a bit of work now, but would save a lot of work when a new firmware appears. It would especially save (me) from trying to figure this out all over again.

  • I can generate the StatusLabel indices one-to-one from the parameters files. I can include a lookup-table for the hand-made translations as they map one-to-one. The hand made translation can thus be auto inserted with high confidence.

Statuslabels are correct and already auto generated, only the translation is manual. No rework needed here I would say.

  • Re-generate the SettingLabels indices and change all labels to be auto generated. This would remove the 150+ duplicates. The indices from v17 change a little bit (one two indices change) and are totally different from v31. Diff: https://www.diffchecker.com/wWbF1Z8F/

The solution above would ofcourse introduce the chance of broken settings (mis-aligned labels) but I can at least check it for version 37 myself and this should give a higher confidence in the correctness.

@arjenhiemstra Looking at your php-script above, I don't think the script can do the above mentioned things, right? (I cannot save some work by trying to figure out PHP?). Would regenerating SettingLabels indices be acceptable?

I think we should indeed use an updated script to autogenerate the settings labels (at first it is fine when this is in Dutch). Then we need to check of the generated settings labels match up with the settings for that version, it seems we can easily check that with your devices for version 37 and 41, some manual and/or automated test can be generated for the other versions. If we have I correct settings map, I can redo the translation once more, after that the process should be repeatable and new additions end up being added to the end and manual work is minimal again.

Other option is to manually add it now for version 41 and hope there will be no other wpu firmware versions but I think that's not the best idea ;-) I dont't have real knowledge of php, @jasperslits was the original author of the script, maybe he has a quick idea how to approach this?

tomkooij commented 4 months ago

Okay, I can do most of this. I’ll submit a PR.

arjenhiemstra commented 4 months ago

Okay, I can do most of this. I’ll submit a PR.

and PHP or Python, either is fine. I think Python might be a bit easier to maintain

jasperslits commented 4 months ago

I agree with Pythong being easier. I created that PHP script for a quick headstart of WPU support, didn't expect it would be considered after 2? years again.

arjenhiemstra commented 4 months ago

I agree with Pythong being easier. I created that PHP script for a quick headstart of WPU support, didn't expect it would be considered after 2? years again.

haha, still in use indeed :)

tomkooij commented 4 months ago

@RobBie1221 #240 seems ready (and tested on fw37 by myself). Can you test?

(I guess you can build it yourself, please do, or use this: https://www.dropbox.com/scl/fi/9hnr8whhgetirn74o51hz/nrgitho-v2.8.0-pr240.bin?rlkey=kq86zy0o9o42zwkolcf0nth85&dl=0)

RobBie1221 commented 4 months ago

I can build myself. I will test it this evening. Thanks for the effort!