[arcam] Added polling option and fixed a couple of bugs

Signed-off-by: Joep Admiraal <joep@groovytunes.nl>

bundles/org.openhab.binding.arcam/README.md

@@ -38,11 +38,17 @@ The most easy way of using this binding is via UPnP discovery.
 This will automatically configure the `hostname` for your device.
 You can manually configure this if you don't want to use the discovery mechanism.
 
+In theory the Arcam devices support pushing data to the binding whenever the values change.
+However during testing I found that this often does not happen.
+In order to work around this you can enable polling by setting the polling interval.
+A good starting value for this setting is 5 seconds.
+
 ### `sample` Thing Configuration
 
-| Name            | Type    | Description                           | Default | Required |
-|-----------------|---------|---------------------------------------|---------|----------|
-| hostname        | text    | Hostname or IP address of the device  | N/A     | yes      |
+| Name             | Type    | Description                           | Default | Required |
+|------------------|---------|---------------------------------------|---------|----------|
+| hostname         | text    | Hostname or IP address of the device  | N/A     | yes      |
+| Polling interval | integer | The amount of seconds to wait before polling the device for updated information. When set to 0 no polling is done and the binding relies on data being pushed from the device to the binding.  | 0       | yes      |
 
 ## Channels
 

bundles/org.openhab.binding.arcam/src/main/java/org/openhab/binding/arcam/internal/ArcamHandler.java

@@ -14,8 +14,13 @@
 
 import static org.openhab.binding.arcam.internal.ArcamBindingConstants.*;
 
+import java.util.concurrent.ScheduledFuture;
+import java.util.concurrent.TimeUnit;
+
 import org.eclipse.jdt.annotation.NonNullByDefault;
+import org.eclipse.jdt.annotation.Nullable;
 import org.openhab.binding.arcam.internal.config.ArcamConfiguration;
+import org.openhab.binding.arcam.internal.connection.ArcamCommandCode;
 import org.openhab.binding.arcam.internal.connection.ArcamConnection;
 import org.openhab.binding.arcam.internal.connection.ArcamConnectionListener;
 import org.openhab.binding.arcam.internal.devices.ArcamDevice;
@@ -36,6 +41,8 @@
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
+import io.micrometer.core.instrument.util.StringUtils;
+
 /**
  * The {@link ArcamHandler} is responsible for handling commands, which are
  * sent to one of the channels.
@@ -51,6 +58,9 @@ public class ArcamHandler extends BaseThingHandler implements ArcamStateChangedL
     private ArcamState state;
     private ArcamDevice device;
 
+    @Nullable
+    private ScheduledFuture<?> reqAllDataTask;
+
     public ArcamHandler(Thing thing) {
         super(thing);
 
@@ -199,25 +209,42 @@ public void initialize() {
 
         scheduler.execute(() -> {
             String hostname = config.hostname;
+            Integer pollingInterval = config.pollingInterval;
 
             try {
                 if (hostname == null) {
                     updateStatus(ThingStatus.OFFLINE, ThingStatusDetail.CONFIGURATION_ERROR, "No hostname specified");
                     return;
                 }
 
+                if (pollingInterval == null) {
+                    pollingInterval = 0;
+                }
+
                 connection.connect(hostname);
+
+                if (pollingInterval > 0) {
+                    reqAllDataTask = scheduler.scheduleWithFixedDelay(this::requestAllValues, pollingInterval,
+                            pollingInterval, TimeUnit.SECONDS);
+                }
+
             } catch (Exception e) {
                 updateStatus(ThingStatus.OFFLINE, ThingStatusDetail.COMMUNICATION_ERROR, e.getMessage());
                 return;
             }
 
-            logger.debug("handler initialized. ip: {}", config.hostname);
+            logger.debug("handler initialized. hostname: {}", config.hostname);
         });
     }
 
     @Override
     public void dispose() {
+        final ScheduledFuture<?> reqAllDataTask = this.reqAllDataTask;
+        if (reqAllDataTask != null) {
+            reqAllDataTask.cancel(true);
+            this.reqAllDataTask = null;
+        }
+
         connection.dispose();
 
         super.dispose();
@@ -246,5 +273,24 @@ private boolean equalsWithoutGroup(String channelStr, ChannelUID channelUID) {
     @Override
     public void onConnection() {
         updateStatus(ThingStatus.ONLINE);
+
+        requestAllValues();
+    }
+
+    public void requestAllValues() {
+        logger.debug("requestAllValues");
+
+        for (ArcamCommandCode commandCode : ArcamCommandCode.values()) {
+            // Only request the commandCodes for which a channel can exist
+            if (StringUtils.isEmpty(commandCode.channel)) {
+                continue;
+            }
+
+            if (!isLinked(commandCode.channel)) {
+                continue;
+            }
+
+            connection.requestState(commandCode.channel);
+        }
     }
 }

bundles/org.openhab.binding.arcam/src/main/java/org/openhab/binding/arcam/internal/ArcamState.java

@@ -17,10 +17,6 @@
 
 import org.eclipse.jdt.annotation.NonNullByDefault;
 import org.eclipse.jdt.annotation.Nullable;
-import org.openhab.core.library.types.DecimalType;
-import org.openhab.core.library.types.OnOffType;
-import org.openhab.core.library.types.PercentType;
-import org.openhab.core.library.types.StringType;
 import org.openhab.core.types.State;
 
 /**
@@ -39,27 +35,7 @@ public ArcamState(ArcamStateChangedListener handler) {
         this.handler = handler;
     }
 
-    public void setState(String channelId, @Nullable String value) {
-        StringType newValue = new StringType(value);
-        setState(channelId, newValue);
-    }
-
-    public void setState(String channelId, int value) {
-        DecimalType newValue = new DecimalType(value);
-        setState(channelId, newValue);
-    }
-
-    public void setPercentageState(String channelId, int value) {
-        PercentType newValue = new PercentType(value);
-        setState(channelId, newValue);
-    }
-
-    public void setState(String channelId, boolean value) {
-        OnOffType newValue = value ? OnOffType.ON : OnOffType.OFF;
-        setState(channelId, newValue);
-    }
-
-    private synchronized void setState(String channelId, State newValue) {
+    public synchronized void setState(String channelId, State newValue) {
         State oldValue = states.get(channelId);
         if (newValue.equals(oldValue)) {
             return;

bundles/org.openhab.binding.arcam/src/main/java/org/openhab/binding/arcam/internal/config/ArcamConfiguration.java

@@ -24,12 +24,14 @@
 public class ArcamConfiguration {
 
     public static final String HOSTNAME = "hostname";
-    public static final String SERIAL = "serial";
-    public static final String NAME = "name";
     public static final String MODEL = "model";
+    public static final String NAME = "name";
+    public static final String POLLING_INTERVAL = "pollingInterval";
+    public static final String SERIAL = "serial";
 
     public @Nullable String hostname;
-    public @Nullable String serial;
-    public @Nullable String name;
     public @Nullable String model;
+    public @Nullable String name;
+    public @Nullable Integer pollingInterval;
+    public @Nullable String serial;
 }

bundles/org.openhab.binding.arcam/src/main/java/org/openhab/binding/arcam/internal/connection/ArcamCommandFinder.java

@@ -28,14 +28,14 @@
 public class ArcamCommandFinder {
     private final Logger logger = LoggerFactory.getLogger(ArcamCommandFinder.class);
 
-    public byte[] getCommandFromCode(ArcamCommandCode code, List<ArcamCommand> list) {
+    public byte[] getCommandDataFromCode(ArcamCommandCode code, List<ArcamCommand> list) {
         for (ArcamCommand command : list) {
             if (command.code.equals(code)) {
                 return command.data;
             }
         }
 
-        logger.error("Could not find ArcamCommandfor code: {}.", code);
+        logger.trace("Could not find ArcamCommand data for code: {}.", code);
         return new byte[] {};
     }
 }

bundles/org.openhab.binding.arcam/src/main/java/org/openhab/binding/arcam/internal/connection/ArcamCommandInTransit.java

@@ -0,0 +1,65 @@
+/**
+ * Copyright (c) 2010-2022 Contributors to the openHAB project
+ *
+ * See the NOTICE file(s) distributed with this work for additional
+ * information.
+ *
+ * This program and the accompanying materials are made available under the
+ * terms of the Eclipse Public License 2.0 which is available at
+ * http://www.eclipse.org/legal/epl-2.0
+ *
+ * SPDX-License-Identifier: EPL-2.0
+ */
+package org.openhab.binding.arcam.internal.connection;
+
+import org.eclipse.jdt.annotation.NonNullByDefault;
+import org.eclipse.jdt.annotation.Nullable;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+/**
+ * This class allows thread safe access to a command that is currently being executed on the Arcam device.
+ *
+ * @author Joep Admiraal - Initial contribution
+ */
+@NonNullByDefault
+public class ArcamCommandInTransit {
+
+    private final Logger logger = LoggerFactory.getLogger(ArcamCommandInTransit.class);
+
+    @Nullable
+    private ArcamCommandCode commandInTransit;
+
+    public void waitFor() {
+        int counter = 0;
+        while (hasCommand()) {
+            counter++;
+            if (counter > 20) {
+                synchronized (this) {
+                    logger.debug("Stop waiting for a command to finish, command: {}", commandInTransit);
+                    commandInTransit = null;
+                }
+                return;
+            }
+
+            try {
+                Thread.sleep(10);
+            } catch (InterruptedException e) {
+            }
+        }
+    }
+
+    public synchronized void set(ArcamCommandCode commandCode) {
+        commandInTransit = commandCode;
+    }
+
+    public synchronized void finish(ArcamCommandCode commandCode) {
+        if (commandCode.equals(commandInTransit)) {
+            commandInTransit = null;
+        }
+    }
+
+    public synchronized boolean hasCommand() {
+        return commandInTransit != null;
+    }
+}