From 8c9006e635350afb1e01ae535ec6c3f3ae9be9ce Mon Sep 17 00:00:00 2001 From: Maciej Korzeniewski Date: Tue, 3 Dec 2024 11:00:25 +0200 Subject: [PATCH] Fix removeOfflineRecord so that it does not rely on class-level fileDeletion map Additionally: 1. Fixed data directory deletion. Now it works as follow: a) iterate over the parent directory of deleted offline recording (e.g. /U/0/20241203/R/094150) and add files to filesList, b) if filesList is empty, remove directory. Previously it was worked as follow: a) iterate over all offline recording files and add them to filesList, b) if filesList is empty, remove data directory. The problem with previous approach was that it checking all data directories rather than the one from which offline recording gets removed. 2. Added some comments for better understand what the code does. 3. Added more logs. --- .../java/com/polar/sdk/impl/BDBleApiImpl.kt | 65 ++++++++++++++----- 1 file changed, 48 insertions(+), 17 deletions(-) diff --git a/sources/Android/android-communications/library/src/sdk/java/com/polar/sdk/impl/BDBleApiImpl.kt b/sources/Android/android-communications/library/src/sdk/java/com/polar/sdk/impl/BDBleApiImpl.kt index 579b63b2..5c910f12 100644 --- a/sources/Android/android-communications/library/src/sdk/java/com/polar/sdk/impl/BDBleApiImpl.kt +++ b/sources/Android/android-communications/library/src/sdk/java/com/polar/sdk/impl/BDBleApiImpl.kt @@ -1271,34 +1271,56 @@ class BDBleApiImpl private constructor(context: Context, features: Set = mutableMapOf() + val parentDirectory = entry.path.substringBeforeLast("/") + val client = session.fetchClient(BlePsFtpUtils.RFC77_PFTP_SERVICE) as BlePsFtpClient? ?: return Completable.error(PolarServiceNotAvailable()) val fsType = getFileSystemType(session.polarDeviceType) return if (fsType == FileSystemType.SAGRFC2_FILE_SYSTEM) { + /// Fetch the sub recording count to iterate over all subrecording and delete them one by one. + /// + /// Context: Offline recordings are stored in batches of sub recordings. For example, + /// HR recording /U/0/20241203/R/094150/HR.REC may be actually stored as the following + /// sub recording files: + /// - /U/0/20241203/R/094150/HR0.REC, + /// - /U/0/20241203/R/094150/HR1.REC, + /// - ... + /// + /// Note that the file /U/0/20241203/R/094150/HR.REC doesn't really exist on the device, + /// unless it's the old format where the recording is stored in a single file (no sub recordings). getSubRecordingCount(identifier, entry) .flatMap { count -> + BleLogger.d(TAG, "Remove offline record from device $identifier at path ${entry.path}: sub recording count $count") + if (count == 0 || entry.path.contains(Regex("""(\D+)(\d+)\.REC"""))) { val builder = PftpRequest.PbPFtpOperation.newBuilder() builder.command = PftpRequest.PbPFtpOperation.Command.REMOVE builder.path = entry.path return@flatMap client.request(builder.build().toByteArray()) } else { + /// Example entry.path for HR recording: /U/0/20241203/R/094150/HR.REC Observable.fromIterable(0 until count) - .flatMap { subRecordingIndex -> + /// Use concatMap to make sure that the sub recordings are removed one by one. + .concatMap { subRecordingIndex -> + /// Example sub recording paths for HR recording: + /// - /U/0/20241203/R/094150/HR0.REC + /// - /U/0/20241203/R/094150/HR1.REC + /// - /U/0/20241203/R/094150/HR2.REC + /// - ... val recordingPath = entry.path.replace( Regex("(\\.REC)$"), "$subRecordingIndex.REC" ) - - fileDeletionMap.put(listOf(recordingPath.split("/") - .subList(0, recordingPath.split("/") - .lastIndex - 1))[0].joinToString(separator = "/"), false) + + BleLogger.d(TAG, "Remove offline record from device $identifier at path ${entry.path}: removing sub recording $recordingPath") + val builder = PftpRequest.PbPFtpOperation.newBuilder() builder.command = PftpRequest.PbPFtpOperation.Command.REMOVE builder.path = recordingPath @@ -1306,30 +1328,40 @@ class BDBleApiImpl private constructor(context: Context, features: Set = mutableListOf() listFiles(identifier, dir, condition = { entry: String -> - entry.matches(Regex("^(\\d{8})(/)")) || - entry.matches(Regex("^(\\d{6})(/)")) || - entry == "R/" || - entry.contains(".REC") && - !entry.contains(".BPB") && - !entry.contains("HIST") + entry.startsWith(parentDirectory) }) .map { fileList.add(it) } .doFinally { + BleLogger.d(TAG, "Remove offline record from device $identifier path ${entry.path}: files in parentDirectory: $fileList") + if (fileList.isEmpty()) { - deleteDataDirectories(identifier).subscribe() + BleLogger.d(TAG, "Remove offline record from device $identifier path ${entry.path}: parent directory $parentDirectory is empty, removing it.") + deleteDataDirectories(identifier, fileDeletionMap).subscribe() } } .doOnError { error -> BleLogger.e( TAG, - "Failed to list files from directory $dir from device $identifier. Error: $error" + "Remove offline record from device $identifier path ${entry.path}: failed to list files in parent directory $parentDirectory, error: $error", ) Completable.error(error) } @@ -2586,8 +2618,7 @@ class BDBleApiImpl private constructor(context: Context, features: Set { - + private fun deleteDataDirectories(identifier: String, fileDeletionMap: MutableMap): Flowable { return Flowable.fromIterable(fileDeletionMap.asIterable()) .map { file -> val dir = listOf(file.key.split("/").subList(0, file.key.split("/").lastIndex))[0].joinToString(separator = "/")