rkoshak / sensorReporter

A python based service that receives sensor inputs and publishes them in various ways.
Apache License 2.0
105 stars 41 forks source link

MQTT — Retain Messages #81

Closed jamesgoodhouse closed 2 years ago

jamesgoodhouse commented 3 years ago

I am using MQTT to publish message information about a sensor. I see that the "retain" parameter is hardcoded to False. This is causing issues as the sensor reporter code only publishes to MQTT when there's been a change in status for a sensor. This means that anything consuming the topic will have no idea of the status of the sensor should it disconnect and reconnect later. Is this the intended behavior? What are your thoughts on possibly supplying an optional Retain1 parameter to go along with the Address1 and Destination1 parameters for a sensor?

rkoshak commented 3 years ago

I'm OK with there being an option to choose whether a not a message is retained. But it needs to be done on a sensor-by-sensor basis which makes it a bit bigger of a job. I've had it on my todos to add that but it's a little bit bigger of a problem than just changing the one MQTT class which is why I haven't done it yet.

Part of the issue is that I have a common interface for both the MQTT Communicator and for the openHAB RST API Communicator. Adding a new retain flag to the sensors would mean adding a new MQTT specific parameter to an interface that is a generic interface used by both Communicators. I've not thought of a good way to do that yet in a way that makes sense and keeps knowledge of which Communicator is being used out of the individual Sensors and Actuators.

This is largely why I've not done this already.

Ideally the QOS should also be configurable in the same way.

And if we can figure out a reasonable way to do this, we could potentially fix the Govee sensor so it properly supports the openHAB Communicator.

jamesgoodhouse commented 3 years ago

That makes sense. I started digging through the code some to see if there was a quick win but soon realized there likely wasn't. I'm happy to keep digging a bit and if I come up with anything I'll certainly open a PR.

rkoshak commented 3 years ago

Good luck! It'll be a nice addition.

DanielDecker commented 2 years ago

With the merge #97 this issue is now fixed. Any sensor and actuator an specify the Retain flag in the Actuator / sensor relevant parameters I guess the issue can be closed.