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
563 stars 101 forks source link

EMS-ESP-3_5_0b2 - HA Discovery not working #634

Closed zibous closed 1 year ago

zibous commented 1 year ago

Bug description Version v3.4.2b5 is working.

After installing the new version, wrong topic entries are made at HA Discovery. Some are correct but most are created under the hostname of the device
and the i can`nt see any EMS ESP Devices.

Steps to reproduce

  1. Delete all HA Discovery Items
  2. Install Version EMS-ESP-3_5_0b2
  3. Check HA Devices EMS_ESP

Screenshots

Bildschirmfoto 2022-09-22 um 19 10 13

Device information emsesp_info.txt

MichaelDvP commented 1 year ago

I think this is #596. But the commit only addresses ems entities, dallas/analog follows the old scheme. As far as i understand (i'm not using ha) the config is now generated as homeassistant/sensor/hostname/entity/config with "~":base, "stat_t":"~/device_data" and entities are published in base/device_data. What exactly is not working?

zibous commented 1 year ago

What exactly is not working?

Previous Version - shows devices:

Bildschirmfoto 2022-09-23 um 11 13 20

  1. Remove all ems-esp devices form Homeassistant
  2. Remove all items for MQTT Brocker homeassistant/ems-heizung

Install version EMS-ESP-3_5_0b2

Result: EMS-ESP-3_5_0b2 - No EMS-ESP Devices found Bildschirmfoto 2022-09-23 um 11 29 23

Switch back to Firmware: v3.4.2b5 All devices are present and working.

proddy commented 1 year ago

I'm back behind a PC now and have reproduced this. If you use a FQDN as the hostname it doesn't work. Thomas was also complaining on Discord about this. The best solution is to create a new option in the MQTT Settings called 'prefix' or something which we'll use as prefix instead of base or hostname. https://www.home-assistant.io/docs/mqtt/discovery/#discovery-topic

MichaelDvP commented 1 year ago

Could we keep it mostly compatible to v3.4.2 and use the base for discover node-id, but replace possible slashes by underscore. e.g base emsesp keeps as is, but base: home/esp/emsesp as in #596 is set as homeassistant/sensor/home_ems_emsesp/name/config. Most users have a single base, so there is no change. Maybe this could also used as prefix for entities. Is this prefix realy needed?

zibous commented 1 year ago

@proddy

The best solution is to create a new option in the MQTT Settings called 'prefix' or something which we'll use as prefix instead of base or hostname. https://www.home-assistant.io/docs/mqtt/discovery/#discovery-topic

I can not find the prefix for the MQTT Settings, only discovery_prefix string (Optional, default: homeassistant), which I cannot change, because otherwise all other devices will no longer work.

EMS-ESP will create retained MQTT messages prefixed with homeassistant/ for each device 
and their values (called entities). For example "EMS-ESP Thermostat". 
You can view which ones have been created by going into Home Assistant's 
Configuration->Integrations and select the devices under MQTT.

see: https://emsesp.github.io/docs/#/Home-Assistant

MichaelDvP commented 1 year ago

@zibous : This is a prefix proddy added to object_id to make it more unique. see here

proddy commented 1 year ago

Could we keep it mostly compatible to v3.4.2 and use the base for discover node-id, but replace possible slashes by underscore. e.g base emsesp keeps as is, but base: home/esp/emsesp as in #596 is set as homeassistant/sensor/home_ems_emsesp/name/config. Most users have a single base, so there is no change. Maybe this could also used as prefix for entities. Is this prefix realy needed?

that would work. Replace . with _ using the base. Shall I make the change?

MichaelDvP commented 1 year ago

As you like. I've already started a test, so it's half way implemented here. But i can't test with HA. This is how it looks like with base ems/esp grafik Mainly in load_settings set:

    // create basename from base
    mqtt_basename_ = mqtt_base_;
    std::replace(mqtt_basename_.begin(), mqtt_basename_.end(), '/', '_');

and use mqtt_basename_ instead of system_hostname_ everywhere.

proddy commented 1 year ago

looks good. I can test it from your repo. I have a project setup in VSC that pulls from your dev

MichaelDvP commented 1 year ago

Ok, just added analog/dallas, now compiling, will push in a few minutes to my repo.

zibous commented 1 year ago

@proddy

I think the problem is:

: The ID of the device. This is only to allow for separate topics for each device and is not used for the entity_id. The ID of the device must only consist of characters from the character class [a-zA-Z0-9_-] (alphanumerics, underscore and hyphen).
{
    "~": "ems-heizung",
    "uniq_id": "system_ntp_status",
    "stat_t": "~/heartbeat",
    "name": "NTP status",
    "object_id": "ems-heizung.siebler.home_system_NTP status",
    "val_tpl": "{{value_json.ntp_status}}",
    "payload_on": "on",
    "payload_off": "off",
    "state_class": "measurement",
    "dev": { "ids": ["ems-esp"] }
}

"object_id": "ems-heizung.siebler.home_system_NTP status"

I my case the object_id is not valid. Why not slugify the object_id value ems-heizungsieblerhomesystemntp-status ?

see: https://slugify.online/

MichaelDvP commented 1 year ago

HA documentation and namings are also confusing to me. There is a object_id in the discovery topic with naming restrictions and this is not the entity. And there is an object_id in payload, this is entity and allows whitespace, etc. Now we have the topic-node id as a single word base, topic-object_id as device_shortname as before and payload object_id without prefix as device_EN-name (as in v3.4). Have you tried the new dev?

proddy commented 1 year ago

@zibous try the latest dev. Michael has fixed this. The object_id and names are no longer prefixed

zibous commented 1 year ago

HA documentation and namings are also confusing to me. Yes me too...

Have you tried the new dev? sorry now testing Firmware: v3.5.0b3 and it works.

Thanks.

proddy commented 1 year ago

great. closing. do re-open if you find an issue