Skip to content
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

Bolus settings #5633

Closed
wants to merge 4 commits into from
Closed

Conversation

jasoncalabrese
Copy link
Member

@jasoncalabrese jasoncalabrese commented Apr 25, 2020

This adds a (fake) bolus plugin so that extended settings are available and splits the bolus rendering settings into BOLUS_RENDER_OVER and BOLUS_RENDER_FORMAT for extra control and cleaner logic.

Also adds a new TREATMENTNOTIFY_INCLUDE_BOLUSES_OVER setting to prevent automated micro boluses from triggering notifications and snoozing alarms

image

@sulkaharo
Copy link
Member

Sorry for the delay, aiming to review tomorrow

@sulkaharo
Copy link
Member

Right, so - @jasoncalabrese this PR changes the functionality and removes the current release behaviour, which is likely to anger users. The current release (which the previous change retained) renders boluses above a certain threshold (1U) using the large font, and boluses that are below threshold are assumed to be microboluses and rendered without the 0 and the U, so the user can still see the amount in the plot, but also the bolus size is less likely to overlap in case of frequent microbolusing from a loop. I'd like to retain that behaviour and I'd prefer to actually make it hard to hide the bolus amount altogether, since many users don't have IOB pill enabled at all and if we make it easy to hide the bolus amount, it's going to lead to users double bolusing. So while the code in this one is cleaner, the change in behaviour is undesirable?

@jasoncalabrese
Copy link
Member Author

jasoncalabrese commented May 5, 2020

since BOLUS_RENDER_OVER defaults to 0 it shouldn't hide any boluses unless the setting is changed and BOLUS_RENDER_FORMAT will be set to 'default' unless it's changed so it should render the full/normal format

maybe I'm missing something

@sulkaharo
Copy link
Member

Ok with screenshots - the behaviour we have now, that I can't replicate with this PR, is having full rendering for the manual boluses (over 1U) and the minimized rendering for the SMBs. What this PR does is enforces the same rendering logic for boluses of all sizes. The two intents I've seen for bolus rendering have so far been 1) clear rendering of data for maximal understandability of the plot and 2) reducing the space needed to render SBMs due to overlapping text in the timeline to make the text readable. At least in my view the "full rendering" is the best in terms of being explicit on the amount boluses, but that doesn't fit the second intent, so the current implementation meets this mid-way by using the minimised rendering for small boluses and full rendering for manual / large boluses.

So the view that's in current implementation but impossible to replicate with this PR is this one, where small boluses are minimized and large are rendered fully:

Screenshot 2020-05-15 at 10 34 45

@jasoncalabrese
Copy link
Member Author

thanks @sulkaharo makes more sense now, I think the names of the settings should probably change to better align with this behaviour, also the copy in the settings panel.

Since SMB is a OpenAPS only term, maybe we could call these micro boluses to be clear. Then we could have settings/defaults like:

  • BOLUS_MICRO_SIZE = 0
  • BOLUS_RENDER_MICRO = 'default'
  • TREATMENTNOTIFY_INCLUDE_MICRO_BOLUSES=true

But then by default micro boluses would snooze treatment notifications, as they did before, maybe that's ok.

@mountrcg
Copy link

Hi, may I ask what the config vars for Heroku are and which values they could have for the 4 "Render Bolus Amount" options?

@sulkaharo sulkaharo mentioned this pull request Feb 2, 2021
@sulkaharo
Copy link
Member

Merged through #6834

@sulkaharo sulkaharo closed this Feb 2, 2021
@mountrcg
Copy link

mountrcg commented Feb 3, 2021

to answer my own question - the 3 variables available for the heroku config vars now are:

  1. BOLUS_RENDER_OVER - defaulted to 0, U value over which the bolus labels use the format defined in BOLUS_RENDER_FORMAT, this value can be an integer or a float, e.g. 0.3, 1.5, 2, etc.
  2. BOLUS_RENDER_FORMAT - Possible values are hidden, default (with leading zero and U), concise (with U, without leading zero), and minimal (without leading zero and U).
  3. BOLUS_RENDER_FORMAT_SMALL - Possible values are hidden, default (with leading zero and U), concise (with U, without leading zero), and minimal (without leading zero and U).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants