Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix PackageKit not emitting network state changed signal when stopped #30

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions src/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ set(packagekitqt_SRC
transactionprivate.cpp
details.cpp
offline.cpp
networkmanagermonitor.cpp
)

find_path(PK_INTERFACES_DIR org.freedesktop.PackageKit.xml
Expand Down
1 change: 1 addition & 0 deletions src/daemon.h
Original file line number Diff line number Diff line change
Expand Up @@ -869,6 +869,7 @@ class PACKAGEKITQT_LIBRARY Daemon : public QObject
Q_DECLARE_PRIVATE(Daemon)
Q_PRIVATE_SLOT(d_func(), void propertiesChanged(QString,QVariantMap,QStringList))
Q_PRIVATE_SLOT(d_func(), void updateProperties(QVariantMap))
Q_PRIVATE_SLOT(d_func(), void getAllPropertiesIfPackageKitNotRunning())
Daemon(QObject *parent = nullptr);
static Daemon *m_global;
};
Expand Down
14 changes: 14 additions & 0 deletions src/daemonprivate.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,20 @@ DaemonPrivate::DaemonPrivate(Daemon* parent)
});

getAllProperties();

// If PackageKit is not running, we are not getting a signal when
// properties are updated (like when the network changes its state)
// so we connect to the NetworkManager dbus signal directly and then
// get all properties again to force packagekit to run and
// update the properties so we can emit the appropiate signals.
q_ptr->connect(&networkManagerMonitor, SIGNAL(networkStateChanged(uint)),
SLOT(getAllPropertiesIfPackageKitNotRunning()));
}

void DaemonPrivate::getAllPropertiesIfPackageKitNotRunning()
{
if (!running)
getAllProperties();
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If PK is not running I believe getAllProperties() is likely to wake it up again, thus, talking to NM useless if you don't let it stop.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well, that's the whole point of this PR. We talk to NM because, when PK is not running, it doesn't emit the propertiesChanged signal when the network state changes, so we use NM to have an alternate way to find out we need to update PK properties while PK is not running. Once we got notified the network is up, we call getAllPropertiesIfPackageKitNotRunning, which, as you said, wakes PK so it can give us the updated properties through the usual channels.

Copy link
Author

@antlarr-suse antlarr-suse Apr 4, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note that the check for !running is important since that controls that we only do something different than without this PR when PK is stopped. Apart from that, the PR is just signal connections

}

void DaemonPrivate::getAllProperties()
Expand Down
3 changes: 3 additions & 0 deletions src/daemonprivate.h
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@

#include "daemon.h"
#include "offline.h"
#include "networkmanagermonitor.h"

Q_DECLARE_LOGGING_CATEGORY(PACKAGEKITQT_TRANSACTION)
Q_DECLARE_LOGGING_CATEGORY(PACKAGEKITQT_DAEMON)
Expand Down Expand Up @@ -72,10 +73,12 @@ class DaemonPrivate
uint versionMinor = 0;
bool locked = false;
bool running = false;
NetworkManagerMonitor networkManagerMonitor;

protected Q_SLOTS:
void propertiesChanged(const QString &interface, const QVariantMap &properties, const QStringList &invalidatedProperties);
void updateProperties(const QVariantMap &properties);
void getAllPropertiesIfPackageKitNotRunning();
};

} // End namespace PackageKit
Expand Down
66 changes: 66 additions & 0 deletions src/networkmanagermonitor.cpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,66 @@
/*
* This file is part of the QPackageKit project
* Copyright (C) 2019 Daniel Nicoletti <[email protected]>
* Copyright (C) 2019 Antonio Larrosa <[email protected]>
*
* This library is free software; you can redistribute it and/or
* modify it under the terms of the GNU Library General Public
* License as published by the Free Software Foundation; either
* version 2 of the License, or (at your option) any later version.
*
* This library is distributed in the hope that it will be useful,
* but WITHOUT ANY WARRANTY; without even the implied warranty of
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
* Library General Public License for more details.
*
* You should have received a copy of the GNU Library General Public License
* along with this library; see the file COPYING.LIB. If not, write to
* the Free Software Foundation, Inc., 51 Franklin Street, Fifth Floor,
* Boston, MA 02110-1301, USA.
*/

#include "networkmanagermonitor.h"

#include <QDBusConnection>
#include <QDBusMessage>
#include <QList>
#include <QVariant>
#include <QString>

static QString NM_DBUS_SERVICE = QStringLiteral("org.freedesktop.NetworkManager");
static QString NM_DBUS_PATH = QStringLiteral("/org/freedesktop/NetworkManager");
static QString NM_DBUS_INTERFACE = QStringLiteral("org.freedesktop.NetworkManager");

using namespace PackageKit;

NetworkManagerMonitor::NetworkManagerMonitor(QObject *parent)
: QObject(parent)
{
QDBusConnection::systemBus().connect(NM_DBUS_SERVICE,
NM_DBUS_PATH,
NM_DBUS_INTERFACE,
QLatin1String("StateChanged"),
this, SIGNAL(networkStateChanged(uint)));
}

NetworkManagerMonitor::~NetworkManagerMonitor()
{
QDBusConnection::systemBus().disconnect(NM_DBUS_SERVICE,
NM_DBUS_PATH,
NM_DBUS_INTERFACE,
QLatin1String("StateChanged"),
this, SIGNAL(networkStateChanged(uint)));
}

NetworkManagerMonitor::NMState NetworkManagerMonitor::state()
{
QDBusMessage message = QDBusMessage::createMethodCall(NM_DBUS_SERVICE,
NM_DBUS_PATH,
NM_DBUS_INTERFACE,
QLatin1String("state"));

QDBusMessage reply = QDBusConnection::systemBus().call(message);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a blocking call.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, if you check the commit, NetworkManagerMonitor::state is not really used. I just added it for completeness (and for debug purposes). If it bothers you that someone might change the code in the future and call it, we can remove the method without any problem

if (reply.arguments().isEmpty()) return NM_STATE_UNKNOWN;

return static_cast<NMState>(reply.arguments()[0].toUInt());
}
55 changes: 55 additions & 0 deletions src/networkmanagermonitor.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,55 @@
/*
* This file is part of the QPackageKit project
* Copyright (C) 2019 Daniel Nicoletti <[email protected]>
* Copyright (C) 2019 Antonio Larrosa <[email protected]>
*
* This library is free software; you can redistribute it and/or
* modify it under the terms of the GNU Library General Public
* License as published by the Free Software Foundation; either
* version 2 of the License, or (at your option) any later version.
*
* This library is distributed in the hope that it will be useful,
* but WITHOUT ANY WARRANTY; without even the implied warranty of
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
* Library General Public License for more details.
*
* You should have received a copy of the GNU Library General Public License
* along with this library; see the file COPYING.LIB. If not, write to
* the Free Software Foundation, Inc., 51 Franklin Street, Fifth Floor,
* Boston, MA 02110-1301, USA.
*/

#ifndef NETWORKMANAGERMONITOR_H
#define NETWORKMANAGERMONITOR_H

#include <QObject>

namespace PackageKit {

class NetworkManagerMonitor : public QObject
{
Q_OBJECT
public:
enum NMState {
NM_STATE_UNKNOWN = 0,
NM_STATE_ASLEEP = 10,
NM_STATE_DISCONNECTED = 20,
NM_STATE_DISCONNECTING = 30,
NM_STATE_CONNECTING = 40,
NM_STATE_CONNECTED_LOCAL = 50,
NM_STATE_CONNECTED_SITE = 60,
NM_STATE_CONNECTED_GLOBAL = 70
};

NetworkManagerMonitor(QObject *parent = nullptr);
~NetworkManagerMonitor();

NMState state();

Q_SIGNALS:
void networkStateChanged(uint state);
};

};

#endif