Skip to content

Conversation

@andrewfg
Copy link
Contributor

@andrewfg andrewfg commented Apr 8, 2025

This PR contains some tweaks to Point and Property tags arising from #4706 and #4668

Signed-off-by: Andrew Fiddian-Green [email protected]

Signed-off-by: Andrew Fiddian-Green <[email protected]>
@andrewfg andrewfg requested a review from a team as a code owner April 8, 2025 10:59
Signed-off-by: Andrew Fiddian-Green <[email protected]>
@jimtng
Copy link
Contributor

jimtng commented Apr 8, 2025

  • No mediaplayer?
  • I think you also need to run the generate-tag-classes plugin, whenever semantictags.csv is changed, and include the generated files in the same PR. That's how I usually do it.
    mvn clean install -Pgenerate-tag-classes -pl :org.openhab.core.semantics

Signed-off-by: Andrew Fiddian-Green <[email protected]>
@andrewfg
Copy link
Contributor Author

andrewfg commented Apr 8, 2025

@jimtng I have run the script

andrewfg added 2 commits April 8, 2025 16:11
Signed-off-by: Andrew Fiddian-Green <[email protected]>
Signed-off-by: Andrew Fiddian-Green <[email protected]>
@jimtng
Copy link
Contributor

jimtng commented Apr 10, 2025

What would OPENING be for that isn't covered by OPEN_LEVEL and OPEN_STATE?

@andrewfg andrewfg changed the title Semantic Tags: Tweaks to Point and Property tags Semantic Tags: Tweaks to tags Apr 10, 2025
@andrewfg
Copy link
Contributor Author

What would OPENING be for that isn't covered by OPEN_LEVEL and OPEN_STATE?

Indeed. And we also have OpenClosed too. I think there should probably be just OpenState (binary) and OpenLevel (analog) that are children of Opening. => WDYT?

@jimtng
Copy link
Contributor

jimtng commented Apr 10, 2025

I think there should probably be just OpenState (binary) and OpenLevel (analog) that are children of Opening. => WDYT?

This sounds good.

And we also have OpenClosed too.

Should this be deleted? This sounds just like OpenState. Isn't it a new one too? So deleting it shouldn't be a problem?

@andrewfg
Copy link
Contributor Author

andrewfg commented Apr 10, 2025

deleting it shouldn't be a problem?

Indeed.

Signed-off-by: Andrew Fiddian-Green <[email protected]>
Signed-off-by: Andrew Fiddian-Green <[email protected]>
@jimtng
Copy link
Contributor

jimtng commented Apr 12, 2025

We have OpenState => Should we delete it?

No, OpenState is good. OpenClosed isn't.

We have LowBattery => We should not delete it.

This is unfortunately a legacy tag, although since we changed it to Property, it may cause some problems. In any case, I'm OK with LowBattery, but not "LowFullBattery".

LowBattery can be assigned to a SwitchItem, when it's ON, it's low battery.

But if it was LowFullBattery, it's just awkward.

In general I'm trying to say that we should not have "OnestateOppositestate" format.

@jimtng
Copy link
Contributor

jimtng commented Apr 12, 2025

TwoState is no different to Binary / OpenClosed / HighLow / AutoManual / EnabledDisabled. They describe the possible values, but not what the values mean (ie. what property they describe).

As I said, instead of AutoManual, the Item should be tagged with "Mode", and instead of EnabledDisabled, it should be tagged "State" (or Status), Instead of OnOff, it should be tagged Power, and so on.

@andrewfg
Copy link
Contributor Author

we should not have "OnestateOppositestate" format

So lets change these three as follows..

  • OnOff => Power (as in Switch + Power or Status + Power)
  • AutoManual => Mode
  • EnabledDisabled => Enabled (it's main use is for sensors which can be disabled)

Note: these three are new to this PR so changing or deleting them would only impact my own PRs. So let us make a quick decision now..

@jimtng
Copy link
Contributor

jimtng commented Apr 12, 2025

So lets change these three as follows..

  • OnOff => Power (as in Switch + Power or Status + Power)
  • AutoManual => Mode
  • EnabledDisabled => Enabled (it's main use is for sensors which can be disabled)

Yes, and Enabled shouldn't be a child of Mode. I think it should just stand alone.

Signed-off-by: Andrew Fiddian-Green <[email protected]>
Copy link
Contributor

@jimtng jimtng left a comment

Choose a reason for hiding this comment

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

Thanks! LGTM

@andrewfg
Copy link
Contributor Author

I have uploaded the as per LGTM schema file to openhab/openhab-website#511

@ccutrer
Copy link
Member

ccutrer commented Apr 12, 2025

Television and Projector lost its parent of Screen.

It depends on what me mean by screen. If it's "device that generates images", Projector and Television are both subclasses. If it's "the physical thing that images are displayed on", they are not. The difference is that a Television is both types of screens, but with a projector, you have the image generating device (the projector), and the screen the image is on. I don't think one interoperation is necessarily more correct than others (being able to automate the screen itself in a projector+screen setup is fairly common - it can roll out of the ceiling or other cavity, or have masks that are controllable). It was just previously Screen was the parent of Television and Projector, and I had rules that relied on that (when a receiver turns on, turn on the screen in that room, regardless of if it's a television or projector). I also have a motorized screen, which I just don't semantically tag (I have no reason to directly expose it in a UI, or need to generically find it and its properties in rules, because it's 100% controlled by dedicated automation rules).

  • OnOff => Power (as in Switch + Power or Status + Power)

I'm not sure "Power" is the right word here. The Power property refers to how much power (generally in watts) a device is using. Though maybe it's okay to overload it, since such a property would only ever be paired with the Measurement point, leaving Switch,Power to mean the point that can turn a device on and off, and Status,Power to mean a point you can't control, but indicates if the device is active at the moment based on some other means (such as a hot water heater that is always "on", but is now "heating" because it is below the setpoint temperature.

@andrewfg
Copy link
Contributor Author

@ccutrer please align with @jimtng and when you have agreed just tell me what to do. Just please do it quickly so when can move on.

@ccutrer
Copy link
Member

ccutrer commented Apr 12, 2025

Any objection to Display being the parent of Television and Projector, and Screen being separate (with a description of Projection Screen, so it's clear that's what is intended)?

@andrewfg
Copy link
Contributor Author

@ccutrer / @jimtng just to summarise: I need to have a unanimous proposal from you both on two topics..

  1. the parent child hierarchy of tv, screen, projector, etc.
  2. the property to use for on state

I have been dancing back and forwards on various conflicting proposals, it looks like I am wasting my time, so you two please make the decision. Fast.

@ccutrer
Copy link
Member

ccutrer commented Apr 12, 2025

  1. Just waiting on @jimtng to approve my latest proposal (Display -> (Television, Projector), Screen)
  2. I've convinced myself I'm +1 on the current proposal of using Power for on state

@jimtng
Copy link
Contributor

jimtng commented Apr 12, 2025

  1. Just waiting on @jimtng to approve my latest proposal (Display -> (Television, Projector), Screen)
  2. I've convinced myself I'm +1 on the current proposal of using Power for on state

Yes I agree on Display for the parent of Television and Projector, and add Screen separately. This is good!
Should Screen also be a child of AudioVisual?

@ccutrer
Copy link
Member

ccutrer commented Apr 12, 2025

Should Screen also be a child of AudioVisual?

Yes

@holgerfriedrich
Copy link
Member

Sorry for joining the party late. I have not been involved in previous discussions about semantics, and would leave it to the other maintainers to review.

OnOff => Power (as in Switch + Power or Status + Power)
I'm not sure "Power" is the right word here. > The Power property refers to how much power (generally in watts) a device is using.

This confuses me.
I would interpret power in the same way as stated above, because this is a common term in physics.

In addition, there might be logical switches. I am not sure if power(ed) is the correct term for those cases. Or would you see this as a mode?

@jimtng
Copy link
Contributor

jimtng commented Apr 13, 2025

I understand that Power is used in physics and engineering with the unit of Watt. It is used to measure the rate of energy per unit time.

Power is also a very common terminology in consumer electronics to label a button which is used to turn something on or off.

Here is a photo of an appliance with a power button
image

Many devices / API uses POWER to turn it on / off, e.g. Tasmota MQTT, See how it's implemented in openhab's mqtt:
https://community.openhab.org/t/help-with-a-simple-mqtt-on-off-to-sonoff-tasmota-solved/98811/5

@andrewfg
Copy link
Contributor Author

Apropos Switch Power .. there are still channels where Power is not appropriate e.g. the Zwave binary sensor (where, as you see, I simply don’t declare a Property at all)..

andrewfg/org.openhab.binding.zwave@174bb85#diff-d8b76539195caa467bce060c1a64ebcad9d02640002ab809b761e11e102c52d0R1233

@jimtng
Copy link
Contributor

jimtng commented Apr 13, 2025

Apropos Switch Power .. there are still channels where Power is not appropriate e.g. the Zwave binary sensor (where, as you see, I simply don’t declare a Property at all)..

andrewfg/org.openhab.binding.zwave@174bb85#diff-d8b76539195caa467bce060c1a64ebcad9d02640002ab809b761e11e102c52d0R1233

For such a generic sensor, we don't need to assign any property at all, and leave it to the user to decide what that sensor actually represents for them.

@andrewfg
Copy link
Contributor Author

For such a generic sensor, we don't need to assign any property at all

Indeed.

And we will not delete the Power property from the set. It is a separate discussion in other PRs in addons about whether its use therein is appropriate. In some cases it is. And in some cases not.

Signed-off-by: Andrew Fiddian-Green <[email protected]>
@andrewfg
Copy link
Contributor Author

@ccutrer / @jimtng / @holgerfriedrich see latest commit. Are we done now?

@holgerfriedrich holgerfriedrich merged commit ad7612d into openhab:main Apr 13, 2025
4 checks passed
@holgerfriedrich holgerfriedrich added this to the 5.0 milestone Apr 13, 2025
@holgerfriedrich holgerfriedrich added the enhancement An enhancement or new feature of the Core label Apr 13, 2025
@holgerfriedrich
Copy link
Member

Thanks for the discussion and the reviews!

@andrewfg
Copy link
Contributor Author

Phew that was a hard one. But many thanks to you all!

@andrewfg andrewfg deleted the poin-property-tags branch December 14, 2025 12:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement An enhancement or new feature of the Core

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants