search

hackgvl / trolley-tracker-api-dot-net

API for tracking Greenville's trolleys with .NET
MIT License
4 stars 6 forks source link

Potential Time Zone issue. #18

Closed qvaughan closed 8 years ago

qvaughan commented 8 years ago

I noticed inside of TrolleyTracker/Controllers/UTCToLocalTime.cs on line 14, the time zone is set as "Eastern Standard Time." I'm not sure if this has caused any issues in the project, but it would give incorrect results while we're in daylight savings time. However, you should be able to use "Eastern Time" to cover both standard and daylight savings. Either way, great work on the project!

ajhodges commented 8 years ago

Happy first GitHub issue! Is there any documentation to back this up? Mostly curious because I use this value in a production quartz job at work :scream::scream::scream::scream::scream:

Also @bikeoid I saw your comment about nodatime and just wanted to say that I'm using jodatime in the Android project and it's very nice.

qvaughan commented 8 years ago

This may absolutely be a lack of knowledge on my part regarding .NET, but generally speaking, Greenville is covered by three time zone IDs. Eastern Standard Time (EST UTC-5:00) refers to Greenville's time while not on daylight savings, Eastern Daylight Time (EDT UTC-4:00) during daylight savings, and Eastern Time (ET) that is dynamic since it always refers to our time zone no matter whether DST or not. However, maybe .NET doesn't use the standard time zone IDs. I'm not really from a .NET world.

ajhodges commented 8 years ago

Ah, I see. You know more about time zones than I do :)

From what I can read about Windows/.NET, it will always be "Eastern Standard Time". Looks like it is dynamic: http://stackoverflow.com/questions/964894/how-to-convert-time-between-timezones-utc-to-edt

Definitely a confusing label, seems to me that ET should be used instead of EST if it is going to be dynamic.

bikeoid commented 8 years ago

Good observation! The call makes use of a Windows setting, and it is querying into a fixed registry key, which unfortunately includes 'Standard' as part of the name, although it really refers to a time zone and contains variable information depending on whether the user has configured for daylight saving time and if it is currently DST. In practice, this has worked correctly on either side of the Daylight Saving Time season in our app. More information at http://stackoverflow.com/questions/14149346/what-value-should-i-pass-into-timezoneinfo-findsystemtimezonebyidstring , particularly in the first answer.

I could probably comment that section better.

The last answer that uses TimeZoneInfo.Local.Id appears to be a better self-documenting and self-adapting solution, but it only works if the server is in the same time zone as the clients, and not at all if run on a cloud server such as Azure.

algonzalez commented 8 years ago

EST is just the standard/normal time. Daylight savings (EDT) is an adjustment to that, based on rules that may change, but are usually maintained in OS or Framework. AFAIK, on Windows it is maintained in the OS and is updated as necessary with OS updates.

The docs for ConvertTimeFromUtc (https://msdn.microsoft.com/en-us/library/system.timezoneinfo.converttimefromutc%28v=vs.110%29.aspx) state that the "method applies any adjustment rules in effect in the destinationTimeZone", so it should give the correct value whether daylight savings is in effect or not. The example for the method shows this for Central Standard Time.

@bikeoid beat me to it. Cheers.

qvaughan commented 8 years ago

LOL, I learned a lot of useless information about time zones when I worked on scheduling software a little while back that had to work internationally.

However, this "potential issue" definitely seems to stem from my misunderstanding of .NET's naming convention vs standard tzdata naming conventions. My bad for raising a false issue.

ajhodges commented 8 years ago

:+1: I'll do the honors of closing this issue and leave ya'll with this (related): https://www.youtube.com/watch?v=-5wpm-gesOY