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

MQTT data for thermostat_data_hc2 corrupted #354

Closed
majdaf opened this issue Feb 7, 2022 · 7 comments
Closed

MQTT data for thermostat_data_hc2 corrupted #354

majdaf opened this issue Feb 7, 2022 · 7 comments
Labels
bug Something isn't working
Milestone

Comments

@majdaf
Copy link

majdaf commented Feb 7, 2022

Bug description

  • EMS-ESP Version: v3.4.0b3
  • Device (Platform / SDK): ESP32 / v3.3.5-1-g85c43024c

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:

  • QoS: 0
  • Set Clean Session: true
  • Always use retain flag: false
  • Topic/Payload format: As individual topics
  • Publish command output to a 'response' topic: false
  • Publish single value topics on change: false
  • Enable MQTT Discovery (for Home Assistant, Domoticz): true
  • Prefix for the Discovery topic: homeassistant
@majdaf majdaf added the bug Something isn't working label Feb 7, 2022
@proddy
Copy link
Contributor

proddy commented Feb 7, 2022

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
Copy link
Contributor

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
Copy link
Contributor

proddy commented Feb 12, 2022

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
Copy link
Contributor

proddy commented Feb 12, 2022

@MichaelDvP
Copy link
Contributor

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
Copy link
Contributor

proddy commented Feb 12, 2022

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.

proddy added a commit that referenced this issue Feb 12, 2022
@MichaelDvP
Copy link
Contributor

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

3 participants