[gce] Avoid conflicts with things file defined (#13526)

* Avoiding dynamic creation of channels when channels already exists

Signed-off-by: clinique <gael@lhopital.org>

diff --git a/bundles/org.openhab.binding.gce/src/main/java/org/openhab/binding/gce/internal/GCEHandlerFactory.java b/bundles/org.openhab.binding.gce/src/main/java/org/openhab/binding/gce/internal/GCEHandlerFactory.java
index 81fc987c9cf371004f3d3d8d8879a27ae054b8a8..fa363ca13bbdd21be3bbe86aedcdde84ad7eafd6 100644 (file)
--- a/bundles/org.openhab.binding.gce/src/main/java/org/openhab/binding/gce/internal/GCEHandlerFactory.java
+++ b/bundles/org.openhab.binding.gce/src/main/java/org/openhab/binding/gce/internal/GCEHandlerFactory.java
@@ -14,7 +14,6 @@ package org.openhab.binding.gce.internal;
 
 import static org.openhab.binding.gce.internal.GCEBindingConstants.IPXV3_THING_TYPE;
 
-import java.util.Collections;
 import java.util.Set;
 
 import org.eclipse.jdt.annotation.NonNullByDefault;
@@ -36,7 +35,7 @@ import org.osgi.service.component.annotations.Component;
 @NonNullByDefault
 @Component(service = ThingHandlerFactory.class, configurationPid = "binding.gce")
 public class GCEHandlerFactory extends BaseThingHandlerFactory {
-    private static final Set<ThingTypeUID> SUPPORTED_THING_TYPES_UIDS = Collections.singleton(IPXV3_THING_TYPE);
+    private static final Set<ThingTypeUID> SUPPORTED_THING_TYPES_UIDS = Set.of(IPXV3_THING_TYPE);
 
     @Override
     public boolean supportsThingType(ThingTypeUID thingTypeUID) {

diff --git a/bundles/org.openhab.binding.gce/src/main/java/org/openhab/binding/gce/internal/handler/Ipx800DeviceConnector.java b/bundles/org.openhab.binding.gce/src/main/java/org/openhab/binding/gce/internal/handler/Ipx800DeviceConnector.java
index 67da7f26321333269c9c90995c3bf319c8bcf740..0673230b7f41151b19794befbddc1d363a28d5be 100644 (file)
--- a/bundles/org.openhab.binding.gce/src/main/java/org/openhab/binding/gce/internal/handler/Ipx800DeviceConnector.java
+++ b/bundles/org.openhab.binding.gce/src/main/java/org/openhab/binding/gce/internal/handler/Ipx800DeviceConnector.java
@@ -42,7 +42,7 @@ public class Ipx800DeviceConnector extends Thread {
     private static final String ENDL = "\r\n";
 
     private final String hostname;
-    public final int portNumber;
+    private final int portNumber;
     private @Nullable M2MMessageParser parser;
 
     private @NonNullByDefault({}) Socket client;

diff --git a/bundles/org.openhab.binding.gce/src/main/java/org/openhab/binding/gce/internal/handler/Ipx800v3Handler.java b/bundles/org.openhab.binding.gce/src/main/java/org/openhab/binding/gce/internal/handler/Ipx800v3Handler.java
index 57ac972e05585e1e7002100847626f73e2a001ca..04962adf243ff7ad3e60543d3ded1bc3d490c157 100644 (file)
--- a/bundles/org.openhab.binding.gce/src/main/java/org/openhab/binding/gce/internal/handler/Ipx800v3Handler.java
+++ b/bundles/org.openhab.binding.gce/src/main/java/org/openhab/binding/gce/internal/handler/Ipx800v3Handler.java
@@ -24,6 +24,7 @@ import java.util.Collections;
 import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
+import java.util.Optional;
 import java.util.concurrent.ScheduledFuture;
 import java.util.concurrent.TimeUnit;
 
@@ -79,11 +80,12 @@ public class Ipx800v3Handler extends BaseThingHandler implements Ipx800EventList
 
     private @NonNullByDefault({}) Ipx800Configuration configuration;
     private @NonNullByDefault({}) Ipx800DeviceConnector connector;
-    private @Nullable M2MMessageParser parser;
     private @NonNullByDefault({}) StatusFileInterpreter statusFile;
-    private @Nullable ScheduledFuture<?> refreshJob;
 
-    private final Map<String, PortData> portDatas = new HashMap<>();
+    private Optional<M2MMessageParser> parser = Optional.empty();
+    private Optional<ScheduledFuture<?>> refreshJob = Optional.empty();
+
+    private final Map<String, @Nullable PortData> portDatas = new HashMap<>();
 
     private class LongPressEvaluator implements Runnable {
         private final ZonedDateTime referenceTime;
@@ -97,10 +99,10 @@ public class Ipx800v3Handler extends BaseThingHandler implements Ipx800EventList
         }
 
         @Override
-        @SuppressWarnings("PMD.CompareObjectsWithEquals")
         public void run() {
             PortData currentData = portDatas.get(port);
-            if (currentData != null && currentData.getValue() == 1 && currentData.getTimestamp() == referenceTime) {
+            if (currentData != null && currentData.getValue() == 1
+                    && referenceTime.equals(currentData.getTimestamp())) {
                 triggerChannel(eventChannelId, EVENT_LONG_PRESS);
             }
         }
@@ -113,7 +115,6 @@ public class Ipx800v3Handler extends BaseThingHandler implements Ipx800EventList
 
     @Override
     public void initialize() {
-
         configuration = getConfigAs(Ipx800Configuration.class);
 
         logger.debug("Initializing IPX800 handler for uid '{}'", getThing().getUID());
@@ -124,55 +125,57 @@ public class Ipx800v3Handler extends BaseThingHandler implements Ipx800EventList
             discoverAttributes();
         }
 
+        List<Channel> channels = new ArrayList<>(getThing().getChannels());
+        ThingBuilder thingBuilder = editThing();
+        PortDefinition.asStream().forEach(portDefinition -> {
+            int nbElements = statusFile.getMaxNumberofNodeType(portDefinition);
+            for (int i = 0; i < nbElements; i++) {
+                createChannels(portDefinition, i, channels);
+            }
+        });
+        thingBuilder.withChannels(channels);
+        updateThing(thingBuilder.build());
+
         connector = new Ipx800DeviceConnector(configuration.hostname, configuration.portNumber, getThing().getUID());
-        parser = new M2MMessageParser(connector, this);
+        parser = Optional.of(new M2MMessageParser(connector, this));
 
         updateStatus(ThingStatus.UNKNOWN);
 
-        refreshJob = scheduler.scheduleWithFixedDelay(statusFile::read, 3000, configuration.pullInterval,
-                TimeUnit.MILLISECONDS);
+        refreshJob = Optional.of(scheduler.scheduleWithFixedDelay(statusFile::read, 3000, configuration.pullInterval,
+                TimeUnit.MILLISECONDS));
 
         connector.start();
     }
 
     @Override
     public void dispose() {
-        if (refreshJob != null) {
-            refreshJob.cancel(true);
-            refreshJob = null;
-        }
+        refreshJob.ifPresent(job -> job.cancel(true));
+        refreshJob = Optional.empty();
 
         if (connector != null) {
             connector.destroyAndExit();
         }
-        parser = null;
+        parser = Optional.empty();
 
         portDatas.values().stream().forEach(portData -> {
-            portData.destroy();
+            if (portData != null) {
+                portData.dispose();
+            }
         });
         super.dispose();
     }
 
     protected void discoverAttributes() {
-        final Map<String, String> properties = new HashMap<>();
-
-        properties.put(Thing.PROPERTY_VENDOR, "GCE Electronics");
-        properties.put(Thing.PROPERTY_FIRMWARE_VERSION, statusFile.getElement(StatusEntry.VERSION));
-        properties.put(Thing.PROPERTY_MAC_ADDRESS, statusFile.getElement(StatusEntry.CONFIG_MAC));
-        updateProperties(properties);
-
-        ThingBuilder thingBuilder = editThing();
-        List<Channel> channels = new ArrayList<>(getThing().getChannels());
-
-        PortDefinition.asStream().forEach(portDefinition -> {
-            int nbElements = statusFile.getMaxNumberofNodeType(portDefinition);
-            for (int i = 0; i < nbElements; i++) {
-                createChannels(portDefinition, i, channels);
-            }
-        });
+        updateProperties(Map.of(Thing.PROPERTY_VENDOR, "GCE Electronics", Thing.PROPERTY_FIRMWARE_VERSION,
+                statusFile.getElement(StatusEntry.VERSION), Thing.PROPERTY_MAC_ADDRESS,
+                statusFile.getElement(StatusEntry.CONFIG_MAC)));
+    }
 
-        thingBuilder.withChannels(channels);
-        updateThing(thingBuilder.build());
+    private void addIfChannelAbsent(ChannelBuilder channelBuilder, List<Channel> channels) {
+        Channel newChannel = channelBuilder.build();
+        if (channels.stream().noneMatch(c -> c.getUID().equals(newChannel.getUID()))) {
+            channels.add(newChannel);
+        }
     }
 
     private void createChannels(PortDefinition portDefinition, int portIndex, List<Channel> channels) {
@@ -184,32 +187,32 @@ public class Ipx800v3Handler extends BaseThingHandler implements Ipx800EventList
         ChannelTypeUID channelType = new ChannelTypeUID(BINDING_ID, advancedChannelTypeName);
         switch (portDefinition) {
             case ANALOG:
-                channels.add(ChannelBuilder.create(mainChannelUID, CoreItemFactory.NUMBER)
-                        .withLabel("Analog Input " + ndx).withType(channelType).build());
-                channels.add(ChannelBuilder
-                        .create(new ChannelUID(groupUID, ndx + "-voltage"), "Number:ElectricPotential")
-                        .withLabel("Voltage " + ndx).withType(new ChannelTypeUID(BINDING_ID, CHANNEL_VOLTAGE)).build());
+                addIfChannelAbsent(ChannelBuilder.create(mainChannelUID, CoreItemFactory.NUMBER)
+                        .withLabel("Analog Input " + ndx).withType(channelType), channels);
+                addIfChannelAbsent(
+                        ChannelBuilder.create(new ChannelUID(groupUID, ndx + "-voltage"), "Number:ElectricPotential")
+                                .withType(new ChannelTypeUID(BINDING_ID, CHANNEL_VOLTAGE)).withLabel("Voltage " + ndx),
+                        channels);
                 break;
             case CONTACT:
-                channels.add(ChannelBuilder.create(mainChannelUID, CoreItemFactory.CONTACT).withLabel("Contact " + ndx)
-                        .withType(channelType).build());
-                channels.add(ChannelBuilder.create(new ChannelUID(groupUID, ndx + "-event"), null)
-                        .withLabel("Contact " + ndx + " Event").withKind(ChannelKind.TRIGGER)
+                addIfChannelAbsent(ChannelBuilder.create(mainChannelUID, CoreItemFactory.CONTACT)
+                        .withLabel("Contact " + ndx).withType(channelType), channels);
+                addIfChannelAbsent(ChannelBuilder.create(new ChannelUID(groupUID, ndx + "-event"), null)
                         .withType(new ChannelTypeUID(BINDING_ID, TRIGGER_CONTACT + (portIndex < 8 ? "" : "Advanced")))
-                        .build());
+                        .withLabel("Contact " + ndx + " Event").withKind(ChannelKind.TRIGGER), channels);
                 break;
             case COUNTER:
-                channels.add(ChannelBuilder.create(mainChannelUID, CoreItemFactory.NUMBER).withLabel("Counter " + ndx)
-                        .withType(channelType).build());
+                addIfChannelAbsent(ChannelBuilder.create(mainChannelUID, CoreItemFactory.NUMBER)
+                        .withLabel("Counter " + ndx).withType(channelType), channels);
                 break;
             case RELAY:
-                channels.add(ChannelBuilder.create(mainChannelUID, CoreItemFactory.SWITCH).withLabel("Relay " + ndx)
-                        .withType(channelType).build());
+                addIfChannelAbsent(ChannelBuilder.create(mainChannelUID, CoreItemFactory.SWITCH)
+                        .withLabel("Relay " + ndx).withType(channelType), channels);
                 break;
         }
-        channels.add(ChannelBuilder.create(new ChannelUID(groupUID, ndx + "-duration"), "Number:Time")
-                .withLabel("Previous state duration " + ndx)
-                .withType(new ChannelTypeUID(BINDING_ID, CHANNEL_LAST_STATE_DURATION)).build());
+        addIfChannelAbsent(ChannelBuilder.create(new ChannelUID(groupUID, ndx + "-duration"), "Number:Time")
+                .withType(new ChannelTypeUID(BINDING_ID, CHANNEL_LAST_STATE_DURATION))
+                .withLabel("Previous state duration " + ndx), channels);
     }
 
     @Override
@@ -227,16 +230,11 @@ public class Ipx800v3Handler extends BaseThingHandler implements Ipx800EventList
             if (portDefinition == PortDefinition.ANALOG) { // For analog values, check histeresis
                 AnalogInputConfiguration config = configuration.as(AnalogInputConfiguration.class);
                 long hysteresis = config.hysteresis / 2;
-                if (newValue <= prevValue + hysteresis && newValue >= prevValue - hysteresis) {
-                    return true;
-                }
-            }
-            if (portDefinition == PortDefinition.CONTACT) { // For contact values, check debounce
+                return (newValue <= prevValue + hysteresis && newValue >= prevValue - hysteresis);
+            } else if (portDefinition == PortDefinition.CONTACT) { // For contact values, check debounce
                 DigitalInputConfiguration config = configuration.as(DigitalInputConfiguration.class);
-                if (config.debouncePeriod != 0
-                        && now.isBefore(portData.getTimestamp().plus(config.debouncePeriod, ChronoUnit.MILLIS))) {
-                    return true;
-                }
+                return (config.debouncePeriod != 0
+                        && now.isBefore(portData.getTimestamp().plus(config.debouncePeriod, ChronoUnit.MILLIS)));
             }
         }
         return false;
@@ -338,9 +336,7 @@ public class Ipx800v3Handler extends BaseThingHandler implements Ipx800EventList
                 && PortDefinition.fromGroupId(groupId) == PortDefinition.RELAY) {
             RelayOutputConfiguration config = channel.getConfiguration().as(RelayOutputConfiguration.class);
             String id = channelUID.getIdWithoutGroup();
-            if (parser != null) {
-                parser.setOutput(id, (OnOffType) command == OnOffType.ON ? 1 : 0, config.pulse);
-            }
+            parser.ifPresent(p -> p.setOutput(id, (OnOffType) command == OnOffType.ON ? 1 : 0, config.pulse));
             return;
         }
         logger.debug("Can not handle command '{}' on channel '{}'", command, channelUID);
@@ -368,20 +364,16 @@ public class Ipx800v3Handler extends BaseThingHandler implements Ipx800EventList
         super.channelUnlinked(channelUID);
         PortData portData = portDatas.remove(channelUID.getId());
         if (portData != null) {
-            portData.destroy();
+            portData.dispose();
         }
     }
 
     public void resetCounter(int counter) {
-        if (parser != null) {
-            parser.resetCounter(counter);
-        }
+        parser.ifPresent(p -> p.resetCounter(counter));
     }
 
     public void reset() {
-        if (parser != null) {
-            parser.resetPLC();
-        }
+        parser.ifPresent(M2MMessageParser::resetPLC);
     }
 
     @Override

diff --git a/bundles/org.openhab.binding.gce/src/main/java/org/openhab/binding/gce/internal/model/PortData.java b/bundles/org.openhab.binding.gce/src/main/java/org/openhab/binding/gce/internal/model/PortData.java
index fbe0f234099e1796dbb36fb4d1b4b854dc9dd144..925fd760e5fb3b653f176ecddc4da390e07772cb 100644 (file)
--- a/bundles/org.openhab.binding.gce/src/main/java/org/openhab/binding/gce/internal/model/PortData.java
+++ b/bundles/org.openhab.binding.gce/src/main/java/org/openhab/binding/gce/internal/model/PortData.java
@@ -13,10 +13,10 @@
 package org.openhab.binding.gce.internal.model;
 
 import java.time.ZonedDateTime;
+import java.util.Optional;
 import java.util.concurrent.ScheduledFuture;
 
 import org.eclipse.jdt.annotation.NonNullByDefault;
-import org.eclipse.jdt.annotation.Nullable;
 
 /**
  * The {@link PortData} is responsible for holding data regarding current status of a port.
@@ -27,16 +27,14 @@ import org.eclipse.jdt.annotation.Nullable;
 public class PortData {
     private double value = -1;
     private ZonedDateTime timestamp = ZonedDateTime.now();
-    private @Nullable ScheduledFuture<?> pulsing;
+    private Optional<ScheduledFuture<?>> pulsing = Optional.empty();
 
     public void cancelPulsing() {
-        if (pulsing != null) {
-            pulsing.cancel(true);
-        }
-        pulsing = null;
+        pulsing.ifPresent(pulse -> pulse.cancel(true));
+        pulsing = Optional.empty();
     }
 
-    public void destroy() {
+    public void dispose() {
         cancelPulsing();
     }
 
@@ -54,7 +52,7 @@ public class PortData {
     }
 
     public void setPulsing(ScheduledFuture<?> pulsing) {
-        this.pulsing = pulsing;
+        this.pulsing = Optional.of(pulsing);
     }
 
     public boolean isInitializing() {