From fce64ebcc62e0e611dbd041f09bff65fc887d5dc Mon Sep 17 00:00:00 2001 From: Christian Niessner Date: Sun, 6 Sep 2020 23:01:09 +0200 Subject: [PATCH] [tacmi] Additional changes from code review: Thread name, reduced logging output, Exception handling Signed-off-by: Christian Niessner --- .../internal/coe/TACmiCoEBridgeHandler.java | 14 ++++--- .../tacmi/internal/schema/ApiPageParser.java | 39 ++++++++++--------- .../internal/schema/ChangerX2Parser.java | 26 ++++++------- .../internal/schema/TACmiSchemaHandler.java | 12 ++---- 4 files changed, 45 insertions(+), 46 deletions(-) diff --git a/bundles/org.openhab.binding.tacmi/src/main/java/org/openhab/binding/tacmi/internal/coe/TACmiCoEBridgeHandler.java b/bundles/org.openhab.binding.tacmi/src/main/java/org/openhab/binding/tacmi/internal/coe/TACmiCoEBridgeHandler.java index 96cdc6a843941..c12d7211e6a7a 100644 --- a/bundles/org.openhab.binding.tacmi/src/main/java/org/openhab/binding/tacmi/internal/coe/TACmiCoEBridgeHandler.java +++ b/bundles/org.openhab.binding.tacmi/src/main/java/org/openhab/binding/tacmi/internal/coe/TACmiCoEBridgeHandler.java @@ -32,6 +32,7 @@ import org.eclipse.smarthome.core.thing.binding.BaseBridgeHandler; import org.eclipse.smarthome.core.types.Command; import org.eclipse.smarthome.core.types.RefreshType; +import org.openhab.binding.tacmi.internal.TACmiBindingConstants; import org.openhab.binding.tacmi.internal.message.AnalogMessage; import org.openhab.binding.tacmi.internal.message.DigitalMessage; import org.openhab.binding.tacmi.internal.message.Message; @@ -76,8 +77,8 @@ public TACmiCoEBridgeHandler(final Bridge br) { private class ReceiveThread extends Thread { private final Logger logger = LoggerFactory.getLogger(ReceiveThread.class); - ReceiveThread() { - super("tacmi TA C.M.I. CoE ReceiveThread"); + ReceiveThread(String threadName) { + super(threadName); } @Override @@ -121,8 +122,8 @@ public void run() { } } if (!found) { - logger.info("Received CoE-Packet from {} Node {} and we don't have a Thing for!", remoteAddress, - node); + logger.debug("Received CoE-Packet from {} Node {} and we don't have a Thing for!", + remoteAddress, node); } } catch (final IOException e) { if (isInterrupted()) { @@ -167,7 +168,8 @@ public void initialize() { return; } - ReceiveThread reciveThreadNN = new ReceiveThread(); + ReceiveThread reciveThreadNN = new ReceiveThread( + "OH-" + TACmiBindingConstants.BINDING_ID + "-" + getThing().getUID().getAsString()); reciveThreadNN.setDaemon(true); reciveThreadNN.start(); this.receiveThread = reciveThreadNN; @@ -239,7 +241,7 @@ public void dispose() { // caused to stop. receiveThread.join(250); } catch (final InterruptedException e) { - logger.info("Unexpected interrupt in receiveThread.join(): {}", e.getMessage(), e); + logger.debug("Unexpected interrupt in receiveThread.join(): {}", e.getMessage(), e); } this.receiveThread = null; } diff --git a/bundles/org.openhab.binding.tacmi/src/main/java/org/openhab/binding/tacmi/internal/schema/ApiPageParser.java b/bundles/org.openhab.binding.tacmi/src/main/java/org/openhab/binding/tacmi/internal/schema/ApiPageParser.java index 785654c40bf9f..4e07a76814dd4 100644 --- a/bundles/org.openhab.binding.tacmi/src/main/java/org/openhab/binding/tacmi/internal/schema/ApiPageParser.java +++ b/bundles/org.openhab.binding.tacmi/src/main/java/org/openhab/binding/tacmi/internal/schema/ApiPageParser.java @@ -20,6 +20,8 @@ import java.util.Map; import java.util.Map.Entry; import java.util.Set; +import java.util.concurrent.ExecutionException; +import java.util.concurrent.TimeoutException; import org.attoparser.ParseException; import org.attoparser.simple.AbstractSimpleMarkupHandler; @@ -119,7 +121,7 @@ public void handleStandaloneElement(final @Nullable String elementName, final @Nullable Map attributes, final boolean minimized, final int line, final int col) throws ParseException { - logger.info("Unexpected StandaloneElement in {}:{}: {} [{}]", line, col, elementName, attributes); + logger.debug("Unexpected StandaloneElement in {}:{}: {} [{}]", line, col, elementName, attributes); } @Override @@ -165,7 +167,7 @@ public void handleOpenElement(final @Nullable String elementName, final @Nullabl && "span".equals(elementName)) { // ignored... } else { - logger.info("Unexpected OpenElement in {}:{}: {} [{}]", line, col, elementName, attributes); + logger.debug("Unexpected OpenElement in {}:{}: {} [{}]", line, col, elementName, attributes); } } @@ -184,7 +186,8 @@ public void handleCloseElement(final @Nullable String elementName, final int lin int lids = sb.lastIndexOf(":"); int fsp = sb.indexOf(" "); if (fsp < 0 || lids < 0 || fsp > lids) { - logger.info("Invalid format for setting {}:{}:{} [{}] : {}", id, line, col, this.fieldType, sb); + logger.debug("Invalid format for setting {}:{}:{} [{}] : {}", id, line, col, this.fieldType, + sb); } else { String shortName = sb.substring(0, fsp).trim(); String description = sb.substring(fsp + 1, lids).trim(); @@ -196,7 +199,7 @@ public void handleCloseElement(final @Nullable String elementName, final int lin int fsp = sbt.indexOf(" "); if (fsp < 0) { - logger.info("Invalid format for setting {}:{}:{} [{}] : {}", id, line, col, this.fieldType, + logger.debug("Invalid format for setting {}:{}:{} [{}] : {}", id, line, col, this.fieldType, sbt); } else { String shortName = sbt.substring(0, fsp).trim(); @@ -206,47 +209,47 @@ public void handleCloseElement(final @Nullable String elementName, final int lin } else if (this.fieldType == FieldType.IGNORE) { // ignore } else { - logger.info("Unhandled setting {}:{}:{} [{}] : {}", id, line, col, this.fieldType, sb); + logger.debug("Unhandled setting {}:{}:{} [{}] : {}", id, line, col, this.fieldType, sb); } } } else if (this.parserState == ParserState.DATA_ENTRY && this.fieldType == FieldType.BUTTON && "span".equals(elementName)) { // ignored... } else { - logger.info("Unexpected CloseElement in {}:{}: {}", line, col, elementName); + logger.debug("Unexpected CloseElement in {}:{}: {}", line, col, elementName); } } @Override public void handleAutoCloseElement(final @Nullable String elementName, final int line, final int col) throws ParseException { - logger.info("Unexpected AutoCloseElement in {}:{}: {}", line, col, elementName); + logger.debug("Unexpected AutoCloseElement in {}:{}: {}", line, col, elementName); } @Override public void handleUnmatchedCloseElement(final @Nullable String elementName, final int line, final int col) throws ParseException { - logger.info("Unexpected UnmatchedCloseElement in {}:{}: {}", line, col, elementName); + logger.debug("Unexpected UnmatchedCloseElement in {}:{}: {}", line, col, elementName); } @Override public void handleDocType(final @Nullable String elementName, final @Nullable String publicId, final @Nullable String systemId, final @Nullable String internalSubset, final int line, final int col) throws ParseException { - logger.info("Unexpected DocType in {}:{}: {}/{}/{}/{}", line, col, elementName, publicId, systemId, + logger.debug("Unexpected DocType in {}:{}: {}/{}/{}/{}", line, col, elementName, publicId, systemId, internalSubset); } @Override public void handleComment(final char @Nullable [] buffer, final int offset, final int len, final int line, final int col) throws ParseException { - logger.info("Unexpected comment in {}:{}: {}", line, col, new String(buffer, offset, len)); + logger.debug("Unexpected comment in {}:{}: {}", line, col, new String(buffer, offset, len)); } @Override public void handleCDATASection(final char @Nullable [] buffer, final int offset, final int len, final int line, final int col) throws ParseException { - logger.info("Unexpected CDATA in {}:{}: {}", line, col, new String(buffer, offset, len)); + logger.debug("Unexpected CDATA in {}:{}: {}", line, col, new String(buffer, offset, len)); } @Override @@ -268,20 +271,20 @@ public void handleText(final char @Nullable [] buffer, final int offset, final i // single newline - ignore/drop it... } else { String msg = new String(buffer, offset, len).replace("\n", "\\n").replace("\r", "\\r"); - logger.info("Unexpected Text {}:{}: ParserState: {} ({}) `{}`", line, col, parserState, len, msg); + logger.debug("Unexpected Text {}:{}: ParserState: {} ({}) `{}`", line, col, parserState, len, msg); } } @Override public void handleXmlDeclaration(final @Nullable String version, final @Nullable String encoding, final @Nullable String standalone, final int line, final int col) throws ParseException { - logger.info("Unexpected XML Declaration {}:{}: {} {} {}", line, col, version, encoding, standalone); + logger.debug("Unexpected XML Declaration {}:{}: {} {} {}", line, col, version, encoding, standalone); } @Override public void handleProcessingInstruction(final @Nullable String target, final @Nullable String content, final int line, final int col) throws ParseException { - logger.info("Unexpected ProcessingInstruction {}:{}: {} {}", line, col, target, content); + logger.debug("Unexpected ProcessingInstruction {}:{}: {} {}", line, col, target, content); } private void getApiPageEntry(@Nullable String id2, int line, int col, String shortName, String description, @@ -415,8 +418,8 @@ private void getApiPageEntry(@Nullable String id2, int line, int col, String sho URI uri = this.taCmiSchemaHandler.buildUri("INCLUDE/changerx2.cgi?sadrx2=" + address); final ChangerX2Parser pp = this.taCmiSchemaHandler.parsePage(uri, new ChangerX2Parser(shortName)); cx2e = pp.getParsedEntry(); - } catch (final Exception ex) { - logger.error("Error loading API Scheme: {} ", ex.getMessage(), ex); + } catch (final ParseException | InterruptedException | TimeoutException | ExecutionException ex) { + logger.warn("Error loading API Scheme: {} ", ex.getMessage(), ex); } } if (channel == null) { @@ -462,9 +465,7 @@ private void getApiPageEntry(@Nullable String id2, int line, int col, String sho ChannelType ct = ChannelTypeBuilder .state(new ChannelTypeUID(TACmiBindingConstants.BINDING_ID, shortName), shortName, itemType) .withDescription("Auto-created for " + shortName) - .withStateDescription(sdb.build().toStateDescription()) - // .withCategory("CategoryName") can we do something useful ? - .build(); + .withStateDescription(sdb.build().toStateDescription()).build(); channelTypeProvider.addChannelType(ct); channelBuilder.withType(ct.getUID()); } else { diff --git a/bundles/org.openhab.binding.tacmi/src/main/java/org/openhab/binding/tacmi/internal/schema/ChangerX2Parser.java b/bundles/org.openhab.binding.tacmi/src/main/java/org/openhab/binding/tacmi/internal/schema/ChangerX2Parser.java index ec09f857a3627..3270c1e5d22fb 100644 --- a/bundles/org.openhab.binding.tacmi/src/main/java/org/openhab/binding/tacmi/internal/schema/ChangerX2Parser.java +++ b/bundles/org.openhab.binding.tacmi/src/main/java/org/openhab/binding/tacmi/internal/schema/ChangerX2Parser.java @@ -78,7 +78,7 @@ public void handleDocumentEnd(final long endTimeNanos, final long totalTimeNanos public void handleStandaloneElement(final String elementName, final Map attributes, final boolean minimized, final int line, final int col) throws ParseException { - logger.info("Error parsing options for {}: Unexpected StandaloneElement in {}{}: {} [{}]", channelName, line, + logger.debug("Error parsing options for {}: Unexpected StandaloneElement in {}{}: {} [{}]", channelName, line, col, elementName, attributes); } @@ -128,7 +128,7 @@ public void handleOpenElement(final String elementName, final Map scheduledFuture = this.scheduledFuture; if (scheduledFuture != null) { - try { - scheduledFuture.cancel(true); - this.scheduledFuture = null; - } catch (final Exception e) { - // swallow this - } + scheduledFuture.cancel(true); + this.scheduledFuture = null; } super.dispose(); }