After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 430797 - gnome-typing-monitor: timer should be reset after standby
gnome-typing-monitor: timer should be reset after standby
Status: RESOLVED FIXED
Product: gnome-control-center
Classification: Core
Component: [obsolete] Typing break
unspecified
Other All
: Normal normal
: ---
Assigned To: Control-Center Maintainers
Control-Center Maintainers
: 491800 545113 554265 583886 602491 (view as bug list)
Depends on: 552994
Blocks:
 
 
Reported: 2007-04-17 20:45 UTC by self
Modified: 2010-01-12 03:21 UTC
See Also:
GNOME target: ---
GNOME version: 2.17/2.18


Attachments
proposed fix (11.60 KB, patch)
2009-09-27 04:13 UTC, Nathaniel Smith
needs-work Details | Review
second try (11.12 KB, patch)
2009-11-10 05:42 UTC, Nathaniel Smith
committed Details | Review

Description self 2007-04-17 20:45:49 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.
Comment 1 Richard Hult 2007-05-08 17:14:05 UTC
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?
Comment 2 self 2007-05-08 17:34:39 UTC
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 ;)
Comment 3 Richard Hult 2007-05-08 17:44:02 UTC
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 :).
Comment 4 Michael Gratton 2007-05-10 15:10:58 UTC
I can confirm this. It started happening after upgrading to Ubuntu Fiesty, i.e. from control-center 2.16 to 2.18.
Comment 5 Alexander Hunziker 2007-09-28 14:44:05 UTC
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...
Comment 6 Richard Hult 2007-10-30 17:43:26 UTC
*** Bug 491800 has been marked as a duplicate of this bug. ***
Comment 7 Richard Hult 2007-10-30 17:44:55 UTC
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.
Comment 8 Jan Skowron 2008-06-11 15:26:11 UTC
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
Comment 9 Chris Conway 2008-06-20 23:35:30 UTC
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
Comment 10 Richard Hult 2008-06-24 07:46:49 UTC
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.

Comment 11 Richard Hult 2008-07-28 09:37:53 UTC
*** Bug 545113 has been marked as a duplicate of this bug. ***
Comment 12 David Hugh-Jones 2008-09-14 12:24:38 UTC
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.
Comment 13 David Hugh-Jones 2008-09-19 22:56:26 UTC
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.
Comment 14 Richard Hult 2008-09-20 07:39:52 UTC
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.

Comment 15 David Hugh-Jones 2008-09-20 11:44:25 UTC
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?

Comment 16 David Hugh-Jones 2008-09-20 12:04:22 UTC
Created bug #552994 
Comment 17 Jens Granseuer 2008-09-29 19:29:44 UTC
*** Bug 554265 has been marked as a duplicate of this bug. ***
Comment 18 Jan Skowron 2009-04-03 19:31:45 UTC
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. 
Comment 19 Jens Granseuer 2009-05-26 14:50:54 UTC
*** Bug 583886 has been marked as a duplicate of this bug. ***
Comment 20 Hamish Downer 2009-07-02 20:43:03 UTC
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
Comment 21 Jan Skowron 2009-09-09 06:59:01 UTC
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.
Comment 22 Nathaniel Smith 2009-09-27 04:04:16 UTC
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.
Comment 23 Nathaniel Smith 2009-09-27 04:13:51 UTC
Created attachment 144099 [details] [review]
proposed fix
Comment 24 Nathaniel Smith 2009-11-07 10:23:18 UTC
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.
Comment 25 Jens Granseuer 2009-11-07 13:49:15 UTC
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.
Comment 26 Nathaniel Smith 2009-11-10 05:42:04 UTC
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.
Comment 27 Jens Granseuer 2009-11-11 18:49:29 UTC
(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 28 Jens Granseuer 2009-11-11 18:52:10 UTC
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
Comment 29 Nathaniel Smith 2009-11-11 19:47:30 UTC
Thanks, and no worries.
Comment 30 Jens Granseuer 2009-11-20 11:59:45 UTC
*** Bug 602491 has been marked as a duplicate of this bug. ***
Comment 31 Jack Leigh 2010-01-11 14:42:33 UTC
It says it's fixed but this is still an issue for me in Ubuntu Karmic
Comment 32 Nathaniel Smith 2010-01-12 03:21:13 UTC
(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.