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

Extra device properties #17

Closed
bertmelis opened this issue Feb 28, 2017 · 16 comments
Closed

Extra device properties #17

bertmelis opened this issue Feb 28, 2017 · 16 comments

Comments

@bertmelis
Copy link

The convention now states a limited list of device properties.
I would however want to add a few more: a device could have a battery level, cpu load, free memory, temperature,...

For not to break the convention, I now create a "device-node" with the node properties as above, which I find not entirely correct.

(It is working however, so if the community -and off course you Marvin- doesn't agree with me, I'll stick to my current solution.)

@euphi
Copy link
Member

euphi commented Aug 2, 2017

There should be a standard for the device node :-)

So, in Homie Convention there is an optional node (e.g. "$deviceinfo") with a standardized set of properties.

The devicenode could be implemented as extra library for various platforms, e.g. Homie-esp8266-DeviceInfo, so everyone is free to decide whether to use it.

@bertmelis
Copy link
Author

I'm not following. Did I overlook something in the PR?

@marvinroger
Copy link
Member

I also don't get it 🤔

@euphi
Copy link
Member

euphi commented Aug 2, 2017

@bertmelis the PR currently in discussions does not define more detailed device properties. In your proposal you ask for properties like CPU load, battery state, free mem etc.

Based on this I propose to define an optional "$deviceinfo" node, that is like a normal node, but with standardized properties, e.g.

homie/myDevice/$deviceinfo/CPUtemperature --> 32

Where CPUtemperature is a standardized property, so there is no need to send all the unit info etc.

@ThomDietrich
Copy link
Collaborator

ThomDietrich commented Aug 2, 2017

May I ask what we would need this for?

The Homie convention as an MQTT convention is here to support auto discovery of devices and applications on an MQTT broker through a standardized way of announcing and publishing data and actions of that entity. Data like the CPU temperature is exactly that, data. It can be published by defining a node (e.g. CPU) and a property (e.g. temperature). No need for special attendance. Am I correct? @marvinroger would you agree with that definition? Maybe defining a proper use case for Homie would also help in finalizing #27

I did btw also not get your initial request @bertmelis, did you talk about something like standardized nodes, e.g. similarly to http://www.eclipse.org/smarthome/documentation/development/bindings/thing-definition.html#channel-categories ??

@bertmelis
Copy link
Author

bertmelis commented Aug 2, 2017

Well, the Homie convention now gives a number of stats ($stats/uptime, signal, interval) which are also merely data. Signal is even optional. They are all device and configuration specific and do not relate to the thing being automated but to the device doing the automation.

The paragraph says device attributes MUST the one of the listed.

So I was thinking about making the list less strict and give some space to the $stats/# branch.

@marvinroger
Copy link
Member

@ThomDietrich I like your definition of the Homie convention. But @bertmelis remark is right, the convention already allows to embed things like IP, mac address, signal, etc, metadata, to name it. Thinking about it, there's one thing, though:

MQTT works over TCP/IP. Each network interface will have at least two things: an IP and a MAC address. And each device will also have an uptime, anyway. The $signal property depends on the network interface (Wi-Fi, 4G...), so it depends on the implementation.

So I would remove the $stats/signal property, therefore, all attributes in the convention would be required. Then, every other implementation-dependant stats (signal, battery level, cpu load, free memory, temperature) would be available under the already available "$implementation/#" namespace. That way, you can show a specific UI depending on the "$implementation" attribute (e.g. we know that an "esp8266" would have "$implementation/wifisignal", "$implementation/freemem", etc.). It feels more organized and less messy that way. What do you think?

@ThomDietrich
Copy link
Collaborator

ThomDietrich commented Aug 4, 2017

Sounds good. We could also make the signal property optional. No need to move it!? Do you feel that something being required is that important? The controller can react on a non-published retained topic just the same as on a published one. If there is no $signal topic the device doesn't have a signal. Easy.
Besides, I actually never knew what the implementation branch was supposed to stand for...

@marvinroger
Copy link
Member

Having all attributes required makes it easy to determine when a device is fully discovered. Imagine you store the data received in a database. If we "CREATE" a device in DB with all required attributes and you receive an optional attribute after, we would "UPDATE" to add the optional property in the DB. Whereas, based on the "$implementation" topic, you can determine that if the implementation is for example "esp8266", you know that the device is not fully discovered if you didn't receive an "$implementation/wifisignal" value.

@ThomDietrich
Copy link
Collaborator

ThomDietrich commented Aug 4, 2017

I don't see the issue. In fact I feel this step to be one in the wrong direction.

All discoverable data is available as retained messages. These are sent over by the broker upon subscription or published and sent by the device upon connection. Either way these topics will be available momentarily as the controller initiates and processes the auto discovery, attributes will not change after the device reached its online=true state. (A state "online and fully announced" in addition to a simple "online" might be something interesting for #24)
Each attribute will characterize the nature of a node/property a little better. The controller will receive this data and update his inner presentation of the device accordingly. All not received details will stay in their default condition. Expecting the device to communicate each and every attribute in order to implicitly reach the "completely discovered" state is not viable. Making sure, that a device is not used in a partly initialized state is furthermore task of the controller (e.g. by waiting for all retained messages), not of the IoT device. Avoiding certain edge case race conditions is also the task of the controller. The convention needs to keep the broker clean while communicating special details and staying open for additions.

See it from another angle: A developer building a Homie compatible application or device doesn't want to implement all the for him unimportant aspects of the convention. (Example) He also doesn't want to add new attributes as they get added to the convention.
If we went down that path now, we will loose the flexibility and ease of use Homie is I believe made for.

@euphi
Copy link
Member

euphi commented Aug 4, 2017

I prefer to have optional properties in the $stats branch, because information like signal strength or batterie level are not implementation specific. They have the same meaning regardless of the implementation and only depend on, if the are appropriate for the device on which Homie is running.

Regarding $online: A state "poweron" (or "connect", or "configuration") that must be send first after (re-)connecting to MQTT would solve #24. If the "online" state must be sent last (after Eventhandler, but before loop()), it also helps to know if all mandatory information has been send.

This is something already implicitly required by the revision of Homie 2.0:

Description of $online:

When sending the device is online, this message must be sent last, to indicate every other required messages are sent and the device is ready

@euphi
Copy link
Member

euphi commented Aug 4, 2017

Possible definiton of optional $stats properties (to be extented):

Topic Description Remark
$signal Signal strength in % rssi=-100 --> 0%, rssi = -50 --> 100%
$cputemp CPU Temperature in °C
$cpuload CPU Load in % Average of last $interval including all CPUs
$battery Batterie level in %
$heap Free Heap in Bytes (or better kbytes?)
$supply Supply Voltage in V

@marvinroger
Copy link
Member

Alright, I am convinced! Let's add some optional $stats, then. 😇
Let's move the $online discussion to #24, to separate concerns.

@marvinroger
Copy link
Member

08666c3

Do we close this issue?

@euphi
Copy link
Member

euphi commented Aug 6, 2017

Thanks! From my point of view, issue can be closed. ✨

@marvinroger
Copy link
Member

Feel free to reopen if needed!

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

4 participants