Bug 381173

Summary: Wrong countdown speed in timer plasmoid's window.
Product: [Plasma] kdeplasma-addons Reporter: Kott <kv>
Component: timerAssignee: Plasma Bugs List <plasma-bugs>
Status: RESOLVED FIXED    
Severity: major CC: bugseforuns, devel, kde, kde, kossebau, mariusz.g.mazur, nate, rikmills, rknv7, spearhead2k3
Priority: NOR    
Version: unspecified   
Target Milestone: ---   
Platform: openSUSE   
OS: Linux   
Latest Commit: Version Fixed In: 5.12.6

Description Kott 2017-06-14 01:20:03 UTC
Steps:

1. Add the timer widget (desktop or panel).
2. Assign hotkey to the widget.
3. Select desired time.
4. Press hotkey, big window with digits appears.

Result:

Timer counting down at double speed.
Comment 1 Kai Uwe Broulik 2017-06-14 05:35:44 UTC
I have seen this in another project, seems QtQuick Timer isn't that reliable. Should probably be rewritten to use a QElapsedTimer.
Comment 2 Natalia 2017-06-14 19:18:29 UTC
I have the same problem at openSUSE 42.2.
Comment 3 Mariusz Mazur 2018-04-24 11:56:17 UTC
Two days left to official ubuntu 18.04 release and this bug is present in kde.
Comment 4 Mariusz Mazur 2018-04-24 14:03:08 UTC
QtQuick's Timer became unreliable and is notifying the plasmoid that a second has passed twice each time it passes. I'll rewrite the handling to make sure that's no longer the issue.

(And while I'm at it, I'll fix the precision, cause the timer plasmoid really isn't precise, which I wasn't ware of before.)
Comment 5 David Edmundson 2018-04-24 14:07:27 UTC
*If* there is a Qt bug if you can make a tiny test we should submit a report to Qt as well as any workarounds.
Comment 6 Mariusz Mazur 2018-04-24 14:45:05 UTC
Ok, so I've rewritten the timer logic and now the plasmoid is (much more) precise independently of issues with Qt. And fixed a few other bugs while I was at it.

So first order of business is getting that code merged. Dealing with the Qt bug, well, isn't really my priority, considering that (a) I don't want to spend the time on learning how to build qt things and (b) if it's not just the qt timer itself, but it interacting with something else in the plasmoid, then it could take a while to pinpoint a test case. Especially for someone who has no idea how to write plasmoids.

If anybody in here has a qt dev env and the know-how to use it to do a quick test, then the offending code I'd start with would look something like this:

Timer {
  id: t;
  interval: 1000;
  onTriggered: {
    var ts = new Date().getTime();
    console.log(ts);
  }
  repeat: true;
  running: true;
}

What should happen is a log output roughly every second. What instead happened in my debugging of the plasmoid was two log outputs every second, a few milliseconds apart.

Which nicely explains why the plasmoid on my ubuntu 17.10 worked fine, and then got the double boost when I upgraded to 18.04, all the while the code of the plasmoid itself has not changed one bit. Only possible conclusion: the problem must be in the underlying libraries.
Comment 7 Mariusz Mazur 2018-04-25 22:25:47 UTC
Fixes for this bug (and probably 391634) can be found here: https://github.com/mmazur/kdeplasma-addons/tree/timerfixes

Anybody willing to get them merged has my blessing. I'll probably try to do it eventually, but likely not anytime soon, since that phabricator thing does not look overly inviting to someone trying to quickly drop a few lines of patches.
Comment 8 Nate Graham 2018-04-25 22:43:18 UTC
It's actually pretty easy, and I'd be happy to walk you through it. The documentation is at https://community.kde.org/Infrastructure/Phabricator

Submitting via the web interface is literally a 5-minute process.
Comment 9 Mariusz Mazur 2018-04-26 08:53:57 UTC
Ok, found a spare hour to figure this out (courtesy of car mechanic having scheduling problems). All patches submitted, including the one that fixes this issue (D12536).
Comment 10 Nate Graham 2018-04-26 14:22:48 UTC
Thank you!
Comment 11 David 2018-05-09 13:49:22 UTC
I got the same problem - timer skips uneven numbers.
Comment 12 David 2018-05-09 14:01:34 UTC
same problem
Comment 13 Mariusz Mazur 2018-05-09 14:05:57 UTC
Upstream reviewer went silent two weeks ago without making a decision (https://phabricator.kde.org/D12536), so anybody interested in having this applet working can patch it themselves (no rebuild needed), though out of the box it'll remain broken for the foreseeable future.
Comment 14 Nate Graham 2018-05-09 14:10:49 UTC
(In reply to Mariusz Mazur from comment #13)
> Upstream reviewer went silent two weeks ago without making a decision
> (https://phabricator.kde.org/D12536), so anybody interested in having this
> applet working can patch it themselves (no rebuild needed), though out of
> the box it'll remain broken for the foreseeable future.

There are outstanding change requests for you. We appreciate the patch for sure, but the purpose of the review system is to allow patch submitters to receive and act on feedback from reviewers. It's not just a rubber-stamp. If you're interested in your patch being accepted, you'll need to act on David's change requests.
Comment 15 Mariusz Mazur 2018-05-09 14:29:41 UTC
My last comment contains a proposal to work around the issue pointed out by the reviewer. And I end with: "(…)  I'm waiting to know how you'd like to proceed on this patch and the other one.".
Comment 16 Nate Graham 2018-05-09 22:58:44 UTC
A friendly ping with a "How would you like me to proceed?" might do the trick.
Comment 17 Peter Mühlenpfordt 2018-05-10 12:14:46 UTC
I don't think this is a bug of QtQuick `Timer`. IMO the problem is a duplicate
creation of `TimerView` and therefore duplicate timer trigger.
If I remove/comment out e.g. the second (main.qml, line 60):
`Plasmoid.fullRepresentation: TimerView { }`
the counter works correct. Adding a third `TimerView`, the counter jumps three
seconds at once.
I'm not familiar with QtQuick so I'm not sure what the best solution could be,
maybe moving the timer from `TimerView` to `main`?
Comment 18 Nate Graham 2018-05-23 14:06:39 UTC
Peter, since the original patch seems to have stalled, would you like to submit one based on your findings so far?
Comment 19 Friedrich W. H. Kossebau 2018-05-23 15:22:51 UTC
Peter's observation had been good, agree that this is the issue

Patch up for fixing this issue at https://phabricator.kde.org/D13065
Comment 20 Friedrich W. H. Kossebau 2018-05-23 15:42:05 UTC
Git commit 6ce58880151dad9e0adb3607087de12bca65a6cc by Friedrich W. H. Kossebau.
Committed on 23/05/2018 at 15:41.
Pushed by kossebau into branch 'Plasma/5.12'.

[Timer applet] Fix double speed countdown & commands run multiple times

Summary:
Both the compact and the fullpresentation variants of the applet had
an instance of a timer, and both were connected to the central "running"
flag. So if both variants had been created, there were 2 timers triggering
the decreasing of the seconds, and also firing off the command once done.

Moving the countdown timer (and at the same time also a timer for
delayed saving of the state, which had the same issue) to the central root
item fixes this.
FIXED-IN: 5.12.6

Test Plan:
Applet still works as before on panel and background pane, now does proper
countdown in both cases.

Reviewers: #plasma, davidedmundson

Reviewed By: #plasma, davidedmundson

Subscribers: davidedmundson, plasma-devel

Tags: #plasma

Differential Revision: https://phabricator.kde.org/D13065

M  +0    -43   applets/timer/package/contents/ui/TimerView.qml
M  +41   -0    applets/timer/package/contents/ui/main.qml

https://commits.kde.org/kdeplasma-addons/6ce58880151dad9e0adb3607087de12bca65a6cc