From 0f38eb34a4ee25f5a1c182822cebe5b44c788aaa Mon Sep 17 00:00:00 2001 From: Albert Vaca Date: Wed, 14 Aug 2013 01:35:12 +0200 Subject: [PATCH] Fixed crash: OMG QDbusAdaptors can not be removed! Added a fixme for future reference reloadPlugins() now recycles plugins already present Lots of debug messages and minor changes added trying to fix the bug --- daemon/device.cpp | 64 +++++++++++-------- .../plugins/battery/batterydbusinterface.cpp | 4 ++ daemon/plugins/battery/batterydbusinterface.h | 3 +- daemon/plugins/battery/batteryplugin.cpp | 12 +++- .../kdeconnect_notifications.desktop | 2 +- .../pausemusic/kdeconnect_pausemusic.desktop | 4 +- daemon/plugins/ping/pingplugin.cpp | 5 ++ daemon/plugins/ping/pingplugin.h | 3 +- .../telephony/kdeconnect_telephony.desktop | 2 +- 9 files changed, 64 insertions(+), 35 deletions(-) diff --git a/daemon/device.cpp b/daemon/device.cpp index 6445309bf..32ee92692 100644 --- a/daemon/device.cpp +++ b/daemon/device.cpp @@ -68,36 +68,46 @@ bool Device::hasPlugin(const QString& name) void Device::reloadPlugins() { + QMap< QString, KdeConnectPlugin* > newPluginMap; + if (paired()) { //Do not load any plugin for unpaired devices + + QString path = KStandardDirs().resourceDirs("config").first()+"kdeconnect/"; + QMap pluginStates = KSharedConfig::openConfig(path + id())->group("Plugins").entryMap(); + + PluginLoader* loader = PluginLoader::instance(); + + //Code borrowed from KWin + foreach (const QString& pluginName, loader->getPluginList()) { + + const QString value = pluginStates.value(pluginName + QString::fromLatin1("Enabled"), QString()); + KPluginInfo info = loader->getPluginInfo(pluginName); + bool enabled = (value.isNull() ? info.isPluginEnabledByDefault() : QVariant(value).toBool()); + + if (enabled) { + + if (m_plugins.contains(pluginName)) { + //Already loaded, reuse it + newPluginMap[pluginName] = m_plugins[pluginName]; + m_plugins.remove(pluginName); + } else { + KdeConnectPlugin* plugin = loader->instantiatePluginForDevice(pluginName, this); + + connect(this, SIGNAL(receivedPackage(const NetworkPackage&)), + plugin, SLOT(receivePackage(const NetworkPackage&))); + + newPluginMap[pluginName] = plugin; + } + } + } + } + + //Erase all the plugins left in the original map (it means that we don't want + //them anymore, otherways they would have been moved to the newPluginMap) qDeleteAll(m_plugins); m_plugins.clear(); - QString path = KStandardDirs().resourceDirs("config").first()+"kdeconnect/"; - QMap pluginStates = KSharedConfig::openConfig(path + id())->group("Plugins").entryMap(); - - PluginLoader* loader = PluginLoader::instance(); - - //Code borrowed from KWin - foreach (const QString& pluginName, loader->getPluginList()) { - - const QString value = pluginStates.value(pluginName + QString::fromLatin1("Enabled"), QString()); - KPluginInfo info = loader->getPluginInfo(pluginName); - bool enabled = (value.isNull() ? info.isPluginEnabledByDefault() : QVariant(value).toBool()); - - qDebug() << pluginName << "enabled:" << enabled; - - if (enabled) { - KdeConnectPlugin* plugin = loader->instantiatePluginForDevice(pluginName, this); - - connect(this, SIGNAL(receivedPackage(const NetworkPackage&)), - plugin, SLOT(receivePackage(const NetworkPackage&))); - // connect(plugin, SIGNAL(sendPackage(const NetworkPackage&)), - // device, SLOT(sendPackage(const NetworkPackage&))); - - - m_plugins[pluginName] = plugin; - } - } + m_plugins = newPluginMap; Q_EMIT pluginsChanged(); @@ -106,7 +116,6 @@ void Device::reloadPlugins() void Device::setPair(bool b) { - qDebug() << "setPair" << b; m_paired = b; KSharedConfigPtr config = KSharedConfig::openConfig("kdeconnectrc"); if (b) { @@ -116,6 +125,7 @@ void Device::setPair(bool b) qDebug() << name() << "unpaired"; config->group("devices").group("paired").deleteGroup(id()); } + reloadPlugins(); } static bool lessThan(DeviceLink* p1, DeviceLink* p2) diff --git a/daemon/plugins/battery/batterydbusinterface.cpp b/daemon/plugins/battery/batterydbusinterface.cpp index d44a99839..4efdd3777 100644 --- a/daemon/plugins/battery/batterydbusinterface.cpp +++ b/daemon/plugins/battery/batterydbusinterface.cpp @@ -25,7 +25,11 @@ BatteryDbusInterface::BatteryDbusInterface(QObject *parent) : QDBusAbstractAdaptor(parent) { +} +BatteryDbusInterface::~BatteryDbusInterface() +{ + qDebug() << "Destroying BatteryDbusInterface"; } void BatteryDbusInterface::updateValues(bool isCharging, int currentCharge) diff --git a/daemon/plugins/battery/batterydbusinterface.h b/daemon/plugins/battery/batterydbusinterface.h index 457cbb09e..56317ff29 100644 --- a/daemon/plugins/battery/batterydbusinterface.h +++ b/daemon/plugins/battery/batterydbusinterface.h @@ -33,7 +33,8 @@ class BatteryDbusInterface public: explicit BatteryDbusInterface(QObject *parent); - + virtual ~BatteryDbusInterface(); + int charge() { return mCharge; } bool isCharging() { return mIsCharging; } diff --git a/daemon/plugins/battery/batteryplugin.cpp b/daemon/plugins/battery/batteryplugin.cpp index 5de2fd1ef..b4a9862f8 100644 --- a/daemon/plugins/battery/batteryplugin.cpp +++ b/daemon/plugins/battery/batteryplugin.cpp @@ -32,8 +32,9 @@ K_EXPORT_PLUGIN( KdeConnectPluginFactory("kdeconnect_battery", "kdeconnect_batte BatteryPlugin::BatteryPlugin(QObject *parent, const QVariantList &args) : KdeConnectPlugin(parent, args) { - batteryDbusInterface = new BatteryDbusInterface(parent); + batteryDbusInterface = new BatteryDbusInterface(parent); + NetworkPackage np(PACKAGE_TYPE_BATTERY); np.set("request",true); device()->sendPackage(np); @@ -42,7 +43,14 @@ BatteryPlugin::BatteryPlugin(QObject *parent, const QVariantList &args) BatteryPlugin::~BatteryPlugin() { - batteryDbusInterface->deleteLater(); + //FIXME: Qt dbus does not allow to remove an adaptor! (it causes a crash in + // the next access to device via dbus). The implication of not deleting this + // is that disabling the plugin does not remove the interface (that will + // return outdated values) and that enabling it again instantiates a second + // adaptor. This is a partial memory leak (the memory will be freed when the + // device is destroyed anyway) + + //batteryDbusInterface->deleteLater(); } bool BatteryPlugin::receivePackage(const NetworkPackage& np) diff --git a/daemon/plugins/notifications/kdeconnect_notifications.desktop b/daemon/plugins/notifications/kdeconnect_notifications.desktop index 916e6dead..a30cbb4d1 100644 --- a/daemon/plugins/notifications/kdeconnect_notifications.desktop +++ b/daemon/plugins/notifications/kdeconnect_notifications.desktop @@ -12,4 +12,4 @@ X-KDE-PluginInfo-License=GPL X-KDE-PluginInfo-EnabledByDefault=true Icon=preferences-desktop-notification Name=Notification sync -Comment=Show every notification in KDE and keep them in sync +Comment=Show phone notifications in KDE and keep them in sync diff --git a/daemon/plugins/pausemusic/kdeconnect_pausemusic.desktop b/daemon/plugins/pausemusic/kdeconnect_pausemusic.desktop index f28563c7f..67c1f5e20 100644 --- a/daemon/plugins/pausemusic/kdeconnect_pausemusic.desktop +++ b/daemon/plugins/pausemusic/kdeconnect_pausemusic.desktop @@ -11,5 +11,5 @@ X-KDE-PluginInfo-Website=http://albertvaka.wordpress.com X-KDE-PluginInfo-License=GPL X-KDE-PluginInfo-EnabledByDefault=true Icon=speaker -Name=Pause music on call -Comment=Pause music during a phone call +Name=Pause media during calls +Comment=Pause music/videos during a phone call diff --git a/daemon/plugins/ping/pingplugin.cpp b/daemon/plugins/ping/pingplugin.cpp index 2532edec1..a6061bd44 100644 --- a/daemon/plugins/ping/pingplugin.cpp +++ b/daemon/plugins/ping/pingplugin.cpp @@ -33,6 +33,11 @@ PingPlugin::PingPlugin(QObject* parent, const QVariantList& args) qDebug() << "Ping plugin constructor for device" << device()->name(); } +PingPlugin::~PingPlugin() +{ + qDebug() << "Ping plugin destructor for device" << device()->name(); +} + bool PingPlugin::receivePackage(const NetworkPackage& np) { diff --git a/daemon/plugins/ping/pingplugin.h b/daemon/plugins/ping/pingplugin.h index d8d692453..6de5b977d 100644 --- a/daemon/plugins/ping/pingplugin.h +++ b/daemon/plugins/ping/pingplugin.h @@ -32,7 +32,8 @@ class KDE_EXPORT PingPlugin public: explicit PingPlugin(QObject *parent, const QVariantList &args); - + virtual ~PingPlugin(); + public Q_SLOTS: virtual bool receivePackage(const NetworkPackage& np); diff --git a/daemon/plugins/telephony/kdeconnect_telephony.desktop b/daemon/plugins/telephony/kdeconnect_telephony.desktop index fc395e850..e56a12de4 100644 --- a/daemon/plugins/telephony/kdeconnect_telephony.desktop +++ b/daemon/plugins/telephony/kdeconnect_telephony.desktop @@ -12,4 +12,4 @@ X-KDE-PluginInfo-License=GPL X-KDE-PluginInfo-EnabledByDefault=true Icon=call-start Name=Telephony integration -Comment=Currently it only shows notifications for calls and SMS +Comment=Show notifications for calls and SMS (answering coming soon)