Stop relying on QtQuicks Timer type to be precise and instead
use system clock. Also makes the applet resistant to weird Timer
bugs like:
BUG: 381173
Details
Details
- Reviewers
davidedmundson - Group Reviewers
Plasma
Diff Detail
Diff Detail
- Repository
- R114 Plasma Addons
- Branch
- timer3 (branched from master)
- Lint
No Linters Available - Unit
No Unit Test Coverage
| applets/timer/package/contents/ui/TimerView.qml | ||
|---|---|---|
| 54 | that's a lot of wakeups. Do you really need to do this? | |
| 56 | There's a behavioural change not mentioned in the commit message :/ If I have a timer for 5 minutes and suspend after 1 minute for an hour in the old code when we resume we will see a timer for 4 minutes left. In the new code we will see the timer has finished. You can make an argument this is better, but it shouldn't be accidental. *if* we want this behaviour the clock dataengine might be a good solution, that aligns itself to seconds automatically. | |
| applets/timer/package/contents/ui/TimerView.qml | ||
|---|---|---|
| 54 | Typical display is 60Hz, so 16.6 ms per frame. A 20ms interval means each clock tick will be displayed between 58-62 frames after the previous one, though usually 59-61. That's 966.8-1033.2ms delay between each tick. I'm pretty sure that's not noticeable to a human, even including any additional random delays caused by all the display logic. But If I increase it too much, it will become noticeable at some point. I guess I can change that to, say 25ms, while still being in the 58-62 range, but I really don't know what difference that 20Hz vs 25Hz makes on a 1GHz cpu? | |
| 56 | I wasn't aware that Date().getTime() wasn't in UTC. I can change it to a UTC call if there is one. I'm aware of the sleep behavior change, however I do not know how to handle it. (I was hoping the reviewer wouldn't notice.) If there's a trivial way to bind to signals for 'system is suspending'/'system just woke up', then I can pause the timer on those signals. What I cannot do is rewrite the code to use QSomethingOrOther, since I don't know Qt and I'm not going to learn Qt. Just so we're on the same page – nobody cared about this plasmoid for a few years now. To the extent that it plain stopped working when I upgraded to ubuntu 18.04 due to bug 381173. I fixed it the way I knew how and I'm happy with my code, since I can now use the plasmoid. If you have a perfect solution in mind, then you either need to code it yourself or find someone else who will. As far as interacting with me goes, it's either merge my code with minor improvements here and there (I can do the sleep handling if there's a signal for it) or don't and have the thing not work. If it's the former, please tell me upfront, so we don't waste each other's time on further discussions. | |
| applets/timer/package/contents/ui/TimerView.qml | ||
|---|---|---|
| 56 | Latter, if it's the latter. | |
Comment Actions
I was hoping the reviewer wouldn't notice.
:/
Had you said it explicitly I probably would have been fine with it.
As far as interacting with me goes, it's either merge my code with minor improvements here and there
I understand your time is limited, but lets not approach things as effectively threats.
I wasn't aware that Date().getTime() wasn't in UTC. I can change it to a UTC call if there is one.
Edit, I was wrong. On closer checking this will work.
Comment Actions
I'm operating on the assumption that working code with some changes to behavior > non-working code. Consequently I was trying to get my patches merged above any other considerations and with minimal time investment on my part. (Granted, I would have done things a bit differently knowing what I know now. :)
Though come to think of it, there is a simple workaround I could add: if the time jump exceeds 60 seconds, assume the system was asleep for long enough, that the timer should be treated as paused. Still – it's a workaround and not a proper fix.
As far as interacting with me goes, it's either merge my code with minor improvements here and there
I understand your time is limited, but lets not approach things as effectively threats.
Considering time constraints, my goal was to concisely communicate my priorities and limitations. Now that you understand those, I'm waiting to know how you'd like to proceed on this patch and the other one.
Comment Actions
I think a C++ part should be added using QElapsedTimer and then a display update timer