Added Attributes to node properties #27

gorootde · 2017-07-31T11:55:37Z

In order to releave @marvinroger a little bit, I've created a PR which contains the changes discussed in #12 . Let's proceed with the discussion directly in the PR

ThomDietrich · 2017-07-31T12:32:55Z

Hey @Kwave,
thanks for taking up this task!
Big coincidence, I've developed an MQTT speaking daemon that uses "Homie Convention v2.1.0-alpha" 😃 just this weekend and I was able to gain some head-on experience.

Not a complete review but a few early additions:

Increment the version number
I believe you missed to add homie/device-id/$nodes to the list of "Device Properties"
I realized that "$name Friendly name" should also be available for Nodes
The required flag should not be Yes as often as it is currently. The settable property should for example have a default false nature instead of requiring the topic. Other topics are similarly not needed with a proper default.
You've chosen a different regex format. Any reason why?
You've changed the example listing to read $unit → LED Status. I'm not sure of this is correct. I'd say the status is unit-less and "LED 2 Status" is published to homie/ledstrip-device/ledstrip/led_2/$name
In the README the convention structure is confusing. Maybe you could rename the section "Device properties" to "Device" and describe the deviceID topic as well as all homie/deviceID/$keyword topics under this section. The same goes for "Nodes" and for "Properties". As I mentioned in the original issue I also don't like the confusion around Property and properties. My suggestion would be that "Property" should only be used for the hierarchy level under Nodes (e.g. temperature) and the special .../$keyword topics should be addressed with "parameter", "meta-topic" or similar

euphi · 2017-07-31T16:02:01Z

README.md

+            <code>%</code> Percent<br>
+            <code>m</code> Meter<br>
+            <code>ft</code> Feet<br>
+            <code>#</code> Count or Amount


I miss a recommendation for (air) pressure. I propose Hectopascal (Unit hPa), because it is a (derived) SI-unit, other than Bar or mBar. You may add "PSI" as imperial unit.

I've added Pascal Pa(all other units are also listed in their in basic unit) and psi.

euphi · 2017-07-31T16:05:45Z

README.md

+homie/ledstrip-device/ledstrip/led_3/$settable → true
+homie/ledstrip-device/ledstrip/led_3/$unit → LED Status
+homie/ledstrip-device/ledstrip/led_3/$datatype → enum
+homie/ledstrip-device/ledstrip/led_3/$range → on,off
 homie/ledstrip-device/ledstrip/led_3 → on
 ```



The following (unchanged) section is inconsistent with the LED example. I like the new definition with "On" or "Off" as state, so I propose to update the following section to make use of it. (No more "On"-state that is set to true or false).

Why is it inconsistent? It describes how a client can set a property of the homie device, and how the device gives feedback about whether the value has been processed or not. Please clarify.

The next section in README.MD is unchanged and thus somehow inconsistent:

Homie is state-based. You don't tell your smartlight to turn on, but you tell it to put it's on state to true. This especially fits well with MQTT, because of retained message. For example, a kitchen-light device exposing a light node would subscribe to homie/kitchen-light/light/on/set and it would receive: homie/kitchen-light/light/on/set ← true

According to your propasal it is better to use

homie/kitchen-light/light/state/set ← on

agreed. Alternatively homie/kitchen-light/light/power/set ← on

Agreed too. @Kwave feel free to choose between state and power. 😉

Ok done. Replaced "on" with power in all topics.

euphi · 2017-07-31T16:06:59Z

README.md

+       <td>Device → Controller</td>
+       <td>Describes the format of data.</td>
+       <td><code>integer</code>, <code>float</code>, <code>boolean</code>, <code>string</code>, <code>enum</code> </td>
+       <td>Yes</td>


Format of boolean should be defined. (What is allowed, is it case sensitive?) "true" and "false" or also "TRUE", "FALSE", 0, 1?

Good point. I've added this to the datatype.

gorootde · 2017-07-31T19:11:48Z

Fixed most of the stuff mentioned above.

I realized that "$name Friendly name" should also be available for Nodes

I've added this for nodes and also properties.

The required flag should not be Yes as often as it is currently. The settable property should for example have a default false nature instead of requiring the topic. Other topics are similarly not needed with a proper default.

I thought the same when writing down the changes. But there is one caveat that came to my mind: MQTT is message based. If you subscribe to ´property/#` you will receive a single message for each retained topic in any order. So how does the client know whether an attribute is not present (--> Use the default!) or if the retained message is about to be delivered later (--> Use the attribute value instead of the default). Any Ideas?

You've chosen a different regex format. Any reason why?

Your suggestion was lacking the possibility to add flags to the pattern. Thats all ;-)

In the README the convention structure is confusing. Maybe you could rename the section "Device properties" to "Device" and describe the deviceID topic as well as all homie/deviceID/$keyword topics under this section. The same goes for "Nodes" and for "Properties". As I mentioned in the original issue I also don't like the confusion around Property and properties. My suggestion would be that "Property" should only be used for the hierarchy level under Nodes (e.g. temperature) and the special .../$keyword topics should be addressed with "parameter", "meta-topic" or similar

I've changed the naming. Every subtopic starting with $ is called "attribute" whereas the level below a node is still called "property"

ThomDietrich · 2017-07-31T20:16:12Z

README.md

@@ -35,14 +35,17 @@ An ID MAY contain only lowercase letters from `a` to `z`, numbers from `0` to `9
 To efficiently parse messages, Homie defines a few rules related to topic names. The base topic you will see in the following convention will be `homie/`. You can however choose whatever base topic you want.

 * `homie` / **`device ID`**: this is the base topic of a device. Each device must have an unique device ID which adhere to the [ID format](#id-format).


ThomDietrich · 2017-07-31T20:34:48Z

README.md

@@ -18,7 +18,7 @@ You can find an implementation of the Homie convention:

 ## Background

-An instance of a physical piece of hardware (an Arduino, an ESP8266...) is called a **device**. A device has **device properties**, like the current local IP, the Wi-Fi signal, etc. A device can expose multiple **nodes**. For example, a weather device might expose a `temperature` node and an `humidity` node. A node can have multiple **node properties**. The `temperature` node might for example expose a `degrees` property containing the actual temperature, and an `unit` property. Node properties can be **ranges**. For example, if you have a LED strip, you can have a node property `led` ranging from `1` to `10`, to control LEDs independently. Node properties can be **settable**. For example, you don't want your `degrees` property to be settable in case of a temperature sensor: this depends on the environment and it would not make sense to change it. However, you will want the `degrees` property to be settable in case of a thermostat.
+An instance of a physical piece of hardware (an Arduino, an ESP8266...) is called a **device**. A device has **attributes**, like the current local IP, the Wi-Fi signal, etc. A device can expose multiple **nodes**. For example, a weather device might expose a `temperature` node and an `humidity` node. A node can have multiple **properties**. The `temperature` node might for example expose a `degrees` property containing the actual temperature, and an `unit` property. Node properties can be **ranges**. For example, if you have a LED strip, you can have a node property `led` ranging from `1` to `10`, to control LEDs independently. Node properties can be **settable**. For example, you don't want your `degrees` property to be settable in case of a temperature sensor: this depends on the environment and it would not make sense to change it. However, you will want the `degrees` property to be settable in case of a thermostat.


The example is not a good fit anymore. Unit is now it's dedicated attribute after all.
To illustrate the usage well we need more levels.

I'm not an expert on weather stations but how about something along this example:

homie/

weatherstation/

central/

temperature

humidity

windsensor/

direction

speed

coffeemachine/

...

...Plus all the additional attributes

Good point.

I've modified the example to be more precise. Basically added a new property for the battery level to the example.

Be aware that you missed out on the "central" and "windsensor" nodes. With the text as it is right now the meaning of nodes and properties doesn't get clear.

ThomDietrich · 2017-07-31T20:41:02Z

README.md


-* `homie` / **`device ID`** / `$` **`device property`**: a topic starting with a `$` after the base topic of a device represents a device property. A device property MUST be one of these:
+### Device attributes


My suggestion was to move the sentence from above regarding the device below this headline and rename the headline to "Device". For the new reader it is pretty confusing that there is a headline e.g. "Node ...." and it immediately addresses attributes while the Node is not yet discussed. I'd suggest four big headlines "Homie Base", "Devices", "Nodes", "Properties".

I've added the heading "Devices", containing everything that describes devices in general

Thanks but it's still not how I imagine it. Feel free to ask if my suggestion wasn't clear. Also I'm not saying that my suggestion is the only and best solution, so let me know if you disagree with anything ;)

ThomDietrich · 2017-07-31T20:49:35Z

One general remark that might go beyond your PR: As you can see git management of long paragraphs is rather unfitting. Because of that it's often recommended to break down paragraphs into "one sentence per line" code. The markdown result will be the same but the git management of the document is better.

marvinroger · 2017-08-01T07:10:10Z

README.md

@@ -4,7 +4,7 @@

 **Please note this v2 branch is a work-in-progress. It might change before the final release.**

-Version: **2.0.0**.
+Version: **2.1.0**.


As written in the above line, the v2 is not yet final, so we don't even need to bump the version. 👍

I would suggest to version the smaller increments anyways. As the final version is still a v2, what about 2.0.1 for the changes discussed here?

I would say no, but as the v2 is already widely used, let's keep it this way.

marvinroger · 2017-08-01T08:45:43Z

README.md

+  <tr>
+    <td>$nodes</td>
+    <td>Device → Controller</td>
+    <td>Nodes the device exposes, with format `id` separated by a `,` if there are multiple nodes. For ranges, define the range after the `id`, within `[]` and separated by a `-`.</td>


Can you replace the ` with <code> tags? The ` doesn't work in HTML arrays.

Strange, in my Atom-Markdown preview this works as expected. I've changed it.

marvinroger · 2017-08-01T08:47:16Z

README.md

@@ -18,7 +18,7 @@ You can find an implementation of the Homie convention:

 ## Background

-An instance of a physical piece of hardware (an Arduino, an ESP8266...) is called a **device**. A device has **device properties**, like the current local IP, the Wi-Fi signal, etc. A device can expose multiple **nodes**. For example, a weather device might expose a `temperature` node and an `humidity` node. A node can have multiple **node properties**. The `temperature` node might for example expose a `degrees` property containing the actual temperature, and an `unit` property. Node properties can be **ranges**. For example, if you have a LED strip, you can have a node property `led` ranging from `1` to `10`, to control LEDs independently. Node properties can be **settable**. For example, you don't want your `degrees` property to be settable in case of a temperature sensor: this depends on the environment and it would not make sense to change it. However, you will want the `degrees` property to be settable in case of a thermostat.
+An instance of a physical piece of hardware (an Arduino, an ESP8266...) is called a **device**. A device has **attributes**, like the current local IP, the Wi-Fi signal, etc. A device can expose multiple **nodes**. For example, a weather device might expose a `temperature` node and an `humidity` node. A node can have multiple **properties**. The `temperature` node might for example expose a `degrees` property containing the actual temperature, and an `unit` property. Node properties can be **ranges**. For example, if you have a LED strip, you can have a node property `led` ranging from `1` to `10`, to control LEDs independently. Node properties can be **settable**. For example, you don't want your `degrees` property to be settable in case of a temperature sensor: this depends on the environment and it would not make sense to change it. However, you will want the `degrees` property to be settable in case of a thermostat.


Good point.

marvinroger · 2017-08-01T08:50:53Z

README.md


-* `homie` / **`device ID`** / `$` **`device property`**: a topic starting with a `$` after the base topic of a device represents a device property. A device property MUST be one of these:
+### Device attributes


marvinroger · 2017-08-01T08:56:33Z

README.md

+homie/ledstrip-device/ledstrip/led_3/$settable → true
+homie/ledstrip-device/ledstrip/led_3/$unit → LED Status
+homie/ledstrip-device/ledstrip/led_3/$datatype → enum
+homie/ledstrip-device/ledstrip/led_3/$range → on,off
 homie/ledstrip-device/ledstrip/led_3 → on
 ```



Agreed too. @Kwave feel free to choose between state and power. 😉

euphi · 2017-08-01T21:49:13Z

README.md

+homie/686f6d6965/humidity/percentage/$settable → false
+homie/686f6d6965/humidity/percentage/$unit → %
+homie/686f6d6965/humidity/percentage/$datatype → integer
+homie/686f6d6965/humidity/percentage/$range → 0:100
 homie/686f6d6965/humidity/percentage → 79
 ```

 A LED strip would look like this. Note that the topic for a range properties is the name of the property followed by a `_` and the index getting updated:



The term range is now used in two different meanings in the convention.

To allow "arrays" of properties.

To define allowed value range

--> possible Solution: change $range to $valuerange
An alternative would be to change to range term (as used in the "old" convention) to something else, e.g. vector .

Agreed, this might get somehow confusing. I've changed the $range to $format, as "range" is the wrong term anyways. e.g. a regex is definitely not a "range", nor are enums.

$format seems perfect. @euphi ?

euphi · 2017-08-01T22:03:58Z

README.md

 homie/ledstrip-device/ledstrip/led_1 → on
+


Personally, I don't think it is a good idea to set the attribute of each property range-index individually. $settable, $unit, $datatype and $range (or $valuerange) should be the same for the complete range of the property and thus be defined on property level.
Ranges (or vectors) should simplify things and not make them more complicated. If you need individual attributes, use individual properties.

There may be a use for naming each ranged property:

I/O modules of industrial automation systems usually use formal names based on a simple numbering scheme (comparable to homie's range (or vector)), but allow to set an optional symbolic name.

ThomDietrich · 2017-08-03T17:43:57Z

Hey guys, great progress so far!
I did a little brainstorming with a colleague today and we found room for two improvements I'd like to discuss with you:

Property class/category: A property is a distinct reading or characteristic value produced by a node of a device. The other side of the network, the discovering entity, needs to know about the nature of this value (e.g. to show a thermometer in a GUI). We are already providing attributes to characterize its unit, range, pretty name and "set'ability" but not a clear type.
A not-settable property with the unit "°C" does at best imply that the property is a temperature reading of a thermometer, not e.g. a thermostat or a nominal temperature. The discovering entity should not have to resort to guessing and interpreting of the other attributes.
I propose a counted number of property classes to characterize the nature of a property. The $class or $category attribute is required but can be "Unspecified"/"Other"/"" for special non-trivial properties. The classes list can be extended over time and the discovering entity is allowed to ignore the provided class if unknown.
Conversion Attribute: When working with embedded systems or low-energy micro controllers, we can not expect the device to be capable to convert a raw sensor reading into a final human-comprehendible value. An example would be a temperature sensor reading between [0..255] which would map onto a degree Celcius value with decimal places.
I propose the introduction of a $conversion property attribute. The published String value is either "x"/"" for no conversion or the right hand of an f(x) formula, e.g. x / 255 * 50 - 20.

Both points are of course open for discussion. What do you think?

marvinroger · 2017-08-04T06:49:55Z

@ThomDietrich I see a problem with property classes. Imagine you have an heater node, with two properties: mode (off,comfort,night), and temperature. How do you assing a "class" to mode, for example? The most logical thing to do would be to assing classes to nodes, not properties. Therefore we would have heater, light, weatherstation... But this would introduce another problem: for the UI to be able to show a thermometer with the temperature embedded in the picture, we would have to define a standard set of properties for each class. Which defeat the flexibility of the homie convention, right?

For the conversion attribute, I am not that sure. It's rather complicated, and I don't see in practice what kind of value can't be computed directly on-device. I mean, even a 3$ chip is able to read a temperature in °C, °F.

marvinroger · 2017-08-04T09:33:47Z

@Kwave

Your suggestion was lacking the possibility to add flags to the pattern. Thats all ;-)

A simple regex pattern is enough, there's no need to use any flags in such a case, right? (global, unicode, multiline...). The only exception being the ignore flag, but for ease of implementation by parser and simplicity, I think it's better to do [a-zA-Z] rather than /[a-z]/i. Objections?

euphi · 2017-08-04T10:27:01Z

I didn't had the time yet to look at @ThomDietrich 's proposal, but I suggest to move this discussion to an own issue, because it is out of scope of the original intention of this PR, so this PR can be merged soon.

ThomDietrich · 2017-08-04T11:20:47Z

@marvinroger I might have to counter on both topics but:
I agree with @euphi. Let's merge the PR soon and incorporate further additions later.
There are many details I'd like to add or modify and individual PRs for these, based on the state of this PR now, would be easier to manage...

Added description of property attributes

3ebeee7

gorootde changed the title ~~Added property attributes~~ Added Attributes to node properties Jul 31, 2017

ThomDietrich mentioned this pull request Jul 31, 2017

Initial contribution of Homie Binding openhab/openhab-addons#1741

Closed

euphi reviewed Jul 31, 2017

View reviewed changes

Corrections suggested in the PR.

5473108

Differentiated between properties and attributes

a0fb66f

ThomDietrich reviewed Jul 31, 2017

View reviewed changes

marvinroger reviewed Aug 1, 2017

View reviewed changes

marvinroger requested changes Aug 1, 2017

View reviewed changes

euphi requested changes Aug 1, 2017

View reviewed changes

Changes discussed in PR

3954576

ThomDietrich mentioned this pull request Aug 2, 2017

Extra device properties #17

Closed

marvinroger approved these changes Aug 4, 2017

View reviewed changes

marvinroger merged commit 779093a into homieiot:v2 Aug 4, 2017

ThomDietrich referenced this pull request Aug 4, 2017

🎨 Clarify background

c8a270a

gorootde deleted the discoverable-nodes branch August 6, 2017 14:56

ThomDietrich mentioned this pull request Aug 7, 2017

Property class and conversion attributes #38

Closed

		@@ -35,14 +35,17 @@ An ID MAY contain only lowercase letters from `a` to `z`, numbers from `0` to `9
		To efficiently parse messages, Homie defines a few rules related to topic names. The base topic you will see in the following convention will be `homie/`. You can however choose whatever base topic you want.

		* `homie` / `device ID`: this is the base topic of a device. Each device must have an unique device ID which adhere to the [ID format](#id-format).


		* `homie` / `device ID` / `$` `device property`: a topic starting with a `$` after the base topic of a device represents a device property. A device property MUST be one of these:
		### Device attributes

Added Attributes to node properties #27

Added Attributes to node properties #27

Conversation

gorootde commented Jul 31, 2017

ThomDietrich commented Jul 31, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

gorootde commented Jul 31, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ThomDietrich Jul 31, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ThomDietrich Aug 2, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ThomDietrich commented Jul 31, 2017

marvinroger Aug 1, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

gorootde Aug 2, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ThomDietrich commented Aug 3, 2017 • edited Loading

marvinroger commented Aug 4, 2017

marvinroger commented Aug 4, 2017

euphi commented Aug 4, 2017

ThomDietrich commented Aug 4, 2017 • edited Loading

ThomDietrich commented Jul 31, 2017 •

edited

Loading

gorootde commented Jul 31, 2017 •

edited

Loading

ThomDietrich Jul 31, 2017 •

edited

Loading

ThomDietrich Aug 2, 2017 •

edited

Loading

marvinroger Aug 1, 2017 •

edited

Loading

gorootde Aug 2, 2017 •

edited

Loading

ThomDietrich commented Aug 3, 2017 •

edited

Loading

ThomDietrich commented Aug 4, 2017 •

edited

Loading