Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Retained set commands #150

Open
MofX opened this issue Dec 24, 2018 · 44 comments
Open

Retained set commands #150

MofX opened this issue Dec 24, 2018 · 44 comments

Comments

@MofX
Copy link

MofX commented Dec 24, 2018

Is there a reason, why set command are not allowed to be retained?

When I implemented my first device using mqtt, I liked the possibility to retain the color value send from openhab in mqtt, because my device was able to regain the value after it lost power or connection.

The only homie compliant way to achieve this is using a rule in openhab to detect the device coming back online. Or maybe subscribe to the retained property (without /set), to get the last value.

@ThomDietrich
Copy link
Collaborator

That indeed seems like a not yet covered use case for Homie. Thanks for bringing up this logical issue.

So far the set command is intended as a one-time event to instruct the device. A set command does not (necessarily) define the actual state of the device at a given point, nor should it be valid and wrongfully (?) be re-evaluated at a later point. Because of that reasoning the set topic is non-retained.

The only valid use of a re-evaluated (retained?) set command is the case of "Restore state after disconnect/power-off" as you described it. I don't like the idea to abuse the set topic by sending retained messages though.

We should discuss alternative solutions.

@jamesmyatt
Copy link

Remember that retained messages are only sent by the broker in response to a new subscription. So if Homie doesn't reconnect with a clean session, then the retained messages won't be resent. If Homie does connect with a clean session then any QoS 1 and 2 messages waiting for it will be discarded.

@davidgraeff
Copy link
Member

davidgraeff commented Dec 25, 2018

The property values are retained (or allowed to be retained). A homie device could after initialization restore its internal states by those retained values (if it wants too). Is that not enough?

@ThomDietrich
Copy link
Collaborator

@davidgraeff I believe everyone agrees there. If I am correct, then @MofX's intend was to discuss the use case and define a solution, e.g. in the implementation guideline for clients or in the FAQ. Correct?

@riemp
Copy link

riemp commented Mar 15, 2019

@davidgraeff How can I retain the property values?

homie/devicename/nodename/property/$retained -->true

Is this the way to do it?
It isn't working for me.

@davidgraeff
Copy link
Member

@riemp If it works depends on your implementation and not on the Homie convention. You are in the wrong repository.

The convention states that if $retained is set to true, then the implementation is required to retain the mqtt topic value.

@riemp
Copy link

riemp commented Mar 15, 2019

@davidgraeff
I'm using homie convention in openhab(2.4)autodiscovery

@davidgraeff
Copy link
Member

davidgraeff commented Mar 15, 2019

I'm not taking about the controller. The controller is using the /set topic. I'm talking about your client IoT device esp8266 or whatever.

As you can see in this topic, we agreed that /set topics will never be retained.

@riemp
Copy link

riemp commented Mar 15, 2019

@davidgraeff
I have 3 light devices in my esp32(as 3 nodes --channels ) .and I'm using openhab to control them ,I'm able to make esp discoverable to openhab (as thing /by following homie convention) and also able to control it from openhab
. esp is subscribed the topic homie/devicename/nodename/property/set

it works fine but .but I need my esp32 to go-to the last knwn state while it boots/whenever it reconnect to the broker

So I thought retainig /set topics from openhab is the best way to implement this.

Can u please suggest me any other way to do the same ?!

@davidgraeff
Copy link
Member

Disclaimer: I'm a bit in a bad mood because its raining here in my hometown.

But please RESPECT other peoples time. I could make beautiful open source commitments right now, instead I'm responding to a question of yours that is answered just one page of scrolling up. I quote myself:

The property values are retained (or allowed to be retained). A homie device could after initialization restore its internal states by those retained values (if it wants too). Is that not enough?

@kollokollo
Copy link

I like the idea that one can have Setpoint-Values and Measured-values. Both retained. This would cover an aspect, also mentioned here: https://github.com/kollokollo/MQTT-Hyperdash/blob/master/doc/MQTT-dashgen-naming-conventions.md

Maybe one can bring this together. I do not care if the suffix for a setpoint value is "_AC" or "/set". Also I like the idea to treat topic names as measured values by default (suffix not needed).

@davidgraeff
Copy link
Member

davidgraeff commented Feb 1, 2020

I wouldn't make this part of the core specification because I can think of use-cases where you do not want /set topics to be retained.

Since we do have the extension system in Homie now, I would suggest an "org.homie.retained-set" extension. It should talk about topic priorities (if same /set and / topics differ) and about implications for controllers and devices.

@ThomDietrich
Copy link
Collaborator

ThomDietrich commented Feb 3, 2020

Extensions good and fine, we should not misuse the set topic in this way. Reason given above: #150 (comment)

Imho we should clarify this point (it would logically be wrong) in documentation.

A retained set topic would play the role of an unconditional default value, rather than generating a new value or restoring the last state of a property from the broker topic (depending on the client implementation).
If an unconditional default value is desired as a functionality of a Homie device property this should simply be implemented as an additional property. No change to the convention needed.

@kollokollo
Copy link

Probably there is no change in the convention needed because you can do it anyways with a different topic. It is just a way to make automation easy: The general imperative to any device would be: Try to make the measurement value equal or come as close as possible to the corresponding setpoint value. And therefor the setpoint value must be present (retained) all the time. This could be fundamental to IoT. But maybe its just one way to look at it.

@jamesmyatt
Copy link

Abuse of retained messages is one of my pet hates in MQTT. Their purpose is only for "what should should be sent to device when they create a new subscription to this topic", i.e. when a new consumer wants access to the telemetry. Existing devices restoring their connection will not be sent retained messages that they have previously received. They will only receive messages that were sent while they were offline if the messages are QoS 1 or 2. Existing devices restoring their connection should not be using clean session otherwise those QoS 1 or 2 messages will be lost. Equally they should not be subscribing to their own telemetry topics.

If you want to use MQTT to store the device internal state, then I would create a separate topic that contains the entire device state retained as a JSON or binary message, then subscribe to it only when you want to restore the state. Otherwise, you risk getting inconsistent/incomplete state information from the broker from the command or telemetry topics. This is an implementation detail for the device firmware and nothing to do with the OpenHAB (or equivalent) controller.

@kollokollo
Copy link

Well, there is no (generic) way to poll the last seen value of a topic from the broker other than using retained messages. You could implement an explicit get operation on the device, but this violates the decoupling of consumer from producer. Assume 1000 consumers connect to the broker, they initially do not know about the topics content, so they have to ask the producer device to resent it, individually. This way the load on the producer depends on the number of consumers (like on server-client models). One of the strengths on MQTT is that this is not necessary.
Storage abuse is normally handeled by the broker using an overal or per-user quota. That looks OK to me.

@jamesmyatt
Copy link

jamesmyatt commented Feb 4, 2020

Well, there is no (generic) way to poll the last seen value of a topic from the broker other than using retained messages.

It's fine to use retained messages to provide the last value to new consumers, where it makes sense. But MQTT is designed to be a lightweight message queue not a database.

Also, the set command has only one consumer---the target device---so once it's been processed, there's no need to keep it.

Finally, if there's is information that a device needs storing in order to work correctly, then that device should take responsibility for ensuring that it's stored, and not rely on third parties storing it correctly for them. For example, there's no guarantee that the command message is even valid, let alone retained.

@ThomDietrich
Copy link
Collaborator

ThomDietrich commented Feb 4, 2020

Thankas @jamesmyatt.
So in summary, how do we think should we document this topic, or does it need any documentation at all? Imho every topic coming up as an issue must be addressed in documentation, otherwise it will come up again and again.

@kollokollo
Copy link

kollokollo commented Feb 4, 2020

Also, the set command has only one consumer---the target device---so once it's been processed, there's no need to keep it.

But what if the device itself gets offline and then reconnected? It needs to find out what the setpoint should be. How can he do this, not knowing who did the last /set command?
If it has sored the last seen value by itself on the device FLASH memoy, OK, but what if the /set was changed during the time it was not connected? (assuming qos0)

@jamesmyatt
Copy link

jamesmyatt commented Feb 4, 2020

Also, the set command has only one consumer---the target device---so once it's been processed, there's no need to keep it.

But what if the device itself gets offline and then reconnected? It needs to find out what the setpoint should be. How can he do this, not knowing who did the last /set command?
If it has sored the last seen value by itself on the device FLASH memoy, OK, but what if the /set was changed during the time it was not connected? (assuming qos0)

If the message was sent while the device was offline, then it should use QoS 1 or 2. If it's QoS 0, then you can assume it wasn't important and you're still meeting requirements by dropping the message. You also may not receive a retained QoS 0 message when reconnecting unless you remove the subscription (unsubscribe or "clean session") and resubscribe to the topic.

If the message was sent while the device was online, then you ought to store the state yourself. FLASH/EEPROM would be the most reliable way since you're in total control of that. But if you can't do that (no non-volatile storage or you want to limit the number of writes), then I'd suggest a separate internal state topic as discussed above (#150 (comment)); although, there's no guarantee that a third-party won't interfere with it.

It's also worth remembering that plenty of devices have multiple interfaces, e.g. MQTT + physical. Hence, if you use a retained /set message, then a physical button (or timer or HTTP API or whatever) to change the state to something else, then the MQTT set message now wrong. Equally, the contents of the /set message might not directly reflect the actual status, e.g. your command could be "toggle" (although that's probably bad practice anyway since it's not idempotent, but you get the idea) and it would make no sense for restoring.

@kollokollo
Copy link

Ok, I agree. But just for the fun of discussion: If the device also had a hardware button to change the setpoint, why not also publish itself to /set. This way it would be consistent, and in addition reflect the state of the hardware button. I know, there are many ways to do it. Maybe looking at real-life examples?

@ThomDietrich
Copy link
Collaborator

@kollokollo you are circling. Let's establish that the set topic is only written to to command the device (no retained state purpose) - why would the device itself bother to do so?

Once again: If you are interested in a default value or for your purpose a retained setpoint, define a dedicated attribute for it. That is by far the cleanest way to stay within the logical framework

@jamesmyatt
Copy link

jamesmyatt commented Feb 4, 2020

@ThomDietrich , there seems to be a general lack of MQTT best practice guides, as far as I can tell. Might be worth having separately from the Homie site.

@ThomDietrich
Copy link
Collaborator

ThomDietrich commented Feb 4, 2020

Excellent! I was just about to propose that we should start documenting these "Why does the convention do thing X like that and how am I supposed to solve use case Y around that?". This should imho be a document next to the convention:


MOVED to #51

@ThomDietrich
Copy link
Collaborator

We already started to discuss this over in #51

@mjcumming
Copy link
Contributor

For my use case, set commands cannot be retained. My lighting is connected to OpenHAB via Homie. If the connection between OH and my lighting controller is down and lights in the home are controlled manually, and then the connection is restored, any set commands are resent and the lights are changed to whatever OpenHAB last set to them. Not desirable.

@Thalhammer
Copy link
Member

@mjcumming Not sure but if the physical state of your home and the virtual state are not in sync that sounds like a design error to me. Your light switches should update OpenHAB (I know that might be hard to do).

@piegamesde
Copy link
Contributor

@Thalhammer The above example is sub-optimal because of this, yes. But there are a lot of other similar cases that are still problematic even if the state is always in sync.

Say I have a music player, and I want to play a song. But the music player fails to do so for some reason and thus updates the current playing item to none. If /set-messages are retained, the state could look like this:

  • homie/pi/player/song → none
  • home/pi/player/song/set → foo.mp3

If now the music player reboots or reconnects for some other reason, it will receive the song command again and try to play that file. If it succeeds this time, it will suddenly and unexpectedly start playing music, possibly hours later.

@mjcumming
Copy link
Contributor

@Thalhammer the issue occurs when there a loss of connection between the device and the controller (OpenHAB). If that connection is lost and then restored, any set messages from OH will be delivered and this behavior (for my use case) does not work as the state of the device may have changed and the set command from OH is no longer valid.

@mjcumming
Copy link
Contributor

We need to solve for essentially (I think) 2 use cases when there a disruption in communication between a controller and a device....

  1. The controller is the source of truth and the device should always set it self using last set command from the controller
  2. The device is the source of truth and any set command from a controller is only valid for a short interval

Does that cover all the relevant scenarios?

@Thalhammer
Copy link
Member

I think so.
At least I cant think of any other scenario right now.

@mjcumming
Copy link
Contributor

Reading through the comments of @ThomDietrich and @jamesmyatt we should keep the /set command as non retained as per the convention. I think we should also specify that /set messages are to be sent with a QoS of 0.

Controllers can monitor the result of a set command by checking the value of the property and determine actions to take if the /set command does not result in a value change. This is where that logic belongs - not on the device itself.

We still need to come up with a scheme for devices where it is desirable to restore their state to the last command sent from the controller. This could be done without changing anything, as the controller knows if a device is or isn't online and when a device reconnects with a $state ready message. That should trigger the controller to use a /set command to restore the device's state.

Another option, which I like less, would be another command topic /restore, with retained and QoS of 1 or 2, which devices could subscribe to and controllers could set as needed.

@piegamesde
Copy link
Contributor

This could be done without changing anything, as the controller knows if a device is or isn't online and when a device reconnects with a $state ready message. That should trigger the controller to use a /set command to restore the device's state.

I really like this idea, but what should happen if a controller doesn't do this? Can devices depend on this behaviour? Or should they use sane defaults instead? We could make that option more explicit by adding $state = restore or so (but I'm not sure if that's a good idea, and it would leave us with devices not working properly if the controller doesn't support this feature).

@mjcumming
Copy link
Contributor

A controller should know enough about the devices it controls that on reconnect, a device needs it's state set. Also, devices (when programmed) will know if they can determine their own state or need to set startup/default values. This isn't something the convention should determine.

As mentioned here we need to document these discussions either in the convention or an FAQ

@Thalhammer
Copy link
Member

A device should always use sane default values.
Also I think we dont need a $restore topic. A device can just subscribe to its property value (which is retained) so it should get the last state it published after a reconnect. It can then use that value to restore the state if it wants to.

I still think we should allow sending set as a retained topic though as it makes implementation of controllers extremly simple if the controller is your single point of truth (which is often the case).

@mjcumming
Copy link
Contributor

@Thalhammer - I feel we really need a different solution. This would break my implementation and change the convention as outlined in 5.3.2. Are you using a custom controller or OpenHAB?

@Thalhammer
Copy link
Member

I use an all custom homeautomation system I wrote from scratch a couple of years back.

How would allowing retained sets break something ?
A device should be ready to receive a set at any time and it cant differentiate between a set stored as a retained and a set sent right at that time by the controller.
I am not saying we should require them to be sent as retained. Just that we should allow them being sent as retained. Could you explain how you think it breaks something ?

@mjcumming
Copy link
Contributor

I have about 100 homie light/switch devices that are controlled by OpenHAB and also local control. So the source of truth about device state is the actual device, not the controller. If the connection between the device and controller breaks, on reconnect all the set commands get sent again and the lights will change - not desired behavior.

How would a device determine if the set message was "old" or current?

@Thalhammer
Copy link
Member

In that case you would not send as retained. Thats why it would be allowed and not required. There was a discussion about using an optional attribute to indicate whether retained should be used.

@mjcumming
Copy link
Contributor

mjcumming commented May 5, 2020

For me the cons of allowing set to be retained are:

  1. From a controller perspective (with OpenHAB probably being the most frequently used), there would need to be a way to set the retain flag in the UI depending on the device being controller. We would need someone to change the binding (I don't do OH development).
  2. Existing Homie device implementations might be broken with this change.
  3. The controller can already solve for this problem by sending a set command after the device reconnects.

That being said, having the default be not retained but allowing retained may solve for # 2 above.

@mjcumming
Copy link
Contributor

@Thalhammer how will the controller know when it is allowed to use retained set commands?

@Thalhammer
Copy link
Member

Thalhammer commented May 13, 2020

The original idea was to have an optional attribute which if present indicates how the controller should behave. So the default would be the behaviour as it is currently defined in the convention (thus keeping existing devices compatible) and can be changed by broadcasting the attribute.

But I am completely to any other idea on how to handle it.

@qoole
Copy link

qoole commented Jan 18, 2021

Hi all,

I'm quite new to the Homie convention and in general it has been a great help in rapidly setting up a few random IOT devices.
However I find the inability to have a retained set topic limiting. I believe that fixed either one way or another is not correct however an option on a property '$retainset' perhaps might be a good way forward.

I have an ESP32 device that I want to be able to sleep and periodically wake up to check for updates to a topic... currently that is not possible.

You have included the ability to 'Sleep' in the convention, surely that is rendered useless if, when a device wakes up, it doesn't receive the most current 'set' command.

Yes, there are absolutely times you don't want the set command to be retained. However to not have the ability for the developer to pick and choose seems a little short sighted and opinionated.

EDIT:
I also notice that this fundamentally changed between v3 and v4 of the convention
in V3.0.0 and V3.0.1 the following is true:
image
This then changed in V4:
image

So amusingly Openhab (which proports to abide by v3) uses a non-retained /set topic.

I wonder what would be simpler, getting Homie v4 onwards changed or get Openhab to correctly abide by v3....

EDIT 2:
I decided to find the pull request to see if I could find the rationale behind this change
Pull request: #145

"Added the hint that controllers publish to "/set" only with non retained messages." - this is a complete 180° change from "This especially fits well with MQTT, because of retained messages" to "non-maintained messages only".

It seems like a change to the convention that was glossed over to go from retained to non-retained on "/set" without any reasoning.

@davidgraeff
Copy link
Member

"This especially fits well with MQTT, because of retained messages" was directed at the value topic, not the /set topic. V4 just clarifies that misunderstanding.

This is why we have this issue here, to discuss how to progress. But no idea found enough supporters for a pull request yet.

You can of course just create one and let people decide with a concrete suggestion.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

10 participants