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

Erroneous +0 BG deltas reported #544

Open
aug0211 opened this issue Jun 19, 2024 · 3 comments · Fixed by #575
Open

Erroneous +0 BG deltas reported #544

aug0211 opened this issue Jun 19, 2024 · 3 comments · Fixed by #575

Comments

@aug0211
Copy link
Contributor

aug0211 commented Jun 19, 2024

See PR #545

xDrip occasionally reports a +0 BG delta when in fact BG delta is not +0. This most often occurs with +1 or -1 deltas (mgdl). Have discussed in the past but could not find an issue for it. I've implemented a very minor code change that resolves this issue for me. I'm opening the issue to accompany the PR.

I've reproduced this issue across 4 iOS devices and 2 WatchOS devices and both master and dev builds over the past ~6 weeks.

We first suspected multiple sources uploading to Nightscout as the cause. This was addressed and the issue did not resolve. This was not the cause.

Here are recent screen shots showing xDrip reporting a delta of +0, when in fact Nightscout reports a -1 (which is accurate because BG went from 87 to 86).

imageimageimage

I have implemented a trivial fix for this which I've tested and validated as working as desired across watch complication, watch app, phone widget, phone live activities, phone app. See PR #545.

paulplant added a commit to paulplant/xdripswift that referenced this issue Oct 6, 2024
Related to #JohanDegraeve#544

although the previous commits fixed the issue of erroneous (0 or close to 0) delta changes in mg/dL (the calculation was originally ported from an early xDrip+ many years ago), the mmol/L calculation has never been sufficiently accurate due to the conversion/rounding of mmol/L meaning that sometimes a +0.2mmol/L delta was actually calculated to +0.1mmol/L.

This changes fixes that and also updates the calculation and string representation methods for all instances in the app, Watch, Widgets etc

Also renamed a bunch of stuff to keep things constant over the whole project
@paulplant
Copy link
Collaborator

The PR you opened worked fine for mg/dL deltas and fixed a long-standing issue that seems to have existed since the very early code was ported over from xDrip+ many years ago. Nobody ever really noticed it, but you're right that it existed!

But once I started troubleshooting, it seemed that the overall calculation for mmol/L had never really worked correctly for certain values either. This is because there are distinct "steps" in the displayed "mg/dL converted to mmol/L" values and this meant that we shouldn't have been just converting the resultant mg/dL delta value into mmol/L.

There is a new commit into the 5.4 staging branch that will finally fix this: 07f4656

This works by first converting (throughout the app) the values into mmol/L (if needed) and then performing the delta calculation and string generation.

@aug0211 , feel free to try building the 5.4 Staging Branch if you fancy giving it a test in both mg/dL and mmol/L.

paulplant added a commit to paulplant/xdripswift that referenced this issue Oct 12, 2024
Related to #JohanDegraeve#544

although the previous commits fixed the issue of erroneous (0 or close to 0) delta changes in mg/dL (the calculation was originally ported from an early xDrip+ many years ago), the mmol/L calculation has never been sufficiently accurate due to the conversion/rounding of mmol/L meaning that sometimes a +0.2mmol/L delta was actually calculated to +0.1mmol/L.

This changes fixes that and also updates the calculation and string representation methods for all instances in the app, Watch, Widgets etc

Also renamed a bunch of stuff to keep things constant over the whole project
@paulplant paulplant linked a pull request Oct 14, 2024 that will close this issue
@paulplant
Copy link
Collaborator

@aug0211 , not sure if you're still following this PR.

If you are, ok to close?

@aug0211
Copy link
Contributor Author

aug0211 commented Oct 22, 2024

Hey! I haven't had a chance to test but I think I saw the PR merged, is that right? The fix was pretty straight forward, so can probably close it out if so!

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 a pull request may close this issue.

2 participants