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

Limit high blood glucose values shown on the graph to 400 mg/dL #2652

Merged
merged 4 commits into from
Feb 11, 2023

Conversation

Navid200
Copy link
Collaborator

@Navid200 Navid200 commented Jan 29, 2023

This is what a user has got with show Libre trend enabled.
326923318_582882383746798_5979460552736510366_n

This makes the variations on the curve look nonexistent obviously due to the significant dynamic range.

We already limit readings to 400 here:

} else if (bgReading.calculated_value >= 400) {
highValues.add(new HPointValue((double) (bgReading.timestamp / FUZZER), (float) unitized(400)));

But, not everywhere.
This PR does the same to other sources.
It would make xDrip's behavior consistent.

I am currently running this.

@Navid200 Navid200 changed the title Limit shown blood glucose values to 400 mg/dL Limit high blood glucose values to 400 mg/dL Jan 29, 2023
@Navid200 Navid200 changed the title Limit high blood glucose values to 400 mg/dL Limit high blood glucose values shown on the graph to 400 mg/dL Jan 29, 2023
@jamorham
Copy link
Collaborator

Instead of doing it conditionally like this use Math.min(x,y) to cap to a high value. Needs review from @tzachi-dar when ready as well.

@Navid200
Copy link
Collaborator Author

@tzachi-dar I have made the change that jamorham suggested.
Please review when you get a chance.
I am currently using this as my main xDrip.

@jamorham
Copy link
Collaborator

You can do the math.min before the unitized I think to avoid doing additional conversions where one will be discarded.

@tzachi-dar
Copy link
Contributor

Generally looks good.
You can do here two more improvements:

  1. Make a function that does Math.min((float) unitized(xxx), (float) unitized(400) and use it in all places.
  2. Replace the 400 with a constant (for example MAX_SHOWN_BG). it is already 4 times in the code.

@Navid200
Copy link
Collaborator Author

@tzachi-dar @jamorham I now do Math.min first to pick one of the two candidates. Then, I run unitized only once on the result. This was caught by jamorham and thanks for catching it.

With respect to your suggestion, I can add a constant if necessary. Is it?

But, are you OK with the overall concept? This affects Libre raw. Are you OK with us limiting what is shown on screen to 400 in that case?

@tzachi-dar
Copy link
Contributor

yes, we should not have values on the screen higher than 400. In most times this is just noise, and the 'damage' remains for 24 hours.

@jamorham
Copy link
Collaborator

jamorham commented Feb 1, 2023

You can use BgReading.BG_READING_MAXIMUM_VALUE instead of 400 directly. That way if it changes no code changes will be needed.

@Navid200
Copy link
Collaborator Author

Navid200 commented Feb 1, 2023

Replaced 400 with the existing max value constant. Compiled and is running on my phone.

Copy link
Collaborator

@jamorham jamorham left a comment

Choose a reason for hiding this comment

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

Looks like it should work

@jamorham jamorham merged commit 54ba5b0 into NightscoutFoundation:master Feb 11, 2023
@Navid200 Navid200 deleted the Navid_2023_01_28 branch February 12, 2023 03:42
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.

None yet

3 participants