Room temperature - Githubissues

jarrod180 commented 7 years ago

I thought it's a good idea to raise a new issue for improvements or changes to the room temp.

Is there a reason it sends every 60 seconds? I thought the MQTT philosophy is to send when it changes, same way the other settings work. I already modified and tested the following changes: -create callback for room temp modified -separate room temp from the settings struct -trigger the callback when room temp changed with mqtt retain set to true

What do you guys think??

kayno commented 7 years ago

@jarrod180 60s was just a number I chose, and I've seen a lot of MQTT examples where sensor values are sent periodically, rather than only when they change. I know for current settings we don't send them periodically, however I more see these as a "status" of the heatpump, and not a value of a sensor.

I'm not sure on what's the best approach. Reading http://www.hivemq.com/blog/mqtt-essentials-part-8-retained-messages, it says:

A retained message makes sense, when newly connected subscribers should receive messages immediately and shouldn’t have to wait until a publishing client sends the next message. This is extremely helpful when for status updates of components or devices on individual topics.

This seems to match nicely with the heatpump being the device, and it has it's individual topic, which just has a retained message showing the last known status of all the different settings.

The article then goes on to say:

The same is true for clients, which send data in intervals, temperature, GPS coordinates and other data. Without retained messages new subscribers are kept in the dark between publish intervals. So using retained messages helps to provide the last good value to a connecting client immediately.

It talks about "clients" sending data in intervals, and mentions temperature. Not sure if that is justification for periodic updates or not, but I do agree that it would be good to have the message retained on the heatpump/temperature topic, for newly connected clients so they don't have to wait.

I'm not sure either way if it should be a callback for temp or not. I struggle to think of a reason about why it should be different to the settings coming from the heatpump though. That said I also worry about the readability/maintainability/usability of the callbacks, and having many of them might be messy?

One reason I can think to keep it on the 60s (or whatever number, that really should be changed to a constant in the code that the user can set, much like their SSID or topic names) is that it's kind of like a heartbeat for the heatpump - you know it's still working if you have regular updates. Even in the application that I have consuming the temperature topic (home assistant), when I click on the temperature I get a nice little graph and a timestamp telling me "updated 30s ago", so I know it's still communicating with the heatpump.

If you have retained messages and the device dies for whatever reason, there is no way for newly connected clients to know this - they will assume it's ok because the retained message says so, but it would never update. I guess that's a reason to scrap the callback for currentSettings too, and have it start polling to ensure it's still alive!

Or perhaps both settings and temp should poll and have callbacks - periodic 60s polling to ensure it's still alive and make sure a settings change wasn't missed, and also callbacks to catch the settings calls as they happen, e.g. when someone changes the heatpump with it's IR remote control instead. In fact I kind of like this idea - imagine if your wifi dropped out and you missed an update from the heatpump because someone changed the settings with the remote - when the wifi came back up, you'd still be relying on that retained message that was now stale...

Thoughts!?

Does last will and testament help us at all? I haven't looked into this yet, so I might have a read :)

jarrod180 commented 7 years ago

I guess I was failing to see the benefit in sending the temperature constantly, since the retained MQTT message should theoretically always be up to date?? But i can see the merit in getting regular updates. The benefit of a callback for me is also, if the application needs to trigger something immediately on change, it can do that without storing states and looping on the get method.

BUT there seems to be a few scenarios here

ESP publishes heat pump settings, or room temp and then dies Last messages sent with "retain" will be received by any subscriber. Subscriber gets potentially stale data. It looks like the retained message won't be deleted. Stale data might be better than no data at all, I guess ESP should get current settings and publish when it starts.

This seems like a pretty good idea, we should think about adding it:

Best Practices – When should you use LWT? LWT is ideal for notifying other interested clients about the connection loss. In real world scenarios LWT is often used together with retained messages, in order to store the state of a client on a specific topic. For example after a client has connected to a broker, it will send a retained message to the topic client1/status with the payload “online“. When connecting to the broker, the client sets the LWT message on the same topic to the payload “offline” and marks this LWT message as a retained message. If the client now disconnects ungracefully, the broker will publish the retained message with the content “offline“. This pattern allows for other clients to observe the status of the client on a single topic and due to the retained message even newly connected client now immediately the current status.

imagine if your wifi dropped out and you missed an update from the heatpump because someone changed the settings with the remote - when the wifi came back up, you'd still be relying on that retained message that was now stale... Whatever happens the ESP should know the current settings and transmit them, if the ESP wifi drops out, does the MQTT library queue the messages until connected again? If the subscriber wifi drops out and you miss it .... connect back up again and the retained message should be published to you immediately shouldn't it?