Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix for incorrect results if local day is "tomorrow" in UTC #48

Open
wants to merge 5 commits into
base: master
Choose a base branch
from

Conversation

aaronfreimark
Copy link

Fixes #40

This issue appears if the date/time given to calculate is already "tomorrow" at UTC. For example, I'm in New York City, which is GMT-5. If I calculate between 00:00 - 18:59 hrs local time, I get a correct result. But if I calculate 19:00-23:59 local time, Solar return's tomorrow's result. A test is included in this PR to demonstrate.

To fix, I added an optional timezone: parameter to Solar(). If timezone is not specified, UTC is assumed. This makes the patch backwards compatible. If timezone is included, the current time (in seconds) is offset by the time different between local and UTC. This ensures the calculations are done on the correct date, and not possibly "tomorrow's" date.

@benck
Copy link

benck commented Feb 13, 2021

I use only the "isNighttime/isDaytime" method from the original ceeK/Solar package. And the result was sometimes wrong.
Your fix-gmt-offsets branch fixes my issues and no need to pass the timezone parameter if I only use the "isNighttime/isDaytime" method.

Thank you @aaronfreimark !

@1ec5
Copy link
Collaborator

1ec5 commented Feb 17, 2021

(This PR is based on #45 and #47.)

@benck
Copy link

benck commented May 8, 2021

@aaronfreimark in some extreme cases, there is no sunrise or sunset time (polar day or polar night), your fork fix will crash.

date: 2021-05-08 18:38:00 +0000
lat: 71.89013997880043
long: 165.3655457455773

When accessing isDaytime() with this example data, the "tomorrowSunSet" variable will be nil (returned from calculate() method) which will be unwrapped later in the following code:

let isDayTomorrow = tomorrowSunrise! < currentTime && currentTime < tomorrowSunSet!

Unwrapping the nil value leads to crashes. Is there a fix for this bug? Thanks.

@dasmer
Copy link

dasmer commented Feb 26, 2022

@benck were you ever able to resolve the bug you stated above?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Sunrise and Sunset dates off by 1 day.
6 participants