nightscout / Trio

MIT License
45 stars 125 forks source link

[Bug?] Bolus reduced without warning #271

Open LiroyvH opened 1 month ago

LiroyvH commented 1 month ago

Somewhat related to #247:

Describe the bug

I noticed that for a while now, despite config changes, I kept getting highs after dinner. As it turns out, Trio seems to cap the maximum bolus to deliver to 25U despite a higher amount being requested. Either that, or the pump is reducing it to 25U itself but that a.) wouldn't make sense to me (couldn't find any such limitation in the design), b.) Trio instantly logs it as 25U, its not retroactively reduced such as is the case when for example you run in to "Motor Error" whilst the pump is giving a bolus. (You'll first see the amount you requested and on the next loop, the actually delivered value is retroactively updated everywhere.)

The maxBolus is set to 40U, yet when a 35U bolus is requested: this is suddenly changed to 25U. There is zero indication within the app that the bolus is not being executed as requested unless you look at the pump or the entry list.

Attach a Log

If logs are needed, a more specific request for what is needed has to be given as the log file is 55MB in size and I'm not about to attach that for a.) pragmatism, b.) privacy.

To Reproduce

Set the max bolus to a value greater than 25U on both the pump as well as within the app. Then attempt to bolus something larger than 25U. This will get reduced to 25U without warning.

Expected behavior

The app should request the actual amount that was asked, not reduce it to 25U.

Setup Information (please complete the following information):

Smartphone:

Pump:

marionbarker commented 1 month ago

Did this work differently with iAPS? And if so, what version were you using?

If so, it may be because Trio is using the LoopKit/MinimedKit submodule which has a maxBolus of 25 U hard coded in it.

When PR #266 is implemented, the maxBolus used by Trio will match the maxBolus used by MinimedKit. You will still probably want a modification to the code (as per Issue #247).

LiroyvH commented 1 month ago

@marionbarker I haven't tested this live with iAPS (for several reasons). I suppose I could test it with iAPS and a spare 754 to see if it behaves differently.

marionbarker commented 1 month ago

No reason to do that. I looked and did not find any obvious code changes in iAPS that would have modified this limit.

The latest version of Trio - dev branch has been fixed so that if you enter a value greater than max Bolus on the Enact Bolus screen, you get a greyed out button (as shown in the graphic below). Perhaps you need to update your version.

With the current code, no MinimedPump will be commanded to more than 25 U (unless you modify the code). Omnipod is limited to 30 U. I'm not sure how they will handle the DanaKit code. Perhaps set the Preferences -> Pump Settings screen up to read what is on the pump for maxBolus and Basal since that is a read-only function for Dana.

bolus-exceeds-max

LiroyvH commented 1 month ago

@marionbarker But I'm not exceeding the max bolus though, that's the whole problem hehe. So why should it have to grey out when its not being exceeded? I'm running Trio with the latest state of the repository.

dnzxy commented 3 weeks ago

You are exceeding the max bolus as you are "falling" into the issue that was reported with #247 and needs fixing. Once that is fixed, you will not be able to set a bolus value greater than the max bolus dictated by the pump; so setting it to that value and then exceeding that value with your input in bolus view will result in a situation like Marion has screenshotted in her last comment.

I'll assign this a wontfix for now; pending a review if it's still occurring or required once #247 is solved.

LiroyvH commented 5 days ago

@dnzxy: I think the changes that would potentially fix this then should be in the latest version, correct? Then I can test. Didn't work with dev 408ce7b, but not 100% sure that incorporated the changes.

dnzxy commented 5 days ago

@LiroyvH yes should be in the app, but no warning or UI enhancement 😊

LiroyvH commented 5 days ago

@dnzxy I'll build latest dev and try again then. :) In the version I'm running now it still reduces 35U (example) to 25U.

dnzxy commented 5 days ago

My understanding was that this should be fixed by #266 (apologies if I am confusing things here PR wise vs. this issue). You were conversing there with Bastiaan, who wrote that fix initially (I only ported it over), looking at this and this comment specifically.

If not I may not 100% be sure what the issue here is or if it's a different issue.

Looking at this comment here also makes me think there are multiple somewhat related issues, but it's not really clear what is needed where and what is fixed or still needs fixing.

LiroyvH commented 4 days ago

@dnzxy Ok, so now running latest dev. To clarify: The problem I have is that my Max Bolus is set to 35U in both Trio as well as on the pump. So far so good. Now when I try to bolus 35U: Trio says OK! and pretends its doing it. But then a bolus of 25U is actually delivered instead.

dnzxy commented 4 days ago

I was under the impression that this ought to be fixed. @marionbarker wasn't this and verified by testing? I'm confused.

marionbarker commented 4 days ago

I confirm maxBolus works with respect to whatever maxBolus is configured under Pump Settings. (It does not check what the max allowed is on a per-pump basis.) The interim fix is to modify that setting on your Trio app.

Note that there is nothing in Pump Settings that requires you to have a Max Bolus that is supported by your pump - or in fact by any pump. I just successfully set Max Bolus to 900 U.

Perhaps this issue should be renamed to reflect these issues:

  1. handle case where bolus entered exceeds what pump can deliver when the Max Bolus value entered on the Pump Settings screen is larger than that limit
  2. Limit the value accepted for max bolus to be max for current pump (if one is selected) or 80 U (if no pump is selected)

max-bolus-message

dnzxy commented 4 days ago

Okay, but according to @LiroyvH 's comments, he tried to set the pump limit to a number that reflects the pump-set max bolus (even though he could've set it to 900 U), but the bolus was silently limited to 25 U despite his pump allowing for 35 U.

So aren't there 2 things at play here?

  1. what you have listed above
  2. remove the "silent" limitation of a bolus to 25 U; always respect the pump max value and/or the app-set max value (since see 1. it would be the pump max value)
marionbarker commented 4 days ago

This has been discussed before. All Medtronic pumps are currently limited to 25 U in the MinimedKit submodule. There is no model-number specific variations in the code. (Allowed basal rates are modified by model number but not max bolus settings.)

If there is a desire to modify this for Medtronic pumps, the issue needs to be very specific that this is what is requested and it is a feature request that may not be handled any time soon.

For now, all Medtronic pumps will only allow a max bolus of 25 U when commanded with Trio (or Loop for that matter).

dnzxy commented 4 days ago

Thank you. In other words: cannot be fixed given the technical limitations at this time. Correct? Should we close or keep this as open with wontfix (due to technical limitations)?

marionbarker commented 4 days ago

I think the ability to put any number on the Pump Settings page is a bug and should be fixed. Are people likely to run into that - not very likely.

BTW - if you attempt to update that setting while on a Medtronic Pump - it will limit you to what the pump manager allows. For Pods and (I think) for Dana, you don't write those values to the pump so you are allowed to put whatever you want. I think the pump manager could be queried and that value limited. I would not put this at the top of anyone's list, but it would be a nice to have.

marionbarker commented 4 days ago

Re reading the original issue statement - this is not a bug as stated. There is indeed code that limits the pump manager to 25 U for Medtronic. It would be cleaner to have a new Issue that the Pump Settings screen needs an overall in the way those numbers are managed. The Save to Pump row - is only valid for Medtronic.

LiroyvH commented 4 days ago

@marionbarker I consider the app not being able to send more than 25U to the pump a bug. The display bug is just a consequence of the underlying problem. Not being able to bolus what is necessary whilst the pump supports it is rather problematic imho. And it's unfortunate to hear that it's considered a non-issue that the app cannot deliver the required bolus amount.

Out of curiosity: this bug could be solved by either altering MinimedKit to support per pump max boluses (most sane and a clean solution, not doing so in the first place and choosing an arbitrary limit of 25U seems very very weird to me.) or otherwise by upping the limit to 75U in MinimedKit and then put a sanity check on the maxBolus we try to put in to Pump Settings when a model is detected that can only go up to 25U?

dnzxy commented 4 days ago

I'm not entirely sure but Marion earlier stated that

There is no model-number specific variations in the code.

which lets me think currently MinimedKit cannot properly convey that information from the pump currently?

I don't think this is a non-issue, but given the small number of MDT users, probably a lower priority issue, especially given the edge case of the bolus size being this large.

If it's a rather easy change to MinimedKit and technically feasible, I think it's more of a non-issue and just needs to be tackled.

Can either of you shed light onto whether this is not doable due to technical limitation, or doable but needs to be done (with the technical limitation here being so we diverge from LoopKit/MinimedKit modules for this when it has been programmed like this since the creation of MinimedKit).

Thank you both @LiroyvH & @marionbarker 🙏

dnzxy commented 4 days ago

It would be cleaner to have a new Issue that the Pump Settings screen needs an overall in the way those numbers are managed. The Save to Pump row - is only valid for Medtronic.

I think there is a word missing after "overall" - maybe "refactoring"?

Why is the save to pump row only valid for Minimed pumps? Aren't these values/settings stored on pods as well?

marionbarker commented 4 days ago

Remember - this is an all volunteer effort.

I agree the way Pump Settings is handled is odd and (older) Medtronic centric. It is not appropriate for Pods or for Dana or possibly for future pumps.

This breaks down to 2 separate issues:

  1. Feature request that Medtronic Pumps have a model-specific max bolus capability added instead of using a single value limited by the pumps with the lowest available value
  2. Feature request that the max basal and max bolus settings be redesigned to work appropriately for all pumps
    • There is no range checking except when saving to Medtronic pumps
    • No other pumps, to date, accept these values as a setting
LiroyvH commented 4 days ago

@marionbarker I must've missed it - what is the workaround?

dnzxy commented 4 days ago

This breaks down to 2 separate issues:

  1. Feature request that Medtronic Pumps have a model-specific max bolus capability added instead of using a single value limited by the pumps with the lowest available value

  2. Feature request that the max basal and max bolus settings be redesigned to work appropriately for all pumps

Thank you for elaborating. I will create these two enhancement tickets tomorrow.

Topic 1 should probably be created under LoopKit/MinimedKit, right?

  • No other pumps, to date, accept these values as a setting

I may be confusing or misremembering things here, but can't a user set a max bolus setting on a PDM? So, setting this for a pod via Trio/iAPS does not work with the current implementations of OnniKit/OmniBLE, but it just uses the hardcoded limits in the submodule code, @marionbarker? 🤔

marionbarker commented 4 days ago

@LiroyvH Sorry - I realized after I hit send that only half your problem is solved.

But it sounds like your real problem is you want 35 U entered up front, and the automatic delivery part of Trio is insufficient to give you an SMB as soon as the 25 U completes delivery, (the SMB would automatically increase the dose up to 35 U - but maybe not fast enough because Trio waits for glucose to start rising - doesn't use carbs like Loop does).

Here's a stupid work-around, but short of having someone with time and skills jump in to fix this, I don't know what else to suggest.

marionbarker commented 4 days ago

Topic 1 should probably be created under LoopKit/MinimedKit, right?

Yes - but no issues are created in MinimedKit only in Loop. So you need to open an Issue in Loop to support max bolus by pump and pump model. The DanaKit PR has not been opened at LoopWorkspace yet (I think) but I'm not sure. I know the LoopKit PR for DanaKit simulator has been opened at LoopKit/LoopKit.

I may be confusing or misremembering things here, but can't a user set a max bolus setting on a PDM? So, setting this for a pod via Trio/iAPS does not work with the current implementations of OnniKit/OmniBLE, but it just uses the hardcoded limits in the submodule code, @marionbarker? 🤔

The PDM does indeed accept a max bolus and uses it internally to decide what commands to send to the pod.