Insulin concentration v2

[MilosKozak]

Tasks to do:

• Remove ICfg.FAKE
• Make InsulinConfiguration in DB not nullable
• Do proper conversion on ICfg to/from db
• Upgrade database schema
• Fill insulin configuration on existing ProfileSwitches
• Fill insulin configuration on existing EffectiveProfileSwitches
• Fill insulin configuration on existing Boluses
• Review Autotune
• ProfileFragment and ProfilePlugin
• NS synchronization
• PumpSync::expectedPumpState conversion
• Replace caling profileFunction::getProfile by PumpSync::expectedPumpState.profile in Pump drivers
• Do proper migration in NSEffectiveProfileSwitch.toEffectiveProfileSwitch and NSProfileSwitch.toProfileSwitch
• UI for profile activation
• UI for refill
• Remove ActivePlugin.activeInsulin
• Test synchronization to AAPS client
• Test full sync from NS

[MilosKozak]

My proposal is to put layers between
CORE <-> Pump .... PumpWithConcetration interface
CORE <-> ProfileStore .... ProfileStoreWithConcentration interface (maybe it could not be neccessary, see bellow)
and probably
CORE <-> PumpSync too (depend on other decisions) because it's a way how Pump talk back

this way we can change the behavior and only hardly touch the original code
Bolus progress can be adpated after refactoring (in progress)

We must clearly specify what data is stored and used where like:
UI ... U100 or both
DB ... U100
pump DBs ... UXXX?
profileStore ... U100?
TDD ... ????

this must be documented on interfaces

[MilosKozak]

@Philoul read the code if you understand my ideas pls

[Philoul]

Proposed Vocabulary:

• IU for International Units (the real number of Insulin units delivered)
• CU for Concentrated Units (Converted values sent to the pump to deliver the right volume of insulin according to user request)
• concentration: Double value used to manage convertion between IU and CU (i.e. for U200, concentration equals 2.0):
  IU = concentration * CU

Proposal of answer on my point of view:

• User Interface: IU for all information from or to DB (including status light in main screen, even if reservoir level comes from pump), both = IU (CU) for all values managed by the pump or information coming from Pump Driver (i.e. BolusProgress Dialog, Alerts from Pump like Low Reservoir level, Current Status within Pump Driver...). TBC for history values within Pump Driver (could be only CU or both values IU (CU)...
• DB : IU (Real amount should be recorded within database for everything, Loop algo, APS algo, User Interface, Autotune, NS Synchronisation, ...)
• Pump DBs : CU + TBC concentration (CU to store raw value in Pump DB, consistant with Pump history, and concentration value can be added to each absolute record (bolus or rate), to be able to calculate and show both values IU (CU) in pump user interface. % rates are based on Profile so remains identical. Profile managed within Pump Driver and sent to the Pump should be a "Concentrated Profile" (mainly basal rates converted). Pump Current Status should be shown with Both values: IU (CU)
• profileStore : IU
• TDD : IU (calculated from DB), TBC IU or IU (CU), for TDD got from Pump History

Additional opened topic to be decided:

• should we show Pump history with both values: IU (CU)? Better but probably not mandatory (CU remains consistant with Pump history so user can verify consistancy between AAPS (Pump Driver) and Pump (both with CU), but better because User can also check consistancy within AAPS between Pump Driver using IU.
• should we record in AAPS DB the concentration information with timestamp ? (if Yes, no need to record or modify Pump DB, IU can be calculated using AAPS DB concentration and timestamp of each record in CU, if No, then Pump DB should be updated to show both values IU and CU within Pump History, or none of them are recorded, and Pump history is only available with CU

[MilosKozak]

I changed VU to CU ...... hard to think about it like about "virtual" when they are "real"

[MilosKozak]

Thinking about to create something like:

class Insulin(val concentration: Double) {
   var u100Units: Double
   val concentratedUnits: Double get() = u100Units * concentration

   fun setIU(): Insulin
   fun setCU(): Insulin
}

and use it where we have insulin: Double now to be clear what value is inside
But only on places where we do the conversion (not to touch original code, at least for now) and in PumpSync where Pump reports back

ProfileStoreWithConcentration will not be needed then. We can revert the code

[MilosKozak]

I hope you didn't remove previous code. We will re-use most of it :)

[MilosKozak]

regarding pump history I'd keep only CU. Pump driver should not be aware of concentration at all. It will simplify pump drivers development

[MilosKozak]

regarding storing to DB I'd utilize InsulinConfiguration (extended by concentration) for Bolus, ProfileSwitch and EffectiveProfileSwitch
It would mean

• update DB schema
• create new ProfileSwitch if concentration change
• update core code to make using InsulinConfiguration mandatory

Do you agree? Do you see some sideeffects or something I missed?
This preparation should be the first step, I think

[Philoul]

I hope you didn't remove previous code. We will re-use most of it :)

I always keep everything ;-) (at least for a long time)... and I'm currrently use it for my loop so no risk to remove it before final architecture...).
We can reopen the PR if you prefer...

[Philoul]

Thinking about to create something like:

class Insulin(val concentration: Double) {
var u100Units: Double
val concentratedUnits: Double get() = u100Units * concentration

fun setIU(): Insulin
fun setCU(): Insulin
}
and use it where we have insulin: Double now to be clear what value is inside
But only on places where we do the conversion (not to touch original code, at least for now) and in PumpSync where Pump reports back

On my PR, (after code refactoring), I created a ConcentrationHelper Interface to group maximum of calculation within a unique class...
We need both convertion: (fromPump in PumpSync, and toPump)
fun toPump(val u100Units: Double) = u100Units * concentration
fun fromPump(val concentratedUnits: Double) = concentratedUnits / concentration

[Philoul]

regarding storing to DB I'd utilize InsulinConfiguration (extended by concentration) for Bolus, ProfileSwitch and EffectiveProfileSwitch
It would mean
update DB schema
create new ProfileSwitch if concentration change
update core code to make using InsulinConfiguration mandatory
Do you agree? Do you see some sideeffects or something I missed?
This preparation should be the first step, I think

Yes, following this idea, I had in mind to also update ICfg with a new concentration property. (ICfg is already included within EPS class and is miroring InsulinConfiguration).

[Philoul]

create new ProfileSwitch if concentration change

Modifying concentration should be synchronized with a Reservoir Change event.

• Switching from U100 in reservoir to U200 in reservoir should not require a Profile Switch but only a synchronization with Reservoir Change Event...

But, Yes, create new ProfileSwitch if concentration change make sense if we include within ProfileSwitch dialog the selection of Profile AND the selection of Insulin (ICfg with Peak, DIA and concentration...)

• It make the bridge with my second huge PR Insulin Management outside ProfilePlugin #3952 (same story, first idea was quite simple, but at the end this second PR also modify a huge number of different files...)

Do you think we should rework both topics together?

[MilosKozak]

May it would be not necessary to popup ProfileSwitch. We can do it on background.
If there is PS with duration running we'd have to create 2 of them.
But it's necessary because EPS will be holding the information about concentration

[MilosKozak]

Regarding insulin management: yes, we should cover both in this preparation and make code ready for that some clean way

[MilosKozak]

5d97c17
I did a few things, please review or PR @Philoul marked code and confirm the rest

• IOB calculation is not InsulinPlugin based, but it takes ICfg from BS or EffectiveProfile directly ( is it enough?)
• PumpSync for BS (so far) updated
• tests fixed except 2 (sync to NS is still missing)
• storing to DB is not done yet (it will need upgrade DB schema)
• anything forgotten?

[Philoul]

I started to work on update db and ICfg, will update my working branch, compare and provide a first PR.

[Philoul]

anything forgotten?

• a few Pump plugins (patch pumps) directly pull profile using profileFunction.getProfile() on POD change. This should be forbidden to include profile convertion
• currently running EPS should probably be disabled if enable_insulin_concentration file is deleted and concentration<>1.0

[MilosKozak]

OK, added tasks (on top)
Let's PR your conversion class and update PumpSync::expectedPumpState. Spet by step so we can thinks about side effects

[MilosKozak]

Search code for "@Philoul" . I need your assistance there

[Philoul]

Additional tasks to do:

• remove dia from profilePlugin (should be managed within iCfg by InsulinPlugin)
• replace Pump.pumpDescription by PumpWithConcentration.pumpDescription everywhere outside Pump Drivers to get correct converted values (min/Max Values, Steps, level...)
• InsulinPlugin management (include DIA and concentration parameter)

[Philoul]

Additional comment after deeper code analysis, EventOverviewBolusProgress should be updated because BolusProgressData is initialized with insulin, but delivered should be provided with a PumpInsulin value and not a Double if I'm not wrong ?

[Philoul]

There is a regression (at least for Virtual Pump) on this branch, ProfileSwitch doesn't work...
First analysis is within CommandQueueImplepentation.kt

• a log just before this line is seen in logcat

  AndroidAPS/implementation/src/main/kotlin/app/aaps/implementation/queue/CommandQueueImplementation.kt

  Line 127 in 7d18f72

  setProfile(ProfileSealed.PS(it, activePlugin), it.ids.nightscoutId != null, object : Callback() {

• a log just after this line not

  AndroidAPS/implementation/src/main/kotlin/app/aaps/implementation/queue/CommandQueueImplementation.kt

  Line 458 in 7d18f72

  fun setProfile(profile: EffectiveProfile, hasNsId: Boolean, callback: Callback?): Boolean {

So command never arrives to the pump...
Any idea?

Note that Boluses or TBR works in VirtualPump...

[Philoul]

There is a regression (at least for Virtual Pump) on this branch, ProfileSwitch doesn't work...
First analysis is within CommandQueueImplepentation.kt

Fixed in #4286 with c753089

[mountrcg]

I have implemented Insulin concentration in Tai. In general the hard part was not putting in a layer between UI <> pump manager <> pump history, but handling the IU increments available at different concentrations for basal profiles, bolus, calculators etc, and especially what happens to basal profile when you change to a higher concentrated insulin.

Also I made sure that you can only change the insulin concentration when no pump is attached, as with the different pump models it would be hard to make sure to only do that when changing pod or insulin reservoir or whatnot. But changing insulin concentration in app needs to be tied to physical act of changing insulin.

The initial PR was: https://github.com/mountrcg/Trio-dev/pull/2
But I did more safety work later on.

[Philoul]

@mountrcg concerning UI, the plan is currently:

• first it will require a specific action from user to enable this feature (the same way a user enable "engineering mode", he will have to enable insulin concentration)
• of course changing concentration will only be possible during an insulin change event

My proposal for changing concentration will be done in several steps:

• user defines the different insulins he uses (most of them will have only one insulin defined within the "insulin editor", but before a switch he will have to setup at least a new insulin including the new expected concentration (concentration is a property of insulin configuration, like peak or dia, currently selected with pre-defined values, with drop-down menu. default value is U100)
• then he will have to setup a reminder with the new expected concentration on the following "insulin change event" to confirm the usage of this new insulin within pump
• third step. When the user change reservoir/patch, the reminder will show a popup to explicitly confirm the new concentration (this time with a number picker, to avoid "automatic approval")
• once new concentration has been confirmed, then he will have to select the insulin with new approved concentration, and manage a profile switch.
• later, after each new insulin change event, the confirmation popup will be raised to remind/confirm (within number picker) the "non standard concentration", otherwise loop will be disabled.

If the user doesn't confirm the new concentration, or select a wrong value in number picker, then loop will be disabled (until the confirmation of the right concentration).

This is the plan, of course we can adjust everything.
On my personal loop, I'm using a close UI including the pop-up confirmation on each reservoir change, and I think it's important to regularly remind the user that concentrations other than U100 are not "Standard"...

UI proposals will be the purpose for the next 2 or 3 PRs 😉

[Philoul]

Also I made sure that you can only change the insulin concentration when no pump is attached, as with the different pump models it would be hard to make sure to only do that when changing pod or insulin reservoir or whatnot. But changing insulin concentration in app needs to be tied to physical act of changing insulin

Hum, pump disconnection is IMO, on a safety point of view, not the right trigger.
With tube pumps you can disconnect and reconnect the pump several times without changing reservoir.

• so changing concentration without being sure the insulin within the system has been changed will be dangerous I can directly lead to over or under dosing...

I don't know how it's managed within Trio, but within AAPS, we have a standard interface whatever the pump (Tube, Patch, ...) and dedicated events are rised on different actions, especially when "Insulin has been Changed".

• it can be a reservoir change on Tube pumps
• a patch change (or a pod change) on patch pumps

We can trigger this "Insulin Change" event to immediately send a popup window (even above another application if AAPS is in the background during a reservoir change managed in a tube pump)

This guarantee the concentration switch is synchronized and allowed only when really a "new insulin" has been put within the system 🤔

[Philoul]

The initial PR was: https://github.com/mountrcg/Trio-dev/pull/2

It seems I don't have a read access to this repo
However I'm not a Swift developer, so I will probably not be able to understand quickly how you managed the implementation of concentration within Trio 😉

[dnzxy]

The initial PR was: mountrcg/Trio-dev#2

It seems I don't have a read access to this repo However I'm not a Swift developer, so I will probably not be able to understand quickly how you managed the implementation of concentration within Trio 😉

@Philoul It’s not Trio, it’s the autoISF fork of Trio.

Turning to insulin concentration: We actually chatted about this briefly during the virtual hack (if you remember). Our plan overlaps with quite a lot of what you and Milos are doing here: storing insulin concentration, ideally storing insulin model, i.e., type, per dose, keeping the UI strictly in IU, adding a middleware layer between the pump driver, backend, and oref dosing logic, etc.

Happy to dive into details once you’re further along. We're definitely not trying to copy or rip off your work, but I really believe cross-ecosystem collaboration in the community is important so each OS-AID project doesn’t reinvent the wheel, or repeat the same mistakes.

We’re almost done with the oref-swift port. After that, we can hopefully tackle (multi-)insulin-concentration support. Until then, I’ll just keep lurking here, reading along, and admiring what you folks are building 😅

[dnzxy]

Re: your question, we have pump managers per pump driver, they share a lot of the same UI/UX, and we have the same kinds of events that you are mentioning here, i.e., suspend, resume, prime, etc., and we know when a user starts a new patch or pod for example 👍

[sonarqubecloud]

Quality Gate failed

Failed conditions
3 Security Hotspots
4.1% Duplication on New Code (required ≤ 4%)

See analysis details on SonarQube Cloud

[codecov]

Codecov Report

❌ Patch coverage is 32.74251% with 2808 lines in your changes missing coverage. Please review.
✅ Project coverage is 27.61%. Comparing base (4e8306f) to head (5960e06).

Files with missing lines	Patch %	Lines
...pp/src/main/kotlin/app/aaps/ComposeMainActivity.kt	0.00%	829 Missing ⚠️
...kotlin/app/aaps/core/objects/wizard/BolusWizard.kt	1.86%	211 Missing ⚠️
app/src/main/kotlin/app/aaps/MainApp.kt	0.00%	161 Missing ⚠️
...n/kotlin/app/aaps/core/ui/compose/GeneralColors.kt	0.00%	139 Missing ⚠️
...app/aaps/core/graph/TargetBgProfileGraphCompose.kt	0.00%	103 Missing ⚠️
...pp/aaps/core/ui/compose/ImportSummaryComponents.kt	0.00%	97 Missing ⚠️
...tlin/app/aaps/core/graph/IsfProfileGraphCompose.kt	0.00%	93 Missing ⚠️
.../kotlin/app/aaps/core/graph/InsulinGraphCompose.kt	0.00%	91 Missing ⚠️
...otlin/app/aaps/core/graph/IcProfileGraphCompose.kt	0.00%	86 Missing ⚠️
...in/app/aaps/core/graph/BasalProfileGraphCompose.kt	0.00%	85 Missing ⚠️
... and 72 more

Additional details and impacted files

@@             Coverage Diff              @@
##                dev    #4248      +/-   ##
============================================
- Coverage     34.38%   27.61%   -6.77%     
- Complexity     9500    10060     +560     
============================================
  Files          2411     2911     +500     
  Lines        125812   174614   +48802     
  Branches      16319    20394    +4075     
============================================
+ Hits          43260    48226    +4966     
- Misses        79362   123153   +43791     
- Partials       3190     3235      +45     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.