From 7a1daae0833ed32e961108838c7224f360a4daa3 Mon Sep 17 00:00:00 2001 From: clinique Date: Sat, 21 Dec 2024 15:20:10 +0100 Subject: [PATCH 01/11] Ensure ressources are freed. A bit of code revamp Signed-off-by: clinique --- .../handler/Ipx800DeviceConnector.java | 7 ++++- .../gce/internal/handler/Ipx800v3Handler.java | 14 +++++---- .../gce/internal/model/M2MMessageParser.java | 10 +++++-- .../binding/gce/internal/model/PortData.java | 8 ++--- .../gce/internal/model/PortDefinition.java | 30 +++++-------------- .../internal/model/StatusFileInterpreter.java | 18 +++++------ 6 files changed, 42 insertions(+), 45 deletions(-) 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 965347d440b..ceb623504e2 100644 --- 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 @@ -120,6 +120,7 @@ public class Ipx800DeviceConnector extends Thread { public void dispose() { interrupt(); disconnect(); + releaseParser(); } /** @@ -185,6 +186,10 @@ public class Ipx800DeviceConnector extends Thread { } public void setParser(M2MMessageParser parser) { - this.messageParser = Optional.of(parser); + messageParser = Optional.of(parser); + } + + public void releaseParser() { + messageParser = Optional.empty(); } } 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 d69902345f4..783555ca6b2 100644 --- 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 @@ -15,8 +15,7 @@ package org.openhab.binding.gce.internal.handler; import static org.openhab.binding.gce.internal.GCEBindingConstants.*; import java.time.Duration; -import java.time.ZoneId; -import java.time.ZonedDateTime; +import java.time.Instant; import java.time.temporal.ChronoUnit; import java.util.ArrayList; import java.util.Collection; @@ -82,7 +81,7 @@ public class Ipx800v3Handler extends BaseThingHandler implements Ipx800EventList private final Map portDatas = new HashMap<>(); private class LongPressEvaluator implements Runnable { - private final ZonedDateTime referenceTime; + private final Instant referenceTime; private final String port; private final String eventChannelId; @@ -121,7 +120,7 @@ public class Ipx800v3Handler extends BaseThingHandler implements Ipx800EventList } List channels = new ArrayList<>(getThing().getChannels()); - PortDefinition.asStream().forEach(portDefinition -> { + PortDefinition.AS_STREAM.forEach(portDefinition -> { int nbElements = statusFile.getMaxNumberofNodeType(portDefinition); for (int i = 0; i < nbElements; i++) { ChannelUID portChannelUID = createChannels(portDefinition, i, channels); @@ -150,9 +149,12 @@ 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(); + super.dispose(); } @@ -209,7 +211,7 @@ public class Ipx800v3Handler extends BaseThingHandler implements Ipx800EventList } private boolean ignoreCondition(double newValue, PortData portData, Configuration configuration, - PortDefinition portDefinition, ZonedDateTime now) { + PortDefinition portDefinition, Instant now) { if (!portData.isInitializing()) { // Always accept if portData is not initialized double prevValue = portData.getValue(); if (newValue == prevValue) { // Always reject if the value did not change @@ -237,7 +239,7 @@ public class Ipx800v3Handler extends BaseThingHandler implements Ipx800EventList String groupId = channel.getUID().getGroupId(); PortData portData = portDatas.get(channelId); if (portData != null && groupId != null) { - ZonedDateTime now = ZonedDateTime.now(ZoneId.systemDefault()); + Instant now = Instant.now(); long sinceLastChange = Duration.between(portData.getTimestamp(), now).toMillis(); Configuration configuration = channel.getConfiguration(); PortDefinition portDefinition = PortDefinition.fromGroupId(groupId); diff --git a/bundles/org.openhab.binding.gce/src/main/java/org/openhab/binding/gce/internal/model/M2MMessageParser.java b/bundles/org.openhab.binding.gce/src/main/java/org/openhab/binding/gce/internal/model/M2MMessageParser.java index 1b4d97d8106..b982136724e 100644 --- a/bundles/org.openhab.binding.gce/src/main/java/org/openhab/binding/gce/internal/model/M2MMessageParser.java +++ b/bundles/org.openhab.binding.gce/src/main/java/org/openhab/binding/gce/internal/model/M2MMessageParser.java @@ -44,6 +44,10 @@ public class M2MMessageParser { connector.setParser(this); } + public void dispose() { + connector.releaseParser(); + } + /** * * @param data @@ -67,7 +71,7 @@ public class M2MMessageParser { portNumShift = 0; // Align counters on 1 based array case ANALOG: { int portNumber = Integer.parseInt(statusPart[0].substring(1)) + portNumShift; - setStatus(portDefinition.getPortName() + portNumber, Double.parseDouble(statusPart[1])); + setStatus(portDefinition.portName + portNumber, Double.parseDouble(statusPart[1])); } } } @@ -80,7 +84,7 @@ public class M2MMessageParser { private void decodeDataLine(PortDefinition portDefinition, String data) { for (int count = 0; count < data.length(); count++) { - setStatus(portDefinition.getPortName() + (count + 1), (double) data.charAt(count) - '0'); + setStatus(portDefinition.portName + (count + 1), (double) data.charAt(count) - '0'); } } @@ -94,7 +98,7 @@ public class M2MMessageParser { this.expectedResponse = expectedResponse; } else { // GetAnx or GetCountx PortDefinition portType = PortDefinition.fromM2MCommand(expectedResponse); - this.expectedResponse = expectedResponse.replaceAll(portType.getM2mCommand(), portType.getPortName()); + this.expectedResponse = expectedResponse.replaceAll(portType.m2mCommand, portType.portName); } } 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 8e21165bdfe..d155e284c33 100644 --- 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 @@ -12,7 +12,7 @@ */ package org.openhab.binding.gce.internal.model; -import java.time.ZonedDateTime; +import java.time.Instant; import java.util.Optional; import java.util.concurrent.ScheduledFuture; @@ -26,7 +26,7 @@ import org.eclipse.jdt.annotation.NonNullByDefault; @NonNullByDefault public class PortData { private double value = -1; - private ZonedDateTime timestamp = ZonedDateTime.now(); + private Instant timestamp = Instant.now(); private Optional> pulsing = Optional.empty(); public void cancelPulsing() { @@ -38,7 +38,7 @@ public class PortData { cancelPulsing(); } - public void setData(double value, ZonedDateTime timestamp) { + public void setData(double value, Instant timestamp) { this.value = value; this.timestamp = timestamp; } @@ -47,7 +47,7 @@ public class PortData { return value; } - public ZonedDateTime getTimestamp() { + public Instant getTimestamp() { return timestamp; } diff --git a/bundles/org.openhab.binding.gce/src/main/java/org/openhab/binding/gce/internal/model/PortDefinition.java b/bundles/org.openhab.binding.gce/src/main/java/org/openhab/binding/gce/internal/model/PortDefinition.java index f6a2cba01be..26ea30be311 100644 --- a/bundles/org.openhab.binding.gce/src/main/java/org/openhab/binding/gce/internal/model/PortDefinition.java +++ b/bundles/org.openhab.binding.gce/src/main/java/org/openhab/binding/gce/internal/model/PortDefinition.java @@ -29,9 +29,9 @@ public enum PortDefinition { RELAY("led", "O", "GetOut", 8), CONTACT("btn", "I", "GetIn", 8); - private final String nodeName; // Name used in the status xml file - private final String portName; // Name used by the M2M protocol - private final String m2mCommand; // associated M2M command + 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 PortDefinition(String nodeName, String portName, String m2mCommand, int quantity) { @@ -41,13 +41,7 @@ public enum PortDefinition { this.quantity = quantity; } - public String getNodeName() { - return nodeName; - } - - public String getPortName() { - return portName; - } + public static final Stream AS_STREAM = Stream.of(PortDefinition.values()); @Override public String toString() { @@ -58,20 +52,12 @@ public enum PortDefinition { return id >= quantity; } - public String getM2mCommand() { - return m2mCommand; - } - - public static Stream asStream() { - return Stream.of(PortDefinition.values()); - } - public static PortDefinition fromM2MCommand(String m2mCommand) { - return asStream().filter(v -> m2mCommand.startsWith(v.m2mCommand)).findFirst().get(); + return AS_STREAM.filter(v -> m2mCommand.startsWith(v.m2mCommand)).findFirst().get(); } public static PortDefinition fromPortName(String portName) { - return asStream().filter(v -> portName.startsWith(v.portName)).findFirst().get(); + return AS_STREAM.filter(v -> portName.startsWith(v.portName)).findFirst().get(); } public static PortDefinition fromGroupId(String groupId) { @@ -80,7 +66,7 @@ public enum PortDefinition { public static String asChannelId(String portDefinition) { String portKind = portDefinition.substring(0, 1); - PortDefinition result = asStream().filter(v -> v.portName.startsWith(portKind)).findFirst().get(); - return result.toString() + "#" + portDefinition.substring(1); + PortDefinition result = AS_STREAM.filter(v -> v.portName.startsWith(portKind)).findFirst().get(); + return "%s#%s".formatted(result.toString(), portDefinition.substring(1)); } } diff --git a/bundles/org.openhab.binding.gce/src/main/java/org/openhab/binding/gce/internal/model/StatusFileInterpreter.java b/bundles/org.openhab.binding.gce/src/main/java/org/openhab/binding/gce/internal/model/StatusFileInterpreter.java index 652dc82bca7..99012270f63 100644 --- a/bundles/org.openhab.binding.gce/src/main/java/org/openhab/binding/gce/internal/model/StatusFileInterpreter.java +++ b/bundles/org.openhab.binding.gce/src/main/java/org/openhab/binding/gce/internal/model/StatusFileInterpreter.java @@ -19,9 +19,9 @@ import java.util.Comparator; import java.util.List; import java.util.Objects; import java.util.Optional; -import java.util.stream.Collectors; import java.util.stream.IntStream; +import javax.ws.rs.HttpMethod; import javax.xml.parsers.DocumentBuilder; import javax.xml.parsers.DocumentBuilderFactory; import javax.xml.parsers.ParserConfigurationException; @@ -78,7 +78,7 @@ public class StatusFileInterpreter { public void read() { try { - String statusPage = HttpUtil.executeUrl("GET", url, 5000); + String statusPage = HttpUtil.executeUrl(HttpMethod.GET, url, 5000); InputStream inputStream = new ByteArrayInputStream(statusPage.getBytes()); Document document = builder.parse(inputStream); document.getDocumentElement().normalize(); @@ -92,13 +92,13 @@ public class StatusFileInterpreter { private void pushDatas() { getRoot().ifPresent(root -> { - PortDefinition.asStream().forEach(portDefinition -> { - List xmlNodes = getMatchingNodes(root.getChildNodes(), portDefinition.getNodeName()); + PortDefinition.AS_STREAM.forEach(portDefinition -> { + List xmlNodes = getMatchingNodes(root.getChildNodes(), portDefinition.nodeName); xmlNodes.forEach(xmlNode -> { - String sPortNum = xmlNode.getNodeName().replace(portDefinition.getNodeName(), ""); + String sPortNum = xmlNode.getNodeName().replace(portDefinition.nodeName, ""); int portNum = Integer.parseInt(sPortNum) + 1; double value = Double.parseDouble(xmlNode.getTextContent().replace("dn", "1").replace("up", "0")); - listener.dataReceived(String.format("%s%d", portDefinition.getPortName(), portNum), value); + listener.dataReceived("%s%d".formatted(portDefinition.portName, portNum), value); }); }); }); @@ -113,12 +113,12 @@ public class StatusFileInterpreter { private List getMatchingNodes(NodeList nodeList, String criteria) { return IntStream.range(0, nodeList.getLength()).boxed().map(nodeList::item) .filter(node -> node.getNodeName().startsWith(criteria)).sorted(Comparator.comparing(Node::getNodeName)) - .collect(Collectors.toList()); + .toList(); } public int getMaxNumberofNodeType(PortDefinition portDefinition) { - return getRoot().map(root -> getMatchingNodes(root.getChildNodes(), portDefinition.getNodeName()).size()) - .orElse(0); + return Objects.requireNonNull(getRoot() + .map(root -> getMatchingNodes(root.getChildNodes(), portDefinition.nodeName).size()).orElse(0)); } private Optional getRoot() { From 365b84192a5b4ac50216b3bee10cb2cfa437403f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ga=C3=ABl=20L=27hopital?= Date: Tue, 24 Dec 2024 17:10:49 +0100 Subject: [PATCH 02/11] Some more code refactoring MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Gaël L'hopital --- .../handler/Ipx800DeviceConnector.java | 19 ++--- .../gce/internal/handler/Ipx800v3Handler.java | 84 ++++++++++++------- .../gce/internal/model/M2MMessageParser.java | 9 +- .../binding/gce/internal/model/PortData.java | 1 + .../gce/internal/model/PortDefinition.java | 2 +- .../gce/internal/model/StatusFile.java | 58 +++++++++++++ .../internal/model/StatusFileAccessor.java | 56 +++++++++++++ 7 files changed, 181 insertions(+), 48 deletions(-) create mode 100644 bundles/org.openhab.binding.gce/src/main/java/org/openhab/binding/gce/internal/model/StatusFile.java create mode 100644 bundles/org.openhab.binding.gce/src/main/java/org/openhab/binding/gce/internal/model/StatusFileAccessor.java 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 ceb623504e2..50d266eb750 100644 --- 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 @@ -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 messageParser = Optional.empty(); private Optional socket = Optional.empty(); private Optional input = Optional.empty(); private Optional 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) { - messageParser = Optional.of(parser); - } - - public void releaseParser() { - messageParser = Optional.empty(); + public M2MMessageParser getParser() { + return messageParser; } } 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 783555ca6b2..1e80f138541 100644 --- 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 @@ -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.StatusFileInterpreter.StatusEntry; +import org.openhab.binding.gce.internal.model.StatusFile; +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 connector = Optional.empty(); - private Optional parser = Optional.empty(); private Optional> refreshJob = Optional.empty(); + private Optional statusConnector = Optional.empty(); private final Map 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()) { - 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))); + statusConnector = Optional.of(new StatusFileAccessor(config.hostname)); + connector = Optional + .of(new Ipx800DeviceConnector(config.hostname, config.portNumber, getThing().getUID(), this)); + + 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 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 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())); - parser = Optional.of(new M2MMessageParser(connector.get(), this)); - - updateStatus(ThingStatus.UNKNOWN); - - refreshJob = Optional.of( - scheduler.scheduleWithFixedDelay(statusFile::read, 3000, config.pullInterval, TimeUnit.MILLISECONDS)); - - connector.get().start(); + private void setProperties(@Nullable StatusFile status) { + Map properties = thing.getProperties(); + properties.put(Thing.PROPERTY_VENDOR, "GCE Electronics"); + if (status != null) { + properties.put(Thing.PROPERTY_FIRMWARE_VERSION, status.getVersion()); + properties.put(Thing.PROPERTY_MAC_ADDRESS, status.getMac()); + } + updateProperties(properties); } @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 diff --git a/bundles/org.openhab.binding.gce/src/main/java/org/openhab/binding/gce/internal/model/M2MMessageParser.java b/bundles/org.openhab.binding.gce/src/main/java/org/openhab/binding/gce/internal/model/M2MMessageParser.java index b982136724e..cfb5ff1a904 100644 --- a/bundles/org.openhab.binding.gce/src/main/java/org/openhab/binding/gce/internal/model/M2MMessageParser.java +++ b/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) { 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 d155e284c33..cddafa7efb9 100644 --- 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 @@ -52,6 +52,7 @@ public class PortData { } public void setPulsing(ScheduledFuture pulsing) { + cancelPulsing(); this.pulsing = Optional.of(pulsing); } diff --git a/bundles/org.openhab.binding.gce/src/main/java/org/openhab/binding/gce/internal/model/PortDefinition.java b/bundles/org.openhab.binding.gce/src/main/java/org/openhab/binding/gce/internal/model/PortDefinition.java index 26ea30be311..a870e73f162 100644 --- a/bundles/org.openhab.binding.gce/src/main/java/org/openhab/binding/gce/internal/model/PortDefinition.java +++ b/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; diff --git a/bundles/org.openhab.binding.gce/src/main/java/org/openhab/binding/gce/internal/model/StatusFile.java b/bundles/org.openhab.binding.gce/src/main/java/org/openhab/binding/gce/internal/model/StatusFile.java new file mode 100644 index 00000000000..dca907e6861 --- /dev/null +++ b/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 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(); + } +} diff --git a/bundles/org.openhab.binding.gce/src/main/java/org/openhab/binding/gce/internal/model/StatusFileAccessor.java b/bundles/org.openhab.binding.gce/src/main/java/org/openhab/binding/gce/internal/model/StatusFileAccessor.java new file mode 100644 index 00000000000..16a6e15cb9a --- /dev/null +++ b/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; + } +} From 0989eb7d3c5ea3b12e31918b2bf7410c603da4d1 Mon Sep 17 00:00:00 2001 From: clinique Date: Thu, 26 Dec 2024 01:02:21 +0100 Subject: [PATCH 03/11] Debugging Signed-off-by: clinique --- .../handler/Ipx800DeviceConnector.java | 9 ++ .../gce/internal/handler/Ipx800v3Handler.java | 85 ++++++++++--------- .../gce/internal/model/PortDefinition.java | 10 +-- 3 files changed, 61 insertions(+), 43 deletions(-) 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 50d266eb750..e28b787073d 100644 --- 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 @@ -22,9 +22,12 @@ import java.util.Optional; import org.eclipse.jdt.annotation.NonNullByDefault; import org.openhab.binding.gce.internal.model.M2MMessageParser; +import org.openhab.binding.gce.internal.model.StatusFile; +import org.openhab.binding.gce.internal.model.StatusFileAccessor; import org.openhab.core.thing.ThingUID; import org.slf4j.Logger; import org.slf4j.LoggerFactory; +import org.xml.sax.SAXException; /** * The {@link Ipx800DeviceConnector} is responsible for connecting, @@ -44,6 +47,7 @@ public class Ipx800DeviceConnector extends Thread { private final String hostname; private final int portNumber; private final M2MMessageParser messageParser; + private final StatusFileAccessor statusAccessor; private Optional socket = Optional.empty(); private Optional input = Optional.empty(); @@ -57,6 +61,7 @@ public class Ipx800DeviceConnector extends Thread { this.hostname = hostname; this.portNumber = portNumber; this.messageParser = new M2MMessageParser(this, listener); + this.statusAccessor = new StatusFileAccessor(hostname); setDaemon(true); } @@ -187,4 +192,8 @@ public class Ipx800DeviceConnector extends Thread { public M2MMessageParser getParser() { return messageParser; } + + public StatusFile readStatusFile() throws SAXException, IOException { + return statusAccessor.read(); + } } 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 1e80f138541..8a95ee7b2a0 100644 --- 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 @@ -37,7 +37,6 @@ import org.openhab.binding.gce.internal.config.RelayOutputConfiguration; import org.openhab.binding.gce.internal.model.PortData; import org.openhab.binding.gce.internal.model.PortDefinition; import org.openhab.binding.gce.internal.model.StatusFile; -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; @@ -61,7 +60,6 @@ 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; /** @@ -76,13 +74,11 @@ public class Ipx800v3Handler extends BaseThingHandler implements Ipx800EventList private static final double ANALOG_SAMPLING = 0.000050354; private final Logger logger = LoggerFactory.getLogger(Ipx800v3Handler.class); - - private Optional connector = Optional.empty(); - private Optional> refreshJob = Optional.empty(); - private Optional statusConnector = Optional.empty(); - private final Map portDatas = new HashMap<>(); + private @Nullable Ipx800DeviceConnector deviceConnector; + private Optional> refreshJob = Optional.empty(); + private class LongPressEvaluator implements Runnable { private final Instant referenceTime; private final String port; @@ -114,9 +110,7 @@ public class Ipx800v3Handler extends BaseThingHandler implements Ipx800EventList Ipx800Configuration config = getConfigAs(Ipx800Configuration.class); - statusConnector = Optional.of(new StatusFileAccessor(config.hostname)); - connector = Optional - .of(new Ipx800DeviceConnector(config.hostname, config.portNumber, getThing().getUID(), this)); + deviceConnector = new Ipx800DeviceConnector(config.hostname, config.portNumber, getThing().getUID(), this); updateStatus(ThingStatus.UNKNOWN); @@ -125,32 +119,42 @@ public class Ipx800v3Handler extends BaseThingHandler implements Ipx800EventList } private void readStatusFile() { - StatusFile status = null; - try { - status = statusConnector.get().read(); - for (PortDefinition portDefinition : PortDefinition.values()) { - List 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); - }); + if (deviceConnector instanceof Ipx800DeviceConnector connector) { + StatusFile status = null; + try { + status = connector.readStatusFile(); + } catch (SAXException | IOException e) { + logger.warn("Unable to read status file for {}", thing.getUID()); + } + + if (Thread.State.NEW.equals(connector.getState())) { + setProperties(status); + updateChannels(status); + connector.start(); + } + + if (status != null) { + for (PortDefinition portDefinition : PortDefinition.values()) { + status.getMatchingNodes(portDefinition.nodeName).forEach(node -> { + String sPortNum = node.getNodeName().replace(portDefinition.nodeName, ""); + try { + 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 (NumberFormatException e) { + logger.warn(e.getMessage()); + } + }); + } } - } 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 channels = new ArrayList<>(getThing().getChannels()); - PortDefinition.AS_STREAM.forEach(portDefinition -> { + PortDefinition.AS_SET.forEach(portDefinition -> { int nbElements = status != null ? status.getMaxNumberofNodeType(portDefinition) : portDefinition.quantity; for (int i = 0; i < nbElements; i++) { ChannelUID portChannelUID = createChannels(portDefinition, i, channels); @@ -161,7 +165,7 @@ public class Ipx800v3Handler extends BaseThingHandler implements Ipx800EventList } private void setProperties(@Nullable StatusFile status) { - Map properties = thing.getProperties(); + Map properties = new HashMap<>(thing.getProperties()); properties.put(Thing.PROPERTY_VENDOR, "GCE Electronics"); if (status != null) { properties.put(Thing.PROPERTY_FIRMWARE_VERSION, status.getVersion()); @@ -175,14 +179,14 @@ public class Ipx800v3Handler extends BaseThingHandler implements Ipx800EventList refreshJob.ifPresent(job -> job.cancel(true)); refreshJob = Optional.empty(); - connector.ifPresent(Ipx800DeviceConnector::dispose); - connector = Optional.empty(); + if (deviceConnector instanceof Ipx800DeviceConnector connector) { + connector.dispose(); + connector = null; + } portDatas.values().stream().forEach(PortData::dispose); portDatas.clear(); - statusConnector = Optional.empty(); - super.dispose(); } @@ -357,10 +361,11 @@ public class Ipx800v3Handler extends BaseThingHandler implements Ipx800EventList return; } if (command instanceof OnOffType onOffCommand && isValidPortId(channelUID) - && PortDefinition.fromGroupId(groupId) == PortDefinition.RELAY) { + && PortDefinition.fromGroupId(groupId) == PortDefinition.RELAY + && deviceConnector instanceof Ipx800DeviceConnector connector) { RelayOutputConfiguration config = channel.getConfiguration().as(RelayOutputConfiguration.class); String id = channelUID.getIdWithoutGroup(); - connector.ifPresent(p -> p.getParser().setOutput(id, onOffCommand == OnOffType.ON ? 1 : 0, config.pulse)); + connector.getParser().setOutput(id, onOffCommand == OnOffType.ON ? 1 : 0, config.pulse); return; } logger.debug("Can not handle command '{}' on channel '{}'", command, channelUID); @@ -371,11 +376,15 @@ public class Ipx800v3Handler extends BaseThingHandler implements Ipx800EventList } public void resetCounter(int counter) { - connector.ifPresent(p -> p.getParser().resetCounter(counter)); + if (deviceConnector instanceof Ipx800DeviceConnector connector) { + connector.getParser().resetCounter(counter); + } } public void reset() { - connector.ifPresent(p -> p.getParser().resetPLC()); + if (deviceConnector instanceof Ipx800DeviceConnector connector) { + connector.getParser().resetPLC(); + } } @Override diff --git a/bundles/org.openhab.binding.gce/src/main/java/org/openhab/binding/gce/internal/model/PortDefinition.java b/bundles/org.openhab.binding.gce/src/main/java/org/openhab/binding/gce/internal/model/PortDefinition.java index a870e73f162..6872ed205ed 100644 --- a/bundles/org.openhab.binding.gce/src/main/java/org/openhab/binding/gce/internal/model/PortDefinition.java +++ b/bundles/org.openhab.binding.gce/src/main/java/org/openhab/binding/gce/internal/model/PortDefinition.java @@ -12,7 +12,7 @@ */ package org.openhab.binding.gce.internal.model; -import java.util.stream.Stream; +import java.util.EnumSet; import org.eclipse.jdt.annotation.NonNullByDefault; @@ -41,7 +41,7 @@ public enum PortDefinition { this.quantity = quantity; } - public static final Stream AS_STREAM = Stream.of(PortDefinition.values()); + public static final EnumSet AS_SET = EnumSet.allOf(PortDefinition.class); @Override public String toString() { @@ -53,11 +53,11 @@ public enum PortDefinition { } public static PortDefinition fromM2MCommand(String m2mCommand) { - return AS_STREAM.filter(v -> m2mCommand.startsWith(v.m2mCommand)).findFirst().get(); + return AS_SET.stream().filter(v -> m2mCommand.startsWith(v.m2mCommand)).findFirst().get(); } public static PortDefinition fromPortName(String portName) { - return AS_STREAM.filter(v -> portName.startsWith(v.portName)).findFirst().get(); + return AS_SET.stream().filter(v -> portName.startsWith(v.portName)).findFirst().get(); } public static PortDefinition fromGroupId(String groupId) { @@ -66,7 +66,7 @@ public enum PortDefinition { public static String asChannelId(String portDefinition) { String portKind = portDefinition.substring(0, 1); - PortDefinition result = AS_STREAM.filter(v -> v.portName.startsWith(portKind)).findFirst().get(); + PortDefinition result = AS_SET.stream().filter(v -> v.portName.equals(portKind)).findFirst().get(); return "%s#%s".formatted(result.toString(), portDefinition.substring(1)); } } From 5d578d1e563c774165be082b989003b8acc877a7 Mon Sep 17 00:00:00 2001 From: clinique Date: Fri, 27 Dec 2024 10:21:21 +0100 Subject: [PATCH 04/11] Review the whole binding Signed-off-by: clinique --- .../handler/Ipx800DeviceConnector.java | 105 +++++++----- .../gce/internal/handler/Ipx800v3Handler.java | 153 +++++++----------- .../gce/internal/model/M2MMessageParser.java | 57 ++----- .../binding/gce/internal/model/PortData.java | 30 +++- .../gce/internal/model/PortDefinition.java | 2 +- .../gce/internal/model/StatusFile.java | 33 ++-- .../internal/model/StatusFileAccessor.java | 9 +- 7 files changed, 186 insertions(+), 203 deletions(-) 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 e28b787073d..17661e0dfe7 100644 --- 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 @@ -17,11 +17,14 @@ import java.io.IOException; import java.io.InputStreamReader; import java.io.PrintWriter; import java.net.Socket; +import java.net.SocketException; import java.net.SocketTimeoutException; import java.util.Optional; +import java.util.Random; import org.eclipse.jdt.annotation.NonNullByDefault; import org.openhab.binding.gce.internal.model.M2MMessageParser; +import org.openhab.binding.gce.internal.model.PortDefinition; import org.openhab.binding.gce.internal.model.StatusFile; import org.openhab.binding.gce.internal.model.StatusFileAccessor; import org.openhab.core.thing.ThingUID; @@ -38,16 +41,18 @@ import org.xml.sax.SAXException; */ @NonNullByDefault public class Ipx800DeviceConnector extends Thread { - private static final int DEFAULT_SOCKET_TIMEOUT_MS = 5000; + private static final int DEFAULT_SOCKET_TIMEOUT_MS = 10000; private static final int DEFAULT_RECONNECT_TIMEOUT_MS = 5000; private static final int MAX_KEEPALIVE_FAILURE = 3; - private static final String ENDL = "\r\n"; private final Logger logger = LoggerFactory.getLogger(Ipx800DeviceConnector.class); + private final Random randomizer = new Random(); + private final String hostname; private final int portNumber; - private final M2MMessageParser messageParser; + private final M2MMessageParser parser; private final StatusFileAccessor statusAccessor; + private final Ipx800EventListener listener; private Optional socket = Optional.empty(); private Optional input = Optional.empty(); @@ -60,19 +65,12 @@ public class Ipx800DeviceConnector extends Thread { super("OH-binding-" + uid); this.hostname = hostname; this.portNumber = portNumber; - this.messageParser = new M2MMessageParser(this, listener); + this.listener = listener; + this.parser = new M2MMessageParser(listener); this.statusAccessor = new StatusFileAccessor(hostname); setDaemon(true); } - public synchronized void send(String message) { - output.ifPresentOrElse(out -> { - logger.debug("Sending '{}' to Ipx800", message); - out.write(message + ENDL); - out.flush(); - }, () -> logger.warn("Trying to send '{}' while the output stream is closed.", message)); - } - /** * Connect to the ipx800 * @@ -84,39 +82,27 @@ public class Ipx800DeviceConnector extends Thread { logger.debug("Connecting to {}:{}...", hostname, portNumber); Socket socket = new Socket(hostname, portNumber); socket.setSoTimeout(DEFAULT_SOCKET_TIMEOUT_MS); - socket.getInputStream().skip(socket.getInputStream().available()); + // socket.getInputStream().skip(socket.getInputStream().available()); this.socket = Optional.of(socket); - input = Optional.of(new BufferedReader(new InputStreamReader(socket.getInputStream()))); output = Optional.of(new PrintWriter(socket.getOutputStream(), true)); + input = Optional.of(new BufferedReader(new InputStreamReader(socket.getInputStream()))); } /** * Disconnect the device */ private void disconnect() { - logger.debug("Disconnecting"); - - input.ifPresent(in -> { - try { - in.close(); - } catch (IOException ignore) { - } - input = Optional.empty(); - }); - - output.ifPresent(PrintWriter::close); - output = Optional.empty(); - socket.ifPresent(client -> { try { + logger.debug("Closing socket"); client.close(); } catch (IOException ignore) { } socket = Optional.empty(); + input = Optional.empty(); + output = Optional.empty(); }); - - logger.debug("Disconnected"); } /** @@ -127,23 +113,35 @@ public class Ipx800DeviceConnector extends Thread { disconnect(); } + public synchronized void send(String message) { + output.ifPresentOrElse(out -> { + logger.debug("Sending '{}' to Ipx800", message); + out.println(message); + }, () -> logger.warn("Unable to send '{}' when the output stream is closed.", message)); + } + /** * Send an arbitrary keepalive command which cause the IPX to send an update. * If we don't receive the update maxKeepAliveFailure time, the connection is closed and reopened */ private void sendKeepalive() { - output.ifPresent(out -> { + output.ifPresentOrElse(out -> { + PortDefinition pd = PortDefinition.values()[randomizer.nextInt(PortDefinition.AS_SET.size())]; + String command = "%s%d".formatted(pd.m2mCommand, randomizer.nextInt(pd.quantity) + 1); + if (waitingKeepaliveResponse) { failedKeepalive++; - logger.debug("Sending keepalive, attempt {}", failedKeepalive); + logger.debug("Sending keepalive {}, attempt {}", command, failedKeepalive); } else { failedKeepalive = 0; - logger.debug("Sending keepalive"); + logger.debug("Sending keepalive {}", command); } - out.println("GetIn01"); - out.flush(); + + out.println(command); + parser.setExpectedResponse(command); + waitingKeepaliveResponse = true; - }); + }, () -> logger.warn("Unable to send keepAlive when the output stream is closed.")); } @Override @@ -160,7 +158,7 @@ public class Ipx800DeviceConnector extends Thread { try { String command = in.readLine(); waitingKeepaliveResponse = false; - messageParser.unsolicitedUpdate(command); + parser.unsolicitedUpdate(command); } catch (IOException e) { handleException(e); } @@ -182,18 +180,43 @@ public class Ipx800DeviceConnector extends Thread { if (e instanceof SocketTimeoutException) { sendKeepalive(); return; + } else if (e instanceof SocketException) { + logger.debug("SocketException raised by streams while closing socket"); } else if (e instanceof IOException) { logger.warn("Communication error: '{}'. Will retry in {} ms", e, DEFAULT_RECONNECT_TIMEOUT_MS); } - messageParser.errorOccurred(e); + listener.errorOccurred(e); } } - public M2MMessageParser getParser() { - return messageParser; - } - public StatusFile readStatusFile() throws SAXException, IOException { return statusAccessor.read(); } + + /** + * Set output of the device sending the corresponding command + * + * @param targetPort + * @param targetValue + */ + public void setOutput(String targetPort, int targetValue, boolean pulse) { + logger.debug("Sending {} to {}", targetValue, targetPort); + String command = "Set%02d%s%s".formatted(Integer.parseInt(targetPort), targetValue, pulse ? "p" : ""); + send(command); + } + + /** + * Resets the counter value to 0 + * + * @param targetCounter + */ + public void resetCounter(int targetCounter) { + logger.debug("Resetting counter {} to 0", targetCounter); + send("ResetCount%d".formatted(targetCounter)); + } + + public void resetPLC() { + send("Reset"); + } + } 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 8a95ee7b2a0..1529ea772dd 100644 --- 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 @@ -57,7 +57,6 @@ import org.openhab.core.thing.type.ChannelKind; import org.openhab.core.thing.type.ChannelTypeUID; import org.openhab.core.types.Command; import org.openhab.core.types.State; -import org.openhab.core.types.UnDefType; import org.slf4j.Logger; import org.slf4j.LoggerFactory; import org.xml.sax.SAXException; @@ -92,8 +91,7 @@ public class Ipx800v3Handler extends BaseThingHandler implements Ipx800EventList @Override public void run() { - PortData currentData = portDatas.get(port); - if (currentData != null && currentData.getValue() == 1 + if (portDatas.get(port) instanceof PortData currentData && currentData.getValue() == 1 && referenceTime.equals(currentData.getTimestamp())) { triggerChannel(eventChannelId, EVENT_LONG_PRESS); } @@ -133,29 +131,18 @@ public class Ipx800v3Handler extends BaseThingHandler implements Ipx800EventList connector.start(); } - if (status != null) { - for (PortDefinition portDefinition : PortDefinition.values()) { - status.getMatchingNodes(portDefinition.nodeName).forEach(node -> { - String sPortNum = node.getNodeName().replace(portDefinition.nodeName, ""); - try { - 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 (NumberFormatException e) { - logger.warn(e.getMessage()); - } - }); - } + if (status instanceof StatusFile statusFile) { + PortDefinition.AS_SET.forEach(portDefinition -> statusFile.getPorts(portDefinition).forEach( + (portNum, value) -> dataReceived("%s%d".formatted(portDefinition.portName, portNum), value))); } - } + } private void updateChannels(@Nullable StatusFile status) { List channels = new ArrayList<>(getThing().getChannels()); PortDefinition.AS_SET.forEach(portDefinition -> { - int nbElements = status != null ? status.getMaxNumberofNodeType(portDefinition) : portDefinition.quantity; + int nbElements = status != null ? status.getPorts(portDefinition).size() : portDefinition.quantity; for (int i = 0; i < nbElements; i++) { ChannelUID portChannelUID = createChannels(portDefinition, i, channels); portDatas.put(portChannelUID.getId(), new PortData()); @@ -181,7 +168,7 @@ public class Ipx800v3Handler extends BaseThingHandler implements Ipx800EventList if (deviceConnector instanceof Ipx800DeviceConnector connector) { connector.dispose(); - connector = null; + deviceConnector = null; } portDatas.values().stream().forEach(PortData::dispose); @@ -205,29 +192,25 @@ public class Ipx800v3Handler extends BaseThingHandler implements Ipx800EventList ChannelUID mainChannelUID = new ChannelUID(groupUID, ndx); ChannelTypeUID channelType = new ChannelTypeUID(BINDING_ID, advancedChannelTypeName); switch (portDefinition) { - case ANALOG: + case ANALOG -> { 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: + } + case CONTACT -> { 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"))) .withLabel("Contact " + ndx + " Event").withKind(ChannelKind.TRIGGER), channels); - break; - case COUNTER: - addIfChannelAbsent(ChannelBuilder.create(mainChannelUID, CoreItemFactory.NUMBER) - .withLabel("Counter " + ndx).withType(channelType), channels); - break; - case RELAY: - addIfChannelAbsent(ChannelBuilder.create(mainChannelUID, CoreItemFactory.SWITCH) - .withLabel("Relay " + ndx).withType(channelType), channels); - break; + } + case COUNTER -> addIfChannelAbsent(ChannelBuilder.create(mainChannelUID, CoreItemFactory.NUMBER) + .withLabel("Counter " + ndx).withType(channelType), channels); + case RELAY -> addIfChannelAbsent(ChannelBuilder.create(mainChannelUID, CoreItemFactory.SWITCH) + .withLabel("Relay " + ndx).withType(channelType), channels); } addIfChannelAbsent(ChannelBuilder.create(new ChannelUID(groupUID, ndx + "-duration"), "Number:Time") @@ -244,7 +227,7 @@ public class Ipx800v3Handler extends BaseThingHandler implements Ipx800EventList private boolean ignoreCondition(double newValue, PortData portData, Configuration configuration, PortDefinition portDefinition, Instant now) { - if (!portData.isInitializing()) { // Always accept if portData is not initialized + if (portData.isInitialized()) { // Always accept if portData is not initialized double prevValue = portData.getValue(); if (newValue == prevValue) { // Always reject if the value did not change return true; @@ -265,68 +248,57 @@ public class Ipx800v3Handler extends BaseThingHandler implements Ipx800EventList @Override public void dataReceived(String port, double value) { updateStatus(ThingStatus.ONLINE); - Channel channel = thing.getChannel(PortDefinition.asChannelId(port)); - if (channel != null) { + if (thing.getChannel(PortDefinition.asChannelId(port)) instanceof Channel channel) { String channelId = channel.getUID().getId(); - String groupId = channel.getUID().getGroupId(); - PortData portData = portDatas.get(channelId); - if (portData != null && groupId != null) { + + if (portDatas.get(channelId) instanceof PortData portData + && channel.getUID().getGroupId() instanceof String groupId) { Instant now = Instant.now(); - long sinceLastChange = Duration.between(portData.getTimestamp(), now).toMillis(); Configuration configuration = channel.getConfiguration(); PortDefinition portDefinition = PortDefinition.fromGroupId(groupId); if (ignoreCondition(value, portData, configuration, portDefinition, now)) { - logger.debug("Ignore condition met for port '{}' with data '{}'", port, value); + logger.trace("Ignore condition met for port '{}' with data '{}'", port, value); return; } logger.debug("About to update port '{}' with data '{}'", port, value); - State state = UnDefType.NULL; - switch (portDefinition) { - case COUNTER: - state = new DecimalType(value); - break; - case RELAY: - state = OnOffType.from(value == 1); - break; - case ANALOG: - state = new DecimalType(value); + long sinceLastChange = Duration.between(portData.getTimestamp(), now).toMillis(); + State state = switch (portDefinition) { + case COUNTER -> new DecimalType(value); + case RELAY -> OnOffType.from(value == 1); + case ANALOG -> { updateIfLinked(channelId + PROPERTY_SEPARATOR + CHANNEL_VOLTAGE, new QuantityType<>(value * ANALOG_SAMPLING, Units.VOLT)); - break; - case CONTACT: - DigitalInputConfiguration config = configuration.as(DigitalInputConfiguration.class); + yield new DecimalType(value); + } + case CONTACT -> { portData.cancelPulsing(); - state = value == 1 ? OpenClosedType.CLOSED : OpenClosedType.OPEN; - switch ((OpenClosedType) state) { - case CLOSED: - if (config.longPressTime != 0 && !portData.isInitializing()) { - scheduler.schedule(new LongPressEvaluator(channel, port, portData), - config.longPressTime, TimeUnit.MILLISECONDS); - } else if (config.pulsePeriod != 0) { - portData.setPulsing(scheduler.scheduleWithFixedDelay(() -> { - triggerPushButtonChannel(channel, EVENT_PULSE); - }, config.pulsePeriod, config.pulsePeriod, TimeUnit.MILLISECONDS)); - if (config.pulseTimeout != 0) { - scheduler.schedule(portData::cancelPulsing, config.pulseTimeout, - TimeUnit.MILLISECONDS); - } + DigitalInputConfiguration config = configuration.as(DigitalInputConfiguration.class); + + if (value == 1) { // CLOSED + if (config.longPressTime != 0 && portData.isInitialized()) { + scheduler.schedule(new LongPressEvaluator(channel, port, portData), + config.longPressTime, TimeUnit.MILLISECONDS); + } else if (config.pulsePeriod != 0) { + portData.setPulsing(scheduler.scheduleWithFixedDelay(() -> { + triggerPushButtonChannel(channel, EVENT_PULSE); + }, config.pulsePeriod, config.pulsePeriod, TimeUnit.MILLISECONDS)); + if (config.pulseTimeout != 0) { + portData.setPulseCanceler(scheduler.schedule(portData::cancelPulsing, + config.pulseTimeout, TimeUnit.MILLISECONDS)); } - break; - case OPEN: - if (!portData.isInitializing() && config.longPressTime != 0 - && sinceLastChange < config.longPressTime) { - triggerPushButtonChannel(channel, EVENT_SHORT_PRESS); - } - break; + } + } else if (portData.isInitialized() && sinceLastChange < config.longPressTime) { + triggerPushButtonChannel(channel, EVENT_SHORT_PRESS); } - if (!portData.isInitializing()) { + if (portData.isInitialized()) { triggerPushButtonChannel(channel, value == 1 ? EVENT_PRESSED : EVENT_RELEASED); } - break; - } + yield value == 1 ? OpenClosedType.CLOSED : OpenClosedType.OPEN; + } + }; updateIfLinked(channelId, state); - if (!portData.isInitializing()) { + if (portData.isInitialized()) { updateIfLinked(channelId + PROPERTY_SEPARATOR + CHANNEL_LAST_STATE_DURATION, new QuantityType<>(sinceLastChange / 1000, Units.SECOND)); } @@ -354,21 +326,18 @@ public class Ipx800v3Handler extends BaseThingHandler implements Ipx800EventList public void handleCommand(ChannelUID channelUID, Command command) { logger.debug("Received channel: {}, command: {}", channelUID, command); - Channel channel = thing.getChannel(channelUID.getId()); - String groupId = channelUID.getGroupId(); - - if (channel == null || groupId == null) { - return; - } - if (command instanceof OnOffType onOffCommand && isValidPortId(channelUID) - && PortDefinition.fromGroupId(groupId) == PortDefinition.RELAY + if (thing.getChannel(channelUID.getId()) instanceof Channel channel + && channelUID.getGroupId() instanceof String groupId // + && command instanceof OnOffType onOffCommand // + && isValidPortId(channelUID) // + && PortDefinition.RELAY.equals(PortDefinition.fromGroupId(groupId)) && deviceConnector instanceof Ipx800DeviceConnector connector) { RelayOutputConfiguration config = channel.getConfiguration().as(RelayOutputConfiguration.class); - String id = channelUID.getIdWithoutGroup(); - connector.getParser().setOutput(id, onOffCommand == OnOffType.ON ? 1 : 0, config.pulse); - return; + connector.setOutput(channelUID.getIdWithoutGroup(), OnOffType.ON.equals(onOffCommand) ? 1 : 0, + config.pulse); + } else { + logger.debug("Can not handle command '{}' on channel '{}'", command, channelUID); } - logger.debug("Can not handle command '{}' on channel '{}'", command, channelUID); } private boolean isValidPortId(ChannelUID channelUID) { @@ -377,13 +346,13 @@ public class Ipx800v3Handler extends BaseThingHandler implements Ipx800EventList public void resetCounter(int counter) { if (deviceConnector instanceof Ipx800DeviceConnector connector) { - connector.getParser().resetCounter(counter); + connector.resetCounter(counter); } } public void reset() { if (deviceConnector instanceof Ipx800DeviceConnector connector) { - connector.getParser().resetPLC(); + connector.resetPLC(); } } diff --git a/bundles/org.openhab.binding.gce/src/main/java/org/openhab/binding/gce/internal/model/M2MMessageParser.java b/bundles/org.openhab.binding.gce/src/main/java/org/openhab/binding/gce/internal/model/M2MMessageParser.java index cfb5ff1a904..f18ef61c19b 100644 --- a/bundles/org.openhab.binding.gce/src/main/java/org/openhab/binding/gce/internal/model/M2MMessageParser.java +++ b/bundles/org.openhab.binding.gce/src/main/java/org/openhab/binding/gce/internal/model/M2MMessageParser.java @@ -15,7 +15,6 @@ package org.openhab.binding.gce.internal.model; import java.util.regex.Pattern; import org.eclipse.jdt.annotation.NonNullByDefault; -import org.openhab.binding.gce.internal.handler.Ipx800DeviceConnector; import org.openhab.binding.gce.internal.handler.Ipx800EventListener; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -33,22 +32,19 @@ public class M2MMessageParser { .compile("I=" + IO_DESCRIPTOR + "&O=" + IO_DESCRIPTOR + "&([AC]\\d{1,2}=\\d+&)*[^I]*"); private final Logger logger = LoggerFactory.getLogger(M2MMessageParser.class); - private final Ipx800DeviceConnector connector; private final Ipx800EventListener listener; private String expectedResponse = ""; - public M2MMessageParser(Ipx800DeviceConnector connector, Ipx800EventListener listener) { - this.connector = connector; + public M2MMessageParser(Ipx800EventListener listener) { this.listener = listener; } - /** - * - * @param data - */ public void unsolicitedUpdate(String data) { - if (IO_PATTERN.matcher(data).matches()) { + if ("OK".equals(data)) { // If OK, do nothing special + } else if ("? Bad command".equals(data)) { + logger.warn(data); + } else if (IO_PATTERN.matcher(data).matches()) { PortDefinition portDefinition = PortDefinition.fromM2MCommand(expectedResponse); decodeDataLine(portDefinition, data); } else if (VALIDATION_PATTERN.matcher(data).matches()) { @@ -72,8 +68,9 @@ public class M2MMessageParser { } } else if (!expectedResponse.isEmpty()) { setStatus(expectedResponse, Double.parseDouble(data)); + } else { + logger.warn("Unable to handle data received: {}", data); } - expectedResponse = ""; } @@ -84,47 +81,17 @@ public class M2MMessageParser { } private void setStatus(String port, double value) { - logger.debug("Received {} : {}", port, value); + logger.debug("Received {} on port {}", value, port); listener.dataReceived(port, value); } public void setExpectedResponse(String expectedResponse) { if (expectedResponse.endsWith("s")) { // GetInputs or GetOutputs this.expectedResponse = expectedResponse; - } else { // GetAnx or GetCountx - PortDefinition portType = PortDefinition.fromM2MCommand(expectedResponse); - this.expectedResponse = expectedResponse.replaceAll(portType.m2mCommand, portType.portName); + return; } - } - - /** - * Set output of the device sending the corresponding command - * - * @param targetPort - * @param targetValue - */ - public void setOutput(String targetPort, int targetValue, boolean pulse) { - logger.debug("Sending {} to {}", targetValue, targetPort); - String command = "Set%02d%s%s".formatted(Integer.parseInt(targetPort), targetValue, pulse ? "p" : ""); - connector.send(command); - } - - /** - * Resets the counter value to 0 - * - * @param targetCounter - */ - public void resetCounter(int targetCounter) { - logger.debug("Resetting counter {} to 0", targetCounter); - connector.send("ResetCount%d".formatted(targetCounter)); - } - - public void errorOccurred(Exception e) { - logger.warn("Error received from connector : {}", e.getMessage()); - listener.errorOccurred(e); - } - - public void resetPLC() { - connector.send("Reset"); + // GetAnx or GetCountx + PortDefinition portType = PortDefinition.fromM2MCommand(expectedResponse); + this.expectedResponse = expectedResponse.replaceAll(portType.m2mCommand, portType.portName); } } 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 cddafa7efb9..94ac768e04c 100644 --- 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.Instant; -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,15 +27,27 @@ import org.eclipse.jdt.annotation.NonNullByDefault; public class PortData { private double value = -1; private Instant timestamp = Instant.now(); - private Optional> pulsing = Optional.empty(); + private @Nullable ScheduledFuture pulsing; + private @Nullable ScheduledFuture pulseCanceler; public void cancelPulsing() { - pulsing.ifPresent(pulse -> pulse.cancel(true)); - pulsing = Optional.empty(); + if (pulsing instanceof ScheduledFuture job) { + job.cancel(true); + pulsing = null; + } + cancelCanceler(); + } + + public void cancelCanceler() { + if (pulseCanceler instanceof ScheduledFuture job) { + job.cancel(true); + pulseCanceler = null; + } } public void dispose() { cancelPulsing(); + cancelCanceler(); } public void setData(double value, Instant timestamp) { @@ -53,10 +65,14 @@ public class PortData { public void setPulsing(ScheduledFuture pulsing) { cancelPulsing(); - this.pulsing = Optional.of(pulsing); + this.pulsing = pulsing; } - public boolean isInitializing() { - return value == -1; + public boolean isInitialized() { + return value != -1; + } + + public void setPulseCanceler(ScheduledFuture pulseCanceler) { + this.pulseCanceler = pulseCanceler; } } diff --git a/bundles/org.openhab.binding.gce/src/main/java/org/openhab/binding/gce/internal/model/PortDefinition.java b/bundles/org.openhab.binding.gce/src/main/java/org/openhab/binding/gce/internal/model/PortDefinition.java index 6872ed205ed..1362107f049 100644 --- a/bundles/org.openhab.binding.gce/src/main/java/org/openhab/binding/gce/internal/model/PortDefinition.java +++ b/bundles/org.openhab.binding.gce/src/main/java/org/openhab/binding/gce/internal/model/PortDefinition.java @@ -34,7 +34,7 @@ public enum PortDefinition { public final String m2mCommand; // associated M2M command public final int quantity; // base number of ports - PortDefinition(String nodeName, String portName, String m2mCommand, int quantity) { + private PortDefinition(String nodeName, String portName, String m2mCommand, int quantity) { this.nodeName = nodeName; this.portName = portName; this.m2mCommand = m2mCommand; diff --git a/bundles/org.openhab.binding.gce/src/main/java/org/openhab/binding/gce/internal/model/StatusFile.java b/bundles/org.openhab.binding.gce/src/main/java/org/openhab/binding/gce/internal/model/StatusFile.java index dca907e6861..6b7f285d000 100644 --- a/bundles/org.openhab.binding.gce/src/main/java/org/openhab/binding/gce/internal/model/StatusFile.java +++ b/bundles/org.openhab.binding.gce/src/main/java/org/openhab/binding/gce/internal/model/StatusFile.java @@ -12,14 +12,15 @@ */ package org.openhab.binding.gce.internal.model; -import java.util.Comparator; -import java.util.List; +import java.util.HashMap; +import java.util.Map; import java.util.stream.IntStream; import org.eclipse.jdt.annotation.NonNullByDefault; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; import org.w3c.dom.Document; import org.w3c.dom.Element; -import org.w3c.dom.Node; import org.w3c.dom.NodeList; /** @@ -29,12 +30,14 @@ import org.w3c.dom.NodeList; */ @NonNullByDefault public class StatusFile { - + private final Logger logger = LoggerFactory.getLogger(StatusFile.class); private final Element root; + private final NodeList childs; public StatusFile(Document doc) { this.root = doc.getDocumentElement(); root.normalize(); + this.childs = root.getChildNodes(); } public String getMac() { @@ -45,14 +48,20 @@ public class StatusFile { return root.getElementsByTagName("version").item(0).getTextContent(); } - public List 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 Map getPorts(PortDefinition portDefinition) { + Map result = new HashMap<>(); - public int getMaxNumberofNodeType(PortDefinition portDefinition) { - return getMatchingNodes(portDefinition.nodeName).size(); + String searched = portDefinition.nodeName; + + IntStream.range(0, childs.getLength()).boxed().map(childs::item) + .filter(node -> node.getNodeName().startsWith(searched)).forEach(node -> { + try { + result.put(Integer.parseInt(node.getNodeName().replace(searched, "")) + 1, + Double.parseDouble(node.getTextContent().replace("dn", "1").replace("up", "0"))); + } catch (NumberFormatException e) { + logger.warn(e.getMessage()); + } + }); + return result; } } diff --git a/bundles/org.openhab.binding.gce/src/main/java/org/openhab/binding/gce/internal/model/StatusFileAccessor.java b/bundles/org.openhab.binding.gce/src/main/java/org/openhab/binding/gce/internal/model/StatusFileAccessor.java index 16a6e15cb9a..9c17f49471c 100644 --- a/bundles/org.openhab.binding.gce/src/main/java/org/openhab/binding/gce/internal/model/StatusFileAccessor.java +++ b/bundles/org.openhab.binding.gce/src/main/java/org/openhab/binding/gce/internal/model/StatusFileAccessor.java @@ -22,7 +22,7 @@ import org.eclipse.jdt.annotation.NonNullByDefault; import org.xml.sax.SAXException; /** - * This class takes care of interpreting the status.xml file + * This class takes care of providing the IPX status file * * @author Gaël L'hopital - Initial contribution */ @@ -34,7 +34,7 @@ public class StatusFileAccessor { private final String url; public StatusFileAccessor(String hostname) { - this.url = String.format(URL_TEMPLATE, hostname); + this.url = URL_TEMPLATE.formatted(hostname); DocumentBuilderFactory factory = DocumentBuilderFactory.newInstance(); factory.setXIncludeAware(false); factory.setExpandEntityReferences(false); @@ -45,12 +45,11 @@ public class StatusFileAccessor { 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); + throw new IllegalArgumentException("Error initializing StatusFileAccessor", e); } } public StatusFile read() throws SAXException, IOException { - StatusFile document = new StatusFile(builder.parse(url)); - return document; + return new StatusFile(builder.parse(url)); } } From f69d5214abbae1884c237fca79f428f32ca62938 Mon Sep 17 00:00:00 2001 From: clinique Date: Fri, 27 Dec 2024 10:28:05 +0100 Subject: [PATCH 05/11] Apply spotless Signed-off-by: clinique --- .../binding/gce/internal/handler/Ipx800DeviceConnector.java | 1 - .../openhab/binding/gce/internal/handler/Ipx800v3Handler.java | 1 - 2 files changed, 2 deletions(-) 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 17661e0dfe7..f2130f99b6f 100644 --- 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 @@ -218,5 +218,4 @@ public class Ipx800DeviceConnector extends Thread { public void resetPLC() { send("Reset"); } - } 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 1529ea772dd..c909d55a2f6 100644 --- 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 @@ -136,7 +136,6 @@ public class Ipx800v3Handler extends BaseThingHandler implements Ipx800EventList (portNum, value) -> dataReceived("%s%d".formatted(portDefinition.portName, portNum), value))); } } - } private void updateChannels(@Nullable StatusFile status) { From b51b0e2a0ee4bb48d89ef1546630e2610d958e2d Mon Sep 17 00:00:00 2001 From: clinique Date: Fri, 27 Dec 2024 10:34:21 +0100 Subject: [PATCH 06/11] Solving log errors Signed-off-by: clinique --- .../openhab/binding/gce/internal/model/M2MMessageParser.java | 2 +- .../java/org/openhab/binding/gce/internal/model/StatusFile.java | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/bundles/org.openhab.binding.gce/src/main/java/org/openhab/binding/gce/internal/model/M2MMessageParser.java b/bundles/org.openhab.binding.gce/src/main/java/org/openhab/binding/gce/internal/model/M2MMessageParser.java index f18ef61c19b..959141d3f20 100644 --- a/bundles/org.openhab.binding.gce/src/main/java/org/openhab/binding/gce/internal/model/M2MMessageParser.java +++ b/bundles/org.openhab.binding.gce/src/main/java/org/openhab/binding/gce/internal/model/M2MMessageParser.java @@ -43,7 +43,7 @@ public class M2MMessageParser { public void unsolicitedUpdate(String data) { if ("OK".equals(data)) { // If OK, do nothing special } else if ("? Bad command".equals(data)) { - logger.warn(data); + logger.warn("{}", data); } else if (IO_PATTERN.matcher(data).matches()) { PortDefinition portDefinition = PortDefinition.fromM2MCommand(expectedResponse); decodeDataLine(portDefinition, data); diff --git a/bundles/org.openhab.binding.gce/src/main/java/org/openhab/binding/gce/internal/model/StatusFile.java b/bundles/org.openhab.binding.gce/src/main/java/org/openhab/binding/gce/internal/model/StatusFile.java index 6b7f285d000..850f407ed37 100644 --- a/bundles/org.openhab.binding.gce/src/main/java/org/openhab/binding/gce/internal/model/StatusFile.java +++ b/bundles/org.openhab.binding.gce/src/main/java/org/openhab/binding/gce/internal/model/StatusFile.java @@ -59,7 +59,7 @@ public class StatusFile { result.put(Integer.parseInt(node.getNodeName().replace(searched, "")) + 1, Double.parseDouble(node.getTextContent().replace("dn", "1").replace("up", "0"))); } catch (NumberFormatException e) { - logger.warn(e.getMessage()); + logger.warn("{}", e.getMessage()); } }); return result; From a2befde47b7ddad08691a474de74bdab4b95be5a Mon Sep 17 00:00:00 2001 From: clinique Date: Wed, 1 Jan 2025 01:44:56 +0100 Subject: [PATCH 07/11] Trying again to clean everything Signed-off-by: clinique --- .../handler/Ipx800DeviceConnector.java | 61 ++++++++++++------- .../gce/internal/handler/Ipx800v3Handler.java | 48 +++++---------- 2 files changed, 56 insertions(+), 53 deletions(-) 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 f2130f99b6f..f2114a13c4f 100644 --- 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 @@ -19,10 +19,10 @@ import java.io.PrintWriter; import java.net.Socket; import java.net.SocketException; import java.net.SocketTimeoutException; -import java.util.Optional; import java.util.Random; import org.eclipse.jdt.annotation.NonNullByDefault; +import org.eclipse.jdt.annotation.Nullable; import org.openhab.binding.gce.internal.model.M2MMessageParser; import org.openhab.binding.gce.internal.model.PortDefinition; import org.openhab.binding.gce.internal.model.StatusFile; @@ -54,9 +54,10 @@ public class Ipx800DeviceConnector extends Thread { private final StatusFileAccessor statusAccessor; private final Ipx800EventListener listener; - private Optional socket = Optional.empty(); - private Optional input = Optional.empty(); - private Optional output = Optional.empty(); + private @Nullable Socket socket; + private @Nullable BufferedReader input; + private @Nullable PrintWriter output; + private @Nullable InputStreamReader streamReader; private int failedKeepalive = 0; private boolean waitingKeepaliveResponse = false; @@ -82,27 +83,41 @@ public class Ipx800DeviceConnector extends Thread { logger.debug("Connecting to {}:{}...", hostname, portNumber); Socket socket = new Socket(hostname, portNumber); socket.setSoTimeout(DEFAULT_SOCKET_TIMEOUT_MS); - // socket.getInputStream().skip(socket.getInputStream().available()); - this.socket = Optional.of(socket); + this.socket = socket; - output = Optional.of(new PrintWriter(socket.getOutputStream(), true)); - input = Optional.of(new BufferedReader(new InputStreamReader(socket.getInputStream()))); + output = new PrintWriter(socket.getOutputStream(), true); + streamReader = new InputStreamReader(socket.getInputStream()); + input = new BufferedReader(streamReader); } /** * Disconnect the device */ private void disconnect() { - socket.ifPresent(client -> { + if (socket instanceof Socket client) { try { + if (output instanceof PrintWriter out) { + out.close(); + output = null; + } + + if (input instanceof BufferedReader in) { + in.close(); + input = null; + } + + if (streamReader instanceof InputStreamReader stream) { + stream.close(); + streamReader = null; + } + logger.debug("Closing socket"); client.close(); - } catch (IOException ignore) { + } catch (IOException e) { + logger.warn("Exception closing streams: {}", e.getMessage()); } - socket = Optional.empty(); - input = Optional.empty(); - output = Optional.empty(); - }); + socket = null; + } } /** @@ -114,18 +129,20 @@ public class Ipx800DeviceConnector extends Thread { } public synchronized void send(String message) { - output.ifPresentOrElse(out -> { + if (output instanceof PrintWriter out) { logger.debug("Sending '{}' to Ipx800", message); out.println(message); - }, () -> logger.warn("Unable to send '{}' when the output stream is closed.", message)); + } else { + logger.warn("Unable to send '{}' when the output stream is closed.", message); + } } /** - * Send an arbitrary keepalive command which cause the IPX to send an update. + * Send a random keepalive command which cause the IPX to send an update. * If we don't receive the update maxKeepAliveFailure time, the connection is closed and reopened */ private void sendKeepalive() { - output.ifPresentOrElse(out -> { + if (output instanceof PrintWriter out) { PortDefinition pd = PortDefinition.values()[randomizer.nextInt(PortDefinition.AS_SET.size())]; String command = "%s%d".formatted(pd.m2mCommand, randomizer.nextInt(pd.quantity) + 1); @@ -141,7 +158,9 @@ public class Ipx800DeviceConnector extends Thread { parser.setExpectedResponse(command); waitingKeepaliveResponse = true; - }, () -> logger.warn("Unable to send keepAlive when the output stream is closed.")); + } else { + logger.warn("Unable to send keepAlive when the output stream is closed."); + } } @Override @@ -154,7 +173,7 @@ public class Ipx800DeviceConnector extends Thread { if (failedKeepalive > MAX_KEEPALIVE_FAILURE) { throw new IOException("Max keep alive attempts has been reached"); } - input.ifPresent(in -> { + if (input instanceof BufferedReader in) { try { String command = in.readLine(); waitingKeepaliveResponse = false; @@ -162,7 +181,7 @@ public class Ipx800DeviceConnector extends Thread { } catch (IOException e) { handleException(e); } - }); + } } disconnect(); } catch (IOException e) { 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 c909d55a2f6..a250813a36f 100644 --- 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 @@ -23,7 +23,6 @@ import java.util.Collection; 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; @@ -73,30 +72,10 @@ public class Ipx800v3Handler extends BaseThingHandler implements Ipx800EventList private static final double ANALOG_SAMPLING = 0.000050354; private final Logger logger = LoggerFactory.getLogger(Ipx800v3Handler.class); - private final Map portDatas = new HashMap<>(); + private final Map portDatas = new HashMap<>(); private @Nullable Ipx800DeviceConnector deviceConnector; - private Optional> refreshJob = Optional.empty(); - - private class LongPressEvaluator implements Runnable { - private final Instant referenceTime; - private final String port; - private final String eventChannelId; - - public LongPressEvaluator(Channel channel, String port, PortData portData) { - this.referenceTime = portData.getTimestamp(); - this.port = port; - this.eventChannelId = "%s-%s".formatted(channel.getUID().getId(), TRIGGER_CONTACT); - } - - @Override - public void run() { - if (portDatas.get(port) instanceof PortData currentData && currentData.getValue() == 1 - && referenceTime.equals(currentData.getTimestamp())) { - triggerChannel(eventChannelId, EVENT_LONG_PRESS); - } - } - } + private List> jobs = new ArrayList<>(); public Ipx800v3Handler(Thing thing) { super(thing); @@ -112,7 +91,7 @@ public class Ipx800v3Handler extends BaseThingHandler implements Ipx800EventList updateStatus(ThingStatus.UNKNOWN); - refreshJob = Optional.of(scheduler.scheduleWithFixedDelay(this::readStatusFile, 1500, config.pullInterval, + jobs.add(scheduler.scheduleWithFixedDelay(this::readStatusFile, 1500, config.pullInterval, TimeUnit.MILLISECONDS)); } @@ -144,7 +123,7 @@ public class Ipx800v3Handler extends BaseThingHandler implements Ipx800EventList int nbElements = status != null ? status.getPorts(portDefinition).size() : portDefinition.quantity; for (int i = 0; i < nbElements; i++) { ChannelUID portChannelUID = createChannels(portDefinition, i, channels); - portDatas.put(portChannelUID.getId(), new PortData()); + portDatas.put(portChannelUID, new PortData()); } }); updateThing(editThing().withChannels(channels).build()); @@ -162,8 +141,8 @@ public class Ipx800v3Handler extends BaseThingHandler implements Ipx800EventList @Override public void dispose() { - refreshJob.ifPresent(job -> job.cancel(true)); - refreshJob = Optional.empty(); + jobs.forEach(job -> job.cancel(true)); + jobs.clear(); if (deviceConnector instanceof Ipx800DeviceConnector connector) { connector.dispose(); @@ -248,10 +227,11 @@ public class Ipx800v3Handler extends BaseThingHandler implements Ipx800EventList public void dataReceived(String port, double value) { updateStatus(ThingStatus.ONLINE); if (thing.getChannel(PortDefinition.asChannelId(port)) instanceof Channel channel) { - String channelId = channel.getUID().getId(); + ChannelUID channelUID = channel.getUID(); + String channelId = channelUID.getId(); - if (portDatas.get(channelId) instanceof PortData portData - && channel.getUID().getGroupId() instanceof String groupId) { + if (portDatas.get(channelUID) instanceof PortData portData + && channelUID.getGroupId() instanceof String groupId) { Instant now = Instant.now(); Configuration configuration = channel.getConfiguration(); PortDefinition portDefinition = PortDefinition.fromGroupId(groupId); @@ -275,8 +255,12 @@ public class Ipx800v3Handler extends BaseThingHandler implements Ipx800EventList if (value == 1) { // CLOSED if (config.longPressTime != 0 && portData.isInitialized()) { - scheduler.schedule(new LongPressEvaluator(channel, port, portData), - config.longPressTime, TimeUnit.MILLISECONDS); + jobs.add(scheduler.schedule(() -> { + if (portData.getValue() == 1 && now.equals(portData.getTimestamp())) { + String eventChannelId = "%s-%s".formatted(channelUID.getId(), TRIGGER_CONTACT); + triggerChannel(eventChannelId, EVENT_LONG_PRESS); + } + }, config.longPressTime, TimeUnit.MILLISECONDS)); } else if (config.pulsePeriod != 0) { portData.setPulsing(scheduler.scheduleWithFixedDelay(() -> { triggerPushButtonChannel(channel, EVENT_PULSE); From 7d46776956214a6da140b9873e83d14567daf40d Mon Sep 17 00:00:00 2001 From: "gael@lhopital.org" Date: Thu, 2 Jan 2025 18:05:46 +0100 Subject: [PATCH 08/11] Reviewing connector logic Signed-off-by: gael@lhopital.org --- .../handler/Ipx800DeviceConnector.java | 179 ++++++------------ .../gce/internal/handler/Ipx800v3Handler.java | 17 +- 2 files changed, 71 insertions(+), 125 deletions(-) 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 f2114a13c4f..fc1c373eea0 100644 --- 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 @@ -17,12 +17,11 @@ import java.io.IOException; import java.io.InputStreamReader; import java.io.PrintWriter; import java.net.Socket; -import java.net.SocketException; import java.net.SocketTimeoutException; +import java.net.UnknownHostException; import java.util.Random; import org.eclipse.jdt.annotation.NonNullByDefault; -import org.eclipse.jdt.annotation.Nullable; import org.openhab.binding.gce.internal.model.M2MMessageParser; import org.openhab.binding.gce.internal.model.PortDefinition; import org.openhab.binding.gce.internal.model.StatusFile; @@ -42,43 +41,26 @@ import org.xml.sax.SAXException; @NonNullByDefault public class Ipx800DeviceConnector extends Thread { private static final int DEFAULT_SOCKET_TIMEOUT_MS = 10000; - private static final int DEFAULT_RECONNECT_TIMEOUT_MS = 5000; private static final int MAX_KEEPALIVE_FAILURE = 3; private final Logger logger = LoggerFactory.getLogger(Ipx800DeviceConnector.class); private final Random randomizer = new Random(); - private final String hostname; - private final int portNumber; private final M2MMessageParser parser; private final StatusFileAccessor statusAccessor; private final Ipx800EventListener listener; - - private @Nullable Socket socket; - private @Nullable BufferedReader input; - private @Nullable PrintWriter output; - private @Nullable InputStreamReader streamReader; + private final Socket socket; + private final BufferedReader input; + private final PrintWriter output; private int failedKeepalive = 0; private boolean waitingKeepaliveResponse = false; + private boolean interrupted = false; - public Ipx800DeviceConnector(String hostname, int portNumber, ThingUID uid, Ipx800EventListener listener) { + public Ipx800DeviceConnector(String hostname, int portNumber, ThingUID uid, Ipx800EventListener listener) + throws UnknownHostException, IOException { super("OH-binding-" + uid); - this.hostname = hostname; - this.portNumber = portNumber; this.listener = listener; - this.parser = new M2MMessageParser(listener); - this.statusAccessor = new StatusFileAccessor(hostname); - setDaemon(true); - } - - /** - * Connect to the ipx800 - * - * @throws IOException - */ - private void connect() throws IOException { - disconnect(); logger.debug("Connecting to {}:{}...", hostname, portNumber); Socket socket = new Socket(hostname, portNumber); @@ -86,126 +68,85 @@ public class Ipx800DeviceConnector extends Thread { this.socket = socket; output = new PrintWriter(socket.getOutputStream(), true); - streamReader = new InputStreamReader(socket.getInputStream()); - input = new BufferedReader(streamReader); - } - - /** - * Disconnect the device - */ - private void disconnect() { - if (socket instanceof Socket client) { - try { - if (output instanceof PrintWriter out) { - out.close(); - output = null; - } - - if (input instanceof BufferedReader in) { - in.close(); - input = null; - } - - if (streamReader instanceof InputStreamReader stream) { - stream.close(); - streamReader = null; - } - - logger.debug("Closing socket"); - client.close(); - } catch (IOException e) { - logger.warn("Exception closing streams: {}", e.getMessage()); - } - socket = null; - } + input = new BufferedReader(new InputStreamReader(socket.getInputStream())); + parser = new M2MMessageParser(listener); + statusAccessor = new StatusFileAccessor(hostname); + setDaemon(true); } /** * Stop the device thread */ public void dispose() { - interrupt(); - disconnect(); + interrupted = true; } public synchronized void send(String message) { - if (output instanceof PrintWriter out) { - logger.debug("Sending '{}' to Ipx800", message); - out.println(message); - } else { - logger.warn("Unable to send '{}' when the output stream is closed.", message); - } + logger.debug("Sending '{}' to Ipx800", message); + output.println(message); } /** * Send a random keepalive command which cause the IPX to send an update. - * If we don't receive the update maxKeepAliveFailure time, the connection is closed and reopened + * If we don't receive the update maxKeepAliveFailure time, the connection is closed */ private void sendKeepalive() { - if (output instanceof PrintWriter out) { - PortDefinition pd = PortDefinition.values()[randomizer.nextInt(PortDefinition.AS_SET.size())]; - String command = "%s%d".formatted(pd.m2mCommand, randomizer.nextInt(pd.quantity) + 1); + PortDefinition pd = PortDefinition.values()[randomizer.nextInt(PortDefinition.AS_SET.size())]; + String command = "%s%d".formatted(pd.m2mCommand, randomizer.nextInt(pd.quantity) + 1); - if (waitingKeepaliveResponse) { - failedKeepalive++; - logger.debug("Sending keepalive {}, attempt {}", command, failedKeepalive); - } else { - failedKeepalive = 0; - logger.debug("Sending keepalive {}", command); - } - - out.println(command); - parser.setExpectedResponse(command); - - waitingKeepaliveResponse = true; + if (waitingKeepaliveResponse) { + failedKeepalive++; + logger.debug("Sending keepalive {}, attempt {}", command, failedKeepalive); } else { - logger.warn("Unable to send keepAlive when the output stream is closed."); + failedKeepalive = 0; + logger.debug("Sending keepalive {}", command); } + + output.println(command); + parser.setExpectedResponse(command); + + waitingKeepaliveResponse = true; } @Override public void run() { - try { - waitingKeepaliveResponse = false; - failedKeepalive = 0; - connect(); - while (!interrupted()) { - if (failedKeepalive > MAX_KEEPALIVE_FAILURE) { - throw new IOException("Max keep alive attempts has been reached"); - } - if (input instanceof BufferedReader in) { - try { - String command = in.readLine(); - waitingKeepaliveResponse = false; - parser.unsolicitedUpdate(command); - } catch (IOException e) { - handleException(e); - } - } + while (!interrupted) { + if (failedKeepalive > MAX_KEEPALIVE_FAILURE) { + interrupted = true; + listener.errorOccurred(new IOException("Max keep alive attempts has been reached")); } - disconnect(); - } catch (IOException e) { - handleException(e); - } - try { - Thread.sleep(DEFAULT_RECONNECT_TIMEOUT_MS); - } catch (InterruptedException e) { - dispose(); - } - } - - private void handleException(Exception e) { - if (!interrupted()) { - if (e instanceof SocketTimeoutException) { + try { + String command = input.readLine(); + waitingKeepaliveResponse = false; + parser.unsolicitedUpdate(command); + } catch (SocketTimeoutException e) { sendKeepalive(); - return; - } else if (e instanceof SocketException) { - logger.debug("SocketException raised by streams while closing socket"); - } else if (e instanceof IOException) { - logger.warn("Communication error: '{}'. Will retry in {} ms", e, DEFAULT_RECONNECT_TIMEOUT_MS); + } catch (IOException e) { + interrupted = true; + listener.errorOccurred(e); } - listener.errorOccurred(e); } + if (output instanceof PrintWriter out) { + out.close(); + } + + if (input instanceof BufferedReader in) { + try { + in.close(); + } catch (IOException e) { + logger.warn("Exception input stream: {}", e.getMessage()); + } + } + + if (socket instanceof Socket client) { + try { + logger.debug("Closing socket"); + client.close(); + } catch (IOException e) { + logger.warn("Exception closing socket: {}", e.getMessage()); + } + } + } public StatusFile readStatusFile() throws SAXException, IOException { 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 a250813a36f..4ee32a43bb0 100644 --- 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 @@ -15,6 +15,7 @@ package org.openhab.binding.gce.internal.handler; import static org.openhab.binding.gce.internal.GCEBindingConstants.*; import java.io.IOException; +import java.net.UnknownHostException; import java.time.Duration; import java.time.Instant; import java.time.temporal.ChronoUnit; @@ -87,12 +88,16 @@ public class Ipx800v3Handler extends BaseThingHandler implements Ipx800EventList Ipx800Configuration config = getConfigAs(Ipx800Configuration.class); - deviceConnector = new Ipx800DeviceConnector(config.hostname, config.portNumber, getThing().getUID(), this); - - updateStatus(ThingStatus.UNKNOWN); - - jobs.add(scheduler.scheduleWithFixedDelay(this::readStatusFile, 1500, config.pullInterval, - TimeUnit.MILLISECONDS)); + try { + deviceConnector = new Ipx800DeviceConnector(config.hostname, config.portNumber, getThing().getUID(), this); + updateStatus(ThingStatus.UNKNOWN); + jobs.add(scheduler.scheduleWithFixedDelay(this::readStatusFile, 1500, config.pullInterval, + TimeUnit.MILLISECONDS)); + } catch (UnknownHostException e) { + updateStatus(ThingStatus.OFFLINE, ThingStatusDetail.CONFIGURATION_ERROR, e.getMessage()); + } catch (IOException e) { + updateStatus(ThingStatus.OFFLINE, ThingStatusDetail.COMMUNICATION_ERROR, e.getMessage()); + } } private void readStatusFile() { From 828e3726441348f77f63f249914373b6d76eb553 Mon Sep 17 00:00:00 2001 From: "gael@lhopital.org" Date: Thu, 2 Jan 2025 18:09:34 +0100 Subject: [PATCH 09/11] Upgrading to 2025 Signed-off-by: gael@lhopital.org --- .../java/org/openhab/binding/gce/internal/model/StatusFile.java | 2 +- .../openhab/binding/gce/internal/model/StatusFileAccessor.java | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/bundles/org.openhab.binding.gce/src/main/java/org/openhab/binding/gce/internal/model/StatusFile.java b/bundles/org.openhab.binding.gce/src/main/java/org/openhab/binding/gce/internal/model/StatusFile.java index 850f407ed37..27b148b7b71 100644 --- a/bundles/org.openhab.binding.gce/src/main/java/org/openhab/binding/gce/internal/model/StatusFile.java +++ b/bundles/org.openhab.binding.gce/src/main/java/org/openhab/binding/gce/internal/model/StatusFile.java @@ -1,5 +1,5 @@ /** - * Copyright (c) 2010-2024 Contributors to the openHAB project + * Copyright (c) 2010-2025 Contributors to the openHAB project * * See the NOTICE file(s) distributed with this work for additional * information. diff --git a/bundles/org.openhab.binding.gce/src/main/java/org/openhab/binding/gce/internal/model/StatusFileAccessor.java b/bundles/org.openhab.binding.gce/src/main/java/org/openhab/binding/gce/internal/model/StatusFileAccessor.java index 9c17f49471c..794e655cec5 100644 --- a/bundles/org.openhab.binding.gce/src/main/java/org/openhab/binding/gce/internal/model/StatusFileAccessor.java +++ b/bundles/org.openhab.binding.gce/src/main/java/org/openhab/binding/gce/internal/model/StatusFileAccessor.java @@ -1,5 +1,5 @@ /** - * Copyright (c) 2010-2024 Contributors to the openHAB project + * Copyright (c) 2010-2025 Contributors to the openHAB project * * See the NOTICE file(s) distributed with this work for additional * information. From 4b32f8e605df4731abd4f079d43e35f5b304ff76 Mon Sep 17 00:00:00 2001 From: "gael@lhopital.org" Date: Thu, 2 Jan 2025 18:11:39 +0100 Subject: [PATCH 10/11] Rebased Some more code refactoring MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Gaël L'hopital Signed-off-by: gael@lhopital.org --- .../binding/gce/internal/model/StatusFileInterpreter.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/bundles/org.openhab.binding.gce/src/main/java/org/openhab/binding/gce/internal/model/StatusFileInterpreter.java b/bundles/org.openhab.binding.gce/src/main/java/org/openhab/binding/gce/internal/model/StatusFileInterpreter.java index 99012270f63..593a1b460e3 100644 --- a/bundles/org.openhab.binding.gce/src/main/java/org/openhab/binding/gce/internal/model/StatusFileInterpreter.java +++ b/bundles/org.openhab.binding.gce/src/main/java/org/openhab/binding/gce/internal/model/StatusFileInterpreter.java @@ -92,7 +92,7 @@ public class StatusFileInterpreter { private void pushDatas() { getRoot().ifPresent(root -> { - PortDefinition.AS_STREAM.forEach(portDefinition -> { + PortDefinition.AS_SET.forEach(portDefinition -> { List xmlNodes = getMatchingNodes(root.getChildNodes(), portDefinition.nodeName); xmlNodes.forEach(xmlNode -> { String sPortNum = xmlNode.getNodeName().replace(portDefinition.nodeName, ""); From 8e19f2c19bf32e235d05dd4ccc1e6b6be4fa7aa4 Mon Sep 17 00:00:00 2001 From: clinique Date: Sat, 4 Jan 2025 09:44:18 +0100 Subject: [PATCH 11/11] Apply spotless Signed-off-by: clinique --- .../openhab/binding/gce/internal/action/Ipx800Actions.java | 6 ++---- .../binding/gce/internal/handler/Ipx800DeviceConnector.java | 1 - 2 files changed, 2 insertions(+), 5 deletions(-) diff --git a/bundles/org.openhab.binding.gce/src/main/java/org/openhab/binding/gce/internal/action/Ipx800Actions.java b/bundles/org.openhab.binding.gce/src/main/java/org/openhab/binding/gce/internal/action/Ipx800Actions.java index 9ae9a97d803..268b3ce1fdf 100644 --- a/bundles/org.openhab.binding.gce/src/main/java/org/openhab/binding/gce/internal/action/Ipx800Actions.java +++ b/bundles/org.openhab.binding.gce/src/main/java/org/openhab/binding/gce/internal/action/Ipx800Actions.java @@ -58,8 +58,7 @@ public class Ipx800Actions implements ThingActions { public void resetCounter( @ActionInput(name = "counter", label = "Counter", required = true, description = "Id of the counter", type = "java.lang.Integer") Integer counter) { logger.debug("IPX800 action 'resetCounter' called"); - Ipx800v3Handler theHandler = this.handler; - if (theHandler != null) { + if (handler instanceof Ipx800v3Handler theHandler) { theHandler.resetCounter(counter); } else { logger.warn("Method call resetCounter failed because IPX800 action service ThingHandler is null!"); @@ -70,8 +69,7 @@ public class Ipx800Actions implements ThingActions { public void reset( @ActionInput(name = "placeholder", label = "Placeholder", required = false, description = "This parameter is not used", type = "java.lang.Integer") @Nullable Integer placeholder) { logger.debug("IPX800 action 'reset' called"); - Ipx800v3Handler theHandler = this.handler; - if (theHandler != null) { + if (handler instanceof Ipx800v3Handler theHandler) { theHandler.reset(); } else { logger.warn("Method call reset failed because IPX800 action service ThingHandler is null!"); 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 fc1c373eea0..e3779f2e031 100644 --- 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 @@ -146,7 +146,6 @@ public class Ipx800DeviceConnector extends Thread { logger.warn("Exception closing socket: {}", e.getMessage()); } } - } public StatusFile readStatusFile() throws SAXException, IOException {