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

Extend OnOffType so it can be used as drop-in-replacement for Number types #3923

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

Cybso
Copy link

@Cybso Cybso commented Dec 14, 2023

I have added the methods of java class Number to OnOffType. This allows items of this class to be used as drop-on-replacement for numeric types in scripted rules:

    if (ir.getItem("someItem").getState().intValue() > 0) { .... }

Additionally, I have added a method booleanValue(), which allows this state to be used without importing OnOffType first.

@Cybso Cybso requested a review from a team as a code owner December 14, 2023 08:43
…types in rules

Signed-off-by: Roland Tapken <dev@cybso.de>
@wborn
Copy link
Member

wborn commented Jan 16, 2024

It's less code and easier to understand if you just use a static import for OnOffType.ON:

if (ir.getItem("someItem").getState() == ON) {

@Cybso
Copy link
Author

Cybso commented Jan 16, 2024

It's less code and easier to understand if you just use a static import for OnOffType.ON:

if (ir.getItem("someItem").getState() == ON) {

Yes, but this is not what the patch intended in the first place. The patch allows to replace an Number item with a Boolean item without breaking existing rules.

@wborn
Copy link
Member

wborn commented Jan 16, 2024

OnOffType is also used in add-on code and tests. Adding these options will probably result in contributors using the type in the wrong way and maintainers having to add review comments for this.

@rkoshak
Copy link

rkoshak commented Jan 16, 2024

which allows this state to be used without importing OnOffType first.

Where do you need to import OnOffType in rules? It's imported by default in Rules DSL. jRuby I think provides a wrapper. JS Scripting always returns the state of Items as a String unless otherwise indicated. I don't know about Groovy. Is this actually solving a real problem or a perceived problem in rules.

If it's a real problem, maybe the place to correct it is in the helper library for the language instead of in core where, as @wborn points out, it could have more knock on effects than the improvement is worth.

Wouldn't it make more sense to add a getStateAs() method than all of these new methods? That would be consistent with other Types like DimmerType, ColorType, etc.

At a minimum I would expect to see just one toNumber() method. It's left to the caller to pull the primitive from the Number. Everywhere else a number appears in the OH API as seen from rules, when there's a number OH presents it as a Number, not a primitive. Consistency is important.

Also, what about ContactType?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants