From 99838c4664a40317bcb6847a40f071a77442b89b Mon Sep 17 00:00:00 2001 From: Arne Seime Date: Mon, 22 Jul 2024 10:12:13 +0200 Subject: [PATCH] Some cleanup and improved logging --- .../internal/comm/AbstractFrameHelper.java | 18 +++++++++++++++--- .../internal/comm/ESPHomeConnection.java | 7 ++----- .../internal/comm/EncryptedFrameHelper.java | 16 ++++++++-------- .../internal/comm/PlainTextFrameHelper.java | 2 +- .../internal/handler/ESPHomeHandler.java | 7 ++++++- .../internal/message/LightMessageHandler.java | 2 -- 6 files changed, 32 insertions(+), 20 deletions(-) diff --git a/src/main/java/no/seime/openhab/binding/esphome/internal/comm/AbstractFrameHelper.java b/src/main/java/no/seime/openhab/binding/esphome/internal/comm/AbstractFrameHelper.java index cb36fcd..6b5bdfa 100644 --- a/src/main/java/no/seime/openhab/binding/esphome/internal/comm/AbstractFrameHelper.java +++ b/src/main/java/no/seime/openhab/binding/esphome/internal/comm/AbstractFrameHelper.java @@ -7,6 +7,7 @@ import java.net.InetSocketAddress; import java.nio.ByteBuffer; +import org.apache.commons.lang3.StringUtils; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -24,6 +25,12 @@ public abstract class AbstractFrameHelper { protected CommunicationListener listener; protected ByteBuffer internalBuffer = ByteBuffer.allocate(READ_BUFFER_SIZE * 2); protected ESPHomeConnection connection; + protected String logPrefix; + + public AbstractFrameHelper(String logPrefix, CommunicationListener listener) { + this.logPrefix = logPrefix; + this.listener = listener; + } protected static byte[] concatArrays(byte[] length, byte[] additionalLength) { byte[] result = new byte[length.length + additionalLength.length]; @@ -87,7 +94,7 @@ protected byte[] readBytes(int numBytes) { } protected void decodeProtoMessage(int messageType, byte[] bytes) { - logger.trace("Received packet of type {} with data {}", messageType, bytes); + logger.trace("[{}] Received packet of type {} with data {}", logPrefix, messageType, bytes); try { Method parseMethod = messageTypeToClassConverter.getMethod(messageType); @@ -96,11 +103,11 @@ protected void decodeProtoMessage(int messageType, byte[] bytes) { if (invoke != null) { listener.onPacket(invoke); } else { - logger.warn("Received null packet of type {}", parseMethod); + logger.warn("[{}] Received null packet of type {}", logPrefix, parseMethod); } } } catch (Exception e) { - logger.warn("Error parsing packet", e); + logger.warn("[{}] Error parsing packet", logPrefix, e); listener.onParseError(CommunicationError.PACKET_ERROR); } } @@ -121,6 +128,11 @@ public void onParseError(CommunicationError error) { } public void send(GeneratedMessageV3 message) throws ProtocolAPIError { + if (logger.isDebugEnabled()) { + // ToString method costs a bit + logger.debug("[{}] Sending message type {} with content '{}'", logPrefix, + message.getClass().getSimpleName(), StringUtils.trimToEmpty(message.toString())); + } connection.send(encodeFrame(message)); } } diff --git a/src/main/java/no/seime/openhab/binding/esphome/internal/comm/ESPHomeConnection.java b/src/main/java/no/seime/openhab/binding/esphome/internal/comm/ESPHomeConnection.java index d507d19..1daf2bf 100644 --- a/src/main/java/no/seime/openhab/binding/esphome/internal/comm/ESPHomeConnection.java +++ b/src/main/java/no/seime/openhab/binding/esphome/internal/comm/ESPHomeConnection.java @@ -23,13 +23,10 @@ public class ESPHomeConnection { private final Logger logger = LoggerFactory.getLogger(ESPHomeConnection.class); - - private SocketChannel socketChannel; - private final AbstractFrameHelper frameHelper; private final ConnectionSelector connectionSelector; - private final String logPrefix; + private SocketChannel socketChannel; public ESPHomeConnection(ConnectionSelector connectionSelector, AbstractFrameHelper frameHelper, String logPrefix) { this.frameHelper = frameHelper; @@ -40,7 +37,7 @@ public ESPHomeConnection(ConnectionSelector connectionSelector, AbstractFrameHel public synchronized void send(ByteBuffer buffer) throws ProtocolAPIError { try { while (buffer.hasRemaining()) { - logger.trace("[{}] Writing data", logPrefix); + logger.trace("[{}] Writing data {} bytes", logPrefix, buffer.remaining()); socketChannel.write(buffer); } } catch (IOException e) { diff --git a/src/main/java/no/seime/openhab/binding/esphome/internal/comm/EncryptedFrameHelper.java b/src/main/java/no/seime/openhab/binding/esphome/internal/comm/EncryptedFrameHelper.java index 65e9db3..be4c05c 100644 --- a/src/main/java/no/seime/openhab/binding/esphome/internal/comm/EncryptedFrameHelper.java +++ b/src/main/java/no/seime/openhab/binding/esphome/internal/comm/EncryptedFrameHelper.java @@ -41,13 +41,20 @@ public class EncryptedFrameHelper extends AbstractFrameHelper { public EncryptedFrameHelper(ConnectionSelector connectionSelector, CommunicationListener listener, String encryptionKeyBase64, @Nullable String expectedServername, String logPrefix) { + super(logPrefix, listener); this.encryptionKeyBase64 = encryptionKeyBase64; this.expectedServername = expectedServername; - this.listener = listener; connection = new ESPHomeConnection(connectionSelector, this, logPrefix); } + private static short bytesToShort(final byte[] data) { + short value = (short) (data[0] & 0xff); + value <<= 8; + value |= data[1] & 0xff; + return value; + } + @Override public void connect(InetSocketAddress espHomeAddress) throws ProtocolException { try { @@ -222,13 +229,6 @@ private byte[] decryptPacket(byte[] msg) throws Exception { return result; } - private static short bytesToShort(final byte[] data) { - short value = (short) (data[0] & 0xff); - value <<= 8; - value |= data[1] & 0xff; - return value; - } - private enum NoiseProtocolState { HELLO, HANDSHAKE, diff --git a/src/main/java/no/seime/openhab/binding/esphome/internal/comm/PlainTextFrameHelper.java b/src/main/java/no/seime/openhab/binding/esphome/internal/comm/PlainTextFrameHelper.java index 092e3ad..a088c73 100644 --- a/src/main/java/no/seime/openhab/binding/esphome/internal/comm/PlainTextFrameHelper.java +++ b/src/main/java/no/seime/openhab/binding/esphome/internal/comm/PlainTextFrameHelper.java @@ -28,7 +28,7 @@ public class PlainTextFrameHelper extends AbstractFrameHelper { public PlainTextFrameHelper(ConnectionSelector connectionSelector, CommunicationListener listener, String logPrefix) { - this.listener = listener; + super(logPrefix, listener); connection = new ESPHomeConnection(connectionSelector, this, logPrefix); } diff --git a/src/main/java/no/seime/openhab/binding/esphome/internal/handler/ESPHomeHandler.java b/src/main/java/no/seime/openhab/binding/esphome/internal/handler/ESPHomeHandler.java index 71b3c8f..3beb3fb 100644 --- a/src/main/java/no/seime/openhab/binding/esphome/internal/handler/ESPHomeHandler.java +++ b/src/main/java/no/seime/openhab/binding/esphome/internal/handler/ESPHomeHandler.java @@ -19,6 +19,7 @@ import java.util.concurrent.ScheduledFuture; import java.util.concurrent.TimeUnit; +import org.apache.commons.lang3.StringUtils; import org.eclipse.jdt.annotation.NonNull; import org.eclipse.jdt.annotation.NonNullByDefault; import org.eclipse.jdt.annotation.Nullable; @@ -324,7 +325,11 @@ private void remoteDisconnect() { } private void handleConnected(GeneratedMessageV3 message) throws ProtocolAPIError { - logger.debug("[{}] Received message {}", logPrefix, message); + if (logger.isDebugEnabled()) { + // ToString method costs a bit + logger.debug("[{}] Received message type {} with content '{}'", logPrefix, + message.getClass().getSimpleName(), StringUtils.trimToEmpty(message.toString())); + } if (message instanceof DeviceInfoResponse rsp) { Map props = new HashMap<>(); props.put("esphome_version", rsp.getEsphomeVersion()); diff --git a/src/main/java/no/seime/openhab/binding/esphome/internal/message/LightMessageHandler.java b/src/main/java/no/seime/openhab/binding/esphome/internal/message/LightMessageHandler.java index 86bc339..a951066 100644 --- a/src/main/java/no/seime/openhab/binding/esphome/internal/message/LightMessageHandler.java +++ b/src/main/java/no/seime/openhab/binding/esphome/internal/message/LightMessageHandler.java @@ -178,11 +178,9 @@ public void handleState(LightStateResponse rsp) { handler.updateState(channel.getUID(), hsbType); } else if (capabilities.contains(LightColorCapability.BRIGHTNESS)) { - // Convert to color PercentType percentType = new PercentType((int) (rsp.getState() ? rsp.getBrightness() * 100 : 0)); handler.updateState(channel.getUID(), percentType); } else if (capabilities.contains(LightColorCapability.ON_OFF)) { - // Convert to color OnOffType onOffType = rsp.getState() ? OnOffType.ON : OnOffType.OFF; handler.updateState(channel.getUID(), onOffType); }