Timezone handling is putting date to one day ahead and daylight savings is not being processed

[grumBit]

Hi Chris,

I think I've found an issue to do with time zones. When calling Solar with my lat/long (latitude: -38.2613, longitude: 145.1896), the returned date is always one day ahead of the for-date for all times (sunset, sunrise, all modes). Also, daylight savings is not being processed for my timezone (Melbourne, Australia)

I think the day-ahead problem may stem from line 188;

let localT = UT + (Double(timeZone.secondsFromGMT) / 3600.0)

as this pushes the "hour" variable in the following line;

let hour = floor(localT)

into numbers between 24 and 48 (i.e. the next day)

I tried fixing this up by putting the hour back into 0-24 hour range, but this mucked up the minute and second calculations. So, I had a think about timezones and went back to the underlying returned NSDate being an absolute number of seconds, regardless of timezone and that all your calculations were for UTC time. Given this, I (hack) changed line 188 to do nothing as follows;

let localT = UT

and replaced line 199;

var date = calendar.dateFromComponents(components)

with;

calendar.timeZone = NSTimeZone(abbreviation: "UTC")!
return calendar.dateFromComponents(components)

By changing the the time zone to suit your calculated UTC time, the returned NSDate will be the correct absolute number of seconds and any code using the returned NSDate will have the appropriate timezone set. This means the time zone calculations for the calling apps purposes will all be handled by the Swift libraries. As a side effect of getting the day correct, this also fixed up the issue where daylight savings was not being handled correctly.

NB: the early return I added in line 200 intentionally means lines 202 onwards are not called as they are no longer required.

I tested this fix for both Melbourne, Australia and London, England. It got the correct day, time and daylight savings was processed correctly. What do you think?

Cheers, Graham.

[grumBit]

Hi Chris,

I forgot to say, but no doubt you worked it out for yourself, that my hack of "let localT= UT" on line 199 would be done better by getting rid of the localT variable entirely and just using UT throughout instead. So, if my suggested fix is good, you'd probably want to tidy that up and also delete line 202 onwards.

Cheers, Graham.

[ceeK]

Sorry for the delay Graham - I have been away the past week. Thank you so much for these reports as it means a great deal. I'll look into this an evening this week and try and sort something out. Unfortunately Solar wasn't tested beyond my use case (which is pretty limited, I honestly didn't expect other people to actually use it!).

[grumBit]

Hi Chris,

Sorry for the delay in replying, I've been busy at home with the kids over the school holidays. I greatly appreciate your work with Solar. I'm just getting back to developing software after 8 years out the industry. I was a physio for a while, until I got a pick axe stuck in my foot, which means going back to a desk bound job. So, I'm teaching myself Swift and iOS development and hoping to pick up working for home development down the track.

Right now I'm writing myself an app for notifications to remind me to put our chickens to bed just before dark. They won't go to bed when it's light and if left too late, they roost in the trees and become vulnerable to foxes. Hence, Solar is perfect for me to work out when to schedule the notification. To be honest, I'm quite surprised such functionality is not included in standard libraries. It seems like a fundamental calculation that's been around for a long time. So, I'm very glad you put the effort in to create a Swift implementation!

Anyway, I cobbled together the use cases below to check out Solar, which I thought might be of use to you. They don't qualify as unit tests as they don't automate the testing and don't have expected values, but they were useful for me to compare to online sources like http://www.timeanddate.com/sun/australia/melbourne.

Generally speaking Solar is very close to timeanddate.com, which makes it perfect for my purposes. However, I did also notice that according to Solar, the longest and shortest days in the southern hemisphere are 5th of Jan and 14th of June respectively, when I think they should be 21-22 of Dec and 21-22 of June. I had a look at Solar and I couldn't work out where these anomalies were coming from. Sorry, but to me Solar looks good!

Cheers, Graham.

//
// SolarNotificationsTests.swift
// Notifications
//
// Created by Grum on 16/06/2016.
// Copyright © 2016 Grum. All rights reserved.
//

import Foundation

class SolarNotificationTests{

static func solarTesting(){

    print("test sunset every day for a year")
    var testDate = NSDate()
    for _ in 1...365 {
        solarTest(testDate)
        testDate = testDate.dateByAddingTimeInterval(60*60*24)
    }

    print("test one day a month for a year")
    testDate = NSDate()
    for _ in 1...12 {
        solarTest(testDate)
        testDate = testDate.dateByAddingTimeInterval(60*60*24*30.5)
    }

    //print("test every hour for 2 weeks")
    testDate = NSDate()
    for _ in 1...14 {
        for _ in 1...24 {
            //solarTest(testDate)
            testDate = testDate.dateByAddingTimeInterval(60*60)

        }
    }

    //print("test every minute for two days")
    testDate = NSDate()
    for _ in 1...2 {
        for _ in 1...(24*60) {
            //solarTest(testDate)
            testDate = testDate.dateByAddingTimeInterval(60)
        }
    }

    let calendar:NSCalendar! = NSCalendar(calendarIdentifier: NSCalendarIdentifierGregorian)
    calendar.timeZone = NSTimeZone.systemTimeZone()

    print("test daylight savings start in Melbourne")
    let dateComp:NSDateComponents = NSDateComponents()
    dateComp.year = 2016;
    dateComp.month = 04;
    dateComp.day = 02;
    dateComp.hour = 23;
    dateComp.minute = 55;
    dateComp.timeZone = NSTimeZone.systemTimeZone()

    testDate = calendar.dateFromComponents(dateComp)!
    solarTest(testDate)

    for _ in 1...10 {
        solarTest(testDate)
        testDate = testDate.dateByAddingTimeInterval(60)
    }

    print("test daylight savings end in Melbourne")
    dateComp.month = 10;
    dateComp.day = 01;
    dateComp.hour = 23;
    dateComp.minute = 55;
    testDate = calendar.dateFromComponents(dateComp)!
    solarTest(testDate)

    for _ in 1...10 {
        solarTest(testDate)
        testDate = testDate.dateByAddingTimeInterval(60)
    }

    print("test end of year")
    dateComp.month = 12;
    dateComp.day = 31;
    dateComp.hour = 23;
    dateComp.minute = 55;
    testDate = calendar.dateFromComponents(dateComp)!
    solarTest(testDate)

    for _ in 1...10 {
        solarTest(testDate)
        testDate = testDate.dateByAddingTimeInterval(60)
    }
}

static func solarTest(testDate: NSDate){
    let testSolar = Solar(latitude: -38.2613, longitude: 145.1896, forDate: testDate)
    if let testSunsetDate = testSolar?.sunset {
        print("For test date \(testDate.getLocalFormatDate()), sunset is \(testSunsetDate.getLocalFormatDate()), (UTC \(testSunsetDate.getFormatDate(forTimeZone: "UTC")), \(testSunsetDate.timeIntervalSinceReferenceDate))")
    } else {
        print("For test date \(testDate.getLocalFormatDate()), sunset is nil ")
    }
}

}

extension NSDate {

func getLocalFormatDate() -> String {
    let dateFormatter = NSDateFormatter()
    let timeZone = NSTimeZone()
    dateFormatter.timeZone = timeZone
    dateFormatter.dateFormat = "yyyy-MM-dd HH:mm:ss"
    return dateFormatter.stringFromDate(self)
}
func getFormatDate(forTimeZone timeZone: String) -> String {
    let dateFormatter = NSDateFormatter()
    let timeZone = NSTimeZone(name: timeZone)
    dateFormatter.timeZone = timeZone
    dateFormatter.dateFormat = "yyyy-MM-dd HH:mm:ss"

    return dateFormatter.stringFromDate(self)
}

}

[ceeK]

Hi Graham,

It surprised me that the Sunrise / Sunset calculation isn't provided as well. I'd at least expect it in iOS 9 because of Night shift mode!

In regards to it being a bit off the timeanddate.com times, I believe this is due to the algorithm I use. As far as I know, it uses a few simplifications here and there. I think a more accurate implementation may be put in for 1.0, because some of the calculations for issue #4 tie in with the more accurate algorithm.

For the actual bug, I applied your changes but I'm still coming out with sunrise being +1 day for Melbourne. The UTC dates returned from Solar have the same date (9th July), but this then means that the sunrise comes back as UTC21:30 9th July, and sunset as UTC7:30 9th July. The 21:30 needs to be on the 8th July to line up.

When you subsequently use this UTC date in a NSDateFormatter with AEST timezone, it adds 10 hours to both dates pushing the sunrise into 10th July and keeping the sunset in 9th July. Did you experience this? Here is my code:

if let solar = Solar(forDate: NSDate(), latitude: -38.2613, longitude: 145.1896) {
        print("UTC Sunrise: \(solar.sunrise!)")
        print("UTC Sunset: \(solar.sunset!)")
        let dateFormatter = NSDateFormatter()
        dateFormatter.timeZone = NSTimeZone(name: "Australia/Melbourne")!
        dateFormatter.dateFormat = "yyyy-MM-dd'T'HH:mm:ssZZZ"
        print("AEST sunrise: \(dateFormatter.stringFromDate(solar.sunrise!))")
        print("AEST sunset: \(dateFormatter.stringFromDate(solar.sunset!))")
}

For the sunrise to be AEST7:30 in Melbourne on the 9th July, the UTC time would be have to be (-10 hours for AEST) 21:30 on the 8th July. However, when the algorithm normalises the UT time, it gives us this correct 21:30, but it doesn't care about the actual date. When we then go on to apply that date of 21:30 on line 195, we don't account for the fact that it's supposed to be the 8th July.

To fix this, we need to know the timezone for the latitude / longitude in order to apply the local offset (present in the actual algorithm). When we add 10 hours to the 21:30, we get a normalised 7:30. The algorithm has technically succeeded here. It has given us the localised sunrise time. However, to make this universal, we'd prefer for it to be in UTC.

We can do this by setting the calendar with the supplied time zone, which can give us the correct UTC time and date as it knows the local offset for the date in that timezone, which is -10 hours for AEST in July.

The downside to all this is that the local timezone needs to be passed in correctly for it to work. If you passed in the wrong timezone, the offset would be different, and the day / hour would be completely off. The solution to this is to calculate the timezone based off of the lat/long, but as far as I know this needs a local lookup table to match a lat/long to a closest city and use the timezone of that city.

From line 187, this is what I've got:

        // Convert UT value to local time zone of lat/long provided
        var localT = UT + (Double(timeZone.secondsFromGMT) / 3600.0)

        // As applying the offset can push localT above 24 or below 0, we need to normalise
        localT = normalise(localT, withMaximum: 24)

        let hour = floor(localT)
        let minute = floor((localT - hour) * 60.0)
        let second = (((localT - hour) * 60) - minute) * 60.0

        let components = calendar.components([.Day, .Month, .Year], fromDate: date)
        components.hour = Int(hour)
        components.minute = Int(minute)
        components.second = Int(second)

        calendar.timeZone = timeZone
        return calendar.dateFromComponents(components)

To test this:

        if let solar = Solar(forDate: NSDate(), withTimeZone: NSTimeZone(name: "Australia/Melbourne")!, latitude: -38.2613, longitude: 145.1896) {
            print("UTC Sunrise: \(solar.sunrise!)")
            print("UTC Sunset: \(solar.sunset!)")
            let dateFormatter = NSDateFormatter()
            dateFormatter.timeZone = NSTimeZone(name: "Europe/London")!
            dateFormatter.dateFormat = "yyyy-MM-dd'T'HH:mm:ssZZZ"
            print("BST sunrise: \(dateFormatter.stringFromDate(solar.sunrise!))")
            print("BST sunset: \(dateFormatter.stringFromDate(solar.sunset!))")
        }

This gives me:

UTC Sunrise: 2016-07-08 21:35:11 +0000
UTC Sunset: 2016-07-09 07:13:57 +0000
BST sunrise: 2016-07-08T22:35:11+0100
BST sunset: 2016-07-09T08:13:57+0100

Which states that your sunrise happens at 22:35 London local time.

I also noticed this issue:
In the calculate function, we were calculating the day of the year using a UTC NSDate but applying the passed in timezone (line 125). Therefore, if we called NSDate() and applied the passed in AEST timezone, the calculated day of the year would be +10 hours and thus would actually return the next day. Therefore when trying to calculate the sunrise for the 9th July (day 191 of the year), it was returning day 192. That said, this wasn't affecting the return day.

Let me know if this makes sense or I've mucked something up somewhere!

Chris

[grumBit]

Hi Chris,

Apologies for mucking up. I thought I'd tested sunrise and sunset and it looked good, but apparently I only tested sunset! Re-testing my fix here shows the same problem you found :(

Having a quick look, your fix keeps the date correct for sunrise and sunset, however, daylight savings is not being applied correctly (in Melbourne, daylight savings goes from 3/10/16 to 2/4/17). I think this is because the input to calendar.dateFromComponents expects the wall-clock / daylight saving-ed time, rather than the "true" time that Solar has calculated. Looking at the NSDate returned, the value jumps by 1 hour over the daylight savings time start/finish, instead of just the small dawn/dusk change we should see.

I couldn't find a way to get NSCalendar to ignore daylight savings, however, I found a way to get the daylight savings time off set that has been applied from NSTimeZone and adding it back on. What do you think of replacing final line "return calendar.dateFromComponents(components)" with;

    if let returnDate = calendar.dateFromComponents(components) {
        let dstOffset = timeZone.daylightSavingTimeOffsetForDate(returnDate)
        return returnDate.dateByAddingTimeInterval(dstOffset)
    }
    return calendar.dateFromComponents(components)

In my quick testing for Melbourne, this seems to produce correct output for daylight savings.

Cheers, Graham.

[ceeK]

Hi Graham,

Indeed it doesn't seem to be handling timezones correctly! Although your code works, I was convinced something funny was happening in the code.

After combing through, I believe this line is incorrect:
var localT = UT + (Double(timeZone.secondsFromGMT) / 3600.0)

The documentation states that secondsFromGMT is the current difference between GMT and the timeZone offset. This must use today's date in the calculation.

I have fixed this by replacing that line with this one:
var localT = UT + (Double(timeZone.secondsFromGMTForDate(date)) / 3600.0)

This means localT is now offset correctly for the timeZone for the given date, which means the offset should include the daylight savings.

I'll do a bit more testing and then I'll push the fix.

Chris

[grumBit]

Hi Chris,

That's a much cleaner solution! Well done.

I was thinking about exploring test support in Xcode sometime soon and thought Solar might be good to try simple black box testing before trying GUI testing on apps. Would that be something useful to add to your Solar project on GitHub?

Off the top of my head, I was thinking of the following tests;

• accurate to within +/-configurable mins of timeanddate.com
• sunset / sunrise for all zeniths (on a day in London)
• same output date as forDate (all tests)
• forDate at start / end day (on a day in London)
• forDate at start / end year (on a day in London)
• forDate once a month for a year across a number of locations; e.g. London, Melbourne, Boston, Beijing
• forDate at start / end of location with continuous day / night (e.g. Murmansk) for all zeniths
• start/end daylight savings - Melbourne
• Non-standard timezone - e.g. South Australia (ACST) is GMT +9:30

Anyway, I'm assuming Xcode will have some JUnit like framework for capturing a number of tests and expected results for automated regression testing. If I put something together, maybe you could use it to quickly add further regression tests down the track if you need to. However, I'm not sure if an Xcode based test should be in GitHub. What do you think? Would this be useful and are there other tests you'd like to see?

Cheers, Graham.

[ceeK]

Hi Graham,

Great - I'll get this merged in this evening!

Tests are definitely something I'd like to see; there's been an empty tick box in the readme for quite a while!. I've made ticket #9 to capture this requirement so that we can discuss it further it there :)

Chris