Merge pull request 'Fix .FIT file decoding' (#6038)

Reviewed-on: https://codeberg.org/Freeyourgadget/Gadgetbridge/pulls/6038

app/src/main/java/nodomain/freeyourgadget/gadgetbridge/model/ActivitySummaryData.java

@@ -88,7 +88,11 @@ public class ActivitySummaryData {
 
     /// @return {@code true} if the value was actually added
     public boolean add(final String group, final String key, final Number value, final String unit, boolean forceDisplay) {
-        if (value != null && (value.doubleValue() != 0 || forceDisplay)) {
+        if (value == null || !Double.isFinite(value.doubleValue())) {
+            return false;
+        }
+
+        if (value.doubleValue() != 0 || forceDisplay) {
             entries.put(key, new ActivitySummarySimpleEntry(group, value, unit));
             return true;
         }

app/src/main/java/nodomain/freeyourgadget/gadgetbridge/service/devices/garmin/fit/FieldDefinition.java

@@ -42,29 +42,20 @@ public class FieldDefinition implements FieldInterface {
         int size = garminByteBufferReader.readByte();
         int baseTypeIdentifier = garminByteBufferReader.readByte();
         BaseType baseType = BaseType.fromIdentifier(baseTypeIdentifier);
-        FieldDefinition nativeFITMessageFieldDefinition = nativeFITMessage.getFieldDefinition(number, size);
+        FieldDefinition nativeFITMessageFieldDefinition = nativeFITMessage.getFieldDefinition(number, size, baseType);
         if (nativeFITMessageFieldDefinition != null) {
-            if (nativeFITMessageFieldDefinition.getBaseType().equals(baseType)) {
-                return nativeFITMessageFieldDefinition;
-            } else {
-                if (baseType == BaseType.UINT8 && nativeFITMessageFieldDefinition.getBaseType() == BaseType.ENUM) {
-                    // Allow uint8 -> enum, as older fit versions changed the type of some fields
-                    return nativeFITMessageFieldDefinition;
-                }
-                LOG.warn(
-                        "Native for {}[{}] is of type {}, but message declares {}",
-                        nativeFITMessage.name(),
-                        nativeFITMessageFieldDefinition.name,
-                        nativeFITMessageFieldDefinition.getBaseType(),
-                        baseType
-                );
-            }
+            return nativeFITMessageFieldDefinition;
         }
 
         if (number == 253 && size == 4 && baseType.equals(BaseType.UINT32)) {
             return new FieldDefinitionTimestamp(number, size, baseType, "253_timestamp");
         }
 
+        if (0 != (size % baseType.getSize())) {
+            LOG.warn("inconsistent size of field {} in record {}/{} - total size: {}, base size: {}, base type: {}",
+                    number, nativeFITMessage.getNumber(), nativeFITMessage.name(), size,
+                    baseType.getSize(), baseType);
+        }
         return new FieldDefinition(number, size, baseType, "");
     }
 

app/src/main/java/nodomain/freeyourgadget/gadgetbridge/service/devices/garmin/fit/NativeFITMessage.java

@@ -1,7 +1,12 @@
 package nodomain.freeyourgadget.gadgetbridge.service.devices.garmin.fit;
 
+import androidx.annotation.NonNull;
 import androidx.annotation.Nullable;
 
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+import org.slf4j.event.Level;
+
 import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.HashMap;
@@ -2567,17 +2572,58 @@ public class NativeFITMessage {
     }
 
     @Nullable
-    FieldDefinition getFieldDefinition(int id, int size) {
+    FieldDefinition getFieldDefinition(int id, int size, @NonNull BaseType baseType) {
         if (null == fieldDefinitionPrimitives)
             return null;
         for (NativeFITMessage.FieldDefinitionPrimitive fieldDefinitionPrimitive :
                 fieldDefinitionPrimitives) {
             if (fieldDefinitionPrimitive.number == id) {
+                // some .FIT encoders don't strictly stick to current standard
+                // try to handle common cases gracefully
+                Level logLevel = null;
+
+                final BaseType stdBaseType = fieldDefinitionPrimitive.baseType;
+                if (stdBaseType == BaseType.ENUM && baseType == BaseType.UINT8) {
+                    // very common issue
+                    logLevel = Level.DEBUG;
+                } else if (stdBaseType == BaseType.UINT32Z && baseType == BaseType.UINT32) {
+                    // quite common issue
+                    logLevel = Level.INFO;
+                } else if (stdBaseType != baseType) {
+                    logLevel = Level.WARN;
+                }
+
+                if ((size % baseType.getSize()) != 0) {
+                    logLevel = Level.WARN;
+                }
+
+                if (logLevel != null) {
+                    LOG.atLevel(logLevel).log(
+                            "Native for {}[{}] is of type {} with size {} (base: {}), but message declares {} with size {} (base: {})",
+                            name(),
+                            fieldDefinitionPrimitive.name,
+                            stdBaseType,
+                            fieldDefinitionPrimitive.size,
+                            stdBaseType.getSize(),
+                            baseType,
+                            size,
+                            baseType.getSize()
+                    );
+
+                    if (size == 1 && (baseType == BaseType.UINT16 || baseType == BaseType.UINT32 || baseType == BaseType.UINT64)) {
+                        // very common issue for COROS:
+                        // Native for EVENT[data] is of type UINT32 with size 4 (base: 4), but message declares UINT32 with size 1 (base: 4)
+                        LOG.warn("redefining field base type from {} to {} due to size",
+                                baseType, BaseType.UINT8);
+                        baseType = BaseType.UINT8;
+                    }
+                }
+
                 return FieldDefinitionFactory.create(
                         fieldDefinitionPrimitive.number,
                         size,
                         fieldDefinitionPrimitive.type,
-                        fieldDefinitionPrimitive.baseType,
+                        baseType,
                         fieldDefinitionPrimitive.name,
                         fieldDefinitionPrimitive.scale,
                         fieldDefinitionPrimitive.offset
@@ -2690,4 +2736,6 @@ public class NativeFITMessage {
             return result;
         }
     }
+
+    private static final Logger LOG = LoggerFactory.getLogger(NativeFITMessage.class);
 }

app/src/main/java/nodomain/freeyourgadget/gadgetbridge/service/devices/garmin/fit/RecordData.java

@@ -174,7 +174,16 @@ public class RecordData {
     }
 
     public <T> T getFieldByNumber(int number, final Class<T> clazz) {
-        return safeCast(getFieldByNumber(number), clazz);
+        Object object = getFieldByNumber(number);
+        if (object == null)
+            return null;
+
+        // when CIQ fields are used value arrays instead of single values are sometimes recorded
+        if (!clazz.isArray() && object.getClass().isArray() && 0 < Array.getLength(object)) {
+            object = Array.get(object, 0);
+        }
+
+        return safeCast(object, clazz);
     }
 
     public <T> T[] getArrayFieldByNumber(int number, final Class<T> clazz) {
@@ -215,6 +224,23 @@ public class RecordData {
             return clazz.cast(object);
         }
 
+        if(object instanceof Number number){
+            // some older Garmin devices encoded e.g.
+            // [distance] and [enhanced_speed] as float instead of double
+            // [cadence] as float instead of integer
+            if(clazz.equals(Double.class)){
+                return clazz.cast(number.doubleValue());
+            }
+            if(clazz.equals(Integer.class)){
+                return clazz.cast(number.intValue());
+            }
+
+            // required for common COROS size mismatches like: FitEvent[event] type UINT32 with actual size 1
+            if (Long.class.equals(clazz)) {
+                return clazz.cast(number.longValue());
+            }
+        }
+
         LOG.error(
                 "Unable to cast {} ({}) to {}, returning null - this is likely a bug. Record: {}",
                 object,

app/src/main/java/nodomain/freeyourgadget/gadgetbridge/service/devices/garmin/fit/messages/FitSession.java

@@ -1918,6 +1918,12 @@ public class FitSession extends RecordData {
 
     public List<ActivityPoint> toActivityPoints() {
         final List<ActivityPoint> activityPoints = new ArrayList<ActivityPoint>();
+
+        if(getComputedTimestamp() == null) {
+            // some sessions - especially from Strava, have no time stamp
+            return activityPoints;
+        }
+
         final ActivityPoint startActivityPoint = new ActivityPoint();
         startActivityPoint.setTime(new Date(getComputedTimestamp() * 1000L));
         if (getStartLatitude() != null && getStartLongitude() != null) {