openhab / openhab-addons

Add-ons for openHAB
https://www.openhab.org/
Eclipse Public License 2.0
1.9k stars 3.59k forks source link

Check existing semantic tags defined on channel types #12262

Open lolodomo opened 2 years ago

lolodomo commented 2 years ago

The guidelines about semantic tags are not yet really clear. We found the following temporary agreement with @kaikreuzer to start adding semantic tags. Here are the current guidelines. Kai, please correct me if something is wrong or not enough clear:

  1. Semantic tags should be added to a channel type only when the assignment is absolutely obvious. For example, the tags "Measurement" + "Temperature" should not be set on a channel type used for forecasted temperature as forecast is not a measurement.
  2. It is not allowed to set only "Control" or "Switch" tag on a writable channel type. Also not expected to set "Status" tag on all read-only channel types.
  3. No property tag is expected with the "Status" point tag; a property tag is generally expected with a "Control" or "Measurement" point tag. As an example, Status + Temperature is not expected.

Now here are all the bindings which defined semantic tags on static channel types. We have to check if they respect the rules defined above:

And here are all the bindings using withDefaultTags (channel) and/or withTags / withTag (channel type) that should be checked:

We can discuss in this topic each time we find a tag which does not match the rules.

lolodomo commented 2 years ago

@kaikreuzer / I believe we should moderate the fourth rule because for old bindings you have sometimes Number channels without any dimension but the semantic tags "Measurement" + "something" make sense (I guess the binding was created before UoM was introduced in openHAB core, or maybe the developer decided to not use UoM). And another case is the UV index, it is generally a simple Number channel but it make sense to tag it with "Measurement" + "Ultraviolet", like it is done in the openweathermap binding.

lolodomo commented 2 years ago

For the sonyprojector binding, I defined the channel type "powerstate" (Current detailed power state of the projector). I tagged the channel type with "Status" + "Power". It does not respect the third rule. @kaikreuzer : should I switch to "Measurement" + "Power" ? Or maybe only "Status" ?

lolodomo commented 2 years ago

@kaikreuzer : for the sagercaster binding, we have 2 cases to discuss:

  1. the channel "is-raining": this is an ON/OFF information. It was tagged with "Status" + "Rain". Should we remove all the tags ? If not, what tags should we use ?
  2. the channel "timestamp" (Timestamp of the last weather forecast update): this is another interesting case as we could find it in many bindings. It was tagged with "Status" + "Timestamp". Should we remove all the tags ? If not, what tags should we use ?

@clinioque as author of the binding

wborn commented 2 years ago

You could add some SAT checks so it is easier to check and enforce the logic on tags. 😉

jlaur commented 2 years ago

@lolodomo - addressing the danfossairunit system channel type comment: #12277

kaikreuzer commented 2 years ago

Kai, please correct me if something is wrong or not enough clear

Thanks @lolodomo for your efforts here, the summary above sounds good to me. Just FWIW: I am not the one to ultimately decide here, so these guidelines were just a suggestion on how to proceed for now, as they are probably the least controversial. Anyhow, everyone should feel free to challenge and suggest changes. But in order to move forward, let's go with this unless anyone complains!

I'll try to look into and answer your questions later today...

lolodomo commented 2 years ago

I am not the one to ultimately decide here, so these guidelines were just a suggestion on how to proceed for now

Yes, of course. And that is clear for me that is the current and temporary proposed guidelines, not the final ones.

kaikreuzer commented 2 years ago

I believe we should moderate the fourth rule

Yes, I agree, given your examples.

@kaikreuzer : should I switch to "Measurement" + "Power" ? Or maybe only "Status" ?

I think only "Status" would be the best match here.

the channel "is-raining": this is an ON/OFF information. It was tagged with "Status" + "Rain". Should we remove all the tags ? If not, what tags should we use ?

An option might be to also treat that as a special case for rule 4, i.e. it could be "Measurement"+"Rain". It can be a valid situation that we have sensors that can only provide two different measurement values for the property that they are made fore, so having a Switch item instead of Number could be accepted. Wdyt?

the channel "timestamp" (Timestamp of the last weather forecast update)

Imho on these there is usually also the semantic missing about what the timestamp actually refers to - we can have different timestamps for different channels and we cannot express the relation. I'd therefore think not putting tags is the best option here.

lolodomo commented 2 years ago

I believe we should moderate the fourth rule

Yes, I agree, given your examples.

Ok, I removed the rule 4.

lolodomo commented 2 years ago

@kaikreuzer : should I switch to "Measurement" + "Power" ? Or maybe only "Status" ?

I think only "Status" would be the best match here.

Ok, changed.

the channel "is-raining": this is an ON/OFF information. It was tagged with "Status" + "Rain". Should we remove all the tags ? If not, what tags should we use ?

An option might be to also treat that as a special case for rule 4, i.e. it could be "Measurement"+"Rain". It can be a valid situation that we have sensors that can only provide two different measurement values for the property that they are made fore, so having a Switch item instead of Number could be accepted. Wdyt?

Yes, that makes sense.

the channel "timestamp" (Timestamp of the last weather forecast update)

Imho on these there is usually also the semantic missing about what the timestamp actually refers to - we can have different timestamps for different channels and we cannot express the relation. I'd therefore think not putting tags is the best option here.

I agree. I will remove tags.

lolodomo commented 2 years ago

For the omnilink binding, we have this unique channel on the thing type "lock":

    <channel-type id="lock_switch">
        <item-type>Switch</item-type>
        <label>Lock/Unlock</label>
        <description>Lock or unlock this lock.</description>
        <category>Switch</category>
        <tags>
            <tag>OpenState</tag>
        </tags>
    </channel-type>

I am not sure that the tag is appropriate. Opening and locking are two different things, aren't they. @kaikreuzer, what would be your advice here. On the "zone" thing type, we have another channel with this tag, the channel "contact" (channel type "zone_contact") but here it looks to me appropriate.

kaikreuzer commented 2 years ago

@lolodomo I'd agree with you that being unlocked and being open are two different things, so the tag is probably not appropriate here.

lolodomo commented 2 years ago

For the millheat binding, we have several channel types with tags Setpoint + Temprature. It corresponds to different target temperatures, one for the confort mode, one for the away mode, one for vacuation, and so on. I am not sure if this is expected to have the tags on all channels or if they should be only on once ?

kaikreuzer commented 2 years ago

Pfew, that's a tricky case indeed. I would expect it to be on the one that actually applies at that moment, but as this depends on the mode, one cannot tell which channel it is. Ideally, the binding would model a setpoint channel that sets the setpoint in the device for the currently active mode - then this channel would indeed deserve the tags. So in the case here, the logical conclusion would be to remove the tags from all setpoints. Wdyt?

lolodomo commented 2 years ago

Pfew, that's a tricky case indeed. I would expect it to be on the one that actually applies at that moment, but as this depends on the mode, one cannot tell which channel it is. Ideally, the binding would model a setpoint channel that sets the setpoint in the device for the currently active mode - then this channel would indeed deserve the tags. So in the case here, the logical conclusion would be to remove the tags from all setpoints. Wdyt?

Having one unique channel is not possible as the user needs to set different temperature target temperatures for the different "periods" whatever is the current "period". But reading the README of the binding and having a better look, I think we should keep tags only on the channel type "targetTemperatureHeater" as it is for the current mode.

lolodomo commented 2 years ago

@Markinus @kaikreuzer : for the km200 binding, some channel types are built dynamically sometimes with tags. The only tags that are set by the binding are "CurrentTemperature" and "TargetTemperature" which are non official semantic tags. https://github.com/openhab/openhab-addons/blob/da59cdd255a66275dd7ae11dd294fedca4942d30/bundles/org.openhab.binding.km200/src/main/java/org/openhab/binding/km200/internal/handler/KM200ThingHandler.java#L119 https://github.com/openhab/openhab-addons/blob/da59cdd255a66275dd7ae11dd294fedca4942d30/bundles/org.openhab.binding.km200/src/main/java/org/openhab/binding/km200/internal/handler/KM200ThingHandler.java#L121 Should I remove them or are they required by the binding ?

lolodomo commented 2 years ago

@marcelrv @kaikreuzer : for the miio binding, it looks like channel types are created dynamically taking the tags defined in database/*.json files. Here we are exactly in what we do not want, like for example a lot of channels with only tag "Status". A massive cleanup of this database would be required.

kaikreuzer commented 2 years ago

Should I remove them or are they required by the binding ?

I wouldn't know, how they could be required - I'd hence suggest to remove them.

A massive cleanup of this database would be required.

@marcelrv Would that be fine for you?

marcelrv commented 2 years ago

@kaikreuzer In the past year indeed I've put quite some effort in trying to add semantic tags to the channels. the semantic model is dat from complete to be able to put full tag to all d significant channels. Esp for something like status.

I'd rather see someone take stab at improving then and come with meaningful additions to the semantic model than just removing them.

So to the question are they nessesary: not at all If the question is do I go ahead and remove them all: I will most definitely NOT spend my time on that

lolodomo commented 2 years ago

I just discovered that loxone and webthing are two other bindings using non standard tags. For loxone: Lighting, CurrentTemperature, Blinds, Switchable, Scene For webthing: Switchable, Lighting, ContactSensor, TargetTemperature, CurrentTemperature

lolodomo commented 2 years ago

... and also the sensibo binding (TargetTemperature).

lsiepel commented 1 year ago

Would it cool if this was secured in may be SAT or https://www.openhab.org/schemas/thing-description-1.0.0.xsd Was not aware of this topic and it is not mentioned in openhab-docs. Just openend an issue so that it may be added to the developer docs.

jlaur commented 1 year ago

Just openend an issue so that it may be added to the developer docs.

Or maybe even user docs? After all, deriving consistent default tags should have user impact, otherwise there would be no point. So if we could explain the rules in relation to the end result, this might be worth doing.

lsiepel commented 1 year ago

To add a questions to the 'rules' discussion i want to point to https://github.com/openhab/openhab-addons/pull/14160

Some tags where added (Setpoint + Temperature) to the "Intended Boiler Temperature" channel. While this channel is read-only, it does hold the setpoint for that temperature. It is not writable from openHAB, but it is controlled by the device.

So how would this channel be tagged? If Setpoint+Temperature is wrong, then should the rules be adapted to never ever allow control + read-only?

I hardly see any bindings with default tags, and that might be because these rules are not clear yet. I really think the out-of-the-box experience could be improved by adding as much as tags as possible (offcourse only by rules and that make sense). I'm willing to spend a lot of time on many bindings to add those tags, but only if we cvan come up with some more rules / guidance.

openhab-bot commented 1 year ago

This issue has been mentioned on openHAB Community. There might be relevant details there:

https://community.openhab.org/t/setting-semantic-class-from-binding/146975/4