emsesp / EMS-ESP32

ESP32 firmware to read and control EMS and Heatronic compatible equipment such as boilers, thermostats, solar modules, and heat pumps
https://emsesp.github.io/docs
GNU Lesser General Public License v3.0
622 stars 107 forks source link

MQTT data for thermostat_data_hc2 corrupted #354

Closed majdaf closed 2 years ago

majdaf commented 2 years ago

Bug description

Thermostat data from RC310 are read correctly and are desiplayed on dashboard in UI. E.g. "hc2 selected room temperature" reads correct 20,0 C. The data gets passed to MQTT: 2022-02-07 20:01:36.504 D 108: [mqtt] Publishing topic ems-esp/thermostat_data_hc2 (#65272, retain=0, retry=1, size=395, pid=1)

However the data for HC2 passed to MQTT seem to be corrupted: {"auto":20,"fort":20.2,"r":"auto","mized":"comfort","ecotemp":17,"manualtemp":21,"comforttemp":20,"summertemp":15,"designtemp":40,"offsettemp":0,"minflowtemp":25,"maxflowtemp":40,"roominfluence":10,"curroominfl":-0.8,"nofrosttemp":5,"targetflowtemp":29,"heatingtype":"floor","summersetmode":"auto","summermode":"off","controlmode":"optimized","program":"prog_2","tempautotemp":-1,"fastheatup":0}

This does not happend to HC 1 data coming from the same thermostat: {"seltemp":20,"currtemp":20.2,"mode":"auto","modetype":"eco","ecotemp":19,"manualtemp":21,"comforttemp":22,"summertemp":15,"designtemp":65,"offsettemp":0,"minflowtemp":25,"maxflowtemp":65,"roominfluence":0,"curroominfl":0,"nofrosttemp":5,"targetflowtemp":46,"heatingtype":"convector","summersetmode":"auto","summermode":"off","controlmode":"optimized","program":"prog_1","tempautotemp":-1,"fastheatup":0}

Steps to reproduce Connect EMS-ESP to EMS bus Enable MQTT with HA auto discovery

Expected behavior E.g. the seltemp are obvioulsy expected by the HA sensor/number defition: {"~":"ems-esp","uniq_id":"thermostat_hc2_seltemp","command_topic":"~/thermostat/hc2/seltemp","min":5,"max":29,"step":0.5,"ic":"mdi:coolant-temperature","stat_t":"~/thermostat_data_hc2","name":"Thermostat hc2 selected room temperature","val_tpl":"{{value_json.seltemp}}","unit_of_meas":"°C","dev":{"ids":["ems-esp-thermostat"]}}

Context MQTT setting:

proddy commented 2 years ago

Thanks for reporting. I've reproduced this. Seems to only happen when the MQTT is not nested (default is nested and preferred). Working on a fix...

MichaelDvP commented 2 years ago

Seems there is a change in latest arduinojson. It works if not only the document, but also the jsonObject is cleared before reuse. In EMSESP::publish_device_values( add to all doc.clear(); a json.clear();

proddy commented 2 years ago

thanks Michael, I was hunting down the root cause in the arduinojson library so I can report back an issue. It looks like it happens due to the flashstrings in parsing the enums.

proddy commented 2 years ago

https://github.com/bblanchon/ArduinoJson/issues/1712

MichaelDvP commented 2 years ago

That's good, but as far as i understood the document stores the data and the object a reference to the keys. Clearing the object too do no harm and is maybe better because creating a key in cleared structure dont need to search for existing keys if the structure is not cleared.

proddy commented 2 years ago

I wanted to avoid using JsonObject.clear() because of memory leaks https://arduinojson.org/v6/api/jsonobject/clear/

I solved it now by not using clear() and re-assigning the JsonObject each time since doc.to<JsonObject>() also implicitly clears the pool and this way we don't lose the reference.

MichaelDvP commented 2 years ago

The memory leak is only if you clear the object without clearing the document. But recreating the object is a clean solution.