Garmin protocol: fixes

- fix DEVICE_SETTINGS message ID
- put all status messages in own package
- allow protobuf handler to change the returned status message to signal unsupported requests
- fix various bugs

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

@@ -30,9 +30,9 @@ import nodomain.freeyourgadget.gadgetbridge.service.devices.garmin.messages.Conf
 import nodomain.freeyourgadget.gadgetbridge.service.devices.garmin.messages.GFDIMessage;
 import nodomain.freeyourgadget.gadgetbridge.service.devices.garmin.messages.MusicControlEntityUpdateMessage;
 import nodomain.freeyourgadget.gadgetbridge.service.devices.garmin.messages.ProtobufMessage;
-import nodomain.freeyourgadget.gadgetbridge.service.devices.garmin.messages.ProtobufStatusMessage;
 import nodomain.freeyourgadget.gadgetbridge.service.devices.garmin.messages.SetDeviceSettingsMessage;
 import nodomain.freeyourgadget.gadgetbridge.service.devices.garmin.messages.SystemEventMessage;
+import nodomain.freeyourgadget.gadgetbridge.service.devices.garmin.messages.status.ProtobufStatusMessage;
 import nodomain.freeyourgadget.gadgetbridge.util.StringUtils;
 
 
@@ -100,6 +100,14 @@ public class GarminSupport extends AbstractBTLEDeviceSupport implements ICommuni
 
         evaluateGBDeviceEvent(parsedMessage.getGBDeviceEvent());
 
+        if (parsedMessage instanceof ProtobufMessage) {
+            ProtobufMessage protobufMessage = protocolBufferHandler.processIncoming((ProtobufMessage) parsedMessage);
+            if (protobufMessage != null) {
+                communicator.sendMessage(protobufMessage.getOutgoingMessage());
+                communicator.sendMessage(protobufMessage.getAckBytestream());
+            }
+        }
+
         communicator.sendMessage(parsedMessage.getAckBytestream());
 
         byte[] response = parsedMessage.getOutgoingMessage();
@@ -112,14 +120,6 @@ public class GarminSupport extends AbstractBTLEDeviceSupport implements ICommuni
             completeInitialization();
         }
 
-        if (parsedMessage instanceof ProtobufMessage) {
-            ProtobufMessage protobufMessage = protocolBufferHandler.processIncoming((ProtobufMessage) parsedMessage);
-            if (protobufMessage != null) {
-                communicator.sendMessage(protobufMessage.getOutgoingMessage());
-                communicator.sendMessage(protobufMessage.getAckBytestream());
-            }
-        }
-
         if (parsedMessage instanceof ProtobufStatusMessage) {
             ProtobufMessage protobufMessage = protocolBufferHandler.processIncoming((ProtobufStatusMessage) parsedMessage);
             if (protobufMessage != null) {

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

@@ -20,7 +20,7 @@ import nodomain.freeyourgadget.gadgetbridge.proto.vivomovehr.GdiFindMyWatch;
 import nodomain.freeyourgadget.gadgetbridge.proto.vivomovehr.GdiSmartProto;
 import nodomain.freeyourgadget.gadgetbridge.service.devices.garmin.messages.GFDIMessage;
 import nodomain.freeyourgadget.gadgetbridge.service.devices.garmin.messages.ProtobufMessage;
-import nodomain.freeyourgadget.gadgetbridge.service.devices.garmin.messages.ProtobufStatusMessage;
+import nodomain.freeyourgadget.gadgetbridge.service.devices.garmin.messages.status.ProtobufStatusMessage;
 import nodomain.freeyourgadget.gadgetbridge.util.GB;
 import nodomain.freeyourgadget.gadgetbridge.util.calendar.CalendarEvent;
 import nodomain.freeyourgadget.gadgetbridge.util.calendar.CalendarManager;
@@ -75,13 +75,14 @@ public class ProtocolBufferHandler {
             }
             if (!processed) {
                 LOG.warn("Unknown protobuf request: {}", smart);
+                message.setStatusMessage(new ProtobufStatusMessage(message.getMessageType(), GFDIMessage.Status.ACK, message.getRequestId(), message.getDataOffset(), ProtobufStatusMessage.ProtobufChunkStatus.DISCARDED, ProtobufStatusMessage.ProtobufStatusCode.UNKNOWN_REQUEST_ID));
             }
         }
         return null;
     }
 
     public ProtobufMessage processIncoming(ProtobufStatusMessage statusMessage) {
-        LOG.info("Processing protobuf status message #{}@{}:  status={}, error={}", statusMessage.getRequestId(), statusMessage.getDataOffset(), statusMessage.getProtobufStatus(), statusMessage.getError());
+        LOG.info("Processing protobuf status message #{}@{}:  status={}, error={}", statusMessage.getRequestId(), statusMessage.getDataOffset(), statusMessage.getProtobufChunkStatus(), statusMessage.getProtobufStatusCode());
         //TODO: check status and react accordingly, right now we blindly proceed to next chunk
         if (chunkedFragmentsMap.containsKey(statusMessage.getRequestId()) && statusMessage.isOK()) {
             final ProtobufFragment protobufFragment = chunkedFragmentsMap.get(statusMessage.getRequestId());

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

@@ -7,8 +7,11 @@ import org.slf4j.LoggerFactory;
 
 import java.lang.reflect.Method;
 import java.nio.ByteBuffer;
+import java.nio.ByteOrder;
 
 import nodomain.freeyourgadget.gadgetbridge.deviceevents.GBDeviceEvent;
+import nodomain.freeyourgadget.gadgetbridge.service.devices.garmin.messages.status.GFDIStatusMessage;
+import nodomain.freeyourgadget.gadgetbridge.service.devices.garmin.messages.status.GenericStatusMessage;
 
 public abstract class GFDIMessage {
     public static final int MESSAGE_RESPONSE = 5000; //TODO: MESSAGE_STATUS is a better name?
@@ -51,9 +54,9 @@ public abstract class GFDIMessage {
 
         final int messageType = messageReader.readShort();
         try {
-//            Class<? extends GFDIMessage> objectClass = GarminMessage.fromId(messageType);
-            Method m = GarminMessage.fromId(messageType).getMethod("parseIncoming", MessageReader.class, int.class);
-            return GarminMessage.fromId(messageType).cast(m.invoke(null, messageReader, messageType));
+//            Class<? extends GFDIMessage> objectClass = GarminMessage.getClassFromId(messageType);
+            Method m = GarminMessage.getClassFromId(messageType).getMethod("parseIncoming", MessageReader.class, int.class);
+            return GarminMessage.getClassFromId(messageType).cast(m.invoke(null, messageReader, messageType));
         } catch (Exception e) {
             LOG.error("UNHANDLED GFDI MESSAGE TYPE {}, MESSAGE {}", messageType, message);
             return new UnhandledMessage(messageType);
@@ -65,6 +68,7 @@ public abstract class GFDIMessage {
     public byte[] getOutgoingMessage() {
         response.clear();
         boolean toSend = generateOutgoing();
+        response.order(ByteOrder.LITTLE_ENDIAN);
         if (!toSend)
             return null;
         addLengthAndChecksum();
@@ -99,6 +103,7 @@ public abstract class GFDIMessage {
         RESPONSE(5000, GFDIStatusMessage.class), //TODO: STATUS is a better name?
         SYSTEM_EVENT(5030, SystemEventMessage.class),
         DEVICE_INFORMATION(5024, DeviceInformationMessage.class),
+        DEVICE_SETTINGS(5026, SetDeviceSettingsMessage.class),
         FIND_MY_PHONE(5039, FindMyPhoneRequestMessage.class),
         CANCEL_FIND_MY_PHONE(5040, FindMyPhoneRequestMessage.class),
         MUSIC_CONTROL(5041, MusicControlMessage.class),
@@ -117,7 +122,7 @@ public abstract class GFDIMessage {
             this.objectClass = objectClass;
         }
 
-        public static Class<? extends GFDIMessage> fromId(final int id) {
+        public static Class<? extends GFDIMessage> getClassFromId(final int id) {
             for (final GarminMessage garminMessage : GarminMessage.values()) {
                 if (garminMessage.getId() == id) {
                     return garminMessage.getObjectClass();
@@ -126,6 +131,14 @@ public abstract class GFDIMessage {
             return null;
         }
 
+        public static GarminMessage fromId(final int id) {
+            for (final GarminMessage garminMessage : GarminMessage.values()) {
+                if (garminMessage.getId() == id) {
+                    return garminMessage;
+                }
+            }
+            return null;
+        }
         public int getId() {
             return id;
         }

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

@@ -1,28 +0,0 @@
-package nodomain.freeyourgadget.gadgetbridge.service.devices.garmin.messages;
-
-
-public abstract class GFDIStatusMessage extends GFDIMessage {
-    Status status;
-
-    public static GFDIStatusMessage parseIncoming(MessageReader reader, int messageType) {
-        final int requestMessageType = reader.readShort();
-        if (GarminMessage.PROTOBUF_REQUEST.getId() == requestMessageType || GarminMessage.PROTOBUF_RESPONSE.getId() == requestMessageType) {
-            return ProtobufStatusMessage.parseIncoming(reader, messageType);
-        } else {
-            final Status status = Status.fromCode(reader.readByte());
-
-            reader.warnIfLeftover();
-            return new GenericStatusMessage(messageType, status);
-        }
-    }
-
-    @Override
-    protected boolean generateOutgoing() {
-        return false;
-    }
-
-    protected Status getStatus() {
-        return status;
-    }
-
-}

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

@@ -13,7 +13,6 @@ public class MessageReader {
     protected static final Logger LOG = LoggerFactory.getLogger(MessageReader.class);
 
     private final ByteBuffer byteBuffer;
-
     private final int payloadSize;
 
     public MessageReader(byte[] data) {
@@ -33,6 +32,10 @@ public class MessageReader {
         return !byteBuffer.hasRemaining();
     }
 
+    public boolean isEndOfPayload() {
+        return byteBuffer.position() >= payloadSize - 2;
+    }
+
     public int getPosition() {
         return byteBuffer.position();
     }
@@ -99,7 +102,6 @@ public class MessageReader {
         return byteBuffer.capacity();
     }
 
-
     private void checkSize() {
         if (payloadSize > getCapacity()) {
             LOG.error("Received GFDI packet with invalid length: {} vs {}", payloadSize, getCapacity());
@@ -116,12 +118,13 @@ public class MessageReader {
         }
     }
 
-
     public void warnIfLeftover() {
         if (byteBuffer.hasRemaining() && byteBuffer.position() < (byteBuffer.limit() - 2)) {
+            int pos = byteBuffer.position();
             int numBytes = (byteBuffer.limit() - 2) - byteBuffer.position();
             byte[] leftover = new byte[numBytes];
             byteBuffer.get(leftover);
+            byteBuffer.position(pos);
             LOG.warn("Leftover bytes when parsing message. Bytes: {}, complete message: {}", GB.hexdump(leftover), GB.hexdump(byteBuffer.array()));
         }
     }

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

@@ -1,7 +1,11 @@
 package nodomain.freeyourgadget.gadgetbridge.service.devices.garmin.messages;
 
 
-import static nodomain.freeyourgadget.gadgetbridge.service.devices.garmin.messages.ProtobufStatusMessage.ProtobufStatusCode.NO_ERROR;
+import nodomain.freeyourgadget.gadgetbridge.service.devices.garmin.messages.status.GenericStatusMessage;
+import nodomain.freeyourgadget.gadgetbridge.service.devices.garmin.messages.status.ProtobufStatusMessage;
+
+import static nodomain.freeyourgadget.gadgetbridge.service.devices.garmin.messages.status.ProtobufStatusMessage.ProtobufChunkStatus.KEPT;
+import static nodomain.freeyourgadget.gadgetbridge.service.devices.garmin.messages.status.ProtobufStatusMessage.ProtobufStatusCode.NO_ERROR;
 
 public class ProtobufMessage extends GFDIMessage {
 
@@ -14,10 +18,6 @@ public class ProtobufMessage extends GFDIMessage {
     private final byte[] messageBytes;
     private final boolean sendOutgoing;
 
-    public ProtobufMessage(int messageType, int requestId, int dataOffset, int totalProtobufLength, int protobufDataLength, byte[] messageBytes) {
-        this(messageType, requestId, dataOffset, totalProtobufLength, protobufDataLength, messageBytes, true);
-    }
-
     public ProtobufMessage(int messageType, int requestId, int dataOffset, int totalProtobufLength, int protobufDataLength, byte[] messageBytes, boolean sendOutgoing) {
         this.messageType = messageType;
         this.requestId = requestId;
@@ -30,9 +30,16 @@ public class ProtobufMessage extends GFDIMessage {
         if (isComplete()) {
             this.statusMessage = new GenericStatusMessage(messageType, GFDIMessage.Status.ACK);
         } else {
-            this.statusMessage = new ProtobufStatusMessage(messageType, GFDIMessage.Status.ACK, requestId, dataOffset, NO_ERROR, NO_ERROR);
+            this.statusMessage = new ProtobufStatusMessage(messageType, GFDIMessage.Status.ACK, requestId, dataOffset, KEPT, NO_ERROR);
         }
     }
+    public ProtobufMessage(int messageType, int requestId, int dataOffset, int totalProtobufLength, int protobufDataLength, byte[] messageBytes) {
+        this(messageType, requestId, dataOffset, totalProtobufLength, protobufDataLength, messageBytes, true);
+    }
+
+    public void setStatusMessage(ProtobufStatusMessage protobufStatusMessage) {
+        this.statusMessage = protobufStatusMessage;
+    }
 
     public static ProtobufMessage parseIncoming(MessageReader reader, int messageType) {
         final int requestID = reader.readShort();

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

@@ -17,7 +17,7 @@ public class SetDeviceSettingsMessage extends GFDIMessage {
     protected boolean generateOutgoing() {
         final MessageWriter writer = new MessageWriter(response);
         writer.writeShort(0); // packet size will be filled below
-        writer.writeShort(GarminMessage.DEVICE_INFORMATION.getId());
+        writer.writeShort(GarminMessage.DEVICE_SETTINGS.getId());
         writer.writeByte(settings.size());
         for (Map.Entry<GarminDeviceSetting, Object> settingPair : settings.entrySet()) {
             final GarminDeviceSetting setting = settingPair.getKey();

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

@@ -1,5 +1,7 @@
 package nodomain.freeyourgadget.gadgetbridge.service.devices.garmin.messages;
 
+import nodomain.freeyourgadget.gadgetbridge.service.devices.garmin.messages.status.GenericStatusMessage;
+
 public class UnhandledMessage extends GFDIMessage {
 
     private final int messageType;

app/src/main/java/nodomain/freeyourgadget/gadgetbridge/service/devices/garmin/messages/status/GFDIStatusMessage.java

@@ -0,0 +1,39 @@
+package nodomain.freeyourgadget.gadgetbridge.service.devices.garmin.messages.status;
+
+
+import nodomain.freeyourgadget.gadgetbridge.service.devices.garmin.messages.GFDIMessage;
+import nodomain.freeyourgadget.gadgetbridge.service.devices.garmin.messages.MessageReader;
+
+public abstract class GFDIStatusMessage extends GFDIMessage {
+    Status status;
+
+    public static GFDIStatusMessage parseIncoming(MessageReader reader, int messageType) {
+        final GarminMessage garminMessage = GFDIMessage.GarminMessage.fromId(reader.readShort());
+        if (GarminMessage.PROTOBUF_REQUEST.equals(garminMessage) || GarminMessage.PROTOBUF_RESPONSE.equals(garminMessage)) {
+            return ProtobufStatusMessage.parseIncoming(reader, messageType);
+        } else {
+            final Status status = Status.fromCode(reader.readByte());
+
+            switch (status) {
+                case ACK:
+                    LOG.info("Received ACK for message {}", garminMessage.name());
+                    break;
+                default:
+                    LOG.warn("Received {} for message {}", status, garminMessage.name());
+            }
+
+            reader.warnIfLeftover();
+            return new GenericStatusMessage(messageType, status);
+        }
+    }
+
+    @Override
+    protected boolean generateOutgoing() {
+        return false;
+    }
+
+    protected Status getStatus() {
+        return status;
+    }
+
+}

app/src/main/java/nodomain/freeyourgadget/gadgetbridge/service/devices/garmin/messages/status/GenericStatusMessage.java

@@ -1,4 +1,6 @@
-package nodomain.freeyourgadget.gadgetbridge.service.devices.garmin.messages;
+package nodomain.freeyourgadget.gadgetbridge.service.devices.garmin.messages.status;
+
+import nodomain.freeyourgadget.gadgetbridge.service.devices.garmin.messages.MessageWriter;
 
 public class GenericStatusMessage extends GFDIStatusMessage {
 

app/src/main/java/nodomain/freeyourgadget/gadgetbridge/service/devices/garmin/messages/status/ProtobufStatusMessage.java

@@ -1,27 +1,31 @@
-package nodomain.freeyourgadget.gadgetbridge.service.devices.garmin.messages;
+package nodomain.freeyourgadget.gadgetbridge.service.devices.garmin.messages.status;
 
 import androidx.annotation.Nullable;
 
+import nodomain.freeyourgadget.gadgetbridge.service.devices.garmin.messages.MessageReader;
+import nodomain.freeyourgadget.gadgetbridge.service.devices.garmin.messages.MessageWriter;
+
 public class ProtobufStatusMessage extends GFDIStatusMessage {
 
     private final Status status;
     private final int requestId;
     private final int dataOffset;
-    private final ProtobufStatusCode protobufStatus;
-    private final ProtobufStatusCode error; //TODO: why is this duplicated?
+    private final ProtobufChunkStatus protobufChunkStatus;
+    private final ProtobufStatusCode protobufStatusCode;
     private final int messageType;
     private final boolean sendOutgoing;
 
-    public ProtobufStatusMessage(int messageType, Status status, int requestId, int dataOffset, ProtobufStatusCode protobufStatus, ProtobufStatusCode error) {
-        this(messageType, status, requestId, dataOffset, protobufStatus, error, true);
+    public ProtobufStatusMessage(int messageType, Status status, int requestId, int dataOffset, ProtobufChunkStatus protobufChunkStatus, ProtobufStatusCode protobufStatusCode) {
+        this(messageType, status, requestId, dataOffset, protobufChunkStatus, protobufStatusCode, true);
     }
-    public ProtobufStatusMessage(int messageType, Status status, int requestId, int dataOffset, ProtobufStatusCode protobufStatus, ProtobufStatusCode error, boolean sendOutgoing) {
+
+    public ProtobufStatusMessage(int messageType, Status status, int requestId, int dataOffset, ProtobufChunkStatus protobufChunkStatus, ProtobufStatusCode protobufStatusCode, boolean sendOutgoing) {
         this.messageType = messageType;
         this.status = status;
         this.requestId = requestId;
         this.dataOffset = dataOffset;
-        this.protobufStatus = protobufStatus;
-        this.error = error;
+        this.protobufChunkStatus = protobufChunkStatus;
+        this.protobufStatusCode = protobufStatusCode;
         this.sendOutgoing = sendOutgoing;
     }
 
@@ -29,7 +33,7 @@ public class ProtobufStatusMessage extends GFDIStatusMessage {
         final Status status = Status.fromCode(reader.readByte());
         final int requestID = reader.readShort();
         final int dataOffset = reader.readInt();
-        final ProtobufStatusCode protobufStatus = ProtobufStatusCode.fromCode(reader.readByte());
+        final ProtobufChunkStatus protobufStatus = ProtobufChunkStatus.fromCode(reader.readByte());
         final ProtobufStatusCode error = ProtobufStatusCode.fromCode(reader.readByte());
 
         reader.warnIfLeftover();
@@ -40,12 +44,12 @@ public class ProtobufStatusMessage extends GFDIStatusMessage {
         return dataOffset;
     }
 
-    public ProtobufStatusCode getProtobufStatus() {
-        return protobufStatus;
+    public ProtobufChunkStatus getProtobufChunkStatus() {
+        return protobufChunkStatus;
     }
 
-    public ProtobufStatusCode getError() {
-        return error;
+    public ProtobufStatusCode getProtobufStatusCode() {
+        return protobufStatusCode;
     }
 
     public int getMessageType() {
@@ -58,8 +62,8 @@ public class ProtobufStatusMessage extends GFDIStatusMessage {
 
     public boolean isOK() {
         return this.status.equals(Status.ACK) &&
-                this.protobufStatus.equals(ProtobufStatusCode.NO_ERROR) &&
-                this.error.equals(ProtobufStatusCode.NO_ERROR);
+                this.protobufChunkStatus.equals(ProtobufChunkStatus.KEPT) &&
+                this.protobufStatusCode.equals(ProtobufStatusCode.NO_ERROR);
     }
 
     @Override
@@ -71,8 +75,8 @@ public class ProtobufStatusMessage extends GFDIStatusMessage {
         writer.writeByte(status.ordinal());
         writer.writeShort(requestId);
         writer.writeInt(dataOffset);
-        writer.writeByte(protobufStatus.code);
-        writer.writeByte(error.code);
+        writer.writeByte(protobufChunkStatus.ordinal());
+        writer.writeByte(protobufStatusCode.code);
         return sendOutgoing;
     }
 
@@ -80,9 +84,25 @@ public class ProtobufStatusMessage extends GFDIStatusMessage {
         return status;
     }
 
+    public enum ProtobufChunkStatus { //based on the observations of the combination with the StatusCode
+        KEPT,
+        DISCARDED,
+        ;
+
+        @Nullable
+        public static ProtobufChunkStatus fromCode(final int code) {
+            for (final ProtobufChunkStatus status : ProtobufChunkStatus.values()) {
+                if (status.ordinal() == code) {
+                    return status;
+                }
+            }
+
+            return null;
+        }
+    }
+
     public enum ProtobufStatusCode {
         NO_ERROR(0),
-        UNKNOWN_ERROR(1),
         UNKNOWN_REQUEST_ID(100),
         DUPLICATE_PACKET(101),
         MISSING_PACKET(102),