KlausMu / esp32-fan-controller

ESP32 fan controller with temperature sensor and MQTT
299 stars 56 forks source link

homeassistent mqtt discovery #17

Closed s4saken closed 1 year ago

s4saken commented 1 year ago

Hi KlausMu,

I managed to take the changes from #14 and modify the home assistent discovery payloads. I am getting the hang of stuff, but i am not a programmer. I managed to succeed in subscribing to topic [homeassistent/status] and act accordingly when receiving a online message, to facilitate for HA restarts. The fan state also has to be in a topic, this is also created.

For testing, you can use mqtt explorer to send online message to topic [homeassistent/status].

At this points mqtt discovery of the fan and sensors is only fully done on HA restart/online message. This needs a good look by some1 who has programming knowledge in c.

The newly created discovery payloads need to be done at the start of the device, but i could not wrap my head around this, with my knowledge :).

For now there is just a few things left to do, to make this great. Since the groundwork has been done. I assume this could be easy for you to do.

  1. All discovery payloads on esp32 start.
  2. Make pubsubclient set a retained last will message on connect to topic [homeassistant/climate/esp32_fan_controller/status] payload >>[offline]. (seems to be quite easy to do, but my programming skills/time are insufficient).
  3. Enable retained messages. modify "mqtt.cpp" line nr. 97 to mqttClient.publish(topic, payload, true).

If these changes can be made, i can modify the discovery string of the fan to include availability on the last will topic. After this, the discovery on restart part can be removed and retained messages enabled. We make the fan unavailable when pulling the power and available when online. No ghost entities and proper handling of online/offline status of the device. mqttdiscoveryfan

Let me know what you think.

KlausMu commented 1 year ago

Hi,

thanks a lot for your pull request.

The things you are asking for can for sure be done by me. It is not a big thing.

I would like to have a closer look at the additional discovery messages you introduced. I have seen something similar from other devices. Do you have a link for me where it is described which discovery messages are necessary? Any documentation from HA?

And can you please post here the modified discovery string of the fan to include availability on the last will topic, so that I can test it?

Edit: I don't think these values in the discovery payload are correct:

"mode_command_topic":"esp32_fan_controller/tele/STATE2", "mode_command_template":"{{ value_json.mode }}", "mode_state_topic":"homeassistant/climate/esp32_fan_controller/state", "mode_state_template":"{{ value_json.mode }}",

This would mean "mode" is a field in "STATE2", but it is not. STATE2 was introduced by me, and you didn't modify it, right?

Thanks

s4saken commented 1 year ago

Hi,

Well to be honest, the necessary arguments you need in that discovery string is not so clearly described. What i can say for sure, is that discovery is just a payload which is not different from the way you configure the sensor in yaml code manually. But you have to put it in json format and it should match values discribed in the ha mqtt page.

If you look at the code, you will see that homeassistant/sensor is the topic for sensors, likewise for homeassistant/climate. Those 2 we use here. Giving it a name with /config behind it makes sure homeassistent will parse this as code to configure either climate or sensor. I think just yaml in json format will work.

Example: the fan climate string. "{\"name\":\"Fan_Controller\",\"unique_id\":\"Fan_Controller\",\"icon\":\"mdi:fan\",\"min_temp\":10,\"max_temp\":50,\"temp_step\":1.0,\"current_humidity_topic\":\"esp32_fan_controller/tele/STATE1\",\"current_humidity_template\":\"{{value_json.hum}}\",\"current_temperature_topic\":\"esp32_fan_controller/stat/ACTUALTEMP\", \"temperature_command_topic\":\"esp32_fan_controller/cmnd/TARGETTEMP\",\"temperature_state_topic\":\"esp32_fan_controller/stat/TARGETTEMP\",\"modes\":[\"fan_only\"], \"mode_command_topic\":\"esp32_fan_controller/tele/STATE2\",\"mode_command_template\":\"{{ value_json.mode }}\",\"mode_state_topic\":\"homeassistant/climate/esp32_fan_controller/state\",\"mode_state_template\":\"{{ value_json.mode }}\",\"precision\":1.0,\"device\":{\"identifiers\":[\"esp32_fan_controller\"],\"name\":\"esp32_fan_controller\",\"model\":\"esp32_fan_controller\",\"manufacturer\":\"KlausMu\"}}";

{ "name":"Fan_Controller", "unique_id":"Fan_Controller", "icon":"mdi:fan", "min_temp":10, "max_temp":50, "temp_step":1.0, "current_humidity_topic":"esp32_fan_controller/tele/STATE1", "current_humidity_template":"{{value_json.hum}}", "current_temperature_topic":"esp32_fan_controllerstat/ACTUALTEMP", "temperature_command_topic":"esp32_fan_controller/cmnd/TARGETTEMP", "temperature_state_topic":"esp32_fan_controller/stat/TARGETTEMP", "modes":["fan_only"], "mode_command_topic":"esp32_fan_controller/tele/STATE2", "mode_command_template":"{{ value_json.mode }}", "mode_state_topic":"homeassistant/climate/esp32_fan_controller/state", "mode_state_template":"{{ value_json.mode }}", "precision":1.0, "device":{ "identifiers":["esp32_fan_controller"], "name":"esp32_fan_controller", "model":"esp32_fan_controller", "manufacturer":"KlausMu"} }";

The device part, puts it all together in 1 device, those are the same for climate and sensors. This is also a diffrent map seen by {}, i think availability is also done in this way.

I read some posts from other people together with the explanation here : https://www.home-assistant.io/integrations/climate.mqtt/

Adding availability topic, would mean we have to look here for the right stuff to add in the string: https://www.home-assistant.io/integrations/climate.mqtt/#availability

For finding the shortcuts and i used this: https://www.home-assistant.io/integrations/mqtt#discovery-messages

Also with some options the discovery does not work at all, this is what i mean by trial and error. Looking into a way now to look at the ha log to see if we can see how this is being processed.

I produced those strings from trial and error, and looking at other devices and analysing the messages from the broker.

Looking at the supported abbreviations, i also think the strings can stil be shorter.

s4saken commented 1 year ago

Hi,

thanks a lot for your pull request.

The things you are asking for can for sure be done by me. It is not a big thing.

I would like to have a closer look at the additional discovery messages you introduced. I have seen something similar from other devices. Do you have a link for me where it is described which discovery messages are necessary? Any documentation from HA?

And can you please post here the modified discovery string of the fan to include availability on the last will topic, so that I can test it?

Edit: I don't think these values in the discovery payload are correct:

"mode_command_topic":"esp32_fan_controller/tele/STATE2", "mode_command_template":"{{ value_json.mode }}", "mode_state_topic":"homeassistant/climate/esp32_fan_controller/state", "mode_state_template":"{{ value_json.mode }}",

This would mean "mode" is a field in "STATE2", but it is not. STATE2 was introduced by me, and you didn't modify it, right?

Thanks

You are right here, those are indeed not right, but without those values although wrong, the fan device might not show up. So carefull here. I will also test this now. Problem with this also is, fan is off when you restart ha. Having 1 mode is not enough to just tell it, its on. That is where i struggled, until i made the state topic, which worked. but this was key there: "mode_state_topic":"homeassistant/climate/esp32_fan_controller/state",

I first tried to use tele2, to ad a static value like mode : fan_only, but i did not get this to work. This is why it is in there.

Ideal, would be like a complete on/off switch in the software, where it does like screen/fan off. Than using this state would be complete, because then you have a real smart device :D.

s4saken commented 1 year ago

KlausMu

I changed the climate string in this.

"{\"name\":\"Fan_Controller\",\"uniq_id\":\"Fan_Controller\",\"ic\":\"mdi:fan\",\"min_temp\":10,\"max_temp\":50,\"temp_step\":1.0,\"current_humidity_topic\":\"esp32_fan_controller/tele/STATE1\",\"current_humidity_template\":\"{{value_json.hum}}\",\"availability_topic\":\"homeassistant/climate/esp32_fan_controller/status\",\"curr_temp_t\":\"esp32_fan_controller/stat/ACTUALTEMP\",\"temp_cmd_t\":\"esp32_fan_controller/cmnd/TARGETTEMP\",\"temp_stat_t\":\"esp32_fan_controller/stat/TARGETTEMP\",\"modes\":[\"fan_only\"], \"mode_cmd_t\":\"esp32_fan_controller/tele/STATE2\",\"mode_cmd_tpl\":\"{{ value_json.mode }}\",\"mode_stat_t\":\"homeassistant/climate/esp32_fan_controller/state\",\"mode_stat_tpl\":\"{{ value_json.mode }}\",\"precision\":1.0,\"dev\":{\"ids\":[\"esp32_fan_controller\"],\"name\":\"esp32_fan_controller\",\"mdl\":\"esp32_fan_controller\",\"mf\":\"KlausMu\"}}";

including "availability_topic\":\"homeassistant/climate/esp32_fan_controller/status\",

Now device is unavailable, until i use mqtt explorer to dump online in that value and topic. That is great.

Now this has to be included in the sensor strings also, can be the same value, and we are set. Together with that last will message. Wünderbar!

Note,

Using retain on all messages, wil keep this at the broker. I dont know if this is a good thing. >> apart from the last will message. i dont think the discovery messages should be retained.

Having a rediscover message like i made seems like a good solution, but there is just 1 thing i did not mention. After rediscovery was done, the value for: stat/TARGETTEMP, also has to be repeated for home assistant to fill the device status. Otherwise the climate device does not show sliders.

2 options:

1: enable all messages to be retained >> retained discovery messages are heavy on systems. (i read in different posts)

  1. enable only last will message to be retained and use home assistant rediscovery, like a made, including a republish of :stat/TARGETTEMP.

This project is and was gold, putting time in something like this, gives me joy.

s4saken commented 1 year ago

Updated working climate string. Taken out all that is not needed:

"{\"name\":\"Fan_Controller\",\"uniq_id\":\"Fan_Controller\",\"ic\":\"mdi:fan\",\"min_temp\":10,\"max_temp\":50,\"temp_step\":1.0,\"current_humidity_topic\":\"esp32_fan_controller/tele/STATE1\",\"current_humidity_template\":\"{{value_json.hum}}\",\"availability_topic\":\"homeassistant/climate/esp32_fan_controller/status\",\"curr_temp_t\":\"esp32_fan_controller/stat/ACTUALTEMP\",\"temp_cmd_t\":\"esp32_fan_controller/cmnd/TARGETTEMP\",\"temp_stat_t\":\"esp32_fan_controller/stat/TARGETTEMP\",\"modes\":[\"fan_only\"],\"mode_stat_t\":\"homeassistant/climate/esp32_fan_controller/state\",\"precision\":1.0,\"dev\":{\"ids\":[\"esp32_fan_controller\"],\"name\":\"esp32_fan_controller\",\"mdl\":\"esp32_fan_controller\",\"mf\":\"KlausMu\"}}";

KlausMu commented 1 year ago

Hey, seems like you did a great job! I'm still testing, but looks very good.

One thing: don't you think "homeassistant/climate/esp32_fan_controller/status" "online" shouldn't also be retained? Maybe it doesn't matter to HA, because after HA being online it will receive "online" from the device. But another mqtt client will still receive "offline" (which was sent retained) if he connects after the "online" (which was not send retained).

Or I have to delete the retained "offline" and send "online" without retain flag. What do you think?

s4saken commented 1 year ago

Hey, seems like you did a great job! I'm still testing, but looks very good.

One thing: don't you think "homeassistant/climate/esp32_fan_controller/status" "online" shouldn't also be retained? Maybe it doesn't matter to HA, because after HA being online it will receive "online" from the device. But another mqtt client will still receive "offline" (which was sent retained) if he connects after the "online" (which was not send retained).

Or I have to delete the retained "offline" and send "online" without retain flag. What do you think?

Retained for that value seems good, but i think, if last will is honored it will publish this topic as retained topic. Not sure, but i think it works like this. Either way, it does not hurt to have this retained. We can always test what works best. Keep in mind that most of these topics are only made for home assistent and other mqtt devices do not use this.

Otherwise, if you want to use status for the device with other mqtt clients, you have to make a topic like /stat/STATUS. Not within homeassistent defined. "homeassistant/climate/esp32_fan_controller/status" says enought. For example with OPENHAB, you would have to enable the use of homeassistent before you could use it for that. Not wrong, but more like strange.

The only topics / payload that should not be retained are the ones for discovery itself. I dont think its good practice, to have a broker add a sensor, if its not even there :(.

You dont have to delete retained offline, i think it only works on the topic. The value can be overwritten.

s4saken commented 1 year ago

To be honest, i am gonna follow a course on c++ just for these kind of projects.

If i where you, i would hunt the following. If you have time, keeping in mind, its not just handy for this project. I would create: Topics /stat/STATUS /stat/STATE /cmnd/STATE

Use the /cmnd/STATE for accepting incoming message "fan_only" and "off".

Then on off make some code to turn the fan pwm to 0 and the display to off. For most people the device is off in this state.

Seems like a lot of work to me now. But this would make it perfect.

KlausMu commented 1 year ago

Ok, I tried to put everything together: the code from your pull request and all the remarks here. And I read the documenation from HA and did some minor adjustments as well.

Before implementing, please have a look at the wiki (only the first heading called "MQTT discovery"). https://github.com/KlausMu/esp32-fan-controller/wiki/06-Home-Assistant

The other 5 config topics for pressure, altitude and so on were not tested by me until now. If I understand correctly, they are only optional and not necessary for the climate, right?

s4saken commented 1 year ago

Ok, I tried to put everything together: the code from your pull request and all the remarks here. And I read the documenation from HA and did some minor adjustments as well.

Before implementing, please have a look at the wiki (only the first heading called "MQTT discovery"). https://github.com/KlausMu/esp32-fan-controller/wiki/06-Home-Assistant

The other 5 config topics for pressure, altitude and so on were not tested by me until now. If I understand correctly, they are only optional and not necessary for the climate, right?

Just did a check on https://github.com/KlausMu/esp32-fan-controller/wiki/06-Home-Assistant, this seems about right.
Having only retained on that offline message, is something i am not completely sure about, but ill have to test it to confirm.

The other 5 topics are indeed just optional, but very nice to have.