From b5489057b6488aa8c5b098a9b22540b18bb30b77 Mon Sep 17 00:00:00 2001 From: Jacob Laursen Date: Sun, 19 Jun 2022 14:12:51 +0200 Subject: [PATCH] [hdpowerview] Refactor null-handling, maintenance period, response logging (#12950) * Treat HDPowerViewWebTargets as non-null since initialized by initialize() * Simplify maintenance period logic slightly * Improve response logging Signed-off-by: Jacob Laursen --- .../internal/HDPowerViewWebTargets.java | 15 ++++---- .../HDPowerViewDeviceDiscoveryService.java | 6 +-- .../handler/HDPowerViewHubHandler.java | 37 ++----------------- .../handler/HDPowerViewRepeaterHandler.java | 9 ----- .../handler/HDPowerViewShadeHandler.java | 7 ---- 5 files changed, 14 insertions(+), 60 deletions(-) diff --git a/bundles/org.openhab.binding.hdpowerview/src/main/java/org/openhab/binding/hdpowerview/internal/HDPowerViewWebTargets.java b/bundles/org.openhab.binding.hdpowerview/src/main/java/org/openhab/binding/hdpowerview/internal/HDPowerViewWebTargets.java index 13798a8113..db2ea24334 100644 --- a/bundles/org.openhab.binding.hdpowerview/src/main/java/org/openhab/binding/hdpowerview/internal/HDPowerViewWebTargets.java +++ b/bundles/org.openhab.binding.hdpowerview/src/main/java/org/openhab/binding/hdpowerview/internal/HDPowerViewWebTargets.java @@ -12,6 +12,7 @@ */ package org.openhab.binding.hdpowerview.internal; +import java.time.Duration; import java.time.Instant; import java.util.List; import java.util.concurrent.ExecutionException; @@ -81,8 +82,8 @@ public class HDPowerViewWebTargets { * exception handling during the five minute maintenance period after a 423 * error is received */ - private final int maintenancePeriod = 300; - private Instant maintenanceScheduledEnd = Instant.now().minusSeconds(2 * maintenancePeriod); + private final Duration maintenancePeriod = Duration.ofMinutes(5); + private Instant maintenanceScheduledEnd = Instant.MIN; private final String base; private final String firmwareVersion; @@ -576,7 +577,7 @@ public class HDPowerViewWebTargets { int statusCode = response.getStatus(); if (statusCode == HttpStatus.LOCKED_423) { // set end of maintenance window, and throw a "softer" exception - maintenanceScheduledEnd = Instant.now().plusSeconds(maintenancePeriod); + maintenanceScheduledEnd = Instant.now().plus(maintenancePeriod); logger.debug("Hub undergoing maintenance"); throw new HubMaintenanceException("Hub undergoing maintenance"); } @@ -585,13 +586,13 @@ public class HDPowerViewWebTargets { throw new HubProcessingException(String.format("HTTP %d error", statusCode)); } String jsonResponse = response.getContentAsString(); - if ("".equals(jsonResponse)) { - logger.warn("Hub returned no content"); - throw new HubProcessingException("Missing response entity"); - } if (logger.isTraceEnabled()) { logger.trace("JSON response = {}", jsonResponse); } + if (jsonResponse == null || jsonResponse.isEmpty()) { + logger.warn("Hub returned no content"); + throw new HubProcessingException("Missing response entity"); + } return jsonResponse; } diff --git a/bundles/org.openhab.binding.hdpowerview/src/main/java/org/openhab/binding/hdpowerview/internal/discovery/HDPowerViewDeviceDiscoveryService.java b/bundles/org.openhab.binding.hdpowerview/src/main/java/org/openhab/binding/hdpowerview/internal/discovery/HDPowerViewDeviceDiscoveryService.java index d8f5d9862d..98d108befc 100644 --- a/bundles/org.openhab.binding.hdpowerview/src/main/java/org/openhab/binding/hdpowerview/internal/discovery/HDPowerViewDeviceDiscoveryService.java +++ b/bundles/org.openhab.binding.hdpowerview/src/main/java/org/openhab/binding/hdpowerview/internal/discovery/HDPowerViewDeviceDiscoveryService.java @@ -40,9 +40,10 @@ import org.slf4j.Logger; import org.slf4j.LoggerFactory; /** - * Discovers an HD PowerView Shade from an existing hub + * Discovers HD PowerView Shades and Repeaters from an existing hub * * @author Andy Lintner - Initial contribution + * @author Jacob Laursen - Add Repeater discovery */ @NonNullByDefault public class HDPowerViewDeviceDiscoveryService extends AbstractDiscoveryService { @@ -87,9 +88,6 @@ public class HDPowerViewDeviceDiscoveryService extends AbstractDiscoveryService return () -> { try { HDPowerViewWebTargets webTargets = hub.getWebTargets(); - if (webTargets == null) { - throw new HubProcessingException("Web targets not initialized"); - } discoverShades(webTargets); discoverRepeaters(webTargets); } catch (HubMaintenanceException e) { diff --git a/bundles/org.openhab.binding.hdpowerview/src/main/java/org/openhab/binding/hdpowerview/internal/handler/HDPowerViewHubHandler.java b/bundles/org.openhab.binding.hdpowerview/src/main/java/org/openhab/binding/hdpowerview/internal/handler/HDPowerViewHubHandler.java index 18014108b3..56d2b68382 100644 --- a/bundles/org.openhab.binding.hdpowerview/src/main/java/org/openhab/binding/hdpowerview/internal/handler/HDPowerViewHubHandler.java +++ b/bundles/org.openhab.binding.hdpowerview/src/main/java/org/openhab/binding/hdpowerview/internal/handler/HDPowerViewHubHandler.java @@ -22,8 +22,6 @@ import java.util.concurrent.CopyOnWriteArrayList; import java.util.concurrent.ScheduledFuture; import java.util.concurrent.TimeUnit; -import javax.ws.rs.ProcessingException; - import org.eclipse.jdt.annotation.NonNullByDefault; import org.eclipse.jdt.annotation.Nullable; import org.eclipse.jetty.client.HttpClient; @@ -89,7 +87,7 @@ public class HDPowerViewHubHandler extends BaseBridgeHandler { private long hardRefreshPositionInterval; private long hardRefreshBatteryLevelInterval; - private @Nullable HDPowerViewWebTargets webTargets; + private @NonNullByDefault({}) HDPowerViewWebTargets webTargets; private @Nullable ScheduledFuture pollFuture; private @Nullable ScheduledFuture hardRefreshPositionFuture; private @Nullable ScheduledFuture hardRefreshBatteryLevelFuture; @@ -129,10 +127,6 @@ public class HDPowerViewHubHandler extends BaseBridgeHandler { } try { - HDPowerViewWebTargets webTargets = this.webTargets; - if (webTargets == null) { - throw new ProcessingException("Web targets not initialized"); - } int id = Integer.parseInt(channelUID.getIdWithoutGroup()); if (sceneChannelTypeUID.equals(channel.getChannelTypeUID()) && OnOffType.ON == command) { webTargets.activateScene(id); @@ -164,7 +158,6 @@ public class HDPowerViewHubHandler extends BaseBridgeHandler { return; } - updateStatus(ThingStatus.UNKNOWN); pendingShadeInitializations.clear(); webTargets = new HDPowerViewWebTargets(httpClient, host); refreshInterval = config.refresh; @@ -172,6 +165,8 @@ public class HDPowerViewHubHandler extends BaseBridgeHandler { hardRefreshBatteryLevelInterval = config.hardRefreshBatteryLevel; initializeChannels(); firmwareVersions = null; + + updateStatus(ThingStatus.UNKNOWN); schedulePoll(); } @@ -184,7 +179,7 @@ public class HDPowerViewHubHandler extends BaseBridgeHandler { deprecatedChannelsCreated = false; } - public @Nullable HDPowerViewWebTargets getWebTargets() { + public HDPowerViewWebTargets getWebTargets() { return webTargets; } @@ -320,10 +315,6 @@ public class HDPowerViewHubHandler extends BaseBridgeHandler { if (firmwareVersions != null) { return; } - HDPowerViewWebTargets webTargets = this.webTargets; - if (webTargets == null) { - throw new ProcessingException("Web targets not initialized"); - } FirmwareVersions firmwareVersions = webTargets.getFirmwareVersions(); Firmware mainProcessor = firmwareVersions.mainProcessor; if (mainProcessor == null) { @@ -346,11 +337,6 @@ public class HDPowerViewHubHandler extends BaseBridgeHandler { } private void pollShades() throws HubInvalidResponseException, HubProcessingException, HubMaintenanceException { - HDPowerViewWebTargets webTargets = this.webTargets; - if (webTargets == null) { - throw new ProcessingException("Web targets not initialized"); - } - Shades shades = webTargets.getShades(); List shadesData = shades.shadeData; if (shadesData == null) { @@ -434,11 +420,6 @@ public class HDPowerViewHubHandler extends BaseBridgeHandler { private List fetchScenes() throws HubInvalidResponseException, HubProcessingException, HubMaintenanceException { - HDPowerViewWebTargets webTargets = this.webTargets; - if (webTargets == null) { - throw new ProcessingException("Web targets not initialized"); - } - Scenes scenes = webTargets.getScenes(); List sceneData = scenes.sceneData; if (sceneData == null) { @@ -520,11 +501,6 @@ public class HDPowerViewHubHandler extends BaseBridgeHandler { private List fetchSceneCollections() throws HubInvalidResponseException, HubProcessingException, HubMaintenanceException { - HDPowerViewWebTargets webTargets = this.webTargets; - if (webTargets == null) { - throw new ProcessingException("Web targets not initialized"); - } - SceneCollections sceneCollections = webTargets.getSceneCollections(); List sceneCollectionData = sceneCollections.sceneCollectionData; if (sceneCollectionData == null) { @@ -565,11 +541,6 @@ public class HDPowerViewHubHandler extends BaseBridgeHandler { private List fetchScheduledEvents() throws HubInvalidResponseException, HubProcessingException, HubMaintenanceException { - HDPowerViewWebTargets webTargets = this.webTargets; - if (webTargets == null) { - throw new ProcessingException("Web targets not initialized"); - } - ScheduledEvents scheduledEvents = webTargets.getScheduledEvents(); List scheduledEventData = scheduledEvents.scheduledEventData; if (scheduledEventData == null) { diff --git a/bundles/org.openhab.binding.hdpowerview/src/main/java/org/openhab/binding/hdpowerview/internal/handler/HDPowerViewRepeaterHandler.java b/bundles/org.openhab.binding.hdpowerview/src/main/java/org/openhab/binding/hdpowerview/internal/handler/HDPowerViewRepeaterHandler.java index 498a147ee7..037c85184d 100644 --- a/bundles/org.openhab.binding.hdpowerview/src/main/java/org/openhab/binding/hdpowerview/internal/handler/HDPowerViewRepeaterHandler.java +++ b/bundles/org.openhab.binding.hdpowerview/src/main/java/org/openhab/binding/hdpowerview/internal/handler/HDPowerViewRepeaterHandler.java @@ -94,11 +94,6 @@ public class HDPowerViewRepeaterHandler extends AbstractHubbedThingHandler { return; } HDPowerViewWebTargets webTargets = bridge.getWebTargets(); - if (webTargets == null) { - logger.warn("Web targets not initialized"); - return; - } - try { RepeaterData repeaterData; @@ -200,10 +195,6 @@ public class HDPowerViewRepeaterHandler extends AbstractHubbedThingHandler { return; } HDPowerViewWebTargets webTargets = bridge.getWebTargets(); - if (webTargets == null) { - logger.warn("Web targets not initialized"); - return; - } try { logger.debug("Polling for status information"); diff --git a/bundles/org.openhab.binding.hdpowerview/src/main/java/org/openhab/binding/hdpowerview/internal/handler/HDPowerViewShadeHandler.java b/bundles/org.openhab.binding.hdpowerview/src/main/java/org/openhab/binding/hdpowerview/internal/handler/HDPowerViewShadeHandler.java index 35be15d3a9..36a51d1482 100644 --- a/bundles/org.openhab.binding.hdpowerview/src/main/java/org/openhab/binding/hdpowerview/internal/handler/HDPowerViewShadeHandler.java +++ b/bundles/org.openhab.binding.hdpowerview/src/main/java/org/openhab/binding/hdpowerview/internal/handler/HDPowerViewShadeHandler.java @@ -162,10 +162,6 @@ public class HDPowerViewShadeHandler extends AbstractHubbedThingHandler { return; } HDPowerViewWebTargets webTargets = bridge.getWebTargets(); - if (webTargets == null) { - logger.warn("Web targets not initialized"); - return; - } try { handleShadeCommand(channelId, command, webTargets, shadeId); } catch (HubInvalidResponseException e) { @@ -523,9 +519,6 @@ public class HDPowerViewShadeHandler extends AbstractHubbedThingHandler { throw new HubProcessingException("Missing bridge handler"); } HDPowerViewWebTargets webTargets = bridge.getWebTargets(); - if (webTargets == null) { - throw new HubProcessingException("Web targets not initialized"); - } ShadeData shadeData; switch (kind) { case POSITION: -- 2.47.3