Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Added support for dashboard links to external services. #185

Merged
merged 8 commits into from Aug 18, 2017
34 changes: 34 additions & 0 deletions bundles/org.openhab.ui.dashboard/README.md
@@ -0,0 +1,34 @@
# Dashboard tiles
Copy link
Member

Choose a reason for hiding this comment

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

-> Just "Dashboard"


OpenHAB dashboard is landing page for the user where all openHAB UI's can be found. Dashboard support also links to external services. Links can be added to dashboard by ```conf/services/dashboard.cfg``` configuration file.
Copy link
Member

Choose a reason for hiding this comment

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

-> openHAB

Copy link
Member

Choose a reason for hiding this comment

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

-> UIs

Copy link
Member

Choose a reason for hiding this comment

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

-> to the dashboard by editing the...

Copy link
Member

Choose a reason for hiding this comment

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

Please put every sentence in a new line to make diffing and reviewing easier.


## Link Configuration

| Parameter name | Type | Description |
|-----------------|---------|-----------------------------------------------------------------------------------------|
| link-nameX | String | Name which is shown in the openHAB dashboard. |
| link-urlX | String | URL to external service. |
| link-overlayX | String | Image overlay icon. Supported values are empty (no icon), "html5", "android" or "apple" |
Copy link
Member

Choose a reason for hiding this comment

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

Actually many more. If I remember right, all icons from https://icomoon.io/#preview-free should work as an overlay.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Dashboard embedded web/fonts/icomoon.ttf seems to contain only html5, android and apple icons.

Copy link
Member

Choose a reason for hiding this comment

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

I wonder if in that case this feature at all makes sense - the overlay was used in the past, but as everything ended up using the HTML5 overlay, we removed it. So it is rather in there as some legacy, which we might not want to promote again through a new feature. Wdyt?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's totally fine for me to remove this feature. If more icons would be supported, this feature make more sense as well.

Copy link
Member

Choose a reason for hiding this comment

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

Ok, so may I ask you to remove the overlay support here for the external tiles as a last change before merging? Thanks :-)

| link-imageurlX | String | URL to image. |

Where X is link unique number (see examples). All configuration parameters need to start with ```org.openhab.ui.dashboard:``` prefix.
Copy link
Member

Choose a reason for hiding this comment

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

We should change the service pid from org.openhab.ui.dashboard to org.openhab.dashboard. With that change, the pid is derived from the filename and you can leave out the prefix in the file:

link-name1=openHAB Log Viewer
link-url1=http://localhost:9001
link-overlay1=
link-imageurl1=http://localhost:8080/static/image.png

Would be much nicer!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That was the missing piece. I was wondering why some of the configuration files works without prefix.


## Image URL

Image URL support several URL formats. URL support direct http links to local or Internet servers, but also the "data" URL scheme (RFC2397) is supported. See e.g. [https://www.base64-image.de](https://www.base64-image.de) to convert images to base64 coded data.
Copy link
Member

Choose a reason for hiding this comment

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

Who needs access to the links? The server or the browser? Should be clarified.


## Example configuration file
Copy link
Member

Choose a reason for hiding this comment

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

please add an empty line after the header

```
org.openhab.ui.dashboard:link-name1=openHAB Log Viewer
org.openhab.ui.dashboard:link-url1=http://localhost:9001
Copy link
Member

Choose a reason for hiding this comment

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

If the answer to my question above is "the browser", your examples are pretty misleading - "localhost" won't really get you far...

org.openhab.ui.dashboard:link-overlay1=
org.openhab.ui.dashboard:link-imageurl1=http://localhost:8080/static/image.png
Copy link
Member

Choose a reason for hiding this comment

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

To avoid localhost, a relative url like ../static/image.png is probably better.


org.openhab.ui.dashboard:link-name2=Node-RED
org.openhab.ui.dashboard:link-url2=http://localhost:1880
org.openhab.ui.dashboard:link-overlay2=
org.openhab.ui.dashboard:link-imageurl2=data:image/png;base64,iVBORw0KGgoAAAANSUhEUgAAAXwAAACfCAIAAA...QmCC

```

Note: **Link 2** image data URL is not valid (it's shorten for the sake of clarity).
Expand Up @@ -11,6 +11,7 @@
import java.io.IOException;
import java.net.URL;
import java.util.Hashtable;
import java.util.Map;
import java.util.Set;
import java.util.concurrent.CopyOnWriteArraySet;

Expand Down Expand Up @@ -50,7 +51,13 @@ public class DashboardService {

private BundleContext bundleContext;

protected void activate(ComponentContext componentContext) {
private final static String LINK_NAME = "link-name";
private final static String LINK_URL = "link-url";
private final static String LINK_OVERLAY = "link-overlay";
private final static String LINK_IMAGEURL = "link-imageurl";

protected void activate(ComponentContext componentContext, Map<String, Object> properties) {

try {
bundleContext = componentContext.getBundleContext();
Hashtable<String, String> props = new Hashtable<>();
Expand All @@ -69,6 +76,8 @@ protected void activate(ComponentContext componentContext) {
} catch (NamespaceException | ServletException e) {
logger.error("Error during dashboard startup: {}", e.getMessage());
}

addTilesToExternalServices(properties);
}

protected void deactivate(ComponentContext componentContext) {
Expand Down Expand Up @@ -153,4 +162,30 @@ protected HttpServlet createServlet() {
return new DashboardServlet(configurationAdmin, indexTemplate, entryTemplate, warnTemplate, setupTemplate,
tiles);
}

private void addTilesToExternalServices(Map<String, Object> properties) {
Copy link
Member

Choose a reason for hiding this comment

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

I'd prefer addTilesForExternalServices

for (String key : properties.keySet()) {
if (key.startsWith(LINK_NAME)) {
if (key.length() > LINK_NAME.length()) {
try {
// get index number from link name
int index = Integer.parseInt(key.substring(LINK_NAME.length()));

String name = (String) properties.get(LINK_NAME + Integer.toString(index));
String url = (String) properties.get(LINK_URL + Integer.toString(index));
String overlay = (String) properties.get(LINK_OVERLAY + Integer.toString(index));
String imageUrl = (String) properties.get(LINK_IMAGEURL + Integer.toString(index));

logger.debug("Add link: {}", name);

addDashboardTile(new DashboardTileImp.DashboardTileBuilder().withName(name).withUrl(url)
.withOverlay(overlay).withImageUrl(imageUrl).build());
} catch (NumberFormatException e) {
logger.error("Syntax error in dashboard tile '{}'", key);
}

}
}
}
}
}
@@ -0,0 +1,82 @@
/**
* Copyright (c) 2010-2017 by the respective copyright holders.
*
* All rights reserved. This program and the accompanying materials
* are made available under the terms of the Eclipse Public License v1.0
* which accompanies this distribution, and is available at
* http://www.eclipse.org/legal/epl-v10.html
*/
package org.openhab.ui.dashboard.internal;

import org.openhab.ui.dashboard.DashboardTile;

/**
* The dashboard tile for external services.
*
* @author Pauli Anttila - Initial contribution
*/
public class DashboardTileImp implements DashboardTile {
Copy link
Member

Choose a reason for hiding this comment

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

Could we fina a better class name? How about ExternalServiceTile?

private String name;
private String url;
private String overlay;
private String imageUrl;

private DashboardTileImp(DashboardTileBuilder builder) {
this.name = builder.name;
this.url = builder.url;
this.overlay = builder.overlay;
this.imageUrl = builder.imageUrl;
}

@Override
public String getName() {
return name;
}

@Override
public String getUrl() {
return url;
}

@Override
public String getOverlay() {
return overlay;
}

@Override
public String getImageUrl() {
return imageUrl;
}

public static class DashboardTileBuilder {

private String name;
private String url;
private String overlay;
private String imageUrl;

public DashboardTileBuilder withName(String name) {
this.name = name;
return this;
}

public DashboardTileBuilder withUrl(String url) {
this.url = url;
return this;
}

public DashboardTileBuilder withOverlay(String overlay) {
this.overlay = overlay;
return this;
}

public DashboardTileBuilder withImageUrl(String imageUrl) {
this.imageUrl = imageUrl;
return this;
}

public DashboardTileImp build() {
return new DashboardTileImp(this);
}
}
}