Fix alarms firing in server timezone instead of player timezone#1514
Fix alarms firing in server timezone instead of player timezone#1514boudekerk wants to merge 4 commits intoLMS-Community:public/9.2from
Conversation
When a player in a different timezone to the server sets an alarm, the alarm would fire at the correct time according to the server's local clock rather than the player's. For example, a player in America/New_York connected to a server in Europe/Amsterdam would have a 7:00 AM alarm fire at 1:00 AM local time. The fix stores an optional Olson timezone name (_timezone) with each alarm. When present, findNextTime() uses that timezone for scheduling instead of the server's localtime(). Backward compatibility is preserved: alarms without a timezone continue to use server local time unchanged. On the server side, the Jive alarm menu now includes the player's registered timezone in the alarm add/update action params, so it is sent automatically when the player creates or modifies an alarm via the touch UI. Signed-off-by: Bartosz Oudekerk <lot+github@unreachablehost.net>
The rescan plugin's checkScanTimer() used localtime() (server timezone) to interpret the scheduled time, causing misfires for players in different timezones. - Add rescanplugin settimer command accepting time + timezone params - Jive menu now passes the player's timezone (from playerpref) when scheduling a rescan time via the device UI - checkScanTimer() overrides TZ/tzset() when a timezone is stored, falling back to server localtime() for existing schedules (backwards compatible) - Web settings UI captures browser timezone via Intl.DateTimeFormat and stores it alongside the scheduled time Signed-off-by: Bartosz Oudekerk <lot+github@unreachablehost.net>
…mezone When a user sets an alarm time via the web UI, the entered time should be interpreted in the browser's timezone (where the user and player are), not the server's timezone. Capture the browser timezone via Intl.DateTimeFormat and store it on the alarm object, consistent with the approach used for scheduled rescan. Signed-off-by: Bartosz Oudekerk <lot+github@unreachablehost.net>
86ae5db to
339b1db
Compare
|
I'm sorry, that's not going to happen. We're not adding 400+ files to solve one person's issue - which likely could be solved using an environment variable. |
|
@michaelherger It's still work in progress. But thanks for keeping an eye on it. All comments are appreciated. |
… scheduling
POSIX::tzset with $ENV{TZ} does not work on Windows, where Perl requires
Windows-native timezone names rather than Olson identifiers. This made
alarm scheduling in a player's timezone silently fall back to the server
timezone on Windows.
Replace with DateTime::TimeZone, which uses a bundled IANA timezone
database and works identically on all platforms.
- Add Slim::Utils::DateTime::localtimeInTZ($epoch, $tzName), a thin
wrapper around DateTime::TimeZone that returns a localtime()-style
list in the named Olson timezone, with a per-process cache and a
graceful fallback to server timezone for unrecognised names
- Use localtimeInTZ in Slim::Utils::Alarm::findNextTime and
Slim::Plugin::Rescan::Plugin::checkScanTimer, replacing the
POSIX::tzset save/restore blocks
- Bundle DateTime::TimeZone 2.66 (IANA tzdata 2025c) and its
dependencies in CPAN/
Signed-off-by: Bartosz Oudekerk <lot+github@unreachablehost.net>
ec0d329 to
bf3cbd8
Compare
|
@michaelherger @mw9 @ralph-irving Can I please pick your brain here? @michaelherger commented on was trying out using DateTime::TimeZone. That will result in the cleanest code IMO. And, crucially, the Strawberry perl you're using for the Windows installer already includes this dependency anyway. Including it explicitly in lms, it indeed pulls in 400+ files, although ~300 of those simply contain the timezone data. I see the following options available (let me know if you see others):
1b. is my least favourite as it will increase the maintenance burden for little value. Of the ~400 files the DateTime::TimZone dependency introduces ~300 are simply the timezone data. So trimming it down will be marginal anyway and make dependency updates harder. I'd say either we include it as a dependency or we don't, but let me know if you disagree. My preference is actually 1a. Clean code and a clear dependency. 400 files seems like a lot, but those are included on Windows anyway and 300 of those are simply small timezone data files. Easy to update as well. My second choice would be 1c. Please let me know which approach you would prefer and I can make the required changes and put it forward for review. Thanks. P.S. @michaelherger Your suggestion of an env variable, won't actually work on Windows AFAIK. I'm not sure what workaround for this bug there is available there today. |
|
Another option is to have 1c with a fallback to 2. |
I wanted to suggest this: use if (main::ISWINDOWS) {
...
} |
|
@mw9 @ralph-irving Any comments? If not I'll start work next week on the multi-tiered approach as discussed with @michaelherger. |
No I don't really have anything to offer on these changes. I've never used the alarm features of the firmware. |
When a player in a different timezone to the server sets an alarm, the alarm would fire at the correct time according to the server's local clock rather than the player's. For example, a player in America/New_York connected to a server in Europe/Amsterdam would have a 7:00 AM alarm fire at 1:00 AM local time.
The fix stores an optional Olson timezone name (_timezone) with each alarm. When present, findNextTime() uses that timezone for scheduling instead of the server's localtime(). Backward compatibility is preserved: alarms without a timezone continue to use server local time unchanged.
On the server side, the Jive alarm menu now includes the player's registered timezone in the alarm add/update action params, so it is sent automatically when the player creates or modifies an alarm via the touch UI.