Differential D2465

[kded] correctly restore lidOpened configuration on startup
ClosedPublic
Actions

Authored by sebas on Aug 17 2016, 3:34 AM.

Details

Reviewers
graesslin
Group Reviewers
Plasma
Commits
R104:f04d6d3c3c8a: [kded] correctly restore lidOpened configuration on startup
Summary

When shutting down the machine (or kded5, really) with the lid closed
will leave a config with the laptop panel disabled, and also a config
with the same id, but _lidOpened appended. The latter is the config that
is restored when the lid is opened again. When the lid is opened while
kded wasn't running (shut down), the config with the panel disabled is
restored.

We do want to restore the _lidOpened config, so check if it exists, and
if it does, move it to the configId to have it restored.

This effectively avoids a disabled laptop panel after the laptop has
been shut down with an external monitor connected and the lid closed.

BUG:353029

Test Plan
  • added autotest for this case
  • reproduced bug by killing kded with an external output connected and the laptop lid closed, after opening the lid, kded would correctly enable the laptop display on startup

Diff Detail

Repository
R104 KScreen
Branch
sebas/lidopeninit
Lint
No Linters Available
Unit
No Unit Test Coverage
sebas updated this revision to Diff 5986.Aug 17 2016, 3:34 AM
sebas retitled this revision from to [kded] correctly restore lidOpened configuration on startup.
sebas edited the test plan for this revision. (Show Details)
sebas added a reviewer: Plasma.
sebas updated this object.
Restricted Application added a project: Plasma. · View Herald TranscriptAug 17 2016, 3:34 AM
Restricted Application added a subscriber: plasma-devel. · View Herald Transcript
sebas updated this revision to Diff 5987.Aug 17 2016, 3:37 AM
  • --unused vars
sebas updated this revision to Diff 5988.Aug 17 2016, 3:40 AM
  • constness
graesslin added a subscriber: graesslin.Aug 17 2016, 6:53 AM
graesslin added inline comments.
kded/daemon.cpp
167–169 ↗(On Diff #5988)

how does that behave if the device has no lid?

sebas added inline comments.Aug 17 2016, 11:28 AM
kded/daemon.cpp
167–169 ↗(On Diff #5988)

It won't find a config file with _lidOpened appended, since that is only ever created when the lidClosedChanged signal is fired (which only happens when the device is a laptop and actually has a lid).

graesslin added inline comments.Aug 17 2016, 6:19 PM
kded/daemon.cpp
167–169 ↗(On Diff #5988)

So what is the code going to do here? Will it go into the code path here or not?

sebas updated this revision to Diff 6024.Aug 18 2016, 11:16 AM
  • Also check isLaptop)
sebas added inline comments.Aug 18 2016, 11:18 AM
kded/daemon.cpp
167–169 ↗(On Diff #5988)

Ah, I see what you mean. Yes, it would go in there but do nothing since QFile::exists(..) will return false -- the file is never created on non-laptops.

We can also avoid it altogether, by checking Device::isLaptop(), that seems prudent here, so I've added it -- it will save a file system access on non-laptop systems.

(Laptop systems in this sense are detected by querying UPower for the presence of a lid, so it's a logical thing to check.)

sebas marked 2 inline comments as done.Aug 18 2016, 11:19 AM
graesslin accepted this revision.Aug 19 2016, 6:27 AM
graesslin added a reviewer: graesslin.
This revision is now accepted and ready to land.Aug 19 2016, 6:27 AM
Closed by commit R104:f04d6d3c3c8a: [kded] correctly restore lidOpened configuration on startup (authored by sebas). · Explain WhyAug 19 2016, 10:43 AM
This revision was automatically updated to reflect the committed changes.

Revision Contents
Changeset List

PathPackages
Mkded/daemon.cpp (5 lines)
Mkded/serializer.h (2 lines)
Mkded/serializer.cpp (14 lines)
Mtests/kded/serializertest.cpp (87 lines)

Diff 6024

View Options

kded/daemon.cpp

Loading...
View Options

kded/serializer.h

Loading...
View Options

kded/serializer.cpp

Loading...
View Options

tests/kded/serializertest.cpp

Loading...