CrazyIvan359 / openhab-helper-libraries

JSR223-Jython scripts and modules for use with openHAB
Eclipse Public License 1.0
17 stars 5 forks source link

Triggers are not reloaded on item change #11

Closed janwo closed 6 days ago

janwo commented 3 years ago

Describe the bug A clear and concise description of what the bug is.

To Reproduce Steps to reproduce the behavior: 1) Add a rule like@when("Member of gMyGroup received update"). 2) Change the members of gMyGroup. 3) Inspect the triggers via UI. 4) Added or removed members of the group are not added or removed from the rule triggers.

Expected behavior All members of the group are part of the rule triggers.

Screenshots If applicable, add screenshots to help explain your problem.

Environment (please complete the following information):

CrazyIvan359 commented 3 years ago

This is not a bug, but a limitation of OH. These libraries simulate the Member of trigger by creating a separate item trigger for each member of the group.

I know there is now an actual Member of trigger in OH3, but I don't know if it updates when the group members are changed. If it is dynamic like that then the libraries could be updated to use the native Member of trigger when on OH3.

janwo commented 3 years ago

Thanks, that sounds reasonable.👍 Is there a workaround?

Enabling and disabling rules did not work. Only re-save them triggers a reinitialization. Maybe this can be done programmatically?

CrazyIvan359 commented 3 years ago

The resave will be the easiest way.

I have some dynamically generated rules who's generating code watches for group membership changes and then deletes and recreates the rule. This approach would not be ideal unless turned into a library (which I kinda like the idea of, unless the OH3 member of trigger works with membership changes).

What is your use case for this? Maybe there is another way to handle the situation.

janwo commented 3 years ago

I like to track the last activation (e.g. Light) of an item with a given tag. Therefore a rule checks upon item changes, if every item of that tag is part of a given group, e.g. gGroupWithAllLightbulbItems. Why this? There is no trigger upon tags.

When I am changing tags, the rule adds or removes items from the group. The other rule that triggers upon group updates, e.g. on Member of gGroupWithAllLightbulbItems, still has the old group memberships in place. Therefore I need to recreate the rule at this time. That's my pain point atm. You have an idea?

Actually I would really appreciate the reloading of rules with Member of or Descendant of triggers unless the corresponding group memberships change. Makes sense imo.

CrazyIvan359 commented 3 years ago

That sounds like the hard way, what are you triggering with these tags? Wouldn't a DateTimeItem for each light that gets set when it changes be easier?

I agree that dynamic triggers would be better, but it is not a simple thing to do. My current implementation that I use for some of my own rules is not working perfectly even.

janwo commented 3 years ago

Actually I want to setup everything easily without manual creation of items (so people without detailed knowledge can setup their home easily. So thats why I make much use of the metadata.

The rule to sync the tags with the group:

@when("Item added")
@when("Item updated")
@when("Item removed")
@when("System started")
def sync_lights_helpers(event):
    sync_group_with_tags(
        ir.getItem("gCore_Lights_Switchables"),
        LIGHTS_EQUIPMENT_TAGS # e.g. Lightbulb
    )

The rule to set the last light activation of a location:

@when("Descendent of gCore_Lights_Switchables received update")
def set_last_light_activation(event):
    item = ir.getItem(event.itemName)
    if item.getStateAs(OnOffType) == ON:
        location = get_location(item)
        if location:
            set_key_value(
                location.name,
                METADATA_NAMESPACE,
                'lights',
                "last-activation",
                get_date_string(ZonedDateTime.now())
            )

@CrazyIvan359 Maybe we can improve your logic together? ;-)

CrazyIvan359 commented 3 years ago

That is the same reason I'm doing mine the way I did, though I'm the only one using it right now. One problem with it is that all items get removed when OH shuts down, which may cause strange behaviour. The bigger issue is bulk updates causing multiple triggers, this can cause strange behaviour if the rule delete/generate code tries to create a trigger for item 2 that was removed while it was running when triggered by item 1 being removed. A single bad trigger will cause the rule not to be created. You could have the regen code pop bad triggers before creating the rule, but doing that blindly could hide other issues (though I suppose you would see the error from when in the log).

I will post my code here later for you to look at. I am working on other projects at the moment so I won't be able to actively test anything, but I will respond to messages.

CrazyIvan359 commented 3 years ago

What you see below is take 2 of this code. Take 1 was ad-hoc and needs improvement but has seen limited production use, the code below is refined and abstracted, though has not seen production use yet.

This is the starting point, this rule triggers on items being added, removed, or changed. There are no triggers here because this code is built to be published this rule is generated once on load. The triggers are "Item added", "Item updated", and "Item removed". The ItemEventxxx classes are my own, I have included that code at the end of this post.

import typing as t

from community.events.items import ItemEventGroupMember

if t.TYPE_CHECKING: # this will only work if using my stubs with typing installed
    from org.openhab.core.items.events import (
        ItemAddedEvent,
        ItemCommandEvent,
        ItemRemovedEvent,
    )

def rule_item_event(event):
    # type: (t.Union[ItemAddedEvent, ItemRemovedEvent, ItemUpdatedEvent]) -> None
    item_event = ItemEventGroupMember(event, GROUP)
    if (
        item_event.added or (item_event.updated and item_event.membership_changed)
    ) and item_event.is_member:
        # item added to GROUP
        # regenerate rule(s)
    elif (item_event.removed and item_event.is_member) or (
        item_event.updated and item_event.membership_changed and item_event.was_member
    ):
        # item removed from GROUP
        # regenerate rules

And here is my rule (re)generator. The lambda bit is hard to explain but it just provides a group name to substitute into each trigger in the list. You may not need anything that fancy if you are using this in one place. RULE_NAME, RULE_DESC, and RULE_TRIG are all dictionaries with function names as keys, the generator looks up using the function passed. These are in the same file as the generator, the generator itself is generic but I have not managed to get it into a separate library yet.

import typing as t

from core.jsr223.scope import scriptExtension
from core.rules import rule
from core.triggers import when
ruleRegistry = scriptExtension.get("ruleRegistry")  # type: RuleRegistry

if t.TYPE_CHECKING:
    from org.openhab.core.automation import RuleRegistry

def generate_rule(target):
    # type: (function) -> None
    """ Creates a rule, removing it if it already exists """
    UID = getattr(target, "UID", None)  # type: str
    if UID:
        ruleRegistry.remove(UID)
        del UID, target.UID
        del target.triggers

    # this is using a map and lambdas to get the group name from the config
    for trigger in RULE_TRIG[target.__name__]:
        when(trigger.format(group=RULE_TRIG_MAP.get(target.__name__, lambda: "")()))(
            target
        )

    triggers = getattr(target, "triggers", None)  # type: t.Union[t.List, None]
    if triggers and triggers.count(None) != 0:
        log.warn("Rule '%s' not created, a trigger is invalid", target.__name__)
    elif triggers:
        rule(RULE_NAME[target.__name__], RULE_DESC[target.__name__], TAGS)(target)
        if getattr(target, "UID", None):
            log.debug("Rule '%s' created successfully", target.__name__)
        else:
            log.error("Failed to create rule '%s'", target.__name__)
    else:
        log.debug("Rule '%s' not created, no valid triggers", target.__name__)
Event Processors This library pulls out the common code that checks if an item was added, removed, or changed (`ItemEventProcessor`) and also contains `ItemEventGroupMember` and `ItemEventGroupDescendant` which check if an item was added or removed from a group. These classes have not seen much use yet, so you may find errors and I have not documented them yet. You seem pretty competant with Python so I think you'll be able to figure them out. I have the code below in `automation/lib/python/community/events/items.py` and you will also need a blank `automation/lib/python/community/events/__init__.py` for it to work. ```python __all__ = ["ItemEventProcessor", "ItemEventGroupMember", "ItemEventGroupDescendant"] import typing as t from personal.utils import is_member_of_group from java.lang import String from org.openhab.core.items import GenericItem from org.openhab.core.items.dto import ItemDTO from org.openhab.core.items.events import ( ItemAddedEvent, ItemRemovedEvent, ItemUpdatedEvent, ) class ItemEventProcessor(object): __slots__ = ["_added", "_removed", "_updated", "_item", "_old_item"] def __init__(self, event): # type: (t.Union[ItemAddedEvent, ItemRemovedEvent, ItemUpdatedEvent]) -> None self._added = False self._removed = False self._updated = False self._item = None self._old_item = None if isinstance(event, ItemAddedEvent): self._added = True self._item = event.getItem() elif isinstance(event, ItemRemovedEvent): self._removed = True self._item = event.getItem() elif isinstance(event, ItemUpdatedEvent): self._updated = True self._item = event.getItem() self._old_item = event.getOldItem() @property def added(self): # type: () -> bool return self._added @property def removed(self): # type: () -> bool return self._removed @property def updated(self): # type: () -> bool return self._updated @property def item(self): # type: () -> ItemDTO return self._item @property def old_item(self): # type: () -> ItemDTO return self._old_item class ItemEventGroupMember(ItemEventProcessor): __slots__ = ["_is_member", "_was_member"] _recurse = False def __init__( self, event, # type: t.Union[ItemAddedEvent, ItemRemovedEvent, ItemUpdatedEvent] group, # type: t.Union[str, String, GenericItem, ItemDTO] ): # type: (...) -> None super(ItemEventGroupMember, self).__init__(event) self._is_member = False self._was_member = False if self._item: self._is_member = is_member_of_group(self._item, group, self._recurse) if self._old_item: self._was_member = is_member_of_group(self._old_item, group, self._recurse) else: self._was_member = self._is_member @property def is_member(self): # type: () -> bool return self._is_member @property def was_member(self): # type: () -> bool return self._was_member @property def membership_changed(self): # type: () -> bool return self._is_member | self._was_member and not ( self._is_member and self._was_member ) class ItemEventGroupDescendant(ItemEventGroupMember): _recurse = True ```
janwo commented 3 years ago

Looks good, thanks!

I have some questions: 1) Is the regenerated rule removed after reloading the original python rule? 2) The Rule UUID of a python rule is always different imo, does the matching work for you? 3) May you like to share the full code within the community contributions? This way I can test and contribute to it more easily.

When I look among the openHAB community topics, I see this issue multiple times.

CrazyIvan359 commented 3 years ago
  1. This block deletes the existing rule if there was one:

        ​UID​ ​=​ ​getattr​(​target​, ​"UID"​, ​None​)  ​# type: str​
        ​if​ ​UID​:
            ​ruleRegistry​.​remove​(​UID​)
            ​del​ ​UID​, ​target​.​UID​
            ​del​ ​target​.​triggers
  2. Yes, UUID matching works. The UUID is generated by the rule engine of OH and does not change while the rule exists.

  3. Yes, I plan on sharing this eventually. There is not documentation written about how to use it or what it's for, so that will have to come first.

janwo commented 3 years ago

I am still experimenting with these functions. How do you create the original triggers here?

for trigger in RULE_TRIG[target.__name__]:
        when(trigger.format(group=RULE_TRIG_MAP.get(target.__name__, lambda: "")()))(
            target
        )

Unfortunately, I always get the resolved (old) triggers of the old rule instance, so there is no Member of >Group< received update triggers in place, although defined in the files. Instead I have a list of triggers with the corresponding group members only, e.g. Item >GroupMember< received update.

janwo commented 3 years ago

Not easy, surfing through the community forums 🤔 I guess this library needs to save the file metadata in addition to the rule registry (at least the original triggers). This could be done while parsing the rule files. After that we can trigger a reinitialization if those triggers.

Second approach is to get "descendant of" trigger into core" and replace it in triggers.py, see https://github.com/openhab-scripters/openhab-helper-libraries/issues/318

Third approach is to initialize the file parser to restart the parsing of a whole rule file?

janwo commented 3 years ago

I added arrays of original triggers of each rule and add this to the following method call:


def reload_rules(tags=['core-reload'], triggers=[]):
    if len(triggers):
        for _rule in ruleRegistry.getByTags(tags):
            ruleRegistry.remove(_rule.getUID())
            for trigger in triggers:
                Log.logError(
                    'reload_rules',
                    'Add trigger {} to rule {}'.format(trigger, _rule.getName())
                )
                when(trigger)(_rule)
            rule(
                _rule.getName(),
                _rule.getDescription(),
                _rule.getTags()
            )(_rule)
    else:
        Log.logError(
            'reload_rules',
            'Could not reload rule with tags {}'.format(tags)
        )

Unfortunately, I receive the following error: File "/openhab/conf/automation/lib/python/core/triggers.py", line 164, in item_trigger 15.04.21 12:21:18 (+0200) openhab function.triggers.append(ItemStateUpdateTrigger(member.name, state=new_state, trigger_name=trigger_name).trigger) 15.04.21 12:21:18 (+0200) openhab UnsupportedOperationException: java.lang.UnsupportedOperationException

@CrazyIvan359 How can I add those triggers? I would say that I am doing the same as you do.

CrazyIvan359 commented 3 years ago

RULE_TRIG and the like are dictionaries indexed by function name. The one for triggers has an array of trigger strings that need an item name injected into them, as done by the format call. Ignore the lambda in that format call for now, that is a trick I use to get the item name from configuration.py at runtime.

You are correct that the Member of trigger is always resolved to a trigger for each member item. The OH version of that trigger has not been implemented in the libraries yet, though I believe someone has already opened an issue. It is not required for this code though.

In your last post, I'm not sure the exact source of the error, but it is stemming from you passing a Trigger instance to a function that expects a string. The triggers property contains an array of Trigger objects that get used in the creation of the rule in the rule call. In theory you could try reusing them, but invariably you will need to add or remove one trigger each time the regen rule runs. That is far more complicated than ditching the whole thing and starting over, we aren't running on resource strained embedded systems, we don't need that kind of optimization.

CrazyIvan359 commented 3 years ago

Maybe this will help, here is a real example of those dicts from one of my other libraries. You don't need the lambda if you aren't pulling the group name from the config.

RULE_TRIG = {
    "rule_link_changed": ["Member of {group} changed"],
}
RULE_TRIG_MAP = {
    "rule_link_changed": lambda: config.GROUPS[KEY_LINKS],
}  # type: t.Dict[str, t.Callable[[], str]]

Without the lambda the loop would look like this:

for trigger in RULE_TRIG[target.__name__]:
        when(trigger.format(group="MY_GROUP_NAME"))(target)

Which would resolve to "Member of MY_GROUP_NAME changed" for the function rule_link_changed.

janwo commented 3 years ago

Ok, my approach will not solve the whole thing, but I was able to trigger a rule reload. I am using the following method and call it manually as soon as I need a refresh of group members:

# triggers: array of triggers, e.g. ["Member of GROUP received update"]; target: rule function
def reload_rules(triggers, target):
    if not len(triggers) or not target:
        Log.logError(
            'reload_rules',
            'Could not reload rule without triggers or target!'
        )
        return

    UID = getattr(target, "UID", None)
    if not UID:
        Log.logError(
            'reload_rules',
            'Could not reload rule without UID in target object!'
        )
        return

    r = ruleRegistry.get(UID)
    if not r:
        Log.logError(
            'reload_rules',
            'Could not reload rule - uid {} was not found!'.format(UID)
        )
        return

    del target.triggers
    for t in triggers:
        when(t)(target)

    ruleRegistry.remove(UID)
    del UID, target.UID
    rule(
        r.getName(),
        r.getDescription(),
        r.getTags()
    )(target)

Additionally, I also refer to an alternative solution, see here: https://community.openhab.org/t/design-pattern-rule-refresh/102317

Shall we keep this issue open?