Some more code refactoring

Signed-off-by: Gaël L'hopital <gael@lhopital.org>

bundles/org.openhab.binding.gce/src/main/java/org/openhab/binding/gce/internal/handler/Ipx800DeviceConnector.java

@@ -41,11 +41,10 @@ public class Ipx800DeviceConnector extends Thread {
     private static final String ENDL = "\r\n";
 
     private final Logger logger = LoggerFactory.getLogger(Ipx800DeviceConnector.class);
-
     private final String hostname;
     private final int portNumber;
+    private final M2MMessageParser messageParser;
 
-    private Optional<M2MMessageParser> messageParser = Optional.empty();
     private Optional<Socket> socket = Optional.empty();
     private Optional<BufferedReader> input = Optional.empty();
     private Optional<PrintWriter> output = Optional.empty();
@@ -53,10 +52,11 @@ public class Ipx800DeviceConnector extends Thread {
     private int failedKeepalive = 0;
     private boolean waitingKeepaliveResponse = false;
 
-    public Ipx800DeviceConnector(String hostname, int portNumber, ThingUID uid) {
+    public Ipx800DeviceConnector(String hostname, int portNumber, ThingUID uid, Ipx800EventListener listener) {
         super("OH-binding-" + uid);
         this.hostname = hostname;
         this.portNumber = portNumber;
+        this.messageParser = new M2MMessageParser(this, listener);
         setDaemon(true);
     }
 
@@ -120,7 +120,6 @@ public class Ipx800DeviceConnector extends Thread {
     public void dispose() {
         interrupt();
         disconnect();
-        releaseParser();
     }
 
     /**
@@ -156,7 +155,7 @@ public class Ipx800DeviceConnector extends Thread {
                     try {
                         String command = in.readLine();
                         waitingKeepaliveResponse = false;
-                        messageParser.ifPresent(parser -> parser.unsolicitedUpdate(command));
+                        messageParser.unsolicitedUpdate(command);
                     } catch (IOException e) {
                         handleException(e);
                     }
@@ -181,15 +180,11 @@ public class Ipx800DeviceConnector extends Thread {
             } else if (e instanceof IOException) {
                 logger.warn("Communication error: '{}'. Will retry in {} ms", e, DEFAULT_RECONNECT_TIMEOUT_MS);
             }
-            messageParser.ifPresent(parser -> parser.errorOccurred(e));
+            messageParser.errorOccurred(e);
         }
     }
 
-    public void setParser(M2MMessageParser parser) {
+    public M2MMessageParser getParser() {
-        messageParser = Optional.of(parser);
+        return messageParser;
-    }
-
-    public void releaseParser() {
-        messageParser = Optional.empty();
     }
 }

bundles/org.openhab.binding.gce/src/main/java/org/openhab/binding/gce/internal/handler/Ipx800v3Handler.java

@@ -14,6 +14,7 @@ package org.openhab.binding.gce.internal.handler;
 
 import static org.openhab.binding.gce.internal.GCEBindingConstants.*;
 
+import java.io.IOException;
 import java.time.Duration;
 import java.time.Instant;
 import java.time.temporal.ChronoUnit;
@@ -27,16 +28,16 @@ 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.gce.internal.action.Ipx800Actions;
 import org.openhab.binding.gce.internal.config.AnalogInputConfiguration;
 import org.openhab.binding.gce.internal.config.DigitalInputConfiguration;
 import org.openhab.binding.gce.internal.config.Ipx800Configuration;
 import org.openhab.binding.gce.internal.config.RelayOutputConfiguration;
-import org.openhab.binding.gce.internal.model.M2MMessageParser;
 import org.openhab.binding.gce.internal.model.PortData;
 import org.openhab.binding.gce.internal.model.PortDefinition;
-import org.openhab.binding.gce.internal.model.StatusFileInterpreter;
+import org.openhab.binding.gce.internal.model.StatusFile;
-import org.openhab.binding.gce.internal.model.StatusFileInterpreter.StatusEntry;
+import org.openhab.binding.gce.internal.model.StatusFileAccessor;
 import org.openhab.core.config.core.Configuration;
 import org.openhab.core.library.CoreItemFactory;
 import org.openhab.core.library.types.DecimalType;
@@ -60,6 +61,8 @@ import org.openhab.core.types.State;
 import org.openhab.core.types.UnDefType;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
+import org.w3c.dom.Node;
+import org.xml.sax.SAXException;
 
 /**
  * The {@link Ipx800v3Handler} is responsible for handling commands, which are
@@ -75,8 +78,8 @@ public class Ipx800v3Handler extends BaseThingHandler implements Ipx800EventList
     private final Logger logger = LoggerFactory.getLogger(Ipx800v3Handler.class);
 
     private Optional<Ipx800DeviceConnector> connector = Optional.empty();
-    private Optional<M2MMessageParser> parser = Optional.empty();
     private Optional<ScheduledFuture<?>> refreshJob = Optional.empty();
+    private Optional<StatusFileAccessor> statusConnector = Optional.empty();
 
     private final Map<String, PortData> portDatas = new HashMap<>();
 
@@ -88,7 +91,7 @@ public class Ipx800v3Handler extends BaseThingHandler implements Ipx800EventList
         public LongPressEvaluator(Channel channel, String port, PortData portData) {
             this.referenceTime = portData.getTimestamp();
             this.port = port;
-            this.eventChannelId = channel.getUID().getId() + PROPERTY_SEPARATOR + TRIGGER_CONTACT;
+            this.eventChannelId = "%s-%s".formatted(channel.getUID().getId(), TRIGGER_CONTACT);
         }
 
         @Override
@@ -103,7 +106,6 @@ public class Ipx800v3Handler extends BaseThingHandler implements Ipx800EventList
 
     public Ipx800v3Handler(Thing thing) {
         super(thing);
-        logger.debug("Create an IPX800 Handler for thing '{}'", getThing().getUID());
     }
 
     @Override
@@ -111,34 +113,61 @@ public class Ipx800v3Handler extends BaseThingHandler implements Ipx800EventList
         logger.debug("Initializing IPX800 handler for uid '{}'", getThing().getUID());
 
         Ipx800Configuration config = getConfigAs(Ipx800Configuration.class);
-        StatusFileInterpreter statusFile = new StatusFileInterpreter(config.hostname, this);
 
-        if (thing.getProperties().isEmpty()) {
+        statusConnector = Optional.of(new StatusFileAccessor(config.hostname));
-            updateProperties(Map.of(Thing.PROPERTY_VENDOR, "GCE Electronics", Thing.PROPERTY_FIRMWARE_VERSION,
+        connector = Optional
-                    statusFile.getElement(StatusEntry.VERSION), Thing.PROPERTY_MAC_ADDRESS,
+                .of(new Ipx800DeviceConnector(config.hostname, config.portNumber, getThing().getUID(), this));
-                    statusFile.getElement(StatusEntry.CONFIG_MAC)));
+
+        updateStatus(ThingStatus.UNKNOWN);
+
+        refreshJob = Optional.of(scheduler.scheduleWithFixedDelay(this::readStatusFile, 1500, config.pullInterval,
+                TimeUnit.MILLISECONDS));
+    }
+
+    private void readStatusFile() {
+        StatusFile status = null;
+        try {
+            status = statusConnector.get().read();
+            for (PortDefinition portDefinition : PortDefinition.values()) {
+                List<Node> nodes = status.getMatchingNodes(portDefinition.nodeName);
+                nodes.forEach(node -> {
+                    String sPortNum = node.getNodeName().replace(portDefinition.nodeName, "");
+                    int portNum = Integer.parseInt(sPortNum) + 1;
+                    double value = Double.parseDouble(node.getTextContent().replace("dn", "1").replace("up", "0"));
+                    dataReceived("%s%d".formatted(portDefinition.portName, portNum), value);
+                });
+            }
+        } catch (SAXException | IOException e) {
+            logger.warn("Unable to read status file for {}", thing.getUID());
         }
 
+        if (Thread.State.NEW.equals(connector.get().getState())) {
+            setProperties(status);
+            updateChannels(status);
+            connector.get().start();
+        }
+    }
+
+    private void updateChannels(@Nullable StatusFile status) {
         List<Channel> channels = new ArrayList<>(getThing().getChannels());
         PortDefinition.AS_STREAM.forEach(portDefinition -> {
-            int nbElements = statusFile.getMaxNumberofNodeType(portDefinition);
+            int nbElements = status != null ? status.getMaxNumberofNodeType(portDefinition) : portDefinition.quantity;
             for (int i = 0; i < nbElements; i++) {
                 ChannelUID portChannelUID = createChannels(portDefinition, i, channels);
                 portDatas.put(portChannelUID.getId(), new PortData());
             }
         });
-
         updateThing(editThing().withChannels(channels).build());
+    }
 
-        connector = Optional.of(new Ipx800DeviceConnector(config.hostname, config.portNumber, getThing().getUID()));
+    private void setProperties(@Nullable StatusFile status) {
-        parser = Optional.of(new M2MMessageParser(connector.get(), this));
+        Map<String, String> properties = thing.getProperties();
-
+        properties.put(Thing.PROPERTY_VENDOR, "GCE Electronics");
-        updateStatus(ThingStatus.UNKNOWN);
+        if (status != null) {
-
+            properties.put(Thing.PROPERTY_FIRMWARE_VERSION, status.getVersion());
-        refreshJob = Optional.of(
+            properties.put(Thing.PROPERTY_MAC_ADDRESS, status.getMac());
-                scheduler.scheduleWithFixedDelay(statusFile::read, 3000, config.pullInterval, TimeUnit.MILLISECONDS));
+        }
-
+        updateProperties(properties);
-        connector.get().start();
     }
 
     @Override
@@ -149,12 +178,11 @@ public class Ipx800v3Handler extends BaseThingHandler implements Ipx800EventList
         connector.ifPresent(Ipx800DeviceConnector::dispose);
         connector = Optional.empty();
 
-        parser.ifPresent(M2MMessageParser::dispose);
-        parser = Optional.empty();
-
         portDatas.values().stream().forEach(PortData::dispose);
         portDatas.clear();
 
+        statusConnector = Optional.empty();
+
         super.dispose();
     }
 
@@ -332,7 +360,7 @@ public class Ipx800v3Handler extends BaseThingHandler implements Ipx800EventList
                 && PortDefinition.fromGroupId(groupId) == PortDefinition.RELAY) {
             RelayOutputConfiguration config = channel.getConfiguration().as(RelayOutputConfiguration.class);
             String id = channelUID.getIdWithoutGroup();
-            parser.ifPresent(p -> p.setOutput(id, onOffCommand == OnOffType.ON ? 1 : 0, config.pulse));
+            connector.ifPresent(p -> p.getParser().setOutput(id, onOffCommand == OnOffType.ON ? 1 : 0, config.pulse));
             return;
         }
         logger.debug("Can not handle command '{}' on channel '{}'", command, channelUID);
@@ -343,11 +371,11 @@ public class Ipx800v3Handler extends BaseThingHandler implements Ipx800EventList
     }
 
     public void resetCounter(int counter) {
-        parser.ifPresent(p -> p.resetCounter(counter));
+        connector.ifPresent(p -> p.getParser().resetCounter(counter));
     }
 
     public void reset() {
-        parser.ifPresent(M2MMessageParser::resetPLC);
+        connector.ifPresent(p -> p.getParser().resetPLC());
     }
 
     @Override

bundles/org.openhab.binding.gce/src/main/java/org/openhab/binding/gce/internal/model/M2MMessageParser.java

@@ -41,11 +41,6 @@ public class M2MMessageParser {
     public M2MMessageParser(Ipx800DeviceConnector connector, Ipx800EventListener listener) {
         this.connector = connector;
         this.listener = listener;
-        connector.setParser(this);
-    }
-
-    public void dispose() {
-        connector.releaseParser();
     }
 
     /**
@@ -110,7 +105,7 @@ public class M2MMessageParser {
      */
     public void setOutput(String targetPort, int targetValue, boolean pulse) {
         logger.debug("Sending {} to {}", targetValue, targetPort);
-        String command = String.format("Set%02d%s%s", Integer.parseInt(targetPort), targetValue, pulse ? "p" : "");
+        String command = "Set%02d%s%s".formatted(Integer.parseInt(targetPort), targetValue, pulse ? "p" : "");
         connector.send(command);
     }
 
@@ -121,7 +116,7 @@ public class M2MMessageParser {
      */
     public void resetCounter(int targetCounter) {
         logger.debug("Resetting counter {} to 0", targetCounter);
-        connector.send(String.format("ResetCount%d", targetCounter));
+        connector.send("ResetCount%d".formatted(targetCounter));
     }
 
     public void errorOccurred(Exception e) {

bundles/org.openhab.binding.gce/src/main/java/org/openhab/binding/gce/internal/model/PortData.java

@@ -52,6 +52,7 @@ public class PortData {
     }
 
     public void setPulsing(ScheduledFuture<?> pulsing) {
+        cancelPulsing();
         this.pulsing = Optional.of(pulsing);
     }
 

bundles/org.openhab.binding.gce/src/main/java/org/openhab/binding/gce/internal/model/PortDefinition.java

@@ -32,7 +32,7 @@ public enum PortDefinition {
     public final String nodeName; // Name used in the status xml file
     public final String portName; // Name used by the M2M protocol
     public final String m2mCommand; // associated M2M command
-    private final int quantity; // base number of ports
+    public final int quantity; // base number of ports
 
     PortDefinition(String nodeName, String portName, String m2mCommand, int quantity) {
         this.nodeName = nodeName;

bundles/org.openhab.binding.gce/src/main/java/org/openhab/binding/gce/internal/model/StatusFile.java

@@ -0,0 +1,58 @@
+/**
+ * Copyright (c) 2010-2024 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.gce.internal.model;
+
+import java.util.Comparator;
+import java.util.List;
+import java.util.stream.IntStream;
+
+import org.eclipse.jdt.annotation.NonNullByDefault;
+import org.w3c.dom.Document;
+import org.w3c.dom.Element;
+import org.w3c.dom.Node;
+import org.w3c.dom.NodeList;
+
+/**
+ * This class takes care of interpreting the status.xml file
+ *
+ * @author Gaël L'hopital - Initial contribution
+ */
+@NonNullByDefault
+public class StatusFile {
+
+    private final Element root;
+
+    public StatusFile(Document doc) {
+        this.root = doc.getDocumentElement();
+        root.normalize();
+    }
+
+    public String getMac() {
+        return root.getElementsByTagName("config_mac").item(0).getTextContent();
+    }
+
+    public String getVersion() {
+        return root.getElementsByTagName("version").item(0).getTextContent();
+    }
+
+    public List<Node> getMatchingNodes(String criteria) {
+        NodeList nodeList = root.getChildNodes();
+        return IntStream.range(0, nodeList.getLength()).boxed().map(nodeList::item)
+                .filter(node -> node.getNodeName().startsWith(criteria)).sorted(Comparator.comparing(Node::getNodeName))
+                .toList();
+    }
+
+    public int getMaxNumberofNodeType(PortDefinition portDefinition) {
+        return getMatchingNodes(portDefinition.nodeName).size();
+    }
+}

bundles/org.openhab.binding.gce/src/main/java/org/openhab/binding/gce/internal/model/StatusFileAccessor.java

@@ -0,0 +1,56 @@
+/**
+ * Copyright (c) 2010-2024 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.gce.internal.model;
+
+import java.io.IOException;
+
+import javax.xml.parsers.DocumentBuilder;
+import javax.xml.parsers.DocumentBuilderFactory;
+import javax.xml.parsers.ParserConfigurationException;
+
+import org.eclipse.jdt.annotation.NonNullByDefault;
+import org.xml.sax.SAXException;
+
+/**
+ * This class takes care of interpreting the status.xml file
+ *
+ * @author Gaël L'hopital - Initial contribution
+ */
+@NonNullByDefault
+public class StatusFileAccessor {
+    private static final String URL_TEMPLATE = "http://%s/globalstatus.xml";
+
+    private final DocumentBuilder builder;
+    private final String url;
+
+    public StatusFileAccessor(String hostname) {
+        this.url = String.format(URL_TEMPLATE, hostname);
+        DocumentBuilderFactory factory = DocumentBuilderFactory.newInstance();
+        factory.setXIncludeAware(false);
+        factory.setExpandEntityReferences(false);
+        // see https://cheatsheetseries.owasp.org/cheatsheets/XML_External_Entity_Prevention_Cheat_Sheet.html
+        try {
+            factory.setFeature("http://xml.org/sax/features/external-general-entities", false);
+            factory.setFeature("http://xml.org/sax/features/external-parameter-entities", false);
+            factory.setFeature("http://apache.org/xml/features/nonvalidating/load-external-dtd", false);
+            builder = factory.newDocumentBuilder();
+        } catch (ParserConfigurationException e) {
+            throw new IllegalArgumentException("Error initializing StatusFileInterpreter", e);
+        }
+    }
+
+    public StatusFile read() throws SAXException, IOException {
+        StatusFile document = new StatusFile(builder.parse(url));
+        return document;
+    }
+}