spacemanspiff2007 / HABApp

Easy home automation with MQTT and/or openHAB
Apache License 2.0
54 stars 23 forks source link

added missing GroupItemStateChangedEventFilter #382

Closed nobbi1991 closed 1 year ago

nobbi1991 commented 1 year ago

Added missing GroupItemStateChangedEventFilter. I use the filter already in my system :)

spacemanspiff2007 commented 1 year ago

You could already trigger on the event with the EventFilter as show in the example. You're comment reads as if that was not clear to you - is that the case?

In the current dev branch I fixed the groups in anticipating for OH4.0. There you can then trigger on Groups with the ValueUpdateEventFilter and ValueChangeEventFilter. Would you still think a dedicated GroupItemStateChangedEventFilter makes sense in that case? Or wouldn't it be better to recommend the ValueUpdateEvent in the docs to the user?

nobbi1991 commented 1 year ago

Hi, no, for me it is still not clear, how to trigger on a group event. In the linked example I don't see an example for a group item :( Can you please point out, which example you mean?

If you added support for the normal ValueUpdateEventFilter and ValueChangeEventFilter for group items, then we don't need my PR anymore ;)

spacemanspiff2007 commented 1 year ago

Text in the Example:

# This is the same as above but with the generic filter
my_item.listen_event(self.on_val_my_value, EventFilter(ValueUpdateEvent, value='my_value'))

How it already works (transfer logic for GroupItemStateChangedEvent)

my_item.listen_event(self.on_val_my_value, EventFilter(GroupItemStateChangedEvent, value='my_value'))

The ItemStateEventFilter is only a convenience filter. You can achieve the same by passing the event class into EventFilter

If you added support for the normal ValueUpdateEventFilter and ValueChangeEventFilter for group items, then we don't need my PR anymore ;)

It will work as expected with the next release

nobbi1991 commented 1 year ago

Ok, now I get it :) From my side we can remove this PR. Thanks for clarification