homieiot / convention

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

Classify required attributes as optional (or recommended) #45

Closed oeysteinhansen closed 5 years ago

oeysteinhansen commented 7 years ago

Some of the mandatory attributes are device specific / implementation specific. Some are not required for the working of the Homie Convention.

I would like to update some of the attributes mandatory flag. This will allow for an slimmer Homie Convention.

$name => Optional, Device ID may be used when no name is given. $localip => Optional, Device specific. (not all devices have a IP) $mac => Optional, Device specific. (not all devices have a MAC address) $stats/uptime => Optional. (is this required for sensor valid time?) $stats/signal => Optional. Is already defined as optional. $stats/interval => Optional. Not required for the correct operation. $fw/name => Optional. Implementation specific. $fw/version => Optional. Implementation specific. $fw/checksum => Optional. Is already defined as optional. $implementation => Optional. It should be adequate to use $homie attribute.

A simple table may be used for showing what is required for given features.

Topic Homie Core Homie OTA updates
$homie Required Required
$online / $state Required Required
$name Optional Optional
$localip Optional N/A
$mac Optional N/A
$stats/uptime Optional N/A
$stats/signal Optional N/A
$stats/interval Optional N/A
$fw/name Optional Required
$fw/version Optional Required
$fw/checksum Optional Optional
$implementation Optional Required

I do not know the inner workings of the Homie Convention, but my general ide is that there are to many mandatory attributes that do not need to be mandatory.

marvinroger commented 6 years ago

@ThomDietrich I think everything else is done in the redesign branch, but this is the latest piece of the puzzle. Seems like a recurrent request, so, based on the latest redesign revision, what should be required?

Latest issue before v2.1.0 tagging/release. 🀞

ThomDietrich commented 6 years ago

I'd have to review https://github.com/marvinroger/homie/pull/48 once more but I'd say we should this discuss over there.

ThomDietrich commented 6 years ago

@timpur also see #86. wdyt?

timpur commented 6 years ago

Yeah, if there are things not required sure, haven't thought about what those are, but homie is trying to allow for autodiscovery of a device, so if we can uphold that purpose with less required topics without compromising on features, sure. I'll have a look into this when I have a sec, sorry I lack time ATM.

fermuch commented 6 years ago

I'd like to share what my personal implementation uses, which I hope to release soon. I've had to omit some fields for data transfer usage, since this project is meant to run over 2G / satellite links (2kbps).

Topic Details
$state Only init, ready and lost (LWT)
$homie
$name only set one time, doesn't re-set on reconnect
$fw/name
$fw/version
$stats Only uptime is published, and in some cases signal
$stats/interval

This is the most essential implementation I've could come to, which is still usable.

marvinroger commented 6 years ago

I totally understand the frustration of it being so heavy when it's supposed to be lightweight. I'd slim down the required attributes to this:

Device attributes

Topic Required Comment
$homie βœ… Mandatory for viable discovery
$name ❌ Aesthetic
$state βœ… Important to know in which state is the device
$localip ❌ Might not be relevant
$mac ❌ Might not be relevant
$fw/name ❌ Might not be relevant
$fw/version ❌ Might not be relevant
$nodes βœ… Mandatory for viable discovery
$implementation -- $implementation/# ❌ Might not be relevant
$stats -- $stats/# ❌ Might not be relevant

Node attributes

Topic Required Comment
$name ❌ Aesthetic
$type ❌ Might not be relevant
$properties βœ… Mandatory for viable discovery
$array πŸ”Ά Only if it is an array

Property attributes

Topic Required Comment
$name ❌ Aesthetic
$settable βœ… Mandatory for viable discovery
$unit ❌ Might not be relevant
$datatype ❌ Might not be relevant
$format πŸ”Ά Only for $datatype enum or color

It would definitely simplify the creation of a minimal Homie client. Any comments?

ThomDietrich commented 6 years ago

Looks good. In #86 we already agreed that a property $settable attribute is not mandatory and false by default.

marvinroger commented 6 years ago

I can't find anything about the $settable attribute in #86?

I don't agree with an optional $settable, though. Something being actionable or not is pretty fundamental in my opinion, and a controller would first assume that the property is not settable before receiving the $settable to true. Making it mandatory would allow the controller to know whether or not a property is actionable (by waiting for each $settable) before "announcing" it to the controller.

ThomDietrich commented 6 years ago

Sorry, wrong issue. The important detail I wanted to mention is that it is already defined as "not mandatory, false by default". https://github.com/homieiot/convention#property-attributes

Regarding the second part of your message: Isn't one general idea to expect only the sum of announcement messages as the base for discovery? See "ready" here: https://github.com/homieiot/convention#device-behavior

ThomDietrich commented 6 years ago

Found the relevant thread: https://github.com/homieiot/convention/issues/24#issuecomment-322452300

marvinroger commented 6 years ago

Oh, right. If it already is, we won't break that.

There's a problem: the MQTT PUBLISH might come out of order. So the ready might arrive before other messages. You wrote about having some sort of timeout. That's the only solution, then: as soon as we receive the ready message, we consider the device as discovered after, say, 100ms. If we don't receive a settable in that timespan, then the property is non-settable. Fine.

But what if there's latency and we receive the message a second later? For a name attribute, it's not that problematic, as it's aesthetic; we just have to update the display name. But for a settable attribute, it's not that easy.

timpur commented 6 years ago

Sounds like two issue there ^, should we break this up into another issue related to the timeout issue? Don't think that is directly related to what properties are required?

Also IMHO $type for a is required for discovery. This prop defines how you interpret the node for discovery...

marvinroger commented 6 years ago

It should, indeed.

Why is $type required for discovery? It used to be, but now that every property is described explicitely, it's only aesthetic ($type = window would show a window image in the GUI, for example).

timpur commented 6 years ago

True about props being self discovered, but how would you know how to interprate them?

homie/kitchen/kitchen-lights/$name β†’ "Kitchen Lights"
homie/kitchen/kitchen-lights/$type β†’ "light"
homie/kitchen/kitchen-lights/$properties β†’ "state,brightness"
...

With out $type, how would you interpare the two props as a light ? As seperate enterties? A switch and a slider? You'd have no context of what this group of properties doese and thus could only treat them as seperate enterties, thus rendering the node useles as a group of things.... it would just be a container and add no value .... (have i missed something?)

Edit: Also if you guys have time maybe also look at https://github.com/homieiot/convention/issues/75#issuecomment-385351567

davidgraeff commented 5 years ago

I have created a PR for this: #120. I really like to see many of the mandatory topics to go. They are not even looked at in my openhab controller implementation and are of questionable value. The espEasy maintainer has shown interest in implementing homie, but expressed the many topics as a downside. Let's tackle this.

According to semver this will bring us to Homie 4.0 unfortunately, but it really has to be done at some point.

davidgraeff commented 5 years ago

Done