service/upower: track device additions/removals

Also ensures displayDevice is always present.

src/services/upower/core.cpp

@@ -11,7 +11,6 @@
 #include <qloggingcategory.h>
 #include <qobject.h>
 #include <qqmllist.h>
-#include <qtmetamacros.h>
 
 #include "../../core/model.hpp"
 #include "../../dbus/bus.hpp"
@@ -53,15 +52,23 @@ UPower::UPower() {
 }
 
 void UPower::init() {
+	QObject::connect(this->service, &DBusUPowerService::DeviceAdded, this, &UPower::onDeviceAdded);
+
+	QObject::connect(
+	    this->service,
+	    &DBusUPowerService::DeviceRemoved,
+	    this,
+	    &UPower::onDeviceRemoved
+	);
+
 	this->serviceProperties.setInterface(this->service);
 	this->serviceProperties.updateAllViaGetAll();
 
-	this->registerExisting();
+	this->registerDisplayDevice();
+	this->registerDevices();
 }
 
-void UPower::registerExisting() {
-	this->registerDevice("/org/freedesktop/UPower/devices/DisplayDevice");
-
+void UPower::registerDevices() {
 	auto pending = this->service->EnumerateDevices();
 	auto* call = new QDBusPendingCallWatcher(pending, this);
 
@@ -82,34 +89,46 @@ void UPower::registerExisting() {
 	QObject::connect(call, &QDBusPendingCallWatcher::finished, this, responseCallback);
 }
 
+void UPower::registerDisplayDevice() {
+	auto pending = this->service->GetDisplayDevice();
+	auto* call = new QDBusPendingCallWatcher(pending, this);
+
+	auto responseCallback = [this](QDBusPendingCallWatcher* call) {
+		const QDBusPendingReply<QDBusObjectPath> reply = *call;
+
+		if (reply.isError()) {
+			qCWarning(logUPower) << "Failed to get default device:" << reply.error().message();
+		} else {
+			qCDebug(logUPower) << "UPower default device registered at" << reply.value().path();
+			this->mDisplayDevice.init(reply.value().path());
+		}
+
+		delete call;
+	};
+
+	QObject::connect(call, &QDBusPendingCallWatcher::finished, this, responseCallback);
+}
+
+void UPower::onDeviceAdded(const QDBusObjectPath& path) { this->registerDevice(path.path()); }
+
+void UPower::onDeviceRemoved(const QDBusObjectPath& path) {
+	auto iter = this->mDevices.find(path.path());
+
+	if (iter == this->mDevices.end()) {
+		qCWarning(logUPower) << "UPower service sent removal signal for" << path.path()
+		                     << "which is not registered.";
+	} else {
+		auto* device = iter.value();
+		this->mDevices.erase(iter);
+		this->readyDevices.removeObject(device);
+		qCDebug(logUPower) << "UPowerDevice" << device->path() << "removed.";
+	}
+}
+
 void UPower::onDeviceReady() {
 	auto* device = qobject_cast<UPowerDevice*>(this->sender());
 
-	if (device->path() == "/org/freedesktop/UPower/devices/DisplayDevice") {
-		this->mDisplayDevice = device;
-		emit this->displayDeviceChanged();
-		qCDebug(logUPower) << "Display UPowerDevice" << device->path() << "ready";
-		return;
-	}
-
 	this->readyDevices.insertObject(device);
-	qCDebug(logUPower) << "UPowerDevice" << device->path() << "ready";
-}
-
-void UPower::onDeviceDestroyed(QObject* object) {
-	auto* device = static_cast<UPowerDevice*>(object); // NOLINT
-
-	this->mDevices.remove(device->path());
-
-	if (device == this->mDisplayDevice) {
-		this->mDisplayDevice = nullptr;
-		emit this->displayDeviceChanged();
-		qCDebug(logUPower) << "Display UPowerDevice" << device->path() << "destroyed";
-		return;
-	}
-
-	this->readyDevices.removeObject(device);
-	qCDebug(logUPower) << "UPowerDevice" << device->path() << "destroyed";
 }
 
 void UPower::registerDevice(const QString& path) {
@@ -118,7 +137,9 @@ void UPower::registerDevice(const QString& path) {
 		return;
 	}
 
-	auto* device = new UPowerDevice(path, this);
+	auto* device = new UPowerDevice(this);
+	device->init(path);
+
 	if (!device->isValid()) {
 		qCWarning(logUPower) << "Ignoring invalid UPowerDevice registration of" << path;
 		delete device;
@@ -126,13 +147,12 @@ void UPower::registerDevice(const QString& path) {
 	}
 
 	this->mDevices.insert(path, device);
-	QObject::connect(device, &UPowerDevice::ready, this, &UPower::onDeviceReady);
-	QObject::connect(device, &QObject::destroyed, this, &UPower::onDeviceDestroyed);
+	QObject::connect(device, &UPowerDevice::readyChanged, this, &UPower::onDeviceReady);
 
 	qCDebug(logUPower) << "Registered UPowerDevice" << path;
 }
 
-UPowerDevice* UPower::displayDevice() { return this->mDisplayDevice; }
+UPowerDevice* UPower::displayDevice() { return &this->mDisplayDevice; }
 
 ObjectModel<UPowerDevice>* UPower::devices() { return &this->readyDevices; }
 
@@ -142,12 +162,6 @@ UPower* UPower::instance() {
 }
 
 UPowerQml::UPowerQml(QObject* parent): QObject(parent) {
-	QObject::connect(
-	    UPower::instance(),
-	    &UPower::displayDeviceChanged,
-	    this,
-	    &UPowerQml::displayDeviceChanged
-	);
 	QObject::connect(
 	    UPower::instance(),
 	    &UPower::onBatteryChanged,

src/services/upower/core.hpp

@@ -1,5 +1,6 @@
 #pragma once
 
+#include <qdbusextratypes.h>
 #include <qdbusservicewatcher.h>
 #include <qhash.h>
 #include <qobject.h>
@@ -26,21 +27,22 @@ public:
 	static UPower* instance();
 
 signals:
-	void displayDeviceChanged();
 	void onBatteryChanged();
 
 private slots:
+	void onDeviceAdded(const QDBusObjectPath& path);
+	void onDeviceRemoved(const QDBusObjectPath& path);
 	void onDeviceReady();
-	void onDeviceDestroyed(QObject* object);
 
 private:
 	explicit UPower();
 
 	void init();
-	void registerExisting();
+	void registerDevices();
+	void registerDisplayDevice();
 	void registerDevice(const QString& path);
 
-	UPowerDevice* mDisplayDevice = nullptr;
+	UPowerDevice mDisplayDevice {this};
 	QHash<QString, UPowerDevice*> mDevices;
 	ObjectModel<UPowerDevice> readyDevices {this};
 
@@ -57,11 +59,12 @@ class UPowerQml: public QObject {
 	QML_NAMED_ELEMENT(UPower);
 	QML_SINGLETON;
 	// clang-format off
-	/// UPower's DisplayDevice for your system. Can be `null`.
+	/// UPower's DisplayDevice for your system. Cannot be null,
+	/// but might not be initialized (check @@UPowerDevice.ready if you need to know).
 	///
 	/// This is an aggregate device and not a physical one, meaning you will not find it in @@devices.
 	/// It is typically the device that is used for displaying information in desktop environments.
-	Q_PROPERTY(qs::service::upower::UPowerDevice* displayDevice READ displayDevice NOTIFY displayDeviceChanged);
+	Q_PROPERTY(qs::service::upower::UPowerDevice* displayDevice READ displayDevice CONSTANT);
 	/// All connected UPower devices.
 	QSDOC_TYPE_OVERRIDE(ObjectModel<qs::service::upower::UPowerDevice>*);
 	Q_PROPERTY(UntypedObjectModel* devices READ devices CONSTANT);
@@ -81,7 +84,6 @@ public:
 	}
 
 signals:
-	void displayDeviceChanged();
 	void onBatteryChanged();
 };
 

src/services/upower/device.cpp

@@ -65,7 +65,15 @@ QString UPowerDeviceType::toString(UPowerDeviceType::Enum type) {
 	}
 }
 
-UPowerDevice::UPowerDevice(const QString& path, QObject* parent): QObject(parent) {
+UPowerDevice::UPowerDevice(QObject* parent): QObject(parent) {
+	this->bIsLaptopBattery.setBinding([this]() {
+		return this->bType == UPowerDeviceType::Battery && this->bPowerSupply;
+	});
+
+	this->bHealthSupported.setBinding([this]() { return this->bHealthPercentage != 0; });
+}
+
+void UPowerDevice::init(const QString& path) {
 	this->device =
 	    new DBusUPowerDevice("org.freedesktop.UPower", path, QDBusConnection::systemBus(), this);
 
@@ -74,26 +82,25 @@ UPowerDevice::UPowerDevice(const QString& path, QObject* parent): QObject(parent
 		return;
 	}
 
-	this->bIsLaptopBattery.setBinding([this]() {
-		return this->bType == UPowerDeviceType::Battery && this->bPowerSupply;
-	});
-
-	this->bHealthSupported.setBinding([this]() { return this->bHealthPercentage != 0; });
-
 	QObject::connect(
 	    &this->deviceProperties,
 	    &DBusPropertyGroup::getAllFinished,
 	    this,
-	    &UPowerDevice::ready
+	    &UPowerDevice::onGetAllFinished
 	);
 
 	this->deviceProperties.setInterface(this->device);
 	this->deviceProperties.updateAllViaGetAll();
 }
 
-bool UPowerDevice::isValid() const { return this->device->isValid(); }
-QString UPowerDevice::address() const { return this->device->service(); }
-QString UPowerDevice::path() const { return this->device->path(); }
+bool UPowerDevice::isValid() const { return this->device && this->device->isValid(); }
+QString UPowerDevice::address() const { return this->device ? this->device->service() : QString(); }
+QString UPowerDevice::path() const { return this->device ? this->device->path() : QString(); }
+
+void UPowerDevice::onGetAllFinished() {
+	qCDebug(logUPowerDevice) << "UPowerDevice" << device->path() << "ready.";
+	this->bReady = true;
+}
 
 } // namespace qs::service::upower
 

src/services/upower/device.hpp

@@ -154,12 +154,18 @@ class UPowerDevice: public QObject {
 	Q_PROPERTY(bool isLaptopBattery READ isLaptopBattery NOTIFY isLaptopBatteryChanged BINDABLE bindableIsLaptopBattery);
 	/// Native path of the device specific to your OS.
 	Q_PROPERTY(QString nativePath READ nativePath NOTIFY nativePathChanged BINDABLE bindableNativePath);
+	/// If device statistics have been queried for this device yet.
+  /// This will be true for all devices returned from @@UPower.devices, but not the default
+  /// device, which may be returned before it is ready to avoid returning null.
+	Q_PROPERTY(bool ready READ ready NOTIFY readyChanged BINDABLE bindableReady);
 	// clang-format on
 	QML_ELEMENT;
 	QML_UNCREATABLE("UPowerDevices can only be acquired from UPower");
 
 public:
-	explicit UPowerDevice(const QString& path, QObject* parent = nullptr);
+	explicit UPowerDevice(QObject* parent = nullptr);
+
+	void init(const QString& path);
 
 	[[nodiscard]] bool isValid() const;
 	[[nodiscard]] QString address() const;
@@ -180,9 +186,10 @@ public:
 	QS_BINDABLE_GETTER(QString, bIconName, iconName, bindableIconName);
 	QS_BINDABLE_GETTER(bool, bIsLaptopBattery, isLaptopBattery, bindableIsLaptopBattery);
 	QS_BINDABLE_GETTER(QString, bNativePath, nativePath, bindableNativePath);
+	QS_BINDABLE_GETTER(bool, bReady, ready, bindableReady);
 
 signals:
-	QSDOC_HIDE void ready();
+	QSDOC_HIDE void readyChanged();
 
 	void typeChanged();
 	void powerSupplyChanged();
@@ -200,6 +207,9 @@ signals:
 	void isLaptopBatteryChanged();
 	void nativePathChanged();
 
+private slots:
+	void onGetAllFinished();
+
 private:
 	// clang-format off
 	Q_OBJECT_BINDABLE_PROPERTY(UPowerDevice, UPowerDeviceType::Enum, bType, &UPowerDevice::typeChanged);
@@ -217,6 +227,7 @@ private:
 	Q_OBJECT_BINDABLE_PROPERTY(UPowerDevice, QString, bIconName, &UPowerDevice::iconNameChanged);
 	Q_OBJECT_BINDABLE_PROPERTY(UPowerDevice, bool, bIsLaptopBattery, &UPowerDevice::isLaptopBatteryChanged);
 	Q_OBJECT_BINDABLE_PROPERTY(UPowerDevice, QString, bNativePath, &UPowerDevice::nativePathChanged);
+	Q_OBJECT_BINDABLE_PROPERTY(UPowerDevice, bool, bReady, &UPowerDevice::readyChanged);
 
 	QS_DBUS_BINDABLE_PROPERTY_GROUP(UPowerDevice, deviceProperties);
 	QS_DBUS_PROPERTY_BINDING(UPowerDevice, pType, bType, deviceProperties, "Type");

src/services/upower/org.freedesktop.UPower.xml

@@ -3,5 +3,15 @@
 		<method name="EnumerateDevices">
 			<arg direction="out" type="ao" name="devices"/>
 		</method>
+		<method name="GetDisplayDevice">
+			<arg direction="out" type="o" name="devices"/>
+		</method>
+
+		<signal name="DeviceAdded">
+			<arg direction="out" type="o" name="device"/>
+		</signal>
+		<signal name="DeviceRemoved">
+			<arg direction="out" type="o" name="device"/>
+		</signal>
 	</interface>
 </node>