Extended last glucose reading date validation #54
Conversation
…a wrong data format).
| if (minAgo > 10 || minAgo < -5) { // Dexcom data is too old, or way in the future | ||
| var reason = "BG data is too old, or clock set incorrectly "+bgTime; | ||
| if (!checkGlucoseDateValidity(glucose_data[0])) { | ||
| var reason = "BG data is too old, clock set incorrectly, or cannot get BG time"+bgTime; |
There was a problem hiding this comment.
You need a space at the end of the string here before the +bgTime.
|
I like this approach. Let's see if we can also set up some tests of all three data formats, and at least one invalid one, to make sure that it behaves as expected in all conditions. Are you familiar with how to set up tests? |
|
Refactored input of last glucose readings, added unit tests. |
|
Looks like something happened with oref0-determine-basal.js to make the diff think everything changed? Whitespace? |
|
Files were modified by visualStudio and comitted by visual studio embedded git, most probably this effect is due to this. |
|
Now unfortunately I cannot corrdct the commit as I'm not at home. I'll be able to do something next week. |
|
Any update on this? Or should I try to fix the formatting so this can be merged to dev? |
|
@johnmales noticed that he has "on several occasions run into the problem of the medtronic CGM data being stale - sometimes hours old - and oref0 going ahead and setting temp basals. Usually this circumstance happens with that decocare page loading bug and the downloaded cgm data getting “stuck” redownloading old data repeatedly." It looks like he's seeing this issue, which mean this is now a safety-critical bug that we need to get fixed ASAP and merged to both dev and master. |
|
Happened again to us last night. Any chance to get this merged with Dev soon? |
|
If you/someone can fix the formatting we can review this for merge. |
|
I'd be happy to but not sure you guys want me to be mucking with code. What formatting needs to change? |
|
Just whitespace I think. Fix it so that https://github.com/openaps/oref0/pull/54/files shows an actual diff, not "everything changed". |
|
Okay - I think I have done what is needed. However, I HIGHLY recommend someone take a look at what I have done and compare it to ktomy changes. The code has changed since he made his pull request so I think I found all the places he changed his code and made it in the appropriate places but as a non-coder, I can't be sure I caught everything |
|
Thanks. What branch did you update? |
|
I updated the dev branch on oref0 |
|
Ah, #103. Not sure why I didn't notice that earlier. |
|
Sorry I didn't know how to edit this one so just started from scratch |
|
You can't edit @ktomy's, so that was exactly the right thing to do. I just needed to mention one from the other to link them for discoverability. Thanks. Putting out a call for testers now. |
As discussed on gitter, old versions of ns uploader have different date format. I made a better date validation to be sure that in case of "invalid date" determine-basal will return an error.