SafeEyes
SafeEyes copied to clipboard
smartpause: use X API instead of xprintidle subprocess
The xprintidle
utility is a command-line wrapper around an X11 API call. SafeEyes can make the same call itself.
This likely reduces resource usage by avoiding spawning a new subprocess every few seconds, and potentially simplifies installation dependencies.
PR in draft mode because there are some things I could yet clean up. Let me know if it looks like the sort of thing you'd be interested in.
@keturn it is definitely a good direction to go. I used xprintidle because I didn't know too much about X API. I am looking forward to merge your PR.
Thank you very much for your contribution!
One detail worth mentioning: xprintidle has some extra code in it to work around a bug where the idle time resets when the monitor drops in to power-saving standby mode.
However:
- that bug is marked as fixed: https://bugs.freedesktop.org/show_bug.cgi?id=6439
- even if that does happen, it might not be such a problem for this use case, because the times smartpause is interested in are shorter than the time it takes the monitor to go to sleep.
Do you think it's okay to skip that workaround code, or do we need to do more testing to see if it's a problem?
Another thing worth talking about here is the library choice. I replaced xprintidle
with usage of the xcffib library. I think that's a good trade, as it gets rid of the repeated subprocess and it's also available in Debian.
The snag is that makes this package depend on both xcffib and python-xlib (in BreakScreen), which would be a silly thing to do, because they're both X client protocol libraries.
The advantage of python-xlib is that it's pure python, which can be easier to work with and safer from memory errors. The disadvantage is that it's a new implementation and doesn't yet cover the XScreenSaver extension we need.
I think the options are
- implement XScreenSaver QueryInfo for python-xlib.
- accept this PR's use of xcffib, and then potentially address what to do with BreakScreen in another ticket.
While I like pure-python implementations in theory, I think the approach xcffib uses is pretty good as far as things depending on C go.
- [x] xcffib?
One detail worth mentioning: xprintidle has some extra code in it to work around a bug where the idle time resets when the monitor drops in to power-saving standby mode.
Since the bug is fixed long time ago, I think we don't need that fix. However it is better to test it with few distributions to make sure that we are not affected. In case if we are affected, I prefer to have the fix in place.
Usage of
xcffib
I also prefer pure Python based solution but looks like it requires more effort. Therefore I agree with your plan. Lets go with xcffib
and later we can consider switching to python-xlib
.
Btw, both Stretchly and Workrave already have a similar detection, which, unlike SafeEyes with xprintidle, actually work well for me - maybe there's something you could use from their implementations ;)
Yep, I just returned to my computer after starting it half an hour ago - and within few minutes was prompted for a long break. Wut?
I really like the idea, but I guess I gotta go back to Stretchly for now.