nightscout / cgm-remote-monitor

nightscout web monitor
GNU Affero General Public License v3.0
2.42k stars 71.75k forks source link

Fixed offset timezones #4901

Closed ps2 closed 3 years ago

ps2 commented 5 years ago

Is your feature request related to a problem? Please describe. When Loop uploads fixed offset timezones, like "GMT-0500" they don't show up in the timezone picker for the profile editor. The schedule rates themselves seem to be displaying properly in the main screen, so it seems like there is some level of support for them. Fixed offset timezones are important for Loop, as they are more representative of how time is tracked in a pump than timezones that have special rules for daylight savings time or other switches.

Describe the solution you'd like The offset that the schedule is stored in show be displayed in the picker.

sulkaharo commented 5 years ago

I'm not sure I follow what the desired end goal is here? Where do you see the profile time zone being used? Is Loop reading it in? Is some data being displayed wrong somewhere?

Nightscout's current date implementation works so that when data is sent to the REST API, the API expects the dates to use the ISO-8601 format. The date is then normalized to UTC in the database for the purpose of REST queries to work as expected (given the dates are treated as Strings in the database layer and if the system allowed mixed format non-normalized dates, the API would give incorrect query results). The API then returns the normalized strings, unless the instance is configured to de-normalize strings back to format that has the time zone. Internally in the runtime the dates are deserialized to Date objects and logic either uses the API in Date() or "UTC milliseconds from January 1, 1970 00:00:00 UTC" as per Date.now() representation for comparisons. Nightscout clients are expected to basically use whatever native date implementation is in the platform they've been implemented in, and deserialize the ISO-8601 dates to the native date objects and not make assumptions on the string representation other than the date being in the ISO format.

ps2 commented 5 years ago

The profile timezone should be shown in the profile editor, and it isn't for fixed offset timezones.

ps2 commented 5 years ago

I also realized that the console log was filling with error related to the fixed offset timezones:

211_-8_β†’

Based on those errors, I patched Loop to upload fixed offset timezones with identifiers like ETC/GMT+6 instead of "GMT-0600" (https://github.com/ps2/rileylink_ios/pull/535), which does fix the console errors. But, unfortunately, the dropdown in the profile editor is still blank, so I'll keep this ticket open.

Nightfoxy commented 4 years ago

Would this also cause the issue where the basal, IOB and COB data is missing in the day to day report when your timezone isn't the same as the person who is uploading? I tried setting the profile editor timezone manually and re-running the report but setting the profile timezone didn't fix it. image

marionbarker commented 1 year ago

I posted a question on Loop zulipchat and was told by @ps2 that he believes this ticket was closed prematurely.

I'll repeat the zulipchat post here and then add follow-up information in next comment.

This has been a long standing issue for mentors who help Loopers via viewing their Nightscout sites.

Problem:

Figure below shows the same day plotted with my computer set to 3 different time zones

nighscout-time-zone

List of open issues on time zone, Nightscout and Loop.

cgm-remote-monitor (NS):

Loop:

marionbarker commented 1 year ago

The response from @ps2 on Loop zulipchat is as follows:

Pete Schwamb: My guess is it's related to Nightscout not handling fixed offset timezones properly. This ticket was closed: https://github.com/nightscout/cgm-remote-monitor/issues/4901, but I'm not sure it was actually fixed. I think the issues you're seeing might be a better use case to drive proper handling of these timezones in Nightscout.

Pete Schwamb: There has been a widespread misunderstanding/mishandling of pump timezones by Nightscout, OpenAPS, etc. These systems will often take the user's current timezone, with daylight savings time effects, and use it as the timezone of record for basal schedules, etc. The issue is that pumps don't really use those timezones, as they don't account for DST effects. They just use a dumb, fixed offset timezone. If the pump runs by itself through the night during a DST change, the pump events on it should be interpreted with the fixed offset timezone. Loop is doing the right thing by uploading a fixed offset timezone to represent how schedules are being used by Loop, and the pump, at a particular moment in time.

bewest commented 1 year ago

Thanks for digging this up, @marionbarker. I've been meaning to spend some time on this issue.

Looks like the issue is mainly the profile records. Here's what Loop provides:

{"_id":"63bc9a9274a6c7619984d57d","store":{"Default":{"timezone":"ETC/GMT+8","dia":6.166666666666667,"basal":[{"time":"00:00","timeAsSeconds":0,"value":1.8},{"time":"05:30","timeAsSeconds":19800,"value":1.7},{"timeAsSeconds":81000,"value":1.8,"time":"22:30"}],"carbs_hr":"0","carbratio":[{"time":"00:00","value":10,"timeAsSeconds":0}],"delay":"0","target_high":[{"value":102,"timeAsSeconds":0,"time":"00:00"}],"sens":[{"timeAsSeconds":0,"time":"00:00","value":40}],"target_low":[{"timeAsSeconds":0,"value":97,"time":"00:00"}]}},"startDate":"2023-01-09T22:52:01Z","enteredBy":"Loop","defaultProfile":"Default","units":"mg/dL","mills":"1673304721904","loopSettings":{"minimumBGGuard":55,"dosingEnabled":true,"bundleIdentifier":"com.5A8TKU9HN9.loopkit.Loop","preMealTargetRange":[64,65],"overridePresets":[{"duration":3600,"insulinNeedsScaleFactor":0.5,"targetRange":[120,125],"name":"sleepin","symbol":"πŸ€Έβ€β™€οΈ"},{"symbol":"πŸš΅β€β™‚οΈ","duration":10800,"targetRange":[135,136],"insulinNeedsScaleFactor":1.5,"name":"horse"},{"targetRange":[165,180],"insulinNeedsScaleFactor":0.7,"symbol":"⛹️‍♂️","duration":5400,"name":"basketball"},{"symbol":"⚽️","insulinNeedsScaleFactor":0.8,"name":"soccer","targetRange":[155,170],"duration":8100},{"duration":7200,"name":"tennis","targetRange":[160,165],"symbol":"🎾"},{"targetRange":[110,115],"symbol":"🏹","insulinNeedsScaleFactor":1.2,"name":"medicine","duration":0},{"name":"swim","duration":6300,"targetRange":[150,180],"symbol":"πŸŠβ€β™‚οΈπŸŠβ€β™‚οΈ","insulinNeedsScaleFactor":0.8},{"symbol":"πŸ’ƒ","name":"extra","insulinNeedsScaleFactor":1.5,"duration":3600},{"duration":7200,"targetRange":[108,112],"symbol":"βž–","name":"less","insulinNeedsScaleFactor":0.9}],"deviceToken":"b37bba40759e47496f0663025af5ece63feb439c87ec595e75b9d378f72da323","maximumBasalRatePerHour":6,"scheduleOverride":{"insulinNeedsScaleFactor":1.5,"name":"extra","duration":10800,"symbol":"πŸ’ƒ"},"maximumBolus":9.9}}

As Pete mentions, the timezone field is set to timezone":"ETC/GMT+8".

openaps will produce one of the names in the list of official timezone names.

Here's the List. https://en.wikipedia.org/wiki/List_of_tz_database_time_zones

The format Pete mentions is on the list. It would be great to take a look at AndroidAPS/openaps or similar and see if theirs differ. It looks like FreeAPSXNG or similar has the same issue.

bewest commented 1 year ago

I'm not sure the issue are the way timezones are handled at-large, it looks like everything that is a valid timezone name and is 8601 compliant is working as expected. I think the issue might be that the daytoday reports need to have the day start at midnight in the timezone of the profile. The daytoday report in particular is always in the profile's timezone. Currently the day starts for the chart in the browser's timezone.

marionbarker commented 1 year ago

Thanks much Ben.

I looked and there are a lot of day to day report issues open. I think this one is appropriate for me to add to.

https://github.com/nightscout/cgm-remote-monitor/issues/6853

sulkaharo commented 1 year ago

There's several things to do here: 1) Document exactly the list of supported time zones and add validation that those are used in the code, 2) make complete refactor of the graphing code, which has bugs in the time zones but location of those is unknown. The Moment errors above are probably related and caused by Nightscout not uploading the full Moment time zone library to the client, which last I checked was several megabytes of data that significantly slowed down the client. There's been attempts at getting rid of Moment altogether, but I haven't been able to find a replacement that actually works (proposed libraries like Day.js have terrible bugs with... time zone support). Right now the "easy fix" is for Loop to upload recognised time zones - ones from the list you can see in the profile editor.

sulkaharo commented 1 year ago

Ah right, assuming people are using a version of Loop that uploads a zone string from this list, UI in profile editor is probably failing with the time zone due to simple string case mismatch. This is nontrivial to fix though as the UI case right now is strongly tied to the persisted data and we probably don't want the non-cased string comparison to leak into the persisted data

sulkaharo commented 1 year ago

7833 should change the profile editor behaviour so it shows the time zones uploaded by Loop. And maybe changes the reports to work as well. I think what's happening is, the ISO standard is case sensitive with the time zone identifiers (the identifiers are "not just strings") and the time zone code uploaded by Loop doesn't comply with ISO as it's using the wrong case. Loop uploads the zone as ETC/GMT+6, when the correct form is Etc/GMT+6. The workaround in the PR is not guaranteed to stay in the codebase forever. If someone could test that branch with data from Loop, that'd be great.

bewest commented 1 year ago

@marionbarker, I haven't tested, or run this properly, but reading through additional code today, I suspect this change might help line up the daytoday charts so that midnight is always relative to the profile's timezone. https://github.com/nightscout/cgm-remote-monitor/pull/7836

marionbarker commented 1 year ago

I heard that dev won't build on Google Cloud, which is what I'm using for my NS sites. Here's the information: Discussion 2618: NightScout on Google Cloud - update to DEV (15.0.0) version not possible. I tried it anyway and it didn't work; but subsequently heard from Navid how to update my VM to build dev. But that will be another day. If you think I'll get a different result when applying patches to dev, I'll be happy to repeat the test.

Since I couldn't build dev on Google Cloud, I patched the PR into master in my branch: marionbarker:day-to-day-test

There was no discernable difference with or without #7833. The change came from #7836 in terms of where midnight line shows up and where basal, IOB, COB plots are included or cut off. Days are still confused when not in the same time zone. Basal, IOB, COB plots are now complete for -2 hrs wrt my time zone, but date is wrong. Basal, IOB, COB plots completely missing now for +3 hr wrt my time zone (Default line all the way to the right).

To be clear: my time zone = PST, +3 hrs is EST, -2 hrs is Honolulu (don't know the code)

Patch seen here: https://github.com/marionbarker/cgm-remote-monitor/compare/master.../day-to-day-test

Graphic below - left side is using master (14.2.6) and right side is using day-to-day-test (14.2.6-day-to-day-test.7833.7836). The actual day is Thursday Jan 12, 2023. The glucose, prediction, bolus and carbs are correct for each plot - just labeled with wrong day/date if time zone doesn't match.

nightscout-time-zone-test-master-7833-7836

sulkaharo commented 1 year ago

SO wait, am I correct in understanding the only bug is the date in on top of the graph? Looks like additionally the profile switch is rendered at the wrong position.

paddingtonil commented 1 year ago

Although my phone and computer are set to GMT +2, my profile shows GMT -2. However, the reports do not show a time offset.

Screen Shot 2023-02-20 at 12 04 24

sulkaharo commented 1 year ago

@paddingtonil the profile editor shows whatever is saved as the time zone in the profile. If your profiles are saved by Loop, it means the profile sent by Loop to Nightscout says the time zone is GMT-2. The value does not automatically reflect any device, it's intended to be manually set to the time zone of the PWD who's profile this is.