From: Gaël L'hopital Date: Sat, 15 Oct 2022 08:02:01 +0000 (+0200) Subject: [gce] File based items are not updated (#13545) X-Git-Url: https://git.basschouten.com/?a=commitdiff_plain;h=bdbd5a1ede03fc2799795cab88c154a9bd3b36f8;p=openhab-addons.git [gce] File based items are not updated (#13545) * [gce] Correcting non updated input channels Signed-off-by: clinique --- diff --git a/bundles/org.openhab.binding.gce/src/main/java/org/openhab/binding/gce/internal/GCEBindingConstants.java b/bundles/org.openhab.binding.gce/src/main/java/org/openhab/binding/gce/internal/GCEBindingConstants.java index 16a687aee1..c16b9de025 100644 --- a/bundles/org.openhab.binding.gce/src/main/java/org/openhab/binding/gce/internal/GCEBindingConstants.java +++ b/bundles/org.openhab.binding.gce/src/main/java/org/openhab/binding/gce/internal/GCEBindingConstants.java @@ -38,6 +38,4 @@ public class GCEBindingConstants { public static final String EVENT_SHORT_PRESS = "SHORT_PRESS"; public static final String EVENT_LONG_PRESS = "LONG_PRESS"; public static final String EVENT_PULSE = "PULSE"; - - // Adressable thing } 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 0673230b7f..5378062f5d 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 @@ -110,7 +110,7 @@ public class Ipx800DeviceConnector extends Thread { /** * Stop the device thread */ - public void destroyAndExit() { + public void dispose() { interrupt(); disconnect(); } @@ -161,7 +161,7 @@ public class Ipx800DeviceConnector extends Thread { try { Thread.sleep(DEFAULT_RECONNECT_TIMEOUT_MS); } catch (InterruptedException e) { - destroyAndExit(); + dispose(); } } 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 04962adf24..0a1f8452ef 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 @@ -29,7 +29,6 @@ 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; @@ -56,7 +55,6 @@ import org.openhab.core.thing.ThingStatusDetail; import org.openhab.core.thing.binding.BaseThingHandler; import org.openhab.core.thing.binding.ThingHandlerService; import org.openhab.core.thing.binding.builder.ChannelBuilder; -import org.openhab.core.thing.binding.builder.ThingBuilder; import org.openhab.core.thing.type.ChannelKind; import org.openhab.core.thing.type.ChannelTypeUID; import org.openhab.core.types.Command; @@ -78,14 +76,11 @@ public class Ipx800v3Handler extends BaseThingHandler implements Ipx800EventList private final Logger logger = LoggerFactory.getLogger(Ipx800v3Handler.class); - private @NonNullByDefault({}) Ipx800Configuration configuration; - private @NonNullByDefault({}) Ipx800DeviceConnector connector; - private @NonNullByDefault({}) StatusFileInterpreter statusFile; - + private Optional connector = Optional.empty(); private Optional parser = Optional.empty(); private Optional> refreshJob = Optional.empty(); - private final Map portDatas = new HashMap<>(); + private final Map portDatas = new HashMap<>(); private class LongPressEvaluator implements Runnable { private final ZonedDateTime referenceTime; @@ -115,36 +110,37 @@ public class Ipx800v3Handler extends BaseThingHandler implements Ipx800EventList @Override public void initialize() { - configuration = getConfigAs(Ipx800Configuration.class); - logger.debug("Initializing IPX800 handler for uid '{}'", getThing().getUID()); - statusFile = new StatusFileInterpreter(configuration.hostname, this); + Ipx800Configuration config = getConfigAs(Ipx800Configuration.class); + StatusFileInterpreter statusFile = new StatusFileInterpreter(config.hostname, this); if (thing.getProperties().isEmpty()) { - discoverAttributes(); + 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))); } List channels = new ArrayList<>(getThing().getChannels()); - ThingBuilder thingBuilder = editThing(); PortDefinition.asStream().forEach(portDefinition -> { int nbElements = statusFile.getMaxNumberofNodeType(portDefinition); for (int i = 0; i < nbElements; i++) { - createChannels(portDefinition, i, channels); + ChannelUID portChannelUID = createChannels(portDefinition, i, channels); + portDatas.put(portChannelUID.getId(), new PortData()); } }); - thingBuilder.withChannels(channels); - updateThing(thingBuilder.build()); - connector = new Ipx800DeviceConnector(configuration.hostname, configuration.portNumber, getThing().getUID()); - parser = Optional.of(new M2MMessageParser(connector, this)); + 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, configuration.pullInterval, - TimeUnit.MILLISECONDS)); + refreshJob = Optional.of( + scheduler.scheduleWithFixedDelay(statusFile::read, 3000, config.pullInterval, TimeUnit.MILLISECONDS)); - connector.start(); + connector.get().start(); } @Override @@ -152,25 +148,15 @@ public class Ipx800v3Handler extends BaseThingHandler implements Ipx800EventList refreshJob.ifPresent(job -> job.cancel(true)); refreshJob = Optional.empty(); - if (connector != null) { - connector.destroyAndExit(); - } + connector.ifPresent(Ipx800DeviceConnector::dispose); + connector = Optional.empty(); + parser = Optional.empty(); - portDatas.values().stream().forEach(portData -> { - if (portData != null) { - portData.dispose(); - } - }); + portDatas.values().stream().forEach(PortData::dispose); super.dispose(); } - protected void discoverAttributes() { - 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))); - } - private void addIfChannelAbsent(ChannelBuilder channelBuilder, List channels) { Channel newChannel = channelBuilder.build(); if (channels.stream().noneMatch(c -> c.getUID().equals(newChannel.getUID()))) { @@ -178,7 +164,7 @@ public class Ipx800v3Handler extends BaseThingHandler implements Ipx800EventList } } - private void createChannels(PortDefinition portDefinition, int portIndex, List channels) { + private ChannelUID createChannels(PortDefinition portDefinition, int portIndex, List channels) { String ndx = Integer.toString(portIndex + 1); String advancedChannelTypeName = portDefinition.toString() + (portDefinition.isAdvanced(portIndex) ? "Advanced" : ""); @@ -210,9 +196,12 @@ public class Ipx800v3Handler extends BaseThingHandler implements Ipx800EventList .withLabel("Relay " + ndx).withType(channelType), channels); break; } + addIfChannelAbsent(ChannelBuilder.create(new ChannelUID(groupUID, ndx + "-duration"), "Number:Time") .withType(new ChannelTypeUID(BINDING_ID, CHANNEL_LAST_STATE_DURATION)) .withLabel("Previous state duration " + ndx), channels); + + return mainChannelUID; } @Override @@ -258,7 +247,7 @@ public class Ipx800v3Handler extends BaseThingHandler implements Ipx800EventList return; } logger.debug("About to update port '{}' with data '{}'", port, value); - State state = UnDefType.UNDEF; + State state = UnDefType.NULL; switch (portDefinition) { case COUNTER: state = new DecimalType(value); @@ -268,7 +257,7 @@ public class Ipx800v3Handler extends BaseThingHandler implements Ipx800EventList break; case ANALOG: state = new DecimalType(value); - updateState(channelId + PROPERTY_SEPARATOR + CHANNEL_VOLTAGE, + updateIfLinked(channelId + PROPERTY_SEPARATOR + CHANNEL_VOLTAGE, new QuantityType<>(value * ANALOG_SAMPLING, Units.VOLT)); break; case CONTACT: @@ -303,9 +292,9 @@ public class Ipx800v3Handler extends BaseThingHandler implements Ipx800EventList break; } - updateState(channelId, state); + updateIfLinked(channelId, state); if (!portData.isInitializing()) { - updateState(channelId + PROPERTY_SEPARATOR + CHANNEL_LAST_STATE_DURATION, + updateIfLinked(channelId + PROPERTY_SEPARATOR + CHANNEL_LAST_STATE_DURATION, new QuantityType<>(sinceLastChange / 1000, Units.SECOND)); } portData.setData(value, now); @@ -317,6 +306,12 @@ public class Ipx800v3Handler extends BaseThingHandler implements Ipx800EventList } } + private void updateIfLinked(String channelId, State state) { + if (isLinked(channelId)) { + updateState(channelId, state); + } + } + protected void triggerPushButtonChannel(Channel channel, String event) { logger.debug("Triggering event '{}' on channel '{}'", event, channel.getUID()); triggerChannel(channel.getUID().getId() + PROPERTY_SEPARATOR + TRIGGER_CONTACT, event); @@ -342,32 +337,10 @@ public class Ipx800v3Handler extends BaseThingHandler implements Ipx800EventList logger.debug("Can not handle command '{}' on channel '{}'", command, channelUID); } - @Override - public void channelLinked(ChannelUID channelUID) { - logger.debug("channelLinked: {}", channelUID); - final String channelId = channelUID.getId(); - if (isValidPortId(channelUID)) { - Channel channel = thing.getChannel(channelUID); - if (channel != null) { - PortData data = new PortData(); - portDatas.put(channelId, data); - } - } - } - private boolean isValidPortId(ChannelUID channelUID) { return channelUID.getIdWithoutGroup().chars().allMatch(Character::isDigit); } - @Override - public void channelUnlinked(ChannelUID channelUID) { - super.channelUnlinked(channelUID); - PortData portData = portDatas.remove(channelUID.getId()); - if (portData != null) { - portData.dispose(); - } - } - public void resetCounter(int counter) { parser.ifPresent(p -> p.resetCounter(counter)); } 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 3bccd0ea0e..837eb1740e 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.eclipse.jdt.annotation.Nullable; import org.openhab.binding.gce.internal.handler.Ipx800DeviceConnector; import org.openhab.binding.gce.internal.handler.Ipx800EventListener; import org.slf4j.Logger; @@ -35,11 +34,11 @@ public class M2MMessageParser { private final Logger logger = LoggerFactory.getLogger(M2MMessageParser.class); private final Ipx800DeviceConnector connector; - private final @Nullable Ipx800EventListener listener; + private final Ipx800EventListener listener; private String expectedResponse = ""; - public M2MMessageParser(Ipx800DeviceConnector connector, @Nullable Ipx800EventListener listener) { + public M2MMessageParser(Ipx800DeviceConnector connector, Ipx800EventListener listener) { this.connector = connector; this.listener = listener; connector.setParser(this); @@ -87,9 +86,7 @@ public class M2MMessageParser { private void setStatus(String port, double value) { logger.debug("Received {} : {}", port, value); - if (listener != null) { - listener.dataReceived(port, value); - } + listener.dataReceived(port, value); } public void setExpectedResponse(String expectedResponse) { @@ -125,9 +122,7 @@ public class M2MMessageParser { public void errorOccurred(Exception e) { logger.warn("Error received from connector : {}", e.getMessage()); - if (listener != null) { - listener.errorOccurred(e); - } + listener.errorOccurred(e); } public void resetPLC() { 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 57b520ed2c..4b7a63332c 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 @@ -17,6 +17,7 @@ import java.io.IOException; import java.io.InputStream; import java.util.Comparator; import java.util.List; +import java.util.Optional; import java.util.stream.Collectors; import java.util.stream.IntStream; @@ -25,7 +26,6 @@ import javax.xml.parsers.DocumentBuilderFactory; import javax.xml.parsers.ParserConfigurationException; import org.eclipse.jdt.annotation.NonNullByDefault; -import org.eclipse.jdt.annotation.Nullable; import org.openhab.binding.gce.internal.handler.Ipx800EventListener; import org.openhab.core.io.net.http.HttpUtil; import org.slf4j.Logger; @@ -44,47 +44,53 @@ import org.xml.sax.SAXException; @NonNullByDefault public class StatusFileInterpreter { private static final String URL_TEMPLATE = "http://%s/globalstatus.xml"; + private final Logger logger = LoggerFactory.getLogger(StatusFileInterpreter.class); - private final String hostname; - private @Nullable Document doc; + private final DocumentBuilder builder; + private final String url; private final Ipx800EventListener listener; + private Optional doc = Optional.empty(); + public static enum StatusEntry { VERSION, CONFIG_MAC; } public StatusFileInterpreter(String hostname, Ipx800EventListener listener) { - this.hostname = hostname; + this.url = String.format(URL_TEMPLATE, hostname); this.listener = listener; - } - - public void read() { + DocumentBuilderFactory factory = DocumentBuilderFactory.newInstance(); + // see https://cheatsheetseries.owasp.org/cheatsheets/XML_External_Entity_Prevention_Cheat_Sheet.html try { - DocumentBuilderFactory factory = DocumentBuilderFactory.newInstance(); - // see https://cheatsheetseries.owasp.org/cheatsheets/XML_External_Entity_Prevention_Cheat_Sheet.html 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); factory.setXIncludeAware(false); factory.setExpandEntityReferences(false); - DocumentBuilder builder = factory.newDocumentBuilder(); - String statusPage = HttpUtil.executeUrl("GET", String.format(URL_TEMPLATE, hostname), 5000); + builder = factory.newDocumentBuilder(); + } catch (ParserConfigurationException e) { + logger.warn("Error initializing StatusFileInterpreter :{}", e.getMessage()); + throw new IllegalArgumentException(e); + } + } + + public void read() { + try { + String statusPage = HttpUtil.executeUrl("GET", url, 5000); InputStream inputStream = new ByteArrayInputStream(statusPage.getBytes()); Document document = builder.parse(inputStream); document.getDocumentElement().normalize(); - doc = document; - pushDatas(); inputStream.close(); - } catch (IOException | SAXException | ParserConfigurationException e) { + this.doc = Optional.of(document); + pushDatas(); + } catch (IOException | SAXException e) { logger.warn("Unable to read IPX800 status page : {}", e.getMessage()); - doc = null; } } private void pushDatas() { - Element root = getRoot(); - if (root != null) { + getRoot().ifPresent(root -> { PortDefinition.asStream().forEach(portDefinition -> { List xmlNodes = getMatchingNodes(root.getChildNodes(), portDefinition.getNodeName()); xmlNodes.forEach(xmlNode -> { @@ -94,40 +100,29 @@ public class StatusFileInterpreter { listener.dataReceived(String.format("%s%d", portDefinition.getPortName(), portNum), value); }); }); - } + }); } public String getElement(StatusEntry entry) { - Element root = getRoot(); - if (root != null) { - return root.getElementsByTagName(entry.name().toLowerCase()).item(0).getTextContent(); - } else { - return ""; - } + return getRoot().map(root -> root.getElementsByTagName(entry.name().toLowerCase()).item(0).getTextContent()) + .orElse(""); } 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(o -> o.getNodeName())).collect(Collectors.toList()); + .filter(node -> node.getNodeName().startsWith(criteria)).sorted(Comparator.comparing(Node::getNodeName)) + .collect(Collectors.toList()); } public int getMaxNumberofNodeType(PortDefinition portDefinition) { - Element root = getRoot(); - if (root != null) { - List filteredNodes = getMatchingNodes(root.getChildNodes(), portDefinition.getNodeName()); - return filteredNodes.size(); - } - return 0; + return getRoot().map(root -> getMatchingNodes(root.getChildNodes(), portDefinition.getNodeName()).size()) + .orElse(0); } - private @Nullable Element getRoot() { - if (doc == null) { + private Optional getRoot() { + if (doc.isEmpty()) { read(); } - if (doc != null) { - return doc.getDocumentElement(); - } - return null; + return Optional.ofNullable(doc.map(Document::getDocumentElement).orElse(null)); } }