KlausMu / esp32-fan-controller

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

discovery issue #15

Closed s4saken closed 1 year ago

s4saken commented 1 year ago

Hi, Great project, thanks for all the work. I am building a great 3d printer enclosure with this.

Small Problem: After restarting a mqtt client (in my case, ha), the entity is not available until i restart the unit and it republishes via mqtt.

Do you think its possible to add retain on the mqtt messages, so restarting is not necessary?

KlausMu commented 1 year ago

Thanks for your feedback. I'm happy if this project is helpful.

Please try to change in file "mqtt.cpp" line nr. 97 from mqttClient.publish(topic, payload,) to mqttClient.publish(topic, payload, true)

Please let me know if this works.

s4saken commented 1 year ago

Thanks for the quick response. This seems to be a fix.

After restarting ha, the entity is off, until turning it to fan mode. This is ok, i can make an automation for this.

I think, having like a timer on the discovery publish / payload every 5 or 10 minutes might be better for safety/reliability. Right now, when i pull the power from the pcb, all is well and nothing changes :).

I am learning still, but i dont know a lot of c, so ill keep looking for the right solution.

I am happy.

Thanks man

KlausMu commented 1 year ago

Maybe we mix up several things here:

Do you want to immediately get the last state of the fan controller when HA is started, without waiting for the fan controller to publish the next state: then the fan controller has to publish every message with the "retain" flag. In that case, the broker will save the last received message and send it to clients that newly subscribe, even after the last message was published. https://www.hivemq.com/blog/mqtt-essentials-part-8-retained-messages/

If you don't want HA to take the entity offline, even if no new states are sent, then this is something you have to define in HA. The broker will never periodically republish old messages. Please try this: https://www.home-assistant.io/integrations/sensor.mqtt/#expire_after

Discovery is something that is only needed once in a lifetime. As soon as HA has received such a discovery message, the entity is automatically created. No need to resend the discovery (at least this is what I think). But if the entity does not receive further regular updates (not discovery messages), it will be taken offline.

In my case, I want HA to take the fan controller offline when it is switched off, so I explicitly set "expire_after: 60" in my HA instance.

s4saken commented 1 year ago

Let me clarify.

If i manual configure mqtt without discovery, it works as expected with expire_after. Since you have been using this before the discovery was added, i assume you have this setup also. I think this is a different approach. This is also a fix for my problem, since i can just manually configure.

Regarding discovery: After HA does a restart, the device will be unavailable when using just discovery. Setting a retained flag, is indeed a fix, although not the best.

It seems like the solution to this problem is in here https://www.home-assistant.io/integrations/mqtt/#how-to-use-discovery-messages. They explicitly state a warning about using retained messages and possibility of creating ghost entities. It seems like using birth and will messages to trigger discovery, is the right solution. https://www.home-assistant.io/integrations/mqtt/#use-the-birth-and-will-messages-to-trigger-discovery

I am going to dig a little deeper into discovery. Would love to see entities added also.

KlausMu commented 1 year ago

Ok, so you are talking about the pull request about dicovery from nhangeland? https://github.com/KlausMu/esp32-fan-controller/pull/14 It is not yet merged, because nhangeland seems to have the same issue as you have.

I also tried the pull request from nhangeland. But I can't really see the problem.

Here is the entity created by discovery. Offline now since days, but not "unavailable" in HA.

image

Here is how the card in the dashboard looks like, as said, not used since days:

image

If I connect the ESP32, I get updated values within seconds.

With the manually created entities it is different. They get unavailable after 60 seconds.

s4saken commented 1 year ago

Yes, exactingly the same behavior.

I assume you have: line 97 "mqtt.cpp" >> mqttClient.publish(topic, payload, true)

If you change this to original "mqtt.cpp" >> mqttClient.publish(topic, payload) The entity will be unavailable after restarting HA, and the only way forward is restarting esp32.

s4saken commented 1 year ago

Hi Klaus,

Currently i am testing some changes that are working. I managed to make it subscribe to homeassistant/status. Where it waits for a online message from HA, so it does another discovery config/payload.

I had to clear broker database to get rid of those retained messages. The change seems to work, apart from the device not returning to the state is is in. So after a restart, the device comes back in off state. Currently checking out how to fix this.

When i am ready ill submit a pull request.

KlausMu commented 1 year ago

Ok. You can delete specific retained messages: https://community.openhab.org/t/clearing-mqtt-retained-messages/58221

KlausMu commented 1 year ago

I managed to make it subscribe to homeassistant/status. Where it waits for a online message from HA, so it does another discovery config/payload.

That is for sure the wrong way. The sensor should never need to know anything about the home automation software or that it even exists. I never saw it that way.

Edit: ah, I see, this is done via the mqtt broker. Ok, that's fine. Should first have read the docu you posted here.

s4saken commented 1 year ago

Indeed.

I guess this is a good fix already and no reboot needed anymore on the esp32.

The next problem is the status after restart of ha, device is there now, but its off, while in reality it is always on. So i guess we need like status topic. But apart from this i am looking at other projects and mqtt implementations.

Testing some discovery payloads to see if i can get the following working:

Climate Device >> Entities. esp32 powerless >> status changes to unavailable. Ha restart >> Device on status.

KlausMu commented 1 year ago

Great, looking forward to this. Thanks for contributing.

KlausMu commented 1 year ago

Ok, I did a lot of changes now:

You can find the whole updated code in the branch "HA_MQTT_discovery"

You can either directly checkout this branch with another git clone: git clone --branch HA_MQTT_discovery https://github.com/KlausMu/esp32-fan-controller

or you can go into your already existing repository (a clone of my repository, not yours) and say git pull git branch -a (check that HA_MQTT_discovery exists) git checkout HA_MQTT_discovery

if you want to, you can go back to main branch with git checkout main

I'm looking forward to your test feedback.

s4saken commented 1 year ago

case

Klaus, the new commits work like a charm. Thanks man, i can see how much work this was. I knew that those timings would need to be perfect, but you did it.

Now ill take a look to see if i can turn the mode to on if i press the touch display while it is off. Great, ill close this!.

Regardint the fan pwm, people can always modify this value.

KlausMu commented 1 year ago

Glad to hear that it works well. And thanks for the photo.

Now that I understood how HA discovery works, I'm planning to support the MQTT fan as well (no temperature control, only fan). https://www.home-assistant.io/integrations/fan.mqtt/

Do you think it might be helpful to have a power off button on the display as well (which is not really a power off button, more a standby)?

Until now I only have a shutdown button (which you don't use), but this button only calls some endpoint in HA which can do anything - in my case shutdown a Raspberry Pi and then switch off a smart plug to power off the 3D printer, fan controller and Raspberry Pi. But for someone who wants the fan controller to be powered the whole time, such a standby button might be helpful. Would you use it?

s4saken commented 1 year ago

Hi Klaus,

This could indeed be handy. Might be just an option in the config like.

Poweroffbutton 1: your style Poweroffbutten 2: fan/display off style.

The room on the screen is there.

That fan off works great with my fan btw. So i guess other people can get it to work also, but they might need to adjust the value with other fan types.

I seems that i have broken my touch panel, :(.

Atm i am working on a closing mechanism for this case, it has to pull the door inside a rubber. And when that is fixed, i am gonna focus on a 3 channel relays, 1 for heater, added to the temperature controller code for temps higher then the set temp. Same thing for lights and power in the case.

KlausMu commented 1 year ago

I seems that i have broken my touch panel, :(.

Are you using an integrated device where you have to set a jumper for flashing the ESP32? I have such a device. If you forget to remove the jumper, touch does not work.

s4saken commented 1 year ago

I seems that i have broken my touch panel, :(.

Are you using an integrated device where you have to set a jumper for flashing the ESP32? I have such a device. If you forget to remove the jumper, touch does not work.

Unfortunately not, because of this case i use some jumper cables to the display. I think i might have hit a touch pin to the 12v line. I dont think a touch chip is able to survive something like that.