Ignore the previous pod's reservoir volume after a pod change

LoopFollow/Controllers/Nightscout/DeviceStatus.swift

@@ -118,7 +118,7 @@ extension MainViewController {
                     manufacturer: lastPumpRecord["manufacturer"] as? String,
                     model: lastPumpRecord["model"] as? String,
                     cache: Storage.shared.pumpReservoirCache.value,
-                    now: Date()
+                    date: Date(timeIntervalSince1970: lastPumpTime)
                 )
                 Storage.shared.pumpReservoirCache.value = reservoir.cache
 

LoopFollow/Controllers/Nightscout/PumpReservoir.swift

@@ -14,6 +14,24 @@ struct PumpReservoirCache: Codable, Equatable {
     /// When this pump first appeared in a device status record LoopFollow fetched.
     let pumpSince: Date
     var reading: Reading?
+    /// Whether a volume in a record is this pump's own. Right after a pod change Loop
+    /// still uploads the previous pod's final volume.
+    var isSettled: Bool
+
+    init(pumpID: String, pumpSince: Date, reading: Reading? = nil, isSettled: Bool = true) {
+        self.pumpID = pumpID
+        self.pumpSince = pumpSince
+        self.reading = reading
+        self.isSettled = isSettled
+    }
+
+    init(from decoder: Decoder) throws {
+        let container = try decoder.container(keyedBy: CodingKeys.self)
+        pumpID = try container.decode(String.self, forKey: .pumpID)
+        pumpSince = try container.decode(Date.self, forKey: .pumpSince)
+        reading = try container.decodeIfPresent(Reading.self, forKey: .reading)
+        isSettled = try container.decodeIfPresent(Bool.self, forKey: .isSettled) ?? true
+    }
 }
 
 /// What a device status record says about the reservoir.
@@ -31,8 +49,9 @@ enum PumpReservoirResolver {
     /// while the pod is below 50U. A reading carries across those gaps for this long.
     static let maxReadingAge: TimeInterval = 30 * 60
 
-    /// A volume reported within this long of a pump first appearing can still be the
-    /// previous pod's final reading, so it says nothing about the pump now on.
+    /// Loop uploads its last reservoir reading until it is 15 minutes old, whichever pod it
+    /// came from, so the previous pod's final volume can appear for up to this long after a
+    /// pod change. A new pod reports no volume of its own until it drops below 50U.
     static let pumpSettleTime: TimeInterval = 15 * 60
 
     struct Resolution: Equatable {
@@ -41,59 +60,66 @@ enum PumpReservoirResolver {
         let cache: PumpReservoirCache?
     }
 
+    /// - Parameter date: When the record was made, from the pump clock it carries.
     static func resolve(
         reservoir: Double?,
         pumpID: String?,
         manufacturer: String?,
         model: String?,
         cache storedCache: PumpReservoirCache?,
-        now: Date
+        date: Date
     ) -> Resolution {
-        let withoutReading: PumpReservoirState = reportsVolumeOnlyWhenLow(manufacturer: manufacturer, model: model)
-            ? .aboveReportingLimit
-            : .unknown
+        let onlyWhenLow = reportsVolumeOnlyWhenLow(manufacturer: manufacturer, model: model)
+        let withoutReading: PumpReservoirState = onlyWhenLow ? .aboveReportingLimit : .unknown
+
+        let pumpID = pumpID?.trimmingCharacters(in: .whitespacesAndNewlines) ?? ""
+
+        // Loop reports an Omnipod as "Unknown" while no pod is paired, so a volume in the
+        // record is the previous pod's. Medtrum is always "Unknown" on Loop.
+        if pumpID == "Unknown", onlyWhenLow {
+            return Resolution(state: withoutReading, cache: storedCache)
+        }
 
         // A reading is carried between records only when the uploader names the pump it
-        // came from, so that a pod change discards it.
-        guard let pumpID = identifiedPump(pumpID) else {
-            guard let reservoir else { return Resolution(state: withoutReading, cache: nil) }
-            return Resolution(state: .units(reservoir), cache: nil)
+        // came from, so that a pod change discards it. The stored pump is kept across
+        // records that name none, so the next one reads as a change.
+        guard !pumpID.isEmpty, pumpID != "Unknown" else {
+            guard let reservoir else { return Resolution(state: withoutReading, cache: storedCache) }
+            return Resolution(state: .units(reservoir), cache: storedCache)
         }
 
-        var cache = storedCache?.pumpID == pumpID
-            ? storedCache!
-            : PumpReservoirCache(pumpID: pumpID, pumpSince: now, reading: nil)
+        var cache: PumpReservoirCache
+        if let storedCache, storedCache.pumpID == pumpID {
+            cache = storedCache
+        } else {
+            // With no pump stored there is no change to have seen, so the pump is taken
+            // as one already in use.
+            cache = PumpReservoirCache(pumpID: pumpID, pumpSince: date, isSettled: storedCache == nil || !onlyWhenLow)
+        }
+
+        if !cache.isSettled, reservoir == nil || date.timeIntervalSince(cache.pumpSince) >= pumpSettleTime {
+            cache.isSettled = true
+        }
 
         if let reservoir {
-            cache.reading = PumpReservoirCache.Reading(units: reservoir, date: now)
+            guard cache.isSettled else { return Resolution(state: withoutReading, cache: cache) }
+            cache.reading = PumpReservoirCache.Reading(units: reservoir, date: date)
             return Resolution(state: .units(reservoir), cache: cache)
         }
 
         if let reading = cache.reading {
-            let age = now.timeIntervalSince(reading.date)
+            let age = date.timeIntervalSince(reading.date)
             if age >= 0, age <= maxReadingAge {
                 return Resolution(state: .units(reading.units), cache: cache)
             }
-            if reading.date.timeIntervalSince(cache.pumpSince) >= pumpSettleTime {
-                // The pump has reported a volume long enough after coming online for that
-                // to be its own, so it is below its reporting limit and the only thing
-                // missing is a fresh number. A pump first seen mid-pod has no such gap,
-                // so its first reading is treated as a pod change's and dropped below.
-                return Resolution(state: .unknown, cache: cache)
-            }
-            cache.reading = nil
+            // The pump has reported a volume of its own, so it is below its reporting limit
+            // and the only thing missing is a fresh number.
+            return Resolution(state: .unknown, cache: cache)
         }
 
         return Resolution(state: withoutReading, cache: cache)
     }
 
-    private static func identifiedPump(_ pumpID: String?) -> String? {
-        let trimmed = pumpID?.trimmingCharacters(in: .whitespacesAndNewlines) ?? ""
-        // Loop reports "Unknown" while no pod is paired.
-        guard !trimmed.isEmpty, trimmed != "Unknown" else { return nil }
-        return trimmed
-    }
-
     private static func reportsVolumeOnlyWhenLow(manufacturer: String?, model: String?) -> Bool {
         let pump = [manufacturer, model].compactMap { $0 }.joined(separator: " ").lowercased()
         // An uploader that names no pump cannot be told apart from one that reports its

Tests/PumpReservoirTests.swift

@@ -23,7 +23,7 @@ struct PumpReservoirTests {
             manufacturer: manufacturer,
             model: model,
             cache: cache,
-            now: date ?? now
+            date: date ?? now
         )
     }
 
@@ -70,17 +70,59 @@ struct PumpReservoirTests {
         #expect(resolve(cache: cache(units: 9.9, readingAge: -60 * 60)).state == .unknown)
     }
 
-    @Test("a volume reported right after a pod change is shown but not trusted once stale")
-    func podChangeCarryoverIsDiscarded() {
+    @Test("a volume reported right after a pod change is the previous pod's and is not shown")
+    func podChangeCarryoverIsIgnored() {
         // Loop's first records for a new pod still carry the previous pod's final volume.
-        let carryover = resolve(reservoir: 9.9, cache: PumpReservoirCache(pumpID: "17CB71F6", pumpSince: now.addingTimeInterval(-3 * 24 * 60 * 60), reading: nil))
-        #expect(carryover.state == .units(9.9))
-        #expect(carryover.cache?.pumpID == pump)
+        let previousPod = PumpReservoirCache(pumpID: "17CB71F6", pumpSince: now.addingTimeInterval(-3 * 24 * 60 * 60), reading: .init(units: 25.6, date: now.addingTimeInterval(-5 * 60)))
+        var result = resolve(reservoir: 25.5, cache: previousPod)
+        #expect(result.state == .aboveReportingLimit)
+        #expect(result.cache?.pumpID == pump)
+        #expect(result.cache?.reading == nil)
+
+        result = resolve(reservoir: 25.5, cache: result.cache, at: now.addingTimeInterval(10 * 60))
+        #expect(result.state == .aboveReportingLimit)
+        #expect(result.cache?.reading == nil)
+
+        result = resolve(cache: result.cache, at: now.addingTimeInterval(15 * 60))
+        #expect(result.state == .aboveReportingLimit)
+        #expect(result.cache?.isSettled == true)
+    }
+
+    @Test("a new pod's volumes count once it has uploaded a record without one")
+    func podChangeSettlesOnMissingVolume() {
+        let changed = resolve(reservoir: 25.5, cache: PumpReservoirCache(pumpID: "17CB71F6", pumpSince: now.addingTimeInterval(-3 * 24 * 60 * 60)))
+        let gap = resolve(cache: changed.cache, at: now.addingTimeInterval(5 * 60))
+        let reported = resolve(reservoir: 48.5, cache: gap.cache, at: now.addingTimeInterval(10 * 60))
+        #expect(reported.state == .units(48.5))
+        #expect(reported.cache?.reading?.units == 48.5)
+    }
+
+    @Test("a new pod's volumes count 15 minutes after it appeared")
+    func podChangeSettlesAfterTime() {
+        let changed = resolve(reservoir: 25.5, cache: PumpReservoirCache(pumpID: "17CB71F6", pumpSince: now.addingTimeInterval(-3 * 24 * 60 * 60)))
+        #expect(resolve(reservoir: 25.5, cache: changed.cache, at: now.addingTimeInterval(14 * 60)).state == .aboveReportingLimit)
+        #expect(resolve(reservoir: 48.5, cache: changed.cache, at: now.addingTimeInterval(15 * 60)).state == .units(48.5))
+    }
+
+    @Test("a pump change is seen across records that name no pump")
+    func podChangeAcrossUnknown() {
+        let previousPod = PumpReservoirCache(pumpID: "17CB71F6", pumpSince: now.addingTimeInterval(-3 * 24 * 60 * 60), reading: .init(units: 43.3, date: now.addingTimeInterval(-10 * 60)))
+        let pairing = resolve(pumpID: "Unknown", cache: previousPod, at: now.addingTimeInterval(-5 * 60))
+        #expect(pairing.cache == previousPod)
+        #expect(resolve(reservoir: 42.9, cache: pairing.cache).state == .aboveReportingLimit)
+    }
 
-        let laterOnTheSamePod = resolve(cache: carryover.cache, at: now.addingTimeInterval(31 * 60))
-        #expect(laterOnTheSamePod.state == .aboveReportingLimit)
-        #expect(laterOnTheSamePod.cache?.pumpID == pump)
-        #expect(laterOnTheSamePod.cache?.reading == nil)
+    @Test("a pump seen with nothing stored has its volume taken as its own")
+    func firstSeenPumpIsSettled() {
+        let result = resolve(reservoir: 25.5)
+        #expect(result.state == .units(25.5))
+        #expect(result.cache?.isSettled == true)
+    }
+
+    @Test("a pump that reports its volume in every record is used right after a change")
+    func otherPumpChangeIsSettled() {
+        let previous = PumpReservoirCache(pumpID: "17CB71F6", pumpSince: now.addingTimeInterval(-3 * 24 * 60 * 60))
+        #expect(resolve(reservoir: 180, manufacturer: "Medtronic", model: "723", cache: previous).state == .units(180))
     }
 
     @Test("a pod change drops the previous pod's reading")
@@ -119,10 +161,26 @@ struct PumpReservoirTests {
         #expect(withoutVolume.cache == nil)
     }
 
-    @Test("a pump with no pod paired is not an identity to cache against")
+    @Test("an Omnipod with no pod paired reads as 50+ and keeps the stored pump")
     func unknownPumpID() {
-        let result = resolve(pumpID: "Unknown", cache: cache(units: 9.9, readingAge: 5 * 60))
+        let stored = cache(units: 9.9, readingAge: 5 * 60)
+        let result = resolve(pumpID: "Unknown", cache: stored)
+        #expect(result.state == .aboveReportingLimit)
+        #expect(result.cache == stored)
+    }
+
+    @Test("a volume uploaded while no pod is paired is the previous pod's")
+    func unknownPumpIDWithVolume() {
+        let stored = cache(units: 12.6, readingAge: 5 * 60)
+        let result = resolve(reservoir: 12.5, pumpID: "Unknown", cache: stored)
         #expect(result.state == .aboveReportingLimit)
+        #expect(result.cache == stored)
+    }
+
+    @Test("a pump Loop names Unknown that reports its volume in every record shows it")
+    func unknownPumpIDOtherPump() {
+        let result = resolve(reservoir: 185.6, pumpID: "Unknown", manufacturer: "Medtrum", model: "MD8201")
+        #expect(result.state == .units(185.6))
         #expect(result.cache == nil)
     }
 
@@ -148,4 +206,12 @@ struct PumpReservoirTests {
         let decoded = try JSONDecoder().decode(PumpReservoirCache.self, from: JSONEncoder().encode(original))
         #expect(decoded == original)
     }
+
+    @Test("a cache stored without the settled flag decodes as settled")
+    func cacheDecodesWithoutSettledFlag() throws {
+        let json = #"{"pumpID":"17CB71F7","pumpSince":0}"#
+        let decoded = try JSONDecoder().decode(PumpReservoirCache.self, from: Data(json.utf8))
+        #expect(decoded.isSettled)
+        #expect(decoded.reading == nil)
+    }
 }