Merge lp:~lool/qtpowerd/misc-fixes into lp:~music-app-dev/qtpowerd/trunk

Proposed by Loïc Minier
Status: Merged
Merged at revision: 5
Proposed branch: lp:~lool/qtpowerd/misc-fixes
Merge into: lp:~music-app-dev/qtpowerd/trunk
Diff against target: 233 lines (+83/-47)
7 files modified
QtPowerd.cpp (+50/-23)
QtPowerd.h (+12/-7)
debian/changelog (+6/-0)
plugin.cpp (+6/-6)
plugin.h (+3/-4)
qmldir (+2/-3)
qtpowerd.pro (+4/-4)
To merge this branch: bzr merge lp:~lool/qtpowerd/misc-fixes
Reviewer Review Type Date Requested Status
David Planella Needs Information
Review via email: mp+188718@code.launchpad.net

Commit message

Misc fixes allowing qtpowerd to function with music-app.

Description of the change

Hi there,

Misc fixes in here, but more importantly this fixes the requestSysState() and clearSysState() so that it basically works.

On the long run, I think we ought to:
* never use this from apps since some backgrond music service should do it for them
* allow for multiple locks or various types, even if apps should not take them most of the time, sometimes it makes sense; there are other reasons for bindings such as tracking display state changes (as unity8 Shell does)
* allow naming the locks (currently qtpowerd hardcodes "music-app-background")
* move this to powerd itself to provide; this would allow using the header

However I think this is good enough for music-app for 13.10 :-)

I might try to fix some of the other bits there if time allow.

I also wonder if the name Powerd 0.1 might break unity8's Powerd 0.1 plugin.

To post a comment you must log in.
lp:~lool/qtpowerd/misc-fixes updated
17. By Loïc Minier

Rename from Powerd to QtPowerd to avoid clash with Unity8 Shell plugi.

18. By Loïc Minier

Bump to 0.2.

Revision history for this message
Loïc Minier (lool) wrote :

So indeed this clashed with Unity8 Shell's powerd plugin; fixed in r17/r18.

Revision history for this message
Victor Thompson (vthompson) wrote :

The plugin is under Ubuntu.Powerd, so I don't think it should break Unity's plugin. Changing the version to 0.2 doesn't necessary fix the issue--if it exists--however. Maybe we should rename the plugin to something directly related to "keepAlive"?

Revision history for this message
Victor Thompson (vthompson) wrote :

Nevermind my previous comment. I thought you bumped the version to deconflict with the Unity plugin. Renaming the plugin as you are should accomplish this.

Revision history for this message
David Planella (dpm) wrote :

I guess another solution would have been that we install it as a private plugin: the settings app does that for example, each panel being a private plugin installed within the app and not accessible by other applications.

So either the music app or Unity could have installed their powerd plugin privately to prevent it being picked up by other apps.

The question that comes to mind is: could we not just have one single powerd plugin that can be used by both Unity and the Music app?

review: Needs Information

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== renamed file 'Powerd.cpp' => 'QtPowerd.cpp'
2--- Powerd.cpp 2013-09-25 23:31:51 +0000
3+++ QtPowerd.cpp 2013-10-01 21:50:24 +0000
4@@ -16,39 +16,66 @@
5 * Author: Michael Terry <michael.terry@canonical.com>
6 */
7
8-#include "Powerd.h"
9-
10-Powerd::Powerd(QObject* parent)
11+#include <QtDebug>
12+
13+/* FIXME should really include powerd.h, but not available right now */
14+//#include <powerd.h>
15+enum SysPowerStates {
16+ //Note that callers will be notified of suspend state changes
17+ //but may not request this state.
18+ POWERD_SYS_STATE_SUSPEND = 0,
19+
20+ //The Active state will prevent system suspend
21+ POWERD_SYS_STATE_ACTIVE,
22+
23+ POWERD_NUM_POWER_STATES
24+};
25+
26+#include "QtPowerd.h"
27+
28+// XXX this should be set by QML app; in fact it could hold multiple ones
29+#define LOCK_NAME "music-app-background"
30+
31+QtPowerd::QtPowerd(QObject* parent)
32 : QObject(parent),
33- m_powerd(NULL)
34+ m_keepAlive(false),
35+ m_qtpowerd(NULL)
36 {
37- m_powerd = new QDBusInterface("com.canonical.powerd",
38- "/com/canonical/powerd",
39- "com.canonical.powerd",
40- QDBusConnection::SM_BUSNAME(), this);
41+ m_qtpowerd = new QDBusInterface("com.canonical.powerd",
42+ "/com/canonical/powerd",
43+ "com.canonical.powerd",
44+ QDBusConnection::SM_BUSNAME(), this);
45 }
46
47-bool Powerd::keepAlive() const
48+bool QtPowerd::keepAlive() const
49 {
50 return m_keepAlive;
51 }
52
53-void Powerd::setKeepAlive(bool keep)
54+void QtPowerd::setKeepAlive(const bool keep)
55 {
56- m_keepAlive = keep;
57+ if (m_keepAlive == keep) {
58+ // nothing to do
59+ return;
60+ }
61+
62 if (keep) {
63- m_cookie = m_powerd->connection().connect("com.canonical.powerd",
64- "/com/canonical/powerd",
65- "com.canonical.powerd",
66- "RequestSysState",
67- this,
68- METHOD(requestSysState("music-app-background", 1)));
69+ QDBusReply<QString> reply = m_qtpowerd->call("requestSysState",
70+ LOCK_NAME,
71+ POWERD_SYS_STATE_ACTIVE);
72+ if (!reply.isValid()) {
73+ qCritical() << "requestSysState:" << reply.error();
74+ return;
75+ }
76+ m_cookie = reply.value();
77 } else {
78- m_cookie = m_powerd->connection().connect("com.canonical.powerd",
79- "/com/canonical/powerd",
80- "com.canonical.powerd",
81- "ClearSysState",
82- this,
83- METHOD(clearSysState(m_cookie)));
84+ QDBusReply<void> reply = m_qtpowerd->call("clearSysState", m_cookie);
85+ if (!reply.isValid()) {
86+ qCritical() << "clearSysState:" << reply.error();
87+ return;
88+ }
89 }
90+
91+ m_keepAlive = keep;
92+ Q_EMIT keepAliveChanged();
93 }
94
95=== renamed file 'Powerd.h' => 'QtPowerd.h'
96--- Powerd.h 2013-09-25 13:04:12 +0000
97+++ QtPowerd.h 2013-10-01 21:50:24 +0000
98@@ -17,26 +17,31 @@
99 * Michael Terry <michael.terry@canonical.com>
100 */
101
102-#ifndef UBUNTU_POWERD_H
103-#define UBUNTU_POWERD_H
104+#ifndef QTPOWERD_H
105+#define QTPOWERD_H
106
107 #include <QtCore/QObject>
108 #include <QtDBus/QDBusInterface>
109+#include <QtDBus/QDBusReply>
110
111-class Powerd: public QObject
112+class QtPowerd: public QObject
113 {
114 Q_OBJECT
115 Q_PROPERTY(bool keepAlive READ keepAlive WRITE setKeepAlive NOTIFY keepAliveChanged)
116+
117 public:
118- explicit Powerd(QObject *parent = 0);
119+ explicit QtPowerd(QObject *parent = 0);
120+
121 bool keepAlive() const;
122- void setKeepAlive(bool keepAlive);
123-signals:
124+ void setKeepAlive(const bool keepAlive);
125+
126+Q_SIGNALS:
127 void keepAliveChanged();
128+
129 private:
130 QString m_cookie;
131 bool m_keepAlive;
132- QDBusInterface *m_powerd;
133+ QDBusInterface *m_qtpowerd;
134 };
135
136 #endif
137
138=== modified file 'debian/changelog'
139--- debian/changelog 2013-09-25 13:04:12 +0000
140+++ debian/changelog 2013-10-01 21:50:24 +0000
141@@ -1,3 +1,9 @@
142+qtpowerd (0.2-0ubuntu1) UNRELEASED; urgency=low
143+
144+ * Bump to 0.2.
145+
146+ -- Loïc Minier <loic.minier@ubuntu.com> Tue, 01 Oct 2013 23:11:10 +0200
147+
148 qtpowerd (0.0.20130924-0ubuntu1) saucy; urgency=low
149
150 * Initial release.
151
152=== modified file 'plugin.cpp'
153--- plugin.cpp 2013-09-26 00:12:18 +0000
154+++ plugin.cpp 2013-10-01 21:50:24 +0000
155@@ -18,19 +18,19 @@
156 */
157
158 #include "plugin.h"
159-#include "Powerd.h"
160+#include "QtPowerd.h"
161
162 #include <QtQml/qqml.h>
163
164-static QObject *powerd_provider(QQmlEngine *engine, QJSEngine *scriptEngine)
165+static QObject *qtpowerd_provider(QQmlEngine *engine, QJSEngine *scriptEngine)
166 {
167 Q_UNUSED(engine)
168 Q_UNUSED(scriptEngine)
169- return new Powerd();
170+ return new QtPowerd();
171 }
172
173-void PowerdPlugin::registerTypes(const char *uri)
174+void QtPowerdPlugin::registerTypes(const char *uri)
175 {
176- Q_ASSERT(uri == QLatin1String("Powerd"));
177- qmlRegisterSingletonType<Powerd>(uri, 0, 1, "Powerd", powerd_provider);
178+ Q_ASSERT(uri == QLatin1String("QtPowerd"));
179+ qmlRegisterSingletonType<QtPowerd>(uri, 0, 1, "QtPowerd", qtpowerd_provider);
180 }
181
182=== modified file 'plugin.h'
183--- plugin.h 2013-09-25 13:04:12 +0000
184+++ plugin.h 2013-10-01 21:50:24 +0000
185@@ -17,13 +17,12 @@
186 * Michael Terry <michael.terry@canonical.com>
187 */
188
189-#ifndef POWER_PLUGIN_H
190-#define POWER_PLUGIN_H
191+#ifndef QTPOWERD_PLUGIN_H
192+#define QTPOWERD_PLUGIN_H
193
194-#include <QtQml/QQmlEngine>
195 #include <QtQml/QQmlExtensionPlugin>
196
197-class PowerdPlugin : public QQmlExtensionPlugin
198+class QtPowerdPlugin : public QQmlExtensionPlugin
199 {
200 Q_OBJECT
201 Q_PLUGIN_METADATA(IID "org.qt-project.Qt.QQmlExtensionInterface")
202
203=== modified file 'qmldir'
204--- qmldir 2013-09-25 13:04:12 +0000
205+++ qmldir 2013-10-01 21:50:24 +0000
206@@ -1,3 +1,2 @@
207-module Ubuntu.Powerd
208-plugin qmlpowerdplugin
209-typeinfo plugin.qmltypes
210+module QtPowerd
211+plugin qmlqtpowerdplugin
212
213=== modified file 'qtpowerd.pro'
214--- qtpowerd.pro 2013-09-30 14:38:19 +0000
215+++ qtpowerd.pro 2013-10-01 21:50:24 +0000
216@@ -5,14 +5,14 @@
217
218 PKGCONFIG =
219
220-TARGET = qmlpowerdplugin
221-PLUGIN_IMPORT_PATH = Ubuntu/Powerd.0.1
222+TARGET = qmlqtpowerdplugin
223+PLUGIN_IMPORT_PATH = QtPowerd.0.1
224
225-SOURCES += Powerd.cpp plugin.cpp
226+SOURCES += QtPowerd.cpp plugin.cpp
227
228 DEFINES += "SM_BUSNAME=systemBus"
229
230-HEADERS += Powerd.h plugin.h
231+HEADERS += QtPowerd.h plugin.h
232
233 equals(QT_MAJOR_VERSION, 5): target.path = $$[QT_INSTALL_QML]/$$PLUGIN_IMPORT_PATH
234

Subscribers

People subscribed via source and target branches

to all changes: