-
-
Notifications
You must be signed in to change notification settings - Fork 722
jsr223: clarifications on Configuration parameters #2578
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
base: main
Are you sure you want to change the base?
Conversation
166b429 to
b4c152e
Compare
✅ 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
To edit notification comments on pull requests, go to your Netlify project configuration. |
✅ 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
To edit notification comments on pull requests, go to your Netlify project configuration. |
44963eb to
5c9bb0a
Compare
5c9bb0a to
ab26b35
Compare
|
Can you please ask these questions in the forum before creating a doc PR? Thanks. |
Accepting this change is irrelevant on whether the above question will get an answer. I mean, the added text is correct on its own. |
|
Then remove the question from the description as it is otherwise excepted to be answered here before closing this PR. |
|
I have shortened the description, but I do not see big value in such changes. |
|
@dilyanpalauzov Do you mind reflecting on my comments and the intent of PRs! PRs like that are appreciated but any PR has to be reviewed by maintainers like me and all of us take this serious. So, if you write a description I need to read all of this and then reflect on it. If it contains a lot of information that is not relevant to the PR, it still takes a lot of my valuable time to reflect on this which I cannot put into other PRs. Question should therefore be discussed in the forum where even more people can have a look into your questions. I hope therefore that you do now see a big value in not doing this. |
|
@florian-h05 can you review this? |
configuration/jsr223.md
Outdated
|
|
||
| The following trigger types are defined by openHAB (custom triggers can also be defined) and take the specified configuration parameters. | ||
| The following trigger types are defined by openHAB (custom triggers can also be defined) and take the specified parameters in the constructor of [`org.openhab.core.config.core.Configuration`](https://www.openhab.org/javadoc/latest/org/openhab/core/config/core/configuration). | ||
| The value of the map in each configuration must be of type String, Number or Boolean. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Where is that limitation put in place?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It is in ConfigUtil.normalizeType(). It accepts as value also a Collection, but I could not get it running with Groovy/JSR223. The collection ["A", "B"] was converted to String and then this string had to be the watched state in the trigger.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I tried to pass in the configuration map values an item, a collection (ArrayList), a state and a command. The demonstration below is for UnDef, which is a state and cannot be a command, but with command the result is the same.
The fact that as map value item, state and command are rejected with the message „ of configuration value!“ means that ConfigUtil.normalizeType() fails validating, as „ of configuration value!“ appears only in ConfigUtil.normalizeType().
I think it would be more comfortable if states, commands, items and collections can be passed, and then the Configuration does .getName()/.toString() on them - if no better shortcuts exist. Also passing a collection would be nice, let’s say state: ['A', 'B'] creates two triggers with the same id and state:'A' respectively state:'B', or if the collection is after itemName: then many triggers, where only the item is changed.
import org.openhab.core.automation.Action
import org.openhab.core.config.core.Configuration
import org.openhab.core.automation.module.script.rulesupport.shared.simple.SimpleRule
import org.openhab.core.automation.util.TriggerBuilder
scriptExtension.importPreset('provider')
scriptExtension.importPreset('RuleSupport')
m = new org.openhab.core.library.items.StringItem("v")
itemRegistry.add(m)
automationManager.addRule(new SimpleRule() {
{
name = "Demonstration for /openhab-docs/pull/2578"
description = "Pass as configuration map value string, item, collection, state and command"
uid = "gh-docs-2576"
triggers = [
/*
* fails: key cannot be an item, it must be string
* TriggerBuilder.create().withId('a').withTypeUID('core.ItemStateChangeTrigger')
* .withConfiguration(new Configuration([itemName: m])).build()
* [ERROR] [ipt.internal.ScriptEngineManagerImpl] - Error during evaluation of script '/etc/openhab/automation/jsr223/a.groovy': javax.script.ScriptException: java.lang.IllegalArgumentException: Invalid type '{org.openhab.core.library.items.StringItem}' of configuration value!
* “of configuration value!” appears only in the source code of ConfigUtil.normalizeType()
*/
// next line has collecion as value, but means state must be the string "[A, B]"
TriggerBuilder.create().withId('a').withTypeUID('core.ItemStateChangeTrigger')
.withConfiguration(new Configuration([itemName: 'v', state: ['A', 'B']])).build(),
/* fails: key cannot be a state or a command
* TriggerBuilder.create().withId('a').withTypeUID('core.ItemStateChangeTrigger')
* .withConfiguration(new Configuration([itemName: 'v', state: UNDEF])).build()
* ERROR] [ipt.internal.ScriptEngineManagerImpl] - Error during evaluation of script '/etc/openhab/automation/jsr223/a.groovy': javax.script.ScriptException: java.lang.IllegalArgumentException: Invalid type '{org.openhab.core.types.UnDefType}' of configuration value!
*/
];
}
Object execute(Action a, Map<String, Object> inputs) {
org.slf4j.LoggerFactory.getLogger('C').error('Executed! item is ' + m.state)
}
});
org.slf4j.LoggerFactory.getLogger('TYPE').error('TYPE IS ' + ['A', 'B'].getClass().toString())
Thread.sleep(2000)
events.postUpdate('v', 'A') // does not fire
Thread.sleep(3000)
events.postUpdate('v', '[A, B]') // does firewhich prints only
2025-10-27 14:33:43.665 [INFO ] [ort.loader.AbstractScriptFileWatcher] - (Re-)Loading script '/etc/openhab/automation/jsr223/a.groovy'
2025-10-27 14:33:43.981 [ERROR] [TYPE ] - TYPE IS class java.util.ArrayList
2025-10-27 14:33:49.006 [ERROR] [C ] - Executed! item is [A, B]
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What you get running and what not in Groovy is irrelevant for the generic docs.
Looking at ConfigUtil::normalizeType, it does support more than String, Number or Boolean. There is also a more generic code for normalizing based on the Parameter.
Better link to the descriptions of the actual trigger parameters.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have removed this text:
The value of the map in each configuration must be of type String, Number or Boolean.
so that this change can make progress. It is relevant what can be utilized by Groovy, as there seem to be many methods involves and at the end for me only String, Number and Boolean work. In particular it does not work with Command or State. I have not tried with ConfigDescriptionParameter because I do not know how to use it.
I have removed the question from the description. Should the questions from #2573 also be removed? |
I always wondered when I start with
TriggerBuilder.withConfiguration(new Configuration(what to write as next. The current changes makes it clearer.