-
Notifications
You must be signed in to change notification settings - Fork 20
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Set up a basic daemon with node monitoring #126
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Some notes
std::cout << "New Event Created!" << std::endl; | ||
break; | ||
case B_ENTRY_REMOVED: | ||
case B_ENTRY_MOVED: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Aren't we also interested in B_ATTR_CHANGED?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes we are, but currently we are monitoring the events directory only. So, only the B_ENTRY_CREATED & B_ENTRY_REMOVED are being used.
B_ATTR_CHANGED
will be used when we are monitoring the nodes of separate events
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you please describe the complete and final solution you will pursue to have the daemon triggered when something changes in the events?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, basically we are going to node monitor the events directory as well as all the events.
The directory for checking whether any event is created or deleted, using B_ENTRY_CREATED
, B_ENTRY_REMOVED
, B_ENTRY_MOVED
.
When any event is added to the list, AddEventToList()
will be called. It is in this function where we call for node monitor the event file for B_ATTR_CHANGED
(Suggested by @humdingerb here)
daemon/src/CalendarDaemon.h
Outdated
#include <Volume.h> | ||
|
||
class BMessage; | ||
class BString; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is probably not necessary and can be removed. The reason is that if there would not be a full definition of BString, you would probably get a compiler error. I think in general we like to explicitly #include <String.h>, even though it is technically not necessary as it is probably already included indirectly.
Just a point about forward declarations: they work for BMessage and BDirectory, because you only use pointers to them in the class declaration below.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is probably not necessary and can be removed. The reason is that if there would not be a full definition of BString, you would probably get a compiler error. I think in general we like to explicitly #include <String.h>, even though it is technically not necessary as it is probably already included indirectly.
Just a point about forward declarations: they work for BMessage and BDirectory, because you only use pointers to them in the class declaration below.
I think we do not need the forward declarations here and removing all of them does not produce any error. So I am removing all the forward declarations.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I remember the discussion, but I think we should reconsider, because I doubt it is scalable. Let's say I use Calendar a lot for both my work and private events. And let's say I have about 10 entries a day, so 3650 entries a year. I don't know if there is a limit for the number of node monitors there are, but setting it up is not free.
Was the live query idea a dead end?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I remember the discussion, but I think we should reconsider, because I doubt it is scalable. Let's say I use Calendar a lot for both my work and private events. And let's say I have about 10 entries a day, so 3650 entries a year. I don't know if there is a limit for the number of node monitors there are, but setting it up is not free.
Yeah, this is true. But what can be done instead of monitoring all the events? We still need to update the list when the time attribute of any event is changed.
Was the live query idea a dead end?
The live query can track the creation/removal of events, but is unable to get anything for attribute changes.
The creation/removal of events can also be tracked through the node monitoring, so it doesn't provide much different utility.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Was the live query idea a dead end?
The live query can track the creation/removal of events, but is unable to get anything for attribute changes. The creation/removal of events can also be tracked through the node monitoring, so it doesn't provide much different utility.
Have you confirmed that in a test? Because that's not how I understand the documentation. Did you test it with Humdinger's notes about updating indexes?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi, I think Harshit's correct, that a live query doesn't suffice for our purposes. From the BeBook:
A query update message describes a single entry that has changed so that…
- it now satisfies the predicate (where it didn't use to), or…
- it no longer satisfies the predicate (where it did before).
So, if you only change some event from next Tueday at 5pm to 6pm, you' won't get a query update message.
Maybe we should throw out the idea of the daemon being totally independent of the Calendar main app. After all, you cannot easily change the start/end time attributes in Tracker anyway, AFAICS.
Maybe the daemon should just do a simple query (not live) for all events with a set reminder with a start time of now to 6 months in the future (I doubt Haiku has a longer uptime - otherwise one could for example do regularly re-do that query). And if the user add/removes/changes any event in the Calendar main app, have it send a message to the daemon (either with the changed event, or simply a signal to re-do the initial query).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sending a message from the Calendar app to the daemon is not a bad idea either. Also, if we set this up (for add/remove/changes) then neither node monitoring nor the live query would be required.
How do you see this, @nielx ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am not sure. I think receiving messages from the Calendar can be one way to receive updates, but we cannot always assume the Calendar is running, and potentially there is a chance that a user uses a different mechanism to add and edit entries. Furthermore, I think there might not be a guarantee that all messages arrive.
Maybe the answer is in queries after all. The documentation mentions that there is an automatic last_modified
attribute. Maybe run a live query that monitors whether there are entries that change in the future?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If we use the last_modified
attribute, I think of this solution:
- Have another attribute in the event, namely,
EVENT_CREATED
- The the live query predicate will look like this:
('REMINDER_ATTR' > Current Time) && ('last_modified' >= EVENT_CREATED)
Do you think this will work?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So if I were to design it, I would have two separate processes:
- One is to load just enough data to figure out what the next event(s) is (are) to alert for.
- The other is to refresh that bit of information if there is a change of the on disk info.
If it is possible to combine those two, then that's fine, but you need to determine/try whether your approach works for that.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I can merge it if you remove the unused code.
(I mean fTrashDir).
For issue: #122
This PR is supposed to set up a basic daemon which node monitors the Events directory for events adding/removing.
This can be tested as follows:
make
command indaemon/
The daemon should mention regarding the update, whether an even is added or is removed in the terminal.