Re: Review Request 126078: [OS X] modernising the KIdleTime plugin (WIP!)

[René J . V . Bertin]

On Thursday November 19 2015 19:15:39 René J.V. Bertin wrote:

> ... Grand Central Dispatch ...

https://github.com/RJVB/KIdleTiming/commit/9514bf13b1d921e481fa6074d08fef58139dd986

Deliciously imprecise for the amateurs: almost 10% late almost systematically 
for the first 5000ms timeout of the example (against on the order of 1% for the 
alternative using a ballistic timer).

R.
___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

Re: Review Request 126078: [OS X] modernising the KIdleTime plugin (WIP!)

[Valorie Zimmerman]

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/126078/#review88573
---


Hello folks, Martin Graesslin has asked the CWG to step in here. This is 
Valorie writing. I can't comment on the technical aspects here, but I'm 
disappointed in the way this was handled. The review process is really 
important to the entire KDE community. We share our proposals, ask for 
feedback, then engage in dialog. This is *thinking together.* Instead, I'm 
seeing defensiveness, exasperation, and finally anger.

Please, everyone step back, calm down, and let's reconsider the issue afresh. 
Surely we can do a better job in providing great software to our users.

- Valorie Zimmerman


On Nov. 18, 2015, 4:35 p.m., René J.V. Bertin wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/126078/
> ---
> 
> (Updated Nov. 18, 2015, 4:35 p.m.)
> 
> 
> Review request for KDE Software on Mac OS X, KDE Frameworks and Dario Freddi.
> 
> 
> Repository: kidletime
> 
> 
> Description
> ---
> 
> I noticed that the KIdleTime example doesn't work properly on OS X, and that 
> the plugin for OS X still uses the deprecated Carbon-based algorithm that I 
> already patched for KDE4.
> 
> This patch is a work-in-progress (hence the qDebugs) update to use IOKit, 
> IORegistry and CoreServices to do idle-time calculation as it should be done, 
> and allow simulated user activity through a "less deprecated" function.
> 
> 
> Diffs
> -
> 
>   src/plugins/osx/CMakeLists.txt e1b50b8 
> 
> Diff: https://git.reviewboard.kde.org/r/126078/diff/
> 
> 
> Testing
> ---
> 
> On OS X 10.9 with Qt 5.5.1 and frameworks 5.16.0 .
> 
> The example now works: when I set a QTimer with interval==0, the expected 
> wait for user input (`resumingFromIdle` signal) works. However, I am getting 
> a `stopCatchingIdleEvents` signal which means the application waits forever, 
> without ever getting to compare idle time to the list of timeouts.
> I haven't been able to figure out where that signal comes from, nor why this 
> doesn't happen on Linux.
> 
> Surely I'm missing something, but what?
> 
> 
> Thanks,
> 
> René J.V. Bertin
> 
>

___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

Re: Review Request 126078: [OS X] modernising the KIdleTime plugin (WIP!)

[René J . V . Bertin]

On Thursday November 19 2015 08:10:08 Martin Graesslin wrote:

> I want to apologize for saying that. Please be aware that this was only 
> intended as a description of code (yes I also call code written by myself 
> idiotic and worse things) and not against you.

Apology accepted, but please realise for the future that calling code names 
written by someone who has clearly shown he believes in his way of doing things 
will inevitably be taken personnally.

> I hope you don't fork, because I think that would be bad. Not because you 
> move 

Apologies if my use of the word "mirror" suggested that. I'm not intending to 
fork, but I do intend to provide my patch through the MacPorts mechanism, in 
the form that takes into account almost all feedback given, and with 
documentation updated to reflect those changes and what the framework really 
does and the potential side-effects its underlying implementation might have. 
(It doesn't guarantee that the "msec" value in the timeoutReached signal will 
be exactly the requested timeout, for instance, because it can only guarantee 
what the platform backend can make true).

> the code else where, but because the issues pointed out would be unresolved 
> giving users of your code a bad framework and thus harming both your users as 
> well as the frameworks project for distributing bad code. The problem of 
> polling is real. Please accept this feedback.

Now it's just bad code, eh?

Just for the record: no one but Apple really knows what IOKit considers being 
idle but it clearly involves absence of user input events. That's "user idle", 
not "system idle" (term from the KIdleTime docs!), and how long of the former 
you need to attain the latter (or something you consider "system idle") is 
completely up to the software to decide. Because it involves event timing, this 
can be determined with as much precision as native events can be timed (and if 
I'm not mistaken those are accurate enough to support a 1ms resolution on OS X).

I'm not going to repeat my position on polling or on how it's up to the user to 
decide whether or not idle timeout detection is too expensive.
As long as people here remain dead-set against anything the uses it without 
even testing the actual implementation, then I am not going to reconsider 
contributing to this code. Writing great software doesn't necessarily mean 
applying all applicable principles without wiggle room (and I'd hope that's 
about the only thing I'm dogmatic about myself :)).

Apparently I might want to reconsider using QTimer, though, there might be more 
accurate and/or less intrusive mechanisms available natively - and now that I 
incorporated some ObjC it ought to be a lot less work to experiment with those. 

R.

___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

Re: Review Request 126078: [OS X] modernising the KIdleTime plugin (WIP!)

[Nicolás Alvarez]

> On Nov. 18, 2015, 4:22 a.m., Martin Gräßlin wrote:
> > src/plugins/osx/macpoller.cpp, lines 222-227
> > 
> >
> > seriously? You care about idle timeouts below 5 msec? This is a 
> > framework to tell the application whether the user doesn't use input 
> > devices. I don't know how fast you type, but I'm relatively certain that it 
> > takes more than 5 msec to move my finger from one key to another. What 
> > exactly is that you want to detect here? An event after each key press, 
> > maybe even between a key press and a key release? Because that's probably 
> > already more about 5 msec.
> 
> René J.V. Bertin wrote:
> The timer is used only for the detection of idle timeouts (so *absence* 
> of user input), and the precision with which it fires determines the accuracy 
> with which those events can be timed. This is currently inaccessible anyway, 
> but I don't believe in doing things half-bakedly. One might question whether 
> it makes sense to allow the framework to work at high precision (I more or 
> less agree that remains to be seen). But I don't think it makes sense to do 
> that and then leave the QTimer in a mode where the requested precision cannot 
> be met.
> 
> Martin Gräßlin wrote:
> To get this quite clear: you care about a precision of below 5 msec, this 
> is a third of the time it takes at least till the next frame is rendered. If 
> you are lucky you get the result of a key press or mouse move represented on 
> screen after 16 msec, more likely it's more. And you care about a wrong timer 
> precision below 5 msec. Sorry that's ridiculous. Please don't include such 
> non-sense code. You make that code more difficult to all other developers to 
> maintain. This code has cost if it's included.
> 
> Aleix Pol Gonzalez wrote:
> It's not a matter of doing things "right" or "wrong". It's a matter of 
> priorities.
> 
> KIdleTime is a framework for figuring out whether the system is idle. I 
> don't consider 5ms not using a system as it's being idling.
> 
> René J.V. Bertin wrote:
> Again, we're talking about detecting how long a system has been idle, not 
> whether it is (the framework is called KIdle*Time*, right? :)). The precision 
> in question here applies just as well to (very) short periods (timeouts) as 
> well as to long ones.
> Also, don't confound the 5*ms* lower limit under which I switch to a high 
> precision timer with the 5`%` error of a Qt::CoarseTimer. The precision of a 
> high precision timer may be overkill for longer timeout durations in a 
> framework that has a 1ms granularity, but the effect of 5% error scales with 
> duration.
> 
> I consider that it's up to the user of a feature to know what s/he 
> expects of it, not up to a framework. (Turning that around would probably 
> remove the raison d'être of a lot of software.)
> 
> Finally, I have not been able to measure any increase in overhead when 
> using a (single) high precision timer.
> 
> Granted, this is a bit a matter of principle for me, but as I've said 
> elsewere, I could easily see me having used this in a previous job.
> 
> Martin Gräßlin wrote:
> Rene, please stop it. Don't argue! You won't get this code in with 
> polling and setting the qtimer precision based on clearly irrelevant value. 
> It is our responsibility as fellow KDE developers to prevent such code to get 
> it. I absolutely don't care how you intend to use this code, but this is a 
> user idle framework. A system which had an input event in the last five milli 
> seconds is not idle per definition. Thus the code is idiotic and wrong. 
> Please stop arguing and change the code given the feedback you get here. Same 
> above for the nullptr check. That one has to go, it's a wrong check at that 
> place, remove it.

FYI, while you fight and argue about the best way to do high-precision frequent 
polling on something that doesn't need any precision, I'm waist-deep in a 
disassembler making good progress figuring out how to do this without any 
polling at all.


- Nicolás


---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/126078/#review88509
---


On Nov. 18, 2015, 1:35 p.m., René J.V. Bertin wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/126078/
> ---
> 
> (Updated Nov. 18, 2015, 1:35 p.m.)
> 
> 
> Review request for KDE Software on Mac OS X, KDE Frameworks and Dario Freddi.
> 
> 
> Repository: kidletime
> 
> 
> Description
> ---
> 
> I noticed that the KIdleTime example doesn't work properly on OS X, and that 
> the plugin for OS X still uses the deprecated 

Re: Review Request 126078: [OS X] modernising the KIdleTime plugin (WIP!)

[René J . V . Bertin]

> Again - this is not against IOKit or what Apple does, but your usage
> of QTimer in your code. RRs are meant for constructive feedback, and

Oh, I can use Grand Central Dispatch if that makes you feel better, but in the 
end something will have to check at certain times of interest whether indeed no 
events have arrived for the period of interest and it is about certain that 
XSync alarms do just that.

I'm beginning to wonder if anyone actually tried to understand what my code 
really does, though. It seems I got myself on the wrong track at some point, 
and thought up a need to use an adaptive polling interval where is probably no 
need at all to check the idle state ahead of the period of interest. In any 
case not since there's an event handler that can serve to reset the timeout 
measurement.


R.
___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

Re: Review Request 126078: [OS X] modernising the KIdleTime plugin (WIP!)

[Boudhayan Gupta]

> On Nov. 18, 2015, 12:52 p.m., Martin Gräßlin wrote:
> > src/plugins/osx/macpoller.cpp, lines 222-227
> > 
> >
> > seriously? You care about idle timeouts below 5 msec? This is a 
> > framework to tell the application whether the user doesn't use input 
> > devices. I don't know how fast you type, but I'm relatively certain that it 
> > takes more than 5 msec to move my finger from one key to another. What 
> > exactly is that you want to detect here? An event after each key press, 
> > maybe even between a key press and a key release? Because that's probably 
> > already more about 5 msec.
> 
> René J.V. Bertin wrote:
> The timer is used only for the detection of idle timeouts (so *absence* 
> of user input), and the precision with which it fires determines the accuracy 
> with which those events can be timed. This is currently inaccessible anyway, 
> but I don't believe in doing things half-bakedly. One might question whether 
> it makes sense to allow the framework to work at high precision (I more or 
> less agree that remains to be seen). But I don't think it makes sense to do 
> that and then leave the QTimer in a mode where the requested precision cannot 
> be met.
> 
> Martin Gräßlin wrote:
> To get this quite clear: you care about a precision of below 5 msec, this 
> is a third of the time it takes at least till the next frame is rendered. If 
> you are lucky you get the result of a key press or mouse move represented on 
> screen after 16 msec, more likely it's more. And you care about a wrong timer 
> precision below 5 msec. Sorry that's ridiculous. Please don't include such 
> non-sense code. You make that code more difficult to all other developers to 
> maintain. This code has cost if it's included.
> 
> Aleix Pol Gonzalez wrote:
> It's not a matter of doing things "right" or "wrong". It's a matter of 
> priorities.
> 
> KIdleTime is a framework for figuring out whether the system is idle. I 
> don't consider 5ms not using a system as it's being idling.
> 
> René J.V. Bertin wrote:
> Again, we're talking about detecting how long a system has been idle, not 
> whether it is (the framework is called KIdle*Time*, right? :)). The precision 
> in question here applies just as well to (very) short periods (timeouts) as 
> well as to long ones.
> Also, don't confound the 5*ms* lower limit under which I switch to a high 
> precision timer with the 5`%` error of a Qt::CoarseTimer. The precision of a 
> high precision timer may be overkill for longer timeout durations in a 
> framework that has a 1ms granularity, but the effect of 5% error scales with 
> duration.
> 
> I consider that it's up to the user of a feature to know what s/he 
> expects of it, not up to a framework. (Turning that around would probably 
> remove the raison d'être of a lot of software.)
> 
> Finally, I have not been able to measure any increase in overhead when 
> using a (single) high precision timer.
> 
> Granted, this is a bit a matter of principle for me, but as I've said 
> elsewere, I could easily see me having used this in a previous job.
> 
> Martin Gräßlin wrote:
> Rene, please stop it. Don't argue! You won't get this code in with 
> polling and setting the qtimer precision based on clearly irrelevant value. 
> It is our responsibility as fellow KDE developers to prevent such code to get 
> it. I absolutely don't care how you intend to use this code, but this is a 
> user idle framework. A system which had an input event in the last five milli 
> seconds is not idle per definition. Thus the code is idiotic and wrong. 
> Please stop arguing and change the code given the feedback you get here. Same 
> above for the nullptr check. That one has to go, it's a wrong check at that 
> place, remove it.
> 
> Nicolás Alvarez wrote:
> FYI, while you fight and argue about the best way to do high-precision 
> frequent polling on something that doesn't need any precision, I'm waist-deep 
> in a disassembler making good progress figuring out how to do this without 
> any polling at all.

Talk is cheap, show me the code!

**wink**


- Boudhayan


---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/126078/#review88509
---


On Nov. 18, 2015, 10:05 p.m., René J.V. Bertin wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/126078/
> ---
> 
> (Updated Nov. 18, 2015, 10:05 p.m.)
> 
> 
> Review request for KDE Software on Mac OS X, KDE Frameworks and Dario Freddi.
> 
> 
> Repository: kidletime
> 
> 
> Description
> ---
> 
> I noticed that the KIdleTime example 

Re: Review Request 126078: [OS X] modernising the KIdleTime plugin (WIP!)

[René J . V . Bertin]

On Thursday November 19 2015 15:21:10 Dario Freddi wrote:

> * KIdleTime strives to be as lightweight as possible. This is something
>which came from the old approach used in KDE3/4 powermanagement which was
...
>resource (aka: the power manager) to take care of this, but it was deemed
>crazy to bring in such complexity for those applications which might care

That kind of application simply doesn't make sense on operating systems that 
already have their own, built-in power management...

OTOH, I wouldn't be amazed if it is non-trivial to guarantee good and reliable 
timing accuracy on an OS like Linux: that would fit in with the experience I 
have with its use in time-critical applications (where it always required 
realtime extensions to the kernel).
But I continue the think that a simple framework like this can very well 
attempt to provide the best accuracy/cost compromise that the host allows, esp. 
if that doesn't mean adding crazy complexity. As it happens, OS X returns the 
"idle time" in *nano* seconds; do not think for a second (pun intended) that it 
would do that if the underlying measurement didn't have that kind of accuracy. 
I think that makes it safe to say that my current implementation indeed 
provides just short of a 1ms accuracy, with a CoarseTimer and an adaptive 
polling interval that spends most of the timeout duration waiting for the timer 
to fire.

It's quite possible that's enough, and that there will never be any reason 
(demand) to export the interval control I drafted. But consider this: KF5 
frameworks are young (despite the almost crazy progression of the release 
number, which for me is a telltale sign of immaturity). They are also hardly 
(nor have they been) used beyond Linux, and in practice not at all on OS X. Who 
knows what uses any framework might be considered for that wasn't really part 
of its original mission statement? If that's always going to elicit a rejecting 
attitude, I'll be better off going back to making the minimum modifications 
required to make things work for MacPorts, and only request inclusion if it 
turns out maintaining those changes requires too much continuous effort.

As to use over a remote, non-X11 connection: applications should not attempt to 
determine idle time in that case. They shouldn't receive return values that 
suggest 0 idle time, they should receive an error that tells them they're out 
of bounds. If Qt provided a way to raise an error to the user (and force the 
app to exit) without provoking a crash (abort), I'd use that, but I'm not aware 
of any such facility. As it is, the OS X implementation will probably signal 
the actual idle time of whomever is logged in locally ...

R.
___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

Re: Review Request 126078: [OS X] modernising the KIdleTime plugin (WIP!)

[Boudhayan Gupta]

Hi all,

On 19 November 2015 at 15:06, René J.V.  wrote:
> On Thursday November 19 2015 08:10:08 Martin Graesslin wrote:
>> the code else where, but because the issues pointed out would be unresolved
>> giving users of your code a bad framework and thus harming both your users as
>> well as the frameworks project for distributing bad code. The problem of
>> polling is real. Please accept this feedback.
>
> Now it's just bad code, eh?

Thou shalt not poll.

This is for one reason and one reason only - polling causes wakeups,
which consumes both power and processor resources. You're running
warmer, slower and consuming more energy. Microwatts count, because
they add up.

The irony here is that you're polling in a library with the purpose of
increasing power usage efficiency.

That said, here's a concrete way for you to detect both Idletime and
catch resume events:

For idletime, run a QTimer with the interval set to the idle timeout.
Poll the HID time since last event twice - once at timer start and
once at timer end. Check the difference. If it's round about the same
as the timer difference, the system has been idle. Trigger the idle
signal. If the difference is significantly less, adjust the QTimer
interval to check again at a shorter interval - the time at which you
expect the HID time since last event to be the expected value.
Basically, do some math, keep adjusting the timeout value everytime
you reach the timeout.

Now once you're in idle mode, you'll need some way to resume from
idle. This needs a high-precision event source. Not a timer, an event
source. For this, get a keylogger, like the one here -
https://github.com/caseyscarborough/keylogger/blob/master/keylogger.c
- and adjust it so that it responds to key as well as mouse events.
Instead of actually logging the key though, make it emit a tick
through a socket pair or a named pipe, with KIdleTime on the listening
end. Or just use QProcess - start the process when you enter idle, end
the process on the first instance you get some output from the stdout,
which is in response to an input event.

Let's hope this helps you out.

-- Boudhayan
___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

Re: Review Request 126078: [OS X] modernising the KIdleTime plugin (WIP!)

[Lamarque Souza]

> On Nov. 17, 2015, 10:58 p.m., Lamarque Souza wrote:
> > src/plugins/osx/macpoller.h, line 24
> > 
> >
> > Nitpick: this should go after #include 
> 
> René J.V. Bertin wrote:
> Any guidelines that dictate this?

https://wiki.qt.io/Coding_Conventions there will be more guidelines in 
https://techbase.kde.org/Policies/Frameworks_Coding_Style, until there please 
use Qt's code conventions.


> On Nov. 17, 2015, 10:58 p.m., Lamarque Souza wrote:
> > src/plugins/osx/macpoller.cpp, line 178
> > 
> >
> > Use qCWarning instead.
> 
> René J.V. Bertin wrote:
> I presume that should be using KIDLETIME for the category?
> 
> Isn't it possible to get the `KIDLETIME()` symbol through libKF5KIdleTime 
> rather than having to pull in `../../logging.cpp`?

Read https://community.kde.org/Frameworks/Porting_To_qCDebug for more 
information on how to define KIDLETIME.


> On Nov. 17, 2015, 10:58 p.m., Lamarque Souza wrote:
> > src/plugins/osx/macpoller.cpp, line 213
> > 
> >
> > Shouldn't you check the return value of this method?
> 
> René J.V. Bertin wrote:
> As far as I can see all it can be used for is to print a warning, right?

I guess so, since the "additional" prefix implies that it is not required.


> On Nov. 17, 2015, 10:58 p.m., Lamarque Souza wrote:
> > src/plugins/osx/macpoller.cpp, line 215
> > 
> >
> > This should be changed to "return true", right?
> 
> René J.V. Bertin wrote:
> Yeah, that seems logical, but I don't see any documentation on what 
> setupPoller() should return. Then again the upstream code doesn't appear to 
> use the return value anyway so the question is a bit moot ...

The point is that you changed the semantics of this method. The original code 
returns true on all successful setUpPoller() calls. Now it returns false on the 
first successfull setUpPoller() call and returns true only on the second call 
and on. Besides, this is a framework, do not assume that there will always be 
just one "upstream" for a framework.


> On Nov. 17, 2015, 10:58 p.m., Lamarque Souza wrote:
> > src/plugins/osx/macpoller.cpp, line 422
> > 
> >
> > Remove trailing space and shouldn't the commented code in this method 
> > be removed?
> 
> René J.V. Bertin wrote:
> I put the commented code as a reference for alternative approaches that 
> could be investigated once more if ever `updateSystemActivity` is removed. If 
> there's a policy against such comments I can also store the snippets in a 
> separate file, but that means adding one. What's preferable?

Well, it is a general consensus that adding commented code is not a good 
programming practice. I am not aware of any written recomendation in Frameworks 
or Qt code conventions about that but I have never seen a patch with commented 
code to pass any code review either.


> On Nov. 17, 2015, 10:58 p.m., Lamarque Souza wrote:
> > src/plugins/osx/macpoller_helper.mm, line 78
> > 
> >
> > There is no point in doing this if the next line will delete 
> > nativeGrabber.
> 
> René J.V. Bertin wrote:
> I've learned to be fool-proof with this kind of thing (I don't trust 
> `delete` to zero memory before releasing it; there's no 
> `~CocoaEventFilter()`) but what does make it redundant here is setting 
> `m_nativeGrabber = 0` just after deleting `nativeGrabber`.

Delete never zeros memory before releasing it for performance reasons. The 
overhead of zeroing m_monitorId here is minimum though. Anyway nobody will be 
able to access m_monitorId after the next line or the program will crash.


> On Nov. 17, 2015, 10:58 p.m., Lamarque Souza wrote:
> > src/plugins/osx/macpoller_helper.mm, line 54
> > 
> >
> > Usually when a new operation returns 0 it is because system is on short 
> > on RAM memory (or memory is too fragmented). I would add assert here 
> > instead of silently ignoring the failure to allocate memory.
> 
> René J.V. Bertin wrote:
> And I would argue that it is up to the calling software to decide how to 
> react to a failure to set up idle detection: in this case it's only the 
> "resume from idle" functionality that can no longer work. The idle timeout 
> detection feature should still be able to function (possibly by adding some 
> kind of reset when the calculated idle time returns to 0).
> Asserting would mean that the application aborts, which I find a bit 
> overkill for a situation that could be handled a lot more elegantly. That 
> 

Re: Review Request 126078: [OS X] modernising the KIdleTime plugin (WIP!)

[René J . V . Bertin]

There's something else that's been bothering me since I realised it: KIdleTime 
has been written with the assumption that'll always work, and that whatever 
initialisation is required never fails. That must be why the return value from 
setupPoller() is discarded.

Maybe this all holds true on Linux, where initialisation probably only fails if 
there's no valid connection to an X server or a severe out-of-memory condition. 
The former will lead to sufficient explicative feedback to the user, the 
latter, well, it'd still be nice to simply print a message and then exit 
without triggering crash reporting stuff that's only going to increase the 
system load.

On OS X, there is a very valid situation in which initialisation could (or 
should) fail and that cannot be predicted from something like the absence of an 
env. variable like DISPLAY : remote login. That's a more generic problem that 
affects Qt applications in general, though.

Anyway, I don't see anything in the documentation that suggests that 
applications can verify whether any of the KIdleTime features are going to work 
(= are implemented). Idle time is reported as 0 when there is no platform 
plugin (which is a valid time, not an error value), or whatever the platform 
plugin returns if it cannot determine a sensible value. The other, signalling 
features probably just fail to deliver signals if they are not supported. I 
haven't seen a warning for that in the documentation, so that could lead to 2 
things:

- either dependent software just never gets to do the (possibly important) 
things it is designed to do when an idle period ends or reaches a given 
duration.
- or developers deduce that they have to write additional checks, possibly 
through polling to detect and handle situation in which KIdleTime fails to do 
its job ... but then why would they even use KIdleTime at all?

R.
___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

Re: Review Request 126078: [OS X] modernising the KIdleTime plugin (WIP!)

[Dario Freddi]

Hi René,

just thought I'd clarify some things as the creator/former maintainer of
all this - which might also justify some of Martin's positions.

The idea behind KIdleTime is that the framework shall be a lightweight,
non-critical framework for those applications which want to know about the
idle time of the system. The only critical application relying on KIdleTime
is (as expected) the power manager (which was also where the framework came
from, if you remember). As such, we work under the following assumption:

 * We don't want/need to report the idle time precisely. We care about
timeouts and an error in the order of seconds is acceptable (here comes
Martin's argument). The reason why the API returns the idle time in msec is
just a matter of consistency with Qt's API, which always return msec as the
base unit for time.
 * If an application is relying on having a msec-accurate idle time, I have
very bad news for its developer :) It's just impossible to provide
warranties that we can be that accurate on all platforms.
 * KIdleTime strives to be as lightweight as possible. This is something
which came from the old approach used in KDE3/4 powermanagement which was
basically stressing the system just to find out how long it has been idle
(the irony)


I agree with you about the lack of documentation, and it'd be great if
something put that in a better shape. We're fine to assume that KIdleTime
will be initialised with no functional backend, because we assume that if
we can't determine if the system is idle, we assume it is always active. It
is indeed not documented, and I agree with you on that, but the approach
definitely makes sense given the assumption above. It is, on the other
hand, responsibility of applications which use the idle time as a critical
resource (aka: the power manager) to take care of this, but it was deemed
crazy to bring in such complexity for those applications which might care
only about doing stuff in a more conservative way when the system is not
idle.

Also, this applies to strange setups (remote login): it is tied to a series
of limitations and, especially in the case you mentioned, the concept of an
"idle system" is definitely ambiguous, as you are dealing with... two
different systems. And if we can't give warranties about computing, we
simply don't.

That said, I really appreciate you improving the OSX backend. Thanks!


I hope to have shed some light on this.

Best



2015-11-19 14:38 GMT+01:00 René J.V. :

> There's something else that's been bothering me since I realised it:
> KIdleTime has been written with the assumption that'll always work, and
> that whatever initialisation is required never fails. That must be why the
> return value from setupPoller() is discarded.
>
> Maybe this all holds true on Linux, where initialisation probably only
> fails if there's no valid connection to an X server or a severe
> out-of-memory condition. The former will lead to sufficient explicative
> feedback to the user, the latter, well, it'd still be nice to simply print
> a message and then exit without triggering crash reporting stuff that's
> only going to increase the system load.
>
> On OS X, there is a very valid situation in which initialisation could (or
> should) fail and that cannot be predicted from something like the absence
> of an env. variable like DISPLAY : remote login. That's a more generic
> problem that affects Qt applications in general, though.
>
> Anyway, I don't see anything in the documentation that suggests that
> applications can verify whether any of the KIdleTime features are going to
> work (= are implemented). Idle time is reported as 0 when there is no
> platform plugin (which is a valid time, not an error value), or whatever
> the platform plugin returns if it cannot determine a sensible value. The
> other, signalling features probably just fail to deliver signals if they
> are not supported. I haven't seen a warning for that in the documentation,
> so that could lead to 2 things:
>
> - either dependent software just never gets to do the (possibly important)
> things it is designed to do when an idle period ends or reaches a given
> duration.
> - or developers deduce that they have to write additional checks, possibly
> through polling to detect and handle situation in which KIdleTime fails to
> do its job ... but then why would they even use KIdleTime at all?
>
> R.
>
___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

Re: Review Request 126078: [OS X] modernising the KIdleTime plugin (WIP!)

[Dario Freddi]

Given I already stepped in...

2015-11-19 10:36 GMT+01:00 René J.V. :
>
> Now it's just bad code, eh?
>
> Just for the record: no one but Apple really knows what IOKit considers being 
> idle but it clearly involves absence of user input events. That's "user 
> idle", not "system idle" (term from the KIdleTime docs!), and how long of the 
> former you need to attain the latter (or something you consider "system 
> idle") is completely up to the software to decide. Because it involves event 
> timing, this can be determined with as much precision as native events can be 
> timed (and if I'm not mistaken those are accurate enough to support a 1ms 
> resolution on OS X).
>
> I'm not going to repeat my position on polling or on how it's up to the user 
> to decide whether or not idle timeout detection is too expensive.
> As long as people here remain dead-set against anything the uses it without 
> even testing the actual implementation, then I am not going to reconsider 
> contributing to this code. Writing great software doesn't necessarily mean 
> applying all applicable principles without wiggle room (and I'd hope that's 
> about the only thing I'm dogmatic about myself :)).
>
> Apparently I might want to reconsider using QTimer, though, there might be 
> more accurate and/or less intrusive mechanisms available natively - and now 
> that I incorporated some ObjC it ought to be a lot less work to experiment 
> with those.

René, Martin was just questioning your choice of Qt::PreciseTimer. From Qt docs:

For Qt::CoarseTimer, the interval will be adjusted up to 5% to align
the timer with other timers that are expected to fire at or around the
same time. The objective is to make most timers wake up at the same
time, thereby reducing CPU wakeups and power consumption.

Which is *exactly* the purpose of KIdleTime - notifying with a relaxed
precision (because let's face it, as Martin already explained the
added precision value is completely pointless if we're talking about
**user** activity) with the minimum consumption of system resources.
And this is exactly what :CoarseTimer does. So I stand behind Martin's
comment and agree with it. It does not relate to IOKit or anything
else, but just with the purpose for which you're using QTimer in that
context. And note that people here are not dead-set without concrete
and actual reason: the new Qt5 API of QTimer was designed with these
kind of use cases in mind. The fact that (of course) you don't notice
any concrete difference might change when you scale back to a more
constrained system - and it's the purpose of frameworks to be as
efficient as possible everywhere. And most of all, there's no gain in
using PreciseTimer at all here - actually KIdleTime might even use
VeryCoarse. That's all.

In the bigger scheme: would you be happy to know that your laptop is
consuming more energy just to know more accurately when it's a good
time to suspend to save energy? :)

P.S.: On you reconsidering QTimer: the fact you might be using a
different API won't change the base problem, which you will face
anyway: having a precise timer implies consuming more resources and
having more wakeups.
___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

Re: Review Request 126078: [OS X] modernising the KIdleTime plugin (WIP!)

[Dario Freddi]

2015-11-19 17:17 GMT+01:00 René J.V. :
> On Thursday November 19 2015 15:21:10 Dario Freddi wrote:
>
>> * KIdleTime strives to be as lightweight as possible. This is something
>>which came from the old approach used in KDE3/4 powermanagement which was
> ...
>>resource (aka: the power manager) to take care of this, but it was deemed
>>crazy to bring in such complexity for those applications which might care
>
> That kind of application simply doesn't make sense on operating systems that 
> already have their own, built-in power management...
>
> OTOH, I wouldn't be amazed if it is non-trivial to guarantee good and 
> reliable timing accuracy on an OS like Linux: that would fit in with the 
> experience I have with its use in time-critical applications (where it always 
> required realtime extensions to the kernel).
> But I continue the think that a simple framework like this can very well 
> attempt to provide the best accuracy/cost compromise that the host allows, 
> esp. if that doesn't mean adding crazy complexity. As it happens, OS X 
> returns the "idle time" in *nano* seconds; do not think for a second (pun 
> intended) that it would do that if the underlying measurement didn't have 
> that kind of accuracy. I think that makes it safe to say that my current 
> implementation indeed provides just short of a 1ms accuracy, with a 
> CoarseTimer and an adaptive polling interval that spends most of the timeout 
> duration waiting for the timer to fire.
>
> It's quite possible that's enough, and that there will never be any reason 
> (demand) to export the interval control I drafted. But consider this: KF5 
> frameworks are young (despite the almost crazy progression of the release 
> number, which for me is a telltale sign of immaturity). They are also hardly 
> (nor have they been) used beyond Linux, and in practice not at all on OS X. 
> Who knows what uses any framework might be considered for that wasn't really 
> part of its original mission statement? If that's always going to elicit a 
> rejecting attitude, I'll be better off going back to making the minimum 
> modifications required to make things work for MacPorts, and only request 
> inclusion if it turns out maintaining those changes requires too much 
> continuous effort.

Again - this is not against IOKit or what Apple does, but your usage
of QTimer in your code. RRs are meant for constructive feedback, and
everyone here is striving to keep a positive attitude and explain the
reasons why such a comment has been made (speaking of which, thanks
Boudhayan for the insightful message). I really hope you will
reconsider this and accept the feedback, it would be really sad to see
MacPorts differ from upstream. As Martin said before, we're here to
help and keep code consistent, there's no interest in giving you this
kind of feedback just for the sake of it. KIdleTime is a framework
with a purpose and some requirements, if you want to build something
which can cover time-critical applications, it definitely does not
belong there.

>
> As to use over a remote, non-X11 connection: applications should not attempt 
> to determine idle time in that case. They shouldn't receive return values 
> that suggest 0 idle time, they should receive an error that tells them 
> they're out of bounds. If Qt provided a way to raise an error to the user 
> (and force the app to exit) without provoking a crash (abort), I'd use that, 
> but I'm not aware of any such facility. As it is, the OS X implementation 
> will probably signal the actual idle time of whomever is logged in locally ...
>
> R.
___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

Re: Review Request 126078: [OS X] modernising the KIdleTime plugin (WIP!)

[Luca Beltrame]

Il Wed, 18 Nov 2015 17:28:27 +0100, René J.V. Bertin ha scritto:

> If we're starting to call names arguing indeed becomes pointless as the
> apparent lack of actual reading my arguments already suggested. It

In the interest of the CoC and before people heat up, Martin said:

"Thus the code is idiotic"

so I don't see anything like name calling. Anyway, since this is likely 
off-topic, if you feel you've been treated unfairly from a non-development 
point of view, feel free to contact KDE's community working group.

___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

Re: Review Request 126078: [OS X] modernising the KIdleTime plugin (WIP!)

[René J . V . Bertin]

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/126078/
---

(Updated Nov. 18, 2015, 5:35 p.m.)


Status
--

This change has been discarded.


Review request for KDE Software on Mac OS X, KDE Frameworks and Dario Freddi.


Repository: kidletime


Description
---

I noticed that the KIdleTime example doesn't work properly on OS X, and that 
the plugin for OS X still uses the deprecated Carbon-based algorithm that I 
already patched for KDE4.

This patch is a work-in-progress (hence the qDebugs) update to use IOKit, 
IORegistry and CoreServices to do idle-time calculation as it should be done, 
and allow simulated user activity through a "less deprecated" function.


Diffs
-

  src/plugins/osx/CMakeLists.txt e1b50b8 

Diff: https://git.reviewboard.kde.org/r/126078/diff/


Testing
---

On OS X 10.9 with Qt 5.5.1 and frameworks 5.16.0 .

The example now works: when I set a QTimer with interval==0, the expected wait 
for user input (`resumingFromIdle` signal) works. However, I am getting a 
`stopCatchingIdleEvents` signal which means the application waits forever, 
without ever getting to compare idle time to the list of timeouts.
I haven't been able to figure out where that signal comes from, nor why this 
doesn't happen on Linux.

Surely I'm missing something, but what?


Thanks,

René J.V. Bertin

___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

Re: Review Request 126078: [OS X] modernising the KIdleTime plugin (WIP!)

[René J . V . Bertin]

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/126078/
---

(Updated Nov. 18, 2015, 5:35 p.m.)


Review request for KDE Software on Mac OS X, KDE Frameworks and Dario Freddi.


Changes
---

AFAIC you can just remove the whole OS X plugin, I'm not going to commit 
something I don't stand behind.


Repository: kidletime


Description
---

I noticed that the KIdleTime example doesn't work properly on OS X, and that 
the plugin for OS X still uses the deprecated Carbon-based algorithm that I 
already patched for KDE4.

This patch is a work-in-progress (hence the qDebugs) update to use IOKit, 
IORegistry and CoreServices to do idle-time calculation as it should be done, 
and allow simulated user activity through a "less deprecated" function.


Diffs (updated)
-

  src/plugins/osx/CMakeLists.txt e1b50b8 

Diff: https://git.reviewboard.kde.org/r/126078/diff/


Testing
---

On OS X 10.9 with Qt 5.5.1 and frameworks 5.16.0 .

The example now works: when I set a QTimer with interval==0, the expected wait 
for user input (`resumingFromIdle` signal) works. However, I am getting a 
`stopCatchingIdleEvents` signal which means the application waits forever, 
without ever getting to compare idle time to the list of timeouts.
I haven't been able to figure out where that signal comes from, nor why this 
doesn't happen on Linux.

Surely I'm missing something, but what?


Thanks,

René J.V. Bertin

___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

Re: Review Request 126078: [OS X] modernising the KIdleTime plugin (WIP!)

[René J . V . Bertin]

On Wednesday November 18 2015 15:01:59 Martin Gräßlin wrote:

> idiotic

If we're starting to call names arguing indeed becomes pointless as the 
apparent lack of actual reading my arguments already suggested. It leaves me 
with no other choice but to discard the RR and put up a mirror in place.

R
___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

Re: Review Request 126078: [OS X] modernising the KIdleTime plugin (WIP!)

[René J . V . Bertin]

> On Nov. 18, 2015, 8:22 a.m., Martin Gräßlin wrote:
> > src/plugins/osx/macpoller.cpp, lines 222-227
> > 
> >
> > seriously? You care about idle timeouts below 5 msec? This is a 
> > framework to tell the application whether the user doesn't use input 
> > devices. I don't know how fast you type, but I'm relatively certain that it 
> > takes more than 5 msec to move my finger from one key to another. What 
> > exactly is that you want to detect here? An event after each key press, 
> > maybe even between a key press and a key release? Because that's probably 
> > already more about 5 msec.

The timer is used only for the detection of idle timeouts (so *absence* of user 
input), and the precision with which it fires determines the accuracy with 
which those events can be timed. This is currently inaccessible anyway, but I 
don't believe in doing things half-bakedly. One might question whether it makes 
sense to allow the framework to work at high precision (I more or less agree 
that remains to be seen). But I don't think it makes sense to do that and then 
leave the QTimer in a mode where the requested precision cannot be met.


- René J.V.


---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/126078/#review88509
---


On Nov. 17, 2015, 10:13 p.m., René J.V. Bertin wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/126078/
> ---
> 
> (Updated Nov. 17, 2015, 10:13 p.m.)
> 
> 
> Review request for KDE Software on Mac OS X, KDE Frameworks and Dario Freddi.
> 
> 
> Repository: kidletime
> 
> 
> Description
> ---
> 
> I noticed that the KIdleTime example doesn't work properly on OS X, and that 
> the plugin for OS X still uses the deprecated Carbon-based algorithm that I 
> already patched for KDE4.
> 
> This patch is a work-in-progress (hence the qDebugs) update to use IOKit, 
> IORegistry and CoreServices to do idle-time calculation as it should be done, 
> and allow simulated user activity through a "less deprecated" function.
> 
> 
> Diffs
> -
> 
>   src/plugins/osx/CMakeLists.txt e1b50b8 
>   src/plugins/osx/macpoller.h ef51ea5 
>   src/plugins/osx/macpoller.cpp ad9c10f 
>   src/plugins/osx/macpoller_helper.mm PRE-CREATION 
> 
> Diff: https://git.reviewboard.kde.org/r/126078/diff/
> 
> 
> Testing
> ---
> 
> On OS X 10.9 with Qt 5.5.1 and frameworks 5.16.0 .
> 
> The example now works: when I set a QTimer with interval==0, the expected 
> wait for user input (`resumingFromIdle` signal) works. However, I am getting 
> a `stopCatchingIdleEvents` signal which means the application waits forever, 
> without ever getting to compare idle time to the list of timeouts.
> I haven't been able to figure out where that signal comes from, nor why this 
> doesn't happen on Linux.
> 
> Surely I'm missing something, but what?
> 
> 
> Thanks,
> 
> René J.V. Bertin
> 
>

___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

Re: Review Request 126078: [OS X] modernising the KIdleTime plugin (WIP!)

[René J . V . Bertin]

> On Nov. 17, 2015, 11:58 p.m., Lamarque Souza wrote:
> > src/plugins/osx/macpoller.h, line 24
> > 
> >
> > Nitpick: this should go after #include 

Any guidelines that dictate this?


> On Nov. 17, 2015, 11:58 p.m., Lamarque Souza wrote:
> > src/plugins/osx/macpoller.cpp, line 127
> > 
> >
> > Initialize m_nativeGrabber here too.

Oops, indeed it could be left un-initialised. My bad.


> On Nov. 17, 2015, 11:58 p.m., Lamarque Souza wrote:
> > src/plugins/osx/macpoller.cpp, line 178
> > 
> >
> > Use qCWarning instead.

I presume that should be using KIDLETIME for the category?

Isn't it possible to get the `KIDLETIME()` symbol through libKF5KIdleTime 
rather than having to pull in `../../logging.cpp`?


> On Nov. 17, 2015, 11:58 p.m., Lamarque Souza wrote:
> > src/plugins/osx/macpoller.cpp, line 213
> > 
> >
> > Shouldn't you check the return value of this method?

As far as I can see all it can be used for is to print a warning, right?


> On Nov. 17, 2015, 11:58 p.m., Lamarque Souza wrote:
> > src/plugins/osx/macpoller.cpp, line 215
> > 
> >
> > This should be changed to "return true", right?

Yeah, that seems logical, but I don't see any documentation on what 
setupPoller() should return. Then again the upstream code doesn't appear to use 
the return value anyway so the question is a bit moot ...


> On Nov. 17, 2015, 11:58 p.m., Lamarque Souza wrote:
> > src/plugins/osx/macpoller.cpp, line 300
> > 
> >
> > Remove commented code.

OK, but among the final things to do before committing :)


> On Nov. 17, 2015, 11:58 p.m., Lamarque Souza wrote:
> > src/plugins/osx/macpoller.cpp, line 353
> > 
> >
> > Remove commented code.

The bingo isn't mine!

Oh, the code. As above, in final cleanup :)


> On Nov. 17, 2015, 11:58 p.m., Lamarque Souza wrote:
> > src/plugins/osx/macpoller.cpp, line 404
> > 
> >
> > I do not get the "Unsetting m_catch is enough" comment. What is it 
> > supposed to mean since unsetting m_catch is the original code.

The original code worked rather differently. It used a mechanism with an 
internal poller, but you're right, the wording as it is makes sense only to 
someone who went about replacing that original code and made a number of 
(trial-and-)errors on the way.


> On Nov. 17, 2015, 11:58 p.m., Lamarque Souza wrote:
> > src/plugins/osx/macpoller.cpp, line 422
> > 
> >
> > Remove trailing space and shouldn't the commented code in this method 
> > be removed?

I put the commented code as a reference for alternative approaches that could 
be investigated once more if ever `updateSystemActivity` is removed. If there's 
a policy against such comments I can also store the snippets in a separate 
file, but that means adding one. What's preferable?


> On Nov. 17, 2015, 11:58 p.m., Lamarque Souza wrote:
> > src/plugins/osx/macpoller_helper.mm, line 43
> > 
> >
> > Use qCDebug instead.

That one actually got through and was never meant to!


> On Nov. 17, 2015, 11:58 p.m., Lamarque Souza wrote:
> > src/plugins/osx/macpoller_helper.mm, line 78
> > 
> >
> > There is no point in doing this if the next line will delete 
> > nativeGrabber.

I've learned to be fool-proof with this kind of thing (I don't trust `delete` 
to zero memory before releasing it; there's no `~CocoaEventFilter()`) but what 
does make it redundant here is setting `m_nativeGrabber = 0` just after 
deleting `nativeGrabber`.


- René J.V.


---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/126078/#review88505
---


On Nov. 17, 2015, 10:13 p.m., René J.V. Bertin wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/126078/
> ---
> 
> (Updated Nov. 17, 2015, 10:13 p.m.)
> 
> 
> Review request for KDE Software on Mac OS X, KDE Frameworks and Dario Freddi.
> 
> 
> Repository: kidletime
> 
> 
> 

Re: Review Request 126078: [OS X] modernising the KIdleTime plugin (WIP!)

[René J . V . Bertin]

> On Nov. 17, 2015, 11:58 p.m., Lamarque Souza wrote:
> > src/plugins/osx/macpoller_helper.mm, line 54
> > 
> >
> > Usually when a new operation returns 0 it is because system is on short 
> > on RAM memory (or memory is too fragmented). I would add assert here 
> > instead of silently ignoring the failure to allocate memory.

And I would argue that it is up to the calling software to decide how to react 
to a failure to set up idle detection: in this case it's only the "resume from 
idle" functionality that can no longer work. The idle timeout detection feature 
should still be able to function (possibly by adding some kind of reset when 
the calculated idle time returns to 0).
Asserting would mean that the application aborts, which I find a bit overkill 
for a situation that could be handled a lot more elegantly. That would require 
handling the return value from `setupPoller()` in 
`KIdleTimePrivate::loadSystem()` and up, of course.


- René J.V.


---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/126078/#review88505
---


On Nov. 17, 2015, 10:13 p.m., René J.V. Bertin wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/126078/
> ---
> 
> (Updated Nov. 17, 2015, 10:13 p.m.)
> 
> 
> Review request for KDE Software on Mac OS X, KDE Frameworks and Dario Freddi.
> 
> 
> Repository: kidletime
> 
> 
> Description
> ---
> 
> I noticed that the KIdleTime example doesn't work properly on OS X, and that 
> the plugin for OS X still uses the deprecated Carbon-based algorithm that I 
> already patched for KDE4.
> 
> This patch is a work-in-progress (hence the qDebugs) update to use IOKit, 
> IORegistry and CoreServices to do idle-time calculation as it should be done, 
> and allow simulated user activity through a "less deprecated" function.
> 
> 
> Diffs
> -
> 
>   src/plugins/osx/CMakeLists.txt e1b50b8 
>   src/plugins/osx/macpoller.h ef51ea5 
>   src/plugins/osx/macpoller.cpp ad9c10f 
>   src/plugins/osx/macpoller_helper.mm PRE-CREATION 
> 
> Diff: https://git.reviewboard.kde.org/r/126078/diff/
> 
> 
> Testing
> ---
> 
> On OS X 10.9 with Qt 5.5.1 and frameworks 5.16.0 .
> 
> The example now works: when I set a QTimer with interval==0, the expected 
> wait for user input (`resumingFromIdle` signal) works. However, I am getting 
> a `stopCatchingIdleEvents` signal which means the application waits forever, 
> without ever getting to compare idle time to the list of timeouts.
> I haven't been able to figure out where that signal comes from, nor why this 
> doesn't happen on Linux.
> 
> Surely I'm missing something, but what?
> 
> 
> Thanks,
> 
> René J.V. Bertin
> 
>

___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

Re: Review Request 126078: [OS X] modernising the KIdleTime plugin (WIP!)

[René J . V . Bertin]

> On Nov. 18, 2015, 8:22 a.m., Martin Gräßlin wrote:
> > src/plugins/osx/macpoller.cpp, lines 222-227
> > 
> >
> > seriously? You care about idle timeouts below 5 msec? This is a 
> > framework to tell the application whether the user doesn't use input 
> > devices. I don't know how fast you type, but I'm relatively certain that it 
> > takes more than 5 msec to move my finger from one key to another. What 
> > exactly is that you want to detect here? An event after each key press, 
> > maybe even between a key press and a key release? Because that's probably 
> > already more about 5 msec.
> 
> René J.V. Bertin wrote:
> The timer is used only for the detection of idle timeouts (so *absence* 
> of user input), and the precision with which it fires determines the accuracy 
> with which those events can be timed. This is currently inaccessible anyway, 
> but I don't believe in doing things half-bakedly. One might question whether 
> it makes sense to allow the framework to work at high precision (I more or 
> less agree that remains to be seen). But I don't think it makes sense to do 
> that and then leave the QTimer in a mode where the requested precision cannot 
> be met.
> 
> Martin Gräßlin wrote:
> To get this quite clear: you care about a precision of below 5 msec, this 
> is a third of the time it takes at least till the next frame is rendered. If 
> you are lucky you get the result of a key press or mouse move represented on 
> screen after 16 msec, more likely it's more. And you care about a wrong timer 
> precision below 5 msec. Sorry that's ridiculous. Please don't include such 
> non-sense code. You make that code more difficult to all other developers to 
> maintain. This code has cost if it's included.
> 
> Aleix Pol Gonzalez wrote:
> It's not a matter of doing things "right" or "wrong". It's a matter of 
> priorities.
> 
> KIdleTime is a framework for figuring out whether the system is idle. I 
> don't consider 5ms not using a system as it's being idling.

Again, we're talking about detecting how long a system has been idle, not 
whether it is (the framework is called KIdle*Time*, right? :)). The precision 
in question here applies just as well to (very) short periods (timeouts) as 
well as to long ones.
Also, don't confound the 5*ms* lower limit under which I switch to a high 
precision timer with the 5`%` error of a Qt::CoarseTimer. The precision of a 
high precision timer may be overkill for longer timeout durations in a 
framework that has a 1ms granularity, but the effect of 5% error scales with 
duration.

I consider that it's up to the user of a feature to know what s/he expects of 
it, not up to a framework. (Turning that around would probably remove the 
raison d'être of a lot of software.)

Finally, I have not been able to measure any increase in overhead when using a 
(single) high precision timer.

Granted, this is a bit a matter of principle for me, but as I've said elsewere, 
I could easily see me having used this in a previous job.


- René J.V.


---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/126078/#review88509
---


On Nov. 17, 2015, 10:13 p.m., René J.V. Bertin wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/126078/
> ---
> 
> (Updated Nov. 17, 2015, 10:13 p.m.)
> 
> 
> Review request for KDE Software on Mac OS X, KDE Frameworks and Dario Freddi.
> 
> 
> Repository: kidletime
> 
> 
> Description
> ---
> 
> I noticed that the KIdleTime example doesn't work properly on OS X, and that 
> the plugin for OS X still uses the deprecated Carbon-based algorithm that I 
> already patched for KDE4.
> 
> This patch is a work-in-progress (hence the qDebugs) update to use IOKit, 
> IORegistry and CoreServices to do idle-time calculation as it should be done, 
> and allow simulated user activity through a "less deprecated" function.
> 
> 
> Diffs
> -
> 
>   src/plugins/osx/CMakeLists.txt e1b50b8 
>   src/plugins/osx/macpoller.h ef51ea5 
>   src/plugins/osx/macpoller.cpp ad9c10f 
>   src/plugins/osx/macpoller_helper.mm PRE-CREATION 
> 
> Diff: https://git.reviewboard.kde.org/r/126078/diff/
> 
> 
> Testing
> ---
> 
> On OS X 10.9 with Qt 5.5.1 and frameworks 5.16.0 .
> 
> The example now works: when I set a QTimer with interval==0, the expected 
> wait for user input (`resumingFromIdle` signal) works. However, I am getting 
> a `stopCatchingIdleEvents` signal which means the application waits forever, 
> without ever getting to compare idle time to the list of timeouts.
> I haven't been able to figure out where that signal comes from, nor 

Re: Review Request 126078: [OS X] modernising the KIdleTime plugin (WIP!)

[Martin Gräßlin]

> On Nov. 18, 2015, 8:22 a.m., Martin Gräßlin wrote:
> > src/plugins/osx/macpoller.cpp, lines 222-227
> > 
> >
> > seriously? You care about idle timeouts below 5 msec? This is a 
> > framework to tell the application whether the user doesn't use input 
> > devices. I don't know how fast you type, but I'm relatively certain that it 
> > takes more than 5 msec to move my finger from one key to another. What 
> > exactly is that you want to detect here? An event after each key press, 
> > maybe even between a key press and a key release? Because that's probably 
> > already more about 5 msec.
> 
> René J.V. Bertin wrote:
> The timer is used only for the detection of idle timeouts (so *absence* 
> of user input), and the precision with which it fires determines the accuracy 
> with which those events can be timed. This is currently inaccessible anyway, 
> but I don't believe in doing things half-bakedly. One might question whether 
> it makes sense to allow the framework to work at high precision (I more or 
> less agree that remains to be seen). But I don't think it makes sense to do 
> that and then leave the QTimer in a mode where the requested precision cannot 
> be met.
> 
> Martin Gräßlin wrote:
> To get this quite clear: you care about a precision of below 5 msec, this 
> is a third of the time it takes at least till the next frame is rendered. If 
> you are lucky you get the result of a key press or mouse move represented on 
> screen after 16 msec, more likely it's more. And you care about a wrong timer 
> precision below 5 msec. Sorry that's ridiculous. Please don't include such 
> non-sense code. You make that code more difficult to all other developers to 
> maintain. This code has cost if it's included.
> 
> Aleix Pol Gonzalez wrote:
> It's not a matter of doing things "right" or "wrong". It's a matter of 
> priorities.
> 
> KIdleTime is a framework for figuring out whether the system is idle. I 
> don't consider 5ms not using a system as it's being idling.
> 
> René J.V. Bertin wrote:
> Again, we're talking about detecting how long a system has been idle, not 
> whether it is (the framework is called KIdle*Time*, right? :)). The precision 
> in question here applies just as well to (very) short periods (timeouts) as 
> well as to long ones.
> Also, don't confound the 5*ms* lower limit under which I switch to a high 
> precision timer with the 5`%` error of a Qt::CoarseTimer. The precision of a 
> high precision timer may be overkill for longer timeout durations in a 
> framework that has a 1ms granularity, but the effect of 5% error scales with 
> duration.
> 
> I consider that it's up to the user of a feature to know what s/he 
> expects of it, not up to a framework. (Turning that around would probably 
> remove the raison d'être of a lot of software.)
> 
> Finally, I have not been able to measure any increase in overhead when 
> using a (single) high precision timer.
> 
> Granted, this is a bit a matter of principle for me, but as I've said 
> elsewere, I could easily see me having used this in a previous job.

Rene, please stop it. Don't argue! You won't get this code in with polling and 
setting the qtimer precision based on clearly irrelevant value. It is our 
responsibility as fellow KDE developers to prevent such code to get it. I 
absolutely don't care how you intend to use this code, but this is a user idle 
framework. A system which had an input event in the last five milli seconds is 
not idle per definition. Thus the code is idiotic and wrong. Please stop 
arguing and change the code given the feedback you get here. Same above for the 
nullptr check. That one has to go, it's a wrong check at that place, remove it.


- Martin


---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/126078/#review88509
---


On Nov. 17, 2015, 10:13 p.m., René J.V. Bertin wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/126078/
> ---
> 
> (Updated Nov. 17, 2015, 10:13 p.m.)
> 
> 
> Review request for KDE Software on Mac OS X, KDE Frameworks and Dario Freddi.
> 
> 
> Repository: kidletime
> 
> 
> Description
> ---
> 
> I noticed that the KIdleTime example doesn't work properly on OS X, and that 
> the plugin for OS X still uses the deprecated Carbon-based algorithm that I 
> already patched for KDE4.
> 
> This patch is a work-in-progress (hence the qDebugs) update to use IOKit, 
> IORegistry and CoreServices to do idle-time calculation as it should be done, 
> and allow simulated user activity through a "less deprecated" function.
> 
> 
> 

Re: Review Request 126078: [OS X] modernising the KIdleTime plugin (WIP!)

[Martin Gräßlin]

> On Nov. 18, 2015, 8:22 a.m., Martin Gräßlin wrote:
> > src/plugins/osx/macpoller.cpp, lines 222-227
> > 
> >
> > seriously? You care about idle timeouts below 5 msec? This is a 
> > framework to tell the application whether the user doesn't use input 
> > devices. I don't know how fast you type, but I'm relatively certain that it 
> > takes more than 5 msec to move my finger from one key to another. What 
> > exactly is that you want to detect here? An event after each key press, 
> > maybe even between a key press and a key release? Because that's probably 
> > already more about 5 msec.
> 
> René J.V. Bertin wrote:
> The timer is used only for the detection of idle timeouts (so *absence* 
> of user input), and the precision with which it fires determines the accuracy 
> with which those events can be timed. This is currently inaccessible anyway, 
> but I don't believe in doing things half-bakedly. One might question whether 
> it makes sense to allow the framework to work at high precision (I more or 
> less agree that remains to be seen). But I don't think it makes sense to do 
> that and then leave the QTimer in a mode where the requested precision cannot 
> be met.

To get this quite clear: you care about a precision of below 5 msec, this is a 
third of the time it takes at least till the next frame is rendered. If you are 
lucky you get the result of a key press or mouse move represented on screen 
after 16 msec, more likely it's more. And you care about a wrong timer 
precision below 5 msec. Sorry that's ridiculous. Please don't include such 
non-sense code. You make that code more difficult to all other developers to 
maintain. This code has cost if it's included.


- Martin


---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/126078/#review88509
---


On Nov. 17, 2015, 10:13 p.m., René J.V. Bertin wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/126078/
> ---
> 
> (Updated Nov. 17, 2015, 10:13 p.m.)
> 
> 
> Review request for KDE Software on Mac OS X, KDE Frameworks and Dario Freddi.
> 
> 
> Repository: kidletime
> 
> 
> Description
> ---
> 
> I noticed that the KIdleTime example doesn't work properly on OS X, and that 
> the plugin for OS X still uses the deprecated Carbon-based algorithm that I 
> already patched for KDE4.
> 
> This patch is a work-in-progress (hence the qDebugs) update to use IOKit, 
> IORegistry and CoreServices to do idle-time calculation as it should be done, 
> and allow simulated user activity through a "less deprecated" function.
> 
> 
> Diffs
> -
> 
>   src/plugins/osx/CMakeLists.txt e1b50b8 
>   src/plugins/osx/macpoller.h ef51ea5 
>   src/plugins/osx/macpoller.cpp ad9c10f 
>   src/plugins/osx/macpoller_helper.mm PRE-CREATION 
> 
> Diff: https://git.reviewboard.kde.org/r/126078/diff/
> 
> 
> Testing
> ---
> 
> On OS X 10.9 with Qt 5.5.1 and frameworks 5.16.0 .
> 
> The example now works: when I set a QTimer with interval==0, the expected 
> wait for user input (`resumingFromIdle` signal) works. However, I am getting 
> a `stopCatchingIdleEvents` signal which means the application waits forever, 
> without ever getting to compare idle time to the list of timeouts.
> I haven't been able to figure out where that signal comes from, nor why this 
> doesn't happen on Linux.
> 
> Surely I'm missing something, but what?
> 
> 
> Thanks,
> 
> René J.V. Bertin
> 
>

___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

Re: Review Request 126078: [OS X] modernising the KIdleTime plugin (WIP!)

[Aleix Pol Gonzalez]

> On Nov. 18, 2015, 8:22 a.m., Martin Gräßlin wrote:
> > src/plugins/osx/macpoller.cpp, lines 222-227
> > 
> >
> > seriously? You care about idle timeouts below 5 msec? This is a 
> > framework to tell the application whether the user doesn't use input 
> > devices. I don't know how fast you type, but I'm relatively certain that it 
> > takes more than 5 msec to move my finger from one key to another. What 
> > exactly is that you want to detect here? An event after each key press, 
> > maybe even between a key press and a key release? Because that's probably 
> > already more about 5 msec.
> 
> René J.V. Bertin wrote:
> The timer is used only for the detection of idle timeouts (so *absence* 
> of user input), and the precision with which it fires determines the accuracy 
> with which those events can be timed. This is currently inaccessible anyway, 
> but I don't believe in doing things half-bakedly. One might question whether 
> it makes sense to allow the framework to work at high precision (I more or 
> less agree that remains to be seen). But I don't think it makes sense to do 
> that and then leave the QTimer in a mode where the requested precision cannot 
> be met.
> 
> Martin Gräßlin wrote:
> To get this quite clear: you care about a precision of below 5 msec, this 
> is a third of the time it takes at least till the next frame is rendered. If 
> you are lucky you get the result of a key press or mouse move represented on 
> screen after 16 msec, more likely it's more. And you care about a wrong timer 
> precision below 5 msec. Sorry that's ridiculous. Please don't include such 
> non-sense code. You make that code more difficult to all other developers to 
> maintain. This code has cost if it's included.

It's not a matter of doing things "right" or "wrong". It's a matter of 
priorities.

KIdleTime is a framework for figuring out whether the system is idle. I don't 
consider 5ms not using a system as it's being idling.


- Aleix


---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/126078/#review88509
---


On Nov. 17, 2015, 10:13 p.m., René J.V. Bertin wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/126078/
> ---
> 
> (Updated Nov. 17, 2015, 10:13 p.m.)
> 
> 
> Review request for KDE Software on Mac OS X, KDE Frameworks and Dario Freddi.
> 
> 
> Repository: kidletime
> 
> 
> Description
> ---
> 
> I noticed that the KIdleTime example doesn't work properly on OS X, and that 
> the plugin for OS X still uses the deprecated Carbon-based algorithm that I 
> already patched for KDE4.
> 
> This patch is a work-in-progress (hence the qDebugs) update to use IOKit, 
> IORegistry and CoreServices to do idle-time calculation as it should be done, 
> and allow simulated user activity through a "less deprecated" function.
> 
> 
> Diffs
> -
> 
>   src/plugins/osx/CMakeLists.txt e1b50b8 
>   src/plugins/osx/macpoller.h ef51ea5 
>   src/plugins/osx/macpoller.cpp ad9c10f 
>   src/plugins/osx/macpoller_helper.mm PRE-CREATION 
> 
> Diff: https://git.reviewboard.kde.org/r/126078/diff/
> 
> 
> Testing
> ---
> 
> On OS X 10.9 with Qt 5.5.1 and frameworks 5.16.0 .
> 
> The example now works: when I set a QTimer with interval==0, the expected 
> wait for user input (`resumingFromIdle` signal) works. However, I am getting 
> a `stopCatchingIdleEvents` signal which means the application waits forever, 
> without ever getting to compare idle time to the list of timeouts.
> I haven't been able to figure out where that signal comes from, nor why this 
> doesn't happen on Linux.
> 
> Surely I'm missing something, but what?
> 
> 
> Thanks,
> 
> René J.V. Bertin
> 
>

___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

Re: Review Request 126078: [OS X] modernising the KIdleTime plugin (WIP!)

[Martin Graesslin]

On Wednesday, November 18, 2015 5:28:27 PM CET René J.V. Bertin wrote:
> On Wednesday November 18 2015 15:01:59 Martin Gräßlin wrote:
> > idiotic
> 
> If we're starting to call names arguing indeed becomes pointless as the
> apparent lack of actual reading my arguments already suggested. It leaves
> me with no other choice but to discard the RR and put up a mirror in place.

I want to apologize for saying that. Please be aware that this was only 
intended as a description of code (yes I also call code written by myself 
idiotic and worse things) and not against you.

I hope you don't fork, because I think that would be bad. Not because you move 
the code else where, but because the issues pointed out would be unresolved 
giving users of your code a bad framework and thus harming both your users as 
well as the frameworks project for distributing bad code. The problem of 
polling is real. Please accept this feedback.

Best regards
Martin Gräßlin


signature.asc
Description: This is a digitally signed message part.
___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

Re: Review Request 126078: [OS X] modernising the KIdleTime plugin (WIP!)

[Lamarque Souza]

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/126078/#review88505
---



src/plugins/osx/macpoller.h (line 24)


Nitpick: this should go after #include 



src/plugins/osx/macpoller.h (line 75)


One line per variable.



src/plugins/osx/macpoller.h (line 76)


Here too.



src/plugins/osx/macpoller.cpp (line 2)


This should be appended to the list, not prepended. I know, the next line 
is wrong too.



src/plugins/osx/macpoller.cpp (line 29)


Remove trailing space character.



src/plugins/osx/macpoller.cpp (line 49)


Initialize m_nativeGrabber here too.



src/plugins/osx/macpoller.cpp (line 100)


Use qCWarning instead.



src/plugins/osx/macpoller.cpp (line 135)


Shouldn't you check the return value of this method?



src/plugins/osx/macpoller.cpp (line 137)


This should be changed to "return true", right?



src/plugins/osx/macpoller.cpp (line 212)


We usually join the last } with the next "else {", like this:

} else {



src/plugins/osx/macpoller.cpp (line 221)


Remove commented code.



src/plugins/osx/macpoller.cpp (line 242)


We usually do not use extra space after ( and before )



src/plugins/osx/macpoller.cpp (line 274)


Remove commented code.



src/plugins/osx/macpoller.cpp (line 325)


I do not get the "Unsetting m_catch is enough" comment. What is it supposed 
to mean since unsetting m_catch is the original code.



src/plugins/osx/macpoller.cpp (line 343)


Remove trailing space and shouldn't the commented code in this method be 
removed?



src/plugins/osx/macpoller_helper.mm (line 2)


Append new entries in copyright section, do not prepend.



src/plugins/osx/macpoller_helper.mm (line 43)


Use qCDebug instead.



src/plugins/osx/macpoller_helper.mm (line 54)


Usually when a new operation returns 0 it is because system is on short on 
RAM memory (or memory is too fragmented). I would add assert here instead of 
silently ignoring the failure to allocate memory.



src/plugins/osx/macpoller_helper.mm (line 78)


There is no point in doing this if the next line will delete nativeGrabber.



src/plugins/osx/macpoller_helper.mm (line 82)


This line can go to inside the if clause above.


- Lamarque Souza


On Nov. 17, 2015, 9:13 p.m., René J.V. Bertin wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/126078/
> ---
> 
> (Updated Nov. 17, 2015, 9:13 p.m.)
> 
> 
> Review request for KDE Software on Mac OS X, KDE Frameworks and Dario Freddi.
> 
> 
> Repository: kidletime
> 
> 
> Description
> ---
> 
> I noticed that the KIdleTime example doesn't work properly on OS X, and that 
> the plugin for OS X still uses the deprecated Carbon-based algorithm that I 
> already patched for KDE4.
> 
> This patch is a work-in-progress (hence the qDebugs) update to use IOKit, 
> IORegistry and CoreServices to do idle-time calculation as it should be done, 
> and allow simulated user activity through a "less deprecated" function.
> 
> 
> Diffs
> -
> 
>   src/plugins/osx/CMakeLists.txt e1b50b8 
>   src/plugins/osx/macpoller.h ef51ea5 
>   src/plugins/osx/macpoller.cpp ad9c10f 
>   src/plugins/osx/macpoller_helper.mm PRE-CREATION 
> 
> Diff: https://git.reviewboard.kde.org/r/126078/diff/
> 
> 
> Testing
> ---
> 
> On OS X 10.9 with Qt 5.5.1 and frameworks 5.16.0 .
> 
> The example now works: when I set a QTimer with interval==0, the expected 
> wait for user input (`resumingFromIdle` signal) works. However, I am getting 
> a `stopCatchingIdleEvents` signal which means 

Re: Review Request 126078: [OS X] modernising the KIdleTime plugin (WIP!)

[Martin Gräßlin]

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/126078/#review88509
---



src/plugins/osx/macpoller.cpp (lines 144 - 149)


seriously? You care about idle timeouts below 5 msec? This is a framework 
to tell the application whether the user doesn't use input devices. I don't 
know how fast you type, but I'm relatively certain that it takes more than 5 
msec to move my finger from one key to another. What exactly is that you want 
to detect here? An event after each key press, maybe even between a key press 
and a key release? Because that's probably already more about 5 msec.


- Martin Gräßlin


On Nov. 17, 2015, 10:13 p.m., René J.V. Bertin wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/126078/
> ---
> 
> (Updated Nov. 17, 2015, 10:13 p.m.)
> 
> 
> Review request for KDE Software on Mac OS X, KDE Frameworks and Dario Freddi.
> 
> 
> Repository: kidletime
> 
> 
> Description
> ---
> 
> I noticed that the KIdleTime example doesn't work properly on OS X, and that 
> the plugin for OS X still uses the deprecated Carbon-based algorithm that I 
> already patched for KDE4.
> 
> This patch is a work-in-progress (hence the qDebugs) update to use IOKit, 
> IORegistry and CoreServices to do idle-time calculation as it should be done, 
> and allow simulated user activity through a "less deprecated" function.
> 
> 
> Diffs
> -
> 
>   src/plugins/osx/CMakeLists.txt e1b50b8 
>   src/plugins/osx/macpoller.h ef51ea5 
>   src/plugins/osx/macpoller.cpp ad9c10f 
>   src/plugins/osx/macpoller_helper.mm PRE-CREATION 
> 
> Diff: https://git.reviewboard.kde.org/r/126078/diff/
> 
> 
> Testing
> ---
> 
> On OS X 10.9 with Qt 5.5.1 and frameworks 5.16.0 .
> 
> The example now works: when I set a QTimer with interval==0, the expected 
> wait for user input (`resumingFromIdle` signal) works. However, I am getting 
> a `stopCatchingIdleEvents` signal which means the application waits forever, 
> without ever getting to compare idle time to the list of timeouts.
> I haven't been able to figure out where that signal comes from, nor why this 
> doesn't happen on Linux.
> 
> Surely I'm missing something, but what?
> 
> 
> Thanks,
> 
> René J.V. Bertin
> 
>

___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

Re: Review Request 126078: [OS X] modernising the KIdleTime plugin (WIP!)

[René J . V . Bertin]

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/126078/
---

(Updated Nov. 17, 2015, 10:12 p.m.)


Review request for KDE Software on Mac OS X and KDE Frameworks.


Changes
---

This revision takes comments from the mailing list into account. Pure 
end-of-idle detection is done in a way similar to how `WidgetBasedPoller` does 
it, by detecting global UI events. That part explains the newly introduced 
ObjC++ file.
Like `WidgetBasedPoller`, the new implementation has to resort to a polling 
approach, and like that example class, the the polling frequency is adaptive. 
However, the adaptive algorithm:
- uses the difference between the current idle time and the next timeout from 
the timeouts list as the source of the "error" value used to calculate the 
timer interval
- uses <1 gain (difference/2) 
- relies on a sorted timeouts list, which ensures that the initial "next 
timeout" is equal to the minimal timeout

In addition, using a sorted timeout list means that determination of a timeout 
hit becomes much simpler, basically `idle >= timeout[i]`. That removes the 
possibility to signal early, or to reject signalling because a timeout was 
detected after the acceptance window.

I have left the `kidletime_example` run idling for over 3 minutes; as far as I 
can tell it behaves as expected, and yields a better-than-1ms accuracy (on 
average timeouts are signalled less than 1ms late). It only shows near the top 
in a running `top` listing on rare occasions, and then only at a few percents 
CPU.

I have left in the hooks to configure a fixed, higher polling frequency, for 
now, until I'm convinced that they're indeed not justified given the 1ms 
resolution imposed by the API.


Repository: kidletime


Description
---

I noticed that the KIdleTime example doesn't work properly on OS X, and that 
the plugin for OS X still uses the deprecated Carbon-based algorithm that I 
already patched for KDE4.

This patch is a work-in-progress (hence the qDebugs) update to use IOKit, 
IORegistry and CoreServices to do idle-time calculation as it should be done, 
and allow simulated user activity through a "less deprecated" function.


Diffs (updated)
-

  src/plugins/osx/CMakeLists.txt e1b50b8 
  src/plugins/osx/macpoller.h ef51ea5 
  src/plugins/osx/macpoller.cpp ad9c10f 
  src/plugins/osx/macpoller_helper.mm PRE-CREATION 

Diff: https://git.reviewboard.kde.org/r/126078/diff/


Testing
---

On OS X 10.9 with Qt 5.5.1 and frameworks 5.16.0 .

The example now works: when I set a QTimer with interval==0, the expected wait 
for user input (`resumingFromIdle` signal) works. However, I am getting a 
`stopCatchingIdleEvents` signal which means the application waits forever, 
without ever getting to compare idle time to the list of timeouts.
I haven't been able to figure out where that signal comes from, nor why this 
doesn't happen on Linux.

Surely I'm missing something, but what?


Thanks,

René J.V. Bertin

___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

Re: Review Request 126078: [OS X] modernising the KIdleTime plugin (WIP!)

[René J . V . Bertin]

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/126078/
---

(Updated Nov. 16, 2015, 4:42 p.m.)


Review request for KDE Software on Mac OS X and KDE Frameworks.


Changes
---

This patch addresses most issues. I'm still getting the 1 timeout twice in 
the KIdleTime example, but otherwise it appears to behave as intended.

`simulateUserActivity` does *not* reset HIDIdleTime on recent OS X versions (if 
it ever did). I'm simulating that now via a software reset (idle time offset); 
please check if I'm resetting it properly.
Since that method uses a deprecated function, I've included a code snippet to 
show how to simulate user activity by rocking the mouse cursor back and forth 
over a tiny amount. That doesn't reset HIDIdleTime either, but might suffice 
for other applications of `simulateUserActivity` (if there are any such) .

TODO : cleanup


Repository: kidletime


Description
---

I noticed that the KIdleTime example doesn't work properly on OS X, and that 
the plugin for OS X still uses the deprecated Carbon-based algorithm that I 
already patched for KDE4.

This patch is a work-in-progress (hence the qDebugs) update to use IOKit, 
IORegistry and CoreServices to do idle-time calculation as it should be done, 
and allow simulated user activity through a "less deprecated" function.


Diffs (updated)
-

  src/plugins/osx/macpoller.cpp ad9c10f 
  src/plugins/osx/macpoller.h ef51ea5 
  src/plugins/osx/CMakeLists.txt e1b50b8 

Diff: https://git.reviewboard.kde.org/r/126078/diff/


Testing
---

On OS X 10.9 with Qt 5.5.1 and frameworks 5.16.0 .

The example now works: when I set a QTimer with interval==0, the expected wait 
for user input (`resumingFromIdle` signal) works. However, I am getting a 
`stopCatchingIdleEvents` signal which means the application waits forever, 
without ever getting to compare idle time to the list of timeouts.
I haven't been able to figure out where that signal comes from, nor why this 
doesn't happen on Linux.

Surely I'm missing something, but what?


Thanks,

René J.V. Bertin

___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

Re: Review Request 126078: [OS X] modernising the KIdleTime plugin (WIP!)

[René J . V . Bertin]

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/126078/#review88418
---


Here's a debugging trace after hitting a breakpoint set on the qFatal() 
statement I put into `MacPoller::stopCatchingIdleEvents` . One can see 
`KIdleTime::stopCatchingResumeEvent` being called in frame 1, but I cannot 
identify the calling frame. It doesn't appear to be 
`KIdleTimePrivate::_k_resumingFromIdle()` (a breakpoint in there isn't hit).


```C++
Process 88123 launched: 
'/Applications/MacPorts/KF5/examples/kidletime_example.app/Contents/MacOS/kidletime_example'
 (x86_64)
starting the idletime poll QTimer with interval 0 and m_minTimeout -1 ; start 
of catch
Your idle time is 595
Welcome!! Move your mouse or do something to start...
Great! Now stay idle for 5 seconds to get a nice message. From 10seconds on, 
you can move your mouse to get back here.
If you will stay idle for too long, I will simulate your activityafter 25 
seconds, and make everything start back
Appended timeout 5000 ; min.== -1
Appended timeout 1 ; min.== -1
Appended timeout 25000 ; min.== -1
stopping the idletime poll QTimer; end of catch, idle= 2
Process 88123 stopped
* thread #1: tid = 0x37451f, 0x000104fb4359 
KF5IdleTimeOsxPlugin.so`MacPoller::stopCatchingIdleEvents(this=) + 
297 at macpoller.cpp:372, queue = 'com.apple.main-thread', stop reason = 
breakpoint 1.1
frame #0: 0x000104fb4359 
KF5IdleTimeOsxPlugin.so`MacPoller::stopCatchingIdleEvents(this=) + 
297 at macpoller.cpp:372
   369  if (m_idleTimer) {
   370  qDebug() << "stopping the idletime poll QTimer; end of catch, 
idle=" << poll(false);
   371  // stopCatchingIdleEvents is called after each resumingFromIdle 
signal??!
-> 372  qFatal(__FUNCTION__);
   373  m_idleTimer->stop();
   374  }
   375  #endif
(lldb) bt
* thread #1: tid = 0x37451f, 0x000104fb4359 
KF5IdleTimeOsxPlugin.so`MacPoller::stopCatchingIdleEvents(this=) + 
297 at macpoller.cpp:372, queue = 'com.apple.main-thread', stop reason = 
breakpoint 1.1
  * frame #0: 0x000104fb4359 
KF5IdleTimeOsxPlugin.so`MacPoller::stopCatchingIdleEvents(this=) + 
297 at macpoller.cpp:372
frame #1: 0x0001b2c1 
libKF5IdleTime.5.dylib`KIdleTime::qt_static_metacall(QObject*, 
QMetaObject::Call, int, void**) [inlined] KIdleTime::stopCatchingResumeEvent() 
+ 43 at kidletime.cpp:113
frame #2: 0x0001b296 
libKF5IdleTime.5.dylib`KIdleTime::qt_static_metacall(_o=, 
_c=, _id=, _a=) + 582 at 
moc_kidletime.cpp:113
frame #3: 0x000101141252 
QtCore`QMetaObject::activate(sender=0x0001041161f0, 
signalOffset=, local_signal_index=, 
argv=) + 2978 at qobject.cpp:3713
frame #4: 0x000101141252 
QtCore`QMetaObject::activate(sender=0x0001041071c0, 
signalOffset=, local_signal_index=, 
argv=) + 2978 at qobject.cpp:3713
frame #5: 0x000101139740 QtCore`QObject::event(this=0x0001041071c0, 
e=) + 48 at qobject.cpp:1220
frame #6: 0x00010004854b 
QtWidgets`QApplicationPrivate::notify_helper(this=, 
receiver=0x0001041071c0, e=0x7fff5fbfd4e0) + 251 at 
qapplication.cpp:3716
frame #7: 0x00010004b904 
QtWidgets`QApplication::notify(this=, receiver=, 
e=) + 8212 at qapplication.cpp:3681
frame #8: 0x000101110703 
QtCore`QCoreApplication::notifyInternal(this=, 
receiver=, event=) + 115 at qcoreapplication.cpp:970
frame #9: 0x000101163d16 
QtCore`QTimerInfoList::activateTimers(this=0x000104059b10) + 1270 at 
qcoreapplication.h:224
frame #10: 0x000104ca5502 
libqcocoa.dylib`QCocoaEventDispatcherPrivate::activateTimersSourceCallback(info=0x000104059a90)
 + 18 at qcocoaeventdispatcher.mm:121
frame #11: 0x7fff90e4e5b1 
CoreFoundation`__CFRUNLOOP_IS_CALLING_OUT_TO_A_SOURCE0_PERFORM_FUNCTION__ + 17
frame #12: 0x7fff90e3fc62 CoreFoundation`__CFRunLoopDoSources0 + 242
frame #13: 0x7fff90e3f3ef CoreFoundation`__CFRunLoopRun + 831
frame #14: 0x7fff90e3ee75 CoreFoundation`CFRunLoopRunSpecific + 309
frame #15: 0x7fff93440a0d HIToolbox`RunCurrentEventLoopInMode + 226
frame #16: 0x7fff934407b7 HIToolbox`ReceiveNextEventCommon + 479
frame #17: 0x7fff934405bc 
HIToolbox`_BlockUntilNextEventMatchingListInModeWithFilter + 65
frame #18: 0x7fff8ac9c24e AppKit`_DPSNextEvent + 1434
frame #19: 0x7fff8ac9b89b AppKit`-[NSApplication 
nextEventMatchingMask:untilDate:inMode:dequeue:] + 122
frame #20: 0x7fff8ac8f99c AppKit`-[NSApplication run] + 553
frame #21: 0x000104ca611d 
libqcocoa.dylib`QCocoaEventDispatcher::processEvents(this=0x000104056170, 
flags=) + 2189 at qcocoaeventdispatcher.mm:418
frame #22: 0x00010110de2d 
QtCore`QEventLoop::exec(QFlags) [inlined] 
QEventLoop::processEvents(QFlags) + 381 at 
qeventloop.cpp:128
frame #23: 0x00010110de14 

Re: Review Request 126078: [OS X] modernising the KIdleTime plugin (WIP!)

[René J . V . Bertin]

On Monday November 16 2015 12:29:49 René J.V. Bertin wrote:

> If that's the intended behaviour, why does the KIdleTime example work on 
> Linux, without "restarting" `catchNextResumeEvent`??

Sorry for the rapid fire of growing messages - that comes with asking questions 
through RRs ...

I realised stopCatchingIdleEvents() shouldn't stop the timer, only unset the 
state variable that indicates resume signals are desired.

Now, I have to figure out an elegant way to stop sending identical 
timeOutReached signals (until the next timeout level is reached)!

R.
___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

Re: Review Request 126078: [OS X] modernising the KIdleTime plugin (WIP!)

[René J . V . Bertin]

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/126078/
---

(Updated Nov. 16, 2015, 11:03 p.m.)


Review request for KDE Software on Mac OS X and KDE Frameworks.


Changes
---

Code cleaned up.

In reaction to my message on the kde-frameworks ML ("KIdleTime : provide a 
settable resolution for the polling backends?"), I've added a prototype 
implementation of a mechanism to set the poller timer resolution. That 
resolution is directly relevant for my implementation, which is why I didn't 
present it in a separate RR.


Repository: kidletime


Description
---

I noticed that the KIdleTime example doesn't work properly on OS X, and that 
the plugin for OS X still uses the deprecated Carbon-based algorithm that I 
already patched for KDE4.

This patch is a work-in-progress (hence the qDebugs) update to use IOKit, 
IORegistry and CoreServices to do idle-time calculation as it should be done, 
and allow simulated user activity through a "less deprecated" function.


Diffs (updated)
-

  src/plugins/osx/CMakeLists.txt e1b50b8 
  src/plugins/osx/macpoller.h ef51ea5 
  src/plugins/osx/macpoller.cpp ad9c10f 

Diff: https://git.reviewboard.kde.org/r/126078/diff/


Testing
---

On OS X 10.9 with Qt 5.5.1 and frameworks 5.16.0 .

The example now works: when I set a QTimer with interval==0, the expected wait 
for user input (`resumingFromIdle` signal) works. However, I am getting a 
`stopCatchingIdleEvents` signal which means the application waits forever, 
without ever getting to compare idle time to the list of timeouts.
I haven't been able to figure out where that signal comes from, nor why this 
doesn't happen on Linux.

Surely I'm missing something, but what?


Thanks,

René J.V. Bertin

___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

Re: Review Request 126078: [OS X] modernising the KIdleTime plugin (WIP!)

[René J . V . Bertin]

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/126078/
---

(Updated Nov. 16, 2015, 11:15 p.m.)


Review request for KDE Software on Mac OS X and KDE Frameworks.


Repository: kidletime


Description
---

I noticed that the KIdleTime example doesn't work properly on OS X, and that 
the plugin for OS X still uses the deprecated Carbon-based algorithm that I 
already patched for KDE4.

This patch is a work-in-progress (hence the qDebugs) update to use IOKit, 
IORegistry and CoreServices to do idle-time calculation as it should be done, 
and allow simulated user activity through a "less deprecated" function.


Diffs (updated)
-

  src/plugins/osx/CMakeLists.txt e1b50b8 
  src/plugins/osx/macpoller.h ef51ea5 
  src/plugins/osx/macpoller.cpp ad9c10f 

Diff: https://git.reviewboard.kde.org/r/126078/diff/


Testing
---

On OS X 10.9 with Qt 5.5.1 and frameworks 5.16.0 .

The example now works: when I set a QTimer with interval==0, the expected wait 
for user input (`resumingFromIdle` signal) works. However, I am getting a 
`stopCatchingIdleEvents` signal which means the application waits forever, 
without ever getting to compare idle time to the list of timeouts.
I haven't been able to figure out where that signal comes from, nor why this 
doesn't happen on Linux.

Surely I'm missing something, but what?


Thanks,

René J.V. Bertin

___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

Re: Review Request 126078: [OS X] modernising the KIdleTime plugin (WIP!)

[Aleix Pol Gonzalez]

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/126078/#review88390
---



src/plugins/osx/macpoller.h (line 31)


You removed Carbon from cmake, no?

I'm not sure why you're keeping both really. If IOKit is the way to go, 
then do it.


In fact, I'm quite sure that as is it already is broken without the #define

- Aleix Pol Gonzalez


On Nov. 15, 2015, 11:58 p.m., René J.V. Bertin wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/126078/
> ---
> 
> (Updated Nov. 15, 2015, 11:58 p.m.)
> 
> 
> Review request for KDE Software on Mac OS X and KDE Frameworks.
> 
> 
> Repository: kidletime
> 
> 
> Description
> ---
> 
> I noticed that the KIdleTime example doesn't work properly on OS X, and that 
> the plugin for OS X still uses the deprecated Carbon-based algorithm that I 
> already patched for KDE4.
> 
> This patch is a work-in-progress (hence the qDebugs) update to use IOKit, 
> IORegistry and CoreServices to do idle-time calculation as it should be done, 
> and allow simulated user activity through a "less deprecated" function.
> 
> 
> Diffs
> -
> 
>   src/plugins/osx/macpoller.cpp ad9c10f 
>   src/plugins/osx/CMakeLists.txt e1b50b8 
>   src/plugins/osx/macpoller.h ef51ea5 
> 
> Diff: https://git.reviewboard.kde.org/r/126078/diff/
> 
> 
> Testing
> ---
> 
> On OS X 10.9 with Qt 5.5.1 and frameworks 5.16.0 .
> 
> The example now works: when I set a QTimer with interval==0, the expected 
> wait for user input (`resumingFromIdle` signal) works. However, I am getting 
> a `stopCatchingIdleEvents` signal which means the application waits forever, 
> without ever getting to compare idle time to the list of timeouts.
> I haven't been able to figure out where that signal comes from, nor why this 
> doesn't happen on Linux.
> 
> Surely I'm missing something, but what?
> 
> 
> Thanks,
> 
> René J.V. Bertin
> 
>

___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel

Re: Review Request 126078: [OS X] modernising the KIdleTime plugin (WIP!)

[René J . V . Bertin]

On Nov. 16, 2015, 12:19 a.m., René J.V. Bertin wrote:
> > In fact, I'm quite sure that as is it already is broken without the #define

Hmm? What do you think I broke that wasn't broken before (again, apart from the 
bare idle time detection, the original code doesn't work for me).

Anyway, I'm only keeping the Carbon code around as a reference until the new 
code works.


- René J.V.


---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/126078/#review88390
---


On Nov. 15, 2015, 11:58 p.m., René J.V. Bertin wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/126078/
> ---
> 
> (Updated Nov. 15, 2015, 11:58 p.m.)
> 
> 
> Review request for KDE Software on Mac OS X and KDE Frameworks.
> 
> 
> Repository: kidletime
> 
> 
> Description
> ---
> 
> I noticed that the KIdleTime example doesn't work properly on OS X, and that 
> the plugin for OS X still uses the deprecated Carbon-based algorithm that I 
> already patched for KDE4.
> 
> This patch is a work-in-progress (hence the qDebugs) update to use IOKit, 
> IORegistry and CoreServices to do idle-time calculation as it should be done, 
> and allow simulated user activity through a "less deprecated" function.
> 
> 
> Diffs
> -
> 
>   src/plugins/osx/macpoller.cpp ad9c10f 
>   src/plugins/osx/CMakeLists.txt e1b50b8 
>   src/plugins/osx/macpoller.h ef51ea5 
> 
> Diff: https://git.reviewboard.kde.org/r/126078/diff/
> 
> 
> Testing
> ---
> 
> On OS X 10.9 with Qt 5.5.1 and frameworks 5.16.0 .
> 
> The example now works: when I set a QTimer with interval==0, the expected 
> wait for user input (`resumingFromIdle` signal) works. However, I am getting 
> a `stopCatchingIdleEvents` signal which means the application waits forever, 
> without ever getting to compare idle time to the list of timeouts.
> I haven't been able to figure out where that signal comes from, nor why this 
> doesn't happen on Linux.
> 
> Surely I'm missing something, but what?
> 
> 
> Thanks,
> 
> René J.V. Bertin
> 
>

___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel