Add a feature to limit maximum Insulin On Board #395
Conversation
|
In case it is not obvious, but this is factored out of my master branch at https://github.com/trixing/Loop - that's where the merge conflicts come from. |
ps2
left a comment
ps2
left a comment
There was a problem hiding this comment.
Thanks @trixing. I think there is going to be some work to get ready to merge. I hope the number of requests doesn't put you off; we really appreciate you working on this!
So I've added some inline comments to the code, and have a few general remarks as well.
It's seems like the code is validating current IOB, not the potential IOB from enacting a dose. You could be at max - n but then allow a dose that puts you > max. It seems to me that this feature should be preventing that. I think it's ok to let a user override that, but we should not ever recommend a bolus that would put them over max.
| case GlucoseTargetRangeSchedule = "com.loudnate.Naterade.GlucoseTargetRangeSchedule" | ||
| case MaximumBasalRatePerHour = "com.loudnate.Naterade.MaximumBasalRatePerHour" | ||
| case MaximumBolus = "com.loudnate.Naterade.MaximumBolus" | ||
| case MaximumIOB = "com.loudnate.Naterade.MaximumIOB" |
There was a problem hiding this comment.
New defaults keys should have the com.loopkit prefix.
| ) | ||
|
|
||
| XCTAssertEqual(0.0, dose) | ||
| } |
There was a problem hiding this comment.
There should be tests around the case when maxBolus is nil
There was a problem hiding this comment.
And also for when a low temp is needed but IOB is above max
There was a problem hiding this comment.
It should validate that it still low-temps correctly.
There was a problem hiding this comment.
What should happen for maxIOB = nil? throw an error like all the other missing preferences or not have a limit?
| return vc | ||
| } | ||
|
|
||
| static func maxIOB(_ value: Double?) -> T { |
There was a problem hiding this comment.
The setting VC should have explanatory text to explain how the feature works and that it can be left blank to disable the feature.
|
"It's seems like the code is validating current IOB, not the potential IOB from enacting a dose. You could be at max - n but then allow a dose that puts you > max. It seems to me that this feature should be preventing that. " Are you referring to the temp basal or bolus calculation? For the temp basal calculation: For the bolus calculation - I think that already takes the last basal rate into account. I guess I should take the lastBolus amount into account here as well? Thanks for the feedback. |
|
Please re-open this as a PR against dev. |
|
@ps2 If you're not already aware, Github recently added a feature that lets you (as the maintainer) change the branch a PR targets, via an edit button at the top. That lets you preserve all the comment history while still getting it retargeted as needed. Not sure if that's what you need in this case, but it took me quite awhile to figure out that feature existed, so not sure if you know about it yet. |
|
Cool, no, I didn't know that. Will have to check it out. Thanks!
…Sent from my iPhone
On Mar 18, 2017, at 12:17 PM, Scott Leibrand ***@***.***> wrote:
@ps2 If you're not already aware, Github recently added a feature that lets you (as the maintainer) change the branch a PR targets, via an edit button at the top. That lets you preserve all the comment history while still getting it retargeted as needed. Not sure if that's what you need in this case, but it took me quite awhile to figure out that feature existed, so not sure if you know about it yet.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub, or mute the thread.
|
#395) - https://tidepool.atlassian.net/browse/LOOP-3464 - https://tidepool.atlassian.net/browse/LOOP-3465 - Refresh device data immediately after initializing home screen
3 participants
With this settings the Bolus and Basal recommendation will be limited to a certain amount of Insulin. It does not reject a Bolus above this amount if manually entered in the Bolus view. If an exceeding amount is detected, the Basal rate will be set to the default rate.
[first pull request on github, please be kind]