Skip to content
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

Jump to bottom

Display the previous day's carb entries on the carb absorption screen. #2182

Open
wants to merge 1 commit into
base: dev
Choose a base branch
from
Open

Display the previous day's carb entries on the carb absorption screen. #2182

wants to merge 1 commit into from

Conversation

oliverstory
Copy link

The primary purpose of this change, as expressed in several comments on the issue report, is to allow review of carb absorption from the previous day.

This involves several changes:

  1. Change Loop to calculate insulin counteraction effects for the previous 48 hours (was 24 hours). This is necessary so that dynamic carb absorption can be calculated for the previous day's entries.

  2. Fetch carb entries up to the previous midnight, plus any entries that might still be absorbing at that time

  • previous day's entries are not shown prior to any counteraction effects if these are missing
  • counteraction effects could be missing if there are data gaps in glucose (I think). But I have not done any testing of this.
  • This design choice is driven by the primary purpose: reviewing dynamic glucose absorption. Without counteraction effects, entries will show as absorbing at their minimum rate, which could be misleading. But the lack of entries could create an inconsistent user experience, because entries will be shown for today and then disappear for 'yesterday'.

  1. Filter these entries to only display up to the previous midnight

  2. Add the (localised) phrase 'Yesterday' in front of the date for yesterday's entries

  • This seemed the simplest way to present the entries, and within my limited skills
  • A more skilled UI designer might be able to do something more sophisticated; but having all the entries on one screen is pretty straightforward.

I have tested this on a simulator over several days with flat glucose simulator.

To test, you can add a series of carb entries and:

  • bolus slightly more than the recommended amount for some. This will trigger dynamic absorption and the entry will show up as absorbing more than the entry.
  • bolus less than the recommended amount for others. This will trigger minimum rate absorption and the entry will show up as absorbing less than entered.

Closes #1861

@oliverstory
Display the previous day's carb entries on the carb absorption screen.
5b69a22 
This involves several changes:
1) Change Loop to calculate insulin counteraction effects for the previous 48 hours (was 24 hours).
This is necessary so that dynamic carb absorption can be calculated for the previous day's entries.

2) Fetch carb entries up to the previous midnight, plus any entries that might still be absorbing at that time
  - previous day's entries are not shown prior to any counteraction effects if these are missing

3) Filter these entries to only display up to the previous midnight

4) Add the (localised) phrase 'Yesterday' in front of the date for yesterday's entries
@marionbarker
Copy link
Contributor

Summary

This provided additional ICE screen rows.

  • expected result, ICE history extends back 48 hours instead of 24, but is displayed only from previous midnight

Questions?

  • Do we want to cut off at midnight? - what about late evening snackers, shift workers, night owls
  • The middle graphic is at 11 pm. If I had stayed up past midnight, the display would have appeared as it did in the right graphic, acquired the next morning

Test Details

Added this to my personal phone because my test phones don't have many carb entries.

  • iPhone 15, iOS 15.7.1
  • LoopWorkspace dev branch, commit 3542408
  • Grab screenshot (left side of graphic below, 10:58 PM)
  • Apply patches for both PR Use full ICE history for displaying estimated carbohydrate effects #2163 and this PR (2182) and build again (see other test details below)
  • Grab screenshot (middle of graphic below, 11:02 PM)
  • Next morning, grab screenshot (right of graphic below, 05:30 AM)

pr-2182-composite-01

Other Test Details

@oliverstory
Copy link
Author

I can adjust the thresholds easily enough. The midnight cut-off is a bit arbitrary but for most people means you have a break with no overlapping carb entries.

Options could be:

  • all of today, and at least 18 hours. This would show dinner the previous day, when you wake up, which was the main driver in people's requests
  • 24 hours. This lets you look at yesterday's entries up to 'now'.
  • 24 hours plus maximum absorption time i.e. 34 hours. This lets you look at entries that might still be absorbing at this time yesterday. Better than above because
  • all of today and yesterday (current patch)
  • 48 hours
  • etc etc

There was some previous discussion in #738

Some technical considerations are:

  • Whatever is shown on the display, entries that might still be absorbing should be fetched as well
  • In theory, we should keep going back until there is an entry that doesn't overlap with any previous entries. That's not done in current dev and also not done with this patch.
  • We need ICE to be available that overlaps with all the entries fetched. This is not 100% guaranteed with current dev and also not guaranteed with this patch; but could be fixed by extending cached ICE further back.

Without the patch, current loop dev shows entries up to and including midnight of today, plus anything that might still be absorbing on the left edge of the glucose change graph. So in the early part of the day you sometimes get the tail end of a late night snack of fat-protein entry. This disappears later in the day.

@ps2
Copy link
Collaborator

ps2 commented Jul 11, 2024

From a feature perspective I think this is fine. From a code perspective; the code here has already changed on the Tidepool side to work with the new LoopAlgorithm package. Once that is landed in dev, this should be updated to work against those changes, and then this can be landed.

marionbarker
marionbarker reviewed Jul 25, 2024
View reviewed changes
Loop/Managers/LoopDataManager.swift
@@ -992,7 +992,7 @@ extension LoopDataManager {

let retrospectiveStart = lastGlucoseDate.addingTimeInterval(-type(of: retrospectiveCorrection).retrospectionInterval)

let earliestEffectDate = Date(timeInterval: .hours(-24), since: now())
let earliestEffectDate = Date(timeInterval: .hours(-48), since: now())
Copy link
Contributor

@marionbarker marionbarker Jul 25, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

Proposed change to see a week of data in the table (need this and the next one):

        let earliestEffectDate = Date(timeInterval: .hours(-7*24), since: now())

The data store contains 7 days of data. Perhaps this should be a variable tied to the days of storage parameter. (not sure where that is - just remember that it exists)

Copy link

Choose a reason for hiding this comment

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

Making this 7 days, makes the app very slowly on iPhone 11. It took a good 30-60sec before the UI got updated (aka the charts got filled with data). Maybe we should find a middle ground?

marionbarker
marionbarker reviewed Jul 25, 2024
View reviewed changes
Loop/View Controllers/CarbAbsorptionViewController.swift
@@ -139,7 +139,7 @@ final class CarbAbsorptionViewController: LoopChartsTableViewController, Identif
charts.updateEndDate(chartStartDate.addingTimeInterval(.hours(totalHours+1))) // When there is no data, this allows presenting current hour + 1

let midnight = Calendar.current.startOfDay(for: Date())
let listStart = min(midnight, chartStartDate, Date(timeIntervalSinceNow: -deviceManager.carbStore.maximumAbsorptionTimeInterval))
let previousMidnight = midnight - .hours(24)
Copy link
Contributor

@marionbarker marionbarker Jul 25, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

Proposed change to see a week of data in the table (need this and the previous one):

        let previousMidnight = midnight - .hours(6*24)

Probably should also change previousMidnight to a new name - just in this one file.
Edited my comment to indicate @oliverstory suggestion of earliestMidnight.

@marionbarker
Copy link
Contributor

I understand this will not be a suitable PR to go with dev when changes start coming out. However, I have applied it to my copy of the released code.
A lot of people are interested in longer ICE table display, so this is suitable for a customization for that purpose.
I am running the full week worth of ICE table display on my personal Loop phone.

@oliverstory
Copy link
Author

oliverstory commented Aug 4, 2024 via email

@bastiaanv
Copy link

It was the initial start and getting the app in foreground

@marionbarker
Copy link
Contributor

I did some tests today in response to @bastiaanv comments.

I added and removed the customization for a week of meals and did some testing.This was done by counting - so not super precise. This is using my real phone (15 pro) and I enter all my carbs.

  • The initial build takes about the same amount of time for the main screen to show up (around 15 sec), but with the customization, there's an additional delay of about 8 sec before the graph populates.
  • I then tested a quit/restart of the app with and without the customization
    • with the default 1 day of ICE, takes about 2 sec to see the plots
    • with the week of ICE takes 15 to 20 sec to see the plots

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@marionbarker marionbarker marionbarker left review comments

@bastiaanv bastiaanv bastiaanv left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Increase Loop Carb History ICE Screen Timeline
4 participants
@oliverstory @marionbarker @ps2 @bastiaanv