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.

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

Advanced TempTarget slider #241

Draft
wants to merge 2 commits into
base: dev
Choose a base branch
from

Conversation

mountrcg
Copy link
Contributor

The current Experimental TT slider has disadvantages.

old Slider arguments
Simulator Screenshot - Dev 12 mini - 2024-05-26 at 22 19 27 The slider for the Target Glucose actualy slides HalfBasalExercixeTarget (HBT) and shows the calculated TempTarget with that HBT and the the chosen Insulin fraction above. This will result in above shown example that with a high sensitivity of 75% Insulin one could set a High TT of max 165mg/dL. This bears no justification. Also from my sports activities, which is the major application, I first choose at which target glucose I want to exercise and when at which sensitivity. The refactored slider adjusts the desired Insulin % (sensitivity ratio) and calculates HBT. So here the boundaries are not applied like in the old solution to HBT but to the sensitivity ratio. For highTT the min sensitivity ratio is 15%, for lowTT the autosensMax defines the border of low sensitivity applied by the TT.

New Advanced TT slider:

Simulator.Screen.Recording.-.Dev.12.mini.-.2024-05-26.at.22.11.48.mp4

The feature itself is standard oref, but it requires setting an HBT to arrive at desired sensitivity. This is a comfortable solution to use that feature for exercise TT's. The new slider addresses the issues of boundaries and also increments the single slider by 5% points. This solution still uses Jon's oref2 variable injection of HBT into oref as a variable that overrides the HBT as defined in user preferences. This PR could be a good playing ground to change that and make the HBT calculation result the new user preference for the time the TT is active. Once the TT is not active anymore the user preference should revert to the state before TT. Unfortunately I would need assistance with that.

Once this is discussed and implemented it could be also conceptually used for Overrides and how the temporary replace user preferences. This way no additional injections into oref are needed for TT's and Profile Overrides.

@mountrcg
Copy link
Contributor Author

mountrcg commented May 27, 2024

Test cases for review include

  1. preference lowTTlowersSens is disabled/enabled
  2. preference highTTraisesSens is disabled/enabled
  3. autosensMax is =1 / >1
  4. autsensMin is =1 / <1
  5. change level of HBT in prefs

@mountrcg mountrcg marked this pull request as draft May 27, 2024 02:36
@dnzxy
Copy link
Contributor

dnzxy commented May 27, 2024

Just a side note: it looks like you had checked out an older version of the MinimedKit submodule in your local branch. Please update (by pulling in latest module SHA) or else this PR would regress the submodule in comparison to dev.

@mountrcg
Copy link
Contributor Author

rebased on current dev and fixed submodule issue.

@mountrcg mountrcg changed the title Draft: Advanced TempTarget slider Advanced TempTarget slider May 30, 2024
@mountrcg mountrcg marked this pull request as ready for review May 30, 2024 06:18
@mountrcg
Copy link
Contributor Author

@dnzxy could you request the reviews, I do not have permission to do this. I do not think the work around the oref2-Variable thing will be done, as I do not have the expertise. But it could go like this, same structure as the Experimental Slider, just without the disadvantages and with a little more information.
@dsnallfot it does however need a rework of the edit part you just did for experimental, I tried but could not get it to re-calculate the percentage on edit, nor the save of the new HBT if edited.

@dnzxy
Copy link
Contributor

dnzxy commented May 30, 2024

Reviews requested, thanks for the ping 😊

Regarding your editing issue: Not sure, but I think #236 can be considered a "given", so while I generally do not advise to base features on other features while they are being worked on, I think this PR should be based on 236, as 236 will very likely be merged the next 24-72 hrs and then this PR would regress the functionality that got introduced with 236. @dsnallfot can you help Robert with his feature here?

@dsnallfot
Copy link
Contributor

Reviews requested, thanks for the ping 😊

Regarding your editing issue: Not sure, but I think #236 can be considered a "given", so while I generally do not advise to base features on other features while they are being worked on, I think this PR should be based on 236, as 236 will very likely be merged the next 24-72 hrs and then this PR would regress the functionality that got introduced with 236. @dsnallfot can you help Robert with his feature here?

Of course. I can try to help. But not before the weekend since I'm totally busy at work right now

@mountrcg
Copy link
Contributor Author

... I think this PR should be based on 236, as 236 will very likely be merged the next 24-72 hrs ...

ok, so will rebase once more when #236 is merged, and then try to fix editing of adv.TT.

@Sjoerd-Bo3 Sjoerd-Bo3 requested a review from dsnallfot May 30, 2024 12:28
@Sjoerd-Bo3
Copy link
Contributor

... I think this PR should be based on 236, as 236 will very likely be merged the next 24-72 hrs ...

ok, so will rebase once more when #236 is merged, and then try to fix editing of adv.TT.

It is merged :)

@mountrcg
Copy link
Contributor Author

mountrcg commented May 30, 2024

So I am not lucky to change a Preset with a different HBT, also known as Experimental/Advanced TT. When editing I do not know how to acquire the saved HBT that belongs to that preset. Therefore it uses standard 160mg/dL - see screenshot 1. Also the editing does not save the new entered percentage. Due to this changed slider the percentage does not need to be computetd, it's a direct slider input, but HBT gets computed! See screenshot 2 of edited percentage, however the old 200% will be enacted! I put some comments into the code.
@dsnallfot I wait for your return patiently to help - no big deal. Never mind the TempTarget Name - it's actualy a lowTTlowersSens

Screenshot 1 Screenshot 2
Error-EditPreset with changed HBT Simulator Screenshot - Dev mini - 2024-05-30 at 17 18 12

@mountrcg
Copy link
Contributor Author

mountrcg commented May 30, 2024

I left the computePercentage function in there as it would be needed to set the Slider at the appropriate position when editing the Advanced TT, if one could retrieve the proper HBT for this preset. Most probably that function should be moved from FreeAPS/Sources/Modules/AddTempTarget/AddTempTargetStateModel.swift to FreeAPS/Sources/Modules/AddTempTarget/View/AddTempTargetRootView.swift, right?

@dsnallfot
Copy link
Contributor

Sorry for the late reply. Will now test build and tinker with this a bit before the kids wake up in a couple of hours. Get back to you as soon as I have any input.

@dsnallfot
Copy link
Contributor

dsnallfot commented Jun 1, 2024

ok, have not solved anything yet, but can provide a small clue on how you could do to save the preset hbt part at least - I think this would work and not cause issue with NS uploads etc:
Skärmavbild 2024-06-01 kl  11 41 49

Temp targets currently don't use "notes" as an attribute (which other treatments do), but I guess we could add that attribute and save/write/read the hbt in all newly created presets there.
Skärmavbild 2024-06-01 kl  11 52 47

And this means that the "notes" needs to be added in the Temp Target model (and in the temp target entry and updatedPreset and watchManager tempTargetID also).
Skärmavbild 2024-06-01 kl  11 40 34

To be continued later...

@dsnallfot
Copy link
Contributor

Or even better maybe, to introduce a new attribute "hbt" to all temp target presets. Instead of "notes". (I initially thought about notes since that's part of the standard nightscout attributes)

Skärmavbild 2024-06-01 kl  11 57 04

@JeremyStorring
Copy link
Contributor

Are we in love with "Advanced TT"? It's not immediately clear what it is. I think it would be better to try to be clear and concise, can we fit "Advanced Temp Target" or something similar?

@Sjoerd-Bo3 Sjoerd-Bo3 marked this pull request as draft June 8, 2024 23:06
@mountrcg
Copy link
Contributor Author

Was away for while on holiday, so it will take a while for me to catch up on this.

@mountrcg
Copy link
Contributor Author

Are we in love with "Advanced TT"? It's not immediately clear what it is. I think it would be better to try to be clear and concise, can we fit "Advanced Temp Target" or something similar?

We could just do "Advanced" as we are in the Temp Target Screen anyway.

@mountrcg
Copy link
Contributor Author

Or even better maybe, to introduce a new attribute "hbt" to all temp target presets. Instead of "notes". (I initially thought about notes since that's part of the standard nightscout attributes)

It is stored in coreDate, @dnzxy gave a hint to fetch the coreData object and edit it. Will give it a go with GPT.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
Status: No status
Development

Successfully merging this pull request may close these issues.

5 participants