Skip to content
This repository has been archived by the owner on May 17, 2021. It is now read-only.

Jsr223 fixes and updates #3576

Closed
wants to merge 4 commits into from
Closed

Jsr223 fixes and updates #3576

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
11d3c4d
Fixed UpdatedEventTrigger in jsr223 nashorn engine
gerrieg Dec 20, 2015
07a46ad
Fixed TypeParser for UnDefType parsing
gerrieg Dec 21, 2015
2f697f3
Added additional ChangedEventTrigger constructor to simplify scripts
gerrieg Dec 21, 2015
b88e5c7
Added additional CommandEventTrigger constructor to simplify scripts
gerrieg Dec 21, 2015
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions ...re.jsr223/src/main/java/org/openhab/core/jsr223/internal/engine/scriptmanager/Script.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -153,6 +153,7 @@ private void initializeNashornGlobals() {
+"Rule = Java.type('org.openhab.core.jsr223.internal.shared.Rule'),\n"
+"ChangedEventTrigger = Java.type('org.openhab.core.jsr223.internal.shared.ChangedEventTrigger'),\n"
+"CommandEventTrigger = Java.type('org.openhab.core.jsr223.internal.shared.CommandEventTrigger'),\n"
+"UpdatedEventTrigger = Java.type('org.openhab.core.jsr223.internal.shared.UpdatedEventTrigger'),\n"
+"Event = Java.type('org.openhab.core.jsr223.internal.shared.Event'),\n"
+"EventTrigger = Java.type('org.openhab.core.jsr223.internal.shared.EventTrigger'),\n"
+"ShutdownTrigger = Java.type('org.openhab.core.jsr223.internal.shared.ShutdownTrigger'),\n"
Expand Down
14 changes: 8 additions & 6 deletions ...ore.jsr223/src/main/java/org/openhab/core/jsr223/internal/shared/ChangedEventTrigger.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,18 +23,20 @@ public class ChangedEventTrigger implements EventTrigger {
private State fromState;
private State toState;

public ChangedEventTrigger(String itemName) {
this(itemName, null, null);
}

public ChangedEventTrigger(String itemName, State toState) {
this(itemName, null, toState);
}

public ChangedEventTrigger(String itemName, State fromState, State toState) {
this.itemName = itemName;
this.fromState = fromState;
this.toState = toState;
}

public ChangedEventTrigger(String itemName) {
this.itemName = itemName;
this.fromState = null;
this.toState = null;
}

@Override
public boolean evaluate(Item item, State oldState, State newState, Command command, TriggerType type) {
return type == TriggerType.CHANGE && item.getName().equals(itemName)
Expand Down
4 changes: 4 additions & 0 deletions ...ore.jsr223/src/main/java/org/openhab/core/jsr223/internal/shared/CommandEventTrigger.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,10 @@ public class CommandEventTrigger implements EventTrigger {
private String itemName;
private Command command;

public CommandEventTrigger(String itemName) {
this(itemName, null);
}

public CommandEventTrigger(String itemName, Command command) {
this.itemName = itemName;
this.command = command;
Expand Down
6 changes: 5 additions & 1 deletion bundles/core/org.openhab.core/src/main/java/org/openhab/core/types/TypeParser.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,11 @@ public class TypeParser {
*/
public static State parseState(List<Class<? extends State>> types, String s) {
for(Class<? extends Type> type : types) {
try {
try {
// special UnDefType handling, valueOf does not work because of the toString method.
if (type.getName().equals(UnDefType.class.getName())) {
return UnDefType.parse(s);
}
Copy link
Contributor

Choose a reason for hiding this comment

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

please see comment on UnDefType

Method valueOf = type.getMethod("valueOf", String.class);
State state = (State) valueOf.invoke(type, s);
if(state!=null) return state;
Expand Down
16 changes: 16 additions & 0 deletions bundles/core/org.openhab.core/src/main/java/org/openhab/core/types/UnDefType.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,4 +34,20 @@ public String format(String pattern) {
return String.format(pattern, this.toString());
}

/**
* Returns the enum constant with the specified name.
*/
public static UnDefType parse(String name) {
if (name == null) {
throw new NullPointerException("Name is null");
}
if (UNDEF.toString().equals(name)) {
return UNDEF;
} else if (NULL.toString().equals(name)) {
return NULL;
}
throw new IllegalArgumentException("No enum constant "
+ UnDefType.class.getCanonicalName() + "." + name);
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Why not valueOf instead of parse so you don't need a special case in TypeParser? What do you mean in the comment "valueOf does not work because of the toString method"?

Also, what is the likely impact of this core change? Could you separate out the UnDefType-related changes since they are wider in scope than Jsr223?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I can't override the valueOf method in an enum:

The enum UnDefType already defines the method valueOf(String) implicitly

Try it:

UnDefType.valueOf("NULL"):  Uninitialized
UnDefType.valueOf("UNDEF"): Undefined

UnDefType.valueOf(UnDefType.NULL.toString()): IllegalArgumentException: No enum constant org.openhab.core.types.UnDefType.Uninitialized
UnDefType.valueOf(UnDefType.UNDEF.toString()): IllegalArgumentException: No enum constant org.openhab.core.types.UnDefType.Undefined

The TypeParser never worked with the UnDefType

I found this problem while i wrote a jsr223 script, i wanted to update a item:

postUpdate("ITEM_NAME", UnDefType.UNDEF);

i got the IllegalArgumentException and searched the reason

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes, this is a case where a core type's toString() result is not acceptable to the class's valueOf static method. But I'm concerned that there would be wide ripple effects from this change, as some item classes list UnDefType ahead of other types in the class's acceptedDataType list, so other code elsewhere in openHAB could experience a breaking change. Could a code change to fix the call from a Jsr223 script to

postUpdate("ITEM_NAME", UnDefType.UNDEF);

happen in the Jsr223 code? Is the script language just unable to reference Java enums using that syntax?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is a general bug, i am surprised that this did not pop up earlier. I can implement a workaround especially for jsr223, but i think the TypeParser is the correct place and fixes it globally.
It only affects the UnDefType class, so there should not be much side effects i think. My installation runs fine without any problems. Maybe we should ask @kaikreuzer or @teichsta ?

Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe we should ask @kaikreuzer or @teichsta ?

👍

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes i know, postUpdate(ITEM, UnDefType.UNDEF) works as expected.

Note that you can't just do UnDefType.UNDEF.toString() for the second argument because depending on the item type it might match another acceptable type. StringItem is one example where that would happen.

The TypeParser uses foreach for the getAcceptedDataTypes list. foreach uses the iterator from the ArrayList which returns the entries in proper sequence. In the StringItem, StringType is the first acceptedDataType which always match first.

In each Item, the UnDefType is the last acceptedDataType, except the RollershutterItem. But this item works too, because it's values never matches the string NULL or Undefined.

Copy link
Contributor

Choose a reason for hiding this comment

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

OK, I suppose I'm not clear then about the JSR223 problem being solved with this change.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It is solved, i am currently writing some JSR223 scripts and with this changes it works as expected for some days now without any issues so far.

Copy link
Contributor

Choose a reason for hiding this comment

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

That's great, but the specific problem the change is intended to fix is still not clear to me. I have many JSR223/Jython scripts and they work as expected even without this change. I agree with @watou that this should be broken out into a separate PR.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I am using JavaScript an run into this issue. OK, then close this PR and i can create two separate


}