[orbitbhyve] improvements based on code review

Signed-off-by: Ondrej Pecta <opecta@gmail.com>
openhab · Jul 18, 2021 · a6ec3c9 · a6ec3c9
1 parent a853bf8
commit a6ec3c9
Show file tree

Hide file tree

Showing 13 changed files with 107 additions and 104 deletions.
diff --git a/bom/openhab-addons/pom.xml b/bom/openhab-addons/pom.xml
@@ -1084,6 +1084,11 @@
       <artifactId>org.openhab.binding.oppo</artifactId>
       <version>${project.version}</version>
     </dependency>
+    <dependency>
+      <groupId>org.openhab.addons.bundles</groupId>
+      <artifactId>org.openhab.binding.orbitbhyve</artifactId>
+      <version>${project.version}</version>
+    </dependency>
     <dependency>
       <groupId>org.openhab.addons.bundles</groupId>
       <artifactId>org.openhab.binding.orvibo</artifactId>

diff --git a/bundles/org.openhab.binding.orbitbhyve/README.md b/bundles/org.openhab.binding.orbitbhyve/README.md
@@ -12,10 +12,6 @@ So far only the [Orbit B-hyve 8-zone Indoor Timer](https://bhyve.orbitonline.com
 This binding supports the auto discovery of the sprinklers bound to your Orbit B-hyve account.  
 To start the discovery you need to create a bridge thing and enter valid credentials to your Orbit B-hyve cloud account.
 
-## Binding Configuration
-
-This binding does not require any configuration on the binding level.
-
 ## Thing Configuration
 
 The bridge thing requires a manual configuration. You have to enter valid credentials to your Orbit B-hyve account, and you can also set the refresh time in seconds for polling data from the Orbit cloud.  
@@ -33,16 +29,14 @@ This binding automatically detects all zones and programs for each sprinkler and
 
 Beside the dynamic channels each sprinkler thing provides these standard channels:
 
-| channel        | type     | description                                                        |
-|----------------|----------|--------------------------------------------------------------------|
-| mode           | String   | This channel represents the mode of sprinkler device (auto/manual) |
-| next_start     | DateTime | This channel represents the start time of the next watering        |
-| rain_delay     | Number   | This channel manages the current rain delay in hours               |
-| watering_time  | Number   | This channel manages the manual zone watering time in minutes      |
-| control        | Switch   | This channel controls the sprinkler (ON/OFF)                       |
-| smart_watering | Switch   | This channel controls the smart watering (ON/OFF)                  |
-| control        | String   | This channel manages the manual zone watering time in minutes      |
-
+| channel        | type        | description                                                        |
+|----------------|-------------|--------------------------------------------------------------------|
+| mode           | String      | This channel represents the mode of sprinkler device (auto/manual) |
+| next_start     | DateTime    | This channel represents the start time of the next watering        |
+| rain_delay     | Number      | This channel manages the current rain delay in hours               |
+| watering_time  | Number:Time | This channel manages the manual zone watering time in minutes      |
+| control        | Switch      | This channel controls the sprinkler (ON/OFF)                       |
+| smart_watering | Switch      | This channel controls the smart watering (ON/OFF)                  |
 
 ## Full Example
 
@@ -87,4 +81,4 @@ Text item=IrrigationMode
 Text item=IrrigationRainDelay  
 Switch item=IrrigationRainDelay mappings=[0="OFF", 24="24", 48="48", 72="72"]  
 Text item=IrrigationNextStart visibility=[IrrigationP1Enable==ON]  
-```
+```
diff --git a/bundles/org.openhab.binding.orbitbhyve/pom.xml b/bundles/org.openhab.binding.orbitbhyve/pom.xml
@@ -12,6 +12,6 @@
 
   <artifactId>org.openhab.binding.orbitbhyve</artifactId>
 
-  <name>openHAB Add-ons :: Bundles :: OrbitBhyve Binding</name>
+  <name>openHAB Add-ons :: Bundles :: Orbit B-hyve Binding</name>
 
 </project>
diff --git a/bundles/org.openhab.binding.orbitbhyve/src/main/feature/feature.xml b/bundles/org.openhab.binding.orbitbhyve/src/main/feature/feature.xml
@@ -2,7 +2,7 @@
 <features name="org.openhab.binding.orbitbhyve-${project.version}" xmlns="http://karaf.apache.org/xmlns/features/v1.4.0">
 	<repository>mvn:org.openhab.core.features.karaf/org.openhab.core.features.karaf.openhab-core/${ohc.version}/xml/features</repository>
 
-	<feature name="openhab-binding-orbitbhyve" description="OrbitBhyve Binding" version="${project.version}">
+	<feature name="openhab-binding-orbitbhyve" description="Orbit B-hyve Binding" version="${project.version}">
 		<feature>openhab-runtime-base</feature>
 		<bundle start-level="80">mvn:org.openhab.addons.bundles/org.openhab.binding.orbitbhyve/${project.version}</bundle>
 	</feature>

diff --git a/...bhyve/src/main/java/org/openhab/binding/orbitbhyve/internal/OrbitBhyveHandlerFactory.java b/...bhyve/src/main/java/org/openhab/binding/orbitbhyve/internal/OrbitBhyveHandlerFactory.java
@@ -53,7 +53,7 @@ public class OrbitBhyveHandlerFactory extends BaseThingHandlerFactory {
     /**
      * the shared http client
      */
-    private @NonNullByDefault({}) HttpClient httpClient;
+    private HttpClient httpClient;
 
     /**
      * the shared web socket client

diff --git a/...rc/main/java/org/openhab/binding/orbitbhyve/internal/handler/OrbitBhyveBridgeHandler.java b/...rc/main/java/org/openhab/binding/orbitbhyve/internal/handler/OrbitBhyveBridgeHandler.java
@@ -79,6 +79,8 @@ public class OrbitBhyveBridgeHandler extends ConfigStatusBridgeHandler {
 
     private @Nullable ScheduledFuture<?> future = null;
 
+    private @Nullable ScheduledFuture<?> statusFuture = null;
+
     private @Nullable Session session;
 
     private @Nullable String sessionToken = null;
@@ -125,7 +127,11 @@ public void initialize() {
     @Override
     public void dispose() {
         ScheduledFuture<?> localFuture = future;
-        if (localFuture != null && !(localFuture.isCancelled() || localFuture.isDone())) {
+        if (localFuture != null) {
+            localFuture.cancel(true);
+        }
+        localFuture = statusFuture;
+        if (localFuture != null) {
             localFuture.cancel(true);
         }
         closeSession();
@@ -149,15 +155,16 @@ private boolean login() {
                 logger.debug("token: {}", sessionToken);
                 initializeWebSocketSession();
             } else {
-                updateStatus(ThingStatus.OFFLINE, ThingStatusDetail.CONFIGURATION_ERROR);
+                updateStatus(ThingStatus.OFFLINE, ThingStatusDetail.CONFIGURATION_ERROR,
+                        "Login response status:" + response.getStatus());
                 return false;
             }
-        } catch (InterruptedException | TimeoutException | ExecutionException e) {
-            logger.debug("Exception during login", e);
-            updateStatus(ThingStatus.OFFLINE, ThingStatusDetail.COMMUNICATION_ERROR);
-            if (e instanceof InterruptedException) {
-                Thread.currentThread().interrupt();
-            }
+        } catch (TimeoutException | ExecutionException e) {
+            updateStatus(ThingStatus.OFFLINE, ThingStatusDetail.COMMUNICATION_ERROR, "Exception during login");
+            return false;
+        } catch (InterruptedException e) {
+            updateStatus(ThingStatus.OFFLINE, ThingStatusDetail.COMMUNICATION_ERROR, "Exception during login");
+            Thread.currentThread().interrupt();
             return false;
         }
         updateStatus(ThingStatus.ONLINE);
@@ -180,12 +187,11 @@ private synchronized void ping() {
                     logger.debug("Sending ping");
                     localSession.getRemote().sendString("{\"event\":\"ping\"}");
                 } catch (IOException e) {
-                    logger.debug("Error sending ping to a web socket", e);
                     updateStatus(ThingStatus.OFFLINE, ThingStatusDetail.COMMUNICATION_ERROR,
-                            "web socket communication error");
+                            "Error sending ping to a web socket");
                 }
             } else {
-                updateStatus(ThingStatus.OFFLINE, ThingStatusDetail.COMMUNICATION_ERROR, "web socket creation error");
+                updateStatus(ThingStatus.OFFLINE, ThingStatusDetail.COMMUNICATION_ERROR, "Web socket creation error");
             }
         }
     }
@@ -200,15 +206,14 @@ public List<OrbitBhyveDevice> getDevices() {
                 OrbitBhyveDevice[] devices = gson.fromJson(response.getContentAsString(), OrbitBhyveDevice[].class);
                 return Arrays.asList(devices);
             } else {
-                logger.debug("Returned status: {}", response.getStatus());
-                updateStatus(ThingStatus.OFFLINE);
-            }
-        } catch (InterruptedException | TimeoutException | ExecutionException e) {
-            logger.debug("Error during getting devices", e);
-            updateStatus(ThingStatus.OFFLINE);
-            if (e instanceof InterruptedException) {
-                Thread.currentThread().interrupt();
+                updateStatus(ThingStatus.OFFLINE, ThingStatusDetail.COMMUNICATION_ERROR,
+                        "Get devices returned response status: " + response.getStatus());
             }
+        } catch (TimeoutException | ExecutionException e) {
+            updateStatus(ThingStatus.OFFLINE, ThingStatusDetail.COMMUNICATION_ERROR, "Error during getting devices");
+        } catch (InterruptedException e) {
+            updateStatus(ThingStatus.OFFLINE, ThingStatusDetail.COMMUNICATION_ERROR, "Error during getting devices");
+            Thread.currentThread().interrupt();
         }
         return new ArrayList<>();
     }
@@ -232,13 +237,13 @@ Request sendRequestBuilder(String uri, HttpMethod method) {
                 updateStatus(ThingStatus.OFFLINE, ThingStatusDetail.COMMUNICATION_ERROR,
                         "Returned status: " + response.getStatus());
             }
-        } catch (InterruptedException | TimeoutException | ExecutionException e) {
-            logger.debug("Error during getting device: {}", deviceId, e);
+        } catch (TimeoutException | ExecutionException e) {
             updateStatus(ThingStatus.OFFLINE, ThingStatusDetail.COMMUNICATION_ERROR,
-                    "Error during getting device info");
-            if (e instanceof InterruptedException) {
-                Thread.currentThread().interrupt();
-            }
+                    "Error during getting device info: " + deviceId);
+        } catch (InterruptedException e) {
+            updateStatus(ThingStatus.OFFLINE, ThingStatusDetail.COMMUNICATION_ERROR,
+                    "Error during getting device info: " + deviceId);
+            Thread.currentThread().interrupt();
         }
         return null;
     }
@@ -253,7 +258,6 @@ public synchronized void processStatusResponse(String content) {
     }
 
     private void processEvent(OrbitBhyveSocketEvent event) {
-        Runnable runnableUpdateDeviceStatus = () -> updateDeviceStatus(event.getDeviceId());
         switch (event.getEvent()) {
             case "watering_in_progress_notification":
                 disableZones(event.getDeviceId());
@@ -286,10 +290,10 @@ private void processEvent(OrbitBhyveSocketEvent event) {
                 if (ch != null) {
                     updateState(ch.getUID(), "off".equals(event.getMode()) ? OnOffType.OFF : OnOffType.ON);
                 }
-                scheduler.schedule(runnableUpdateDeviceStatus, 3, TimeUnit.SECONDS);
+                deferredStatusUpdate(event.getDeviceId());
                 break;
             case "rain_delay":
-                scheduler.schedule(runnableUpdateDeviceStatus, 3, TimeUnit.SECONDS);
+                deferredStatusUpdate(event.getDeviceId());
                 break;
             case "skip_active_station":
                 disableZones(event.getDeviceId());
@@ -298,14 +302,22 @@ private void processEvent(OrbitBhyveSocketEvent event) {
                 OrbitBhyveProgram program = gson.fromJson(event.getProgram(), OrbitBhyveProgram.class);
                 if (program != null) {
                     updateDeviceProgramStatus(program);
-                    scheduler.schedule(() -> updateDeviceStatus(program.getDeviceId()), 3, TimeUnit.SECONDS);
+                    deferredStatusUpdate(program.getDeviceId());
                 }
                 break;
             default:
                 logger.debug("Received event: {}", event.getEvent());
         }
     }
 
+    private void deferredStatusUpdate(String deviceId) {
+        ScheduledFuture<?> localFuture = statusFuture;
+        if (localFuture != null) {
+            localFuture.cancel(true);
+        }
+        statusFuture = scheduler.schedule(() -> updateDeviceStatus(deviceId), 3, TimeUnit.SECONDS);
+    }
+
     private void updateDeviceStatus(String deviceId) {
         for (Thing th : getThing().getThings()) {
             if (deviceId.equals(th.getUID().getId())) {
@@ -385,14 +397,11 @@ private void disableChannel(String deviceId, String name) {
             // Wait for Connect
             return fut.get();
         } catch (IOException e) {
-            logger.debug("Cannot connect websocket client", e);
             updateStatus(ThingStatus.OFFLINE, ThingStatusDetail.COMMUNICATION_ERROR, "Cannot connect websocket client");
         } catch (InterruptedException e) {
-            logger.debug("Cannot create websocket session", e);
             updateStatus(ThingStatus.OFFLINE, ThingStatusDetail.COMMUNICATION_ERROR, "Cannot create websocket session");
             Thread.currentThread().interrupt();
         } catch (ExecutionException e) {
-            logger.debug("Cannot create websocket session", e);
             updateStatus(ThingStatus.OFFLINE, ThingStatusDetail.COMMUNICATION_ERROR, "Cannot create websocket session");
         }
         return null;
@@ -410,7 +419,6 @@ private synchronized void initializeWebSocketSession() {
                 logger.trace("sending message:\n {}", msg);
                 localSession.getRemote().sendString(msg);
             } catch (IOException e) {
-                logger.debug("Cannot send hello string to web socket!", e);
                 updateStatus(ThingStatus.OFFLINE, ThingStatusDetail.COMMUNICATION_ERROR,
                         "Cannot send hello string to web socket!");
             }
@@ -436,7 +444,6 @@ public void runZone(String deviceId, String zone, int time) {
                                 + ",\"run_time\":" + time + "}]}");
             }
         } catch (IOException e) {
-            logger.debug("Error during zone watering execution", e);
             updateStatus(ThingStatus.OFFLINE, ThingStatusDetail.COMMUNICATION_ERROR,
                     "Error during zone watering execution");
         }
@@ -452,7 +459,6 @@ public void runProgram(String deviceId, String program) {
                         + program + "\",\"device_id\":\"" + deviceId + "\",\"timestamp\":\"" + dateTime + "\"}");
             }
         } catch (IOException e) {
-            logger.debug("Error during program watering execution", e);
             updateStatus(ThingStatus.OFFLINE, ThingStatusDetail.COMMUNICATION_ERROR,
                     "Error during program watering execution");
         }
@@ -475,12 +481,11 @@ public void enableProgram(OrbitBhyveProgram program, boolean enable) {
                 logger.debug("Returned status: {}", response.getStatus());
                 updateStatus(ThingStatus.OFFLINE);
             }
-        } catch (InterruptedException | TimeoutException | ExecutionException e) {
-            logger.debug("Error during updating program", e);
-            updateStatus(ThingStatus.OFFLINE, ThingStatusDetail.COMMUNICATION_ERROR, "Error during getting programs");
-            if (e instanceof InterruptedException) {
-                Thread.currentThread().interrupt();
-            }
+        } catch (TimeoutException | ExecutionException e) {
+            updateStatus(ThingStatus.OFFLINE, ThingStatusDetail.COMMUNICATION_ERROR, "Error during updating programs");
+        } catch (InterruptedException e) {
+            updateStatus(ThingStatus.OFFLINE, ThingStatusDetail.COMMUNICATION_ERROR, "Error during updating programs");
+            Thread.currentThread().interrupt();
         }
     }
 
@@ -494,7 +499,6 @@ public void setRainDelay(String deviceId, int delay) {
                         + "\",\"delay\":" + delay + ",\"timestamp\":\"" + dateTime + "\"}");
             }
         } catch (IOException e) {
-            logger.debug("Error during rain delay setting", e);
             updateStatus(ThingStatus.OFFLINE, ThingStatusDetail.COMMUNICATION_ERROR, "Error during rain delay setting");
         }
     }
@@ -509,7 +513,6 @@ public void stopWatering(String deviceId) {
                         + "\",\"timestamp\":\"" + dateTime + "\",\"mode\":\"manual\",\"stations\":[]}");
             }
         } catch (IOException e) {
-            logger.debug("Error during watering stopping", e);
             updateStatus(ThingStatus.OFFLINE, ThingStatusDetail.COMMUNICATION_ERROR, "Error during watering stopping");
         }
     }
@@ -527,12 +530,11 @@ public List<OrbitBhyveProgram> getPrograms() {
                 logger.debug("Returned status: {}", response.getStatus());
                 updateStatus(ThingStatus.OFFLINE);
             }
-        } catch (InterruptedException | TimeoutException | ExecutionException e) {
-            logger.debug("Error during getting programs", e);
+        } catch (TimeoutException | ExecutionException e) {
             updateStatus(ThingStatus.OFFLINE, ThingStatusDetail.COMMUNICATION_ERROR, "Error during getting programs");
-            if (e instanceof InterruptedException) {
-                Thread.currentThread().interrupt();
-            }
+        } catch (InterruptedException e) {
+            updateStatus(ThingStatus.OFFLINE, ThingStatusDetail.COMMUNICATION_ERROR, "Error during getting programs");
+            Thread.currentThread().interrupt();
         }
         return new ArrayList<>();
     }
@@ -547,7 +549,6 @@ public void changeRunMode(String deviceId, String mode) {
                         + "\",\"device_id\":\"" + deviceId + "\",\"timestamp\":\"" + dateTime + "\"}");
             }
         } catch (IOException e) {
-            logger.debug("Error during setting run mode", e);
             updateStatus(ThingStatus.OFFLINE, ThingStatusDetail.COMMUNICATION_ERROR, "Error during setting run mode");
         }
     }
@@ -566,23 +567,18 @@ private void updateDevice(String deviceId, String deviceString) {
         try {
             ContentResponse response = sendRequestBuilder(BHYVE_DEVICES + "/" + deviceId, HttpMethod.PUT)
                     .content(new StringContentProvider(payload), "application/json;charset=UTF-8").send();
-            if (response.getStatus() == 200) {
-                if (logger.isTraceEnabled()) {
-                    logger.trace("Device update response: {}", response.getContentAsString());
-                }
-            } else {
-                logger.debug("Returned status: {}", response.getStatus());
-                if (logger.isTraceEnabled()) {
-                    logger.trace("Device update response: {}", response.getContentAsString());
-                }
-                updateStatus(ThingStatus.OFFLINE);
+            if (logger.isTraceEnabled()) {
+                logger.trace("Device update response: {}", response.getContentAsString());
             }
-        } catch (InterruptedException | TimeoutException | ExecutionException e) {
-            logger.debug("Error during updating device", e);
-            updateStatus(ThingStatus.OFFLINE);
-            if (e instanceof InterruptedException) {
-                Thread.currentThread().interrupt();
+            if (response.getStatus() != 200) {
+                updateStatus(ThingStatus.OFFLINE, ThingStatusDetail.COMMUNICATION_ERROR,
+                        "Update device response status: " + response.getStatus());
             }
+        } catch (TimeoutException | ExecutionException e) {
+            updateStatus(ThingStatus.OFFLINE, ThingStatusDetail.COMMUNICATION_ERROR, "Error during updating device");
+        } catch (InterruptedException e) {
+            updateStatus(ThingStatus.OFFLINE, ThingStatusDetail.COMMUNICATION_ERROR, "Error during updating device");
+            Thread.currentThread().interrupt();
         }
     }
 }
diff --git a/...main/java/org/openhab/binding/orbitbhyve/internal/handler/OrbitBhyveSprinklerHandler.java b/...main/java/org/openhab/binding/orbitbhyve/internal/handler/OrbitBhyveSprinklerHandler.java
@@ -256,6 +256,6 @@ public void updateProgram(OrbitBhyveProgram program) {
     }
 
     public void updateSmartWatering(String senseMode) {
-        updateState(CHANNEL_SMART_WATERING, (senseMode.equals("auto")) ? OnOffType.ON : OnOffType.OFF);
+        updateState(CHANNEL_SMART_WATERING, ("auto".equals(senseMode)) ? OnOffType.ON : OnOffType.OFF);
     }
 }
diff --git a/...rbitbhyve/src/main/java/org/openhab/binding/orbitbhyve/internal/net/OrbitBhyveSocket.java b/...rbitbhyve/src/main/java/org/openhab/binding/orbitbhyve/internal/net/OrbitBhyveSocket.java
@@ -27,7 +27,7 @@
  */
 @NonNullByDefault
 public class OrbitBhyveSocket extends WebSocketAdapter {
-    private final Logger logger = LoggerFactory.getLogger(OrbitBhyveBridgeHandler.class);
+    private final Logger logger = LoggerFactory.getLogger(OrbitBhyveSocket.class);
     private OrbitBhyveBridgeHandler handler;
 
     public OrbitBhyveSocket(OrbitBhyveBridgeHandler handler) {