sdetweil
commented
5 months ago yes the rrule processor has trouble with some date combinations.
not our code. and we can't tell as we don't process the ics file in MagicMirror code
Open morozgrafix opened 5 months ago
sdetweil
commented
5 months ago yes the rrule processor has trouble with some date combinations.
not our code. and we can't tell as we don't process the ics file in MagicMirror code
morozgrafix
commented
5 months ago I did a little bit more digging and the extra day is added in calendarfetcherutils.js around line 300:
// RRule can generate dates with an incorrect recurrence date. Process the array here and apply date correction.
if (hasByWeekdayRule) {
Log.debug("Rule has byweekday, checking for correction");
dates.forEach((date, index, arr) => {
// NOTE: getTimezoneOffset() is negative of the expected value. For America/Los_Angeles under DST (GMT-7),
// this value is +420. For Australia/Sydney under DST (GMT+11), this value is -660.
const tzOffset = date.getTimezoneOffset() / 60;
const hour = date.getHours();
if ((tzOffset < 0) && (hour < -tzOffset)) { // east of GMT
Log.debug(`East of GMT (tzOffset: ${tzOffset}) and hour=${hour} < ${-tzOffset}, Subtracting 1 day from ${date}`);
arr[index] = new Date(date.valueOf() - oneDayInMs);
} else if ((tzOffset > 0) && (hour >= (24 - tzOffset))) { // west of GMT
Log.debug(`West of GMT (tzOffset: ${tzOffset}) and hour=${hour} >= 24-${tzOffset}, Adding 1 day to ${date}`);
arr[index] = new Date(date.valueOf() + oneDayInMs);
}
});
if I’m not mistaken the time zone is set to null just before that code is executed. I think if calendar/event is matching the local time zone the adjustment shouldn’t be made. I tried to comment out the code above and it displays the date correctly, but of course it breaks some of the existing tests. I think that it needs additional checks before execution. Maybe checking that event timezone doesn’t match local timezone (although I didn’t dig into the code much yet)
thanks
sdetweil
commented
5 months ago that's my code trying to deal with this problem. no good fix yet
github-actions[bot]
commented
1 month ago This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.
morozgrafix
commented
1 month ago Bump, too keep it open as it’s still an issue. Hopefully @sdetweil has some time to look at it and wontfix tag is just labeled by a bot.
Pollard-J
commented
1 month ago yea ive just updated mine and it is still displaying full day events as 2 days.
sdetweil
commented
1 month ago see this.. waiting on user test for submitting PR here and node-ical all testcases in both run successfully..
here is a test version of the fixes for all kinds of date problems.
best to make a new folder and git clone there
git clone https://github.com/sdetweil/MagicMirror
cd ~/MagicMirror
git checkout fixcaldates
npm run install-mmcopy your config.js and custom.css from the prior folder and the non-default modules you have installed..
this ONLY changes the default calendar but DOES ship an updated node-ical library too
if you need to fall back, just rename the folders around again so that your original is called MagicMirror
all the testcases for node-ical and MagicMirror execute successfully.
the 'BIG' change here is to get the local NON-TZ dates for the rrule.between()
all the checking and conversion code is commented out or not used the node-ical fixes are for excluded dates (exdate) values being adjusted for DST/STD time.. waiting to submit that PR
one fix in calendar.js for checking if a past date was too far back, but it never checked to see IF the event date was in the past.. (before today) so it chopped off too many
and one change in calendarfetcher.js to put out a better diagnostic message of the parsed data.. (exdate was excluded cause JSON stringify couldn't convert the complex structure)
sdetweil
commented
1 month ago @Pollard-J can u try my new fork?
and post a calendar event I can test with.. thx BEGIN:VEVENT ... END:VEVENT
Pollard-J
commented
1 month ago @sdetweil i just tried this but i'm not that good when i come into problems. I cloned your file of MagicMirror, but it wouldn't execute the next 2 lines properly. git checkout fixcaldates npm run install-mm
ill just wait till its released. thank you for trying to help me.
sdetweil
commented
1 month ago sorry i left out a critical step after the git clone
cd ~/MagicMirror fixed above too
morozgrafix
commented
1 month ago @sdetweil thanks for the fix. I just tested the issue and fixcaldates branch is displaying events correctly. I only tested with config.js from original description of this bug. Repeated events should occur every Thursday at 17:30. Below are screenshots from master (not fixed) and fixcaldates (fixed) branches. Thank you for digging into this issue, hopefully other issues can also be tested on that branch and your PR goes through soon.
Before:
After:
sdetweil
commented
1 month ago cool, thx for the feedback.
we wouldn't release til january 1. so keep using and report anything else you see i need to fix
sdetweil
commented
1 month ago fixed a bunch more testcases.. updated tests
both mm and node-ical changed
cd MagicMirror
git pull
rm -rf node_modules
npm run install-mmtry again, note ne branch name
here is an updated test version of the fixes for all kinds of calendar date problems.
NOTE: the changed branch name NOTE: this used the node-cal@0.19.0 library UNCHANGED
best to make a new folder and git clone there
git clone https://github.com/sdetweil/MagicMirror cd MagicMirror git checkout fixcaldates2 // <------ note this is a changed branch name npm run install-mm copy your config.js and custom.css from the prior folder and the non-default modules you have installed…
this ONLY changes the default calendar but DOES ship an updated node-ical library too
if you need to fall back, just rename the folders around again so that your original is called MagicMirror
all the testcases for node-ical and MagicMirror execute successfully.
the ‘BIG’ change here is to get the local NON-TZ dates for the rrule.between()
all the checking and conversion code is commented out or not used the node-ical fixes are for excluded dates (exdate) values being adjusted for DST/STD time… waiting to submit that PR
one fix in calendar.js for checking if a past date was too far back, but it never checked to see IF the event date was in the past… (before today) so it chopped off too many
and one change in calendarfetcher.js to put out a better diagnostic message of the parsed data… (exdate was excluded cause JSON stringify couldn’t convert the complex structure)
I added the tests you all have documented
please re-pull and checkout the new branch (I deleted the old branch) and npm run install-mm again
I found a bug in MagicMirror
Please make sure to only submit reproducible issues. You can safely remove everything above the dividing line. When submitting a new issue, please supply the following information:
Platform: Raspberry Pi 2 - Raspbian GNU/Linux 12 (bookworm). Also reproduced and debugged on OS X - Sonoma 14.5 with Chrome 125.0.6422.142
Node Version: v20.14.0
MagicMirror² Version: 2.27.0
Description: Repeating weekly calendar event is offest by 1 day when processed by Magic Mirror. For example when event is scheduled on Thursday in the calendar, it appears to be scheduled for Friday when displayed on MM. I ran through the
debug.jsand it seems that RRules are affecting it. Full logs and steps to reproduce are below. Possibly related to #3342 and #3291 fixes. @jkriegshauser Not sure if this is an edge case that wasn't tested. My MM installations are set toAmerica/Los_Angelestimezone. The calendar is set in the same timezone, event is set in the same timezone.modules/default/calendar/calendarfetcherutils.jsaround line 300 seems to be adding an extra day.Steps to Reproduce: 1 . Add following ics file to the calendar config or as
urlvariable indebug.jsGist for basic.icsTest Entriesto show up on Fridays at 17:30 or runmodules/default/calendar/debug.jsand observe the output.Expected Results: Repeating calendar events to be displayed to occur on Thursday at 17:30
Actual Results: Events are displayed to occur on Fridays at 17:30
Configuration: Plain vanilla
config.jsbased onconfig.js.samplewith only addition of the sample calendar:Additional Notes: Debug Output is a bit too long, here is a link to a gist of the debug output: Full Debug Output Small relevant part
Calendar file (copy of the gist linked above)