GNOME Bugzilla – Bug 430797
gnome-typing-monitor: timer should be reset after standby
Last modified: 2010-01-12 03:21:13 UTC
Please describe the problem: After going to standby and not using the computer (laptop) for a while, the gnome-typing-break-timer is not reset Steps to reproduce: 1. activate typing break (system -> settings -> keyboard -> typing break) 2. wait until some amount of time is elapsed and gnome suggests you to make a break soon 3. got to standby 4. wait for at least the time of the typing break 5. resume 6. gnome still suggests to make a break Actual results: see above Expected results: gnome should "reward" the time I were in standby and not suggest a break anymore.. Does this happen every time? yes Other information: I think basically the typing-break-timer can just be reset when going into standby.. or the unix-time when going into standby must be memorized and compared to the time, we resume.
This is already handled in the code since several years and I can't reproduce it. Maybe I am missing something? What version are you using?
I'm using Gnome 2.18.1 .. ist not several year's old :D Normally the typing break is automatically reset when I don't typ for the configured amount of minutes. I repoduced it again: I 've gone to standby for 5 minutes when there were only a few minutes left to the next break, typing break is set to 3 minutes. When I resume, the typing break still forces me to make a break soon - but I've not been typing for a while when the laptop was in standby ;)
Alright, I'll look into it then :) The code currently deals with suspending already but maybe there is some case I have missed (I have never seen it not work for me :).
I can confirm this. It started happening after upgrading to Ubuntu Fiesty, i.e. from control-center 2.16 to 2.18.
Same here. Very annoying if I wake up my laptop from standby in the morning and 5 minutes later i'm already forced to have a break, just because the evening before the counter was at 55 minutes...
*** Bug 491800 has been marked as a duplicate of this bug. ***
This is a bit odd btw, since it used to work at some point (I explicitly added code for that several years ago :). Probably a bug that sneaked in somewhere.
In Fedora 9 I have same bug. (control-center 2.22.2.1) After suspend (even for long time) it forces me to make a Typing Break too soon. Timer counting minutes untill break has exactly same value as just before suspend. I thing that in some new distributions, suspend mechanisms were changed in some way, they introduced new solutions, maybe thats a cause that on Fedora 9 and Ubuntu 8.04 this bug have re-appeard. Even though it have been fixed previously. ---- My configuration: Fedora 9, Gnome 2.22.2 Linux 2.6.25.3-18.fc9.x86_64 #1 SMP x86_64 HP Compaq nx7400
I can confirm the behavior on Ubuntu Hardy (and on Gutsy before it). This makes the Typing Monitor nearly useless: one becomes accustomed to dismissing it when it needlessly nags, then forgets to pay attention to it when a reasonable interval has passed. acpid 1.0.4-5ubuntu9 acpi-support 0.109 apmd 3.2.2-8.1ubuntu1 gnome-control-center 1:2.22.1-0ubuntu4.1 linux-image-2.6.24-19-generic 2.6.24-19.33 linux-ubuntu-modules-2.6.24-19-generic 2.6.24-19.27 linux-restricted-modules-2.6.24-19-generic 2.6.24.13-19.42 pm-utils 0.99.2-3ubuntu10
It should be quite easy to fix if someone has some spare time. There used to be code that did something like checking if time() was much more than "last_time" + timeout. If so, either the system clock was changed or the computer has been suspended. The code might have gone missing when other things were changed/fixed. The easiest way to test it while developing is to just start it from the terminal and press ctrl-z for a while.
*** Bug 545113 has been marked as a duplicate of this bug. ***
Richard, I suppose the code you are thinking about is if (elapsed_time > dr->last_elapsed_time + dr->warn_time) { /* If the timeout is delayed by the amount of warning time, then * we must have been suspended or stopped, so we just start * over. */ dr->state = STATE_START; } This sometimes works, but sometimes doesn't. On my computer, I added g_debug("Elapsed time %d", elapsed_time); just before this. After suspending and restarting using the power button, sometimes the elapsed time did not include the time spent in suspension - it is as if no time passed while the computer was on standby. Hence the bug.
OK, so there are two ways to fix this, I guess. (1) fix GTimer. Presumably it is a bug to not count seconds while the computer has suspended. (2) watch for standby or resume notifications, and reset the internal timer when they occur. This works on the theory that if people standby or hibernate, they usually stay that way for at least the length of a typing break. (Unless someone set the break time to an hour, or something.) I can't do (1), so I looked for DBUS signals for (2). The only ones I found were from ConsoleKit, "system.IdleHint". I am not sure what this signal is meant to do, but I have a patch which uses it to reset gnome-typing-monitor. (Is there a canonical way for a Gnome program to learn that the system is being suspended?) Patch available on request.
Something very simple like just checking the difference between time() calls in each callback should definitely work, and will also catch things like suspending the typing-monitor [something like: current_time = time(NULL); if (current_time -prev_time > 20) ....; prev_time = current_time;]. That way you don't rely some technology that is not widely available.
I agree it ought to work, but we have this already - in the code quoted above - and it isn't working. I see your point about including gratuitous dependencies. Perhaps (1) is the best approach. The following test program shows the bug for me: #include <glib/gtimer.h> #include <glib.h> #include <stdio.h> int main (int argc, const char* argv[]) { g_thread_init(NULL); // CREATES BUG GTimer *foo = g_timer_new(); gdouble el; gdouble oldel; oldel = g_timer_elapsed(foo, NULL); while (TRUE) { el = g_timer_elapsed(foo, NULL); if (el > oldel+1) { g_debug("%f", el); oldel = el; } } } On my machine, without the g_thread_init line, after suspend/resume there will be a gap in the numbers printed (as there ought to be). With g_thread_init, there will not. Where should we report this?
Created bug #552994
*** Bug 554265 has been marked as a duplicate of this bug. ***
Unfortunately glib developers do not want to fix bug #552994 with GTimer. They say that GTimer is a simple tool and was not developed for purpose of counting real time. So probably there is a need to write some workaround or introduce some new dependency with desired functionality, to finally fix this bug in Typing break. Many clones of this bug emerges _constantly_ on different bugzillas, as Fedora, Ubuntu, etc. They are marked as duplicates and nothing is happening. But this shows that people are waiting for it. Wait a second, I have looked at top of this page and I can see that this bug report in two weeks will be having second anniversary.
*** Bug 583886 has been marked as a duplicate of this bug. ***
There is an option 3 I think. Basically use the standard time.h library. time.h is already included in typing-break/drwright.c We could use the time() function to get the time at different points and save it, and the difftime() function to get the number of elapsed seconds. Then we should be able to substitute that into the code in comment #12. Might have a go at a patch at some point, but not tonight ... Quick ref for time.h : http://www.acm.uiuc.edu/webmonkeys/book/c_guide/2.15.html
Does anybody know what is ETA on this Bug? I am facing it nearly every day, and this is happening for over a year now (bug itself has nearly 2.5 years). I have installed new system on a new computer (Fedora 11), and this bug jumped on me again.
Here's a patch against gnome-control-center HEAD (04f20139ce217560b44cb53e22cdffb261b29253) It does 2 things: 1) Defines a DrwTimer that we use instead of GTimer. This is just a thin wrapper around g_get_current_time, and it means we can accurately track typing/idle periods based on real-world wall-clock time, which GTimer is apparently not intended to do. 2) The typing monitor has some complicated state handling where it transitions between an IDLE state and a TYPING state. This transition is based on running a callback once per second, and checking whether any keystrokes have been recorded since the last time the callback was called. The actual idle *time* is tracked separately, independently of these two states, but only when we are in the IDLE *state* was the idle *time* checked to see if we should reset the break timer. This leads to a race condition -- if we suspend while in the TYPING state, then eventually we will wake up, notice that no key press has happened in the last second, *reset the idle timer*, transition to the IDLE state, and then check the amount of time on the idle timer. We will thus never notice the amount of time that the computer was suspended. I considered making the IDLE/TYPING transition code smarter, but it turns out the desired behavior for the two states is entirely identical anyway, so rather than adding more complexity to this pointless code, I just diked it out and replaced them both by a single state called RUNNING. Tested and fixes this bug for me. I believe this patch is ready to apply.
Created attachment 144099 [details] [review] proposed fix
This bug has had a patch sitting and waiting review for more than a month -- is there something I should do to get someone to take notice? I've been running with the patch applied all that time with no problems.
Review of attachment 144099 [details] [review]: > This bug has had a patch sitting and waiting review for more than a month -- is > there something I should do to get someone to take notice? No, sorry. Sometimes it just takes a while for someone to look at it. Thanks for the patch and your patience! Apart from the mentioned minor issues the patch looks fine. ::: typing-break/drw-break-window.c @@ -45,3 @@ GtkWidget *postpone_button; - GTimer *timer; The original code uses tabs for identation while you use spaces. Please don't mix styles. ::: typing-break/drw-timer.c @@ +29,3 @@ +DrwTimer * drw_timer_new () +{ + DrwTimer * timer = g_new0(DrwTimer, 1); There are some places where you don't quite adhere to the coding style. This line, for example, should read DrwTimer *timer = g_new0 (DrwTimer, 1); ::: typing-break/drw-timer.h @@ +34,3 @@ + */ + +typedef struct _DrwTimer DrwTimer; You might want to consider just using GTimeVal directly. The additional object (not the functions) doesn't seem to add much value.
Created attachment 147354 [details] [review] second try > The original code uses tabs for identation while you use spaces. Please > don't mix styles. Fixed. > There are some places where you don't quite adhere to the coding style. Whoops, you're right, I missed a few. Fixed. > You might want to consider just using GTimeVal directly. The additional > object (not the functions) doesn't seem to add much value. I've considered, but I think the difference between a timer and a time are large enough that the abstraction is worthwhile. This is such a trivial little corner of code, though, that I don't think it really matters much either way; if you insist then I'll change it. I don't have commit access, so if this is patch is good then please commit.
(In reply to comment #26) > I've considered, but I think the difference between a timer and a time are > large enough that the abstraction is worthwhile. The simplest way would have been typedef GTimeVal DrwTimer; I agree it's no big deal, though.
Comment on attachment 147354 [details] [review] second try commit 543251c2c65663aff367d4ddba719e89de7e03ca Author: Jens Granseuer <...> Sorry about that. Just noticed after I'd pushed. You really should send proper git patches, you know.... Anyway, I'll try to get you mentioned in the NEWS for this one, at least. Date: Wed Nov 11 19:37:04 2009 +0100
Thanks, and no worries.
*** Bug 602491 has been marked as a duplicate of this bug. ***
It says it's fixed but this is still an issue for me in Ubuntu Karmic
(In reply to comment #31) > It says it's fixed but this is still an issue for me in Ubuntu Karmic That's because the fix has been committed to Gnome, but Karmic was released before the fix. Lucid will probably have the fix, or you can try to convince the Ubuntu folks to apply the fix to Karmic here: https://bugs.launchpad.net/ubuntu/+bug/318403 The people here at gnome.org can't help with that part, only the Ubuntu folk can.