iOS: fix the three High OTA and profile-builder bugs (#141, #142, #143)

ios/Bisque/Bisque/Models/FiringSegment.swift

@@ -1,13 +1,70 @@
 import Foundation
 
+/// Smallest usable ramp rate, as a magnitude so cooling segments (negative
+/// rates) are held to the same floor.
+///
+/// Mirrors `MIN_ABS_RAMP_RATE_C_PER_HR` in `web_ui/src/app/types/kiln.ts`, so
+/// the two clients accept the same profiles. Deliberately stricter than the
+/// firmware, which only rejects zero and non-finite (`api_handlers.c`): 0.1°C/hr
+/// passes that check and describes a 5800-hour firing.
+let minAbsRampRateCPerHr: Double = 1
+
+/// Ceiling on the points in a whole charted profile, matching the web UI's
+/// `MAX_PROFILE_PATH_POINTS`. Divided across the segments by the path builder —
+/// applying it per segment would let a 16-segment profile reach 32,000 marks,
+/// which is what it exists to prevent.
+let maxProfilePathPoints = 2000
+
 struct FiringSegment: Codable, Identifiable, Hashable {
     let id: String
     var name: String
     var rampRate: Double    // degrees per hour
     var targetTemp: Double  // degrees C
     var holdTime: Double    // minutes (0 = hold indefinitely)
 
+    /// Whether the ramp arithmetic can be evaluated at all.
+    ///
+    /// The firmware's rule — finite, nonzero (`api_handlers.c`) — deliberately
+    /// looser than `validationError`. A profile already stored with, say,
+    /// 0.5°C/hr is one the controller accepts and will fire, so the chart has to
+    /// draw it: dropping it would misplace every later segment's temperature and
+    /// timing, which is worse than plotting a very slow ramp. Only zero and
+    /// non-finite values are genuinely uncomputable — those are the ones that
+    /// used to trap in `Int(_:)` (#143).
+    var isComputable: Bool {
+        rampRate.isFinite && rampRate != 0 && targetTemp.isFinite && holdTime.isFinite
+    }
+
+    /// Why this segment may not be *saved*, or nil if it is fine.
+    ///
+    /// The builder's policy, stricter than the firmware's: see
+    /// `minAbsRampRateCPerHr`. Not used to decide what to chart.
+    var validationError: String? {
+        guard rampRate.isFinite, targetTemp.isFinite, holdTime.isFinite else {
+            return "\(name): values must be numbers"
+        }
+        guard abs(rampRate) >= minAbsRampRateCPerHr else {
+            return "\(name): ramp rate must be at least \(Int(minAbsRampRateCPerHr))°C/hr to heat "
+                + "or -\(Int(minAbsRampRateCPerHr))°C/hr to cool"
+        }
+        guard targetTemp > 0, targetTemp <= 1400 else {
+            return "\(name): target temperature must be between 1 and 1400°C"
+        }
+        guard holdTime >= 0 else {
+            return "\(name): hold time cannot be negative"
+        }
+        return nil
+    }
+
     var formattedDescription: String {
-        "\(rampRate > 0 ? "+" : "")\(Int(rampRate))°C/hr → \(Int(targetTemp))°C\(holdTime > 0 ? ", hold \(Int(holdTime))m" : "")"
+        /* `Int(_:)` on a non-finite or out-of-range Double is a fatal error, not
+           a garbage number — the same trap as #143, reachable here from a
+           malformed profile decoded off the API rather than from the editor. */
+        func whole(_ v: Double) -> String {
+            v.isFinite && abs(v) < 1e9 ? String(Int(v)) : "—"
+        }
+        let sign = rampRate > 0 ? "+" : ""
+        let hold = holdTime > 0 ? ", hold \(whole(holdTime))m" : ""
+        return "\(sign)\(whole(rampRate))°C/hr → \(whole(targetTemp))°C\(hold)"
     }
 }

ios/Bisque/Bisque/Networking/KilnAPIClient.swift

@@ -60,6 +60,7 @@ struct OtaInstallResponse: Codable {
 actor KilnAPIClient {
     private let baseURL: URL
     private let session: URLSession
+    private let otaSession: URLSession
     private var apiToken: String?
 
     /// Escapes a value being spliced into a URL *path*.
@@ -86,6 +87,24 @@ actor KilnAPIClient {
         config.timeoutIntervalForRequest = 10
         config.timeoutIntervalForResource = 30
         self.session = URLSession(configuration: config)
+
+        /// OTA gets its own session because the timeouts above are wrong for it
+        /// by an order of magnitude (#142).
+        ///
+        /// A 1.5–2 MB image is accepted only as fast as the ESP32 can erase and
+        /// write flash, which routinely exceeds 30s — and `installOTA` returns
+        /// only after the device has pulled the image from GitHub. Under the
+        /// shared session URLSession cancelled the task at the resource
+        /// deadline, mid-transfer, leaving a partial OTA write and showing a
+        /// generic timeout.
+        ///
+        /// Keeping two sessions rather than relaxing the shared one preserves
+        /// the short deadlines where they belong: a control request that has not
+        /// answered in 10s should fail fast, not hang for ten minutes.
+        let otaConfig = URLSessionConfiguration.default
+        otaConfig.timeoutIntervalForRequest = 120
+        otaConfig.timeoutIntervalForResource = 600
+        self.otaSession = URLSession(configuration: otaConfig)
     }
 
     func setToken(_ token: String?) {
@@ -94,10 +113,13 @@ actor KilnAPIClient {
 
     // MARK: - Generic Request
 
+    /// `session:` overrides the short-deadline default for the OTA endpoints,
+    /// which legitimately take minutes — see `otaSession` (#142).
     private func request<T: Decodable>(
         method: String = "GET",
         path: String,
-        body: (any Encodable)? = nil
+        body: (any Encodable)? = nil,
+        session overrideSession: URLSession? = nil
     ) async throws -> T {
         guard let url = URL(string: baseURL.absoluteString + path) else {
             throw APIError.invalidURL
@@ -117,7 +139,7 @@ actor KilnAPIClient {
 
         let (data, response): (Data, URLResponse)
         do {
-            (data, response) = try await session.data(for: request)
+            (data, response) = try await (overrideSession ?? session).data(for: request)
         } catch is URLError {
             throw APIError.connectionFailed
         }
@@ -269,12 +291,20 @@ actor KilnAPIClient {
 
     // MARK: - OTA
 
-    func uploadOTA(fileURL: URL, onProgress: @Sendable @escaping (Double) -> Void) async throws -> OkResponse {
+    /// Uploads an already-read firmware image.
+    ///
+    /// Takes `Data` rather than a `URL` on purpose (#141). A file picked from
+    /// iCloud Drive or Files is reachable only while its security-scoped
+    /// resource is open, and that scope belongs to the picker callback — it
+    /// cannot survive being handed to a detached upload task. Reading the bytes
+    /// while the scope is held, then passing them here, makes the lifetime
+    /// impossible to get wrong rather than merely correct today.
+    func uploadOTA(firmware: Data, onProgress: @Sendable @escaping (Double) -> Void) async throws -> OkResponse {
         guard let url = URL(string: baseURL.absoluteString + "/ota") else {
             throw APIError.invalidURL
         }
 
-        let fileData = try Data(contentsOf: fileURL)
+        let fileData = firmware
 
         var request = URLRequest(url: url)
         request.httpMethod = "POST"
@@ -285,7 +315,7 @@ actor KilnAPIClient {
         }
 
         let delegate = UploadProgressDelegate(onProgress: onProgress)
-        let (data, response) = try await session.upload(for: request, from: fileData, delegate: delegate)
+        let (data, response) = try await otaSession.upload(for: request, from: fileData, delegate: delegate)
 
         guard let httpResponse = response as? HTTPURLResponse,
               (200...299).contains(httpResponse.statusCode) else {
@@ -296,12 +326,21 @@ actor KilnAPIClient {
         return try JSONDecoder().decode(OkResponse.self, from: data)
     }
 
+    /// Also on the OTA session. `handle_ota_check` waits synchronously for
+    /// `ota_check()` to fetch the release manifest from GitHub, bounded by
+    /// CONFIG_OTA_HTTP_TIMEOUT_MS — 15s by default and settable to 120s. Under
+    /// the shared session's 10s request deadline the app gave up before the
+    /// controller answered, so Install never appeared even when an update was
+    /// there (#142).
     func checkOTA() async throws -> OtaCheckResponse {
-        try await request(method: "POST", path: "/ota/check")
+        try await request(method: "POST", path: "/ota/check", session: otaSession)
     }
 
+    /// Uses the OTA session: this returns only once the device has pulled the
+    /// image from GitHub, which outlives the shared session's 30s resource
+    /// deadline (#142).
     func installOTA() async throws -> OtaInstallResponse {
-        try await request(method: "POST", path: "/ota/install")
+        try await request(method: "POST", path: "/ota/install", session: otaSession)
     }
 
     // MARK: - Diagnostics

ios/Bisque/Bisque/State/DashboardViewModel.swift

@@ -62,12 +62,31 @@ final class DashboardViewModel {
 
         path.append(TemperatureDataPoint(time: 0, temp: 20, target: 20))
 
+        /* Budget shared across the whole path, not handed to each segment.
+           The firmware allows 16 segments, so a per-segment allowance of
+           maxProfilePathPoints would admit ~32,000 freshly identified LineMarks
+           on every recompute — defeating the cap rather than enforcing it. */
+        let computableSegments = profile.segments.filter(\.isComputable).count
+        let perSegmentBudget = max(2, maxProfilePathPoints / max(1, computableSegments))
+
         for segment in profile.segments {
+            /* Skipped only when the arithmetic genuinely cannot run: a zero or
+               non-finite rate makes rampTimeMinutes infinite, and
+               `Int(Double.infinity)` is a fatal error in Swift, not a garbage
+               value — so charting such a profile crashed the app outright (#143).
+
+               Deliberately `isComputable` and not `validationError`: the latter
+               is the builder's stricter save policy, and applying it here would
+               drop slow-but-legal segments the controller has stored and is
+               firing, shifting every later segment's temperature and timing. */
+            guard segment.isComputable else { continue }
+
             let tempDifference = segment.targetTemp - currentTemp
             let rampTimeHours = abs(tempDifference) / abs(segment.rampRate)
             let rampTimeMinutes = rampTimeHours * 60
 
-            let steps = max(10, Int(rampTimeMinutes / 5))
+            // Never below 1: `1...steps` traps on an empty range.
+            let steps = max(1, min(perSegmentBudget, max(10, Int(rampTimeMinutes / 5))))
             for i in 1...steps {
                 let progress = Double(i) / Double(steps)
                 let stepTime = currentTime + rampTimeMinutes * progress

ios/Bisque/Bisque/State/ProfileBuilderViewModel.swift

@@ -15,14 +15,27 @@ final class ProfileBuilderViewModel {
     var conePreheat: Bool = true
     var coneSlowCool: Bool = false
 
+    /// Filtered to finite values: this is rendered via `Int(_:)`, which traps on
+    /// infinity rather than producing a wrong number (#143).
     var maxTemp: Double {
-        segments.map(\.targetTemp).max() ?? 0
+        segments.map(\.targetTemp).filter(\.isFinite).max() ?? 0
     }
 
+    /// Total planned minutes, skipping segments that cannot be computed.
+    ///
+    /// Dividing by `abs(rampRate)` makes this infinite for a rate of 0, and
+    /// `JSONEncoder` refuses to encode a non-finite `Double` — so the profile
+    /// save failed with an `EncodingError` that named neither the segment nor
+    /// the field (#143). Skipping keeps the figure finite; `validationError` is
+    /// what stops the save, with a message that says which segment is wrong.
+    ///
+    /// Filtered on `isComputable`, not `validationError`: a slow-but-legal rate
+    /// should contribute its (large, honest) duration rather than vanish from
+    /// the estimate.
     var estimatedDuration: Double {
         var totalMinutes: Double = 0
         var currentTemp: Double = 20
-        for segment in segments {
+        for segment in segments where segment.isComputable {
             let diff = abs(segment.targetTemp - currentTemp)
             let rampMinutes = (diff / abs(segment.rampRate)) * 60
             totalMinutes += rampMinutes + segment.holdTime
@@ -31,6 +44,14 @@ final class ProfileBuilderViewModel {
         return totalMinutes
     }
 
+    /// First reason the current draft cannot be saved, or nil if it is valid.
+    var validationError: String? {
+        if name.isEmpty || segments.isEmpty {
+            return "Profile needs a name and at least one segment"
+        }
+        return segments.compactMap(\.validationError).first
+    }
+
     func loadForEditing(_ profile: FiringProfile) {
         name = profile.name
         description = profile.description
@@ -58,8 +79,12 @@ final class ProfileBuilderViewModel {
     }
 
     func saveProfile(existingId: String?, using client: KilnAPIClient, store: KilnStore) async {
-        guard !name.isEmpty, !segments.isEmpty else {
-            error = "Profile needs a name and at least one segment"
+        /* Covers the name/segment-count check and every per-segment bound. The
+           ramp-rate case is the one that mattered: it used to reach the encoder
+           as an infinite estimatedDuration and fail with an EncodingError naming
+           nothing the user could act on (#143). */
+        if let problem = validationError {
+            error = problem
             return
         }
 

ios/Bisque/Bisque/State/SettingsViewModel.swift

@@ -91,14 +91,17 @@ final class SettingsViewModel {
 
     // MARK: - OTA
 
-    func uploadFirmware(fileURL: URL, using client: KilnAPIClient) async {
+    /// Takes the firmware bytes, not a file URL: the caller reads them while the
+    /// picked file's security scope is still open, which this task could not do
+    /// (#141).
+    func uploadFirmware(_ firmware: Data, using client: KilnAPIClient) async {
         isUploading = true
         otaProgress = 0
         otaMessage = nil
         error = nil
 
         do {
-            _ = try await client.uploadOTA(fileURL: fileURL) { [weak self] progress in
+            _ = try await client.uploadOTA(firmware: firmware) { [weak self] progress in
                 Task { @MainActor in
                     self?.otaProgress = progress
                 }

ios/Bisque/Bisque/Views/ProfileBuilder/SegmentEditorView.swift

@@ -36,6 +36,16 @@ struct SegmentEditorView: View {
                         .keyboardType(.decimalPad)
                 }
             }
+
+            /* Shown as the value is typed rather than only on Save. The rate
+               field is the one that used to crash the app once the profile was
+               charted (#143), so naming the problem where it is entered beats
+               rejecting it two screens later. */
+            if let problem = segment.validationError {
+                Text(problem)
+                    .font(.caption)
+                    .foregroundStyle(.red)
+            }
         }
         .padding(.vertical, 4)
     }

ios/Bisque/Bisque/Views/Settings/OTAUpdateView.swift

@@ -70,10 +70,28 @@ struct OTAUpdateView: View {
         .fileImporter(isPresented: $showFilePicker, allowedContentTypes: [.data]) { result in
             switch result {
             case .success(let url):
-                guard url.startAccessingSecurityScopedResource() else { return }
+                /* Read the bytes here, inside the scope, rather than handing the
+                   URL to the upload task (#141). `defer` fires when this closure
+                   returns — which is before a detached Task would have got as
+                   far as opening the file — so the read used to happen after
+                   access had been revoked. Anything outside the app sandbox
+                   (iCloud Drive, Files) failed with a permission error, which
+                   is to say manual OTA upload was broken for the normal case. */
+                guard url.startAccessingSecurityScopedResource() else {
+                    viewModel.error = "Could not get permission to read that file."
+                    return
+                }
                 defer { url.stopAccessingSecurityScopedResource() }
+
                 guard let client = connection.apiClient else { return }
-                Task { await viewModel.uploadFirmware(fileURL: url, using: client) }
+                let firmware: Data
+                do {
+                    firmware = try Data(contentsOf: url)
+                } catch {
+                    viewModel.error = "Could not read \(url.lastPathComponent): \(error.localizedDescription)"
+                    return
+                }
+                Task { await viewModel.uploadFirmware(firmware, using: client) }
             case .failure:
                 break
             }