Skip to content

[utils] javaify/jsify: Handle wrapped openHAB types - #550

Merged
florian-h05 merged 2 commits into
openhab:mainfrom
Nadahar:extendify
Jul 19, 2026
Merged

[utils] javaify/jsify: Handle wrapped openHAB types#550
florian-h05 merged 2 commits into
openhab:mainfrom
Nadahar:extendify

Conversation

@Nadahar

@Nadahar Nadahar commented Jul 17, 2026

Copy link
Copy Markdown
Contributor

I don't have the overview, so I've only handled the types I could find by searching the code. I'm a bit unsure about the QuantityQuantityType conversion, and if that really is desirable..?

Anyway, any other wrapped types that I've missed should be added as well, but this shows "the principle" of how I think these should be handled.

Signed-off-by: Ravi Nadahar <nadahar@rediffmail.com>
@Nadahar

Nadahar commented Jul 17, 2026

Copy link
Copy Markdown
Contributor Author

@florian-h05 It fails on tests, but the errors seem to be unrelated to what I've done, so I'm not sure what to do about it.

edit: Perhaps it's some circular dependency issue? I really don't know how the files are organized, so I don't know to figure it if that's the case, or how to solve it.

Signed-off-by: Ravi Nadahar <nadahar@rediffmail.com>
@Nadahar

Nadahar commented Jul 17, 2026

Copy link
Copy Markdown
Contributor Author

I moved the problematic "requires" inside the function. If that's not kosher, I think these functions must reside somewhere else than in utils.js.

@florian-h05 florian-h05 left a comment

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.

Code LGTM, thanks!

Comment thread src/utils.js
return val;
}

// Note: Inline requires are used here to prevent circular dependencies during startup, as utils.js is imported by these very classes.

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.

Unfortunately I have no better idea how to handle this ...

@florian-h05

Copy link
Copy Markdown
Contributor

I'm a bit unsure about the Quantity ⇄ QuantityType conversion, and if that really is desirable..?

IMO it is desirable.

@Nadahar
Nadahar marked this pull request as ready for review July 17, 2026 15:34
@Nadahar
Nadahar requested a review from a team as a code owner July 17, 2026 15:34
@Nadahar

Nadahar commented Jul 17, 2026

Copy link
Copy Markdown
Contributor Author

If you think it looks good as it is, there's no need for me to do anything more - but let's check if there's anything more we have forgotten. What about the "event object"? It's not entirely clear to me how that is handled and if it's even relevant to pass it on.

@Nadahar

Nadahar commented Jul 17, 2026

Copy link
Copy Markdown
Contributor Author

The way I read the code, _getTriggeredData() is applied to the whole context for SimpleRules. For other rules, it's not invoked at all. Is that correct?

If so, it means that we have JS rules that both deal with the _getTriggeredData() output, without having access to the original, and others that just deal with the original, Java map?

@florian-h05

florian-h05 commented Jul 19, 2026

Copy link
Copy Markdown
Contributor

What about the "event object"? It's not entirely clear to me how that is handled and if it's even relevant to pass it on.

The event object is a "simple" object of key value pairs, so javaify will make it a Map. As GraalJS allows for .key access on Java Maps, it should be fine. Apart from that, I don't think the event object will be passed around ...

The way I read the code, _getTriggeredData() is applied to the whole context for SimpleRules. For other rules, it's not invoked at all. Is that correct?

It is also invoked for "standard" rules (ScriptModuleHandler-based ones), it is done through code injection inside the add-on there:

https://github.com/openhab/openhab-addons/blob/2f7cd2cca8463d74393fd70a61a2f19405d2bc45/bundles/org.openhab.automation.jsscripting/src/main/java/org/openhab/automation/jsscripting/internal/OpenhabGraalJSScriptEngine.java#L114

What's executed in ScriptModuleHandler cannot be wrapper like done with the execute callback for SimpleRules, so we have to convert the event object through a different path.

@florian-h05 florian-h05 added the enhancement New feature or request label Jul 19, 2026
@florian-h05 florian-h05 added this to the to be released milestone Jul 19, 2026
@florian-h05 florian-h05 added bug Something isn't working and removed enhancement New feature or request labels Jul 19, 2026
@florian-h05 florian-h05 changed the title Handle wrapped OH types [utils] javaify/jsify: Handle wrapped OH types Jul 19, 2026
@florian-h05 florian-h05 changed the title [utils] javaify/jsify: Handle wrapped OH types [utils] javaify/jsify: Handle wrapped openHAB types Jul 19, 2026
@florian-h05
florian-h05 merged commit e1ee37e into openhab:main Jul 19, 2026
6 checks passed
@Nadahar
Nadahar deleted the extendify branch July 19, 2026 15:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants