tobi-wan-kenobi / bumblebee-status

bumblebee-status is a modular, theme-able status line generator for the i3 window manager.
https://bumblebee-status.readthedocs.io/en/main/
MIT License
1.23k stars 228 forks source link

Screen still blanks even if caffeine is enabled (for certain setups) #426

Closed cddmp closed 5 years ago

cddmp commented 5 years ago

Bug Report

Summary

Affected module: caffeine.py

Description: The screen still blanks after 10 minutes, even if caffeine is active. The cause of the problem is that DPMS is still activated, which can cause issues on some setups. A simple solution would be to enable/disable DPMS (+dpms, -dpms) when calling xset in caffeine.py. Though I'm not 100% sure which implications modifying DPMS has on screensaver. Possibly screensaver are intended to handle DPMS on their own, so modifying it directly in caffeine.py might interfere with screensaver.

What is the current behaviour and what is the expected behaviour?

Current behaviour: Screen turns off after 10 minutes Expected behaviour: Screen doesn't turn off when caffeine is activated

How to reproduce

I used a default installation of xorg-server under ArchLinux without modifying any DPMS stuff in the X config. I don't use a display manager and run the X server via startx. When enabling caffeine and waiting for 10 minutes without any user interaction, DPMS will blank the screen.

Expected behavior

Screen doesn't turn off when caffeine is activated.

tobi-wan-kenobi commented 5 years ago

Good question, I notice that also.

@TheEdgeOfRage Since this module is originally from you, I believe, do you have any input?

I'll try to dedicate some time for research for this the coming weekend, thanks for raising this!

cddmp commented 5 years ago

Thanks for all your work on bumblebee-status!

I just came across this: https://wiki.archlinux.org/index.php/XScreenSaver#DPMS_and_blanking_settings So e.g. XScreenSaver seems to handle DPMS on its own. Let's assume a screensaver or any other tool disabled DPMS. At the same time we have disabled the screensaver via caffeine. If we would now run

xset s on +dpms

via caffeine, without having checked the original DPMS state (on/off), we would enable DPMS again and therefore overwrite the original tools/screensaver settings. So I think it would be good to save the original state of DPMS and run xset accordingly. If the original author is lacking time, I could also have a look at fixing this.

TheEdgeOfRage commented 5 years ago

I wrote the module some years back now, so I have zero idea how it works anymore.

I'm currently on vacation so I wont have time to look into it until at least 2019-09-02. Feel free to try and fix it yourself, I find that with even a little knowledge in Python can go a long way in writing a bumblebee module (talking from experience). I started writing modules when I had pretty much no experience with Python, but Tobi's code inspired me to learn it and I've never looked back since.

PS. Though I wont vouch for my own code in the caffeine module, That thing was written a long-ass time ago.

tobi-wan-kenobi commented 5 years ago

@TheEdgeOfRage Glad to hear that bumblebee-status helped you get started with Python. Also, I can totally relate to not vouching for code that's more than a year old ;) I value your contributions to the project a lot, thanks for that!

@cddmp If you are willing to look into fixing this issue, that would be much appreciated. To be honest, I don't have the foggiest idea how DMPS works, so I'm not sure I'd be able to fix this without introducing other bugs :P

cddmp commented 5 years ago

I installed xscreensaver and had a look at what it does. As soon as it is running it disables DPMS and sets length = 0 (determines how long the server must be inactive for screen saving to activate) and period = 0 (time interval to change the screensavers background pattern - meant to prevent display burn in). So xscreensaver basically disables display power management and the screen saver for X and overtakes control. This also brought up another issue: caffeine will interpret length = 0 as "screensaver is off". Therefore, the caffeine switch will switch to "on", though the screensaver is actually running (see implementation of _active(self)).

Trying to find out how other tools deal with that (e.g. VLC), it turned out many screensaver support special reset flags to reset the screensaver (e.g. gnome-screensaver-command --poke) and/or support reset via dbus. Fortunately there is a wrapper which seems to be able to handle most of them: xdg-screensaver. It allows to pass a "reset" flag and then does all the necessary magic in the background. The only downside is, the script is quite huge and does a lot of calls. It should propably not be called on every bumblebee-status update interval.

Would you be happy if I try to come up with a solution based on xdg-screensaver?

TheEdgeOfRage commented 5 years ago

@cddmp Looks good to me. I'm not sure how intensive a xdg-screensaver status request is, but it might be smarter not to call it every time, but rather call once at the start and then store it in memory. Then only change it on user input. It won't react to manual calls to xdg-screensaver from the terminal, but as it stands now, too many bumblebee modules can become a not so insignificant on CPU load.

Also, make sure to add xdg-utils as a dependency in the module list as part of the pull request.

cddmp commented 5 years ago

@TheEdgeOfRage Yes I totally agree, I had that in mind when I stated above that it should propably not be called on every bumblebee-status update interval.

xdg-screensaver is just a bash script which checks which screensaver is running and allows to control various screensaver by wrapping them, see https://github.com/freedesktop/xdg-utils/blob/master/scripts/xdg-screensaver.in

VLC seems to use it as well, looks as they call it every 30 seconds if activated by the user: https://github.com/videolan/vlc/blob/694399e23000232708b2d514a6a265cfc023ddde/modules/misc/inhibit/xdg.c

I guess we would need to do something similar. As I mentioned above there seems to be no generic way to find out the timeout (length) when a screensaver actually is activated. Every screensaver seems to implement that differently. So we might need to set a fixed value of 30 seconds as well. I think that would be acceptable in terms of resource use.

tobi-wan-kenobi commented 5 years ago

Sounds good to me as well! Thanks for your efforts!

cddmp commented 5 years ago

I got it working, before I do a PR I need some more testing with different screensaver.

Still there is one thing I might misunderstand: From reading the code I can use self.interval()to set the interval when the module is updated. In engine.py this value is casted to int and multiplied by 60: https://github.com/tobi-wan-kenobi/bumblebee-status/blob/9b7155b45972b5ac84c46d7307ae56a920e5b2e2/bumblebee/engine.py#L102

Because of that it is not possible to do something like self.interval(0.5) which would yield to 30 seconds. So the minimum time seems to be 1 minute. Or have I overlooked anything? If not, is there a specific reason why it was done like that? With casting to float instead of int and multiplying by 60.0 instead of 60 a more fine grain interval could be set. Which would help in this case to set an interval of 30 seconds. And in theory it should not break existing modules. :)

cddmp commented 5 years ago

While testing the code I came across an interesting issue: The code works fine, if none of the windows is in fullscreen mode. As soon as a window is fullscreen, the code doesn't work any longer. After some debugging it turned out, that as soon as a fullscreen window exists, bumblebee-status is put to sleep. It will sleep as long as the fullscreen window exists. Once the fullscreen windows is minimized, bumblebee-status works again. You can easily test this yourself. The bash one liner below will print the date/time when bumblebee-status ran the last time. Run this in a shell and then put a window to fullscreen and wait for some minutes, then close it and have a look at the output. while true; do ps o state,command axh | grep -q "^[R].*[b]umblebee-status" && date; done

Before I put effort into debugging this: Is this known? Is this propably an i3-wm feature to save resources?

tobi-wan-kenobi commented 5 years ago

That is an interesting find!

Looking at https://github.com/i3/i3/issues/2790 and https://github.com/sardemff7/j4status/issues/19, it seems this is intentional.

cddmp commented 5 years ago

I just came across this: https://github.com/i3/i3/blob/48af067dfe56545ccf0f462db4f62bb12ac16004/docs/userguide It is documented under "=== Display mode". Oh dear, that makes things a bit more complicated. I will think about a solution. :)

tobi-wan-kenobi commented 5 years ago

Thanks for researching this! Maybe we can just add an option that captures and ignores SIGSTOP?

⁣Sent from TypeApp ​

On Aug 26, 2019, 20:58, at 20:58, mw notifications@github.com wrote:

I just came across this: https://github.com/i3/i3/blob/48af067dfe56545ccf0f462db4f62bb12ac16004/docs/userguidee It is documented under "=== Display mode". Oh dear, that makes things a bit more complicated. I will think about a solution. :)

-- You are receiving this because you were assigned. Reply to this email directly or view it on GitHub: https://github.com/tobi-wan-kenobi/bumblebee-status/issues/426#issuecomment-524985478

cddmp commented 5 years ago

From what I remember SIGSTOP and SIGKILL can not be ignored. :(

cddmp commented 5 years ago

I think I have a PoC, but it I need to do some more testing. So far it's just a PoC, not sure if it is too ugly...

The idea is that xdg-screensaver supports some sort of daemon mode. That means, it can run as long as a specific window is open. For this I get the window id from i3bar and let xdg-screensaver run with that. I then do a double fork to run xdg-screensaver. That way I can detach it from from the parent process and therefore escape SIGSTOP. xdg-screenserver can be stopped by killing a specific process (xprop) which xdg-screensaver is waiting for. There should be no damage with killing xprop.

tobi-wan-kenobi commented 5 years ago

Sounds like a working solution, and actually I find it quite elegant. Looking forward to seeing the code!

If you want to, we can put it in an experimental branch for now.

cddmp commented 5 years ago

Thank you and sorry for my late reply, I haven't had much time during the last days. I've tested it in the meantime with XScreenSaver, xautolock and plain DPMS and screensaver settings. It works fine! What I meant with ugly is, that I basically misuse the window id feature. Another way of doing it would be to just let a while() loop run forever in a spawned process which would call "screensaver reset" every n seconds. That way xdotool would not be needed. The downside with this approach it, that if the while() loop process is killed, it might leave an inconsistent state. That's why the current approach seemed better to me but just a bit ugly or hacky. But if you like it, it is fine for me too. Yes, we could put it in an experimental branch for now. :)

tobi-wan-kenobi commented 5 years ago

Ah, I see. No, I think I like your current way of doing it better - the while loop seems quite hacky and fragile.

Looking forward very much to a PR! :)

Thanks a lot for the effort you're putting into this.