Differential D3617 Diff 8848 kcms/touchpad/src/backends/kwin_wayland/kwinwaylandbackend.cpp

Changeset View

Standalone View

View Options

kcms/touchpad/src/backends/kwin_wayland/kwinwaylandbackend.cpp

  • This file was added.
1​/*
2​ * Copyright (C) 2016 Roman Gilg <subdiff@gmail.com>
3​ *
4​ * This program is free software; you can redistribute it and/or modify
5​ * it under the terms of the GNU General Public License as published by
6​ * the Free Software Foundation; either version 2 of the License, or
7​ * (at your option) any later version.
8​ *
9​ * This program is distributed in the hope that it will be useful,
10​ * but WITHOUT ANY WARRANTY; without even the implied warranty of
11​ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
12​ * GNU General Public License for more details.
13​ *
14​ * You should have received a copy of the GNU General Public License
15​ * along with this program; if not, write to the Free Software
16​ * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA.
17​ */
18​
19​#include "kwinwaylandbackend.h"
20​#include "kwinwaylandtouchpad.h"
21​
22​#include <KLocalizedString>
23​
24​#include <QDBusMessage>
25​#include <QDBusInterface>
26​#include <QDBusReply>
27​#include <QStringList>
28​
29​#include "logging.h"
30​
31​KWinWaylandBackend::KWinWaylandBackend(QObject *parent) :
32​ TouchpadBackend(parent)
33​{
34​
35​ m_deviceManager = new QDBusInterface (QStringLiteral("org.kde.KWin"),
36​ QStringLiteral("/org/kde/KWin/InputDevice"),
37​ QStringLiteral("org.kde.KWin.InputDeviceManager"),
38​ QDBusConnection::sessionBus(),
39​ this);
40​
41​ findTouchpads();
42​
43​ m_deviceManager->connection().connect(QStringLiteral("org.kde.KWin"),
44​ QStringLiteral("/org/kde/KWin/InputDevice"),
45​ QStringLiteral("org.kde.KWin.InputDeviceManager"),
46​ QStringLiteral("deviceAddedSysName"),
47​ this,
sebasUnsubmitted
Done

indenting four more spaces aligns it with the above

Yes, OCD. :)

sebas: indenting four more spaces aligns it with the above Yes, OCD. :)
48​ SLOT(onDeviceAdded(QString)));
49​ m_deviceManager->connection().connect(QStringLiteral("org.kde.KWin"),
50​ QStringLiteral("/org/kde/KWin/InputDevice"),
51​ QStringLiteral("org.kde.KWin.InputDeviceManager"),
52​ QStringLiteral("deviceRemovedSysName"),
53​ this,
54​ SLOT(onDeviceRemoved(QString)));
55​}
56​
57​KWinWaylandBackend::~KWinWaylandBackend()
58​{
59​ foreach (QObject *d, m_devices) {
60​ if (d) {
61​ delete d;
graesslinUnsubmitted
Done

in general you can delete a nullptr, so the ifcheck is not needed

graesslin: in general you can delete a nullptr, so the ifcheck is not needed
62​ }
63​ }
graesslinUnsubmitted
Done

check qDeleteAll

graesslin: check qDeleteAll
64​ if (m_deviceManager) {
65​ delete m_deviceManager;
66​ }
67​}
68​
69​void KWinWaylandBackend::findTouchpads()
70​{
71​ QStringList devicesSysNames;
72​ QVariant reply = m_deviceManager->property("devicesSysNames");
knambiarUnsubmitted
Not Done

Just wondering whether these calls are synchronous and may cause delay in startup.

knambiar: Just wondering whether these calls are synchronous and may cause delay in startup.
romanggAuthorUnsubmitted
Not Done

Just wondering whether these calls are synchronous and may cause delay in startup.

I assume so. Should be no problem though. We need the reply values anyway.

romangg: > Just wondering whether these calls are synchronous and may cause delay in startup. I assume…
graesslinUnsubmitted
Done

const

graesslin: const
73​ if (reply.isValid()) {
74​ qCDebug(KCM_TOUCHPAD) << "Devices list received successfully from KWin.";
75​ devicesSysNames = reply.toStringList();
76​ }
77​ else {
78​ qCCritical(KCM_TOUCHPAD) << "Error on receiving device list from KWin.";
79​ m_errorString = i18n("Error on receiving device list from KWin. Please restart Touchpad KCM.");
graesslinUnsubmitted
Done

We normally don't use the name "KWin" in user facing messages. Also I don't expect users to know what a "KCM" is ;-)

Suggestion: "Querying input devices failed. Please reopen this settings module".

graesslin: We normally don't use the name "KWin" in user facing messages. Also I don't expect users to…
80​ return;
81​ }
82​
83​ bool touchpadFound = false;
84​ foreach (QString sn, devicesSysNames) {
graesslinUnsubmitted
Done

please don't use the Q_FOREACH in new code as Qt might deprecate it.

graesslin: please don't use the Q_FOREACH in new code as Qt might deprecate it.
romanggAuthorUnsubmitted
Done

Are you sure? The foreach keyword is still listed in the official Qt docu. What's the best alternative? A normal for-loop? With upcounting integer or iterator?

romangg: Are you sure? The foreach keyword is still listed in the official Qt docu. What's the best…
luebkingUnsubmitted
Done

"might deprecate it" - magic eightball wisdom ;-)

The reason is that there's now
http://en.cppreference.com/w/cpp/language/range-for

luebking: "might deprecate it" - magic eightball wisdom ;-) The reason is that there's now http://en.
graesslinUnsubmitted
Done

https://www.kdab.com/goodbye-q_foreach/

graesslin: https://www.kdab.com/goodbye-q_foreach/
85​ QDBusInterface deviceIface(QStringLiteral("org.kde.KWin"),
86​ QStringLiteral("/org/kde/KWin/InputDevice/") + sn,
87​ QStringLiteral("org.kde.KWin.InputDevice"),
88​ QDBusConnection::sessionBus(),
89​ this);
90​ QVariant reply = deviceIface.property("touchpad");
graesslinUnsubmitted
Done

const

graesslin: const
sebasUnsubmitted
Done

remove whitespace between ! and tp

sebas: remove whitespace between ! and tp
91​ if (reply.isValid() && reply.toBool()) {
92​ KWinWaylandTouchpad* tp = new KWinWaylandTouchpad(sn);
93​ if (! tp->init()) {
94​ qCCritical(KCM_TOUCHPAD) << "Error on creating touchpad object" << sn;
95​ m_errorString = i18n("Error on creating touchpad object. Please restart KCM.");
graesslinUnsubmitted
Done

I doubt a restart would help here. That should run in an error again.

graesslin: I doubt a restart would help here. That should run in an error again.
96​ return;
97​ }
98​ m_devices.append(tp);
99​ touchpadFound = true;
100​ qCDebug(KCM_TOUCHPAD).nospace() << "Touchpad found: " << tp->name() << " (" << tp->sysName() << ")";
101​ }
102​ }
103​}
104​
105​bool KWinWaylandBackend::applyConfig()
106​{
107​ bool success = true;
108​ foreach(QObject *t, m_devices) {
109​ if (!static_cast<KWinWaylandTouchpad*>(t)->applyConfig()) {
110​ success = false;
111​ }
112​ }
113​ return success;
graesslinUnsubmitted
Done
return std::all_of(m_devices.constBegin(), m_devices.constEnd(),
    [] (QObject *t) { return static_cast<KWinWaylandTouchpad*>(t)->applyConfig(); });
graesslin: return std::all_of(m_devices.constBegin(), m_devices.constEnd(), [] (QObject *t) {…
114​}
115​
116​bool KWinWaylandBackend::getConfig()
117​{
118​ bool success = true;
119​ foreach(QObject *t, m_devices) {
120​ if (!static_cast<KWinWaylandTouchpad*>(t)->getConfig()) {
121​ success = false;
122​ }
123​ }
124​ return success;
graesslinUnsubmitted
Done

can also be simplified with algorithm

graesslin: can also be simplified with algorithm
125​}
126​
127​bool KWinWaylandBackend::getDefaultConfig()
128​{
129​ bool success = true;
130​ foreach(QObject *t, m_devices) {
131​ if (!static_cast<KWinWaylandTouchpad*>(t)->getDefaultConfig()) {
132​ success = false;
133​ }
134​ }
135​ return success;
graesslinUnsubmitted
Done

and here as well.

Given that you have three times the same method with just one smallish difference I would even suggest to use a std::function and use one method to rule them all

graesslin: and here as well. Given that you have three times the same method with just one smallish…
136​}
137​
138​bool KWinWaylandBackend::isChangedConfig() const
139​{
140​ bool ret = false;
141​ foreach(QObject *q, m_devices) {
142​ ret |= static_cast<KWinWaylandTouchpad*>(q)->isChangedConfig();
143​ }
144​ return ret;
graesslinUnsubmitted
Done

std::any_of

graesslin: std::any_of
145​}
146​
147​void KWinWaylandBackend::onDeviceAdded(QString sysName)
148​{
149​ foreach (QObject *d, m_devices) {
150​ KWinWaylandTouchpad *t = static_cast<KWinWaylandTouchpad*>(d);
151​ if (t->sysName() == sysName) {
152​ return;
153​ }
154​ }
graesslinUnsubmitted
Done

std::find_if

graesslin: std::find_if
155​
156​ QDBusInterface deviceIface(QStringLiteral("org.kde.KWin"),
157​ QStringLiteral("/org/kde/KWin/InputDevice/") + sysName,
158​ QStringLiteral("org.kde.KWin.InputDevice"),
159​ QDBusConnection::sessionBus(),
160​ this);
161​ QVariant reply = deviceIface.property("touchpad");
162​ if (reply.isValid() && reply.toBool()) {
163​ KWinWaylandTouchpad* tp = new KWinWaylandTouchpad(sysName);
164​ if (!tp->init() || !tp->getConfig()) {
165​ emit touchpadAdded(false);
166​ return;
167​ }
168​ m_devices.append(tp);
169​ qCDebug(KCM_TOUCHPAD).nospace() << "Touchpad connected: " << tp->name() << " (" << tp->sysName() << ")";
170​ emit touchpadAdded(true);
171​ }
172​}
173​
174​void KWinWaylandBackend::onDeviceRemoved(QString sysName)
175​{
176​ auto it = getDeviceIterator(sysName);
177​ if (it == m_devices.end()) {
178​ return;
179​ }
180​ KWinWaylandTouchpad *tp = static_cast<KWinWaylandTouchpad*>(*it);
181​ qCDebug(KCM_TOUCHPAD).nospace() << "Touchpad disconnected: " << tp->name() << " (" << tp->sysName() << ")";
182​ int index = it - m_devices.begin();
183​ m_devices.erase(it);
184​ emit touchpadRemoved(index);
185​}
186​
187​QList<QObject*>::iterator KWinWaylandBackend::getDeviceIterator(QString sysName)
188​{
189​ QList<QObject*>::iterator it;
190​ for (it = m_devices.begin(); it != m_devices.end(); ++it) {
191​ KWinWaylandTouchpad *t = static_cast<KWinWaylandTouchpad*>(*it);
192​ if (t->sysName() == sysName) {
193​ return it;
194​ }
195​ }
196​ return it;
197​}
graesslinUnsubmitted
Done

std::find_if

graesslin: std::find_if