Implemented code review comments from openHAB

ppieczul · Aug 19, 2017 · cfc1cdc · cfc1cdc
1 parent dd6b831
commit cfc1cdc
Show file tree

Hide file tree

Showing 34 changed files with 335 additions and 298 deletions.
diff --git a/.project b/.project
@@ -20,6 +20,11 @@
 			<arguments>
 			</arguments>
 		</buildCommand>
+		<buildCommand>
+			<name>org.eclipse.pde.ds.core.builder</name>
+			<arguments>
+			</arguments>
+		</buildCommand>
 	</buildSpec>
 	<natures>
 		<nature>org.eclipse.pde.PluginNature</nature>

diff --git a/ESH-INF/thing/thing-types.xml b/ESH-INF/thing/thing-types.xml
@@ -6,16 +6,9 @@
 
     <thing-type id="miniserver">
 
-        <label>Miniserver</label>
-        <description>www.loxone.de</description>
-
-        <properties>
-            <property name="Serial number"></property>
-            <property name="Project name"></property>           
-            <property name="Miniserver name"></property>
-            <property name="Cloud address"></property>
-        </properties>
-
+        <label>Loxone Miniserver</label>
+        <description>IP gateway for Loxone Smarthome system</description>
+
         <config-description>
 
             <parameter-group name="miniserver">
@@ -38,12 +31,13 @@
 
             <parameter name="host" type="text" required="true" groupName="miniserver">
                 <label>Host</label>
-                <context>network_address</context>
+                <context>network-address</context>
                 <description>Host address or IP of the Loxone Miniserver</description>
             </parameter>
-            <parameter name="port" type="integer" min="0" required="true" groupName="miniserver">
+            <parameter name="port" type="integer" min="0" groupName="miniserver">
                 <label>Port</label>
                 <description>Web interface port of the Loxone Miniserver</description>
+                <default>80</default>
             </parameter>
             <parameter name="user" type="text" required="true" groupName="miniserver">
                 <label>User</label>
@@ -55,57 +49,75 @@
                 <description>User password on the Loxone Miniserver</description>
             </parameter>
 
-            <parameter name="firstConDelay" type="integer" min="0" max="120" required="true" groupName="timeouts">
+            <parameter name="firstConDelay" type="integer" min="0" max="120" groupName="timeouts">
                 <label>First connection delay</label>
                 <description>Time between binding initialization and first connection attempt (seconds, 0-120)</description>
-                <advanced>true</advanced>
                 <default>1</default>                
             </parameter>
-            <parameter name="keepAlivePeriod" type="integer" min="1" max="600" required="true" groupName="timeouts">
+            <parameter name="keepAlivePeriod" type="integer" min="1" max="600" groupName="timeouts">
                 <label>Period between connection keep-alive messages</label>
                 <description>Time between sending two consecutive keep-alive messages (seconds, 1-600)</description>
-                <advanced>true</advanced>
                 <default>240</default>                
             </parameter>
-            <parameter name="connectErrDelay" type="integer" min="0" max="600" required="true" groupName="timeouts">
+            <parameter name="connectErrDelay" type="integer" min="0" max="600" groupName="timeouts">
                 <label>Connect error delay</label>
                 <description>Time between failed websocket connect attempts (seconds, 0-600)</description>
-                <advanced>true</advanced>
                 <default>10</default>                
             </parameter>
-            <parameter name="responseTimeout" type="integer" min="0" max="60" required="true" groupName="timeouts">
+            <parameter name="responseTimeout" type="integer" min="0" max="60" groupName="timeouts">
                 <label>Miniserver response timeout</label>
                 <description>Time to wait for a response from Miniserver to a request sent from the binding (seconds, 0-60)</description>
-                <advanced>true</advanced>
                 <default>4</default>                
             </parameter>
-            <parameter name="userErrorDelay" type="integer" min="0" max="3600" required="true" groupName="timeouts">
+            <parameter name="userErrorDelay" type="integer" min="0" max="3600" groupName="timeouts">
                 <label>Authentication error delay</label>
                 <description>Time in seconds between user login error as a result of wrong name/password or no authority and next connection attempt (seconds, 0-3600)</description>
-                <advanced>true</advanced>
                 <default>60</default>                
             </parameter>
-            <parameter name="comErrorDelay" type="integer" min="0" max="3600" required="true" groupName="timeouts">
+            <parameter name="comErrorDelay" type="integer" min="0" max="3600" groupName="timeouts">
                 <label>Communication error delay</label>
                 <description>Time between connection close (as a result of some communication error) and next connection attempt (seconds, 0-3600)</description>
-                <advanced>true</advanced>
                 <default>30</default>                
             </parameter>
-            <parameter name="maxBinMsgSize" type="integer" min="0" max="102400" required="true" groupName="sizes">
+            <parameter name="maxBinMsgSize" type="integer" min="0" max="102400" groupName="sizes">
                 <label>Maximum binary message size (kB)</label>
                 <description>Websocket client's maximum binary message size in kB</description>
-                <advanced>true</advanced>
                 <default>3072</default>                
             </parameter>
-            <parameter name="maxTextMsgSize" type="integer" min="0" max="102400" required="true" groupName="sizes">
+            <parameter name="maxTextMsgSize" type="integer" min="0" max="102400" groupName="sizes">
                 <label>Maximum text message size (kB)</label>
                 <description>Websocket client's maximum text message size in kB</description>
-                <advanced>true</advanced>
                 <default>512</default>                
             </parameter>
         </config-description>
 
     </thing-type>
 
+    <channel-type id="switchTypeId">
+        <item-type>Switch</item-type>
+        <label>Loxone Switch</label>
+        <description>Loxone's virtual input of switch type and push button controls.</description>
+    </channel-type>
+
+    <channel-type id="roSwitchTypeId">
+        <item-type>Switch</item-type>
+        <label>Loxone Digital Read-only Information</label>
+        <description>Loxone's digital information controls (InfoOnlyDigital, read-only).</description>
+        <state readOnly="true"/>
+    </channel-type>
+
+    <channel-type id="rollerShutterTypeId">
+        <item-type>Rollershutter</item-type>
+        <label>Loxone Jalousie</label>
+        <description>Loxone's Jalousies (rollershutters, blinds).</description>
+    </channel-type>
+
+    <channel-type id="roTextTypeId">
+        <item-type>String</item-type>
+        <label>Loxone State Read-only Information</label>
+        <description>Loxone's state information controls (TextState, read-only).</description>
+        <state readOnly="true"/>
+    </channel-type>
+
 </thing:thing-descriptions>
 
diff --git a/META-INF/MANIFEST.MF b/META-INF/MANIFEST.MF
@@ -3,7 +3,7 @@ Bundle-ManifestVersion: 2
 Bundle-Name: Loxone Binding
 Bundle-SymbolicName: org.openhab.binding.loxone;singleton:=true
 Bundle-Vendor: openHAB
-Bundle-Version: 2.2.0.2
+Bundle-Version: 2.2.0.3
 Bundle-RequiredExecutionEnvironment: JavaSE-1.8
 Bundle-ClassPath: .
 Import-Package: 

diff --git a/OSGI-INF/LoxoneChannelTypeProvider.xml b/OSGI-INF/LoxoneChannelTypeProvider.xml
@@ -0,0 +1,21 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<!--
+
+    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
+
+-->
+<scr:component xmlns:scr="http://www.osgi.org/xmlns/scr/v1.1.0" immediate="true" name="org.openhab.binding.loxone.internal.LoxoneChannelTypeProvider">
+
+   <implementation class="org.openhab.binding.loxone.internal.LoxoneChannelTypeProvider"/>
+
+   <service>
+      <provide interface="org.eclipse.smarthome.core.thing.type.ChannelTypeProvider"/>
+      <provide interface="org.openhab.binding.loxone.internal.LoxoneChannelTypeProvider"/>
+   </service>
+
+</scr:component>
diff --git a/OSGI-INF/LoxoneHandlerFactory.xml b/OSGI-INF/LoxoneHandlerFactory.xml
@@ -9,15 +9,17 @@
     http://www.eclipse.org/legal/epl-v10.html
 
 -->
-<scr:component xmlns:scr="http://www.osgi.org/xmlns/scr/v1.1.0" immediate="true" name="binding.loxone">
+<scr:component xmlns:scr="http://www.osgi.org/xmlns/scr/v1.1.0" immediate="true" name="org.openhab.binding.loxone.internal.LoxoneHandlerFactory">
 
    <implementation class="org.openhab.binding.loxone.internal.LoxoneHandlerFactory"/>
-   <reference bind="setUpnpIOService" cardinality="1..1" interface="org.eclipse.smarthome.io.transport.upnp.UpnpIOService" name="UpnpIOService" policy="static" unbind="unsetUpnpIOService"/>
-   <reference bind="setDiscoveryServiceRegistry" cardinality="1..1" interface="org.eclipse.smarthome.config.discovery.DiscoveryServiceRegistry" name="DiscoveryServiceRegistry" policy="static" unbind="unsetDiscoveryServiceRegistry"/>
-
    <service>
       <provide interface="org.eclipse.smarthome.core.thing.binding.ThingHandlerFactory"/>
-      <provide interface="org.eclipse.smarthome.core.thing.type.ChannelTypeProvider"/>
    </service>
+   <reference cardinality="1..1" 
+    bind="setChannelTypeProvider" 
+    interface="org.openhab.binding.loxone.internal.LoxoneChannelTypeProvider"
+    name="ChannelTypeProvider" 
+    policy="dynamic" 
+    unbind="unbindChannelTypeProvider"/>
 
 </scr:component>
diff --git a/README.md b/README.md
@@ -2,9 +2,8 @@
 
 This binding integrates [Loxone Miniserver](https://www.loxone.com/enen/products/miniserver-extensions/) with [openHAB](http://www.openhab.org/). Miniserver is represented as a [Thing](http://docs.openhab.org/configuration/things.html). Miniserver controls, that are visible in the Loxone [UI](https://www.loxone.com/enen/kb/user-interface-configuration/), are exposed as openHAB channels.
 
-Binding has the Loxone-specific code separated in a .core package. This code does not depend on the openHAB framework and can be easily used to handle Loxone Miniservers in other Java applications.
-
 ## Features
+
 The following features are currently supported:
 
   * [Discovery](https://en.wikipedia.org/wiki/Simple_Service_Discovery_Protocol) of Miniservers available on the local network
@@ -18,7 +17,7 @@ The following features are currently supported:
 
 This binding supports [Loxone Miniservers](https://www.loxone.com/enen/products/miniserver-extensions/) for accessing controls that are configured in their UI.
 
-Thing ID, for automatically discovered Miniservers, is defined in the following way: `loxone:miniserver:<serial>`, where `<serial>` is a serial number of the Miniserver (effectively this is the MAC address of its network interface).
+The Thing UID of automatically discovered Miniservers is: `loxone:miniserver:<serial>`, where `<serial>` is a serial number of the Miniserver (effectively this is the MAC address of its network interface).
 
 ### Discovery
 
@@ -104,36 +103,6 @@ Channel label is defined in the following way:
   * For controls that belong to a room: `<Room name> / <Control name>` 
   * For controls without a room: `<Control name>`
 
-## Items
-
-Items for Miniserver's controls can be created manually or automatically, depending on openHAB's `Item Linking/Simple Mode` setting. This setting can be modified in PaperUI under `Configuration/System` page and should be set to the desired value before Loxone Thing is created. Please consult [tutorial](http://docs.openhab.org/tutorials/beginner/configuration.html) for more details about item linking simple mode.
-
-
-When `Simple Mode` is enabled, openHAB will automatically build and link items for all channels created by the binding, one for each channel. Item label will be equal to the channel's label, allowing for a human-readable representation of the controls wherever there is a need to choose an item in a user interface, usually from a drop-down list (for example in HABPanel, IFTTT, Alexa).
-
-In case user wants to create items manually, they can be defined in [.items file](http://docs.openhab.org/configuration/items.html#item-definition-and-syntax). Linking of an item to a particular Loxone control is done through passing channel ID to a `{channel=...}` assignment in the item definition. Examples:
-
-  * On-off switch with a tag recognizable by Alexa service, representing a Switch subcontrol (output) of a Miniserver's Lighting Controller:
-
-    `Switch Kitchen_Light "Kitchen Light" <switch> ["lighting"] {channel="loxone:miniserver:504E9420290F:0EC5E0CF-0255-6ABD-FFFF402FB0C24B9E"}`
-
-  * Temperature of the Miniserver (on Loxone: Virtual Analog State functional block or InfoOnlyAnalog control}:
-
-    `Number Miniserver_Temp "Miniserver temperature: [%.1f °C]" <temperature> {channel="loxone:miniserver:504E9420290F:0F2F2133-017D-3C82-FFFF203EB0C34B9E"}`
-
-  * Pushbutton switch representing a Miniserver's Virtual Input of pushbutton type (pushbutton realized by adding `autoupdate="false"` parameter):
-
-    `Switch Reset_Lights "Switch all lights off" ["lighting"] {autoupdate="false",channel="loxone:miniserver:504E9420290F:0F2F2133-01AD-3282-FFFF201EB0C24B9E"}`
-
-
-### Loxone and Amazon Alexa
-
-Your openHAB server can be exposed through [myopenHAB](http://www.myopenhab.org/) cloud service to  [Amazon Alexa](https://en.wikipedia.org/wiki/Amazon_Alexa) device with [openHAB skill](https://www.amazon.com/openHAB-Foundation/dp/B01MTY7Z5L) enabled. To enable this service, please consult instructions available [here](https://community.openhab.org/t/official-alexa-smart-home-skill-for-openhab-2/23533).
-
-When creating a Miniserver Thing in the openHAB's Item Linking Simple Mode, Loxone binding will automatically create item tags required by Alexa, so that Miniserver's controls can be discovered by [Alexa smart home]( https://www.amazon.com/alexasmarthome) module. Tags will be created for switches, which belong to a category of "lighting" type. This will allow you to command Loxone controls with your voice.
-
-Alexa will recognize items by their labels, which will be equal to the corresponding control's name on the Miniserver. In case your controls are named in a way not directly suiting voice commands, you will need to manually add and link new items to the channels, and add proper tags as described in the above instructions.
-
 ## Advanced Parameters
 
 This section describes the optional advanced parameters that can be configured for a Miniserver. They can be set using UI (e.g. PaperUI) or in a .things file. If a parameter is not explicitly defined, binding will use its default value.
@@ -169,15 +138,15 @@ Timeout values control various parts of Websocket connection management. They ca
 
 ## Automatic Configuration Example
 
-The simplest and quickest way of configuring a Loxone Miniserver with openHAB is to use automatic configuration features available in openHAB2 and PaperUI:
+The simplest and quickest way of configuring a Loxone Miniserver with openHAB is to use automatic configuration features:
 
   * Make sure your Miniserver is up and running and on the same network segment as openHAB server.
   * Add Loxone binding from the available `Add-ons`.
   * In `Configuration/System` page, set `Item Linking` to `Simple Mode` (don't forget to save your choice).
   * Add your Miniserver Thing from the `Inbox`, after automatic discovery is performed by the framework during binding initialization.
   * Configure your Miniserver by editing Miniserver Thing in `Configuration/Things` page and providing user name and password.
   * Miniserver Thing should go online. Channels and Items will be automatically created and configured.
-  * On the `Control` page, you can test Miniserver Items and iteract with them.
+  * On the `Control` page, you can test Miniserver Items and interact with them.
   * As the user interface, you may use [HABPanel](http://docs.openhab.org/addons/uis/habpanel/readme.html), where all Miniserver's items are ready for picking up, using entirely the graphical user interface.
 
 ## Manual Configuration Example

diff --git a/pom.xml b/pom.xml
@@ -11,7 +11,7 @@
 
   <groupId>org.openhab.binding</groupId>
   <artifactId>org.openhab.binding.loxone</artifactId>
-  <version>2.2.0-2</version>
+  <version>2.2.0-3</version>
 
   <name>Loxone Binding</name>
   <packaging>eclipse-plugin</packaging>

diff --git a/src/main/java/org/openhab/binding/loxone/LoxoneBindingConstants.java b/src/main/java/org/openhab/binding/loxone/LoxoneBindingConstants.java
@@ -22,14 +22,20 @@ public class LoxoneBindingConstants {
     // List of all Thing Type UIDs
     public static final ThingTypeUID THING_TYPE_MINISERVER = new ThingTypeUID(BINDING_ID, "miniserver");
 
-    // Channel IDs are created dynamically in runtime
+    // Channel Type IDs
+    public static final String MINISERVER_CHANNEL_TYPE_SWITCH = "switchTypeId";
+    public static final String MINISERVER_CHANNEL_TYPE_RO_SWITCH = "roSwitchTypeId";
+    public static final String MINISERVER_CHANNEL_TYPE_ROLLERSHUTTER = "rollerShutterTypeId";
+    public static final String MINISERVER_CHANNEL_TYPE_RO_TEXT = "roTextTypeId";
 
     // Miniserver properties and parameters
     public static final String MINISERVER_PARAM_HOST = "host";
     public static final String MINISERVER_PARAM_PORT = "port";
-    public static final String MINISERVER_PROPERTY_SERIAL = "Serial number";
-    public static final String MINISERVER_PROPERTY_MINISERVER_NAME = "Miniserver name";
-    public static final String MINISERVER_PROPERTY_PROJECT_NAME = "Project name";
-    public static final String MINISERVER_PROPERTY_CLOUD_ADDRESS = "Cloud address";
+    public static final String MINISERVER_PROPERTY_MINISERVER_NAME = "name";
+    public static final String MINISERVER_PROPERTY_PROJECT_NAME = "project";
+    public static final String MINISERVER_PROPERTY_CLOUD_ADDRESS = "cloudAddress";
 
+    // Location as configured on the Miniserver - it may be different to the Thing location property, which is user
+    // defined and influences the grouping of items in the UI
+    public static final String MINISERVER_PROPERTY_PHYSICAL_LOCATION = "physicalLocation";
 }
diff --git a/src/main/java/org/openhab/binding/loxone/discovery/LoxoneMiniserverDiscoveryParticipant.java b/src/main/java/org/openhab/binding/loxone/discovery/LoxoneMiniserverDiscoveryParticipant.java
@@ -49,15 +49,22 @@ public DiscoveryResult createResult(RemoteDevice device) {
         if (uid != null) {
             Map<String, Object> properties = new HashMap<>(2);
 
+            // After correct Thing UID is created, we have confidence that all following parameters exist and we don't
+            // need to check for null objects here in the device details
             DeviceDetails details = device.getDetails();
             String serial = details.getSerialNumber();
             String host = details.getPresentationURI().getHost();
             String label = details.getFriendlyName() + " @ " + host;
             int port = details.getPresentationURI().getPort();
+            String vendor = details.getManufacturerDetails().getManufacturer();
+            String model = details.getModelDetails().getModelName();
 
             logger.debug("Creating discovery result for serial {} label {} port {}", serial, label, port);
             properties.put(LoxoneBindingConstants.MINISERVER_PARAM_HOST, host);
             properties.put(LoxoneBindingConstants.MINISERVER_PARAM_PORT, port);
+            properties.put(Thing.PROPERTY_VENDOR, vendor);
+            properties.put(Thing.PROPERTY_MODEL_ID, model);
+            properties.put(Thing.PROPERTY_SERIAL_NUMBER, serial);
 
             return DiscoveryResultBuilder.create(uid).withProperties(properties).withLabel(label)
                     .withRepresentationProperty(serial).build();