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

conversion of rT.variables in mg/dL #327

Open
wants to merge 5 commits into
base: dev
Choose a base branch
from

Conversation

mountrcg
Copy link
Contributor

Needs to be done in parallel with nightscout/trio-oref#30

This concerns mainly ISF as transmitted from oref to suggestion / enacted. So far I only found one instance where ISF is used.

So far ISF is always transfered from oref in the current user glucose unit, so if mmol/L is used it comes in mmol/L. Therefore currently no conversion is applied in Trio to ISF originating from oref. Drawback is also that a change of user glucose unit will not be reflected in ISF unless a new loop is calculated. Refer also to nightscout/trio-oref#29

Also the watch app needs to adjusted as ISF may also be shown on watch.

@mountrcg
Copy link
Contributor Author

I need a little help on the watch part, still throws an error in the conversion function

from: (units == .mmolL ? isfValue.asMmolL : Decimal(isfValue)) as NSNumber

@mountrcg
Copy link
Contributor Author

mountrcg commented Jun 24, 2024

successfully tested!
As Trio does not show ISF anywhere on UI, only watch can be used to review the changes.

Old sitiuation - ISF is only shown converted after a new loop

original state ISF after unit change, no new loop after new loop
Simulator Screenshot - Dev 13 mini - 2024-06-24 at 09 27 37 Simulator Screenshot - Dev 13 mini - 2024-06-24 at 09 27 59 Simulator Screenshot - Dev 13 mini - 2024-06-24 at 09 32 58
Simulator Screenshot - Apple Watch Series 7 (45mm) - 2024-06-24 at 09 27 39 Simulator Screenshot - Apple Watch Series 7 (45mm) - 2024-06-24 at 09 28 01 Simulator Screenshot - Apple Watch Series 7 (45mm) - 2024-06-24 at 09 32 52

After change

ISF is immediatly converted after changing units on phone. As it can be seen the popup still shows units from last loop, watch already has ISF converted.

original state ISF after unit change, no new loop
Simulator Screenshot - Dev 13 mini - 2024-06-24 at 08 57 34 Simulator Screenshot - Dev 13 mini - 2024-06-24 at 08 57 47
Simulator Screenshot - Apple Watch Series 7 (45mm) - 2024-06-24 at 08 57 35 Simulator Screenshot - Apple Watch Series 7 (45mm) - 2024-06-24 at 08 57 49

@mountrcg mountrcg changed the title Draft: Trio conversion of rT.variables in mg/dL rio conversion of rT.variables in mg/dL Jun 24, 2024
@mountrcg mountrcg changed the title rio conversion of rT.variables in mg/dL conversion of rT.variables in mg/dL Jun 24, 2024
mountrcg pushed a commit to mountrcg/Trio that referenced this pull request Aug 1, 2024
@mountrcg
Copy link
Contributor Author

it can also be reviewed with enacted.js in Developer option.

Copy link
Contributor

@dnzxy dnzxy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Interim fix. Will need changing with Trio 1.0.0, but for 0.2.0 okay.

@bjornoleh
Copy link
Contributor

I tested this now in Xcode / iPhone simulator, and it works great.

  • ISF instantly changes units on the watch when changing the units setting
  • At loops, the ISF value in suggested and enacted are updated, and displayed with the chosen units both in the enacted box in Trio, and in the Nightscout OpenAPS pill.

We will be diverging a little from openaps/oref0 by doing this, which is probably fine.

@mountrcg, did you consider PRing the oref changes to upstream oref0?

@dnzxy
Copy link
Contributor

dnzxy commented Aug 16, 2024

From my conversation with Robert, this actually is not something that diverges from oref0, but is a change that he introduced himself about 2 years ago, and we are making it so that ISF is treated like all other settings.

@bjornoleh
Copy link
Contributor

From my conversation with Robert, this actually is not something that diverges from oref0, but is a change that he introduced himself about 2 years ago, and we are making it so that ISF is treated like all other settings.

Ok, thanks for the info, yes this rings a bell. But when checking relevant lines in determine-basal, it seems his changes then were not 1:1 with the current PR:

image

The PR from 2021 is here:
openaps/oref0@da70837

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

Successfully merging this pull request may close these issues.

3 participants