homieiot / convention

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

Specification is unclear on empty string for property values #121

Closed boc-tothefuture closed 5 years ago

boc-tothefuture commented 5 years ago

From reading the specification it is not clear to me if the convention supports empty/blank strings for property values.

Using the example: homie/super-car/engine/temperature → "21.5" Does the convention support a later message of: homie/super-car/engine/temperature → ""

This should be clarified, because if it is allowed, the controller implementations need to handle blank/empty strings.

In most cases it would work for Strings. I don't know what numbers would do, and I am not sure what enums would do either. It would essentially be "unsetting" that value.

I am coming across it in my kotlin device implementation as I consider what to do if the user passes null value.

It seems complicated to allow this - but right now, unless I missed it, the convention doesn't address this issue one way or the other.

davidgraeff commented 5 years ago

An empty value except for strings is undefined behaviour. Properties can only be unset by setting the $properties attribute of the parent node.

boc-tothefuture commented 5 years ago

Should we state in the convention that an empty string (besides string type) is disallowed?

davidgraeff commented 5 years ago

You cannot disallow it unfortunately. Empty is the same as if the topic value is not published at all. And because of timing and order issues, it can happen that the property type is already published (let's say integer) and the value is not published yet. This is just undefined behaviour and a controller implementation must handle the situation gracefully.

boc-tothefuture commented 5 years ago

@davidgraeff - Can you expand on that a bit.. I am afraid I am not following how empty is different than never published? If the value is not published yet, you wouldn't receive anything on the controller side? If the value was published an an empty string is then published you would receive the empty string.

davidgraeff commented 5 years ago

@boc-tothefuture What do you do in your controller implementation if everything but the actual property value has been received? Do exactly the same for an empty received property.

boc-tothefuture commented 5 years ago

@davidgraeff - It probably depends on the actual controller implementation. There could be some that do nothing (don't expose the received property) until they receive the first value for the property. Doing that would be within the spec, AFAIK.

What would be the counterpoint to disallowing, per spec, the sending of empty strings for non-string values?

My opinion is that we should attempt to make the convention as clear as possible on what is allowed to be transmitted. Items that lead to undefined behavior should be minimized to enable as broad of compatibility between controllers and devices as possible.

davidgraeff commented 5 years ago

An integer property is allowed to publish integers only, a boolean one true, false and so on. I don't really understand the problem. If you publish an empty string it is undefined behaviour, I don't think we need to explicitly say this, do we?

boc-tothefuture commented 5 years ago

I think additional clarity can only help. Someone writing an implementation may think its ok to send empty strings to "unset" or delete a value.

BTW, to your point above. I can't find where in the convention do we state that a boolean is true and not TRUE or not "1". You can intuit it is "true or false" by the examples but we don't state it.

I am new to the convention, and raising issues as I develop a client without preconceived notions and exposure to the history of the binding. Some of it may seem obvious to those with a long history in the convention, but as more people work with the convention it may be important to clarify these points.

Alternatively, it could be that the convention prioritizes conciseness over completeness/precision. Which is fine, but may hinder interoperability.

davidgraeff commented 5 years ago

https://github.com/homieiot/convention/blob/develop/convention.md#payload "If a value is of a numeric data type, it MUST be converted to string. Booleans MUST be converted to "true" or "false""

davidgraeff commented 5 years ago

We can change the sentence to "A numeric data type must contain a numeric value published as string with the decimal separator, if any, being a dot".

boc-tothefuture commented 5 years ago

Thanks, I missed that. I missed it because we refer to it as a value in the property section and payload above.
Let me put a PR or two together and we can discuss in there what makes sense. I am not going to open a PR until #120 is merged. As #120 touches a large section of the convention and I don't want to cause conflicts.

davidgraeff commented 5 years ago

Valid point. It should be referred to as "payload" in all sections.

davidgraeff commented 5 years ago

@boc-tothefuture If you edit the payload section only, you can make a PR anyway :)

davidgraeff commented 5 years ago

Would you say this is fixed with #127?

boc-tothefuture commented 5 years ago

Can we leave this open for a bit? I will put in a PR for discussion and consideration to further define acceptable payloads. It may take me a day to prepare it.