Skip to content

Commit

Permalink
[tacmi] Additional changes from code review: Thread name, reduced log…
Browse files Browse the repository at this point in the history 
…ging output, Exception handling

Signed-off-by: Christian Niessner <[email protected]>
  • Loading branch information
@marvkis
marvkis committed Sep 6, 2020
1 parent fc3c26b commit fce64eb
Show file tree
Hide file tree
Showing 4 changed files with 45 additions and 46 deletions.
14 changes: 8 additions & 6 deletions ...ing.tacmi/src/main/java/org/openhab/binding/tacmi/internal/coe/TACmiCoEBridgeHandler.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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()) {
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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;
}
Expand Down
39 changes: 20 additions & 19 deletions ....binding.tacmi/src/main/java/org/openhab/binding/tacmi/internal/schema/ApiPageParser.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -119,7 +121,7 @@ public void handleStandaloneElement(final @Nullable String elementName,
final @Nullable Map<String, String> 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
Expand Down Expand Up @@ -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);
}
}

Expand All @@ -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();
Expand All @@ -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();
Expand All @@ -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
Expand All @@ -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,
Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -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 {
Expand Down
26 changes: 13 additions & 13 deletions ...inding.tacmi/src/main/java/org/openhab/binding/tacmi/internal/schema/ChangerX2Parser.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,7 @@ public void handleDocumentEnd(final long endTimeNanos, final long totalTimeNanos
public void handleStandaloneElement(final String elementName, final Map<String, String> 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);
}

Expand Down Expand Up @@ -128,7 +128,7 @@ public void handleOpenElement(final String elementName, final Map<String, String
this.curOptionValue = new StringBuilder();
this.curOptionId = attributes == null ? null : attributes.get("value");
} else {
logger.info("Error parsing options for {}: Unexpected OpenElement in {}:{}: {} [{}]", channelName, line,
logger.debug("Error parsing options for {}: Unexpected OpenElement in {}:{}: {} [{}]", channelName, line,
col, elementName, attributes);
}
}
Expand All @@ -149,54 +149,54 @@ public void handleCloseElement(final @Nullable String elementName, final int lin
this.curOptionId = null;
if (value != null) {
if (id == null || id.trim().isEmpty()) {
logger.info("Error parsing options for {}: Got option with empty 'value' in {}:{}: [{}]",
logger.debug("Error parsing options for {}: Got option with empty 'value' in {}:{}: [{}]",
channelName, line, col, value);
return;
}
// we use the value as key and the id as value, as we have to map from the value to the id...
@Nullable
String prev = this.options.put(value, id);
if (prev != null && !prev.equals(value)) {
logger.info("Error parsing options for {}: Got duplicate options in {}:{} for {}: {} and {}",
logger.debug("Error parsing options for {}: Got duplicate options in {}:{} for {}: {} and {}",
channelName, line, col, value, prev, id);
}
}
} else {
logger.info("Error parsing options for {}: Unexpected CloseElement in {}:{}: {}", channelName, line, col,
logger.debug("Error parsing options for {}: Unexpected CloseElement in {}:{}: {}", channelName, 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
Expand All @@ -219,21 +219,21 @@ public void handleText(final char @Nullable [] buffer, final int offset, final i
// this is a label next to the value input field - we currently have no use for it so
// it's dropped...
} else {
logger.info("Error parsing options for {}: Unexpected Text {}:{}: (ctx: {} len: {}) '{}' ",
logger.debug("Error parsing options for {}: Unexpected Text {}:{}: (ctx: {} len: {}) '{}' ",
this.channelName, line, col, this.parserState, len, new String(buffer, offset, len));
}
}

@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);
}

@Nullable
Expand Down
12 changes: 4 additions & 8 deletions ...ing.tacmi/src/main/java/org/openhab/binding/tacmi/internal/schema/TACmiSchemaHandler.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -265,11 +265,11 @@ public void handleCommand(final ChannelUID channelUID, final Command command) {
e.setLastState((State) command);
updateState(channelUID, (State) command);
} else {
logger.error("Error sending update for {} = {}: {} {}", channelUID, command, res.getStatus(),
logger.warn("Error sending update for {} = {}: {} {}", channelUID, command, res.getStatus(),
res.getReason());
}
} catch (InterruptedException | TimeoutException | ExecutionException ex) {
logger.error("Error sending update for {} = {}: {}", channelUID, command, ex.getMessage(), ex);
logger.warn("Error sending update for {} = {}: {}", channelUID, command, ex.getMessage());
}
}

Expand All @@ -283,12 +283,8 @@ protected void updateState(final ChannelUID channelUID, final State state) {
public void dispose() {
final ScheduledFuture<?> 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();
}
Expand Down

0 comments on commit fce64eb

Please sign in to comment.