Some cleanup and improved logging

seime · Jul 22, 2024 · 99838c4 · 99838c4
1 parent b1b703a
commit 99838c4
Show file tree

Hide file tree

Showing 6 changed files with 32 additions and 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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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<String, String> 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
@@ -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);
  }