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

upgrade EMS-ESP breaks my historical graphs in grafana #116

Closed
bartplessers opened this issue Sep 16, 2021 · 22 comments
Closed

upgrade EMS-ESP breaks my historical graphs in grafana #116

bartplessers opened this issue Sep 16, 2021 · 22 comments
Labels
enhancement New feature or request

Comments

@bartplessers
Copy link

Bug description

In HomeAssistant, I configured InfluxDB and Grafana to get the boiler data and show them on a graph.
I upgraded from EMS-ESP v2.2.1 to v3.2.1
No I notice that some of the measurements are missing

Expected behavior
The entities will have same name after upgrade

Screenshots

  1. before upgrade
  2. after upgrade

2021-09-14_23-43-20

Device information

{
"System": {
"version": "3.2.1",
"uptime": "004+13:27:31.642",
"uptime_sec": 394051,
"freemem": 177
},
"Status": {
"bus": "connected",
"bus protocol": "Buderus",
"telegrams received": 1010860,
"read requests sent": 105115,
"write requests sent": 3,
"incomplete telegrams": 3,
"tx fails": 0,
"rx line quality": 100,
"tx line quality": 100,
"MQTT publishes": 59441,
"MQTT publish fails": 0,
"dallas sensors": 0,
"dallas reads": 0,
"dallas fails": 0
},
"Devices": [
{
"type": "Boiler",
"name": "Buderus Logamax Plus/GB192/Condens GC9000 (DeviceID:0x08 ProductID:208, Version:01.04)",
"handlers": "0x10 0x11 0x14 0x15 0x1C 0x18 0x19 0x1A 0x35 0x16 0x33 0x34 0x26 0x2A 0xD1 0xE3 0xE4 0xE5 0xE6 0xE9 0xEA"
},
{
"type": "Thermostat",
"name": "RC300/RC310/Moduline 3000/1010H/CW400/Sense II (DeviceID:0x10, ProductID:158, Version:74.01)",
"handlers": "0xA3 0x06 0xA2 0x12 0x2A5 0x2B9 0x2AF 0x29B 0x2A6 0x2BA 0x2B0 0x29C 0x2A7 0x2BB 0x2B1 0x29D 0x2A8 0x2BC 0x2B2 0x29E 0x2F5 0x31B 0x31D 0x31E 0x23A 0x267 0x240"
},
{
"type": "Gateway",
"name": "KM200/MB LAN 2 (DeviceID:0x48, ProductID:189, Version:04.07)",
"handlers": ""
},
{
"type": "Controller",
"name": "9000i (DeviceID:0x09, ProductID:224, Version:35.06)",
"handlers": ""
}
]
}

Question:
What caused this behavior?
Can it be avoided in the future?
How to resolve it?

kind regards,
Bart

@MichaelDvP
Copy link
Contributor

V3 has changed some names of values and commands. Now all are lower case. In V2 there was a mixture of lower and mixed case and some commands have different names to the value. Read here.
The actual names in v3 you can lookup here.

@proddy
Copy link
Contributor

proddy commented Sep 18, 2021

I think Bart's issue is the entity names in Home Assistant which are formed from the short names. For example curflowtemp is sensor.boiler_current_flow_temperature in HA. If we change any full names in locale_EN.h it will change the entities in HA and break any custom scripts/triggers people may have and annoy them.

Perhaps I can create a new command and endpoint like /api//ha which lists the mappings, similar to how http://ems-esp/api/boiler/commands works.

@proddy proddy transferred this issue from emsesp/EMS-ESP Sep 19, 2021
@MichaelDvP
Copy link
Contributor

I'm a bit confused, doc["uniq_id"] is from short names, e.g. currflowtemp, which was currFlowTemp in v2., doc["name"] contains device and full name. I thought the entity for scripting is the uniq_id?

@proddy
Copy link
Contributor

proddy commented Sep 19, 2021

sorry, typo. The HA Entity is formed from the full name (dv.full_name) in Mqtt::publish_mqtt_ha_sensor() the line

snprintf_P(new_name, sizeof(new_name), "%s %s", device_name, uuid::read_flash_string(name).c_str());

@MichaelDvP
Copy link
Contributor

Happy that i'm not using HA, the namings are to confusing to me.
Line 941: const __FlashStringHelper * entity, contains the short name
Line 966: // create entity by add the tag if present, seperating with a . creates the uniq_id from shortname.

Why do you call this all entity if the real HA-entitiy is published as new_name? I give up understanding HA.

@proddy
Copy link
Contributor

proddy commented Sep 19, 2021

The way HA handles entities is actually straightforward and very well designed. The problem is the interface between EMS-ESP and HA. I had to use something called MQTT Discovery that builds the entities dynamically from MQTT topics which is really shit and been a headache of mine for a long time. (there is an issue somewhere to replace that all with a native interface using the API).

To get an entity you need to create a config topic like:

homeassistant/sensor/ems-esp/boiler_curflowtemp/config

{
  "~": "ems-esp",
  "uniq_id": "boiler_curflowtemp",
  "stat_t": "~/boiler_data",
  "name": "Boiler current flow temperature",
  "val_tpl": "{{value_json.curflowtemp}}",
  "unit_of_meas": "°C",
  "ic": "mdi:coolant-temperature",
  "state_class": "measurement",
  "dev": {
    "ids": [
      "ems-esp-boiler"
    ]
  }
}

The name is built from the full name and used in the HA dashboard.

You can test this running the standalone 'make run' and then 'test ha' to see the output if you're on Win10 with WSL, Linux or MacOSX.

Anyway, we're getting off-topic here! Can I rename 'Boiler Temperature' to 'Actual boiler temperature' ?

@bartplessers
Copy link
Author

Hi,
Thanx for all this attention!
For me -as an end user- it really doesn't matter what the technical name will be. "sensor.boiler_current_flow_temperature" is fine.
As long as it is more or less stable and doesn't change with every update...

@proddy
Copy link
Contributor

proddy commented Sep 19, 2021

hopefully, it won't change too much! I added a command 'entities' which shows the HA entity name, use it like http://ems-esp/api/boiler/entities

@proddy proddy closed this as completed Sep 19, 2021
@proddy proddy added the enhancement New feature or request label Sep 19, 2021
@bartplessers
Copy link
Author

Hi @proddy ,

Sorry I have to come back on this.
Small question: is there any way to connect the (historical)data from the "previous" sensor to the "new" sensor? If an entity get's a new name in esp --> new sensor in hass :-(

Another suggestion: about naming convention: It would be nice if the "system" entities could have a common prefix different from "system". I think this name is too generic and can interference with other "system" entities. It would be more transparent if the (autodiscovered) entities of ems_esp all have the same prefix, i.e. "ems_esp_"

Now I have defined a group as

  my_91_p01_ems_esp_system:
    name: 91.01 EMS-ESP system
    icon: mdi:lightbulb-group
    entities:
    - sensor.ems_esp_status
    - sensor.system_free_memory
    - sensor.system_mqtt_fails
    - sensor.system_rx_fails
    - sensor.system_rx_received
    - sensor.system_tx_fails
    - sensor.system_tx_reads
    - sensor.system_tx_writes
    - sensor.system_uptime
    - sensor.system_uptime_sec
    - sensor.system_wifi_rssi
    - sensor.system_wifi_strength

but I would like to have something like

  my_91_p01_ems_esp_system:
    name: 91.01 EMS-ESP system
    icon: mdi:lightbulb-group
    entities:
    - sensor.ems_esp_status
    - sensor.ems_esp_system_free_memory
    - sensor.ems_esp_system_mqtt_fails
    - sensor.ems_esp_system_rx_fails
    - sensor.ems_esp_system_rx_received
    - sensor.ems_esp_system_tx_fails
    - sensor.ems_esp_system_tx_reads
    - sensor.ems_esp_system_tx_writes
    - sensor.ems_esp_system_uptime
    - sensor.ems_esp_system_uptime_sec
    - sensor.ems_esp_system_wifi_rssi
    - sensor.ems_esp_system_wifi_strength

Same for other entities:

  • sensor.boiler_* --> - sensor.ems_esp_boiler_*
  • sensor.thermostat_* --> - sensor.ems_esp_thermostat_*
  • ...

just a suggestion...

kind regards,
B

@proddy
Copy link
Contributor

proddy commented Sep 23, 2021

damn, didn't realize by renaming a sensor that it'll start a new history. I can't think of way to do that but I'm sure if you ask in the Home Assistant channel on Discord you'll get a few ideas and perhaps scripts to do it. The data after all is in a relational DB via the recorder service.

having system_ prefixed is indeed a bit silly. I like your idea by renaming to emsesp or ems_esp. It would be a breaking change though and piss a lot of people off! :-)

@proddy proddy reopened this Sep 23, 2021
@bartplessers
Copy link
Author

bartplessers commented Sep 23, 2021

just an idea:
on
http://ems-esp/mqtt/settings
you can provide a "Home Assistant MQTT Discovery: use mqtt-base as autodiscover prefix" checkbox (1) somewhere:

2021-09-23_16-58-17

By that, users can choose to select the mqtt-base as a prefix:

  • useful if you are running multiple instances of ems-esp
  • and people can choose to leave this blanc if no change is needed.

So in my case, I would find all my devices under MQTT-topics
homeassistant/+/ems-esp/#
and all my homeassistant autodiscovered devices like

  my_n00_p00_ems:
    name: 00.00 EMS-ESP
    icon: mdi:lightbulb-group
    entities:
    - binary_sensor.ems_esp_boiler_fan
    - binary_sensor.ems_esp_boiler_gas
    - binary_sensor.ems_esp_boiler_heating_activated
	...
    - sensor.ems_esp_boiler_burner_current_power
    - sensor.ems_esp_boiler_burner_max_power
    - sensor.ems_esp_boiler_burner_min_period
    - sensor.ems_esp_boiler_burner_min_power
	...
    - sensor.ems_esp_status
    - sensor.ems_esp_system_free_memory
    - sensor.ems_esp_system_mqtt_fails
    - sensor.ems_esp_system_rx_fails
    - sensor.ems_esp_system_rx_received
    - sensor.ems_esp_system_tx_fails
	...
    - sensor.ems_esp_thermostat_building
    - sensor.ems_esp_thermostat_circuit_1_extra
	...
    - sensor.ems_esp_thermostat_hc1_control_mode
    - sensor.ems_esp_thermostat_hc1_current_room_temperature
    - sensor.ems_esp_thermostat_hc1_design_temperature
    - sensor.ems_esp_thermostat_hc1_eco_temperature
	...
    - sensor.ems_esp_thermostat_minimal_external_temperature
    - sensor.ems_esp_thermostat_mode
    - sensor.ems_esp_thermostat_set_low_temperature
    - sensor.ems_esp_thermostat_set_temperature

Other users who don't want any changes, can leave the checkbox unchecked.

PS:
there are some other inconsistencies: I have only have one thermostat, hc1, but I don't know what happens if you have hc1 and hc2 for example "thermostat_mode"...
For the "hc1_control_mode", the thermostat "hc1" is in the name, so not ambiguous, bu for "thermostat_mode", it don't now what will be displayed...

    - sensor.ems_esp_thermostat_hc1_control_mode
    - sensor.ems_esp_thermostat_mode

@proddy
Copy link
Contributor

proddy commented Sep 23, 2021

I think it's best as a specific HA setting (under Settings) but I like the idea of having it optional. Thanks for the suggestion.

For the "hc1_control_mode", the thermostat "hc1" is in the name, so not ambiguous, bu for "thermostat_mode", it don't now what will be displayed...

I don't have a sensor.ems_esp_thermostat_mode and not sure where it comes from. Could it be left over from an older release?

@MichaelDvP
Copy link
Contributor

This mode is shortname wwmode and not related to heatingcircuits. I earlier versions it was tagged and prefixed with "warm water", but sadly in v3.1.1b6 the tags and prefixes for warm water in thermostat/solar were removed, leading to a bit unspecific names.

@proddy
Copy link
Contributor

proddy commented Sep 24, 2021

This mode is shortname wwmode and not related to heatingcircuits. I earlier versions it was tagged and prefixed with "warm water", but sadly in v3.1.1b6 the tags and prefixes for warm water in thermostat/solar were removed, leading to a bit unspecific names.

didn't realize this. Why not just add the "ww" prefix back to these?

MAKE_PSTR_LIST(wwMode, F("wwmode"), F("mode"))
MAKE_PSTR_LIST(wwSetTempLow, F("wwsettemplow"), F("set low temperature"))
MAKE_PSTR_LIST(wwCharge, F("wwcharge"), F("charge"))
MAKE_PSTR_LIST(wwChargeDuration, F("wwchargeduration"), F("charge duration"))
MAKE_PSTR_LIST(wwExtra1, F("wwextra1"), F("circuit 1 extra"))
MAKE_PSTR_LIST(wwExtra2, F("wwextra2"), F("circuit 2 extra"))

@MichaelDvP
Copy link
Contributor

There are some more in solar`without prefix:

MAKE_PSTR_LIST(wwTemp1, F("wwtemp1"), F("temperature 1"))
MAKE_PSTR_LIST(wwTemp3, F("wwtemp3"), F("temperature 3"))
MAKE_PSTR_LIST(wwTemp4, F("wwtemp4"), F("temperature 4"))
MAKE_PSTR_LIST(wwTemp5, F("wwtemp5"), F("temperature 5"))
MAKE_PSTR_LIST(wwTemp7, F("wwtemp7"), F("temperature 7"))
MAKE_PSTR_LIST(wwPump, F("wwpump"), F("pump"))
MAKE_PSTR_LIST(wwMinTemp, F("wwmintemp"), F("minimum temperature"))

The mixer ww:

MAKE_PSTR_LIST(wwPumpStatus, F("pumpstatus"), F("pump status in assigned wwc (PC1)"))
MAKE_PSTR_LIST(wwTempStatus, F("wwtempstatus"), F("temperature switch in assigned wwc (MC1)"))
MAKE_PSTR_LIST(wwTemp, F("wwtemp"), F("current temperature"))

are prefixed with "wwc" by tag, that's why i prefer to use a TAG_DEVICE_DATA_WW for the thermostat/solar as it was before v3.1.1b6 (and still in my personal build), but you can also edit the names and add a "ww ". In thermostat there is wwprio, F("wwprio"), F("warm water priority"), which in in heatingcircuit and already tagged with `hc.

@proddy
Copy link
Contributor

proddy commented Sep 24, 2021

there was a reason I removed the 'ww' tags, perhaps it was the duplicate long names in HA or something to reduce the json sizes but I just can't remember anymore. In any case we should have build that works for everyone, so your request Michael is a valid as everybody elses. Let's get them back in and find another solution to the original problem (If I can remember it!)

@MichaelDvP
Copy link
Contributor

I've tried to find out, but no luck. I think it started with this commit to add ww to HA entitiy. A bit later in this commit you've removed the tag and set ww to the fullnames for all. I don't know why, maybe another way to do the same. But than you have the double ww in entitys and removed the ww from fullnames here, but do not fix the tag for thermostat/solar ww.

Should i change to TAG_DEVICE_DATA_WW again? Or add ww prefix to the fullnames of thermostat/solar?

@MichaelDvP
Copy link
Contributor

Ok, i've found the reason why you've removed the other devices. In publish_ha_sensor the 'is_nested' checks for boiler, so

    if (is_nested) {
        snprintf(val_tpl, sizeof(val_tpl), "{{value_json.%s}}", new_entity);

adds a wrong ww. path for other devices.
But i think this

    if (have_tag) {
        snprintf(new_entity, sizeof(new_entity), "%s.%s", EMSdevice::tag_to_string(tag).c_str(), uuid::read_flash_string(entity).c_str());

is not very pretty, it creates also for boiler e.g. ww.wwtemp and makes uniq_id ww_wwtemp, to much ww.
If we use if (tag >= DeviceValueTAG::TAG_HC1) { we exclude the ww from new_entity and val_tpl, but not from name and HA-entity.

@proddy
Copy link
Contributor

proddy commented Sep 24, 2021

good find, I remember it was something to do with HA. If the (tag >= DeviceValueTAG::TAG_HC1) works I'm happy to try that out and put the tags back in.

@MichaelDvP
Copy link
Contributor

I've made a pr with the changes to ww prefix for all warm water related names on all devices. Someone using HA has to check the discovery working, a bin is here.

@proddy
Copy link
Contributor

proddy commented Sep 26, 2021

works fine - it's now merged with dev. Still to do is make changes to replace system_ with emsesp_

@proddy
Copy link
Contributor

proddy commented Sep 26, 2021

Actually, this is not possible due to the way the silly HA MQTT Discovery works. The name of the sensor is also the title shown in the Lovelace UI. So to get a sensor.emsesp_rx_fails it would show in the UI as "EMSESP # Rx Fails" which is ugly. I've decided to leave it how it is and focus on #133

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

3 participants