Some openAPS treatments appearing as future dated in Nightscout reports for Australia timezone

[mel2095]

The issue is not affecting all openAPS generated entries - just ones that are created from the afternoon period onwards.

[thebookins]

I looked into this a while back - at the time I came to the conclusion that it was because OpenAPS uploads treatments in local time (i.e. +10 in Sydney). Might be a change required to https://github.com/openaps/oref0/blob/dev/bin/nightscout.sh to upload using a timestamp converted to UTC?

[mel2095]

Did you see it on all entries - I just have it happening with treatments from early afternoon and evening?

[thebookins]

Only with treatments after 2 pm. During daylight savings it's treatments after 1 pm. Which is consistent with the time difference of +10/+11. Is that what you see?

At the time we were switching over from Loop to OpenAPS. Loop treatments displayed fine, which suggested that it was the OpenAPS upload procedure that was at fault - not NS itself.

[mel2095]

Yes that sounds the same as what I see - it's been like it since I setup in Dec. We haven't tried loop to compare it with. Get Outlook for Android<https://aka.ms/ghei36>

…

________________________________ From: Paul Dickens <[email protected]> Sent: Sunday, August 13, 2017 10:49:03 AM To: openaps/oref0 Cc: Mel Clegg; Author Subject: Re: [openaps/oref0] Some openAPS treatments appearing as future dated in Nightscout reports for Australia timezone (#600) Only with treatments after 2 pm. During daylight savings it's treatments after 1 pm. Which is consistent with the time difference of +10/+11. Is that what you see? At the time I we were switching over from Loop to OpenAPS. Loop treatments displayed fine, which suggested that it was the OpenAPS upload procedure that was at fault - not NS itself. — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub<#600 (comment)>, or mute the thread<https://github.com/notifications/unsubscribe-auth/AHs1YDDLeQufIoeZlTGGFQ7Dly3oYmc0ks5sXkf_gaJpZM4O1ise>.

[PieterGit]

@thebookins :the fix must not be in the nightscout.sh, but the created_at in mm-format-ns-treatments.sh must be converted to UTC time. See
#426 (comment)

[thebookins]

Thanks @PieterGit , I knew we'd been over this before. I see that your PR wasn't pulled; do you think that created_at still needs to be converted to UTC?

[PieterGit]

Yes, we need the JSON command to convert created_at to UTC, e.g.
json -e 'this.created_at = new Date(Date.parse(this.timestamp)).toISOString()'

[PieterGit]

@thebookins I just created a pull request #612
that fixes the issue...

BUT BE CAREFULL: I'm seeing duplicate carbs because the mongodb will have both the created_at in UTC format and the created_at in local format (+0200 for me, +1100 for you). I think some/all of the carbs are not seen as duplicates.

@scottleibrand do you know a proper way to add the necessary unduplication code, so that carbs/treatments that are in TIMESTAMP+TZ format are ignored by oref0.

I-to-b discussion is at https://gitter.im/nightscout/intend-to-bolus?at=5992076e614889d4754a04ba

[scottleibrand]

We might want to avoid cross-tz deduplication, and instead just force oref0 to ignore treatments with a non-UTC timezone at the same time we switch it to upload in UTC. We'd still need a compatibility warning not to run older rigs alongside newer ones, though, which probably means this would be best targeted for 0.6.0.

[PieterGit]

I agree that completely switch to ignore treatments with a non-UTC timezone should be targeted at 0.6.x. But I think we can make an easier upgrade path by:

• converting / unifying the created_at for medtronic pump in the Javascript for 0.5.3 and making sure duplicates are not counted twice, .e.g new Date(Date.parse(x.created_at)).toISOString()"
• creating a PR for Nightscout to always convert created_at to UTC when uploading

That way a 0.5.3 rig and 0.6.x rig can still work together fine.

For 0.6.x I think we must also must convert mm-format-ns-treatments.sh to NodeJS and add unit tests for it.

Help is needed/wanted.

I think that we must make a working solution for all the +xx:00 timezones for 0.5.3.

[PieterGit]

Note that this issue is not only related to Australia, but to all +xx:00 timezones. Furthermore, fixing this will also benefit day light saving time issues and possible travelling across timezones if you change your pumps time and your rig timezone.

[thebookins]

@PieterGit converting all date strings to ISO should solve the duplication issue, but I think this change will still cause some disruption since MongoDB will have treatments using the old local timezone and the query we are using does a simple string compare. I guess we could write a python script that crawls over the database and converts all created_at fields, but users would need to remember to do this before updating.

[PieterGit]

The suggestion I make is for 0.5.3 to always convert the created_at to UTC in Javascript, so we don't need a python script (Which off course could be integrated with oref0-setup.sh). For 0.6.0 we can only accept UTC records, so this problem of duplicate carbs would disappear because the timezoned created_at will be ignored. I'm looking for a smooth upgrade path that also support the use of a 0.5.3 and 0.6.x rig in future (for short period of time when users will upgrade their rig).

[scottleibrand]

IMO these changes are all too complicated for 0.5.3, but would be reasonable for 0.6.0. If we want to do something for the 0.5.3 patch release, I think it should be limited to (for example) extending the query duration from 6 hours to ~ 18 hours so that the timezone-blind string comparison doesn't miss anything. Even that would require some testing, though, so if we want to make that change we should do so soon.

[stavlor]

This is something you could potentially use momenjs for potentially

[stavlor]

Might be easier to just make ns do a conversion on created_at if not in utc

[thebookins]

👍 for a longer look back as a short term patch

[scottleibrand]

One of you Aussies or Kiwis want to test out the different lookback durations and see what works well for you, and then prep a PR to change it so the rest of us can test?

[thebookins]

@scottleibrand I'm happy to do that.

[PieterGit]

@scottleibrand : after a good night of sleep I must admit that converting to UTC / unduplicating the created_at timestamps in 0.5.3 is too risky and must be postponed to 0.6.x. I don't see how I can retarget #612 to 0.6.0-dev. Can you do that for me? Or can you change the oref0 github settings so that authors can retarget their own PR's themselves?

I also tried how we should ignore non-UTC created_at timestamps but I couldn't see a clean implementation of that in https://github.com/openaps/oref0/blob/0.6.0-dev/lib/meal/history.js. Help is wanted on that.

@stavlor : I agree momentjs is cool and it's used in oref0 on other places, but I don't think it has any advantages to plain Javascript like: new Date(Date.parse(x.created_at)).toISOString() in this particular case.

[thebookins]

@PieterGit just curious, any reason for wrapping Date.parse in a Date constructor? When I was playing around with this new Date(x.created_at).toISOString() resulted in the same timestamp. Bit more readable too.

[PieterGit]

I think you're right, see https://www.ecma-international.org/ecma-262/5.1/#sec-15.9.3.2 or https://www.ecma-international.org/ecma-262/8.0/index.html#sec-date-value (ECMAScript v8): "parsing v as a date, in exactly the same manner as for the parse method"

I couldn't find good documentation on the Date constructor and Date.parse worked so I didn't look further.

[sulkaharo]

In my experience it's good to be lenient in what data is ingested and strict what data is output; look like a good related improvement would be to add code to the Nightscout API that ensured all dates are normalized to UTC upon input, before inserting to MongoDB.

[thebookins]

Temporary fix in #614 to get temp targets and carb histories working again for everyone at UTC +x.

[PieterGit]

@sulkaharo @thebookins : i created a patch for Nightscout which converts the created_at for inserted or updated/changed records to UTC format. This way Nightscout will always save the created_at timestamp in UTC format.

This follows https://en.wikipedia.org/wiki/Robustness_principle / Postel's law "Be conservative in what you send, be liberal in what you accept".

The branch is based on @sulkaharo branch nightscout/cgm-remote-monitor#2736 and can be found here: https://github.com/PieterGit/cgm-remote-monitor/tree/treatments_utc

Please test this Nightscout branch and check to see if all COB's and created_at timestamps are fine. I tested in on a 0.5.3 rig, and can't find any problems with this Nightscout solution.

[thebookins]

@sulkaharo @PieterGit @scottleibrand I just noticed that autotune uses a local timezone string compare to filter treatments.json:

Query: http://<NIGHTSCOUT_URL>/find\[created_at\]\[$gte\]=2017-08-27T20:00+1000&find\[created_at\]\[$lte\]=2017-08-29T00:00+1000

If I'm not mistaken, this will need to be changed to UTC with nightscout/cgm-remote-monitor#2763. This also means that currently autotune won't be getting all of the carb info if some of it was entered through NS (with UTC timestamp).

Would much of this messiness be resolved by storing created_at as a Date object in MongoDB (https://docs.mongodb.com/manual/reference/method/Date/)? Then, I imagine the query would do a true date comparison, rather than a string comparison. Are there any performance issues with this approach?

[scottleibrand]

I believe I fixed the autotune timezone issue in 0.6.0-dev. Are there any remaining issues here we want to actively work on fixing? Or are we happy with the fixes in place for now?