openhab / openhab-addons

Add-ons for openHAB
https://www.openhab.org/
Eclipse Public License 2.0
1.86k stars 3.58k forks source link

[iCalendar] Introduce Trigger channel for event start/end #13649

Open t2000 opened 1 year ago

t2000 commented 1 year ago

As already mentioned briefly in this discussion: https://community.openhab.org/t/icalendar-binding/85130/12

it would be nice to have a trigger channel which fires every time an given event starts.

I looked at the documentation of this binding, but it offers state channels only. However, it has an option to specify specially encoded event descriptions in the calendar which are executed at the exact same time the event starts or stops.

IMHO one could use this logic to offer trigger channels one that fires on the start of an event and one at the end.

Use-Case for such a feature would be a scheduler that isn't as simple as a cronjob, but controlled via a calendar. One could possible use it for reminders that can be send via telegram bots, email, announcements through speakers, etc.

@daMihe Do you think such a feature would be helpful? Since I would like to have it instead of writing a cron rule that checks every minute the contents of some calendar item, I could look into implementing it and create a PR. WDYT?

daMihe commented 1 year ago

I also thought about this recently and it‘s a great idea. In fact i also thought about channels triggering an update and filtering events similarly like the EventFilter does. That is a use case that is requested quite often.

However, it would need to change the behaviour of the old bridge item which is not able to filter but has command tags. I don‘t really like the idea of extending this bridge item, to have several methods of triggering things.

What do you think about an additional item just for the current event? In my point of view it should have the ability for filtering by text/regex, provide channels for relevant textfields and instead of providing the command execution by tags it just should trigger.

But all that is not necessary for your usecase: You could just trigger a rule by a change of the current events title.

t2000 commented 1 year ago

But all that is not necessary for your usecase: You could just trigger a rule by a change of the current events title. How is this supposed to work? The title of an event changes once the calendar is downloaded and the item is populated with the next event. But the time of populating, i.e. downloading the item is not the same time as stated in the event...

What do you think about an additional item just for the current event? I am not sure what you mean by this, but I would like to keep it rather simple. Simpler solutions are usually the best anyhow ;)

Currently we have:

Channels for eventfilter
The channels of eventfilter are generated using following scheme, all are read-only.

Channel-scheme  Type    Description
result_<no>#begin   DateTime    The begin of an event
result_<no>#end DateTime    The end of an event
result_<no>#title   String  The title of an event

So we have 3 state channels for the eventfilter Thing. My proposal is to just add 2 trigger channels to that Thing. One that fires on the time that is the event's start time and one that fires on the events's end time. These events are even filtered by your logic (text in description, regex, etc.) too! So no need for additional complexity here.

daMihe commented 1 year ago

Nope, this is not possible as the eventfilter thing-type is driven by an update interval and the way it's written it can't find currently present events. So it is not able to trigger exactly when a event starts nor is it able to trigger on event ends as it simply does not have the information when filtering the events.

For the calendar bridge type:

The title of an event changes once the calendar is downloaded and the item is populated with the next event. But the time of populating, i.e. downloading the item is not the same time as stated in the event...

I do not fully understand, please explain. The title changes every time a event ends or starts. With exceptions: If a event with same title ends and starts after, the title stays the same (but i think the "updated" trigger for rule still applies, but i did not verify this, instead the begin and end change for sure) and if there are overlapping events, it may not change until the current event ends. The logic for updating these channels is in second precision, additionally to each reload done by an update of the underlying calendar.

t2000 commented 1 year ago

Nope, this is not possible as the eventfilter thing-type is driven by an update interval and the way it's written it can't find currently present events.

From a first 2 minute look on the documentation and the code I saw some schedulers and thought that they reschedule the "Begin:Item:Command" and ""End:Item:Command" commands in the event description to be triggered ONE the start/end time. And it looked as if it is calculating this based on the time difference between the download interval and the time of the event itself... But I can be wrong on that one as I didnt look into it in full detail.

I do not fully understand, please explain. The title changes every time a event ends or starts.

What I mean is: Once you download the calendar, you fill the items with the event information, i.e. take xmas on the 25th of december for example. Downloading it now will change the item's title to "xmas" also now and not on the 25th of december. That is what I means. However, you have a point that once the 25th of december is over the item information will change to contain the next event. But what if the next event is new years eve? It will then be triggered after the 25t of december is over => 26th of december and not on new years eve.

That is why I think reacting on the changed title won't work. But I might have misunderstood when exactly the binding updates the titles, i.e. maybe it leaves them untouched just until the next event will take place or so...

daMihe commented 1 year ago

Please don't get confused by different thing-types. They use different algorithms to find events. The calendar searches always events around now. eventfilter searches events by start and relative to now when using the timefilter or absolute if not using the timefilter. As the latter searches only by start and maybe relative to another point in time, it is not able to determine whether a event is present now. It is also very complicated to calculate the timestamp of the next change of channels for this thing-type as it is maybe moved relative to now or absolute. That's why this is done by interval refreshTime.

Only the calendar is able to know whether an event is present and only that thing-type is able to execute command tags. If you have no overlapping events, it's easy to predict the channel updates. Let's assume you have following events in you calendar:

Then icalendar:calendar:<usholiday>:event_current_title: will change on:

At these times also the also all other channels of calendar get updated and command tags are executed.

The situation gets a bit more complicated when you have parallel events. In that case the earliest ending event that is currently present is shown on the current channels (this code finds it in the calendar).

The calculation of the Future for updating the calendar channels next time is done by a time difference. The future will be canceled if a calendar reload happens before, which is completely asynchronous to the channel updates by events. But as a reload may load new additional events changing also a current event, the binding has to update all channels earlier. That's why it maybe looks like the calendar updates always on refreshTime especially if the parameter is very short and you use openHABs updated trigger instead of changed. In the example above you could also use a refreshTime of e.g. 14 days. The calendar thing still would update the channels as predicted (not to confuse with the eventfilter).

Apart from the whole discussion how an event is shown: i don't want to introduce a filter for the bridge type as that would lead to confusion when additional things connected to the bridge still get the unfiltered calendar. Instead, i would add a new thing type that behaves much like the calendar and the ability to filter by texts plus the trigger channels but without the ability to execute command tags. In long term, i would also move the tag execution to a separate thing-type. Why do i want to do that: If you are using a public calendar, someone may insert command tags into events. Making the command execution completely optional should make it more safe for most users not using command tags (at least i think it's a more uncommon feature) without losing the functionality if needed (it's very powerful).

Sorry for this wall of text. I read some confusion and hope you are not more confused than before.

openhab-bot commented 1 year ago

This issue has been mentioned on openHAB Community. There might be relevant details there:

https://community.openhab.org/t/which-parts-of-icalendar-do-you-use/140983/3

daMihe commented 1 year ago

@t2000 @brianmichalk1 Made some progress and built a version with triggers and additional channels for live events that can be set off. Maybe you want to check out my branch or a binary built there (same code, compiled for convenience).

daMihe commented 1 year ago

@t2000 @brianmichalk1 Did you have a chance to test the last state in order to push this forward?

brianmichalk1 commented 1 year ago

@daMihe I will test today.

daMihe commented 1 year ago

As maybe a refreshed version helps testing, i rebased my changes on current main. Build versions for 4.0.0-snapshot are the current ones there, the code itself was not touched.

openhab-bot commented 1 year ago

This issue has been mentioned on openHAB Community. There might be relevant details there:

https://community.openhab.org/t/which-parts-of-icalendar-do-you-use/140983/8

t2000 commented 1 year ago

@daMihe Thanks a lot for the implementation and also the rebase!

Unfortunately I didn't find time earlier to test it. However, after reinstalling+configuring the IDE (ALWAYS when I start it a few weeks/month of not using it, there are issues, this time it was: https://github.com/openhab/openhab-distro/pull/1471 among other things), I found some time to test your branch.

Good news is: It works!

However, I think we might need to tweak the options of the trigger channel a bit. Currently you have START and STOP:

<channel-type id="current_event">
        <kind>trigger</kind>
        <label>Current Event</label>
        <description>Current Event trigger for START and END</description>
        <event>
            <options>
                <option value="START">start</option>
                <option value="END">end</option>
            </options>
        </event>
    </channel-type>

What happens if I let it run is:

17:12:00.293 [INFO ] [openhab.event.ChannelTriggeredEvent ] - icalendar:liveevent:mycal:myfilter:current_event triggered START
17:12:00.293 [INFO ] [openhab.event.ItemStateChangedEvent ] - Item 'CurrentPresence' changed from OFF to ON
17:12:00.294 [INFO ] [openhab.event.ItemStateChangedEvent ] - Item 'CurrentSummary' changed from UNDEF to test2
17:12:00.294 [INFO ] [openhab.event.ItemStateChangedEvent ] - Item 'CurrentDescription' changed from UNDEF to 
17:12:00.295 [INFO ] [openhab.event.ItemStateChangedEvent ] - Item 'NextSummary' changed from test2 to test3

So at first the trigger channel fires, but only AFTERWARDS the other items are updated. So If one wants to write a rule that uses the description or the summary of the event, he needs to access those items to get that information. Unfortunately that information may not have reached the items yet once the trigger fires.

Turning this around, i.e. updating the items before the trigger might help a little, but it is still a race condition on when the event bus writes the values into the items and whether that is done before the trigger fires.

I am open to suggestions how this can be solved.

Otherwise: you may want to open a PR so we can start doing a review.

daMihe commented 1 year ago

Hi @t2000, Thank you for your feedback.

In fact, the start and stop-triggers are already triggered after the channels were updated (maybe, this would be for stop not optimal as both triggers are done after channel updates). So it seems you see something like a race condition. Maybe this is also some kind of optimization or so. I did not want to trigger reviews by a PR without any first feedback as the review capacity is limited. But i can do if you think it's okay enough to do so.

Currently, i do not have an idea how to sort those updates and trigger channels correctly. Need to read the docs a little bit more, maybe there is a way. Let's see when i have time - maybe this weekend or next.

t2000 commented 1 year ago

One "solution" that just came to my mind is to introduce an optional delay parameter on the trigger channel where the user can configure that the channel fires x mill is/seconds later than the state channels.

By default the delay would be 0, so it trigger on time as expected. And the README could contain the information that a user might use this parameter so he can read out the items on the state channels once the trigger channel fires.

What do you think?

daMihe commented 1 year ago

I don't like to delay, if there is a right way. Maybe it's possible to listen on the channel updates in the binding and then triggering the start. Just to reproduce this: Were the channels linked with items or rules? This may make a difference. If Yes, which settings were used for the links (esp. transforms?)?

t2000 commented 1 year ago

Items:

Switch CurrentPresence { channel="icalendar:liveevent:myCal:myLiveEvent:current_presence" }
String CurrentSummary { channel="icalendar:liveevent:myCal:myLiveEvent:current_summary" }
String CurrentDescription { channel="icalendar:liveevent:myCal:myLiveEvent:current_description" }

Switch NextPresence { channel="icalendar:liveevent:myCal:myLiveEvent:next_presence" }
String NextSummary { channel="icalendar:liveevent:myCal:myLiveEvent:next_summary" }
String NextDescription { channel="icalendar:liveevent:myCal:myLiveEvent:next_description" }

rules:

rule "cur event trigger"
when
    Channel "icalendar:liveevent:myCal:myLiveEvent:current_event" triggered 
then
    logInfo("Stefan", "Triggered")
end

However, i think it does NOT matter if you have linked the channels to items or not. It is just not predicable how the event bus delivers the events.

I have a lot of knowledge about the openHAB core and listening to events on a channel is certainly not something that should be done in a binding. Let alone that in this case the binding produces the channel events itself, so it already knows what it will publish there.

But I just found the "right solution": https://www.openhab.org/docs/developer/bindings/thing-xml.html#trigger-channel-types

If no tag is specified, the channel can be triggered, but has no event payload. If an empty tag is specified, the channel can trigger any event payload.

That way you can have anything as a payload on the trigger channel and have all information in one place. I would give the user a configuration option on the trigger channel in addition: payload type with the possible options to take the payload from the event summary, description, comment or so.

daMihe commented 1 year ago

I was assuming handling updates and triggers is maybe done by multiple threads, that it's not predictable also points in that direction.

How to trigger with any payload? Currently BaseThingHandlers triggerChannel() is used. I could replace the semi hardcoded start and end by a configured string and maybe even build a string. But that is kind of hacking the framework, also dates would be also strings. Or do i miss domething or don't get your idea yet?

Also a pragmatic question is, why just not listen to the title (or any other required) channel explicitly so you get the information as rule payload? Assuming you need multiple information about the event in that rule and you want to use the start trigger: Then even triggering start later even feels a better way to solve this behaviour, but it's still a workaround around the cores event bus ordering flaw.

t2000 commented 1 year ago

Oh nooo, I think I have misunderstood how you implemented the state channels... So when the current event STARTs you update all current channels and when it STOPs you rest the values to UNDEF, right?

In that case I think we do not need the trigger channel at all. This information can be inferred from a change on any of the current channels.

daMihe commented 1 year ago

Nearly. If stop/end ist triggered and there is a following event starting at the same time channels get updated to the new events state. And start gets triggered after (at least in the code) stop was triggered.

For just the information „an event starts or ends“ the trigger channel works fine, which is okay for heating etc.. For other information i can also trigger, if i find out how it’s useful. E.g triggering start or end with events uid or similar would be nice as then then rule could identify which event actually started or ended, as the live event thing is only able to show one event in its channels.

t2000 commented 1 year ago

If stop/end ist triggered and there is a following event starting at the same time channels get updated to the new events state

Of course if the new event has the start time = end time of the previous it makes sense.

For just the information „an event starts or ends“ the trigger channel works fine

As i mentioned before: this is duplicated information. You can achieve the same with a rule on the state channel item, I.e. "itemName changed" and in there you can even get its new value, SK you know what event has started.

So I would say, please remove the trigger channel under create a PR so we can start reviewing. Thanks for implementing it!

openhab-bot commented 1 year ago

This issue has been mentioned on openHAB Community. There might be relevant details there:

https://community.openhab.org/t/icalendar-binding-triggering-time-offset/144879/2