Improve modelling rules for light channels

[andrewfg]

See discussion in openhab/openhab-webui#3325
See also openhab/openhab-webui#3344

Signed-off-by: Andrew Fiddian-Green software@whitebear.ch

[andrewfg]

@mherwege for info..

[netlify]

✅ Thanks for your pull request to the openHAB documentation! The result can be previewed at the URL below (this comment and the preview will be updated if you add more commits).

Built without sensitive environment variables

Name	Link
🔨 Latest commit	5421ecd
🔍 Latest deploy log	https://app.netlify.com/projects/openhab-docs-preview/deploys/68c09b509be342000804a860
😎 Deploy Preview	https://deploy-preview-2552--openhab-docs-preview.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

---

To edit notification comments on pull requests, go to your Netlify project configuration.

[andrewfg]

This seems to have failed the CI check due to reasons not related to the new/modified markdown content.

[andrewfg]

@jlaur / @lolodomo you probably remember when I wrote the Hue API v2 binding we had some (heated) discussion about the OH "rules" governing what channels a light thing should and should not implement. Well in a recent discussion with @mherwege concerning the Web UI Location card I needed to refer to those rules. But it seems that they were never properly documented as such. There is a FAQ which partly and not very explicitly describes some suggestions. So I have reformulated this FAQ to make it more like rules and more explicit concerning implementation. Therefore please feel free to 1) comment on this PR, and 2) refer to it when you next review a light binding.

[mherwege]

LGTM

[andrewfg]

@florian-h05 the CI build failed due to an npm error which I think is not related to my markdown text, but could you please check and let me know if I need to change something?

[florian-h05]

I have retriggered CI, seems there was a temporary issue with the npm registry.
The build is now properly working and reporting markdownlint errors.

[andrewfg]

@florian-h05 many thanks. I have now fixed the lint errors.

[florian-h05]

Looks good to me, thanks!

Probably one of @openhab/add-ons-maintainers can also have a look.

[lsiepel]

Thank you for providing these rules in the docs. Some textual improvements.

Would it be possible to provide some core utility classes to make this easy for developers to adapt and/or contribute? It might be usefull to mention or provide a link to the helpers.

Maybe also add a not that brigtness channels like a display should not be tagged as they are not considered a light ?

[andrewfg]

Would it be possible to provide some core utility classes to make this easy for developers to adapt and/or contribute?
It might be useful to mention or provide a link to the helpers.

What do you have in mind? Maybe conversion between color HSBType B part <=> Percentype brightness <=> OnOffType brightness != 0 ?? Or do you have something else in mind? Alternatively maybe a link to sample implementation ??

Maybe also add a note that brightness channels like a display should not be tagged as they are not considered a light ?

No. The tag combination Control+Brightness is 100% correct for a point that controls the brightness of a display. There is admittedly a question about whether the UI Location Card should interpret Control+Brightness as being (only) a light. However that is a separate discussion relating to the UI logic, and NOT the tagging logic.

[lsiepel]

Would it be possible to provide some core utility classes to make this easy for developers to adapt and/or contribute?
It might be useful to mention or provide a link to the helpers.

What do you have in mind? Maybe conversion between color HSBType B part <=> Percentype brightness <=> OnOffType brightness != 0 ?? Or do you have something else in mind? Alternatively maybe a link to sample implementation ??

Yes, an implementation example is really helpfull. I'm not that familiair to core helpers regarding color, but i guess developers face the same problems. Conversion helpers between individual channels and HSBType, with or without warm/cold white. I have to look at what you did for the zwavejs binding, it did envolve some logic that many developers are not immediate familiar with.

[mherwege]

No. The tag combination Control+Brightness is 100% correct for a point that controls the brightness of a display. There is admittedly a question about whether the UI Location Card should interpret Control+Brightness as being (only) a light. However that is a separate discussion relating to the UI logic, and NOT the tagging logic.

I understand that, but without that assumption, we can never restore the UI functionality as it was before. We do not have a way to distinguish a light from something else if we do not force it to be part of a LightSource Equipment. And we did not have that requirements before to simplify modelling. After all, forcing creation of an extra group item containing a single item looks ridiculous. And that's what was avoided before.
We have a similar challenge with the exception we made to allow points on a location, e.g. for a light connected to a hub equipment in one location, but physically controlling the light in another location. Also there, we count these points as lights.

One way around this could be to allow the LightSource Equipment tag to be added directly to the Point item if the Equipment is represented by a single point. But that is a much bigger change, not only in core, but in addons and very much so the UI.
Another way is to introduced extra properties distinguishing between properties for lights and for other uses.

[andrewfg]

@mherwege I suppose we could say that Control+Brightness shall be used uniquely for real lights, and (say) Control+Illuminance or Control+Level shall be (mis-)used for displays .. but it seems rather clunky.

[mherwege]

@mherwege I suppose we could say that Control+Brightness shall be used uniquely for real lights, and (say) Control+Illuminance or Control+Level shall be (mis-)used for displays .. but it seems rather clunky.

Yes, agree it feels clunky. But I don't see another easy solution. Allowing a combination of Equipment and Point tags on the same item feels clunky as well. It could solve the modelling issue, but is not easy to generate automatically from channel and thing definitions and has a whole lot of consequences throughout the code.

[lsiepel]

Let's invite @openhab/add-ons-maintainers to share their thoughts.

[andrewfg]

Apropos new helper methods in core .. I would add a link to this class and the ColorUtil class once that PR will have been reviewed and merged.

[andrewfg]

I have been thinking about this some more. And I think we are over thinking the issue.

Let us go back to basics:

1. If an item implements Switch+Light or Control+Brightness or Control+Color then it DOES represent a light. Period.
2. And if it does represent a light it shall be included in the location card. Period.
3. And if the user does not want this light to appear in the card (because he considers it not to be a light but rather a display or whatever) then he can simply manually de-tag or re-tag that item. This is up to the user. Period.
4. As a general rule bindings shall not offer more than one channel with Switch+Light or Control+Brightness or Control+Color default tags unless each such channel does represent a different light.

[lsiepel]

What are your thoughts on the display (control+brightness) of example? How would that fit in those statements (1).

I know it is easy to loose yourself with exemptions.

[andrewfg]

What are your thoughts on the display (control+brightness) of example? How would that fit in those statements (1)

It is a light.

[mherwege]

It is a light.

Can it be switched on and off as well? Is the binding code allowing the channel to be connected to a switch item? I would not consider this as a light, it is the dim level of a display.

[andrewfg]

Can it be switched on and off as well?

No.

@mherwege my suggestion was to not lose sleep over such things; so what is your counter suggestion?

[mherwege]

@andrewfg My preference would be to remove the semantic tags on such a channel. Otherwise, the light badges will always show at least on light on in a room where this device is, and it will cost most people a lot of time to figure out where it comes from. Rather have them add it manually. I would reserve the light, brightness and color tags for use on real lights. This is just my opinion.

[stefan-hoehn]

All: let me know when you are done - I trust your reviews - I will then merge