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

No auto cal on Nightscout sync page #3325

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

Navid200
Copy link
Collaborator

@Navid200 Navid200 commented Feb 6, 2024

Please lets remove automatic calibration from the Nightscout page.
A user can enable automatic calibration on the advanced calibration page as well as on the Glucose Meters page.
There is no relationship between automatic calibration and being a Nightsocut uploader.

Before After
Screenshot_20240206-110218 Screenshot_20240206-110340

…4_02_06

# Conflicts:
#	app/src/main/res/xml/pref_data_sync.xml
@Navid200 Navid200 reopened this Apr 19, 2024
@Navid200
Copy link
Collaborator Author

Is there any relationship between Nightscout and auto calibration?
I don't see why this option should be on the Nightscout page?
Would you please either tell me why we need it there or merge this?

@Navid200
Copy link
Collaborator Author

Navid200 commented Jun 3, 2024

This PR does not remove the option to enable automatic calibration.
It only removes the setting from the Nightscout page.

The only reason I would like this removed is to tidy up the settings.
I would like to do that to any menu.
There is so much on the menus and I would like to clean them up.

The more items we have on any menu, the more anxiety it can create for a new, or even experienced, user.

@jamorham
Copy link
Collaborator

I believe that blood test entries entered on Nightscout can become automatic calibrations sent to sensor

@Navid200
Copy link
Collaborator Author

Navid200 commented Jun 11, 2024

@jamorham I don't disagree with that statement.
Any blood test entries coming from any source (including Nightscout) to xDrip can be used for auto calibration, no?

If I enable automatic calibration, all blood test entries will be used. I don't believe there is any conditional to only use some of them.

I believe this (redundant) menu item can be removed to clean up the menu without confusing users.
If a user wants to use automatic calibration, they will have to look under the calibration menu and they will find it under advanced calibrations.
If I want to enable automatic calibration, I don't think I will go to the Nightscout upload menu looking for it.

I don't believe removing this item is going to cause any confusion.
But, it will clean up the menu.

@tolot27
Copy link
Collaborator

tolot27 commented Jun 13, 2024

Every time a BloodTest is created, it can be used for calibration if it is not older than 5 minutes (see

if (Pref.getBooleanDefaultFalse("bluetooth_meter_for_calibrations_auto")) {
if ((JoH.msSince(bt.timestamp) < Constants.MINUTE_IN_MS * 5) && (JoH.msSince(bt.timestamp) > 0)) {
UserError.Log.d(TAG, "Blood test value recent enough to send to G5");
//Ob1G5StateMachine.addCalibration((int) bt.mgdl, timestamp_ms);
NativeCalibrationPipe.addCalibration((int) bt.mgdl, timestamp_ms);
}
}
).

Downloaded treatments will also create a BloodTest object and will therefore trigger the calibration check:

final BloodTest bt = BloodTest.create(timestamp, mgdl, tr.getString("enteredBy") + " " + NightscoutUploader.VIA_NIGHTSCOUT_TAG);

Indeed, it is not necessary to replicate the setting. But there should be at least a warning on the Nightscout settings page, that recently downloaded blood test values could trigger a calibration if the bluetooth_meter_for_calibrations_auto is enabled.

Or the property should be changed that it reflects downloaded blood values.

@Navid200
Copy link
Collaborator Author

Navid200 commented Jun 13, 2024

@tolot27 Automatic calibration is not something I am happy about having in xDrip.
It was an extremely clever thing when G5 was the latest and the greatest.
Today, it is a mistake. But, I often find individuals who have a strong tie with the idea that blood glucose meter is the king!

Anyway, I cannot remove automatic calibration from xDrip. I would if I was allowed. But, that's a different story.
I am hoping to clean up the menus.
I don't understand your point.

Any blood test entered into xDrip will be used for automatic calibration if the setting is enabled.
If someone enables automatic calibration and they don't understand how horrible it is, putting a warning on the Nightscout page is not going to change anything.
If one day we allow a parent to be able to enter blood glucose meter measurements into an xDrip sync follower to show up on the master, they will also be used for calibration if automatic calibration has been enabled.
But, why would we have to show a warning on the xDrip sync page then?

Edit: Please see a correction added in the next post.

@Navid200
Copy link
Collaborator Author

I made an incorrect statement.
Automatic calibration can still be very useful for any scenario where the calibration formula is maintained by xDrip.
I believe we have a Libre scenario where that is the case.

So, I would remove automatic calibration for G6 and G7 where applying a calibration in the past is not possible.

@Navid200
Copy link
Collaborator Author

If you look at the portion of the code you have quoted and look at line 172, you can see that the submitted calibration has a time parameter.
We are sending a calibration to G6 or G7 with a time parameter.
But, G6 or G7 is not going to receive it now. No, it has to first go through the transmitter queue. If the last read cycle completed 30 seconds ago, the calibration will be sent to G6 or G7 in 4.5 minutes from now.

What will G6 or G7 do to a calibration with a time parameter?
Has anyone performed a test to confirm that they will respect the time value?

We are not supposed to over calibrate G6 or G7 anyway.

@Navid200
Copy link
Collaborator Author

Reading my own posts, I see a lot of ranting! I am sorry about that.

People tell me that I should not have removed the ability to reset calibrations for G6.
The fact is that no one could reset G6 calibrations because the calibrations are done by the transmitter. We just had never cleaned up some of the remaining things from the G5 time to show up for G6. But, I have a hard time convincing them that it really did nothing when they reset their calibrations.
So, making users happy is not easy.

If we remove the ability to run auto calibration with G6, I expect some people will be extremely unhappy with me. Making xDrip do the right thing and keeping the users happy sometimes are not on the same path.

I think JamOrHam has a done a great job making decisions, and keeping me on the right track.

@tolot27
Copy link
Collaborator

tolot27 commented Jun 15, 2024

I don't understand your point.
Any blood test entered into xDrip will be used for automatic calibration if the setting is enabled. If someone enables automatic calibration and they don't understand how horrible it is, putting a warning on the Nightscout page is not going to change anything. If one day we allow a parent to be able to enter blood glucose meter measurements into an xDrip sync follower to show up on the master, they will also be used for calibration if automatic calibration has been enabled. But, why would we have to show a warning on the xDrip sync page then?

I assume that the main intention of repeating the automatic calibration at the Nightscout Sync settings page is to give the user an implicit reminder that this option is potentially turned on. Since it is not helpful to replicate settings, I suggest to replace it by a warning only if automatic calibration is turned on. This will fulfill the original intention.
Nevertheless, I suggest to remove the code to use remotely entered BG values for automatic calibration. It is simply too unsafe.

BTW: Every BG value one is entering in a t:slim X2 pump is used for calibration (as long as the current BG value is stable). Hence, calibration is still active for both G6 and G7 and IMHO is still a useful feature.

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