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

GroupItem causes Inconsistent Items states of subitems #4880

Closed
mjohenneken opened this issue Jan 9, 2018 · 2 comments
Closed

GroupItem causes Inconsistent Items states of subitems #4880

mjohenneken opened this issue Jan 9, 2018 · 2 comments

Comments

@mjohenneken
Copy link

I found that adding an Item to a Group is not reflected in the member item state. Corresponding community discussion: https://community.openhab.org/t/how-to-model-location-of-an-item/32676/10

When using the API i would assume that adding a member to a group will create a link between both, not only from Group -> Item

A first fix for this would be:

public void addMember(Item item) {
       if (item == null) {
           throw new IllegalArgumentException("Item must not be null!");
       }
       members.add(item);
       if (item instanceof GenericItem) {
           GenericItem genericItem = (GenericItem) item;
           genericItem.addStateChangeListener(this);
           genericItem.addGroupName(baseItem.getName());
       }
   }

public void removeMember(Item item) {
       if (item == null) {
           throw new IllegalArgumentException("Item must not be null!");
       }
       members.remove(item);
       if (item instanceof GenericItem) {
           GenericItem genericItem = (GenericItem) item;
           genericItem.removeStateChangeListener(this);
       }
   }

Notice: i came up with thix fix 6 month ago. Should be adapted to the current code base.

What do you guys think? Are there possible cavheats to not update the member item?

@triller-telekom
Copy link
Contributor

At a first glance it looks as simple as you describe, but at a second thought this is more complicated.

At first: Originally only items know in which group they belong and this is also persisted on the file-system. And if you reload an .items file the whole group/item model in memory will be replaced by what is persisted there.

Also we have to create ItemUpdatedEvents to notify for example GUIs that something has been changed.

For things we already have an API to edit them: The editThing method allows for retrieving a copy of a thing and afterwards the edited copy is pushed into the registry again. All necessary events are created etc.

The method GroupItem.addMember should actually not be used from outside the framework, i.e. from scripts. Also the public API is Item, which does not allow any modification so far in contrast to GenericItem.

@kaikreuzer
Copy link
Contributor

I agree with @triller-telekom that Items must not be edited from within rules - I have created #5449 to clarify this for ActiveItem, while it is more complicated for GroupItem as there is no separate interface that only has getter methods.

For use within rules, we should imho rather make (a subset of) the ItemRegistry available, through which item management could be done. In the context of JSR223 rules, we still have to define some "scripting API" (see #4985 (comment)), which should hopefully eventually provide to rules what they need.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants