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

Classic UI: fix dynamic widget visibility and dynamic update of the page title #3495

Merged
merged 1 commit into from
Jun 8, 2017
Merged

Classic UI: fix dynamic widget visibility and dynamic update of the page title #3495

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
66 changes: 41 additions & 25 deletions ...lassic/src/main/java/org/eclipse/smarthome/ui/classic/internal/servlet/WebAppServlet.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,10 @@
import javax.servlet.ServletRequest;
import javax.servlet.ServletResponse;

import org.eclipse.emf.common.util.BasicEList;
import org.eclipse.emf.common.util.EList;
import org.eclipse.smarthome.core.items.GenericItem;
import org.eclipse.smarthome.core.items.GroupItem;
import org.eclipse.smarthome.core.items.Item;
import org.eclipse.smarthome.core.items.ItemNotFoundException;
import org.eclipse.smarthome.core.items.StateChangeListener;
Expand All @@ -29,6 +31,7 @@
import org.eclipse.smarthome.model.sitemap.LinkableWidget;
import org.eclipse.smarthome.model.sitemap.Sitemap;
import org.eclipse.smarthome.model.sitemap.SitemapProvider;
import org.eclipse.smarthome.model.sitemap.VisibilityRule;
import org.eclipse.smarthome.model.sitemap.Widget;
import org.eclipse.smarthome.ui.classic.internal.WebAppConfig;
import org.eclipse.smarthome.ui.classic.internal.render.PageRenderer;
Expand Down Expand Up @@ -129,7 +132,7 @@ public void service(ServletRequest req, ServletResponse res) throws ServletExcep
if (sitemap == null) {
throw new RenderException("Sitemap '" + sitemapName + "' could not be found");
}
logger.debug("reading sitemap {}", sitemap.getName());
logger.debug("reading sitemap {} widgetId {} async {} poll {}", sitemap.getName(), widgetId, async, poll);
if (widgetId == null || widgetId.isEmpty() || widgetId.equals("Home")) {
// we are at the homepage, so we render the children of the sitemap root node
String label = sitemap.getLabel() != null ? sitemap.getLabel() : sitemapName;
Expand All @@ -144,19 +147,23 @@ public void service(ServletRequest req, ServletResponse res) throws ServletExcep
// we are on some subpage, so we have to render the children of the widget that has been selected
Widget w = renderer.getItemUIRegistry().getWidget(sitemap, widgetId);
if (w != null) {
String label = renderer.getItemUIRegistry().getLabel(w);
if (label == null) {
label = "undefined";
}
if (!(w instanceof LinkableWidget)) {
throw new RenderException("Widget '" + w + "' can not have any content");
}
EList<Widget> children = renderer.getItemUIRegistry().getChildren((LinkableWidget) w);
if (poll && waitForChanges(children) == false) {
LinkableWidget lw = (LinkableWidget) w;
EList<Widget> children = renderer.getItemUIRegistry().getChildren(lw);
EList<Widget> parentAndChildren = new BasicEList<Widget>();
parentAndChildren.add(lw);
parentAndChildren.addAll(children);
if (poll && waitForChanges(parentAndChildren) == false) {
// we have reached the timeout, so we do not return any content as nothing has changed
res.getWriter().append(getTimeoutResponse()).close();
return;
}
String label = renderer.getItemUIRegistry().getLabel(w);
if (label == null) {
label = "undefined";
}
result.append(renderer.processPage(renderer.getItemUIRegistry().getWidgetId(w), sitemapName, label,
children, async));
}
Expand Down Expand Up @@ -219,29 +226,34 @@ private boolean waitForChanges(EList<Widget> widgets) {
*/
private Set<GenericItem> getAllItems(EList<Widget> widgets) {
Set<GenericItem> items = new HashSet<GenericItem>();
if (itemRegistry != null) {
if (renderer.getItemUIRegistry() != null) {
for (Widget widget : widgets) {
String itemName = widget.getItem();
if (itemName != null) {
try {
Item item = itemRegistry.getItem(itemName);
if (item instanceof GenericItem) {
final GenericItem gItem = (GenericItem) item;
items.add(gItem);
}
} catch (ItemNotFoundException e) {
// ignore
}
} else {
if (widget instanceof Frame) {
items.addAll(getAllItems(((Frame) widget).getChildren()));
}
addItemWithName(items, widget.getItem());
if (widget instanceof Frame) {
items.addAll(getAllItems(((Frame) widget).getChildren()));
}
for (VisibilityRule vr : widget.getVisibility()) {
addItemWithName(items, vr.getItem());
}
}
}
return items;
}

private void addItemWithName(Set<GenericItem> items, String itemName) {
if (itemName != null) {
try {
Item item = renderer.getItemUIRegistry().getItem(itemName);
if (item instanceof GenericItem) {
final GenericItem gItem = (GenericItem) item;
items.add(gItem);
}
} catch (ItemNotFoundException e) {
// ignore
}
}
}

/**
* This is a state change listener, which is merely used to determine, if a state
* change has occurred on one of a list of items.
Expand All @@ -258,7 +270,9 @@ private static class BlockingStateChangeListener implements StateChangeListener
*/
@Override
public void stateChanged(Item item, State oldState, State newState) {
changed = true;
if (!(item instanceof GroupItem)) {
changed = true;
}
}

/**
Expand All @@ -275,7 +289,9 @@ public boolean hasChangeOccurred() {
*/
@Override
public void stateUpdated(Item item, State state) {
// ignore if the state did not change
if (item instanceof GroupItem) {
changed = true;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks pretty dangerous to me.
Note that the Classic UI intentionally had some constraints regarding dynamic updates as every update results in a full page refresh, which lets the screen flicker, scroll and makes the UI difficult to use, if updates are too frequent.
Especially GroupItems receive MANY state updates and thus they are prone to such negative effects.

@lolodomo & @triller-telekom Did you extensively test this on large sitemaps with lots of events? We really should make sure not to cause any major new issues here. Classic UI should really be in maintenance mode and only severe stuff should be fixed on it.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I did not check this on a very large sitemap and I was not aware of these constraints. But they surely make sense because scrolling and screen flickering isn't nice.

@lolodomo: What did you experiments say when you tested it?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We are in Home Automation systems, I doubt users have setup with lots of event updates per second. That is not my case.
I think I recently read that for standard groups there is no more any state update. Can you please confirm that point ? If that is true, the change will only concern groups with a function. If that is not true, I should change the code to trigger a reload on state update only for groups having a function.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Standard groups can still have a state. However, if they do not have a function their state is not derived from their member items.

If you change the code to only refresh the site for groups with a function you will miss state changes on those without function.

}
}
}

Expand Down