Postpone device settings change if scanning is in progress
ClosedPublic

Authored by trufanov on Feb 12 2019, 5:49 PM.

Details

Summary

This shall detect if device settings change was unsuccessful bcs scanning is in progress and automatically try to apply this change again when device becomes available.

requires https://phabricator.kde.org/D17510

Test Plan

As Skanlite blocks UI during scanning the only way to test this is by attaching some D-Bus hotkeys to some settings profiles and try to use these hotkeys for settings switching while scanning is in progress.

Diff Detail

Repository
R483 Skanlite
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.
trufanov requested review of this revision.Feb 12 2019, 5:49 PM
trufanov created this revision.
sars added a comment.Feb 12 2019, 6:38 PM

The idea is sane :)

src/skanlite.cpp
674

Isn't this instance called at startup? Probably does not need the delayed setting...

src/skanlite.h
106

Why do you complicate it with a QScopedPointer?

you can do: "map = otherMap;" and "map.clear();" to do the stuff you do with QScopedPointer::reset(...)

trufanov added inline comments.Feb 12 2019, 9:14 PM
src/skanlite.cpp
674

Perhaps but I would rather do this for uniformality. Usage applyScannerOptions instead of direct call to setOptVals I consider more safe and tried to replace all its occurrences with this variant. There are no more setOptVals in code except one in applyScannerOptions after this patch.

src/skanlite.h
106

Well, I guess empty QScopedPointer takes a few bytes less RAM than empty QMap . And the case when it's non empty is quite rare and can be (currently) reached only via D-Bus hotkeys. Guess it's just my Symbian smartphone coding background cause me minimize RAM usage. I don't mind to change this.

sars accepted this revision.Feb 20 2019, 12:59 PM

Kåre, could you comment my reply?: https://phabricator.kde.org/D18966#411134

I was under the impression that you already where planning to remove the QScopedPointer... so I was waiting for that update. I would prefer to have the pointer removed for the readability of the code.

Regarding the other comment I had, it is OK as it is.

This revision is now accepted and ready to land.Feb 20 2019, 12:59 PM
trufanov updated this revision to Diff 52349.Feb 22 2019, 11:39 PM

Now without QScopedPointer

This revision was automatically updated to reflect the committed changes.