search

home-assistant / core

:house_with_garden: Open source home automation that puts local control and privacy first.
https://www.home-assistant.io
Apache License 2.0
73.71k stars 30.84k forks source link

CalDav: Repeat events do not show in all views #40127

Closed bverkron closed 1 year ago

bverkron commented 4 years ago

The problem

Repeat calendar events do not show beyond the first entry in Monthly and Weekly views. They do, however, render as expected in Daily view. Seems to happen with CalDav but not Google Calendar entries. Might be specific to iCloud calendars, not sure, don't have another CalDav compatible calendar to test with.

Environment

Problem-relevant configuration.yaml

calendar:
  - platform: caldav
    username: !secret icloud_username
    password: !secret icloud_app_password
    url: https://caldav.icloud.com
    calendars:
      - Family
      - Errands

Traceback/Error logs

N/A

Additional information

In the monthly and weekly view you can see only a single event (the first one). But moving forward to the 17th (or any day beyond the first one) you can see the repeat events.

monthly weekly daily
tobixen commented 2 years ago

@tobixen For servers that do not expand recurring events: do you get back RRULE's from them?

Sure. The problem is with date search - say the event is a daily one, with the first recurrence in January, the non-compliant servers would only return the event when doing a date search for January, while the compliant server would return the event when doing a search for today. I haven't looked into the home assistant calendar code, though I was under the impression that it does a date search.

If so we could explore sharing this logic https://github.com/allenporter/gcal_sync/blob/c1a511f6e59c5bdd79bafdc4c78dcc6b5d4b5a95/gcal_sync/timeline.py#L112 which i'm now using in ical and gcal_sync to locally expand recurring events in preparation for also supporting mutations. It's a lot more complex, but we could at least share the same bugs :)

Calculating recurrences and doing the actual expanding is far outside the scope of the caldav library, so we use third party libraries for that. @niccokunzmann is maintaining a recurring-ical-events library at https://github.com/niccokunzmann/python-recurring-ical-events which @daniele-athome brought forward for doing the expanding. There is also the dateeutil.rrule library that can be used for calculating the recurrences. As for now the caldav library depends on both of them.

allenporter commented 2 years ago

Ok cool. gcal and ical libraries are using dateutil.rrule plus logic on top for making a stack of iterators to merge the timeline. (The recurring events library looks to have similar apis as ical, so looks like we have a roughly similar solution)

Home assistant does date search yes. The integration has to know the full set of possible recurring events that could be repeated, so in the case of gcal it syncs them down from the server then serves locally.

I'm missing something: Why does caldav python have to do anything if the server is expanding? I was assuming this was adding support for non server side expanded events

allenporter commented 2 years ago

Maybe you are saying it does do local expansion but also there are still problematic servers that don't give you back all the events you need.

tobixen commented 2 years ago

Some servers do support rrules but do not support (server side) expanding. They will deliver the original icalendar component rather than the recurrence set. The fix which has been merged to master auto-detects missing expansion and will do client side expansion for those servers.

Now that we have client-side expanding in place it may make sense to stop asking the server to expand and rather do it client-side. Would save some bandwidth, and would ensure consistent result in case the server side expansion is buggy. Anyway, that's for a future release.

As for servers not supporting rrule, as far as I can tell, the only solution is to download all the calendar - or at least all prior events - and check if any of them overlap the date search window. I think that's outside the scope of the caldav library.

daniele-athome commented 2 years ago

It should work in principle, but:

[...]
File "/home/homeassistant/hass/lib/python3.9/site-packages/homeassistant/components/http/view.py", line 136, in >
     result = await result
   File "/home/homeassistant/hass/lib/python3.9/site-packages/homeassistant/components/calendar/__init__.py", line >
     [
   File "/home/homeassistant/hass/lib/python3.9/site-packages/homeassistant/components/calendar/__init__.py", line >
     dataclasses.asdict(event, dict_factory=_api_event_dict_factory)
[...]
TypeError: cannot pickle '_thread.lock' object

Similar to #71048: it's caused by a particular time zone information in datetimes:

datetime.datetime(2022, 11, 20, 17, 55, tzinfo=<tzicalvtz 'Europe/Rome'>

I found the unpickable object inside that "tzicalvtz". It'll probably need to be converted to a more pickle-friendly class... although it seems like a workaround. I'll continue later.

EDIT: happening on Python 3.9.2.
EDIT: opened dateutil/dateutil#1242 while looking for a workaround of some kind EDIT: I'm trying to address the above error too... see dateutil/dateutil#1243

daniele-athome commented 1 year ago

So I'm waiting for feedback on dateutil/dateutil#1243, but the modifications have been working nicely for a few days now on my instance. If you want to try it in your Python (virtual) environment:

Please consider this beta stuff though - YMMV.

allenporter commented 1 year ago

Most PRs in dateutil seem to be not get much traction... so wondering what is the reason for that change? where is the rrule being serialized/pickled? Wondering if that can be avoided.

daniele-athome commented 1 year ago

I think it has something to do with HTTP API serialization:

return self.json(
    [
        dataclasses.asdict(event, dict_factory=_api_event_dict_factory)
        for event in calendar_event_list
    ]
)

rrulebase (or most likely a child class) objects are inside the _tzicalvtz object.

allenporter commented 1 year ago

@daniele-athome ok I think what you should do is not return the rrule as a field of the CalendarEvent and instead only return fields set in the dataclass.

daniele-athome commented 1 year ago

Easier said than done I'm afraid: the rrulebase object is in a collection that is a private member of a _tzicalvtz object which is the tzinfo member of a datetime inside the calendar event. By the time I call dataclasses.asdict it's already too late (the error comes from there). By the way, at first I tried to modify the tzinfos to some other class but it didn't work really well. Also it seems like an even worse workaround.

allenporter commented 1 year ago

@daniele-athome OK, perhaps that can be avoided. I am creating recurring events using datetime.rrule underneath and not running into this problem. I don't think that the timezone object needs to reference a recurrence rule.

daniele-athome commented 1 year ago

I don't think that the timezone object needs to reference a recurrence rule.

I would have the same opinion in principle, but I'm afraid it probably has something to do with the way events are created by the library we use. I'll investigate more on the matter.

allenporter commented 1 year ago

@daniele-athome I tested out your patch and it worked fine with caldav for nextcloud without any special patches, and fixed recurring events. Is there a particular way you have to reproduce the issues you are having?

ACiDGRiM commented 1 year ago

thanks for the continued word. ive moved more critical alerts to the built in calendar.

however i miss having one place to manage my calendars, it would be nice if this cached events until the next sucessful update since losing internet seemed to drop all calendar events until the next check.

lookingforward to the pr merging

daniele-athome commented 1 year ago

@daniele-athome I tested out your patch and it worked fine with caldav for nextcloud without any special patches, and fixed recurring events. Is there a particular way you have to reproduce the issues you are having?

It might have something to do with the server you're using. According to the original issue Nextcloud is supposed to expand recurring events server-side - so client-side nothing is done. Anyway I used SOGo.

By the way, #84955 was just merged: AFAIU that fixed the problem altogether, so we don't need to patch dateutil. I'll do some more tests because I noticed some strange behaviors so I need to check again.

allenporter commented 1 year ago

I realized it was because the response needed to contain timezone information https://github.com/home-assistant/core/issues/81624#issuecomment-1367224721

Anyway, i tested your patch with nextcloud and it definitely works for recurring events. I encourage you to go forward with your bump of the caldav library!

daniele-athome commented 1 year ago

Everything is ok! The PR is there. I should have done everything correctly. Thanks everyone for the feedbacks!

EDIT: and tests didn't pass! I'll need to investigate, if anyone has a clue of what is happening though it will definitely speed up the process. Thanks. EDIT/2: tests are fixed, PR is under review.

bkr1969 commented 1 year ago

Weird behavior. My Apple calendar recurring event shows up for the first instance, but not further on any given month, yet, if I move to the next month , the last instance of it in the prior month shows up (i.e. the recurring even is on every Tuesday and a new month starts on Wednesday. The "greyed out" last Tuesday of the prior month shows the event, even though non of them show for the current month).

SlothCroissant commented 1 year ago

Just as an FYI, while at first glance this doesn't appear to be working, I can confirm the entity itself is working as expected:

Home Assistant 2023.1.7
Supervisor 2023.01.1
Operating System 9.5
Frontend 20230110.0 - latest

calendars YAML:

calendar:
  - platform: caldav
    url: https://caldav.icloud.com
    username: **********
    password: **********
    days: 30
    custom_calendars:
      - name: "on_peak"
        calendar: "Time of Use Schedule"
        search: "Peak"

If you see my iCloud calendar, you can see the recurring meeting (I use this to signify when my Electricity is in peak vs off-peak billing):

Screenshot 2023-01-30 at 22 10 59

However, Home Assistant appears to be wonky, only showing the first day in the month (I started this daily repeat on the 2nd):

Screenshot 2023-01-30 at 22 10 21

Yet however again - Home Assistant's calendar.peak entity I have tied to this calendar is working like a charm:

Screenshot 2023-01-30 at 22 08 45

So in summary, check to see if the underlying bits are working - it could be just the calendar UI that is misbehaving.

tobixen commented 1 year ago

This is likely to be fixed in the February release, if I'm not mistaken

daniele-athome commented 1 year ago

This is likely to be fixed in the February release, if I'm not mistaken

Yes, it's already in the beta.

bkr1969 commented 1 year ago

Just updated to 2023.2 and it's working perfectly for me.

tobixen commented 1 year ago

Please correct me if I'm wrong on this ... but I believe the fix in 2023.2 should work for those calendar servers:

... but I suspect there may still be problems with recurring events with those calendar servers:

Those calendar servers are supposed to be fully compliant to recurring events and should work both with 2023.1 and 2023.2:

I'm also working on some document with recommendations for calendar servers at https://github.com/tobixen/plann/blob/master/CALENDAR_SERVER_RECOMMENDATIONS.md :-)

SlothCroissant commented 1 year ago

Confirmed, iCloud is still an issue. I'm running into this now as well, not sure if related, but just FYI

86938

dsmanning commented 8 months ago

I've noticed something interesting on iCloud. If I create a repeated event with many events (e.g. every day from February to December) then it will NOT show in Home Assistant. If I reduce the number of repeated events (e.g. stop it on May 31st and start a duplicate repeated event on June 1st) then it WILL show up. Perhaps there is a limit on the number of repeated events? Hopefully this is useful for a maintainer!

allenporter commented 8 months ago

@dsmanning Hello, if you'd like to report an issue, please file a new issue and fill out the entire info in the bug template. Thank you