-
Notifications
You must be signed in to change notification settings - Fork 477
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
Set median of lower and upper correction range bound when importing L… #253
Set median of lower and upper correction range bound when importing L… #253
Conversation
FreeAPS/Sources/Modules/NightscoutConfig/NightscoutConfigStateModel.swift
Outdated
Show resolved
Hide resolved
I think perhaps AAPS targets should be handled the same way as Trio targets, using only / mainly the lower target, instead of the median of lower/upper target range, as seems to be the case in this PR currently. We use the same algorithm, and I would expect the same behaviour regarding targets. From https://androidaps.readthedocs.io/en/latest/Configuration/Config-Builder.html#closed-loop: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM (based on code review only, as I do not use Loop and cannot test Loop-imported settings).
@marionbarker or @bjorkert – could you please test this, and if good, give your approval please? |
SummarySuccess: This PR worked as expected. Testing DetailsI applied this patch to dev (commit 4d6e102).
After applying the patch and building again, I imported my settings again.
|
Merging with two approvals |
…ttingsLoopMedianTarget Set median of lower and upper correction range bound when importing L…
Resolves NS Settings Import: Set median of lower and upper correction range bound when importing Loop settings by:
Sets Trio's target correction low and high to median of lower and upper target range when NS profile's enteredBy is Loop. Rounds median value to 0 or 1 significant digits depending on Glucose unit (0 for mg/dL, 1 for mmol/L).
EDIT:
This issue probably applies to AAPS, too, since it also uses a low and high target. examples.json shows its
profileJson
withtarget_low
andtarget_high
. I cannot test it w/out having an actual "openaps://AndroidAPS" profile though.I think merged PR, Remove unused Import Settings in JSON that cause errors decoding JSON #249, also applies to AAPS.