Fix rawbg calculations

[jpcunningh]

Change the rawbg calculations to simply use the noise value to determine whether to use the calibrated filtered value vs the calibrated unfiltered value to display. It was creating a ratio that had the effect of exaggerating acceleration when BG was changing.

[sulkaharo]

This might have unexpected side-effects in situations like when a new sensor is inserted and calibrated values aren't yet available. How extensively have you tested this?

[jpcunningh]

Yes. We've been running it for about 3 months now.

I submitted this modification because the old code was always exaggerating SGV trajectory changes which was providing incorrect data to the user.

Regarding when new sensors are inserted, I am running Lookout, which I changed to send a unity calibration value to NS when a new sensor is inserted. This allows continuing to send the SGV entries with the calibrated glucose value set to 5, and NS displays the raw values exactly as they come out of the transmitter rather than using the previous sensor's last know calibration value.

[sulkaharo]

We have a lot of users still on G4 and other solutions; need someone to test how those behave

[jpcunningh]

I agree. I don't think it will behave poorly, but I know from at least one conversation I've seen on the Loop group there are some who thought the exaggerated trajectory changes were predictions, which they weren't. The different behavior will be thought to be a loss of the predictive logic rather than correcting an error.

…

On Fri, May 18, 2018, 07:38 Sulka Haro ***@***.***> wrote: We have a lot of users still on G4 and other solutions; need someone to test how those behave — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#3360 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/Ag8XaJC00au1rHQlpOWoLlrNsRTDYlspks5tzsDWgaJpZM4SEwWC> .

[PieterGit]

@jpcunningh We use a G4 but no raw and a single rig that is connected to the CGM by USB. I believe that will use the CGM readings from the device.
Is it possible to find testers for this feature, or should we postpone it to 0.11 Nightscout release?

@sulkaharo What side effects are you worried about with this PR? What nees to be tested in this PR?

[jpcunningh]

Since the prior discussions, I've implemented sending raw values to NS from Lookout during the sensor warm-up periods and the code works as expected. I'm as confident as I can be that the code works correctly. I've used it uninterrupted without issues since the PR was created. I would recommend merging it to dev.

…

On Tue, Jun 26, 2018, 15:59 PieterGit ***@***.***> wrote: @jpcunningh <https://github.com/jpcunningh> We use a G4 but no raw and a single rig that is connected to the CGM by USB. I believe that will use the CGM readings from the device. Is it possible to find testers for this feature, or should we postpone it to 0.11 Nightscout release? @sulkaharo <https://github.com/sulkaharo> What side effects are you worried about with this PR? — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#3360 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/Ag8XaO0UntepMQQQDJlIWCUMVWBFpEtcks5uAqDIgaJpZM4SEwWC> .

[PieterGit]

@jpcunningh : i think this can be merged to dev, but would like a 👍 on it from the other core committers like @sulkaharo or @jasoncalabrese or @MilosKozak

[TwistaTim]

Support the move to merge into dev as I want to test :)

[jpcunningh]

I think we determine the rational for the original behavior over in gitter's oref0/hardware-dev channel, so more discussion is probably needed.

It unsmoothes the raw data and doing so makes the white dots for raw bg visible more often and gives early indication of trend trajectory changes. Whereas, I am more interested in accurate plotting of the values rather than use them for trend indications.

[PieterGit]

Based on @jpcunningh remark I postponed this to 0.11 milestone. I think this can be merged to dev as soon as 0.10.3 is out of the door.
@Tornado-Tim you can already test by using the https://github.com/jpcunningh/cgm-remote-monitor/tree/wip/fix-rawbg-calculations branch

[jpcunningh]

@PieterGit, to move this forward, I modified it to be controllable by an extended setting for the rawbg plugin that defaults to existing behavior and added some text in the README to describe the new option.

I've tested the various option configuration scenarios and believe this is good to merge into dev.

[jpcunningh]

I think I've missed a place that needs to be called with extended settings, so please don't merge just yet.

[jpcunningh]

@PieterGit, it's functioning as intended, now. Without setting the new extended settings environment variables, rawbg continues to operate as it did. The new extended settings environment variables provide control over how the rawbg values are calculated.

[jasoncalabrese]

I haven't tested this, and I'm using the G6 now, but the code changes look good

[PieterGit]

@jpcunningh is this ready to merge to dev? How should we proceed with PR's that need testing, but nobody seems to have time to do proper testing.

[jpcunningh]

I believe it is ready. It should be an invisible change to those currently using raw BG.

To change the behavior to use the corrected calculation, the user has to set an extended settings flag. This prevents unfamiliar users from thinking the update broke the raw display due to the raw BGs overlaying on top of the calibrated BGs a much larger percentage of values than previously.

I'm not sure how to address the general need to test PRs. I'll try to start testing other PRs tagged as needing test where I can to help.

[PieterGit]

This PR has been integrated with the https://github.com/nightscout/cgm-remote-monitor/tree/needs_testing
branch. Please test that branch and report at this issue or at #4169

[PieterGit]

@jpcunningh I don't understand the configuration parameters enough. Can you elaborate more on what the differences are and what parameters you should use advice in what situation. I think the docs should be expanded that you should not set both UNFILTERED and SMOOTH to true at the same time. Or perhaps it's better to convert it to a tri-state variable (similar to BASAL_RENDER)

[jpcunningh]

@PieterGit, I changed it to a tri-state variable and updated the README. Great suggestion.

Does the new setting help? Thanks!

[PieterGit]

This PR has been tested as part of the need_testing branch of #4169
@blocklist_twitter @balshor @sulkaharo @jpcunningh @unsoluble @veryfancy @balshor @tynbendad have run this PR for a week.
No negative stuff popped up. Gitter link: https://gitter.im/nightscout/public?at=5c2f33d12863d8612bd78eb0

If you encounter success or failure with this PR, please reopen the issue and share your findings.

Closing PR, will be merged with #4169