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

Classify required attributes as optional (or recommended) #45

Closed
oeysteinhansen opened this issue Sep 12, 2017 · 16 comments
Closed

Classify required attributes as optional (or recommended) #45

oeysteinhansen opened this issue Sep 12, 2017 · 16 comments

Comments

@oeysteinhansen
Copy link

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 marvinroger added this to the v2.1.0 milestone Nov 6, 2017
@marvinroger
Copy link
Member

@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
Copy link
Collaborator

I'd have to review #48 once more but I'd say we should this discuss over there.

@ThomDietrich ThomDietrich changed the title To many mandatory attributes Classify required attributes as optional (or recommended) Apr 26, 2018
@ThomDietrich
Copy link
Collaborator

@timpur also see #86. wdyt?

@timpur
Copy link
Contributor

timpur commented Apr 26, 2018

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
Copy link

fermuch commented Apr 26, 2018

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
Copy link
Member

marvinroger commented Apr 29, 2018

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
Copy link
Collaborator

ThomDietrich commented Apr 29, 2018

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

@marvinroger
Copy link
Member

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
Copy link
Collaborator

ThomDietrich commented Apr 29, 2018

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
Copy link
Collaborator

Found the relevant thread: #24 (comment)

@marvinroger
Copy link
Member

marvinroger commented Apr 29, 2018

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
Copy link
Contributor

timpur commented Apr 29, 2018

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
Copy link
Member

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
Copy link
Contributor

timpur commented Apr 30, 2018

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 #75 (comment)

@davidgraeff
Copy link
Member

davidgraeff commented Oct 31, 2018

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
Copy link
Member

Done

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

6 participants