Skip to content

fix: resolve DST bug in sunrise/sunset trigger timing (#3737) #3754

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.

Sign up for GitHub

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

Open
darinpope wants to merge 4 commits into
base: main
Choose a base branch
from
Open

fix: resolve DST bug in sunrise/sunset trigger timing (#3737) #3754

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
8df6e81
fix: resolve DST bug in sunrise/sunset trigger timing (#3737)
darinpope Nov 4, 2025
e0ce6e6
test: remove FAILING TEST prefixes now that DST bug is fixed
darinpope Nov 4, 2025
daefec1
Merge branch 'main' into fix/dst-sunrise-sunset-trigger
darinpope Dec 10, 2025
07662fb
Merge branch 'main' into fix/dst-sunrise-sunset-trigger
darinpope Dec 19, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
61 changes: 47 additions & 14 deletions companion/lib/Controls/ControlTypes/Triggers/Events/Timer.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -258,16 +258,22 @@ export class TriggersEventTimer {

// Modified function to calculate the sunrise/set time by adam-carter-fms
// https://gist.github.com/adam-carter-fms/a44a14c0a8cdacbbc38276f6d553e024#file-sunriseset-js-L12
//
// FIX for issue #3737 (DST sunset/sunrise timing bug):
// The original code used: `const diff = now - start + (start.getTimezoneOffset() - now.getTimezoneOffset()) * 60 * 1000`
// This caused sunset times to jump 60+ minutes across DST boundaries because getTimezoneOffset()
// changes during DST transitions. The fix: calculate day of year using UTC dates to eliminate
// timezone offset issues entirely. UTC never observes DST, so there's no offset change.
function getSunEvent(sunset: boolean, latitude: number, longitude: number, offset: number, nextDay: number): Date {
const res = new Date()
res.setDate(res.getDate() + nextDay)
const now = res

const start = new Date(now.getFullYear(), 0, 0)
// @ts-expect-error TS claims dates can't be subtracted, this should be revisited but I don't want to touch what works
const diff = now - start + (start.getTimezoneOffset() - now.getTimezoneOffset()) * 60 * 1000
const oneDay = 1000 * 60 * 60 * 24
const day = Math.floor(diff / oneDay)
// Calculate day of year using dayjs to avoid any DST/timezone issues
// dayjs handles calendar calculations correctly regardless of DST
const jan1 = dayjs(new Date(now.getFullYear(), 0, 1))
const today = dayjs(new Date(now.getFullYear(), now.getMonth(), now.getDate()))
Comment on lines +274 to +275

Copilot AI Nov 11, 2025

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There's a potential issue with the day-of-year calculation using dayjs. On lines 274-276, you're creating new Date objects with local timezone for both dates before passing them to dayjs. This means if DST occurs on January 1st or the current date (unlikely but theoretically possible in some locales), or if the system timezone differs from the user's intent, this could still introduce timezone-related issues.

Consider using dayjs with UTC mode consistently:

const jan1 = dayjs.utc(new Date(Date.UTC(now.getUTCFullYear(), 0, 1)))
const today = dayjs.utc(new Date(Date.UTC(now.getUTCFullYear(), now.getUTCMonth(), now.getUTCDate())))
const day = today.diff(jan1, 'day')

This would ensure the day-of-year calculation is purely in UTC, completely eliminating any DST or timezone concerns.

Suggested change
const jan1 = dayjs(new Date(now.getFullYear(), 0, 1))
const today = dayjs(new Date(now.getFullYear(), now.getMonth(), now.getDate()))
const jan1 = dayjs.utc(new Date(Date.UTC(now.getUTCFullYear(), 0, 1)))
const today = dayjs.utc(new Date(Date.UTC(now.getUTCFullYear(), now.getUTCMonth(), now.getUTCDate())))

Copilot uses AI. Check for mistakes.
const day = today.diff(jan1, 'day')

const zenith = 90.83333333333333
const D2R = Math.PI / 180
Expand Down Expand Up @@ -333,17 +339,22 @@ export class TriggersEventTimer {
UT = UT + 24
}

const ms = UT * 60 * 60 * 1000
// UT is the UTC time as decimal hours when sunset occurs
const utHours = Math.floor(UT)
const utMinutes = Math.floor((UT - utHours) * 60)

const sunEventTime = new Date(ms)
sunEventTime.setFullYear(now.getFullYear())
sunEventTime.setMonth(now.getMonth())
sunEventTime.setDate(now.getDate())
// Get the UTC date that corresponds to this local date
const localDateAtMidnight = new Date(now.getFullYear(), now.getMonth(), now.getDate())
const utcYear = localDateAtMidnight.getUTCFullYear()
const utcMonth = localDateAtMidnight.getUTCMonth()
const utcDate = localDateAtMidnight.getUTCDate()
Comment on lines +346 to +350

Copilot AI Nov 11, 2025

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The logic in lines 346-353 may produce incorrect results when the local date crosses a UTC date boundary. For example, if the local time is 11 PM on June 20th but UTC is already June 21st, localDateAtMidnight will be June 20th in local time, but when you extract UTC components from it, you'll get June 21st. This means the sun calculation will be done for June 21st when it should be for June 20th.

Consider using the UTC components directly from now instead:

const utcYear = now.getUTCFullYear()
const utcMonth = now.getUTCMonth()
const utcDate = now.getUTCDate()

This ensures the UTC date components match the date that was used for the day-of-year calculation on line 276.

Suggested change
// Get the UTC date that corresponds to this local date
const localDateAtMidnight = new Date(now.getFullYear(), now.getMonth(), now.getDate())
const utcYear = localDateAtMidnight.getUTCFullYear()
const utcMonth = localDateAtMidnight.getUTCMonth()
const utcDate = localDateAtMidnight.getUTCDate()
// Get the UTC date components directly from 'now'
const utcYear = now.getUTCFullYear()
const utcMonth = now.getUTCMonth()
const utcDate = now.getUTCDate()

Copilot uses AI. Check for mistakes.

const temp_minutes = sunEventTime.getMinutes()
// Create the date at the UTC time using Date.UTC
const sunEventTime = new Date(Date.UTC(utcYear, utcMonth, utcDate, utHours, utMinutes, 0))
Comment on lines +342 to +353

Copilot AI Nov 11, 2025

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[nitpick] There's a subtle issue with the seconds component in the UTC time calculation. Line 353 sets seconds to 0, but the original calculation produces a fractional number of minutes in UT. When converting UT to hours and minutes on lines 343-344, the fractional seconds are lost due to Math.floor().

For more precise timing, consider:

const utSeconds = Math.floor(((UT - utHours) * 60 - utMinutes) * 60)
const sunEventTime = new Date(Date.UTC(utcYear, utcMonth, utcDate, utHours, utMinutes, utSeconds))

This preserves the precision of the astronomical calculation rather than truncating to the nearest minute. While the difference is small (up to 59 seconds), for a sun event trigger this could matter for user expectations.

Copilot uses AI. Check for mistakes.

// Apply the offset in minutes using UTC to avoid DST issues
sunEventTime.setUTCMinutes(sunEventTime.getUTCMinutes() + offset)

// add offset to time
sunEventTime.setMinutes(temp_minutes + 60 + offset)
return sunEventTime
}
}
Expand Down Expand Up @@ -502,11 +513,23 @@ export class TriggersEventTimer {
setSun(id: string, params: Record<string, any>): void {
this.clearSun(id)

const nextExecute = this.#getNextSunExecuteTime(params)
this.#sunEvents.push({
id,
params,
nextExecute: this.#getNextSunExecuteTime(params),
nextExecute,
})

// Debug logging for sun event scheduling
const eventType = params.type === 'sunset' ? 'Sunset' : 'Sunrise'
const location = `${params.latitude}°, ${params.longitude}°`
const nextTimeDate = new Date(nextExecute)
const nextTimeLocal = nextTimeDate.toLocaleString()
const nextTimeUTC = nextTimeDate.toUTCString()
const offset = params.offset || 0
this.#logger.debug(
Comment thread
Julusian marked this conversation as resolved.
`${eventType} trigger '${id}' saved (${location}, offset: ${offset}min) - Next scheduled: ${nextTimeLocal} (UTC: ${nextTimeUTC})`
)
}

/**
Expand All @@ -515,4 +538,14 @@ export class TriggersEventTimer {
clearSun(id: string): void {
this.#sunEvents = this.#sunEvents.filter((sun) => sun.id !== id)
}

/**
* Get the next execute time for a sun event (for testing purposes)
* @param id Id of the event
* @returns Unix timestamp in milliseconds, or null if event not found
*/
getSunNextExecuteTime(id: string): number | null {
const sunEvent = this.#sunEvents.find((sun) => sun.id === id)
return sunEvent?.nextExecute ?? null
}
}
Loading