Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

swayidle does not light up a screen upon pressing a key #2914

Closed
Hi-Angel opened this issue Oct 21, 2018 · 25 comments · Fixed by #3009
Closed

swayidle does not light up a screen upon pressing a key #2914

Hi-Angel opened this issue Oct 21, 2018 · 25 comments · Fixed by #3009

Comments

@Hi-Angel
Copy link
Contributor

Per this comment, the preferred way of lighting up the screen from dpms is through swayidle. Unfortunately, for me the screen remains dark even with swayidle running in background.

Steps to reproduce

  1. Execute in one terminal swayidle timeout 600 'swaymsg "output * dpms off"' resume 'swaymsg "output * dpms on"'
  2. Execute in another terminal sleep 1 && swaymsg "output * dpms off"
  3. Press a key

Expected

Screens light up back

Actual

Screens stay black, and I see no way to light them up.

Additional info

sway build from today, commit a4d6835.

@Hi-Angel Hi-Angel changed the title swayidle does not resume upon pressing a key swayidle does not light up a screen upon pressing a key Oct 21, 2018
@emersion
Copy link
Member

This is expected. Outputs are turned back on only if swayidle turned them off, not if you did so manually.

I'd also like a way to say to swayidle "please turn my outputs off", not sure if it's in scope.

@progandy
Copy link
Contributor

progandy commented Oct 21, 2018

Maybe sway needs something like a oneshot bindkey to bind to any user input like mouse movement, clicks or pressed keys. Something like one or more of these options:

bindsym --once UserInput <command>
bindsym --once AnyKey <command>
bindsym --once MouseMove <command>
bindsym --once MouseClick <command>

@Hi-Angel
Copy link
Contributor Author

@SirCmpwn why did you close it? Shall I open a separate issue, unrelated to swayidle, on not being able to light up a screen?

@ddevault ddevault reopened this Oct 21, 2018
@ddevault
Copy link
Member

I mistook this for the earlier issue and thought I had forgotten to close it.

@emersion
Copy link
Member

Shall I open a separate issue, unrelated to swayidle, on not being able to light up a screen?

You can light up a screen. Just run output * dpms on.

@tg--
Copy link

tg-- commented Oct 23, 2018

I was just stumbling upon the same thing, my usecase is:
I want to turn off the display immediately on demand (for example after a movie finishes), by script.
A solution that would work for me would be, if swayidle itself hat a oneshot/once option, i.e. run:
swayidle --once timeout 1 'swaymsg "output * dpms off"' resume 'swaymsg "output * dpms on"'

After that, swayidle exits only when the display comes back on.

@martinetd
Copy link
Member

The problem is that swayidle wouldn't even get a resume event in that case, because the compositor only sends it if it considers it was idle when activity happens (can't really go around and notify every listener everytime something moves)

We need to add a way to tell the compositor "from here on notify me the next time something happens" or something similar; I don't think there's any way to do that with the current protocols. Maybe just set a 0s idle timeout? Have you tried something like this (can improve the pkill if it works)?
swayidle timeout 0 'swaymsg "output * dpms off"' resume 'swaymsg "output * dpms on"; pkill -n swayidle' ?

@tg--
Copy link

tg-- commented Oct 23, 2018

@martinetd: this won't work for multiple reasons: timeout 0 is not possible right now; but mainly, swayidle will not exit and thus the pkill will never be reached.
That's where the oneshot-option would be helpful (in which case pkill wouldn't be needed anyway).

However, it's of course limited in that it still doesn't detect other "dpms off" calls - but those could just be replaced with a 'oneshot ... off resume on' which could even be its own shortcut (swayidle --now?) or an alias.

@martinetd
Copy link
Member

but mainly, swayidle will not exit and thus the pkill will never be reached.

I have no idea what you're saying here. I just tried with timeout 1 and get what I would expect, but anyway as I said this can be improved and made into oneshot/whatever as you want it's just a user program (and you can always just fork it/maintain a local patch if Sircmpwn doesn't want to see that)

Anyway, the interesting part of what I posted earlier is that you won't get a resume event unless you setup idle, so you need to create an immediate timer and fix that "timeout 0". No idle, no resume.

@Hi-Angel
Copy link
Contributor Author

I think, a way to move forward would be an addition to a protocol between swayidle and sway, in which sway tells swayidle that screens have just been powered off, so swayidle might start looking for "non-idle" events.

@emersion
Copy link
Member

As others have pointed out, this could be done by just handling SIGUSR1 in swayidle, making it toggle the idle state.

@tg--
Copy link

tg-- commented Oct 25, 2018

I've looked at the swayidle code now, and considered a few different things to accomplish this:

  1. SIGUSR1: this sounded like a good solution at first; however, implementing it would be really ugly, since swayidle itself has actually no concept of DPMS and just uses swaymsg to communicate this to the compositor. For SIGUSR1 to work we would then need to either hardcode a swaymsg command to turn the display off and on, which is very ugly, or go through the cmd list and just pick the first one containing dpms which is equally ugly.
    Just picking the first one (i.e. toggling idle state) won't work, since the point is to turn the display off and make sure it comes on again, and the first idle command is not guaranteed to do this.

  2. Oneshot: Still seems somewhat reasonable, but it's a bit clumsy since there are probably 2 instances of swayidle running and it's actually somewhat tricky to figure out when to terminate swayidle (has to be something like at the first resume command)

  3. Adding a top level "resume" command to swayidle which is not conditional to the "timeout" command and will be run at every resume event

@Hi-Angel's solution seems also reasonable, though this would require a new RPC mechanism (or maybe just a registering a new WL event?), however, it would also likely cause side-effects, like in case a user wants to turn a secondary display off permanently.

Thoughts?

@martinetd
Copy link
Member

thought: read what I said; your "new protocol" is exactly requesting a new idle timer, possibly off the same manager, with a 0 timeout.
(that'd work with either signal or oneshot, but as said you need to fix 0. You can test with a 1ms timeout until then)

@Hi-Angel
Copy link
Contributor Author

Hi-Angel commented Oct 25, 2018

however, it would also likely cause side-effects, like in case a user wants to turn a secondary display off permanently.

Not necessarily, if combined with your 3-rd point.

@Hi-Angel
Copy link
Contributor Author

thought: read what I said; your "new protocol" is exactly requesting a new idle timer, possibly off the same manager, with a 0 timeout.

@martinetd but this means that now you can't set a timer to, say, 600 sec. to disable screens automatically.

@martinetd
Copy link
Member

martinetd commented Oct 25, 2018

No. timers can coexist with different timings with no problem, you can have how many timers or how many idle managers you want in parallel. See what I wrote earlier.
(EDIT: ok, I wasn't specific about this part, thought I was; but you can read the idle.xml protocol before asking for a new one, these aren't too bad to look at)

@Hi-Angel
Copy link
Contributor Author

@martinetd but then if you implemented a zero-based timer, how would swayidle know that it don't need to execute "resume" command, i.e. because screens are still enabled. This requires sway to notify swayidle that screens have been disabled; in which case we're down to my proposal, combined with either your zero-based timer, or the third @tg--'s point. And I'm in preference of the @tg--'s 3-rd point, because I really don't see any usecase for binding to it a timer.

@martinetd
Copy link
Member

Err? Why would it not need a resume command? If you don't need a resume command, you don't need swayidle, just run the damn initial command.

I'll repeat myself one last time: you need something to ask the compositor to tell you when there is activity.
idle as it is has two stages: it starts active, then if there is no activity until timer hits notifies the client once (idle command) and enters idle state.
In idle state, it will notify the first activity to the client and exit idle state, waiting for timer again.

Basically what you need here is to add a new more/handle an event/whatever that would let you first execute a command and immediately enter idle state, to be able to wake up screens on first activity, then destroy that thing.

a 0-timer would be just that: create timer, (run your thing now or run it on the immediate idle event), run resume command when it comes, kill timer.

I do not care what kind of API you give to that, you can execute the same command for all resume events or you can do them individually (it'd make more sense to do them individually if you only want to shutdown a single screen one-shot e.g. for video playback on one screen with multiscreen, but that's not my problem), but this doesn't need anything new. It's a pure user/client program, and that doesn't need anything more from the compositor, just do it and send a PR.

(TLDR: I'm sorry if you think that's rude, I'm just tired of repeating what I consider to be the same thing over and over; I won't reply to this issue anymore.)

@Hi-Angel
Copy link
Contributor Author

Err? Why would it not need a resume command? If you don't need a resume command, you don't need swayidle, just run the damn initial command.

@martinetd you misinterpreted what I said, I meant that when screens are turned on, swayidle should not execute resume command, and the question is how would swayidle know about it.

But whatever.

Basically what you need here is to add a new more/handle an event/whatever that would let you first execute a command and immediately enter idle state, to be able to wake up screens on first activity, then destroy that thing.

a 0-timer would be just that: create timer, (run your thing now or run it on the immediate idle event), run resume command when it comes, kill timer.

Now that I'm reading these two paragraphs, I'm completely confused. I just not sure how to interpret the (run your thing now or run it on the immediate idle event). Do you want to run a command immediately as swayidle launched, or what?

I think, to lessen confusion it would help if you pulled up some example of how would your idea be used to implement a generic DPMS (which is "disable screens either on timer or explicitly, then enable them back on the first activity").

I'm sorry if you think that's rude

No, you're not being rude, it's a technical discussion.

I'm just tired of repeating what I consider to be the same thing over and over

You did not repeat that, you wrote it once, and then were referring back to your first comment. Which, as it seems now, somewhat unclear to me.

@emersion
Copy link
Member

I also think it's possible to implement this feature with a zero timer.

I meant that when screens are turned on, swayidle should not execute resume command, and the question is how would swayidle know about it.

We don't care about it. Turning screens on when they are on is a no-op. Additionally, it's unrelated to this issue -- it also happens with the current implementation.

Do you want to run a command immediately as swayidle launched, or what?

No. Set up a timer with a zero timeout on SIGUSR1. When it expires, turn off screens.

@Hi-Angel
Copy link
Contributor Author

Turning screens on when they are on is a no-op.

Well, then the question is 'how often that "no-op" gonna happen"? You don't want, for example, spam it on every activity, right? This is not good from battery perspective.

This is why I'm asking an overall example of how would that be used to implement DPMS: because an overall example would also answer this trivia questions.

@martinetd
Copy link
Member

Now that I'm reading these two paragraphs, I'm completely confused. I just not sure how to interpret the (run your thing now or run it on the immediate idle event). Do you want to run a command immediately as swayidle launched, or what?

As you set the new timer. That can be as you send a signal to the running swayidle, start the swayidle --once, or whatever you want the user experience to be.

I think, to lessen confusion it would help if you pulled up some example of how would your idea be used to implement a generic DPMS (which is "disable screens either on timer or explicitly, then enable them back on the first activity").

I'm not talking about generic DPMS, I'm talking about the idle protocol; it can do DPMS if you want it to. I've stayed abstract because I don't want to impose anything, talking about commands because that's the current swayidle "interface" e.g. run commands when an idle/resume event comes
All I'm saying here is that what you want to do (setup something to black the screen and wake it up on activity) is possible with the current wayland protocols.
(with the exception that timeout = 0 doesn't work, but it's a wlroots bug because wl_event_source_timer_update considers timeout = 0 to be disable timer, not a protocol limitation)

Well, then the question is 'how often that "no-op" gonna happen"? You don't want, for example, spam it on every activity, right? This is not good from battery perspective.

resume event isn't sent if you're not idle, you can create and destroy timers anytime, timers are independant.
You can have your 5/10/whatever minutes timer run, and when you get signal setup a new 0-timer just to get new resume events; then once that's resumed destroy that new timer and be back to normal.
The only overlap is if you've forced idle, and the other timer elapses, you'll get two resume events... That's what he referred to as no-op, but if you want you could notice that and not do it, although I wouldn't recommend it if commands are different.

@emersion
Copy link
Member

'how often that "no-op" gonna happen"?

You only set up the zero timer on SIGUSR1, so this isn't a concern.

@progandy
Copy link
Contributor

If you need pseudo code, here:

swayidle --usr1-idle DPMS_OFF --usr1-resume DPMS_ON
  LOOP:
    wait for SIGUSR1
    create timer 0-timeout
    react to timeout 0 or directly call --usr1-idle
    wait for resume
    call --usr1-resume
    destroy timer 0-timeout
  goto LOOP

killall -USR1 swayidle

Or as oneshot:

swayidle --oneshot DPMS_OFF --resume DPMS_ON
    create timer 0-timeout
    call --oneshot
    wait for resume
    call --resume
    destroy timer 0-timeout
    exit swayidle

@emersion
Copy link
Member

Note that we can drop usr1 from the CLI options and just use the normal ones.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging a pull request may close this issue.

6 participants