Skip to content

fix intermittent crash on deleting carbs #591

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Sep 25, 2017
Merged

fix intermittent crash on deleting carbs #591

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
62 changes: 32 additions & 30 deletions Loop/View Controllers/CarbAbsorptionViewController.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -80,18 +80,7 @@ final class CarbAbsorptionViewController: ChartsTableViewController, Identifiabl

private var refreshContext = RefreshContext.all

private var chartStartDate: Date {
get {
return charts.startDate
}
set {
if newValue != chartStartDate {
refreshContext = RefreshContext.all
}

charts.startDate = newValue
}
}
private var reloading = false

private var carbStatuses: [CarbStatus] = []

Expand All @@ -101,29 +90,36 @@ final class CarbAbsorptionViewController: ChartsTableViewController, Identifiabl

// MARK: - Data loading

override func reloadData(animated: Bool) {
guard active && !self.refreshContext.isEmpty else { return }
override func reloadData(animated: Bool = false) {
guard active && !reloading && !self.refreshContext.isEmpty else { return }
var currentContext = self.refreshContext
var retryContext: Set<RefreshContext> = []
self.refreshContext = []
reloading = true

// How far back should we show data? Use the screen size as a guide.
let minimumSegmentWidth: CGFloat = 75

let size = self.refreshContext.removeNewSize() ?? self.tableView.bounds.size
let size = currentContext.newSize ?? self.tableView.bounds.size
let availableWidth = size.width - self.charts.fixedHorizontalMargin
let totalHours = floor(Double(availableWidth / minimumSegmentWidth))

var components = DateComponents()
components.minute = 0
let date = Date(timeIntervalSinceNow: -TimeInterval(hours: max(1, totalHours)))
chartStartDate = Calendar.current.nextDate(after: date, matching: components, matchingPolicy: .strict, direction: .backward) ?? date
let chartStartDate = Calendar.current.nextDate(after: date, matching: components, matchingPolicy: .strict, direction: .backward) ?? date
if charts.startDate != chartStartDate {
currentContext.formUnion(RefreshContext.all)
}
charts.startDate = chartStartDate

let midnight = Calendar.current.startOfDay(for: Date())
let listStart = min(midnight, chartStartDate)

let reloadGroup = DispatchGroup()
let shouldUpdateGlucose = self.refreshContext.remove(.glucose) != nil
let shouldUpdateCarbs = self.refreshContext.remove(.carbs) != nil
let shouldUpdateGlucose = currentContext.contains(.glucose)
let shouldUpdateCarbs = currentContext.contains(.carbs)

var refreshContext = self.refreshContext
var carbEffects: [GlucoseEffect]?
var carbStatuses: [CarbStatus]?
var carbsOnBoard: CarbValue?
Expand All @@ -135,12 +131,12 @@ final class CarbAbsorptionViewController: ChartsTableViewController, Identifiabl
self.charts.glucoseUnit = unit
}

_ = refreshContext.remove(.status)
// TODO: Don't always assume currentContext.contains(.status)
reloadGroup.enter()
self.deviceManager.loopManager.getLoopState { (manager, state) in
if shouldUpdateGlucose || shouldUpdateCarbs {
let insulinCounteractionEffects = state.insulinCounteractionEffects
self.charts.setInsulinCounteractionEffects(state.insulinCounteractionEffects.filterDateRange(self.chartStartDate, nil))
self.charts.setInsulinCounteractionEffects(state.insulinCounteractionEffects.filterDateRange(chartStartDate, nil))

reloadGroup.enter()
manager.carbStore.getCarbStatus(start: listStart, effectVelocities: manager.settings.dynamicCarbAbsorptionEnabled ? insulinCounteractionEffects : nil) { (result) in
Expand All @@ -150,27 +146,27 @@ final class CarbAbsorptionViewController: ChartsTableViewController, Identifiabl
carbsOnBoard = status.clampedCarbsOnBoard
case .failure(let error):
self.deviceManager.logger.addError(error, fromSource: "CarbStore")
refreshContext.update(with: .carbs)
retryContext.update(with: .carbs)
}

reloadGroup.leave()
}

reloadGroup.enter()
manager.carbStore.getGlucoseEffects(start: self.chartStartDate, effectVelocities: manager.settings.dynamicCarbAbsorptionEnabled ? insulinCounteractionEffects : nil) { (result) in
manager.carbStore.getGlucoseEffects(start: chartStartDate, effectVelocities: manager.settings.dynamicCarbAbsorptionEnabled ? insulinCounteractionEffects : nil) { (result) in
switch result {
case .success(let effects):
carbEffects = effects
case .failure(let error):
carbEffects = []
self.deviceManager.logger.addError(error, fromSource: "CarbStore")
refreshContext.update(with: .carbs)
retryContext.update(with: .carbs)
}
reloadGroup.leave()
}
}

if refreshContext.remove(.targets) != nil {
if currentContext.contains(.targets) {
if let schedule = manager.settings.glucoseTargetRangeSchedule {
self.charts.targetPointsCalculator = GlucoseRangeScheduleCalculator(schedule)
} else {
Expand All @@ -189,7 +185,7 @@ final class CarbAbsorptionViewController: ChartsTableViewController, Identifiabl
carbTotal = total
case .failure(let error):
self.deviceManager.logger.addError(error, fromSource: "CarbStore")
refreshContext.update(with: .carbs)
retryContext.update(with: .carbs)
}

reloadGroup.leave()
Expand All @@ -200,7 +196,6 @@ final class CarbAbsorptionViewController: ChartsTableViewController, Identifiabl
}

reloadGroup.notify(queue: .main) {
self.refreshContext = refreshContext
if let carbEffects = carbEffects {
self.charts.setCarbEffects(carbEffects)
}
Expand All @@ -227,6 +222,15 @@ final class CarbAbsorptionViewController: ChartsTableViewController, Identifiabl
if let cell = self.tableView.cellForRow(at: IndexPath(row: 0, section: Section.totals.rawValue)) as? HeaderValuesTableViewCell {
self.updateCell(cell)
}

self.reloading = false
let reloadNow = !self.refreshContext.isEmpty
self.refreshContext.formUnion(retryContext)

// Trigger a reload if new context exists.
if reloadNow {
self.reloadData()
}
}
}

Expand Down Expand Up @@ -431,9 +435,7 @@ final class CarbAbsorptionViewController: ChartsTableViewController, Identifiabl

public override func tableView(_ tableView: UITableView, commit editingStyle: UITableViewCellEditingStyle, forRowAt indexPath: IndexPath) {
if editingStyle == .delete {
let status = carbStatuses.remove(at: indexPath.row)
tableView.deleteRows(at: [indexPath], with: .automatic)

let status = carbStatuses[indexPath.row]
deviceManager.loopManager.carbStore.deleteCarbEntry(status.entry) { (success, error) -> Void in
DispatchQueue.main.async {
if success {
Expand Down
24 changes: 15 additions & 9 deletions Loop/View Controllers/StatusTableViewController.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -427,13 +427,12 @@ final class StatusTableViewController: ChartsTableViewController {
self.tableView.endUpdates()

self.reloading = false
let reloadNow = !self.refreshContext.isEmpty
self.refreshContext.formUnion(retryContext)

// Trigger a reload if new context exists.
if !self.refreshContext.isEmpty {
self.refreshContext.formUnion(retryContext)
if reloadNow {
self.reloadData()
} else {
// If our only context is retry, wait for the next trigger
self.refreshContext.formUnion(retryContext)
}
}
}
Expand Down Expand Up @@ -739,10 +738,17 @@ final class StatusTableViewController: ChartsTableViewController {
override func tableView(_ tableView: UITableView, heightForRowAt indexPath: IndexPath) -> CGFloat {
switch Section(rawValue: indexPath.section)! {
case .charts:
// 20: Status bar
// 70: HUD
// 44: Toolbar
let availableSize = max(tableView.bounds.width, tableView.bounds.height) - 20 - 70 - 44
// Compute the height of the HUD, defaulting to 70
let hudHeight = ceil(hudView?.systemLayoutSizeFitting(UILayoutFittingCompressedSize).height ?? 70)
var availableSize = max(tableView.bounds.width, tableView.bounds.height)

if #available(iOS 11.0, *) {
availableSize -= (tableView.safeAreaInsets.top + tableView.safeAreaInsets.bottom + hudHeight)
} else {
// 20: Status bar
// 44: Toolbar
availableSize -= hudHeight + 20 + 44
}

switch ChartRow(rawValue: indexPath.row)! {
case .glucose:
Expand Down