rbaron / parasite-scanner

A bridge for b-parasites
MIT License
12 stars 2 forks source link

Update mqtt.go #1

Closed kuerb closed 3 years ago

kuerb commented 3 years ago

In Home Assistant sensor values were not be displayed automatically, even though the payload was submitted: Bildschirmfoto 2021-05-25 um 18 06 13

This change publishes an online status for the whole system and allows home assistant to automatically pick up the values.

rbaron commented 3 years ago

Awesome @kuerb! Thank you for spotting and fixing this.

If you are feeling brave, another nice addition would be to setup a last will & testament message upon connection that publishes "offline" to this topic. I believe we could use SetWill for that (around here).

Let me know if you have interest and time for that, otherwise I will merge this as is.

Thanks!

kuerb commented 3 years ago

Let me know if you have interest and time for that, otherwise I will merge this as is.

Sorry, this is all a bit new for me. I haven't even used Home Assistant or MQTT for anything before this.

rbaron commented 3 years ago

That's alright, your PR looks perfect! I will merge it once we change the retained flag to true. Thank you!

If you're curious about what that means, I can try to explain. As you perfectly figured out, Home Assistant figures out whether a sensor is "healthy" by looking at its status topic. In this case, that's parasite-scanner/status, and if it contains the value "online", Home Assistant considers it to be alive. With your PR (after changing the retained flag to true), the sensor is considered alive forever (since we only ever write "online" there when it connects).

Another technique that goes well with this is using what's called a last will & testament message when connecting to the MQTT broker. This is sort of telling the broker "if you stop hearing from me, publish "offline" to parasite-scanner/status).

This way, if the parasite-scanner dies for whatever reason, the MQTT broker will publish "offline" to parasite-scanner/status and Home Assistant will automatically report it as dead. Pretty cool!

kuerb commented 3 years ago

By using the retained flag true, MQTT will always remember this value, and when Home Assistant is restarted and re-checks this topic, the value "online" will still be there like we want

The online status would be pushed with the very next data point anyways. This depends a bit on the deep sleep interval of the sensors (I currently have it set to a couple of seconds, but with an hourly interval that would be quite annoying). I'll change it to true but can't test it till the evening.

Another technique that goes well with this is using what's called a last will & testament message when connecting to the MQTT broker. This is sort of telling the broker "if you stop hearing from me, publish "offline" to parasite-scanner/status).

With the retained flag to false I was hoping that MQTT automatically sees it as offline after a certain period. Maybe that can be realized with the last will and testament.

So far the online status is applicable to the bridge only. If a single sensor is disconnected, the bridge would still publish data and keep the online status. Maybe it would be a bit more insteresting to add an additional step that sets the intependant sensors as offline if no message was received by a known MAC address after a certain time. That way we could check if a single sensor has an issue or the whole bridge has crashed.

kuerb commented 3 years ago

When changing the retained flag to true, it might be viable to only publish that message once during start up. (And then adding independant availability topics for each sensor during runtime). What do you think?

rbaron commented 3 years ago

To address both your comments, I completely agree- I think we should send the retained online only once upon connection. In fact, that's what your code does already (except it's not retained). The Run() function from MQTTClient is only run once when the program starts. This function is run on its own goroutine, and "blocks" forever on this loop as it waits from messages to arrive, and processes them once they do in the for loop.

So you're on a very good track there, I'd just change the retained flag to true. Let me know if that helps!

As for adding availability to all individual sensors, that would for sure be ideal, but would require some external machinery. This "last will and testament" feature only applies to clients connecting directly to MQTT, so the whole parasite-scanner instead of each individual sensor. For handling each sensor individually, we need another piece of code that keeps a timer running for each sensor topic - when messages arrives, the timer resets. If the timer expires, it posts "offline" to a specific topic. This works and I've used Node-RED for this in the past with some success. We'd also need to update the Home Assistant autodiscovery topic for each individual sensor.

Still, I think we also need the offline/online detector for the parasite-scanner itself, and that's simpler too and good enough in most cases. It's also what ESPHome does, and being compatible is also a plus. Let me know!

kuerb commented 3 years ago

Oh, I see. I thought the whole Run() routine was looped, because the autodiscovery message keeps getting delivered after a reboot of home assistant. But that actually is what the retained flag is doing. Thank you for taking your time explaining all that in detail! That really helps to understand everything.

As for the on/off detector of the bridge I fully agree. I've added a last will message to the config and it seems to be working.

rbaron commented 3 years ago

That looks great. Thanks again @kuerb!