homieiot / convention

🏡 The Homie Convention: a lightweight MQTT convention for the IoT
https://homieiot.github.io/
Other
705 stars 59 forks source link

fix(target): equality of received commands #277

Closed Tieske closed 8 months ago

Tieske commented 11 months ago

This PR is mostly reordering of text to align the description of "Property" (which starts with a bullet list for some reason) with the descriptions of "Device" and "Node".

It has 2 commits;

  1. implements a change requiring byte-by-byte equality of $target
  2. finishes the alignment of property/node/device descriptions

The functional change is that if a device implements a $target attribute to announce an intended/pending change, it should send a value byte-by-byte equal to the received set command. (doesn't apply if the change was initiated from the internal implementation).

This allows a controller to close the control loop and know that the command was successfully received by the device. The byte-by-byte equality makes it easier on the controller side. Since if a device does a color format conversion, a JSON-reencoding, or an int/float rounding, the resulting value might be functionally equivalent, but not byte-by-byte equal, and hence not easily comparable.

Tieske commented 8 months ago

this PR has several issues, it has a double purpose;

  1. closing control loop for a controller
  2. informing about slow moving actors

1. slow moving actors

This can easily turn into a loop. Especially for devices where Homie is just an intermediary representation (eg. a zigbee light controlled through a bridge).

  1. controller send an update (setting target)
  2. the device sends intermediate updates, the Homie device forwards them on the Home bus (setting target again!)

2. target state

due to conversions or calculations the target state may never be reached.

This was clear from the start, but may catch users off guard.

3. closing control loop

if only using for closing the control loop, informing a controller it's command was accepted, then non-retained messages should be used. But that means it is impossible to know before-hand if a device has actually implemented this attribute. Since one would first have to send a command to receive a message on the topic.


so yesterday discused this; agreed to rename to $accepted and only use for control-loop closure. But see 3 above; due to being non-retained, it would require an additional property in the device description. That's something we didn't consider yesterday.

So imho we have 3 options:

  1. drop this all together
  2. keep this PR, but add a warning wrt the possible loop (1 above). So $target may only be updated if the update originates from the Homie network itself.
  3. rename it to $accepted and add a property to the device description.
Tieske commented 8 months ago

PR updated to include notes wrt option 2 above.

So please let me know;

  1. close this PR
  2. accept it
  3. close and replace with a new PR for option 3
schaze commented 8 months ago

PR updated to include notes wrt option 2 above.

So please let me know;

1. close this PR

2. accept it

3. close and replace with a new PR for option 3

I am honestly fine with any option. What's your preferred? Let's go for that one.

Tieske commented 8 months ago

merging it since the warnings are in place