Merge pull request #893 from nightscout/fix-first-pod-alarm

Fix First Pod Alarm + Refactor Alert History Storage

Trio/Sources/APS/DeviceDataManager.swift

@@ -596,7 +596,7 @@ extension BaseDeviceDataManager: PumpManagerDelegate {
 
 extension BaseDeviceDataManager: DeviceManagerDelegate {
     func issueAlert(_ alert: Alert) {
-        alertHistoryStorage.storeAlert(
+        alertHistoryStorage.addAlert(
             AlertEntry(
                 alertIdentifier: alert.identifier.alertIdentifier,
                 primitiveInterruptionLevel: alert.interruptionLevel.storedValue as? Decimal,
@@ -611,7 +611,7 @@ extension BaseDeviceDataManager: DeviceManagerDelegate {
     }
 
     func retractAlert(identifier: Alert.Identifier) {
-        alertHistoryStorage.deleteAlert(identifier: identifier.alertIdentifier)
+        alertHistoryStorage.removeAlert(identifier: identifier.alertIdentifier)
     }
 
     func doesIssuedAlertExist(identifier _: Alert.Identifier, completion _: @escaping (Result<Bool, Error>) -> Void) {
@@ -685,23 +685,23 @@ extension BaseDeviceDataManager: AlertObserver {
             if let omnipodBLE = self.pumpManager as? OmniBLEPumpManager {
                 if omnipodBLE.state.activeAlerts.isEmpty {
                     // force to ack alert in the alertStorage
-                    self.alertHistoryStorage.ackAlert(alertIssueDate, nil)
+                    self.alertHistoryStorage.acknowledgeAlert(alertIssueDate, nil)
                 }
             }
 
             if let omniPod = self.pumpManager as? OmnipodPumpManager {
                 if omniPod.state.activeAlerts.isEmpty {
                     // force to ack alert in the alertStorage
-                    self.alertHistoryStorage.ackAlert(alertIssueDate, nil)
+                    self.alertHistoryStorage.acknowledgeAlert(alertIssueDate, nil)
                 }
             }
 
             self.pumpManager?.acknowledgeAlert(alertIdentifier: alert.alertIdentifier) { error in
                 if let error = error {
-                    self.alertHistoryStorage.ackAlert(alertIssueDate, error.localizedDescription)
+                    self.alertHistoryStorage.acknowledgeAlert(alertIssueDate, error.localizedDescription)
                     debug(.deviceManager, "acknowledge not succeeded with error \(error)")
                 } else {
-                    self.alertHistoryStorage.ackAlert(alertIssueDate, nil)
+                    self.alertHistoryStorage.acknowledgeAlert(alertIssueDate, nil)
                 }
             }
 

Trio/Sources/APS/Storage/AlertStorage.swift

@@ -8,96 +8,252 @@ protocol AlertObserver {
 }
 
 protocol AlertHistoryStorage {
-    func storeAlert(_ alerts: AlertEntry)
+    func addAlert(_ alert: AlertEntry)
+    func acknowledgeAlert(_ issuedAt: Date, _ error: String?)
+    func removeAlert(identifier: String)
+    func unacknowledgedAlertsWithinLast24Hours() -> [AlertEntry]
+    func broadcastAlertUpdates()
     func syncDate() -> Date
-    func recentNotAck() -> [AlertEntry]
-    func deleteAlert(identifier: String)
-    func ackAlert(_ alert: Date, _ error: String?)
-    func forceNotification()
-    var alertNotAck: PassthroughSubject<Bool, Never> { get }
+    var unacknowledgedAlertsPublisher: PassthroughSubject<Bool, Never> { get }
 }
 
 final class BaseAlertHistoryStorage: AlertHistoryStorage, Injectable {
     private let processQueue = DispatchQueue.markedQueue(label: "BaseAlertsStorage.processQueue")
-    @Injected() private var storage: FileStorage!
+
+    private let defaults: UserDefaults
+
+    /// Legacy JSON file storage used only for one-time migration from the historical on-disk JSON file.
+    // FIXME: this can be removed in later releases
+    @Injected() private var fileStorage: FileStorage!
+
     @Injected() private var broadcaster: Broadcaster!
 
-    let alertNotAck = PassthroughSubject<Bool, Never>()
+    /// Emits `true` whenever there is at least one unacknowledged alert in the last 24 hours.
+    let unacknowledgedAlertsPublisher = PassthroughSubject<Bool, Never>()
 
-    init(resolver: Resolver) {
+    private enum Keys {
+        /// UserDefaults key holding the encoded `[AlertEntry]` payload.
+        static let alertsData = "openaps.monitor.alertHistory.data"
+        /// UserDefaults key used as a one-time migration flag.
+        static let alertsMigrationDone = "openaps.monitor.alertHistory.migrated"
+    }
+
+    /// Creates a new alert history storage.
+    ///
+    /// On initialization this performs a one-time migration from the legacy JSON file
+    /// (`OpenAPS.Monitor.alertHistory`, i.e.,`"monitor/alerthistory.json"`) into UserDefaults.
+    /// After initialization, all reads/writes happen via UserDefaults only.
+    ///
+    /// - Parameters:
+    ///   - resolver: Swinject resolver used for dependency injection.
+    ///   - userDefaults: The UserDefaults instance used for persistence. Defaults to `.standard`.
+    init(resolver: Resolver, userDefaults: UserDefaults = .standard) {
+        defaults = userDefaults
         injectServices(resolver)
-        alertNotAck.send(recentNotAck().isNotEmpty)
+
+        // FIXME: this can be removed in later releases
+        migrateFromLegacyJSONIfNeeded()
+
+        unacknowledgedAlertsPublisher.send(unacknowledgedAlertsWithinLast24Hours().isNotEmpty)
     }
 
-    func storeAlert(_ alert: AlertEntry) {
+    /// Stores a new alert entry and notifies observers.
+    ///
+    /// The history is:
+    /// - de-duplicated by `issuedDate`
+    /// - pruned to the last 24 hours
+    /// - sorted with newest first
+    ///
+    /// After persisting, this updates `unacknowledgedAlertsPublisher` and broadcasts the latest list to `AlertObserver`s.
+    /// - Parameter alert: The alert to store.
+    func addAlert(_ alert: AlertEntry) {
         processQueue.sync {
-            let file = OpenAPS.Monitor.alertHistory
-            var uniqEvents: [AlertEntry] = []
-            self.storage.transaction { storage in
-                storage.append(alert, to: file, uniqBy: \.issuedDate)
-                uniqEvents = storage.retrieve(file, as: [AlertEntry].self)?
-                    .filter { $0.issuedDate.addingTimeInterval(1.days.timeInterval) > Date() }
-                    .sorted { $0.issuedDate > $1.issuedDate } ?? []
-                storage.save(Array(uniqEvents), as: file)
-            }
-            alertNotAck.send(self.recentNotAck().isNotEmpty)
+            var all = loadAll()
+            all.append(alert)
+
+            let uniqEvents = pruneAndSort(dedupeByIssuedDate(all))
+            saveAll(uniqEvents)
+
+            unacknowledgedAlertsPublisher.send(self.unacknowledgedAlertsWithinLast24HoursOnQueue().isNotEmpty)
             broadcaster.notify(AlertObserver.self, on: processQueue) {
                 $0.AlertDidUpdate(uniqEvents)
             }
         }
     }
 
+    /// Returns the baseline sync date used by the alert subsystem.
+    ///
+    /// This matches the previous behavior: one day ago from "now".
     func syncDate() -> Date {
         Date().addingTimeInterval(-1.days.timeInterval)
     }
 
-    func recentNotAck() -> [AlertEntry] {
-        storage.retrieve(OpenAPS.Monitor.alertHistory, as: [AlertEntry].self)?
+    /// Returns all unacknowledged alerts from the last 24 hours, sorted newest first.
+    func unacknowledgedAlertsWithinLast24Hours() -> [AlertEntry] {
+        processQueue.sync {
+            self.unacknowledgedAlertsWithinLast24HoursOnQueue()
+        }
+    }
+
+    /// Returns all unacknowledged alerts from the last 24 hours, sorted newest first.
+    /// - Important: Must only be called while already executing on `processQueue`.
+    private func unacknowledgedAlertsWithinLast24HoursOnQueue() -> [AlertEntry] {
+        loadAll()
             .filter { $0.issuedDate.addingTimeInterval(1.days.timeInterval) > Date() && $0.acknowledgedDate == nil }
-            .sorted { $0.issuedDate > $1.issuedDate } ?? []
+            .sorted { $0.issuedDate > $1.issuedDate }
     }
 
-    func ackAlert(_ alert: Date, _ error: String?) {
+    /// Acknowledges an alert (by issued date), or stores an error for it.
+    ///
+    /// If `error` is non-nil, the alert is updated with `errorMessage`.
+    /// Otherwise, the alert is marked as acknowledged by setting `acknowledgedDate = Date()`.
+    ///
+    /// After persisting, this updates `unacknowledgedAlertsPublisher`.
+    /// - Parameters:
+    ///   - issuedAt: The issued date of the alert entry to update.
+    ///   - error: Optional error message to store instead of acknowledging.
+    func acknowledgeAlert(_ issuedAt: Date, _ error: String?) {
         processQueue.sync {
-            var allValues = storage.retrieve(OpenAPS.Monitor.alertHistory, as: [AlertEntry].self) ?? []
-            guard let entryIndex = allValues.firstIndex(where: { $0.issuedDate == alert }) else {
-                return
-            }
+            var all = loadAll()
+            guard let idx = all.firstIndex(where: { $0.issuedDate == issuedAt }) else { return }
 
             if let error {
-                allValues[entryIndex].errorMessage = error
+                all[idx].errorMessage = error
             } else {
-                allValues[entryIndex].acknowledgedDate = Date()
+                all[idx].acknowledgedDate = Date()
             }
-            storage.save(allValues, as: OpenAPS.Monitor.alertHistory)
-            alertNotAck.send(self.recentNotAck().isNotEmpty)
+
+            let cleaned = pruneAndSort(dedupeByIssuedDate(all))
+            saveAll(cleaned)
+            unacknowledgedAlertsPublisher.send(self.unacknowledgedAlertsWithinLast24HoursOnQueue().isNotEmpty)
         }
     }
 
-    func deleteAlert(identifier: String) {
+    /// Deletes an alert entry by its identifier and notifies observers.
+    ///
+    /// After persisting, this updates `unacknowledgedAlertsPublisher` and broadcasts the updated list.
+    /// - Parameter identifier: The `alertIdentifier` of the entry to delete.
+    func removeAlert(identifier: String) {
         processQueue.sync {
-            var allValues = storage.retrieve(OpenAPS.Monitor.alertHistory, as: [AlertEntry].self) ?? []
-            guard let entryIndex = allValues.firstIndex(where: { $0.alertIdentifier == identifier }) else {
-                return
-            }
-            allValues.remove(at: entryIndex)
-            storage.save(allValues, as: OpenAPS.Monitor.alertHistory)
-            alertNotAck.send(self.recentNotAck().isNotEmpty)
+            var all = loadAll()
+            guard let idx = all.firstIndex(where: { $0.alertIdentifier == identifier }) else { return }
+
+            all.remove(at: idx)
+
+            let cleaned = pruneAndSort(dedupeByIssuedDate(all))
+            saveAll(cleaned)
+
+            unacknowledgedAlertsPublisher.send(self.unacknowledgedAlertsWithinLast24HoursOnQueue().isNotEmpty)
             broadcaster.notify(AlertObserver.self, on: processQueue) {
-                $0.AlertDidUpdate(allValues)
+                $0.AlertDidUpdate(cleaned)
             }
         }
     }
 
-    func forceNotification() {
+    /// Forces a broadcast of the current alert list (last 24 hours) to observers.
+    ///
+    /// This does not modify the data; it only re-emits state via `unacknowledgedAlertsPublisher` and `AlertObserver`.
+    func broadcastAlertUpdates() {
         processQueue.sync {
-            let uniqEvents = storage.retrieve(OpenAPS.Monitor.alertHistory, as: [AlertEntry].self)?
-                .filter { $0.issuedDate.addingTimeInterval(1.days.timeInterval) > Date() }
-                .sorted { $0.issuedDate > $1.issuedDate } ?? []
-            alertNotAck.send(self.recentNotAck().isNotEmpty)
+            let uniqEvents = pruneAndSort(loadAll())
+            unacknowledgedAlertsPublisher.send(self.unacknowledgedAlertsWithinLast24HoursOnQueue().isNotEmpty)
             broadcaster.notify(AlertObserver.self, on: processQueue) {
                 $0.AlertDidUpdate(uniqEvents)
             }
         }
     }
+
+    // MARK: - Migration
+
+    /// Migrates alert history from the legacy on-disk JSON file into UserDefaults.
+    ///
+    /// Migration behavior:
+    /// - Runs at most once per install (guarded by `Keys.alertsMigrationDone`).
+    /// - If the new UserDefaults value already exists, migration is considered complete.
+    /// - If legacy alerts exist, they are normalized (dedupe/prune/sort) and stored in UserDefaults.
+    /// - After a successful migration, the legacy file is removed to avoid future drift.
+    private func migrateFromLegacyJSONIfNeeded() { // FIXME: this can be removed in later releases
+        processQueue.sync {
+            // Avoid repeated disk reads forever
+            if defaults.bool(forKey: Keys.alertsMigrationDone) { return }
+
+            // If new store already has data, consider migration done
+            if defaults.data(forKey: Keys.alertsData) != nil {
+                defaults.set(true, forKey: Keys.alertsMigrationDone)
+                return
+            }
+
+            // Read legacy file ("monitor/alerthistory.json") via existing FileStorage
+            let legacyJsonAlerts = fileStorage.retrieve(OpenAPS.Monitor.alertHistory, as: [AlertEntry].self) ?? []
+            guard legacyJsonAlerts.isNotEmpty else {
+                defaults.set(true, forKey: Keys.alertsMigrationDone)
+                return
+            }
+
+            // Normalize before persisting
+            let migrated = pruneAndSort(dedupeByIssuedDate(legacyJsonAlerts))
+            saveAll(migrated)
+
+            // Mark complete FIRST, then cleanup
+            defaults.set(true, forKey: Keys.alertsMigrationDone)
+
+            // Cleanup: remove legacy json so it cannot drift / get re-used accidentally
+            fileStorage.remove(OpenAPS.Monitor.alertHistory)
+        }
+    }
+
+    // MARK: - UserDefaults persistence
+
+    // Uses the same encoder/decoder as file storage to keep Date encoding consistent.
+
+    /// Loads all persisted alerts from UserDefaults.
+    ///
+    /// Decoding uses `JSONCoding.decoder` to match the previous on-disk JSON encoding/decoding behavior.
+    /// If decoding fails, the stored payload is removed so the app can recover cleanly.
+    private func loadAll() -> [AlertEntry] {
+        guard let data = defaults.data(forKey: Keys.alertsData) else { return [] }
+        do {
+            return try JSONCoding.decoder.decode([AlertEntry].self, from: data)
+        } catch {
+            debug(.storage, "Failed to decode alerts from UserDefaults: \(error)")
+            // Clear corrupt payload so app can recover
+            defaults.removeObject(forKey: Keys.alertsData)
+            return []
+        }
+    }
+
+    /// Persists all alerts to UserDefaults.
+    ///
+    /// Encoding uses `JSONCoding.encoder` to match the previous on-disk JSON encoding behavior.
+    private func saveAll(_ alerts: [AlertEntry]) {
+        do {
+            let data = try JSONCoding.encoder.encode(alerts)
+            defaults.set(data, forKey: Keys.alertsData)
+        } catch {
+            debug(.storage, "Failed to encode alerts to UserDefaults: \(error)")
+        }
+    }
+
+    // MARK: - Helpers
+
+    /// Filters the provided alerts to the last 24 hours and sorts them with newest first.
+    private func pruneAndSort(_ alerts: [AlertEntry]) -> [AlertEntry] {
+        alerts
+            .filter { $0.issuedDate.addingTimeInterval(1.days.timeInterval) > Date() }
+            .sorted { $0.issuedDate > $1.issuedDate }
+    }
+
+    /// De-duplicates alert entries by `issuedDate` (keeping the newest occurrence when duplicates exist).
+    ///
+    /// This matches `AlertEntry`'s `Equatable`/`Hashable` semantics (both based on `issuedDate`).
+    private func dedupeByIssuedDate(_ alerts: [AlertEntry]) -> [AlertEntry] {
+        var seen = Set<Date>()
+        var result: [AlertEntry] = []
+        for item in alerts.sorted(by: { $0.issuedDate > $1.issuedDate }) {
+            if seen.insert(item.issuedDate).inserted {
+                result.append(item)
+            }
+        }
+        return result
+    }
 }

Trio/Sources/Localizations/Main/Localizable.xcstrings

@@ -10037,6 +10037,7 @@
       }
     },
     "%lld h" : {
+      "extractionState" : "stale",
       "localizations" : {
         "bg" : {
           "stringUnit" : {
@@ -132071,6 +132072,9 @@
         }
       }
     },
+    "Important: Autosens Min and Autosens Max do not affect Temp Targets in the same way. Autosens Max limits how much insulin can be increased, but Autosens Min does not remove the 15% minimum when insulin is reduced." : {
+
+    },
     "Importing Settings..." : {
       "comment" : "Progress text when importing settings via Nightscout",
       "localizations" : {
@@ -169879,10 +169883,6 @@
         }
       }
     },
-    "Note: Autosens Min and Autosens Max settings do not apply symmetrically to Temp Target sensitivity adjustments. Autosens Max limits how much sensitivity can be decreased (more insulin), but Autosens Min does not override the 15% floor for increased sensitivity (less insulin)." : {
-      "comment" : "A note explaining the asymmetry in how Autosens Min and Max affect Temp Target sensitivity adjustments.",
-      "isCommentAutoGenerated" : true
-    },
     "Note: Basal may be resumed if there is negative IOB and glucose is rising faster than the forecast." : {
       "localizations" : {
         "bg" : {
@@ -202135,9 +202135,8 @@
         }
       }
     },
-    "Sensitivity adjustments from Temp Targets have a hard-coded minimum of 15%. This means even very high Temp Targets cannot reduce insulin delivery below 15% of normal." : {
-      "comment" : "A description of the 15% floor for Temp Target sensitivity adjustments.",
-      "isCommentAutoGenerated" : true
+    "Sensitivity changes from Temp Targets have a built-in minimum of 15%. Even very high Temp Targets cannot reduce insulin delivery below 15% of normal." : {
+
     },
     "Sensitivity Limits" : {
       "comment" : "A label displayed above a section explaining the sensitivity limits of temp targets.",
@@ -232317,9 +232316,8 @@
         }
       }
     },
-    "This 15% floor is a safety limit inherited from oref (OpenAPS reference design) and AndroidAPS. It prevents Temp Targets from reducing insulin to dangerously low levels." : {
-      "comment" : "A text describing the 15% floor that prevents Temp Targets from reducing insulin to dangerously low levels.",
-      "isCommentAutoGenerated" : true
+    "This 15% minimum is a safety limit taken from oref (OpenAPS reference design) and AndroidAPS. It helps prevent insulin delivery from dropping to unsafe levels." : {
+
     },
     "This adjusted ISF is temporary, will change with the next loop cycle, and should not be directly used as your profile ISF value." : {
       "localizations" : {
@@ -232682,10 +232680,6 @@
         }
       }
     },
-    "This asymmetry exists because reducing insulin delivery during exercise, normally realized by using high Temp Targets, typically requires a higher insulin reduction than what autosens would identify in a regular dayly routine." : {
-      "comment" : "A note explaining the asymmetry in the Temp Target sensitivity adjustment rules.",
-      "isCommentAutoGenerated" : true
-    },
     "This cap prevents the system from overestimating how much insulin is needed when carb absorption isn't visible, offering a safeguard for accurate dosing." : {
       "localizations" : {
         "bg" : {
@@ -233158,6 +233152,9 @@
         }
       }
     },
+    "This difference exists because situations like exercise often need a much larger insulin reduction than Autosens would detect during a normal daily routine." : {
+
+    },
     "This dotted line represents the hourly insulin rate of your scheduled basal insulin." : {
       "localizations" : {
         "bg" : {

Trio/Sources/Modules/Onboarding/View/OnboardingSteps/TherapySettings/InsulinSensitivityStepView.swift

@@ -145,7 +145,8 @@ struct InsulinSensitivityStepView: View {
     private var isfChart: some View {
         Chart {
             ForEach(Array(state.isfItems.enumerated()), id: \.element.id) { index, item in
-                let displayValue = state.units == .mgdL ? state.isfRateValues[item.rateIndex] : state.isfRateValues[item.rateIndex].asMmolL
+                let displayValue = state.units == .mgdL ? state.isfRateValues[item.rateIndex] : state
+                    .isfRateValues[item.rateIndex].asMmolL
 
                 let startDate = Calendar.current
                     .startOfDay(for: now)

Trio/Sources/Modules/PumpConfig/PumpConfigProvider.swift

@@ -26,12 +26,12 @@ extension PumpConfig {
                 ?? PumpSettings(insulinActionCurve: 10, maxBolus: 10, maxBasal: 2)
         }
 
-        var alertNotAck: AnyPublisher<Bool, Never> {
-            deviceManager.alertHistoryStorage.alertNotAck.eraseToAnyPublisher()
+        var unacknowledgedAlertsPublisher: AnyPublisher<Bool, Never> {
+            deviceManager.alertHistoryStorage.unacknowledgedAlertsPublisher.eraseToAnyPublisher()
         }
 
-        func initialAlertNotAck() -> Bool {
-            deviceManager.alertHistoryStorage.recentNotAck().isNotEmpty
+        func hasInitialUnacknowledgedAlerts() -> Bool {
+            deviceManager.alertHistoryStorage.unacknowledgedAlertsWithinLast24Hours().isNotEmpty
         }
     }
 }

Trio/Sources/Modules/PumpConfig/PumpConfigStateModel.swift

@@ -9,7 +9,7 @@ extension PumpConfig {
         private(set) var setupPumpType: PumpType = .minimed
         @Published var pumpState: PumpDisplayState?
         private(set) var initialSettings: PumpInitialSettings = .default
-        @Published var alertNotAck: Bool = false
+        @Published var hasUnacknowledgedAlert: Bool = false
         @Injected() var bluetoothManager: BluetoothStateManager!
 
         override func subscribe() {
@@ -18,10 +18,10 @@ extension PumpConfig {
                 .assign(to: \.pumpState, on: self)
                 .store(in: &lifetime)
 
-            alertNotAck = provider.initialAlertNotAck()
-            provider.alertNotAck
+            hasUnacknowledgedAlert = provider.hasInitialUnacknowledgedAlerts()
+            provider.unacknowledgedAlertsPublisher
                 .receive(on: DispatchQueue.main)
-                .assign(to: \.alertNotAck, on: self)
+                .assign(to: \.hasUnacknowledgedAlert, on: self)
                 .store(in: &lifetime)
 
             Task {
@@ -49,7 +49,7 @@ extension PumpConfig {
         }
 
         func ack() {
-            provider.deviceManager.alertHistoryStorage.forceNotification()
+            provider.deviceManager.alertHistoryStorage.broadcastAlertUpdates()
         }
     }
 }

Trio/Sources/Modules/PumpConfig/View/PumpConfigRootView.swift

@@ -41,7 +41,7 @@ extension PumpConfig {
                                     .frame(maxWidth: .infinity, minHeight: 50, alignment: .center)
                                     .font(.title2)
                                 }.padding()
-                                if state.alertNotAck {
+                                if state.hasUnacknowledgedAlert {
                                     Spacer()
                                     Button("Acknowledge all alerts") { state.ack() }
                                 }