Skip to content

nate/fix/LOOP-1927/manual-glucose-entry #256

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
nhamming merged 13 commits into from
Sep 22, 2020
Merged

nate/fix/LOOP-1927/manual-glucose-entry #256

Show file tree
Hide file tree
Changes from 6 commits
Commits
Show all changes
13 commits
Select commit Hold shift + click to select a range
8a38b70
allow manual glucose entry without carb entry
nhamming Sep 16, 2020
0997628
the manual glucose entry displayed in the CGM pill is never overridden
nhamming Sep 16, 2020
f85bf26
Merge branch 'dev' into nate/fix/LOOP-1927/manual-glucose-entry
nhamming Sep 17, 2020
e45842d
manually entered glucose needs interpretation from the CGM manager to…
nhamming Sep 18, 2020
4f3e896
Merge branch 'dev' into nate/fix/LOOP-1927/manual-glucose-entry
nhamming Sep 18, 2020
85ff522
update to glucose icon display if manual glucose entry
nhamming Sep 18, 2020
7efb717
updating the display of a manual glucose entry within the CGM pill
nhamming Sep 21, 2020
188a34a
Merge branch 'dev' into nate/fix/LOOP-1927/manual-glucose-entry
nhamming Sep 22, 2020
fefdcb9
the status bar determines when it should adjust its layout for narrow…
nhamming Sep 22, 2020
2dce93d
updated tests
nhamming Sep 22, 2020
2d5d36a
clean up
nhamming Sep 22, 2020
51b99f0
added hardcoded glucose range categorization when feature is disabled
nhamming Sep 22, 2020
82056ff
DIY categorization glucose range for both manual and CGM values
nhamming Sep 22, 2020
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
13 changes: 7 additions & 6 deletions Loop Status Extension/StatusViewController.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,11 +25,6 @@ class StatusViewController: UIViewController, NCWidgetProviding {
hudView.cgmStatusHUD.tintColor = .label
hudView.pumpStatusHUD.tintColor = .insulinTintColor
hudView.backgroundColor = .clear

if view.bounds.width < 375 {
// need to adjust for narrow display
hudView.adjustViewsForNarrowDisplay = true
}

// given the reduced width of the widget, allow for tighter spacing
hudView.containerView.spacing = 6.0
Expand Down Expand Up @@ -142,6 +137,11 @@ class StatusViewController: UIViewController, NCWidgetProviding {
observers = [
// TODO: Observe cross-process notifications of Loop status updating
]

if view.bounds.width < 375 {
// need to adjust for narrow display
hudView.adjustViewsForNarrowDisplay = true
Copy link

@ps2 ps2 Sep 18, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can't the hud adjust its view itself if it's not given enough room? Adjust views depending on the size given would seem to be the view's responsibility, not a user of the view.

Copy link

@rickpasetto rickpasetto Sep 21, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+1

Copy link
Author

@nhamming nhamming Sep 22, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Overlooked this comment. Will do.

}
}

deinit {
Expand Down Expand Up @@ -305,7 +305,8 @@ class StatusViewController: UIViewController, NCWidgetProviding {
at: lastGlucose.startDate,
unit: unit,
staleGlucoseAge: recencyInterval,
sensor: context.sensor
sensor: context.sensor,
wasUserEntered: lastGlucose.wasUserEntered
)
}

Expand Down
8 changes: 6 additions & 2 deletions Loop.xcodeproj/project.pbxproj
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -408,6 +408,7 @@
B405E35924D2A75B00DD058D /* DerivedAssets.xcassets in Resources */ = {isa = PBXBuildFile; fileRef = A966152923EA5A37005D8B29 /* DerivedAssets.xcassets */; };
B405E35A24D2B1A400DD058D /* HUDAssets.xcassets in Resources */ = {isa = PBXBuildFile; fileRef = 4F2C15961E09E94E00E160D4 /* HUDAssets.xcassets */; };
B405E35B24D2E05600DD058D /* HUDAssets.xcassets in Resources */ = {isa = PBXBuildFile; fileRef = 4F2C15961E09E94E00E160D4 /* HUDAssets.xcassets */; };
B41B2BB32514E9C5006F7014 /* ManualGlucoseState.swift in Sources */ = {isa = PBXBuildFile; fileRef = B41B2BB22514E9C4006F7014 /* ManualGlucoseState.swift */; };
B42C951424A3C76000857C73 /* CGMStatusHUDViewModel.swift in Sources */ = {isa = PBXBuildFile; fileRef = B42C951324A3C76000857C73 /* CGMStatusHUDViewModel.swift */; };
B43DA44124D9C12100CAFF4E /* DismissibleHostingController.swift in Sources */ = {isa = PBXBuildFile; fileRef = B43DA44024D9C12100CAFF4E /* DismissibleHostingController.swift */; };
B47A791C2508009E006C0E11 /* ChartAxisValuesStaticGenerator.swift in Sources */ = {isa = PBXBuildFile; fileRef = B47A791B2508009E006C0E11 /* ChartAxisValuesStaticGenerator.swift */; };
Expand Down Expand Up @@ -1246,6 +1247,7 @@
A9F703722489BC8500C98AD8 /* CarbStore+SimulatedCoreData.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = "CarbStore+SimulatedCoreData.swift"; sourceTree = "<group>"; };
A9F703742489C9A000C98AD8 /* GlucoseStore+SimulatedCoreData.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = "GlucoseStore+SimulatedCoreData.swift"; sourceTree = "<group>"; };
A9F703762489D8AA00C98AD8 /* PersistentDeviceLog+SimulatedCoreData.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = "PersistentDeviceLog+SimulatedCoreData.swift"; sourceTree = "<group>"; };
B41B2BB22514E9C4006F7014 /* ManualGlucoseState.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = ManualGlucoseState.swift; sourceTree = "<group>"; };
B42C951324A3C76000857C73 /* CGMStatusHUDViewModel.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = CGMStatusHUDViewModel.swift; sourceTree = "<group>"; };
B42C951624A3CAF200857C73 /* NotificationsCriticalAlertPermissionsViewModel.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = NotificationsCriticalAlertPermissionsViewModel.swift; sourceTree = "<group>"; };
B43DA44024D9C12100CAFF4E /* DismissibleHostingController.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = DismissibleHostingController.swift; sourceTree = "<group>"; };
Expand Down Expand Up @@ -1567,11 +1569,12 @@
C17824A41E1AD4D100D9D25C /* BolusRecommendation.swift */,
43C2FAE01EB656A500364AFF /* GlucoseEffectVelocity.swift */,
436A0DA41D236A2A00104B24 /* LoopError.swift */,
E9C00EF424C623EF00628F35 /* LoopSettings+Loop.swift */,
4F526D601DF8D9A900A04910 /* NetBasal.swift */,
B41B2BB22514E9C4006F7014 /* ManualGlucoseState.swift */,
438D42F81D7C88BC003244B0 /* PredictionInputEffect.swift */,
4328E0311CFC068900E199AA /* WatchContext+LoopKit.swift */,
C165B8CD23302C5D0004112E /* RemoteCommand.swift */,
E9C00EF424C623EF00628F35 /* LoopSettings+Loop.swift */,
4328E0311CFC068900E199AA /* WatchContext+LoopKit.swift */,
);
path = Models;
sourceTree = "<group>";
Expand Down Expand Up @@ -3270,6 +3273,7 @@
1DDE273E24AEA4B000796622 /* SettingsView.swift in Sources */,
A9B607B0247F000F00792BE4 /* UserNotifications+Loop.swift in Sources */,
43F89CA322BDFBBD006BB54E /* UIActivityIndicatorView.swift in Sources */,
B41B2BB32514E9C5006F7014 /* ManualGlucoseState.swift in Sources */,
A999D40624663D18004C89D4 /* PumpManagerError.swift in Sources */,
437D9BA31D7BC977007245E8 /* PredictionTableViewController.swift in Sources */,
4344628F20A7ADD500C4BE6F /* UserDefaults+CGM.swift in Sources */,
Expand Down
9 changes: 9 additions & 0 deletions Loop/Extensions/DeviceDataManager+DeviceStatus.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,15 @@ import LoopKitUI

extension DeviceDataManager {
var cgmStatusHighlight: DeviceStatusHighlight? {
if let latestGlucose = glucoseStore.latestGlucose {
guard !latestGlucose.wasUserEntered ||
(latestGlucose.wasUserEntered && Date().timeIntervalSince(latestGlucose.startDate) > loopManager.settings.inputDataRecencyInterval) else
{
// do not override display of relevant manual glucose entry
return nil
}
}

if bluetoothState == .poweredOff {
return BluetoothStateManager.bluetoothOffHighlight
} else if bluetoothState == .denied ||
Expand Down
10 changes: 8 additions & 2 deletions Loop/Managers/DeviceDataManager.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -536,8 +536,14 @@ extension DeviceDataManager {
return pumpManager?.status
}

var sensorState: SensorDisplayable? {
return cgmManager?.sensorState
func sensorState(for glucose: GlucoseSampleValue?) -> SensorDisplayable? {
Copy link

@ps2 ps2 Sep 18, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sensorState -> glucoseDisplay?

if let glucose = glucose, glucose.wasUserEntered {
// the CGM manager needs to determine the glucoseValueType for a manual glucose based on its managed glucose thresholds
let glucoseValueType = (cgmManager as? CGMManagerUI)?.glucoseValueType(for: glucose)
Copy link

@ps2 ps2 Sep 18, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's probably not right that we're asking the CGM to categorize the manual glucose value. It suggest that maybe glucose categorization should be a Loop concern. What do we do if we don't have a CGM? If this is too big, let's not tackle it here, but I think this is a sign we need to refactor glucose range categorization.

Copy link
Author

@nhamming nhamming Sep 18, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ps2 The scenario when the CGM is not available was reported to design. They agree that ideally Loop manages the user glucose thresholds. However, they don't have a big concern with the style of the manually entered glucose being normal in the scenario when no CGM is setup. However, they would like to confirm there is no risk in a build at some point.

return ManualGlucoseState(glucoseValueType: glucoseValueType)
} else {
return cgmManager?.sensorState
}
}

func updatePumpManagerBLEHeartbeatPreference() {
Expand Down
2 changes: 1 addition & 1 deletion Loop/Managers/StatusExtensionDataManager.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -106,7 +106,7 @@ final class StatusExtensionDataManager {
context.batteryPercentage = dataManager.pumpManager?.status.pumpBatteryChargeRemaining
context.reservoirCapacity = dataManager.pumpManager?.pumpReservoirCapacity

if let sensorInfo = dataManager.sensorState {
if let sensorInfo = dataManager.sensorState(for: dataManager.glucoseStore.latestGlucose) {
context.sensor = SensorDisplayableContext(
isStateValid: sensorInfo.isStateValid,
stateDescription: sensorInfo.stateDescription,
Expand Down
2 changes: 1 addition & 1 deletion Loop/Managers/WatchDataManager.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -228,7 +228,7 @@ final class WatchDataManager: NSObject {
context.loopLastRunDate = manager.lastLoopCompleted
context.recommendedBolusDose = state.recommendedBolus?.recommendation.amount
context.cob = state.carbsOnBoard?.quantity.doubleValue(for: HKUnit.gram())
context.glucoseTrendRawValue = self.deviceManager.sensorState?.trendType?.rawValue
context.glucoseTrendRawValue = self.deviceManager.sensorState(for: glucose)?.trendType?.rawValue

context.cgmManagerState = self.deviceManager.cgmManager?.rawValue

Expand Down
24 changes: 24 additions & 0 deletions Loop/Models/ManualGlucoseState.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
//
// ManualGlucoseState.swift
// Loop
//
// Created by Nathaniel Hamming on 2020-09-18.
// Copyright © 2020 LoopKit Authors. All rights reserved.
//

import Foundation
import LoopKit

struct ManualGlucoseState: SensorDisplayable {
Copy link

@ps2 ps2 Sep 18, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ManualGlucoseState -> ManualGlucoseDisplayable? And SensorDisplayable -> GlucoseDisplayable? The latter is LoopKit, I know, and maybe we do it later, but if this protocol is used to display both sensor and manual glucose data, the name could use updating.

Copy link
Author

@nhamming nhamming Sep 18, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll definitely update GlucoseDisplayable now. I was confused when writing the code and happy to take this suggestion.

let isStateValid: Bool
let trendType: GlucoseTrend?
let isLocal: Bool
let glucoseValueType: GlucoseValueType?

init(glucoseValueType: GlucoseValueType?) {
isStateValid = true
trendType = nil
isLocal = true
self.glucoseValueType = glucoseValueType
}
}
4 changes: 2 additions & 2 deletions Loop/View Controllers/StatusTableViewController.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -516,9 +516,9 @@ final class StatusTableViewController: LoopChartsTableViewController {
at: glucose.startDate,
unit: unit,
staleGlucoseAge: self.deviceManager.loopManager.settings.inputDataRecencyInterval,
sensor: self.deviceManager.sensorState)
sensor: self.deviceManager.sensorState(for: glucose),
wasUserEntered: glucose.wasUserEntered)
}

hudView.cgmStatusHUD.presentStatusHighlight(self.deviceManager.cgmStatusHighlight)
hudView.cgmStatusHUD.lifecycleProgress = self.deviceManager.cgmLifecycleProgress

Expand Down
2 changes: 2 additions & 0 deletions Loop/View Models/BolusEntryViewModel.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -237,6 +237,8 @@ final class BolusEntryViewModel: ObservableObject {
}
} else if potentialCarbEntry != nil { // Allow user to save carbs without bolusing
self.continueSaving(onSuccess: completion)
} else if manualGlucoseSample != nil { // Allow user to save the manual glucose sample without bolusing
self.continueSaving(onSuccess: completion)
} else {
completion()
}
Expand Down
9 changes: 7 additions & 2 deletions LoopUI/ViewModel/CGMStatusHUDViewModel.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,8 @@ public class CGMStatusHUDViewModel {

var glucoseTrendTintColor: UIColor = .glucoseTintColor

var isManualGlucose: Bool = false

var staleGlucoseValueHandler: (String) -> Void

var isVisible: Bool = true {
Expand Down Expand Up @@ -73,7 +75,8 @@ public class CGMStatusHUDViewModel {
at glucoseStartDate: Date,
unit: HKUnit,
staleGlucoseAge: TimeInterval,
sensor: SensorDisplayable?)
sensor: SensorDisplayable?,
isManualGlucose: Bool)
{
var accessibilityStrings = [String]()

Expand Down Expand Up @@ -108,7 +111,9 @@ public class CGMStatusHUDViewModel {
}

glucoseValueTintColor = sensor?.glucoseValueType?.glucoseColor ?? .label
glucoseTrendTintColor = sensor?.glucoseValueType?.trendColor ?? .glucoseTintColor
glucoseTrendTintColor = isManualGlucose ? .destructive : sensor?.glucoseValueType?.trendColor ?? .glucoseTintColor
Copy link

@ps2 ps2 Sep 18, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm, when the sensor is in a warmup state, and the user enters a manual glucose value, there is not really any "error" that needs attending to. I guess this is a question for design/product. It seems like communicating the value as a manual entry is important, but painting it in error/warning colors when everything is operating as expected and user attention is not required is not great.

Copy link

@ps2 ps2 Sep 18, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I understand that when the CGM is failing in some way, we don't want to paint over the failing state with a manual glucose entry that makes everything look ok.

Copy link
Author

@nhamming nhamming Sep 18, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ps2 yes, I'm working with Matt to determine all the possible states. Currently the known states include a red !, red or glucose colour +, and glucose colour <clock>, but we are still exploring.


self.isManualGlucose = isManualGlucose

unitsString = unit.localizedShortUnitString
accessibilityString = accessibilityStrings.joined(separator: ", ")
Expand Down
7 changes: 5 additions & 2 deletions LoopUI/Views/CGMStatusHUDView.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -90,20 +90,23 @@ public final class CGMStatusHUDView: DeviceStatusHUDView, NibLoadable {
at glucoseStartDate: Date,
unit: HKUnit,
staleGlucoseAge: TimeInterval,
sensor: SensorDisplayable?)
sensor: SensorDisplayable?,
wasUserEntered: Bool)
{
viewModel.setGlucoseQuantity(glucoseQuantity,
at: glucoseStartDate,
unit: unit,
staleGlucoseAge: staleGlucoseAge,
sensor: sensor)
sensor: sensor,
isManualGlucose: wasUserEntered)

glucoseValueHUD.glucoseLabel.text = viewModel.glucoseValueString
glucoseValueHUD.unitLabel.text = viewModel.unitsString
glucoseValueHUD.tintColor = viewModel.glucoseValueTintColor

glucoseTrendHUD.setTrend(viewModel.trend)
glucoseTrendHUD.tintColor = viewModel.glucoseTrendTintColor
glucoseTrendHUD.setManualGlucoseIcon(viewModel.isManualGlucose)

accessibilityValue = viewModel.accessibilityString
}
Expand Down
7 changes: 7 additions & 0 deletions LoopUI/Views/GlucoseTrendHUDView.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,4 +33,11 @@ public final class GlucoseTrendHUDView: BaseHUDView {
public func setTrend(_ trend: GlucoseTrend?) {
trendIcon.image = trend?.image ?? UIImage(systemName: "questionmark.circle")
}

public func setManualGlucoseIcon(_ isManualGlucose: Bool) {
if isManualGlucose {
trendIcon.image = UIImage(systemName: "exclamationmark.circle.fill")
trendIcon.tintColor = .destructive
}
}
}