search

eclipse-archived / smarthome

Eclipse SmartHome™ project
https://www.eclipse.org/smarthome/
Eclipse Public License 2.0
862 stars 787 forks source link

JSR223 rule trigger fires although no trigger has been defined for item. #4279

Open euphi opened 6 years ago

euphi commented 6 years ago

Setup:

Details:


_BTW: I'm aware that I could achieve the goal of converting an uptime in seconds to a readable string in a more elegant way by using a transformation script._
kaikreuzer commented 6 years ago

@smerschjohann Can you provide help here?

euphi commented 6 years ago

Maybe also @steve-bate can have look to ensure that the problem is not inside openhab2-jython . (I'm quite sure, it is NOT).

Which loging instance should be set to DEBUG to see when triggers are added?

I also checked the group members as seen by jython, by adding a AbstractConsoleCommandExtension to the karaf console that shows group members, the output is as expected.

def execute(self, args, console):
    group = itemRegistry.getItem(args[0])
    print unicode(group) + " members:"
    for i in group.getAllMembers():
        print "\t"+unicode(i.name)

Output:

homie_uptime (Type=GroupItem, Members=7, State=245825, Label=null, Category=null) members:
        heizung_uptime
        lab_uptime
        ian_t_uptime
        ian_b_uptime
        kueche_uptime
        aog_t_uptime
        balkon_uptime
steve-bate commented 6 years ago

The root problem is that the ESH GenericEventTriggerHandler uses a substring check on the OSGi event topic to determine if an event should cause a rule to be fired.

 if (!event.getTopic().contains(source)) {
    return;
}

Since "lab_uptime_1" is a substring of "lab_uptime_1_str" it caused the GenericEventTriggerHandler to trigger rules when it should not have done so. This seems like a bug to me, but others may consider it a charming implementation quirk. ;-)

To compensate, I've modified the Jython code to provide a more specific eventSource that will be less likely to pass the substring check. I also made the eventTopic more specific as a minor performance optimization. It seems to be working correctly in my development environment. I've pushed the change to the esh_event_trigger branch of my openhab2-jython repo. Will you try it and confirm that it works for you? If so, I'll merge the change into master.

euphi commented 6 years ago

@steve-bate Yes, this solves the issue for me. Many thanks.

However, I also consider the substring-check a bug :-)

maggu2810 commented 6 years ago

@steve-bate I agree that a substring matching is not good. At least it also differs from my understanding.

WRT to the implementation that should be fixed IMHO the test in void receive(Event event) could be removed at all because the JavaDoc already states: This method is called for every event where the event subscriber is subscribed to and the event filter applies. Or do we call receive also from other places?

steve-bate commented 6 years ago

WRT to the implementation that should be fixed IMHO the test in void receive(Event event) could be removed at all because the JavaDoc already states: This method is called for every event where the event subscriber is subscribed to and the event filter applies.

@maggu2810 I don't know enough about the SmartHome internals to have a strong opinion about it. However, the comment refers to when the receive(Event event) method should be invoked and says nothing about what happens inside the receive method implementation. On the other hand, it does seem like this extra filtering may be handled with the OSGi event topic pattern matching that's done before the method is called.

smerschjohann commented 6 years ago

Last time I've looked into it, it would be enough to filter in the apply() method and remove the other check and yes, I would say this is a bug.