Skip to content

DSL Scripts and Rules: import static all Types/State/Command enums - #5509

Open
dilyanpalauzov wants to merge 1 commit into
openhab:mainfrom
dilyanpalauzov:script_import_type_enums
Open

DSL Scripts and Rules: import static all Types/State/Command enums#5509
dilyanpalauzov wants to merge 1 commit into
openhab:mainfrom
dilyanpalauzov:script_import_type_enums

Conversation

@dilyanpalauzov

Copy link
Copy Markdown
Contributor

This change makes using all Type/Command/State enums as static import simpler … by reducing the code size with 30 lines. It removes all current usages of class org.openhab.core.model.script.scoping.StateAndCommandProvider.

Before this change a global variable

val a5 = ON

rule "System started" … 

produced

2026-04-22 15:06:26.237 [INFO ] [el.core.internal.ModelRepositoryImpl] - Validation issues found in DSL model 'a.rules', using it anyway:
Cannot reference the field 'ON' before it is defined

Now the above works as expected.

@dilyanpalauzov
dilyanpalauzov requested a review from a team as a code owner April 22, 2026 13:05
@Nadahar

Nadahar commented Apr 22, 2026

Copy link
Copy Markdown
Contributor

I'm not sure exactly what you're changing to make the error disappear, but I think removing the centrally defined definition is a bad idea. It means more places in the code to maintain/be aware of that exists, leading to more bugs and inconsistencies.

I assume that the cause for the validation failure has to do with timing, it should be possible to resolve the timing issue without removing the central definition of states and commands?

@dilyanpalauzov

Copy link
Copy Markdown
Contributor Author

I think removing the centrally defined definition is a bad idea.

I am removing centrally defined hacks. ON is now implicit static import, as is ZonedDateTime.now. The centrally defined hacks applied for the body of the rule, not for the global variables.

It means more places in the code to maintain/be aware of that exists, leading to more bugs and inconsistencies.

Here exactly the opposite is the case. OnOffType.ON is handled exactly in the same way as ZonedDateTime.now.

Comment on lines +114 to +123
result.add(IncreaseDecreaseType.class);
result.add(NextPreviousType.class);
result.add(OnOffType.class);
result.add(OpenClosedType.class);
result.add(PlayPauseType.class);
result.add(RefreshType.class);
result.add(RewindFastforwardType.class);
result.add(StopMoveType.class);
result.add(UnDefType.class);
result.add(UpDownType.class);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

@dilyanpalauzov Here you have hardcoded them. Instead, they should be retrieved from a centralized location, so that all code use "the same list" of states and commands.

@Nadahar

Nadahar commented Apr 26, 2026

Copy link
Copy Markdown
Contributor

I am removing centrally defined hacks. ON is now implicit static import, as is ZonedDateTime.now. The centrally defined hacks applied for the body of the rule, not for the global variables.

I made a comment to try to point out what I was referring to. The structural changes you have made to where things are inserted I can't really evaluate, I know too little of the consequences of this.

But, wherever you insert them, you must have a centrally defined "list" of states and commands to insert. This list should be the same here as in parsers, serializers and similar code, which is what StateAndCommandProvider provides. It might be that the location of this "common definition" is wrong, the definition should probably live in org.openhab.core.types instead (which would make them available without extra bundle dependencies in almost all cases), but my main point is to not revert back to having them hardcoded in various locations in the codebase.

@dilyanpalauzov

Copy link
Copy Markdown
Contributor Author

There is no need in this context to have a place containing a list of all command and all states.

In JSR223 context, the default preset contains a list of some magically inserted items - variables, static enum values, classes - https://next.openhab.org/docs/configuration/jsr223.html#default-preset. "Rule DSL”/"Script DSL” is here similar in concept to that default preset, and the hacks exist, because DSL Rules and DSL Scripts do not use presets (thus also not the default preset), so all thing from the default preset have to be repeated in a different way for the "DSL Scripts” and “DSL Rules”. They serve only the purpose that the user does not have to type “import …" at the beginning.

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.

2 participants