-
Notifications
You must be signed in to change notification settings - Fork 509
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
base: dev
Are you sure you want to change the base?
Conversation
Test cases for review include
|
Just a side note: it looks like you had checked out an older version of the |
64aae07
to
c7017cb
Compare
rebased on current dev and fixed submodule issue. |
@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. |
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 |
ok, so will rebase once more when #236 is merged, and then try to fix editing of adv.TT. |
It is merged :) |
c7017cb
to
cf268c2
Compare
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.
|
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? |
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. |
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? |
Was away for while on holiday, so it will take a while for me to catch up on this. |
We could just do "Advanced" as we are in the Temp Target Screen anyway. |
It is stored in coreDate, @dnzxy gave a hint to fetch the coreData object and edit it. Will give it a go with GPT. |
The current Experimental TT slider has disadvantages.
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.