Add day to event end to correct TwenteMilieu event timespan

[bobvandevijver]

Proposed change

Update the TwenteMilieu integration event end date to be one day later, as indicated in #86602.

Type of change

• Dependency upgrade
• Bugfix (non-breaking change which fixes an issue)
• New integration (thank you!)
• New feature (which adds functionality to an existing integration)
• Deprecation (breaking change to happen in the future)
• Breaking change (fix/feature causing existing functionality to break)
• Code quality improvements to existing code or addition of tests

Additional information

• This PR fixes or closes issue: fixes Calendar automation not being triggered #86602
• This PR is related to issue:
• Link to documentation pull request:

Checklist

• The code change is tested and works locally.
• Local tests pass. Your PR cannot be merged unless tests pass
• There is no commented out code in this PR.
• I have followed the development checklist
• The code has been formatted using Black (black --fast homeassistant tests)
• Tests have been added to verify that the new code works.

If user exposed functionality or configuration variables are added/changed:

• Documentation added/updated for www.home-assistant.io

If the code communicates with devices, web services, or third-party tools:

• The manifest file has all fields filled out correctly.
  Updated and included derived files by running: python3 -m script.hassfest.
• New or updated dependencies have been added to requirements_all.txt.
  Updated by running python3 -m script.gen_requirements_all.
• For the updated dependencies - a link to the changelog, or at minimum a diff between library versions is added to the PR description.
• Untested files have been added to .coveragerc.

To help with the load of incoming pull requests:

• I have reviewed two other open pull requests in this repository.

[home-assistant]

Hi @bobvandevijver

It seems you haven't yet signed a CLA. Please do so here.

Once you do that we will be able to review and accept this pull request.

Thanks!

[home-assistant]

Hey there @frenck, mind taking a look at this pull request as it has been labeled with an integration (twentemilieu) you are listed as a code owner for? Thanks!

Code owner commands

Code owners of twentemilieu can trigger bot actions by commenting:

• @home-assistant close Closes the pull request.
• @home-assistant rename Awesome new title Renames the pull request.
• @home-assistant reopen Reopen the pull request.
• @home-assistant unassign twentemilieu Removes the current integration label and assignees on the pull request, add the integration domain after the command.

[allenporter]

IMO I wouldn't mark this as a breaking change (seems quite unlikely someone is looking at the end date of these) and its just a bug fix to match the spec.

There is a merge conflict so address that and this seems fine to me.

[home-assistant]

Please take a look at the requested changes, and use the Ready for review button when you are done, thanks 👍

Learn more about our pull request process.

[bobvandevijver]

MR description has been updated to no longer mark it as breaking change, and the branch has been rebased (although there wasn't any merge conflict).

[frenck]

Wow @allenporter!

Hold you horses, this required code owner approval!

As this integration reached the platinum scale. You where not allowed to merge this.

[allenporter]

Wow @allenporter!

Hold you horses, this required code owner approval!

As this integration reached the platinum scale. You where not allowed to merge this.

Apologies, I misunderstood. My platinum integrations have changed merges on them fairly regularly without my approval based on the judgement of the merger. This integrations implementation of calendar platform is not correct according to the spec in a fairly trivial way.( Intent was to help burn down the PR backlog). Shall I revert?

[frenck]

"Sorry officer he ignored the red light too" 😉

My platinum integrations have changed merges on them fairly regularly without my approval based on the judgement of the merger

Admin do override in case it has time pressure and affects lot of people. Global refactors for not trigger code owner requirements. Lastly, in case of running stale by lack of code owner response.

Otherwise, we should await.

Shall I revert?

Will take a look later, mobile right now.

[allenporter]

"Sorry officer he ignored the red light too" 😉

Consider it more like trying to be helpful burning down the review backlog, not making excuses.

Your message is heard loud and clear.

[frenck]

Consider it more like trying to be helpful burning down

Well appreciated, thanks 🙏

[frenck]

Going to revert this, it is not a correct fix.

We have different issues somewhere, and this is working around the issue. I don't think adding a workaround should be our base to work from. Screenshots from the current (previous to this PR situation)

It is correctly being picked up as an all-day event (for which the behavior was changed in #68843). However, it is making up the end date.

Looking at the RFC:

For cases where a "VEVENT" calendar component
specifies a "DTSTART" property with a DATE value type but no
"DTEND" nor "DURATION" property, the event's duration is taken to
be one day.

Ref: https://www.rfc-editor.org/rfc/rfc5545#section-3.6.1

This means that the specs aren't currently handled correctly in our calendar entity platform, as it should have been a day automatically already in the existing (previous) situation. I suggest we allow creating events without an end date to handle this more correctly toward the RFC we are trying to implement.

That should be fixed and not worked around in the integration.

../Frenck

[frenck]

Second thought 🤔 Canceled the revert, as the optional end date is missing right now. As the end-date is exclusive, this PR makes sense in a way considering the current capabilities and kinda falls into the ruleset.

Yet, we should look in accepting events without end-date I guess.

[allenporter]

We say here that the end date is exclusive:

https://developers.home-assistant.io/docs/core/entity/calendar#calendarevent

which is based on the rfc. (We don't define a duration, but could instead of we want to change the entity spec)

I would like more feedback on:
home-assistant/architecture#853 -- I am proposing to add more validation to the calendar event dataclass as a way to enforce the spec. (In that case for timezones, though my intent was to enforce for other rules in the spec slso). Waiting for a second ACK on that discussion.

[frenck]

I'm not following the logic how the referenced architecture issue is related to this.

[allenporter]

I'm not following the logic how the referenced architecture issue is related to this.

I'll try to clarify my thoughts:

• We should do more data validation in the calendar event. That discussion gives the first example of validation I would like to do. In doing that work to validate new changes, I will also validate the whole spec, and have a single dev blog post.
• My lived experience proposing calendar spec adjustments is that it can take quite some time to review. (Albeit that is lengthy, it's fairly basic calendar stuff). Fixing this to integration to follow our spe, rather than pausing to adjust any standards which can happen separately, seemed like the right call.

[allenporter]

Also my impression is this PR is motivated by trigger misbehavior and the dev seeing this fix it. That is also what motivated that arch discussion I linked. I'm trying to bring more clarity to events to root out any trigger related bugs that may be lurking with more validation.

[frenck]

We should do more data validation in the calendar event.

The problem shown here, is not related to validation... Not even remotely.

[allenporter]

We should do more data validation in the calendar event.

The problem shown here, is not related to validation... Not even remotely.

The problem here is our spec says The end (exclusive) of the event. Must be after start and this is doing something else. I'm saying something fairly simple: validation would catch the misalignment sooner. I hear you that you think the right answer is to change the spec to allow optional end dates or something else, and what i'm talking about is the bigger picture of misalignment of which this is one example. Given we've settled on this PR and my line of thought is not resonating, i'll stop.

[frenck]

and this is doing something else.

Well, there are multiple things going on, however, all would not have been needed if the spec part I've listed above was implemented and accepted. The validation you point out would catch this, but it shouldn't have been needed in the first place (as there should not be a need to specify the end date of the event).

../Frenck