[Hydrawise] concurrent error (openhab#13268)

* Fixes a concurrent modification exception, cleans up handler on dispose, fixes totally broken last contact channel Fixes openhab#13094 Signed-off-by: Dan Cunningham <dan@digitaldan.com>
psmedley · Feb 23, 2023 · c542cdc · c542cdc
1 parent bc126ad
commit c542cdc
Show file tree

Hide file tree

Showing 4 changed files with 27 additions and 13 deletions.
diff --git a/bundles/org.openhab.binding.hydrawise/README.md b/bundles/org.openhab.binding.hydrawise/README.md
@@ -172,7 +172,7 @@ Group SprinklerSensors      "Sensors"       (Sprinkler)
 Group SprinkerForecast      "Forecast"      (Sprinkler)
 
 String SprinkerControllerStatus "Status [%s]" (SprinklerController) {channel="hydrawise:controller:myaccount:123456:controller#status"}
-Number SprinkerControllerLastContact "Last Contact [%d]" (SprinklerController) {channel="hydrawise:controller:myaccount:123456:controller#lastContact"}
+Number SprinkerControllerLastContact "Last Contact [%d]" (SprinklerController) {channel="hydrawise:controller:myaccount:123456:controller#lastcontact"}
 
 Switch SprinklerSensor1 "Sprinler Sensor" (SprinklerSensors) {channel="hydrawise:controller:myaccount:123456:sensor1#active"}
 

diff --git a/...src/main/java/org/openhab/binding/hydrawise/internal/handler/HydrawiseAccountHandler.java b/...src/main/java/org/openhab/binding/hydrawise/internal/handler/HydrawiseAccountHandler.java
@@ -66,7 +66,8 @@ public class HydrawiseAccountHandler extends BaseBridgeHandler implements Access
     private static final String CLIENT_SECRET = "zn3CrjglwNV1";
     private static final String CLIENT_ID = "hydrawise_app";
     private static final String SCOPE = "all";
-    private final List<HydrawiseControllerListener> controllerListeners = new ArrayList<HydrawiseControllerListener>();
+    private final List<HydrawiseControllerListener> controllerListeners = Collections
+            .synchronizedList(new ArrayList<HydrawiseControllerListener>());
     private final HydrawiseGraphQLClient apiClient;
     private final OAuthClientService oAuthService;
     private @Nullable ScheduledFuture<?> pollFuture;
@@ -116,7 +117,9 @@ public void addControllerListeners(HydrawiseControllerListener listener) {
     }
 
     public void removeControllerListeners(HydrawiseControllerListener listener) {
-        this.controllerListeners.remove(listener);
+        synchronized (controllerListeners) {
+            this.controllerListeners.remove(listener);
+        }
     }
 
     public @Nullable HydrawiseGraphQLClient graphQLClient() {
@@ -197,9 +200,11 @@ private void poll(boolean retry) {
                 updateStatus(ThingStatus.ONLINE);
             }
             lastData = response.data.me;
-            controllerListeners.forEach(listener -> {
-                listener.onData(response.data.me.controllers);
-            });
+            synchronized (controllerListeners) {
+                controllerListeners.forEach(listener -> {
+                    listener.onData(response.data.me.controllers);
+                });
+            }
         } catch (HydrawiseConnectionException e) {
             if (retry) {
                 logger.debug("Retrying failed poll", e);

diff --git a/.../main/java/org/openhab/binding/hydrawise/internal/handler/HydrawiseControllerHandler.java b/.../main/java/org/openhab/binding/hydrawise/internal/handler/HydrawiseControllerHandler.java
@@ -96,11 +96,11 @@ public HydrawiseControllerHandler(Thing thing) {
     public void initialize() {
         HydrawiseControllerConfiguration config = getConfigAs(HydrawiseControllerConfiguration.class);
         controllerId = config.controllerId;
-        Bridge bridge = getBridge();
-        if (bridge != null) {
-            HydrawiseAccountHandler handler = (HydrawiseAccountHandler) bridge.getHandler();
-            if (handler != null) {
-                handler.addControllerListeners(this);
+        HydrawiseAccountHandler handler = getAccountHandler();
+        if (handler != null) {
+            handler.addControllerListeners(this);
+            Bridge bridge = getBridge();
+            if (bridge != null) {
                 if (bridge.getStatus() == ThingStatus.ONLINE) {
                     updateStatus(ThingStatus.ONLINE);
                 } else {
@@ -110,6 +110,15 @@ public void initialize() {
         }
     }
 
+    @Override
+    public void dispose() {
+        logger.debug("Controller Handler disposed.");
+        HydrawiseAccountHandler handler = getAccountHandler();
+        if (handler != null) {
+            handler.removeControllerListeners(this);
+        }
+    }
+
     @Override
     public void handleCommand(ChannelUID channelUID, Command command) {
         logger.debug("handleCommand channel {} Command {}", channelUID.getAsString(), command.toFullString());

diff --git a/bundles/org.openhab.binding.hydrawise/src/main/resources/OH-INF/thing/channel-types.xml b/bundles/org.openhab.binding.hydrawise/src/main/resources/OH-INF/thing/channel-types.xml
@@ -68,13 +68,13 @@
 		<channels>
 			<channel id="name" typeId="name"/>
 			<channel id="summary" typeId="summary"/>
-			<channel id="lastcontacttime" typeId="lastcontacttime"/>
+			<channel id="lastcontact" typeId="lastcontact"/>
 		</channels>
 	</channel-group-type>
 
 	<!-- Controller -->
 
-	<channel-type id="lastcontacttime" advanced="true">
+	<channel-type id="lastcontact" advanced="true">
 		<item-type>DateTime</item-type>
 		<label>Last Contact Time</label>
 		<description>Last contact time of a controller</description>