AnaviTechnology / anavi-thermometer-sw

Open source Arduino sketch for the smart WiFi dev board ANAVI Thermometer
GNU General Public License v3.0
27 stars 17 forks source link

Better handling of unavailable values #38

Closed cederlys closed 4 years ago

cederlys commented 4 years ago

Currently, the ANAVI Thermometer publishes all measurements using MQTT with the retain flag set. I'm not sure that is a good idea. If a Thermometer crashes or loses power, the MQTT broker will continue to publish the last seen value, and it might not be obvious at first if a value is stable or stale.

Perhaps it should even publish a will topic so and configure a availability_topic in the MQTT Discovery message, so that Home Assistant can detect when the sensor goes away.

I hope to be able to play with these ideas in the near future and might be able to issue a pull request next week, if things work out. Feedback welcome if somebody has any ideas regardging this.

leon-anavi commented 4 years ago

Hi @cederlys,

Yes, this is a valid and very good point.

The retained values are useful in the common use case when the app reading the sensor data (for example Home Assistant) connects to the MQTT broker after the data has been published.

The idea to use an MQTT Last Will and Testament (LWT) message is really nice. I like it and we definitely need it. It will be great if this can be implemented in a way to purge all retained messages from the sensors when LWT is received. I don't know if there is a straight-forward way to do it in Mosquitto. I will explore the documentation. Of course a simple additional script on the server side can easily take care on doing this.

Thanks, Leon

P.S. I sent you a private email a couple of hours ago. Hope it didn't went into SPAM folder or anything like that :)

jduff001 commented 4 years ago

I would like Home Assistant to somehow identify that either (1) the time value last changed, or (2) the LWT. Since stale / retained values usually mean I have lost the wifi connection to that device

cederlys commented 4 years ago

I've now created an experimental branch named will-null-empty with 3 commits. The first one sets up an availability topic and an MQTT will that sets it to offline when the thermometer disconnects. The second commit tries to remove the DHT22-derived values when the DHT22 sensor isn't available. The third instead sends null values when the DHT22 sensor isn't available.

The first commit works fine. The second and third both have drawbacks, as outlined in the commit messages.

The branch is available here: https://github.com/cederlys/anavi-thermometer-sw/tree/will-null-empty. Comments welcome; more work is needed before this is ready to be merged to master.

cederlys commented 4 years ago

I have two ideas that I intend to explore.

  1. Add a new status topic that is used for the MQTT sensors that depend on the DHT22. The ANAVI Thermometer would set that to offline when DHT22 isn't available and online when it is available. Add an automation in Home Assistant that sets it to offline whenever the main status topic is set to offline.

  2. Like 1, but without the automation. Instead, let the ANAVI Thermometer have two connections to the MQTT broker, so that we can have two wills: one for each status topic. The new connection would only be used to get a second will.

jduff001 commented 4 years ago

I am trying an automation trigger on availability topic .. did not seem to respond, so I am trying the nul values... but since it is variable value of "0" that doesn't seem right. I am a beginner on Home Assistant automations, maybe if the entity value on disconnect had some "fail" value like 999 ?

cederlys commented 4 years ago

I've now managed to set up two availability topic and an automation that sets dht22status to offline whenever the status is set to offline (which it will be by the MQTT will). Seems to work. See https://github.com/cederlys/anavi-thermometer-sw/commit/d6257e360912271d661f4a2305abd307bc1d57f8 where you can see the automation.

Now, let's see if it works to have two MQTT connections. I don't really like the idea of having to set up an automation for each of my ten ANAVI Thermometers.

cederlys commented 4 years ago

It seems to be possible to use two MQTT connections: https://github.com/cederlys/anavi-thermometer-sw/commit/8af6f7091e84cd46ed93d4c3e172a3b8262ddd23. I have obviously not tested this a lot yet, but initial tests are promising. I think this might be the best way forward.

cederlys commented 4 years ago

Two connections only gets you so far. I have now pushed a new commit to the will-null-empty branch (https://github.com/AnaviTechnology/anavi-thermometer-sw/compare/master...cederlys:will-null-empty) that uses a separate MQTT connection for each sensor. I like how this works, but it has a drawback: it uses some heap memory for every connection, so if you have a lot of sensors connected it could potentially become a problem. Especially if we were to add support for connecting more than one DS18B20 sensor, this could become an issue.

I now see three possible methods. N in the table below is the number of connected sensors, plus one for the ESP8266 board itself.

Number of MQTT connections Number of status topics Remarks
N N Works fine, but requires heap memory for every sensor.
1 N Supports hotplug of sensors, but requires an external entity (perhaps a Home Assistant automation) to send "offline" messages for each sensor when the esp8266 goes offline.
1 1 Does not support hotplug of sensors. If you have ever had a particular sensor connected, you must manually remove the Discovery message and probably also remove and re-add the MQTT Home Assistant integration to get to a sane state.

Personally, I like the N + N method, and as long as the number of sensors is small it seems to work fine.

Opinions? Should I add #ifdef machinery for supporting the 1+N and 1+1 methods as well?

jduff001 commented 4 years ago

Could this multi-sensor / multi-connection option be applied to the anvai board that has the dht plus a gas dectector. https://github.com/AnaviTechnology/anavi-gas-detector-sw ?

My home application would be limited to a maximum of 10 esp3266 boards connecting to my local mqtt server (mosquito in an rpi-3). Would that overload the heap stack memory ?

cederlys commented 4 years ago

Could this multi-sensor / multi-connection option be applied to the anvai board that has the dht plus a gas dectector. https://github.com/AnaviTechnology/anavi-gas-detector-sw ?

I don't have that hardware, so I don't know. But I guess it could. I won't port the code to that sketch, though, but I would not mind (quite the opposite, actually!) if somebody else did it.

It might make sense to let this task be finished first, though.

My home application would be limited to a maximum of 10 esp3266 boards connecting to my local mqtt server (mosquito in an rpi-3). Would that overload the heap stack memory ?

Does the mosquitto server have such a limit? I don't quite understand your question. The heap stack memory required on the ANAVI device corresponds to the number of sensors that are connected to that particular device. How many other ANAVI devices are connected to the same MQTT broker does not affect that. The load on the MQTT broker increases with each connection, but I would assume (I might be wrong...) that any decent broker it could handle thousands of connections.

leon-anavi commented 4 years ago

@cederlys, thank you for the hard work and leading this important public discussion. According to me "1+N" is the preferred option because it keeps things simple (just a single MQTT connection) and in theory ANAVI Thermometer can have 4 external sensors (even 5 if the I2C display is replaced by a sensor).

I think the an MQTT watchdog can be implemented on the server side (may be a Home Assistant add-on) to remove retained MQTT messages from a particular device (based on the machine-id in the topic) when LWT message is received.

@jduff001, regarding the limits in Mosquitto: in mosquitto.conf you can set max_connections to -1 to have "unlimited" connections. For more details have a look at: https://mosquitto.org/man/mosquitto-conf-5.html

Thanks, Leon

cederlys commented 4 years ago

I've pushed yet another commit to the will-null-empty branch. It has support for modes 1+1, N+N and 1+N. 1+N is the default. So far, I've only tested the N+N mode, and only made a few tests. But even with 3 external sensors, there seems to be more than 26000 available bytes on the heap.

I will do more testing, hopefully during the upcoming week. If the other modes also work, I will perform a Git history cleanup and then submit a pull request. I can't promise I will be done in a week, but that is the time-frame I'm aiming for.

At the moment, I have 10 ANAVI Thermometers running in the N+N mode, and mosquitto on a Raspberry Pi (running Home Assistant) seems to cope with the load. Most of the devices don't have any external sensors, but I have a total of 4 external sensors connected.

There is one caveat: I use 3-character machineId. The default 32-character machineId uses more memory. I don't know how much that matters in this setup. Probably not much.

cederlys commented 4 years ago

I've fixed a few bugs, squashed the changes to a single commit, and created a new pull request. This time, I've tested all the 3 modes, and as far as I can see they all seem to work. The 1+N method is the default, but personally I think the N+N method is better. I have used it with 3 external sensors attached, and it seems to work just fine.

leon-anavi commented 4 years ago

@cederlys thank you for the huge hard work to add this new fantastic feature. The GitHub PR has been merged.

If you prefer setting N+N method as a default I will be happy to switch to it. With open source at the end of the day we have the flexibility to try all solutions and see what works best in practice.

Thanks again fro this feature!

Thanks, Leon

leon-anavi commented 4 years ago

Hi,

Recently I have been browsing Home Assistant documentation and I found an article about "MQTT Birth and Last will": https://www.home-assistant.io/docs/mqtt/birth_will/

I don't understand how it exactly works. I guess I need to have a look at the code and just try it. According to some forum posts it should be useful for use cases as the one discussed in this thread: https://community.home-assistant.io/t/set-state-of-sensor-to-unavailable-after-not-reporting-for-x-time/110381

Has anyone use it?

Best regards, Leon

jduff001 commented 4 years ago

Maybe this is related to / or fixed with merged #34 , sometimes the dht22 stops reporting, but the attached ds18b20 continues. It is inconsistent, so I never what is current unless I look at the history graph and look for straight lines with no recent changes.

cederlys commented 4 years ago

A "birth" message is just any message that a device sends as soon as it gets connected. You could call most of the MQTT Discovery messages birth messages. Also, the "online" messages that are sent to the +/+/status/+ topics can be considered birth messages.

A "last will", "will", "LWT" or "Last Will and Testament" are all the same thing. See 3.1.2.5-3.1.2.7 and 3.1.3.2-3.1.3.3 in the MQTT specification (http://docs.oasis-open.org/mqtt/mqtt/v3.1.1/os/mqtt-v3.1.1-os.html#_Toc398718028). The will is set up as part of the CONNECT message, which is the first message the MQTT client sends to the server when establishing a connection.

Pull request #39 ensures that the ANAVI Thermometer sets up a will (or several wills, in the N+N scenario), so that the status topic(s) are set to "offline".

https://www.home-assistant.io/docs/mqtt/birth_will/ talks about how you can get Home Assistant itself to set up birth and will messages, so that other MQTT clients can know if Home Assistant is currently running or not. It could perhaps make sense for a Miracle Controller to turn off the light if it detects that Home Assistant is offline. I don't know if the ANAVI Thermometer can do anything useful with that information, though.

jduff001 commented 4 years ago

I was researching the home-assistant community io group. could this be useful ?
https://community.home-assistant.io/t/esp8266-and-ha-lwt/31843 https://community.home-assistant.io/t/send-device-offline-notifications-based-on-mqtt-lwt/155824