Loop icon indicating copy to clipboard operation
Loop copied to clipboard

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

Open oliverstory opened this issue 1 year ago • 10 comments

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 avatar Jul 08 '24 11:07 oliverstory

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 #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

  • Patches for both PR #2163 and this PR (2182) were applied

    • The code change in 2163 is a different "fix" and does not conflict with 2182
  • Ignore the dynamic island display - that is from a different app

    • I have it turned on to enable a lock-screen graph

marionbarker avatar Jul 10 '24 13:07 marionbarker

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 https://github.com/LoopKit/Loop/pull/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.

oliverstory avatar Jul 10 '24 23:07 oliverstory

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.

ps2 avatar Jul 11 '24 21:07 ps2

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.

marionbarker avatar Jul 25 '24 14:07 marionbarker

Do you think the slow down affects operation, or just initial start up?

On Sun, 4 Aug 2024 at 7:29 PM, Bastiaan Verhaar @.***> wrote:

@.**** commented on this pull request.

In Loop/Managers/LoopDataManager.swift https://github.com/LoopKit/Loop/pull/2182#discussion_r1703132265:

@@ -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())
    

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?

— Reply to this email directly, view it on GitHub https://github.com/LoopKit/Loop/pull/2182#discussion_r1703132265, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABFRGMMO2DUAFWPH6W54O5DZPXX7RAVCNFSM6AAAAABKQVCRR6VHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHMZDEMJXGUYTIOJVGM . You are receiving this because you were mentioned.Message ID: @.***>

oliverstory avatar Aug 04 '24 21:08 oliverstory

It was the initial start and getting the app in foreground

bastiaanv avatar Aug 05 '24 04:08 bastiaanv

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

marionbarker avatar Aug 09 '24 14:08 marionbarker

Test

It's been a long time since I tested this in this exact format

ICE screen with dev

  • Start with current LoopWorkspace dev, commit 3d48a5c
  • Build onto phone (no customization)
  • Capture screenshot of ICE screen (left side of graphic below)

ICE screen with this PR

  • Apply this PR to Loop submodule and rebuild
  • Capture screenshot of ICE screen ( right side of graphic below)

pr_2182

marionbarker avatar Mar 27 '25 00:03 marionbarker

This is a new feature, not a bug fix; would prefer to land this after tidepool-merge.

ps2 avatar Apr 17 '25 18:04 ps2

This "feature" has been available as a customization, which many people have applied to their 3.4.x Loop, since August 2024. It has been thoroughly tested by all the people who use this customization.

Let's not make people wait longer for a feature that is quite useful and often requested.

I have tested adding this feature to tidepool-merge. It is an easy change but that test did reveal a bug in tidepool-merge. This was reported in this comment:

  • https://github.com/LoopKit/LoopWorkspace/pull/213#issuecomment-2762409101

marionbarker avatar Apr 17 '25 18:04 marionbarker