Reminders for Events

[harshit-sharma-gits]

For Issue: #122 Started working on the calendar_daemon Currently I tested out sending Notifications from the Application with the help of BNotification

I'm submitting this PR for regular updates regarding the coding and their review

[humdingerb]

Also, I suggest to use a BAlert instead of a BNotification. Currently, notifications cannot be made "sticky", i.e. they automatically disappear after the set amount of seconds. It would be too easy to miss such a notification. BAlerts stay until the user clicks a button.

[nexus6-haiku]

I disagree, the BAlert does not seem an option to me. All major OS use notifications coupled with a sound and so do third party apps like Outlook. Notifications seem the best way to, well... notify the user, IMO. As a side note, I suggest to take inspiration from MacOS and maybe implement something similar to the Notification Center to visualise any missed notification (at the OS level, of course).

[humdingerb]

We should use what works with the task at now. Once Haiku's notification services have improved, it's trivial to replace the BAlert with a BNotification. Unfortunately, while such work was started, there hasn't been any progress for a long while. See https://discuss.haiku-os.org/t/notifications-preflet-changes

[harshit-sharma-gits]

Hey @humdingerb ! Can you please check out the folder structure now?

[humdingerb]

Personally, I'd remove the "src" folders and have the source right under main | daemon.

[harshit-sharma-gits]

Hello! Completed a lot of work! Now the Daemon is in working condition!

Let me summarize what I did:

• Node Monitoring: We were not supposed to do Node Monitoring, but Live Queries. I tried to get the Live Query working ,but in vain. I used the SetTarget(be_app_messenger), so that we can receive B_QUERY_UPDATE message whenever the Events were updated. But the message wasn't getting received. So, for the app to work now I used Node Monitoring. It is successfully Refreshing the fEventList whenever an event is added/removed (But is not working for event time change!)
• EventLoop Thread: The Loop for firing alerts/reminders is working in a separate thread and is successfully showing the alerts/reminders for the events (and yes, the Node Monitoring is working in sync with it!)
• Semaphores: fEventLock and fNotify are the semaphores responsible for syncing between the "Alert Showing" thread and the "Node Monitoring"

I have tested the program. It is showing notifications on the Event Start time and is taking the Adding/Removing of events fruitfully (But the event changes is not Refreshing the list)

[nielx]

So your next job is to find out why live queries are not working. How did you test it? Did you find an example in the Haiku Source Code that can teach you something?

[nielx]

Also... I was looking at the Calendar source code. Can you tell me what QueryDBManager::GetEventsToNotify() does and where it is used?

[harshit-sharma-gits]

How did you test it? Did you find an example in the Haiku Source Code that can teach you something?

I did not find an example, but this documentation. And I tested it out as follows:

In the constructor -

CalendarDaemon::CalendarDaemon()
{
    BQuery query;
    query.SetVolume(&fQueryVolume);
    query.PushAttr(START_ATTR);
    query.PushUInt32(time(NULL));
    query.PushOp(B_GE);

    if(query.SetTarget(be_app_messenger) != B_OK)
        std::cout << "Query Target not set" << std::endl;

    query.Fetch();
...
}

As the target is now set, the query can send the B_QUERY_UPDATE message whenever there is some change in the events. So in the CalendarDaemon::MessageReceived() I put this case to see if the Live Query if working-

CalendarDaemon::MessageReceived(BMessage* message)
{
    switch(message->what)
    {
        case B_QUERY_UPDATE:
            std::cout << "Events Changed - Live Query" << std::endl;
            break;
        ...
    }
}

But the message is not receiving, in any change case of the events (Adding, Removing, or Attr Change) I also tried to make the BQuery query; a class member BQuery fQuery;, so that if the scope of the query is only limited to the constructor, then it can be fixed.

Still doesn't work.

[nielx]

There are two reasons it is not working:

• The first thing is that query is a local object, and as such the live query is ended as soon as you are out of the scope of the constructor. So you should definitely approach it with BQuery being a class member.
• Secondly, you are targeting be_app_messenger. This is a global 'convenience' object that quickly helps you target messages to the BApplication object. However, at this stage you are in the constructor of that BApplication object. Thus the messenger is likely invalid and therefore you are not receiving messages. At this stage, you can target this.

[harshit-sharma-gits]

Also... I was looking at the Calendar source code. Can you tell me what QueryDBManager::GetEventsToNotify() does and where it is used?

QueryDBManager::GetEventsToNotify() just returns the UNNOTIFIED events list There is an implementation for showing the Notifications when the Event starts. I also tested it out, but it is very buggy. Although there is a seperate thread for the NotificationLoop, but the Notifications doesn't show up unless the app is running. The Notifications also doesn't show up, when the start time of an event is changed.

Though this might be a useful utility, but it certainly needs some work. Also, when the calendar_daemon is deployed, we can also set up the Event Start Notifications from there as well.

[harshit-sharma-gits]

• Secondly, you are targeting be_app_messenger. This is a global 'convenience' object that quickly helps you target messages to the BApplication object. However, at this stage you are in the constructor of that BApplication object. Thus the messenger is likely invalid and therefore you are not receiving messages. At this stage, you can target this.

Thanks for the suggestion! I moved the query "build-up" to ReadyToRun() function; as the messenger is set-up after the constructor.

I still am not able to use the be_app_messenger but with this, it works fine!

[harshit-sharma-gits]

The Query is only able to fetch the Addition and Removal of Events, as only B_ENTRY_CREATED and B_ENTRY_REMOVED are available as op-codes in B_QUERY_UPDATE

So, the Refreshing is not working when an Event's start time is changed.

How can we approach to solve this?

[humdingerb]

How can we approach to solve this?

One possibility is to node-monitor the events folder and if some event changed, check if it's a "remindable" event and update the list of events returned by your query.

Another is to send a BMessage from the main Calendar app to the daemon, telling it which event needs to be updated.

The second approach feels simpler, however, if a user changes an event's attribute in Tracker or Terminal or any other way outside the Calendar app, only the first approach works 100%.

[harshit-sharma-gits]

One possibility is to node-monitor the events folder and if some event changed, check if it's a "remindable" event and update the list of events returned by your query

For node-monitoring the events folder, we need to watch the events directory using the B_WATCH_DIRECTORY Flag, which is only able to post B_ENTRY_CREATED and B_ENTRY_REMOVED messages (no support for attribute changes?)

Another thing is to watch using the B_WATCH_ATTR Flag, but it only watches a single file, not a whole directory.

[humdingerb]

I suppose you'll have to watch every entry returned by your query. Should be doable, because nobody has thousands of events planned in advance. I think... :) Maybe, when you have it set up, you can test with various amounts of events (100, 1,000, 10,000) just to see how it impacts the system.

[harshit-sharma-gits]

A simply approached towards this as follows:

While adding the event to the list, called for watch_node(&nodeRef, B_WATCH_ATTR, be_app_messenger) for each event added. Afterwards when the RefreshEventList() is called, firstly it'll stop monitoring all the nodes by stop_watching(be_app_messenger) and then again adding updated events with Node Watching.

But a wierd error is happening and I am not able to figure this out: Whenever the ATTR of any event is changed, that event (updated) is added two times in the list!

Also, after the node monitoring, the update for any change (adding/removing) is happening multiple times

[humdingerb]

calendar_daemon crashes on launch: calendar_daemon-23627-debug-11-08-2022-15-32-06.report.txt

[harshit-sharma-gits]

calendar_daemon crashes on launch:

This crash was due to the fact that I did not put am empty list check on ShowEvents(). Now it is fixed!

[humdingerb]

Fix confirmed. Cannot advise about your issues of multiple node-monitor notifications, though... Keep investigating, but before you get totally stuck, maybe add the new "Remind me" checkbox and additional attribute handling to the main Clendar app. Just for a change of scenery...

[harshit-sharma-gits]

As suggested by @nielx I started working on removing the EventLoop. The idea is to use Pulse() function to periodically wake the thread and check the need for sending the alerts.

Maybe I'm using Pulse() wrongly here, but it isn't working as expected. After setting the Pulse Rate to 1 second in SetPulseRate(bigtime_t(1000)); the Pulse() is supposed to be called every 1 second, right?

[humdingerb]

SetPulseRate() takes microseconds, so that would be 1000000 for 1s. If I understand the BeBook correctly, this sets the rate for all BViews attached to that BWindow. Another way to get regular messages is by using a BMessageRunner. Just in case Pulse() refuses to cooperate... :)

[humdingerb]

I only just saw that you're using the BApplication's Pulse(), not the BWindow one. Strike the above BView/BWindow reference. It's still microseconds though. AFAIK the Pulse() hook function is triggered and you don't need to watch for B_PULSE in MessageReceived().

[pulkomandy]

I didn't check the code to see if you already did that, but you need to set the B_PULSE_NEEDED flag on the application or window or view (depending on where you want to handle the Pulse events)

[harshit-sharma-gits]

I didn't check the code to see if you already did that, but you need to set the B_PULSE_NEEDED flag on the application or window or view (depending on where you want to handle the Pulse events)

The Daemon doesn't have any BView or Window as it is an background process, so we cannot set the B_PULSE_NEEDED flag on the BView.

But we can only set this flag on BView or a BWindow? as written here

[harshit-sharma-gits]

The GUI Changes along with some Event Class changes are done today. The GUI is looking like this:

Now the changes in Saving/Loading the Events are remaining. For them the following classes will get affected:

• EventWindow
• QueryDBManager

[humdingerb]

The time input isn't very nice (of course, the start/end-time input isn't nice either, but at least the user probably surmises it's to be done in hh:mm format...).

How about: [checkmark] Notify before event [text field (only numbers allowed)] [pop-up menu: seconds/minutes/hours]

I don't think we need "days" or longer timeframes. "0" or empty means notify on start-time.

[harshit-sharma-gits]

How about: [checkmark] Notify before event [text field (only numbers allowed)] [pop-up menu: seconds/minutes/hours]

That would be good I guess. Here, check the GUI now:

[humdingerb]

Looks good, just add a colon and use lower case: Notify before event: hours/minutes/seconds

[harshit-sharma-gits]

Hello! I've changed the Event Attributes and their 'Loader' and 'Saver' functions. Although there are some bugs remaining to resolve.

I also have some questions. First of all, what should we do with the SQLiteManager and GoogleCalendar/ classes and utils/ICal? They do not work and are not needed anymore. Should we remove them?

[humdingerb]

The SQLiteManager is still needed to migrate older data to the new attribute system. Only after a few releases, when can assume most users have updated, the migrating code the SQLiteManager can be retired.

[harshit-sharma-gits]

The SQLiteManager is still needed to migrate older data to the new attribute system. Only after a few releases, when can assume most users have updated, the migrating code the SQLiteManager can be retired.

Yes, that seems to be the case, but what should be done with the other classes? I think the choices are: remove them or make necessary changes in the 'Event' declarations there?

And I haven't made any progress on the Node Monitor issue, can you help me with that?

[humdingerb]

Don't know about GoogleCalendar and ICal. AFAIK, both currently don't work. Maybe the code can still be used as a starting point for a future effort to support them? Maybe a note about both not working should be added to the ReadMe.

WRT node monitoring issues, even if I probably won't be capable of helping, we need a detailed description of the problem. (Or did I just miss that? It's a bit unfortunate that everything is in this one big PR...)

[nielx]

What is the node monitoring issue again? I tried to look through the thread here, but all I am finding is that sometimes you get multiple notifications. How is that an issue?

[harshit-sharma-gits]

Hey! Sorry for being inactive here for some period. I was working on resolving some confusing bugs on Saving/Loading of Events with/without Reminder.

Now the Saving/Loading bug is resolved. And The Calendar only writes Event:Reminder attribute for the events whose reminder is set by the user, elsewise the Attribute is not saved in the event file.

[harshit-sharma-gits]

Here I'm having a weird bug, when I try to fetch the events using the START_ATTR it works fine. But when I try to do the same with REMINDER_ATTR it doesn't fetch any events.

It should be optimum to fetch the events with this condition: Reminder Date-Time > Current Time Which is present in the code as follows:

    fQuery.PushAttr(REMINDER_ATTR);
    fQuery.PushUInt32(time(NULL));
    fQuery.PushOp(B_GE);

But it isn't fetching any event. Yet when it is used with the START_ATTR (which denotes the starting time of the event), it fetches the events.

[humdingerb]

The new attribute needs to be added to the index to be queryable. See QueryDBManager::_AddIndices()

[humdingerb]

... and the MIME type, see QueryDBManager::_EventMimetype() above it.

[harshit-sharma-gits]

The new attribute needs to be added to the index to be queryable. See QueryDBManager::_AddIndices() ... and the MIME type, see QueryDBManager::_EventMimetype() above it.

Thanks for the suggestion!

[harshit-sharma-gits]

How to test the daemon?

• Currently the main calendar application is present in Calendar/main and the daemon is present in Calendar/daemon. We can build them by giving a simple make command from their respective directories.
• Then we can access them from their builds.
• Open the CalendarDaemon from its build; try to get it running in the terminal so that you can see the Refreshed Event List there every time when either a Event is added/removed or some event's attribute is changed.
• Now you can Launch the Calendar App, and add some events there with reminder, the Daemon watches for the events and add the appropriate events to the list, then executes them one by one on their respective times.

Also, it is not necessary to get the Daemon running before the Calendar. The Daemon can read through the events while starting itself as well.

[humdingerb]

From a quick test, the daemon picks up remindable events fine. I think, however, that since we can count on the daemon running, the main Calendar app now shouldn't do any notifications and leave it all to the daemon. Which means that additionally to the "notify x minutes before the event" the daemon should also do the "notify the event is now".

The alert is missing the event name and place. The text could also be improved, suggestion:

Calendar notification!

Event: %GetEventName()%
Place: %GetEventPlace()%

The event starts in x minutes.
                                      [OK]

[harshit-sharma-gits]

I have done the Localization in the code, please check that out! Also, should I create the en.catkeys? And add LOCALE = en in the MakeFile as well?

[humdingerb]

Also, should I create the en.catkeys? And add LOCALE = en in the MakeFile as well? Yes, please.

[humdingerb]

@nielx: I find this huge PR increasingly unwieldy. You think it'd be OK to merge after Hashit's next changes and have smaller, separate PRs to address open issues?

[nielx]

@nielx: I find this huge PR increasingly unwieldy. You think it'd be OK to merge after Hashit's next changes and have smaller, separate PRs to address open issues?

Agreed that the PR is unwieldy. I also don't know what I am reviewing and I feel like it is spiraling out of control, especially the changes in the original code. I don't want to merge this as is though, it needs to be cleaned up, but it CANNOT get any new changes added. In the email last week I outlined what I think the scope of the PR should be. I will review it accordingly.

[scottmc]

What's the current status on this one?

[nielx]

Closing; this PR has been superseeded by individual PRs