Add a global shortcut action to turn off the screen
ClosedPublic

Authored by mblumenstingl on Thu, Jul 4, 10:04 AM.

Details

Summary

PowerDevilDPMSAction already provides implementations for X11 and Wayland which allow turning off the screen.
For X11 there was a way to do this on the command line (with "xset dpms force off"), but there's no such replacement for Wayland yet.

Add an action - to which users can assign a shortcut - to turn off the screen.

Test Plan

start a plasma wayland session
open systemsettings5 -> Shortcuts -> Global Shortcuts -> Power Management
assign a shortcut to "Turn Off Screen"
press this shortcut
(screen must now turn off)

Diff Detail

Repository
R122 Powerdevil
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.
mblumenstingl created this revision.Thu, Jul 4, 10:04 AM
Restricted Application added a subscriber: plasma-devel. · View Herald TranscriptThu, Jul 4, 10:04 AM
mblumenstingl requested review of this revision.Thu, Jul 4, 10:04 AM

Thanks a lot for your patch!
I have some minor style nitpicks (you probably just copied them from elsewhere but if we're adding new code, it should be tidy ;)

Bonus points if you also make a patch for kscreenlocker globalaccel.cpp to whitelist this new action so that it also works on the lock screen.

daemon/actions/dpms/powerdevildpmsaction.cpp
42

Use #include <KGlobalAccel> and sort it correctly into the other includes

78

Coding style: KActionCollection *actionCollection (asterisk after space)

83

I suppose there's no Qt::Key_ enum for that, given laptops typically bypass the operating system here anyway.

84

Use new style connect, you can probably just use a lambda

connect(globalAction, &QAction::triggered, this, [this] {
    if (m_helper) {
        m_helper->trigger(QStringLiteral("TurnOff"));
    }
});
daemon/actions/dpms/powerdevildpmsaction.h
57 ↗(On Diff #61118)

For SLOT(...) syntax to work this must be in a section private Q_SLOTS, but note my comment below about using a lambda instead

mblumenstingl marked 5 inline comments as done.

updated patch to address the comments from @broulik

daemon/actions/dpms/powerdevildpmsaction.cpp
83

I couldn't find any appropriate Qt::Key_ so I'm leaving it up to the user to choose one

84

that if (m_helper) is probably the overloaded != nullptr from https://doc.qt.io/qt-5/qscopedpointer.html#operator-not-eq-1
thanks for this hint, it's been a long time since I last did some c++/Qt

daemon/actions/dpms/powerdevildpmsaction.h
57 ↗(On Diff #61118)

sorry for that, I missed that when I moved from a public slot to a private one
your lambda expression is working fine so I'm going with that instead

Bonus points if you also make a patch for kscreenlocker globalaccel.cpp to whitelist this new action so that it also works on the lock screen.

I tried adding |Turn Off Screen to the powerdevil QRegularExpression but I couldn't turn off the display from the lock screen
my test plan for this is:

  • original test plan
  • move mouse so the screen turns on again
  • application launcher -> Leave -> Lock -> confirm
  • press my shortcut
  • (nothing happens)
broulik accepted this revision.Thu, Jul 4, 12:18 PM

Alright, very nice!
I suppose you don't have commit access, so I need an email address from you to add you as commit author. Thanks!

I tried adding |Turn Off Screen to the powerdevil QRegularExpression but I couldn't turn off the display from the lock screen

You probably need to log out and back in for the new screenlocker binary to be loaded (or run the kscreenlocker_test binary in the build/bin directory)

This revision is now accepted and ready to land.Thu, Jul 4, 12:18 PM

Alright, very nice!
I suppose you don't have commit access, so I need an email address from you to add you as commit author. Thanks!

I once had commit access but I'm not sure if I still have.
either way, I would be more comfortable with you committing this with the following author information: Martin Blumenstingl <martin.blumenstingl@googlemail.com>

I tried adding |Turn Off Screen to the powerdevil QRegularExpression but I couldn't turn off the display from the lock screen

You probably need to log out and back in for the new screenlocker binary to be loaded (or run the kscreenlocker_test binary in the build/bin directory)

alright, I'll give this another go and open another review

This revision was automatically updated to reflect the committed changes.