-
-
Notifications
You must be signed in to change notification settings - Fork 432
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
Add semantic equipment tag on things #4617
base: main
Are you sure you want to change the base?
Conversation
Signed-off-by: Andrew Fiddian-Green <[email protected]>
This pull request has been mentioned on openHAB Community. There might be relevant details there: https://community.openhab.org/t/openhab-5-semantic-model-proposal/162526/96 |
Signed-off-by: Andrew Fiddian-Green <[email protected]>
Signed-off-by: Andrew Fiddian-Green <[email protected]>
Signed-off-by: Andrew Fiddian-Green <[email protected]>
Signed-off-by: Andrew Fiddian-Green <[email protected]>
Signed-off-by: Andrew Fiddian-Green <[email protected]>
@mherwege can I kindly ask you to be a sparring partner for this PR? My goal is to allow developers to set a (proposed) semantic (equipment) tag in their thing type xml, and have this permeated through the thing type instances to the actual thing instances so that the UI can use that semantic (equipment) tag (as a hint) in creating a semantic model. |
@andrewfg Sure. I like the idea as long as it does not then enforce anything on the item semantics. So I would think that it should be carried to the point of a having a tag on the thing, but then it should be up to the UI to pick this up as a suggested equipment semantic when creating the group item with its points (create model from thing). |
Indeed. The tags in the ChannelType xml are hints for semantic “point” and semantic “property” tags which are intended for functional Item creation. And the single tag in the ThingType xml would be a hint for the semantic “equipment” tag intended for semantic GroupItem creation. The remaining “location” semantic tag is entirely user dependent and would not come from the xml. PS we could even explicitly name it |
Signed-off-by: Andrew Fiddian-Green <[email protected]>
Signed-off-by: Andrew Fiddian-Green <[email protected]>
Signed-off-by: Andrew Fiddian-Green <[email protected]>
...rg.openhab.core.thing/src/main/java/org/openhab/core/thing/binding/builder/ThingBuilder.java
Outdated
Show resolved
Hide resolved
* @param semanticEquipmentTag semantic (equipment) tag | ||
* @return the updated builder | ||
*/ | ||
public DiscoveryResultBuilder withSemanticEquipmentTag(@Nullable String semanticEquipmentTag) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm wondering if it would be better to stay consistent and generic like e.g. here:
Lines 54 to 68 in 6bf5960
/** | |
* Adds a tag to the ChannelType | |
* | |
* @param tag Tag to be added to the ChannelType | |
* @return this Builder | |
*/ | |
T withTag(String tag); | |
/** | |
* Sets the StateDescription for the ChannelType | |
* | |
* @param tags Collection of tags to be added to the ChannelType | |
* @return this Builder | |
*/ | |
T withTags(Collection<String> tags); |
so simply withTag
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
See #4617 (comment) above.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am fine either way.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@jlaur some background..
In ChannelType we have a tags
element that currently allows an unlimited number of tag
sub-elements, which have no XSD schema validation. So this allows developers to have an open ended number of strings having open ended meanings.
I think it is the wish of OH maintainers to restrict the number and the meaning of these tags. So there should be a maximum of two tag
sub-elements. And strictly speaking one of those tag
elements is a semantic point tag
and the other is a semantic property tag
. And the developer choices when entering such values shall be subject to stricter XSD schema validation in order to restrict the choice to only valid tags.
// as it is today (open ended number and meaning)
<tags>
<tag>blah</tag>
..
<tag>blah</tag>
</tags>
In #4615 we are trying to do this constraint and validation. However since the tag
elements have the same name we cannot do it fully via the xml. We would need to change the schema to do that (see below) which would be a breaking change. So I am loth to go all the way in doing that. But nevertheless we are moving towards more strictness rather than less.
// as it should be (constrained number and meaning)
<tags>
<semanticPropertyTag>XSD validated Property</semanticPropertyTag>
<semanticPointTag>XSD validated Point</semanticPointTag>
</tags>
Now -- finally to get to the point -- as we are moving towards more strictness in the ChannelType
tags (semantic property tag
resp. semantic point tag
), we should NOT be moving to introduce less strictness in ThingType
.
Signed-off-by: Andrew Fiddian-Green <[email protected]>
Signed-off-by: Andrew Fiddian-Green <[email protected]>
Signed-off-by: Andrew Fiddian-Green <[email protected]>
Signed-off-by: Andrew Fiddian-Green <[email protected]>
Ok. I have now tested this, and I think it is ready for review. See OH Rest API output below..
Obviously it needs work now on the OH WebUI side in order to utilise this Notes:
|
Ping @jimtng |
Signed-off-by: Andrew Fiddian-Green <[email protected]>
Signed-off-by: Andrew Fiddian-Green <[email protected]>
Signed-off-by: Andrew Fiddian-Green <[email protected]>
Signed-off-by: Andrew Fiddian-Green <[email protected]>
bundles/org.openhab.core.thing/schema/thing/thing-description-1.0.0.xsd
Outdated
Show resolved
Hide resolved
Signed-off-by: Andrew Fiddian-Green <[email protected]>
I don't think this is a very complex change in the UI. I can take care of it, but would prefer to finish the core code first. There are a lot of moving parts and dependencies here, and creating a core that has it all for testing a UI change against is a bit of a challenge. It will be easier to do when we get the core changes over the line first. |
The purpose of this PR is to add a semantic (equipment) tag to things so that the UI can use that tag for putting items that are bound to channels on such things in the semantic model.
Adds two things:
See also #4616 #4619 #4622
Resolves #4614
Signed-off-by: Andrew Fiddian-Green [email protected]