Add ItemStateUpdatedEvent and enable group channel-links

[J-N-K]

There has been some discussion in the past about the ItemStateEvent which in some cases may not carry the unit of a value even if the item has a unit (#2956).

This PR adds an ItemStateUpdatedEvent which is fired by the item itself (similar to the ItemStateChangedEvent. Since the item knows about its dimension and unit, the new event can carry this information even if the unit is derived from a unit provider or the state description. This also clearly separates the events when the item reports something and when the item is supposed to do something:

• ItemStateEvent -> the item shall update to the contained state
• ItemCommandEvent-> the item receives this command
• ItemStateUpdatedEvent -> the item updated the state
• ItemStateChangedEvent -> the item changed the state

Since this is rather big change in functionality I would suggest to consider this for OH4 because time is running out for extensive testing (even if unit/integration test coverage is quite good).

As a side-effect group items now also report their state which makes it possible to send aggregated states (AVG/SUM/MIN/MAX/...) to the outside world by using the FOLLOW profile.

[openhab-bot]

This pull request has been mentioned on openHAB Community. There might be relevant details there:

https://community.openhab.org/t/follow-profile-on-group/113445/19

[spacemanspiff2007]

Awesome! I really think the differentiation between report and todo makes things more clear for the user.
Could you also add an example how this would look as an SSE event?

@J-N-K
There was also some talk about adding a timestamp when the item state was set.
Have you already had some thoughts about that?

[J-N-K]

@spacemanspiff2007 Looks like the ItemStateEvent, it uses the same payload. It has a different topic (/stateupdated, in line with /statechanged) and always contains the new state of the item.

Regarding the timestamp: IMO it's still better to set the timestamp on the state, as that allows handling historic and future states with the same logic we have now. I wouldn't veto an addition to the events, but that should be a change for all events, not just some, so it's out of scope here.

[openhab-bot]

This pull request has been mentioned on openHAB Community. There might be relevant details there:

https://community.openhab.org/t/habapp-items-some-times-none/140425/11

[openhab-bot]

This pull request has been mentioned on openHAB Community. There might be relevant details there:

https://community.openhab.org/t/uom-default-units-and-consequences/142360/118

[J-N-K]

@openhab/core-maintainers Can we agree that this is useful and merge it? It makes the implementation of a solution for #3282 much easier because we have an event that carries the correct unit for state updates (which is needed e.g. for persistence).

[kaikreuzer]

Thanks @J-N-K. Yes, I think it makes sense and the code looks all good to me.
Just one small question to not miss anything: Why did you label it as "API breaking"? Afaics, it merely adds the new event type(s), but there shouldn't really be an impact on the way the existing events are sent. Could you please briefly explain?

[J-N-K]

I didn't find a better label. The sending of events is not affected (there are only more events), but there is a change in behavior of the CommunicationManager which now listens for ItemStateUpdatedEvent instead of ItemStateEvent. As a result it no longer receives a DecimalType when this is part of the ItemStateEvent but the item has a dimension. Instead a QuantityType is received with the new state of the item (or the same if it doesn't change).

[spacemanspiff2007]

Additionally it's somewhat API breaking for RestAPI consumers because

• The new event should be used
• The legacy code doing the event state to item state magic can be completely removed
  (e.g. ItemStateEvent(1) to Switch results in ON but to Number results in 1).

It's a huge simplification and makes dealing with the RestAPI events much more pleasant.

[kaikreuzer]

Thank you!

[lolodomo]

I have difficulty to understand the impact of this change on the remote OH binding.
Is it ok to ignore this new event and only rely on old events?
Or should I ignore ItemStateEvent and consider rather ItemStateUpdatedEvent when and only when the remote server is OH 4 ?

https://github.com/openhab/openhab-addons/blob/main/bundles/org.openhab.binding.remoteopenhab/src/main/java/org/openhab/binding/remoteopenhab/internal/rest/RemoteopenhabRestClient.java#L416

[lolodomo]

Is it just an additional event that can be ignored or is the behavior with the previous events impacted?

[J-N-K]

There is a benefit in using it, but it does no harm ignoring it. All events are still fired like before.

One of the limitations of ItemStateEvent is that it does not contain information about the real state the item has. E.g. a ON event to a DimmerItem changes the internal state to 100% because internally the representation of the state is PercentType. The ItemStateChangedEvent however does report 100%. The ItemStateUpdatedEvent also contains the new state of the item in the same way it is stored internally. The same applies to DecimalTypes sent to UoM-Number items, Color items and roller shutters.

[jimtng]

@J-N-K would you mind explaining about this change in particular? I am trying to understand why it was necessary. It seems to me that it does exactly the same thing. What have I missed?

I'm trying to understand the difference between:

        if (eventPublisher != null) {
            eventPublisher.post(ItemEventFactory.createStateChangedEvent(this.name, newState, oldState));
        }

vs:

        EventPublisher eventPublisher1 = this.eventPublisher;
        if (eventPublisher1 != null) {
            eventPublisher1.post(ItemEventFactory.createStateChangedEvent(this.name, newState, oldState));
        }

Why the need for a temporary variable? Is it for optimisation purpose, so that it's accessing a local variable instead of resolving a class-member variable twice?

[J-N-K]

The eventPublisher is @Nullable. There is a slight chance that it is set to null by another thread between the null-check and the usage. Therefore taking a copy ensures we don't get a NPE when using it: even if the field is set to null, we still have the non-null reference.

[jimtng]

Thank you! TIL!

[florian-h05]

It seems that the events that trigger rules have changed with this PR, can this be caused here?
And if yes, how did they change? The JS library‘s event object is partly broken now and I need to fix it.

[J-N-K]

What exactly is broken? The ItemStateEvent was replaced by the ItemStateUpdatedEvent because that is what contains the new item's state, but I don't see why that would break something, they both implement .getItemState and inherit from ItemEvent?

[florian-h05]

For file-based scripts you can create rules using rules.JSRule, the callback functions of these are provided the event data that triggered the rule run. The event is not passed 1:1 to the rule, because handling Java types in JS is confusing. Instead, based on the classname of the event, properties are added to a pure JS object.

I just need to adjust the classnames there.

The ItemStateEvent was replaced by the ItemStateUpdatedEvent

I‘ve already figured out that, thanks.

Did anything change for group events? On first look, it seems that GroupItemStateEvent was replaced by ItemUpdatedEvent.

[J-N-K]

GroupItemStateEvent never existed, even though it was in the JavaDoc. This PR introduced the updated event for groups (which was not reported before).

[jimtng]

I'm wondering why ItemStateUpdatedEvent extends ItemEvent and not ItemStateEvent. If it extended ItemStateEvent I think the jruby library would've been unaffected. But this is not a big problem and I've got a fix pending for it inside the jruby library.

The thing that puzzled me though is that previously (e.g. in openhab 3.4), I can send BusEvent.postUpdate("GroupItem", ON) and this will trigger an Item updated event for "GroupItem". You can test this using a MainUI rule in any language, then send the update event through karaf for example.

This stopped working in 4.0 recently. I don't know when exactly, but I know that it wasn't this PR that changed that behaviour. I looked around in the core code but even in 3.4 I can't actually find where this would have triggered, so the fact that it triggered in 3.4 is a mystery to me, but it did. We have a test to verify this behaviour since late last year and it has always passed until recently.

[lolodomo]

No impact on DSL rules?

[J-N-K]

I don't think so. They just pass the event payload and that is the same (except that you really get the item state instead of something that may or may not be the item state).

[openhab-bot]

This pull request has been mentioned on openHAB Community. There might be relevant details there:

https://community.openhab.org/t/openhab-4-0-milestone-discussion/145133/196

[openhab-bot]

This pull request has been mentioned on openHAB Community. There might be relevant details there:

https://community.openhab.org/t/openhab-4-migration-faq/148144/18

[creativeprojects]

Is there any plan to deprecate the ItemStateEvent in the future?
I understand why the new one is better, but then I don't think the original event serves any purpose any more.

[J-N-K]

It still has an important purpose: it is issued to initiate an item state update. So the ItemStateEvent results in an update of the item, which is then communicated as ItemStateUpdatedEvent (and potentially an ItemStateChangedEvent).

[creativeprojects]

OK.
So if I understand well, it could mean that if a channel linked to the item state is down, we might only get ItemStateEvent but not a ItemStateUpdatedEvent in that case?

[spacemanspiff2007]

No.
ItemStateEvent: Event that tells the item to update its state
ItemStateUpdatedEvent: Event that tells that an item has updated it's state

Do you see the subtle but important difference.

Let's take a DimmerItem:
ItemStateEvent: ON (Item should turn on)
ItemUpdatedEvent: 100% (Dimmer that is on has 100%)

[creativeprojects]

Right, I understand the difference, thank you for the explanation 👍🏻

I think we could update the documentation with this information: https://www.openhab.org/docs/developer/utils/events.html