[fineoffsetweatherstation] Fix wrong handling temperature reading for of WH34

bundles/org.openhab.binding.fineoffsetweatherstation/README.md

@@ -19,7 +19,7 @@ Here is a product picture of how this Weather Station looks like:
 
 ![WH2650](doc/WH2650.png)
 
-This binding works offline by [implementing the wire protocol](https://osswww.ecowitt.net/uploads/20210716/WN1900%20GW1000,1100%20WH2680,2650%20telenet%20v1.6.0%20.pdf) of the WiFi gateway device.
+This binding works offline by [implementing the wire protocol](https://osswww.ecowitt.net/uploads/20220407/WN1900%20GW1000,1100%20WH2680,2650%20telenet%20v1.6.4.pdf) of the WiFi gateway device.
 
 ## Discussion
 

bundles/org.openhab.binding.fineoffsetweatherstation/src/main/java/org/openhab/binding/fineoffsetweatherstation/internal/domain/Command.java

@@ -275,4 +275,8 @@ public boolean isHeaderValid(byte[] data) {
     public boolean isResponseValid(byte[] data) {
         return isHeaderValid(data) && Utils.validateChecksum(data, sizeBytes);
     }
+
+    public int getSizeBytes() {
+        return sizeBytes;
+    }
 }

bundles/org.openhab.binding.fineoffsetweatherstation/src/main/java/org/openhab/binding/fineoffsetweatherstation/internal/domain/DebugDetails.java

@@ -0,0 +1,79 @@
+/**
+ * Copyright (c) 2010-2023 Contributors to the openHAB project
+ *
+ * See the NOTICE file(s) distributed with this work for additional
+ * information.
+ *
+ * This program and the accompanying materials are made available under the
+ * terms of the Eclipse Public License 2.0 which is available at
+ * http://www.eclipse.org/legal/epl-2.0
+ *
+ * SPDX-License-Identifier: EPL-2.0
+ */
+package org.openhab.binding.fineoffsetweatherstation.internal.domain;
+
+import java.util.Arrays;
+import java.util.Map;
+import java.util.TreeMap;
+import java.util.stream.Collectors;
+
+import org.apache.commons.lang3.StringUtils;
+import org.openhab.binding.fineoffsetweatherstation.internal.Utils;
+
+/**
+ * Class to collect debug details
+ *
+ * @author Andreas Berger - Initial contribution
+ */
+public class DebugDetails {
+    final byte[] data;
+
+    private final Map<Integer, DebugSegment> segments = new TreeMap<>();
+
+    public DebugDetails(byte[] data, Command command, Protocol protocol) {
+        this.data = data;
+        addDebugDetails(0, 2, "header");
+        addDebugDetails(2, 1, "command: " + command.name());
+        addDebugDetails(3, command.getSizeBytes(), "size");
+        if (protocol == Protocol.ELV) {
+            addDebugDetails(data.length - 2, 1, "ELV checksum");
+        }
+        addDebugDetails(data.length - 1, 1, "checksum");
+    }
+
+    public void addDebugDetails(int start, int length, String description) {
+        segments.put(start, new DebugSegment(start, length, description));
+    }
+
+    @Override
+    public String toString() {
+        int padding = segments.values().stream().mapToInt(value -> value.length).max().orElse(0) * 2;
+        return "0x" + Utils.toHexString(data, data.length, "") + "\n" + segments.values().stream()
+                .map(debugSegment -> debugSegment.toDebugString(padding)).collect(Collectors.joining("\n"));
+    }
+
+    private class DebugSegment {
+        final int start;
+        final int length;
+        final String description;
+
+        DebugSegment(int start, int length, String description) {
+            this.start = start;
+            this.length = length;
+            this.description = description;
+        }
+
+        @Override
+        public String toString() {
+            return toDebugString(0);
+        }
+
+        private String toDebugString(int padding) {
+            String result = "0x";
+            String hexString = Utils.toHexString(Arrays.copyOfRange(data, start, start + length), length, "");
+            result += StringUtils.rightPad(hexString, padding, " ");
+            result += ": " + description;
+            return result;
+        }
+    }
+}

bundles/org.openhab.binding.fineoffsetweatherstation/src/main/java/org/openhab/binding/fineoffsetweatherstation/internal/domain/Measurand.java

@@ -138,8 +138,10 @@ public enum Measurand {
     // `LIGHTNING_POWER` is the name in the spec, so we keep it here as it
     LIGHTNING_POWER("lightning-counter", 0x62, "lightning counter for the day", MeasureType.LIGHTNING_COUNTER),
 
-    TF_USRX("temperature-external-channel", new int[] { 0x63, 0x64, 0x65, 0x66, 0x67, 0x68, 0x69, 0x6A },
-            "Soil or Water temperature", MeasureType.TEMPERATURE),
+    TF_USRX(new int[] { 0x63, 0x64, 0x65, 0x66, 0x67, 0x68, 0x69, 0x6A },
+            new MeasurandParser("temperature-external-channel", "Soil or Water temperature", MeasureType.TEMPERATURE),
+            // skip battery-level, since it is read via Command.CMD_READ_SENSOR_ID_NEW
+            new Skip(1)),
 
     ITEM_SENSOR_CO2(0x70,
             new MeasurandParser("sensor-co2-temperature", "Temperature (CO₂-Sensor)", MeasureType.TEMPERATURE),
@@ -240,18 +242,20 @@ public enum Measurand {
     }
 
     private int extractMeasuredValues(byte[] data, int offset, @Nullable Integer channel, ConversionContext context,
-            @Nullable ParserCustomizationType customizationType, List<MeasuredValue> result) {
+            @Nullable ParserCustomizationType customizationType, List<MeasuredValue> result,
+            DebugDetails debugDetails) {
         int subOffset = 0;
         for (Parser parser : parsers) {
             subOffset += parser.extractMeasuredValues(data, offset + subOffset, channel, context, customizationType,
-                    result);
+                    result, debugDetails);
         }
         return subOffset;
     }
 
     private interface Parser {
         int extractMeasuredValues(byte[] data, int offset, @Nullable Integer channel, ConversionContext context,
-                @Nullable ParserCustomizationType customizationType, List<MeasuredValue> result);
+                @Nullable ParserCustomizationType customizationType, List<MeasuredValue> result,
+                DebugDetails debugDetails);
     }
 
     private static class Skip implements Parser {
@@ -263,7 +267,9 @@ public Skip(int skip) {
 
         @Override
         public int extractMeasuredValues(byte[] data, int offset, @Nullable Integer channel, ConversionContext context,
-                @Nullable ParserCustomizationType customizationType, List<MeasuredValue> result) {
+                @Nullable ParserCustomizationType customizationType, List<MeasuredValue> result,
+                DebugDetails debugDetails) {
+            debugDetails.addDebugDetails(offset, skip, "skipped");
             return skip;
         }
     }
@@ -302,8 +308,14 @@ public SingleChannelMeasurand(Measurand measurand, @Nullable Integer channel) {
         }
 
         public int extractMeasuredValues(byte[] data, int offset, ConversionContext context,
-                @Nullable ParserCustomizationType customizationType, List<MeasuredValue> result) {
-            return measurand.extractMeasuredValues(data, offset, channel, context, customizationType, result);
+                @Nullable ParserCustomizationType customizationType, List<MeasuredValue> result,
+                DebugDetails debugDetails) {
+            return measurand.extractMeasuredValues(data, offset, channel, context, customizationType, result,
+                    debugDetails);
+        }
+
+        public String getDebugString() {
+            return measurand.name() + (channel == null ? "" : " channel " + channel);
         }
     }
 
@@ -337,12 +349,17 @@ private static class MeasurandParser implements Parser {
 
         @Override
         public int extractMeasuredValues(byte[] data, int offset, @Nullable Integer channel, ConversionContext context,
-                @Nullable ParserCustomizationType customizationType, List<MeasuredValue> result) {
+                @Nullable ParserCustomizationType customizationType, List<MeasuredValue> result,
+                DebugDetails debugDetails) {
             MeasureType measureType = getMeasureType(customizationType);
             State state = measureType.toState(data, offset, context);
             if (state != null) {
+                debugDetails.addDebugDetails(offset, measureType.getByteSize(),
+                        measureType.name() + ": " + state.toFullString());
                 ChannelTypeUID channelType = channelTypeUID == null ? measureType.getChannelTypeId() : channelTypeUID;
                 result.add(new MeasuredValue(measureType, channelPrefix, channel, channelType, state, name));
+            } else {
+                debugDetails.addDebugDetails(offset, measureType.getByteSize(), measureType.name() + ": null");
             }
             return measureType.getByteSize();
         }

bundles/org.openhab.binding.fineoffsetweatherstation/src/main/java/org/openhab/binding/fineoffsetweatherstation/internal/service/ELVGatewayQueryService.java

@@ -21,6 +21,7 @@
 import org.openhab.binding.fineoffsetweatherstation.internal.FineOffsetGatewayConfiguration;
 import org.openhab.binding.fineoffsetweatherstation.internal.domain.Command;
 import org.openhab.binding.fineoffsetweatherstation.internal.domain.ConversionContext;
+import org.openhab.binding.fineoffsetweatherstation.internal.domain.DebugDetails;
 import org.openhab.binding.fineoffsetweatherstation.internal.domain.Protocol;
 import org.openhab.binding.fineoffsetweatherstation.internal.domain.SensorGatewayBinding;
 import org.openhab.binding.fineoffsetweatherstation.internal.domain.response.MeasuredValue;
@@ -79,6 +80,10 @@ public List<MeasuredValue> getMeasuredValues() {
         if (data == null) {
             return Collections.emptyList();
         }
-        return fineOffsetDataParser.getMeasuredValues(data, conversionContext);
+        DebugDetails debugDetails = new DebugDetails(data, Command.CMD_WS980_LIVEDATA, Protocol.ELV);
+        List<MeasuredValue> measuredValues = fineOffsetDataParser.getMeasuredValues(data, conversionContext,
+                debugDetails);
+        logger.trace("{}", debugDetails);
+        return measuredValues;
     }
 }

bundles/org.openhab.binding.fineoffsetweatherstation/src/main/java/org/openhab/binding/fineoffsetweatherstation/internal/service/FineOffsetDataParser.java

@@ -27,6 +27,7 @@
 import org.eclipse.jdt.annotation.Nullable;
 import org.openhab.binding.fineoffsetweatherstation.internal.Utils;
 import org.openhab.binding.fineoffsetweatherstation.internal.domain.ConversionContext;
+import org.openhab.binding.fineoffsetweatherstation.internal.domain.DebugDetails;
 import org.openhab.binding.fineoffsetweatherstation.internal.domain.Measurand;
 import org.openhab.binding.fineoffsetweatherstation.internal.domain.Protocol;
 import org.openhab.binding.fineoffsetweatherstation.internal.domain.SensorGatewayBinding;
@@ -151,7 +152,7 @@ public Map<SensorGatewayBinding, SensorDevice> getRegisteredSensors(byte[] data,
         return new SystemInfo(frequency, date, dst, useWh24);
     }
 
-    List<MeasuredValue> getMeasuredValues(byte[] data, ConversionContext context) {
+    List<MeasuredValue> getMeasuredValues(byte[] data, ConversionContext context, DebugDetails debugDetails) {
         /*
          * Pos| Length | Description
          * -------------------------------------------------
@@ -173,26 +174,31 @@ List<MeasuredValue> getMeasuredValues(byte[] data, ConversionContext context) {
         var idx = 5;
         if (protocol == Protocol.ELV) {
             idx++; // at index 5 there is an additional Byte being set to 0x04
+            debugDetails.addDebugDetails(5, 1, "ELV extra byte");
         }
-        return readMeasuredValues(data, idx, context, protocol.getParserCustomizationType());
+        return readMeasuredValues(data, idx, context, protocol.getParserCustomizationType(), debugDetails);
     }
 
-    List<MeasuredValue> getRainData(byte[] data, ConversionContext context) {
-        return readMeasuredValues(data, 5, context, Measurand.ParserCustomizationType.RAIN_READING);
+    List<MeasuredValue> getRainData(byte[] data, ConversionContext context, DebugDetails debugDetails) {
+        return readMeasuredValues(data, 5, context, Measurand.ParserCustomizationType.RAIN_READING, debugDetails);
     }
 
     private List<MeasuredValue> readMeasuredValues(byte[] data, int idx, ConversionContext context,
-            Measurand.@Nullable ParserCustomizationType protocol) {
+            Measurand.@Nullable ParserCustomizationType protocol, DebugDetails debugDetails) {
         var size = toUInt16(data, 3);
+
         List<MeasuredValue> result = new ArrayList<>();
         while (idx < size) {
             byte code = data[idx++];
             Measurand.SingleChannelMeasurand measurand = Measurand.getByCode(code);
             if (measurand == null) {
                 logger.warn("failed to get measurand 0x{}", Integer.toHexString(code));
+                debugDetails.addDebugDetails(idx - 1, 1, "unknown measurand");
                 return result;
+            } else {
+                debugDetails.addDebugDetails(idx - 1, 1, "measurand " + measurand.getDebugString());
             }
-            idx += measurand.extractMeasuredValues(data, idx, context, protocol, result);
+            idx += measurand.extractMeasuredValues(data, idx, context, protocol, result, debugDetails);
         }
         return result;
     }

bundles/org.openhab.binding.fineoffsetweatherstation/src/main/java/org/openhab/binding/fineoffsetweatherstation/internal/service/FineOffsetGatewayQueryService.java

@@ -22,6 +22,7 @@
 import org.openhab.binding.fineoffsetweatherstation.internal.FineOffsetGatewayConfiguration;
 import org.openhab.binding.fineoffsetweatherstation.internal.domain.Command;
 import org.openhab.binding.fineoffsetweatherstation.internal.domain.ConversionContext;
+import org.openhab.binding.fineoffsetweatherstation.internal.domain.DebugDetails;
 import org.openhab.binding.fineoffsetweatherstation.internal.domain.Protocol;
 import org.openhab.binding.fineoffsetweatherstation.internal.domain.SensorGatewayBinding;
 import org.openhab.binding.fineoffsetweatherstation.internal.domain.response.MeasuredValue;
@@ -38,6 +39,7 @@
  */
 @NonNullByDefault
 public class FineOffsetGatewayQueryService extends GatewayQueryService {
+    private static final Protocol PROTOCOL = Protocol.DEFAULT;
     private final Logger logger = LoggerFactory.getLogger(FineOffsetGatewayQueryService.class);
 
     private final FineOffsetDataParser fineOffsetDataParser;
@@ -47,7 +49,7 @@ public class FineOffsetGatewayQueryService extends GatewayQueryService {
     public FineOffsetGatewayQueryService(FineOffsetGatewayConfiguration config,
             @Nullable ThingStatusListener thingStatusListener, ConversionContext conversionContext) {
         super(config, thingStatusListener);
-        this.fineOffsetDataParser = new FineOffsetDataParser(Protocol.DEFAULT);
+        this.fineOffsetDataParser = new FineOffsetDataParser(PROTOCOL);
         this.conversionContext = conversionContext;
     }
 
@@ -92,18 +94,24 @@ public Collection<MeasuredValue> getMeasuredValues() {
 
         byte[] data = executeCommand(Command.CMD_GW1000_LIVEDATA);
         if (data != null) {
-            List<MeasuredValue> measuredValues = fineOffsetDataParser.getMeasuredValues(data, conversionContext);
+            DebugDetails debugDetails = new DebugDetails(data, Command.CMD_GW1000_LIVEDATA, PROTOCOL);
+            List<MeasuredValue> measuredValues = fineOffsetDataParser.getMeasuredValues(data, conversionContext,
+                    debugDetails);
             for (MeasuredValue measuredValue : measuredValues) {
                 valuePerChannel.put(measuredValue.getChannelId(), measuredValue);
             }
+            logger.trace("{}", debugDetails);
         }
 
         data = executeCommand(Command.CMD_READ_RAIN);
         if (data != null) {
-            List<MeasuredValue> measuredRainValues = fineOffsetDataParser.getRainData(data, conversionContext);
+            DebugDetails debugDetails = new DebugDetails(data, Command.CMD_READ_RAIN, PROTOCOL);
+            List<MeasuredValue> measuredRainValues = fineOffsetDataParser.getRainData(data, conversionContext,
+                    debugDetails);
             for (MeasuredValue measuredValue : measuredRainValues) {
                 valuePerChannel.put(measuredValue.getChannelId(), measuredValue);
             }
+            logger.trace("{}", debugDetails);
         }
 
         return valuePerChannel.values();

bundles/org.openhab.binding.fineoffsetweatherstation/src/main/java/org/openhab/binding/fineoffsetweatherstation/internal/service/GatewayQueryService.java

@@ -44,7 +44,7 @@
  */
 @NonNullByDefault
 public abstract class GatewayQueryService implements AutoCloseable {
-    private final Logger logger = LoggerFactory.getLogger(this.getClass());
+    protected final Logger logger = LoggerFactory.getLogger(this.getClass());
 
     private static final Lock REQUEST_LOCK = new ReentrantLock();