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

BREAKING: Homie is about Lightweight topic discovery #120

Merged
merged 2 commits into from
Nov 5, 2018

Conversation

davidgraeff
Copy link
Member

@davidgraeff davidgraeff commented Oct 31, 2018

  • $stats is fully optional
  • Remove $fw, $mac and $localip completely. Those are all for firmware updates but at the same time not sufficient for a firmware update. Requires another spec, independent of how topics are announced.
  • Remove "retained" column where applicable. Because "retained" is per default on, as specified in the header. Makes the spec easier for the eyes.

* $stats is fully optional
* Remove $fw, $mac and $localip completely
* Remove "retained" column where applicable. Because "retained" is per default on, as specified in the header.
@ThomDietrich
Copy link
Collaborator

remove $fw, $mac and $localip

I get your point regarding firmware update but are those not also good for simple self-announcement?

@davidgraeff
Copy link
Member Author

I don't think so. The Mac is required at the moment, but is not useful in an mqtt network where the broker can be in a different subnetwork. The localip attribute is flawed. Because an interface usually has more than one IP address nowadays. And it is the local IP only which is again not so useful for mqtt. And the $fw node is wrongly named, because a homie Linux client is technically not a firmware.

I'm not really sure what the benefit is for a controller to know those information.

An administrative tool would usually want to know about the firmware to perform ota. That's why I think we should make this an add-on convention that specifies exactly all information for ota to work.

@boc-tothefuture
Copy link
Contributor

Are you proposing that we keep $implementation?

@davidgraeff
Copy link
Member Author

Yes, but optional. There might be quirks in some implementations and knowing the implementation together with the homie version might help a controller to overcome those quirks.

@boc-tothefuture
Copy link
Contributor

Would we want an optional implementation version as well? Using the quirks scenario you would need that if the implementation evolved at a different rate than the homie version.

@davidgraeff
Copy link
Member Author

Might be a good idea, but is not part of this PR

@boc-tothefuture
Copy link
Contributor

I can open a PR on it, after this one is merged.

@timpur
Copy link
Contributor

timpur commented Nov 3, 2018

Im with @ThomDietrich for $fw, $mac and $localip

MAC and IP are already required for the IP stack so why not just send them. Maybe make these optional ?

FW I think has quite alot of value actually. Maybe Implementation type makes more sense idk ?

Maybe these just need shuffling around where they should sit ?

Not sure the pros over come the cons here. But I'll let you guys discus.

@davidgraeff
Copy link
Member Author

My controller and device implementations are both not homie compliant, because I don't publish or subscribe to those attributes and I'm not encouraging other device implementers to do so if they ask for the minimum compliance level.
I really thing this should be at least optional yes. Soon.

@boc-tothefuture
Copy link
Contributor

boc-tothefuture commented Nov 3, 2018

@timpur The problem is, which mac and IP address should be sent and does that have value in all situations? If you have multiple interfaces and ip addresses, one can do some work with UDP sockets and figure out which IP and therefore interface the OS will use to get to the broker. That may have value, but that value reduces in many situations. For example, if your client is running in a docker container and routing out via a NAT and is not addressable on the network.

For firmware, lets say my I am fetching and publishing weather information from weather.com over the homie convention. What would my firmware be? The API version of the weather.com device I am fetching data from? I think there are a lot of use cases where firmware doesn't make sense to be required.

I think all of these values should be optional attributes under implementation because they are implementation specific.

@Thalhammer
Copy link
Member

MAC and IP are already required for the IP stack so why not just send them. Maybe make these optional ?

No, they are not. IP does not require a mac to be present and many technologies which have no mac (almost everything which is not BT, WLAN or LAN). The most prominent example being 2G/3G/LTE.

@timpur
Copy link
Contributor

timpur commented Nov 4, 2018

So lets all agree on what and where these things should go.

I think all of these values should be optional attributes under implementation because they are implementation specific.

I really think this should be at least optional yes.

So optional values sitting under implementation ?

Also regarding foks who pointed out it doesnt always have to the mac and ip, why dont we reimagine these to allow for for options? eg address = "IP: 192.168.0.2, MAC: AB:12:CD:34:EF:56" (for standard IP networks) or address = "RF: 42" (for RF networks), also we could show the mqtt identifier "MQTT: esp1234". Basically address is some network identifiers ?? It cold contain the full stack of identifiers that the implementation is using ?

@davidgraeff
Copy link
Member Author

@timpur: yeah that's all doable. But this PR is first about getting rid of Mac and localip and the other stuff that not all implementation need to have. Make a pr proposal for address or similiar

@timpur
Copy link
Contributor

timpur commented Nov 4, 2018

@davidgraeff , yeah i agree with that, i can do so, will think a bit more on the matter first though.

Copy link
Contributor

@timpur timpur left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good, please change the one comment.

convention.md Outdated Show resolved Hide resolved
@ThomDietrich
Copy link
Collaborator

My thought was to make Mac and IP optional, so that clients can implement them, when meaningful. However this is once again nothing that has to be part of the convention. An implementer/user can always add additional data. I agree to remove IP and Mac.

@davidgraeff
Copy link
Member Author

@timpur Could you re-review and approve?

@timpur
Copy link
Contributor

timpur commented Nov 5, 2018

@davidgraeff , sorry different time zone ....

For anyone interested about the address stuff #129

Copy link
Member

@marvinroger marvinroger left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As a reminder, the convention was initially created as part of the homie-esp8266 project, so things like firmware, MAC address and IP address all made sense then.

Now that homie has a broader audience, I'm definitely for these changes!

@davidgraeff davidgraeff merged commit 98ce2a3 into homieiot:develop Nov 5, 2018
@davidgraeff davidgraeff deleted the makelw branch November 5, 2018 22:40
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

Successfully merging this pull request may close these issues.

6 participants