ceeK / Solar

A Swift micro library for generating Sunrise and Sunset times.
MIT License
568 stars 82 forks source link

Adding of 1 day to sunrise and sunset times happens too early #24

Open guidove opened 7 years ago

guidove commented 7 years ago

I'm in mexico city and noticed that the isDayTime returns false at exactly 18:00 local time (12:00 UTC) while current sunset time is around 20:15. After some analyses I found out that, at exactly that 12:00 UTC, the sunrise and sunset days are shifted one day forward. This results in both sunrise and sunset being later than the current time, which results in false for isDayTime.

I'm assuming this 1 day shift should only happen after sunset, right?

I really appreciate what you've done, and, honestly it goes a bit over my head. I understand that eventually the result of this code is probably wrong:

        let shouldBeYesterday = lngHour > 0 && UT > 12 && sunriseSunset == .sunrise
        let shouldBeTomorrow = lngHour < 0 && UT < 12 && sunriseSunset == .sunset

but I guess it's also very well possible, that is somewhere in the code above these lines.

These are the values for lngHour & UT (after normalisation) for sunrise and sunset respectively, in my location, mexico city:

lat: 19.39258888
lon: -99.28117439
lngHour: -6.61874495933333
UT sunrise: 11.9693350962159
UT sunset: 1.24306218714151

I could work around this by adding a day to the currentTime calculation if date > 12:00 UTC, but I'm not sure is a good idea. From what I've seen here it works for most people, so might be something with my location specifically, while the App I'm using this in is used all over the world.

Any thoughts to a fix or workaround?

guidove commented 7 years ago

I found a workaround that works for me for the moment. I basically strip the date component from the dates and only use the time components. It's not that precise as it could compare the current date/time to the sunrise & sunset time of tomorrow, but that's ok for my usage (I use it to switch App color theme at sunset / sunrise).

It's not fail safe as it doesn't work for very northern / very southern locations where the sunset is after midnight / sunrise if before midnight or even doesn't set or rise at all, so a fix or other workaround would be appreciated.

Here it is:

    public var isDaytime: Bool {
        guard
            let sunrise = sunrise,
            let sunset = sunset
        else {
            return false
        }

        let beginningOfDay = sunrise.secondsFromMidnight() // previously sunrise.timeIntervalSince1970
        let endOfDay = sunset.secondsFromMidnight() // previously sunset.timeIntervalSince1970
        let currentTime = self.date.secondsFromMidnight() // previously self.date.timeIntervalSince1970

        let isSunriseOrLater = currentTime >= beginningOfDay
        let isBeforeSunset = currentTime < endOfDay

        return isSunriseOrLater && isBeforeSunset
    }

And the date extension:

extension Date {
    func secondsFromMidnight() -> Int {
        let hours = Calendar.current.component(.hour, from: self)
        let minutes = Calendar.current.component(.minute, from: self)
        let seconds = Calendar.current.component(.second, from: self)
        return hours*60*60 + minutes*60 + seconds
    }
}
hungri-yeti commented 7 years ago

Thank you guidove, my use case matches yours and your solution helps.

ceeK commented 7 years ago

Hi @guidove! Thank you for the report.

Just to help me pin down this bug and implement a correct fix, would you mind creating a failing test that represents your scenario? This would help tons!

guidove commented 7 years ago

Sorry, do not have any experience with making tests. But it's very easy to reproduce by feeding it the lat & lon mentioned above (is Mex city coordinate) and you'll need to set your device timezone to Mexico city as well probably. Then print out calculated sunrise and sunset date&time and compare them to the official ones. (Just Google 'mexico city sunrise sunset' and it will tell you right in the search results.)

BrandonRoehl commented 6 years ago

I had encountered this and had a fix in my fork

rawmean commented 3 years ago

This solved the problem for me. I hope that this fix is not affected by daylight savings.

        sunrise = calculate(.sunrise, for: date, and: .official)! - 3600*24
        sunset = calculate(.sunset, for: date, and: .official)! - 3600*24