[airvisualnode] Fix PR review problems

Signed-off-by: Victor Antonovich <v.antonovich@gmail.com>
openhab · Oct 26, 2017 · 4f1d2d8 · 4f1d2d8
1 parent c74701e
commit 4f1d2d8
Show file tree

Hide file tree

Showing 6 changed files with 104 additions and 89 deletions.
diff --git a/addons/binding/org.openhab.binding.airvisualnode/ESH-INF/binding/binding.xml b/addons/binding/org.openhab.binding.airvisualnode/ESH-INF/binding/binding.xml
@@ -1,8 +1,7 @@
 <?xml version="1.0" encoding="UTF-8"?>
 <binding:binding id="airvisualnode"
-				 xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
-				 xmlns:binding="http://eclipse.org/smarthome/schemas/binding/v1.0.0"
-				 xsi:schemaLocation="http://eclipse.org/smarthome/schemas/binding/v1.0.0 http://eclipse.org/smarthome/schemas/binding-1.0.0.xsd">
+	xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xmlns:binding="http://eclipse.org/smarthome/schemas/binding/v1.0.0"
+	xsi:schemaLocation="http://eclipse.org/smarthome/schemas/binding/v1.0.0 http://eclipse.org/smarthome/schemas/binding-1.0.0.xsd">
 
 	<name>AirVisual Node Binding</name>
 	<description>Binding for AirVisual Node air quality monitor</description>

diff --git a/addons/binding/org.openhab.binding.airvisualnode/ESH-INF/thing/thing-types.xml b/addons/binding/org.openhab.binding.airvisualnode/ESH-INF/thing/thing-types.xml
@@ -1,8 +1,8 @@
 <?xml version="1.0" encoding="UTF-8"?>
 <thing:thing-descriptions bindingId="airvisualnode"
-						  xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
-						  xmlns:thing="http://eclipse.org/smarthome/schemas/thing-description/v1.0.0"
-						  xsi:schemaLocation="http://eclipse.org/smarthome/schemas/thing-description/v1.0.0 http://eclipse.org/smarthome/schemas/thing-description-1.0.0.xsd">
+	xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
+	xmlns:thing="http://eclipse.org/smarthome/schemas/thing-description/v1.0.0"
+	xsi:schemaLocation="http://eclipse.org/smarthome/schemas/thing-description/v1.0.0 http://eclipse.org/smarthome/schemas/thing-description-1.0.0.xsd">
 
 	<thing-type id="avnode">
 		<label>AirVisual Node</label>
@@ -11,8 +11,8 @@
 		<!-- Channel groups -->
 
 		<channel-groups>
-			<channel-group id="measurements" typeId="Measurements"/>
-			<channel-group id="status" typeId="Status"/>
+			<channel-group id="measurements" typeId="Measurements" />
+			<channel-group id="status" typeId="Status" />
 		</channel-groups>
 
 		<!-- Configuration parameters -->
@@ -36,19 +36,19 @@
 				<description>Node network password</description>
 			</parameter>
 			<!-- Advanced parameters -->
-            <parameter name="share" type="text">
-                <label>Share Name</label>
-                <description>Node network share name</description>
-                <default>airvisual</default>
-                <advanced>true</advanced>
-            </parameter>
-            <parameter name="refresh" type="integer" min="30" unit="s">
-                <label>Refresh Interval</label>
-                <description>Node data fetches interval (in seconds)</description>
-                <default>60</default>
-                <unitLabel>s</unitLabel>
-                <advanced>true</advanced>
-            </parameter>
+			<parameter name="share" type="text">
+				<label>Share Name</label>
+				<description>Node network share name</description>
+				<default>airvisual</default>
+				<advanced>true</advanced>
+			</parameter>
+			<parameter name="refresh" type="integer" min="30" unit="s">
+				<label>Refresh Interval</label>
+				<description>Node data fetches interval (in seconds)</description>
+				<default>60</default>
+				<unitLabel>s</unitLabel>
+				<advanced>true</advanced>
+			</parameter>
 		</config-description>
 	</thing-type>
 
@@ -83,7 +83,7 @@
 		<item-type>Number</item-type>
 		<label>CO₂ Level</label>
 		<description>CO&#8322; level, ppm</description>
-		<state readOnly="true" pattern="%d ppm"/>
+		<state readOnly="true" pattern="%d ppm" />
 	</channel-type>
 
 	<channel-type id="Humidity">

diff --git a/addons/binding/org.openhab.binding.airvisualnode/README.md b/addons/binding/org.openhab.binding.airvisualnode/README.md
@@ -1,6 +1,6 @@
 # AirVisual Node Binding
 
-This is an openHAB binding for the [AirVisual Node Air Quality Monitor](https://airvisual.com/node).
+This is an openHAB binding for the [AirVisual Node Air Quality Monitor](https://airvisual.com/node) (also known as IQAir AirVisual Pro).
 
 ## Supported Things
 
@@ -38,13 +38,13 @@ The binding support the following channels:
 
 | Channel ID                   | Item Type | Description                 |
 |------------------------------|-----------|-----------------------------|
-| measurements#co2_ppm         | Number    | CO₂ level, ppm               |
+| measurements#co2_ppm         | Number    | CO2 level, ppm              |
 | measurements#humidity        | Number    | Humidity, %                 |
 | measurements#aqi_cn          | Number    | Air Quality Index (Chinese) |
 | measurements#aqi_us          | Number    | Air Quality Index (US)      |
 | measurements#pm_25           | Number    | PM2.5 level, μg/m³          |
-| measurements#temp_celsius    | Number    | Temperature, ℃                       |
-| measurements#temp_fahrenheit | Number    | Temperature, ℉                        |
+| measurements#temp_celsius    | Number    | Temperature, Celsius        |
+| measurements#temp_fahrenheit | Number    | Temperature, Fahrenheit     |
 
 The Node updates measurements data every 5 minutes in active mode and every 15 minutes in power saving mode (screen off).
 
@@ -59,6 +59,15 @@ The Node updates measurements data every 5 minutes in active mode and every 15 m
 
 ## Example
 
+### Thing
+
+The preferred way to add AirVisual Node to the openHAB installation is autodiscovery,
+but the AirVisual Node also can be configured using `.things` file:
+
+```
+airvisualnode:avnode:1a2b3c4 [ address="192.168.1.32", username="airvisual", password="12345", share="airvisual", refresh=60 ]
+```
+
 ### Items
 
 Here is an example of items for the AirVisual Node:

diff --git a/addons/binding/org.openhab.binding.airvisualnode/about.html b/addons/binding/org.openhab.binding.airvisualnode/about.html
@@ -28,5 +28,22 @@ <h3>License</h3>
     and such source code may be obtained at <a href="http://www.openhab.org/">openhab.org</a>.
 </p>
 
+<h3>Third Party Content</h3>
+
+<p>
+	The Content includes items that have been sourced from third parties as set out below. If you.
+    did not receive this Content directly from the openHAB community, the following is provided.
+    for informational purposes only, and you should look to the Redistributor's license for.
+    terms and conditions of use.
+</p>
+
+<p>
+	<em>
+	    <strong>JCIFS</strong> <br/> <br />
+
+	    <a href="https://jcifs.samba.org/src/jcifs-1.3.18.jar">jcifs-1.3.18</a> is distributed under <a href="https://www.gnu.org/licenses/lgpl-2.1.txt">LGPL</a> license.
+    </em>
+</p>
+
 </body>
 </html>
diff --git a/...ualnode/src/main/java/org/openhab/binding/airvisualnode/handler/AirVisualNodeHandler.java b/...ualnode/src/main/java/org/openhab/binding/airvisualnode/handler/AirVisualNodeHandler.java
@@ -89,13 +89,15 @@ public void initialize() {
 
         if (config.address == null) {
             updateStatus(ThingStatus.OFFLINE, ThingStatusDetail.CONFIGURATION_ERROR, "Node address must be set");
+            return;
         }
         this.nodeAddress = config.address;
 
         this.nodeUsername = config.username;
 
         if (config.password == null) {
             updateStatus(ThingStatus.OFFLINE, ThingStatusDetail.CONFIGURATION_ERROR, "Node password must be set");
+            return;
         }
         this.nodePassword = config.password;
 
@@ -136,17 +138,11 @@ private synchronized void stopPoll() {
     }
 
     private synchronized void schedulePoll() {
-        stopPoll();
         logger.debug("Scheduling poll for 500ms out, then every {} ms", refreshInterval);
-        pollFuture = scheduler.scheduleWithFixedDelay(new Runnable() {
-            @Override
-            public void run() {
-                poll();
-            }
-        }, 500, refreshInterval, TimeUnit.MILLISECONDS);
+        pollFuture = scheduler.scheduleWithFixedDelay(this::poll, 500, refreshInterval, TimeUnit.MILLISECONDS);
     }
 
-    void poll() {
+    private void poll() {
         try {
             logger.debug("Polling for state");
             pollNode();
@@ -155,7 +151,7 @@ void poll() {
             logger.debug("Could not connect to Node", e);
             updateStatus(ThingStatus.OFFLINE, ThingStatusDetail.COMMUNICATION_ERROR, e.getMessage());
         } catch (Exception e) {
-            logger.error("Unexpected error connecting to Node", e);
+            logger.debug("Unexpected error connecting to Node", e);
             updateStatus(ThingStatus.OFFLINE, ThingStatusDetail.COMMUNICATION_ERROR, e.getMessage());
         }
     }
@@ -176,7 +172,7 @@ private String getNodeJsonData() throws IOException {
         String url = "smb://" + nodeAddress +"/" + nodeShareName + "/" + NODE_JSON_FILE;
         NtlmPasswordAuthentication auth = new NtlmPasswordAuthentication(null, nodeUsername, nodePassword);
         try (SmbFileInputStream in = new SmbFileInputStream(new SmbFile(url, auth))) {
-            return IOUtils.toString(new SmbFileInputStream(new SmbFile(url, auth)), StandardCharsets.UTF_8.name());
+            return IOUtils.toString(in, StandardCharsets.UTF_8.name());
         }
     }
 

diff --git a/...a/org/openhab/binding/airvisualnode/internal/discovery/AirVisualNodeDiscoveryService.java b/...a/org/openhab/binding/airvisualnode/internal/discovery/AirVisualNodeDiscoveryService.java
@@ -46,15 +46,12 @@ public class AirVisualNodeDiscoveryService extends AbstractDiscoveryService impl
 
     private static final Pattern AVISUAL_NAME_PATTERN = Pattern.compile("^AVISUAL-([^/]+)$");
 
-    private final Runnable scannerRunnable;
-
     private ScheduledFuture<?> backgroundDiscoveryFuture;
 
     private DiscoveryServiceCallback discoveryServiceCallback;
 
     public AirVisualNodeDiscoveryService() {
         super(Collections.singleton(AirVisualNodeBindingConstants.THING_TYPE_AVNODE), 600, true);
-        scannerRunnable = createScannerRunnable();
     }
 
     @Override
@@ -65,14 +62,13 @@ public void setDiscoveryServiceCallback(DiscoveryServiceCallback discoveryServic
     @Override
     protected void startScan() {
         logger.debug("Starting scan");
-        scheduler.execute(scannerRunnable);
+        scheduler.execute(this::scan);
     }
 
     @Override
     protected void startBackgroundDiscovery() {
         logger.debug("Starting background discovery");
-        cancelBackgroundDiscoveryFuture();
-        backgroundDiscoveryFuture = scheduler.scheduleWithFixedDelay(scannerRunnable, 0, 300, TimeUnit.SECONDS);
+        backgroundDiscoveryFuture = scheduler.scheduleWithFixedDelay(this::scan, 0, 300, TimeUnit.SECONDS);
     }
 
     @Override
@@ -89,62 +85,60 @@ private void cancelBackgroundDiscoveryFuture() {
         }
     }
 
-    private Runnable createScannerRunnable() {
-        return () -> {
-            // Get all workgroup members
-            SmbFile[] workgroupMembers;
-            try {
-                String workgroupUrl = "smb://" + AVISUAL_WORKGROUP_NAME +"/";
-                workgroupMembers = new SmbFile(workgroupUrl).listFiles();
-            } catch (Exception e) {
-                // Can't get workgroup member list
-                return;
-            }
+    private void scan() {
+        // Get all workgroup members
+        SmbFile[] workgroupMembers;
+        try {
+            String workgroupUrl = "smb://" + AVISUAL_WORKGROUP_NAME +"/";
+            workgroupMembers = new SmbFile(workgroupUrl).listFiles();
+        } catch (Exception e) {
+            // Can't get workgroup member list
+            return;
+        }
 
-            // Check found workgroup members for the Node devices
-            for (SmbFile s: workgroupMembers) {
-                String serverName = s.getServer();
+        // Check found workgroup members for the Node devices
+        for (SmbFile s: workgroupMembers) {
+            String serverName = s.getServer();
 
-                // Check workgroup member for the Node device name match
-                Matcher m = AVISUAL_NAME_PATTERN.matcher(serverName);
-                if (!m.find()) {
-                    // Workgroup member server name doesn't match the Node device name pattern
-                    continue;
-                }
+            // Check workgroup member for the Node device name match
+            Matcher m = AVISUAL_NAME_PATTERN.matcher(serverName);
+            if (!m.find()) {
+                // Workgroup member server name doesn't match the Node device name pattern
+                continue;
+            }
+
+            // Extract the Node serial number from device name
+            String nodeSerialNumber = m.group(1);
 
-                // Extract the Node serial number from device name
-                String nodeSerialNumber = m.group(1);
+            // The Node Thing UID is serial number converted to lower case
+            ThingUID thingUID = new ThingUID(AirVisualNodeBindingConstants.THING_TYPE_AVNODE,
+                    nodeSerialNumber.toLowerCase());
 
-                // The Node Thing UID is serial number converted to lower case
-                ThingUID thingUID = new ThingUID(AirVisualNodeBindingConstants.THING_TYPE_AVNODE,
-                        nodeSerialNumber.toLowerCase());
+            if (discoveryServiceCallback.getExistingDiscoveryResult(thingUID) != null ||
+                    discoveryServiceCallback.getExistingThing(thingUID) != null) {
+                // The Node with this Thing UID is already discovered or configured as a Thing, skip it
+                continue;
+            }
 
-                if (discoveryServiceCallback.getExistingDiscoveryResult(thingUID) != null ||
-                        discoveryServiceCallback.getExistingThing(thingUID) != null) {
-                    // The Node with this Thing UID is already discovered or configured as a Thing, skip it
+            try {
+                // Get the Node address by name
+                NbtAddress nodeNbtAddress = NbtAddress.getByName(serverName);
+                if (nodeNbtAddress == null) {
+                    // The Node address not found by some reason, skip it
                     continue;
                 }
 
-                try {
-                    // Get the Node address by name
-                    NbtAddress nodeNbtAddress = NbtAddress.getByName(serverName);
-                    if (nodeNbtAddress == null) {
-                        // The Node address not found by some reason, skip it
-                        continue;
-                    }
-
-                    // Create discovery result
-                    String nodeAddress = nodeNbtAddress.getInetAddress().getHostAddress();
-                    DiscoveryResult result = DiscoveryResultBuilder.create(thingUID)
-                            .withProperty(AirVisualNodeConfig.ADDRESS, nodeAddress)
-                            .withLabel("AirVisual Node (" + nodeSerialNumber + ")")
-                            .build();
-                    thingDiscovered(result);
-                } catch (UnknownHostException e) {
-                    // The Node address resolving failed by some reason, nothing to do there
-                }
+                // Create discovery result
+                String nodeAddress = nodeNbtAddress.getInetAddress().getHostAddress();
+                DiscoveryResult result = DiscoveryResultBuilder.create(thingUID)
+                        .withProperty(AirVisualNodeConfig.ADDRESS, nodeAddress)
+                        .withLabel("AirVisual Node (" + nodeSerialNumber + ")")
+                        .build();
+                thingDiscovered(result);
+            } catch (UnknownHostException e) {
+                logger.debug("The Node address resolving failed ", e);
             }
-        };
+        }
     }
 
 }