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

[mqtt][homie] Implement non-retained and non-settable properties #8246

Merged
merged 1 commit into from
Sep 3, 2020

Conversation

bodiroga
Copy link

@bodiroga bodiroga commented Aug 3, 2020

This PR makes non-retained and non-settable properties usable, because right now they not fully supported as described in #8164.

With the proposed changes, this type of properties are modeled as state channels and as such are able to receive commands, but they don't update their states, as they are meant to be "stateless" (non-retained -> we don't want to store the value). That's why I have defined the VETO option for the AutoUpdatePolicy. At the same time, I have extended the generic Value class so that its children classes can implement the createCommandDescription() method. Usually this type of properties will be used with the "enum" data type, so providing a list of command options makes sense.

Feedback from @jochen314, @J-N-K, @cpmeister, @Hilbrand and @fwolter would be highly appreciated 😉

Closes #8164.

Signed-off-by: Aitor Iturrioz [email protected]

@bodiroga bodiroga requested a review from davidgraeff as a code owner August 3, 2020 01:00
@TravisBuddy
Copy link

Travis tests were successful

Hey @bodiroga,
we found no major flaws with your code. Still you might want to look at this logfile, as we usually suggest some optional improvements.

@Hilbrand Hilbrand added the bug An unexpected problem or unintended behavior of an add-on label Aug 27, 2020
Copy link
Member

@Hilbrand Hilbrand left a comment

Choose a reason for hiding this comment

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

The code looks good to me. I have no idea if it's functionally correct though.

@bodiroga
Copy link
Author

bodiroga commented Sep 3, 2020

Hi @Hilbrand!

Many thanks for your review (and for the awesome job you are doing recently, you are on fire!). Just to help others understand what I have implemented in the PR, Homie properties can behave differently according to two important parameters: whether they are settable (true or false) and whether their values are retained (true or false). Thus we have four different combinations, as described here: https://homieiot.github.io/specification/#properties. The last one (non-retained and settable) was missing and requested by @ccutrer and that is what this PR tries to address, making the binding compatible with the four options.

If you have any question don't hesitate to ask, please, it would be great if this could be merged before the big OH3 refactory was done (@jochen314, @cpmeister, @J-N-K) 😉

@J-N-K J-N-K merged commit e856d93 into openhab:2.5.x Sep 3, 2020
@J-N-K J-N-K added this to the 2.5.9 milestone Sep 3, 2020
CSchlipp pushed a commit to CSchlipp/openhab-addons that referenced this pull request Sep 12, 2020
markus7017 pushed a commit to markus7017/openhab-addons that referenced this pull request Sep 19, 2020
andrewfg pushed a commit to andrewfg/openhab-addons that referenced this pull request Oct 8, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug An unexpected problem or unintended behavior of an add-on
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[mqtt.homie] settable, non-retained channels are unusable
4 participants