-
Notifications
You must be signed in to change notification settings - Fork 477
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
Prevent entries for Carbs, Fat, Protein, and Bolus that exceed Max settings #226
Conversation
Also update color and language of Max Bolus exceeded
This looks good to me! I have not tested yet, but I like what I see from the screenshots. |
I looked quickly after building, and this works as it should, and I could approve this as is. But, I also noticed a peculiarity that was not introduced here. If this is something that could be fixed here, please do, otherwise we can enter a separate issue: Observation: Enter a value in the Enact Bolus: Bolus - Amount field Some recent PR introduced a button to close the keypat, not sure if any of that could be used here. If this is outside of the scope of this PR (it is!), we'll copy this to a separate issue! Edit: a new issue report has been submitted for this: #273 |
Good catch, @bjornoleh. I’ll try to figure that out and add it to this PR as well. Any thoughts on also adding a warning here if MaxIOB or MaxCOB is exceeded? Maybe just keep the text the same but make it orange and add a warning/confirm popup/alert? |
Max IOB and Max COB are originally intended to harness automated dosing, so I am unsure if it's worth adding notifications during manual entries? Just a thought :-) |
This looks great! Similarly to what @bjornoleh found, if you follow the same steps when entering carbs, the same thing will happen :) |
It was recommended in Discord that we use this issue for discussion on max FPU thresholds. In case we do want to bundle that discussion here (or maybe eventually breakout to a different issue?): The idea of reusing the max carb threshold for max fat and max protein entries was floated. A few people shared concerns with this (myself included) due to people with different diets that may lean toward higher fat and protein meals and lower carb meals. I thought I'd upload a sampling of 3 meals someone may eat who targets ~20g carbs in a meal, with significantly higher fat and protein. In these examples (breakfast, lunch, dinner), someone may want a max carb threshold of something like 50g (roughly double the highest meal in these examples), whereas a max fat and protein might need to be more like 150 or 200 (roughly 150% of the highest entry shown). |
For Max FPU, I was thinking of just using Max Carbs as the limit for carb equivalents, not for grams of fat/protein. So if have an “override with a factor of” is set to 1, a 100g max carb limit would be either 111g of fat or 250g of protein or 10.9F0.4P=100 if a combo of fat and protein is entered. If set to 0.5, a 50g Max Carb limit would be 111g fat or 250g protein, or 0.50.9F0.4P=50 My reasoning on this is that when you enter “fat” and “protein”, you’re really just entering carbs since that’s all oref understands anyway. So that option is more algorithm based as you’re limiting the inputs of carbs to oref, whether those carbs are entered as carbs or just converted to carbs from fat and protein. Let’s call the above Option A. Option B could be having three settings: “Max Carbs”, “Max Fat”, “Max Protein”. I think it’d be weird to have protein and fat share a “Max Fat/Protein” setting since Fat is worth twice as many carbs as protein is. Option C could be a single “Max Carbs from Fat and Protein” setting and have work basically the same as Option A except checked against a nee setting instead of just “Max Carbs” Feel free to suggest an option D, E, etc. |
Just to be sure I understand this, this only affects the bolus calculator based on the Max Carbs limit (max you can add in one sitting); but will not have any effect on the MaxCOB safety limiter (nor are any limits in the bolus calculator affected by MaxCOB)? |
Correct. |
Or, well, I suppose not necessarily the bolus calculator, just the “add meal” screen, but that screen will take you to the bolus calculator screen. |
@MikePlante1 No it won't 🥳 Disabled that annoying feature a long time ago haha. Anyway, ok so it only affects the Add Meal-thingy. Splendid. :) |
I’d go with either a single guardrail for “max fat/protein” or two, one for “max fat” and one for “max protein”. I think 1 is probably fine, since the biggest thing this guards against is order of magnitude jumps where fat fingering turns 60 into 600 for instance. More details shared in Discord discussion for reasoning. |
- Adds two new settings: `maxFat` & `maxProtein` - default to 250g, same as `maxCarbs` - Change label of `Carbohydrate limit` to `Limit Per Entry` for Meal Settings - For meal entry module, shorten `grams` to `g` - If bolus, carbs, fat, or protein exceed their limit: - Change `U` or `g` to `⚠️ ` - Disable submit button, change text to red, and replace with warning
Removed: - `Max Bolus exceeded!` Added: - `exceeded` - `Max Bolus of` - `Max Carbs of` - `Max Fat of` - `Max Protein of` - `Max Carbs` - `Max Fat` - `Max Protein`
I think this needs to be added as a new issue, @bjornoleh:
|
Added I think this one is now ready for review. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good and works as expected in a quick test after building to a phone.
Thanks, well done!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Tested on iPhone 11 @ iOS 16.7
- Pulled in feature branch
MikePlant1:max_carbs
to latestdev
- Change all settings
Max Carbs
,Max Fat
andMax Protein
- Opened meal view
- Entered "allowed" values:
- all good
- can proceed and save
- depending on setting (skip bolus yes/no), UI opens bolus view
- Entered values outside of max settings:
- Save button disabled
- Showing respective error message
- Text is red
- Input value for macro shows
⚠️ next to value
This works as designed and expected.
Very small comments regarding usage of hardcoded units when there are translations for it. I just remembered that unless you interpolate a string or concatenate it, localization should actually work already and there are localized entries for both "g"
and "U"
. Changed to approval.
LGTM, great job, Mike!
Merging with two approvals |
maxFat
&maxProtein
maxCarbs
Carbohydrate limit
toLimit Per Entry
for Meal Settingsgrams
tog
U
org
to⚠️
I also thought it might be nice to make the "enact bolus" and "save and continue" text orange if IOB+bolus>MaxIOB or if COB+carbs>MaxCOB, but have not added that yet.PR.226.mov
Before.PR.226.mov