Skip to content

Commit 6571d66

Browse files
authored
Fix UTC daylight-savings-time offset issue. - authored by @cboulay
294 fix utc dst Merge pull request #295 from cboulay/294-fix-utc-dst authored by @cboulay
2 parents fa10b17 + ad686e0 commit 6571d66

4 files changed

Lines changed: 95 additions & 2 deletions

File tree

CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
1919

2020
### Fixed
2121
* Updated nwbinspector validation tests in the CI to: 1) `--ignore=check_subject_exists` and 2) remove dependency on `sanitizer` tests to speed up CI (@oruebel, [#289](https://github.com/NeurodataWithoutBorders/aqnwb/pull/289))
22+
* Fixed `get_utc_offset_seconds` to correctly account for daylight saving time using platform-specific APIs (`tm_gmtoff` on Unix/macOS; `_get_timezone` + `_get_dstbias` on Windows), preventing `session_start_time` from being written ~1 hour ahead of UTC during DST (@cboulay, [#295](https://github.com/NeurodataWithoutBorders/aqnwb/pull/295))
2223

2324

2425
## [0.3.0] - 2026-02-23

src/Utils.hpp

Lines changed: 19 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -64,13 +64,29 @@ inline std::tm to_utc_time(std::time_t time_value)
6464
/**
6565
* @brief Get the UTC offset in seconds for a given time_t value.
6666
* @param time_value The time value to check.
67-
* @return The offset from UTC in seconds.
67+
* @return The offset from UTC in seconds (includes DST when active).
6868
*/
6969
inline long get_utc_offset_seconds(std::time_t time_value)
7070
{
71+
#if defined(__unix__) || defined(__APPLE__)
72+
std::tm local_tm = to_local_time(time_value);
73+
return local_tm.tm_gmtoff;
74+
#elif defined(_WIN32)
75+
long tz_seconds = 0;
76+
_get_timezone(&tz_seconds);
77+
std::tm local_tm = to_local_time(time_value);
78+
if (local_tm.tm_isdst > 0) {
79+
long dstbias = 0;
80+
_get_dstbias(&dstbias);
81+
tz_seconds += dstbias;
82+
}
83+
return -tz_seconds;
84+
#else
85+
// Fallback: force utc_tm.tm_isdst to match local so mktime treats both
86+
// consistently, avoiding the DST double-count.
7187
std::tm local_tm = to_local_time(time_value);
7288
std::tm utc_tm = to_utc_time(time_value);
73-
89+
utc_tm.tm_isdst = local_tm.tm_isdst;
7490
std::time_t local_time = std::mktime(&local_tm);
7591
std::time_t utc_time = std::mktime(&utc_tm);
7692
if (local_time == static_cast<std::time_t>(-1)
@@ -79,6 +95,7 @@ inline long get_utc_offset_seconds(std::time_t time_value)
7995
return 0;
8096
}
8197
return static_cast<long>(std::difftime(local_time, utc_time));
98+
#endif
8299
}
83100

84101
/**

tests/testNWBFile.cpp

Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
#include <cstdio>
12
#include <numeric>
23
#include <unordered_map>
34
#include <unordered_set>
@@ -504,6 +505,52 @@ TEST_CASE("testAttributeAndDatasetFields", "[nwb]")
504505
std::string readSessionStartTime = sessionStartTimeData->values().data[0];
505506
REQUIRE(readSessionStartTime == sessionStartTime);
506507

508+
// Mirrors nwbinspector's check_session_start_time_future_date: the
509+
// session_start_time written to the file must not be in the future.
510+
// The old mktime-based UTC offset was wrong during DST, which caused
511+
// the recorded time to be ~1 hour ahead of actual UTC.
512+
{
513+
std::tm parsed {};
514+
int offset_h = 0, offset_m = 0;
515+
char offset_sign = '+';
516+
// Parse "YYYY-MM-DDTHH:MM:SS.uuuuuu+HH:MM"
517+
#if defined(_MSC_VER)
518+
# pragma warning(push)
519+
# pragma warning(disable : 4996) // sscanf deprecation
520+
#endif
521+
int parsed_fields = std::sscanf(readSessionStartTime.c_str(),
522+
"%4d-%2d-%2dT%2d:%2d:%2d.%*d%c%2d:%2d",
523+
&parsed.tm_year,
524+
&parsed.tm_mon,
525+
&parsed.tm_mday,
526+
&parsed.tm_hour,
527+
&parsed.tm_min,
528+
&parsed.tm_sec,
529+
&offset_sign,
530+
&offset_h,
531+
&offset_m);
532+
#if defined(_MSC_VER)
533+
# pragma warning(pop)
534+
#endif
535+
REQUIRE(parsed_fields == 9);
536+
parsed.tm_year -= 1900;
537+
parsed.tm_mon -= 1;
538+
parsed.tm_isdst = 0;
539+
// Convert struct tm (interpreted as UTC) to time_t
540+
#if defined(_WIN32)
541+
std::time_t utc_epoch = _mkgmtime(&parsed);
542+
#else
543+
std::time_t utc_epoch = timegm(&parsed);
544+
#endif
545+
long offset_total =
546+
((long)offset_h * 3600 + offset_m * 60) * (offset_sign == '-' ? -1 : 1);
547+
// Convert local time to UTC by subtracting the offset
548+
utc_epoch -= offset_total;
549+
auto written_tp = std::chrono::system_clock::from_time_t(utc_epoch);
550+
auto now_tp = std::chrono::system_clock::now();
551+
REQUIRE(written_tp <= now_tp);
552+
}
553+
507554
auto timestampsReferenceTimeData = nwbfile->readTimestampsReferenceTime();
508555
REQUIRE(timestampsReferenceTimeData->exists());
509556
REQUIRE(timestampsReferenceTimeData->getStorageObjectType()

tests/testUtilsFunctions.cpp

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -156,6 +156,34 @@ TEST_CASE("Test time conversion functions", "[utils]")
156156
REQUIRE(offset <= 14 * 3600);
157157
}
158158

159+
SECTION("get_utc_offset_seconds matches tm_gmtoff")
160+
{
161+
// Verify the offset matches the OS-reported value (which includes DST).
162+
// The old mktime-based implementation got this wrong during DST because
163+
// mktime interprets its argument as local time, double-counting DST.
164+
#if defined(__unix__) || defined(__APPLE__)
165+
// tm_gmtoff is a non-standard extension available on Unix/macOS
166+
std::tm local_tm {};
167+
localtime_r(&now, &local_tm);
168+
long expected = local_tm.tm_gmtoff;
169+
long actual = AQNWB::detail::get_utc_offset_seconds(now);
170+
REQUIRE(actual == expected);
171+
#elif defined(_WIN32)
172+
// On Windows, verify against _get_timezone + _get_dstbias
173+
long tz_seconds = 0;
174+
_get_timezone(&tz_seconds);
175+
std::tm local_tm = AQNWB::detail::to_local_time(now);
176+
if (local_tm.tm_isdst > 0) {
177+
long dstbias = 0;
178+
_get_dstbias(&dstbias);
179+
tz_seconds += dstbias;
180+
}
181+
long expected = -tz_seconds;
182+
long actual = AQNWB::detail::get_utc_offset_seconds(now);
183+
REQUIRE(actual == expected);
184+
#endif
185+
}
186+
159187
SECTION("format_utc_offset")
160188
{
161189
REQUIRE(AQNWB::detail::format_utc_offset(0) == "+00:00");

0 commit comments

Comments
 (0)