diff --git a/app/src/main/java/nodomain/freeyourgadget/gadgetbridge/service/devices/garmin/ProtocolBufferHandler.java b/app/src/main/java/nodomain/freeyourgadget/gadgetbridge/service/devices/garmin/ProtocolBufferHandler.java index f91f31e99..3b2a930c4 100644 --- a/app/src/main/java/nodomain/freeyourgadget/gadgetbridge/service/devices/garmin/ProtocolBufferHandler.java +++ b/app/src/main/java/nodomain/freeyourgadget/gadgetbridge/service/devices/garmin/ProtocolBufferHandler.java @@ -124,7 +124,7 @@ public class ProtocolBufferHandler implements MessageHandler { 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 (statusMessage.isOK()) { - DataTransferHandler.onDataSuccessfullyReceived(statusMessage.getRequestId()); + DataTransferHandler.onDataChunkSuccessfullyReceived(statusMessage.getRequestId()); } if (chunkedFragmentsMap.containsKey(statusMessage.getRequestId()) && statusMessage.isOK()) { final ProtobufFragment protobufFragment = chunkedFragmentsMap.get(statusMessage.getRequestId()); diff --git a/app/src/main/java/nodomain/freeyourgadget/gadgetbridge/service/devices/garmin/http/DataTransferHandler.java b/app/src/main/java/nodomain/freeyourgadget/gadgetbridge/service/devices/garmin/http/DataTransferHandler.java index 3dda7e766..1268baa04 100644 --- a/app/src/main/java/nodomain/freeyourgadget/gadgetbridge/service/devices/garmin/http/DataTransferHandler.java +++ b/app/src/main/java/nodomain/freeyourgadget/gadgetbridge/service/devices/garmin/http/DataTransferHandler.java @@ -8,6 +8,7 @@ import org.slf4j.LoggerFactory; import java.util.Arrays; import java.util.HashMap; import java.util.Map; +import java.util.TreeMap; import java.util.concurrent.atomic.AtomicInteger; import nodomain.freeyourgadget.gadgetbridge.proto.vivomovehr.GdiDataTransferService; @@ -16,7 +17,7 @@ public class DataTransferHandler { private static final Logger LOG = LoggerFactory.getLogger(DataTransferHandler.class); private static final AtomicInteger idCounter = new AtomicInteger(0); private static final Map dataById = new HashMap<>(); - private static final Map requestInfoById = new HashMap<>(); + private static final Map unprocessedChunksByRequestId = new HashMap<>(); public GdiDataTransferService.DataTransferService handle( final GdiDataTransferService.DataTransferService dataTransferService, @@ -64,7 +65,7 @@ public class DataTransferHandler { .setOffset(offset) .build(); } - requestInfoById.put(requestId, new RequestInfo(dataId, chunk.length)); + unprocessedChunksByRequestId.put(requestId, new ChunkInfo(dataId, offset, offset + chunk.length)); return GdiDataTransferService.DataTransferService.DataDownloadResponse.newBuilder() .setStatus(GdiDataTransferService.DataTransferService.Status.SUCCESS) .setId(dataId) @@ -80,32 +81,37 @@ public class DataTransferHandler { return id; } - public static void onDataSuccessfullyReceived(final int requestId) { - final RequestInfo requestInfo = requestInfoById.get(requestId); - requestInfoById.remove(requestId); - if (requestInfo == null) { + public static void onDataChunkSuccessfullyReceived(final int requestId) { + final ChunkInfo chunkInfo = unprocessedChunksByRequestId.get(requestId); + if (chunkInfo == null) { return; } - final Data data = dataById.get(requestInfo.dataId); + unprocessedChunksByRequestId.remove(requestId); + final Data data = dataById.get(chunkInfo.dataId); if (data == null) { return; } - int dataLeft = data.onDataSuccessfullyReceived(requestInfo.requestDataLength); - if (dataLeft == 0) { - LOG.info("Data successfully sent to the device (id: {}, size: {})", requestInfo.dataId, data.data.length); - dataById.remove(requestInfo.dataId); + data.onDataChunkSuccessfullyReceived(chunkInfo); + if (data.isDataSuccessfullySent()) { + LOG.info("Data successfully sent to the device (id: {}, size: {})", chunkInfo.dataId, data.data.length); + dataById.remove(chunkInfo.dataId); } else { - LOG.debug("Data chunk successfully sent to the device (dataId: {}, requestId: {}, data left: {})", requestInfo.dataId, requestId, dataLeft); + LOG.debug( + "Data chunk successfully sent to the device (dataId: {}, requestId: {}): {}-{}/{}", + chunkInfo.dataId, requestId, chunkInfo.start, chunkInfo.end, data.data.length + ); } } - private static class RequestInfo { + private static class ChunkInfo { private final int dataId; - private final int requestDataLength; + private final int start; + private final int end; - private RequestInfo(int dataId, int requestDataLength) { + private ChunkInfo(int dataId, int start, int end) { this.dataId = dataId; - this.requestDataLength = requestDataLength; + this.start = start; + this.end = end; } } @@ -113,11 +119,11 @@ public class DataTransferHandler { // TODO Wouldn't it be better to store data as streams? // Because now we have to store the whole data in RAM. private final byte[] data; - private final AtomicInteger dataLeft; + private final TreeMap chunksReceivedByDevice; private Data(byte[] data) { this.data = data; - this.dataLeft = new AtomicInteger(data.length); + chunksReceivedByDevice = new TreeMap<>(); } private byte[] getDataChunk(final int offset, final int maxChunkSize) { @@ -127,11 +133,28 @@ public class DataTransferHandler { return Arrays.copyOfRange(data, offset, Math.min(offset + maxChunkSize, data.length)); } - private int onDataSuccessfullyReceived(int chunkSize) { - // TODO Does this work properly? - // Problems can arise when the app receives two ACKs for the same data. - // It can be solved by storing information about what data was ACKed instead of just dataLeft variable. - return dataLeft.addAndGet(-chunkSize); + private void onDataChunkSuccessfullyReceived(ChunkInfo newlyReceivedChunk) { + final ChunkInfo alreadyReceivedChunk = chunksReceivedByDevice.get(newlyReceivedChunk.start); + if (alreadyReceivedChunk == null || alreadyReceivedChunk.end < newlyReceivedChunk.end) { + chunksReceivedByDevice.put(newlyReceivedChunk.start, newlyReceivedChunk); + } + } + + private boolean isDataSuccessfullySent() { + Integer previousChunkEnd = null; + for (Map.Entry chunkEntry : chunksReceivedByDevice.entrySet()) { + if (previousChunkEnd == null && chunkEntry.getKey() != 0) { + // The head of the data wasn't received by the device. + return false; + } + if (previousChunkEnd != null && chunkEntry.getKey() > previousChunkEnd) { + // There is some gap between received chunks. + return false; + } + previousChunkEnd = chunkEntry.getValue().end; + } + // Check if the end of the last chunk matches the data size. + return previousChunkEnd != null && data.length == previousChunkEnd; } } } diff --git a/app/src/main/java/nodomain/freeyourgadget/gadgetbridge/service/devices/garmin/http/EphemerisHandler.java b/app/src/main/java/nodomain/freeyourgadget/gadgetbridge/service/devices/garmin/http/EphemerisHandler.java index dc549073e..67cf710a1 100644 --- a/app/src/main/java/nodomain/freeyourgadget/gadgetbridge/service/devices/garmin/http/EphemerisHandler.java +++ b/app/src/main/java/nodomain/freeyourgadget/gadgetbridge/service/devices/garmin/http/EphemerisHandler.java @@ -21,6 +21,7 @@ public class EphemerisHandler { } public byte[] handleEphemerisRequest(final String path, final Map query) { + // TODO Return status code 304 (Not Modified) when we don't have newer data and "if-none-match" is set. try { final File exportDirectory = deviceSupport.getWritableExportDirectory(); final File ephemerisDataFile = new File(exportDirectory, "CPE.BIN");