nightscout / Trio

MIT License
46 stars 130 forks source link

Prevent entries for Carbs, Fat, Protein, and Bolus that exceed Max settings #226

Closed MikePlante1 closed 4 weeks ago

MikePlante1 commented 1 month ago

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.

https://github.com/nightscout/Trio/assets/82073483/649f2677-8146-4328-9dd7-5371aec8e4ec

https://github.com/nightscout/Trio/assets/82073483/bb3aa81f-78f8-453a-a9c6-5b547011af84

bjornoleh commented 1 month ago

This looks good to me! I have not tested yet, but I like what I see from the screenshots.

bjornoleh commented 1 month ago

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 Tap Clear and see the entry being cleared out Tap Doneto escape the keypad (there is no other way, except swiping down) Watch how the previous bolus entry reappears.

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

MikePlante1 commented 1 month ago

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?

bjornoleh commented 1 month ago

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 :-)

JeremyStorring commented 1 month ago

This looks great!

Similarly to what @bjornoleh found, if you follow the same steps when entering carbs, the same thing will happen :)

aug0211 commented 1 month ago

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).

Breakfast: image

Lunch: image

Dinner: image

MikePlante1 commented 1 month ago

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.

LiroyvH commented 1 month ago

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)?

MikePlante1 commented 1 month ago

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.

MikePlante1 commented 1 month ago

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.

LiroyvH commented 1 month ago

@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. :)

aug0211 commented 1 month ago

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.

MikePlante1 commented 1 month ago

I think this needs to be added as a new issue, @bjornoleh:

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 Tap Clear and see the entry being cleared out Tap Doneto escape the keypad (there is no other way, except swiping down) Watch how the previous bolus entry reappears.

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!

MikePlante1 commented 1 month ago

Added Max Fat and Max Protein settings.

I think this one is now ready for review.

bjornoleh commented 4 weeks ago

Merging with two approvals