GNOME Bugzilla – Bug 561454
Idle tracking adjustment wrong when locking screen
Last modified: 2009-03-02 15:02:22 UTC
Please describe the problem: hamster is incorrectly subtracting the 'time to regard computer as idle' from the currently running task when you have the 'stop tracking when computer is idle' option set, but you lock the screen manually (i.e., the time for the computer to become idle was 0). Steps to reproduce: 1. In gnome's system->preferences->screensaver, choose a time to regard computer as idle value 2. check the 'stop tracking when computer is idle' option in hamster 3. start a task 4. lock the screen 5. wait a few moments (it takes a short while for hamster to notice the computer is now idle and stop tracking) Actual results: you will see X minutes incorrectly subtracted from the task you were tracking where X is the minutes you set above in step 1. Expected results: the task should have stopped being tracked, but the time should not have been adjusted at all. Does this happen every time? yes Other information: I have hamster set to stop tracking when the computer is idle. This is working fine. It seems that when hamster detects that the system is idle, it assumes the time to 'regard computer as idle' has elapsed (this is set in the screensaver prefs -- I guess my configuration is after 10 minutes). So, when hamster sees an idle computer, it stops tracking and subtracts 10 minutes from the currently tracked task. The problem is, I like to lock my screen when I leave my desk. When the screen is explicitly locked, the computer becomes idle without going through the 'regard computer as idle after X minutes' period. In this case, it is incorrect for hamster to subtract any time from the task, it should just stop tracking. This is potentially serious. If I had set my computer to become idle after 60 minutes, and hit lock screen, I can imagine hamster will subtract an hour from my task even though it shouldn't subtract anything! I'm guessing hamster just periodically checks to see the computer's idle state and if it's idle assumes it became idle through inaction. I don't know if it's possible, but it would probably be more correct if you can somehow notice the lock-screen event (if one exists I don't know much about dbus), and stop tracking immediately.
Thanks for the report, Uri - will look into it!
Hey Toms, I'm working on a patch for this. I've never used dbus before but I figure this is a good way to get my feet wet. I've already got hamster noticing when the screensaver sends the session idle True and False signals but I'm having more trouble detecting the Lock screen method call (since it isn't really a signal), but I think it's doable.
That's cool, Uri - i think, what you could do, is just do the check every time hamster refreshes (which it does once a minute) - that's in the applet.py. So a function like "check_idle" or something could do!
That's pretty much what I have right now. Hamster listens for dbus signals coming from org.gnome.ScreenSaver (does this need to be cross-platform or anything by the way), and stores the current idle state. Then when Hamster does it's once a minute check, it checks the idle state that was stored.
Sounds good - not sure if you have to listen to events at all, just do a once in a minute idle check. While you are at it - one thing that is still missing, is stop tracking, when computer has been put to sleep/hibernate (for the preference "stop tracking on shutdown"). I looked at it at some point, but didn't figure out the proper signal. The windows bit is no really needed now. If somebody decides to try and actually make hamster work in windows, there will be far more critical riddles to solve :)
Hey Toms, I have a patch but I'm having trouble testing it. Every time I try to edit my preferences I get a traceback. I tried grabbing a clean svn tree and even so I still get the same problem. It looks like this: Traceback (most recent call last):
+ Trace 210920
self.preferences_editor = PreferencesEditor()
self.load_config()
self.get_widget("keybinding").set_text(self.config.get_keybinding())
I wanted to delete any prefs or database files, but I can't find them. Any ideas?
fyi I found the database file in .gnome2/hamster-applet and removed it but that didn't help.
haha, well you have overdone with cleaning! Anyway, update hamster from TRUNK - i added default values in the config retrieval for maniacs like you - that should fix the problem :)
Hey there, so I managed to get around the problem last night just by commenting out the offending line so I was able to bring up the preferences. I tested my changes and they seem to work. There is one issue that I'm not sure how to handle. I did an svn update to get the latest from the trunk and it seems there is some work added which partially overlaps what I've done. I'm referring to the code in applet.py that adds a signal handler to the session bus to force a hamster refresh whenever a signal is received on the screensaver interface (at least that's what I *think* it's doing). I had to do something slightly more complicated because the signal handler is not able to pick up method_call type messages which are necessary to detect a "Lock" message. The route I went was I added a dbus inspector class in idle.py that basically keeps track of the current idle/locked state of the session, and then applet.py has an instance of that that it queries during the course of a normal periodic hamster_refresh(). Basically, it has the right locked/idle state, but it doesn't force a refresh when it gets one of these signals, it just reports the state when hamster_refresh runs every minute. On the other hand the change I got from upstream seems to have the purpose of forcing hamster_refresh to run immediately when a proper signal is received. So, to summarize, my change keeps track of the state properly, while the upstream change reduces the latency to notice that state change. Both of these seem good, but the problem is I'm not sure that having both changes in there are compatible. I say that because in my line I merged them in such a way that both are happening, but I do not see the effect of the decreased latency to detect idleness. I'm guessing that this is because the two message handlers are in a way competing to consume messages (but I may be wrong on that account--I am a dbus newbie). So basically I'm not sure how to best merge the functionalities. I'm thinking a way might be to add the ability to add something like a functor to the dbus inspector class I've created, such that it will run the function specified when it receives a signal. Therefore you can do something like (in applet.py--my class is DBusIdleInspector and lives in idle.py): self.idle_inspector = DBusIdleInspector(self.hamster_refresh) Then the idle_inspector's message handler could also call the specified function when it's done. Like I say, I'm not sure. When I get home from work I will send the two files I've modified (idle.py and applet.py--the changes are really quite minor in the end =)), and you can take a look and decide how you want to handle this. I understand there is a lot of work going in to dbus, so maybe you could just use my implementation as a reference and somehow work it into the larger work.
haha, how about less talking more patch, Uri? :) The refresh right after wakeup is necessary for those rare cases, when user suspends laptop and then wakes it up next morning. I think lock/unlock is also happening at that moment, so we could go for your inspector thing! Anyway, could you upload a patch, so we can see the changes? - that'd be aaawesome!
Created attachment 124982 [details] [review] patch for idle.py/applet.py Sorry I was at work or I would have gotten to this sooner..
hey guys, any news on this? did you try the patch?
Ah, totally missed out on this, thanks for reminding! It looks good and seems to work too. Committed to trunk, so we all can test. Thanks!
I probably should have marked the bug as fixed huh... Well anyway, thanks!