network: fix segfault, split backend CMakeLists

- Fix segfault related to network state signal emission
- Add WifiNetwork.forget() and Device.autoconnect
- Create separate CMakeLists for nm
- Use QObject::destroyed to clean up pointer lists
- Use asyncReadProperty to get device type before registration
- Remove mConnectionPaths
- Add default cases to all toString invokables

Author: cpwrs

src/network/CMakeLists.txt

@@ -1,80 +1,11 @@
-set_source_files_properties(nm/org.freedesktop.NetworkManager.xml PROPERTIES
-	CLASSNAME DBusNetworkManagerProxy
-	NO_NAMESPACE TRUE
-	INCLUDE ${CMAKE_CURRENT_SOURCE_DIR}/nm/dbus_types.hpp
-)
-
-qt_add_dbus_interface(NM_DBUS_INTERFACES
-	nm/org.freedesktop.NetworkManager.xml
-	nm/dbus_nm_backend
-)
-
-set_source_files_properties(nm/org.freedesktop.NetworkManager.Device.xml PROPERTIES
-	CLASSNAME DBusNMDeviceProxy
-	NO_NAMESPACE TRUE
-)
-
-qt_add_dbus_interface(NM_DBUS_INTERFACES
-	nm/org.freedesktop.NetworkManager.Device.xml
-	nm/dbus_nm_device
-)
-
-set_source_files_properties(nm/org.freedesktop.NetworkManager.Device.Wireless.xml PROPERTIES
-	CLASSNAME DBusNMWirelessProxy
-	NO_NAMESPACE TRUE
-)
-
-qt_add_dbus_interface(NM_DBUS_INTERFACES
-	nm/org.freedesktop.NetworkManager.Device.Wireless.xml
-	nm/dbus_nm_wireless
-)
-
-set_source_files_properties(nm/org.freedesktop.NetworkManager.AccessPoint.xml PROPERTIES
-	CLASSNAME DBusNMAccessPointProxy
-	NO_NAMESPACE TRUE
-)
-
-qt_add_dbus_interface(NM_DBUS_INTERFACES
-	nm/org.freedesktop.NetworkManager.AccessPoint.xml
-	nm/dbus_nm_accesspoint
-)
-
-set_source_files_properties(nm/org.freedesktop.NetworkManager.Settings.Connection.xml PROPERTIES
-	CLASSNAME DBusNMConnectionSettingsProxy
-	NO_NAMESPACE TRUE
-	INCLUDE ${CMAKE_CURRENT_SOURCE_DIR}/nm/dbus_types.hpp
-)
-
-qt_add_dbus_interface(NM_DBUS_INTERFACES
-	nm/org.freedesktop.NetworkManager.Settings.Connection.xml
-	nm/dbus_nm_connection_settings
-)
-
-set_source_files_properties(nm/org.freedesktop.NetworkManager.Connection.Active.xml PROPERTIES
-	CLASSNAME DBusNMActiveConnectionProxy
-	NO_NAMESPACE TRUE
-)
-
-qt_add_dbus_interface(NM_DBUS_INTERFACES
-	nm/org.freedesktop.NetworkManager.Connection.Active.xml
-	nm/dbus_nm_active_connection
-)
+add_subdirectory(nm)
 
 qt_add_library(quickshell-network STATIC
 	network.cpp
 	device.cpp
 	wifi.cpp
-	nm/backend.cpp
-	nm/device.cpp
-	nm/connection.cpp
-	nm/accesspoint.cpp
-	nm/wireless.cpp
-	nm/utils.cpp
-	nm/enums.hpp
-	${NM_DBUS_INTERFACES}
 )
 
-# dbus headers
 target_include_directories(quickshell-network PRIVATE 
 	${CMAKE_CURRENT_BINARY_DIR}
 )
@@ -87,9 +18,7 @@ qt_add_qml_module(quickshell-network
 
 qs_add_module_deps_light(quickshell-network Quickshell)
 install_qml_module(quickshell-network)
-
-target_link_libraries(quickshell-network PRIVATE Qt::Qml Qt::DBus)
+target_link_libraries(quickshell-network PRIVATE quickshell-network-nm Qt::Qml Qt::DBus)
 qs_add_link_dependencies(quickshell-network quickshell-dbus)
 target_link_libraries(quickshell PRIVATE quickshell-networkplugin)
-
 qs_module_pch(quickshell-network SET dbus)

src/network/device.cpp

@@ -5,6 +5,7 @@
 #include <qloggingcategory.h>
 #include <qobject.h>
 #include <qstringliteral.h>
+#include <qtmetamacros.h>
 
 #include "../core/logcat.hpp"
 
@@ -21,13 +22,15 @@ QString DeviceConnectionState::toString(DeviceConnectionState::Enum state) {
 	case Connected: return QStringLiteral("Connected");
 	case Disconnecting: return QStringLiteral("Disconnecting");
 	case Disconnected: return QStringLiteral("Disconnected");
+	default: return QStringLiteral("Unknown");
 	}
 }
 
 QString DeviceType::toString(DeviceType::Enum type) {
 	switch (type) {
 	case None: return QStringLiteral("None");
 	case Wifi: return QStringLiteral("Wifi");
+	default: return QStringLiteral("Unknown");
 	}
 }
 
@@ -48,12 +51,19 @@ QString NMDeviceState::toString(NMDeviceState::Enum state) {
 	case Activated: return QStringLiteral("Connected");
 	case Deactivating: return QStringLiteral("Disconnecting");
 	case Failed: return QStringLiteral("Failed to connect");
+	default: return QStringLiteral("Unknown");
 	};
 }
 
-NetworkDevice::NetworkDevice(DeviceType::Enum type, QObject* parent)
-    : QObject(parent)
-    , mType(type) {};
+NetworkDevice::NetworkDevice(DeviceType::Enum type, QObject* parent): QObject(parent), mType(type) {
+	this->bConnected.setBinding([this]() { return this->bState == DeviceConnectionState::Connected; }
+	);
+};
+
+void NetworkDevice::setAutoconnect(bool autoconnect) {
+	if (this->bAutoconnect == autoconnect) return;
+	emit this->requestSetAutoconnect(autoconnect);
+}
 
 void NetworkDevice::disconnect() {
 	if (this->bState == DeviceConnectionState::Disconnected) {

src/network/device.hpp

@@ -79,11 +79,15 @@ class NetworkDevice: public QObject {
 	/// The name of the device's control interface.
 	Q_PROPERTY(QString name READ name NOTIFY nameChanged BINDABLE bindableName);
 	/// The hardware address of the device in the XX:XX:XX:XX:XX:XX format.
-	Q_PROPERTY(QString address READ address NOTIFY addressChanged BINDABLE bindableAddress);
+	Q_PROPERTY(QString address READ default NOTIFY addressChanged BINDABLE bindableAddress);
+	/// True if the device is connected.
+	Q_PROPERTY(bool connected READ default NOTIFY connectedChanged BINDABLE bindableConnected);
 	/// Connection state of the device.
-	Q_PROPERTY(qs::network::DeviceConnectionState::Enum state READ state NOTIFY stateChanged BINDABLE bindableState);
+	Q_PROPERTY(qs::network::DeviceConnectionState::Enum state READ default NOTIFY stateChanged BINDABLE bindableState);
 	/// A more specific device state when the backend is NetworkManager.
-	Q_PROPERTY(qs::network::NMDeviceState::Enum nmState READ nmState NOTIFY nmStateChanged BINDABLE bindableNmState);
+	Q_PROPERTY(qs::network::NMDeviceState::Enum nmState READ default NOTIFY nmStateChanged BINDABLE bindableNmState);
+	/// True if the device is allowed to autoconnect.
+	Q_PROPERTY(bool autoconnect READ autoconnect WRITE setAutoconnect NOTIFY autoconnectChanged);
 	// clang-format on
 
 public:
@@ -96,26 +100,32 @@ class NetworkDevice: public QObject {
 	QBindable<QString> bindableName() { return &this->bName; };
 	[[nodiscard]] QString name() const { return this->bName; };
 	QBindable<QString> bindableAddress() { return &this->bAddress; };
-	[[nodiscard]] QString address() const { return this->bAddress; };
+	QBindable<bool> bindableConnected() { return &this->bConnected; };
 	QBindable<DeviceConnectionState::Enum> bindableState() { return &this->bState; };
-	[[nodiscard]] DeviceConnectionState::Enum state() const { return this->bState; };
 	QBindable<NMDeviceState::Enum> bindableNmState() { return &this->bNmState; };
-	[[nodiscard]] NMDeviceState::Enum nmState() const { return this->bNmState; };
+	[[nodiscard]] bool autoconnect() const { return this->bAutoconnect; };
+	QBindable<bool> bindableAutoconnect() { return &this->bAutoconnect; };
+	void setAutoconnect(bool autoconnect);
 
 signals:
 	void requestDisconnect();
+	void requestSetAutoconnect(bool autoconnect);
 	void nameChanged();
 	void addressChanged();
+	void connectedChanged();
 	void stateChanged();
 	void nmStateChanged();
+	void autoconnectChanged();
 
 private:
 	DeviceType::Enum mType;
 	// clang-format off
 	Q_OBJECT_BINDABLE_PROPERTY(NetworkDevice, QString, bName, &NetworkDevice::nameChanged);
 	Q_OBJECT_BINDABLE_PROPERTY(NetworkDevice, QString, bAddress, &NetworkDevice::addressChanged);
+	Q_OBJECT_BINDABLE_PROPERTY(NetworkDevice, bool, bConnected, &NetworkDevice::connectedChanged);
 	Q_OBJECT_BINDABLE_PROPERTY(NetworkDevice, DeviceConnectionState::Enum, bState, &NetworkDevice::stateChanged);
 	Q_OBJECT_BINDABLE_PROPERTY(NetworkDevice, NMDeviceState::Enum, bNmState, &NetworkDevice::nmStateChanged);
+	Q_OBJECT_BINDABLE_PROPERTY(NetworkDevice, bool, bAutoconnect, &NetworkDevice::autoconnectChanged);
 	// clang-format on
 };
 

src/network/module.md

@@ -6,7 +6,7 @@ headers = [
 	"wifi.hpp",
 ]
 -----
-This modules exposes Network management APIs provided by a supported network backend.
+This module exposes Network management APIs provided by a supported network backend.
 For now, the only backend available is the NetworkManager DBus interface.
 Both DBus and NetworkManager must be running to use it.
 

src/network/network.cpp

@@ -20,8 +20,8 @@ Network::Network(QObject* parent): QObject(parent) {
 	// Try to create the NetworkManager backend and bind to it.
 	auto* nm = new NetworkManager(this);
 	if (nm->isAvailable()) {
-		QObject::connect(nm, &NetworkManager::deviceAdded, this, &Network::onDeviceAdded);
-		QObject::connect(nm, &NetworkManager::deviceRemoved, this, &Network::onDeviceRemoved);
+		QObject::connect(nm, &NetworkManager::deviceAdded, this, &Network::deviceAdded);
+		QObject::connect(nm, &NetworkManager::deviceRemoved, this, &Network::deviceRemoved);
 		QObject::connect(this, &Network::requestSetWifiEnabled, nm, &NetworkManager::setWifiEnabled);
 		this->bindableWifiEnabled().setBinding([nm]() { return nm->wifiEnabled(); });
 		this->bindableWifiHardwareEnabled().setBinding([nm]() { return nm->wifiHardwareEnabled(); });
@@ -36,16 +36,12 @@ Network::Network(QObject* parent): QObject(parent) {
 	qCCritical(logNetwork) << "Network will not work. Could not find an available backend.";
 }
 
-void Network::onDeviceAdded(NetworkDevice* dev) { this->mDevices.insertObject(dev); }
-void Network::onDeviceRemoved(NetworkDevice* dev) { this->mDevices.removeObject(dev); }
+void Network::deviceAdded(NetworkDevice* dev) { this->mDevices.insertObject(dev); }
+void Network::deviceRemoved(NetworkDevice* dev) { this->mDevices.removeObject(dev); }
 
 void Network::setWifiEnabled(bool enabled) {
-	if (this->bWifiEnabled == enabled) {
-		const QString state = enabled ? "enabled" : "disabled";
-		qCCritical(logNetwork) << "Wifi is already globally software" << state;
-	} else {
-		emit this->requestSetWifiEnabled(enabled);
-	}
+	if (this->bWifiEnabled == enabled) return;
+	emit this->requestSetWifiEnabled(enabled);
 }
 
 BaseNetwork::BaseNetwork(QString name, QObject* parent): QObject(parent), mName(std::move(name)) {};

src/network/network.hpp

@@ -50,8 +50,8 @@ class Network: public QObject {
 	Q_PROPERTY(qs::network::NetworkBackendType::Enum backend READ backend CONSTANT);
 	/// Switch for the rfkill software block of all wireless devices.
 	Q_PROPERTY(bool wifiEnabled READ wifiEnabled WRITE setWifiEnabled NOTIFY wifiEnabledChanged);
-	/// Switch for the rfkill hardware block of all wireless devices.
-	Q_PROPERTY(bool wifiHardwareEnabled READ wifiHardwareEnabled NOTIFY wifiHardwareEnabledChanged BINDABLE bindableWifiHardwareEnabled);
+	/// State of the rfkill hardware block of all wireless devices.
+	Q_PROPERTY(bool wifiHardwareEnabled READ default NOTIFY wifiHardwareEnabledChanged BINDABLE bindableWifiHardwareEnabled);
 	// clang-format on
 
 public:
@@ -63,16 +63,15 @@ class Network: public QObject {
 	[[nodiscard]] bool wifiEnabled() const { return this->bWifiEnabled; };
 	void setWifiEnabled(bool enabled);
 	QBindable<bool> bindableWifiHardwareEnabled() { return &this->bWifiHardwareEnabled; };
-	[[nodiscard]] bool wifiHardwareEnabled() const { return this->bWifiHardwareEnabled; };
 
 signals:
 	void requestSetWifiEnabled(bool enabled);
 	void wifiEnabledChanged();
 	void wifiHardwareEnabledChanged();
 
-public slots:
-	void onDeviceAdded(NetworkDevice* dev);
-	void onDeviceRemoved(NetworkDevice* dev);
+private slots:
+	void deviceAdded(NetworkDevice* dev);
+	void deviceRemoved(NetworkDevice* dev);
 
 private:
 	ObjectModel<NetworkDevice> mDevices {this};
@@ -93,14 +92,13 @@ class BaseNetwork: public QObject {
 	/// The name of the network.
 	Q_PROPERTY(QString name READ name CONSTANT);
 	/// True if the network is connected.
-	Q_PROPERTY(bool connected READ connected NOTIFY connectedChanged BINDABLE bindableConnected);
+	Q_PROPERTY(bool connected READ default NOTIFY connectedChanged BINDABLE bindableConnected);
 
 public:
 	explicit BaseNetwork(QString name, QObject* parent = nullptr);
 
 	[[nodiscard]] QString name() const { return this->mName; };
 	QBindable<bool> bindableConnected() { return &this->bConnected; }
-	[[nodiscard]] bool connected() const { return this->bConnected; };
 
 signals:
 	void connectedChanged();

src/network/nm/CMakeLists.txt

@@ -0,0 +1,79 @@
+set_source_files_properties(org.freedesktop.NetworkManager.xml PROPERTIES
+	CLASSNAME DBusNetworkManagerProxy
+	NO_NAMESPACE TRUE
+	INCLUDE ${CMAKE_CURRENT_SOURCE_DIR}/dbus_types.hpp
+)
+
+qt_add_dbus_interface(NM_DBUS_INTERFACES
+	org.freedesktop.NetworkManager.xml
+	dbus_nm_backend
+)
+
+set_source_files_properties(org.freedesktop.NetworkManager.Device.xml PROPERTIES
+	CLASSNAME DBusNMDeviceProxy
+	NO_NAMESPACE TRUE
+)
+
+qt_add_dbus_interface(NM_DBUS_INTERFACES
+	org.freedesktop.NetworkManager.Device.xml
+	dbus_nm_device
+)
+
+set_source_files_properties(org.freedesktop.NetworkManager.Device.Wireless.xml PROPERTIES
+	CLASSNAME DBusNMWirelessProxy
+	NO_NAMESPACE TRUE
+)
+
+qt_add_dbus_interface(NM_DBUS_INTERFACES
+	org.freedesktop.NetworkManager.Device.Wireless.xml
+	dbus_nm_wireless
+)
+
+set_source_files_properties(org.freedesktop.NetworkManager.AccessPoint.xml PROPERTIES
+	CLASSNAME DBusNMAccessPointProxy
+	NO_NAMESPACE TRUE
+)
+
+qt_add_dbus_interface(NM_DBUS_INTERFACES
+	org.freedesktop.NetworkManager.AccessPoint.xml
+	dbus_nm_accesspoint
+)
+
+set_source_files_properties(org.freedesktop.NetworkManager.Settings.Connection.xml PROPERTIES
+	CLASSNAME DBusNMConnectionSettingsProxy
+	NO_NAMESPACE TRUE
+	INCLUDE ${CMAKE_CURRENT_SOURCE_DIR}/dbus_types.hpp
+)
+
+qt_add_dbus_interface(NM_DBUS_INTERFACES
+	org.freedesktop.NetworkManager.Settings.Connection.xml
+	dbus_nm_connection_settings
+)
+
+set_source_files_properties(org.freedesktop.NetworkManager.Connection.Active.xml PROPERTIES
+	CLASSNAME DBusNMActiveConnectionProxy
+	NO_NAMESPACE TRUE
+)
+
+qt_add_dbus_interface(NM_DBUS_INTERFACES
+	org.freedesktop.NetworkManager.Connection.Active.xml
+	dbus_nm_active_connection
+)
+
+qt_add_library(quickshell-network-nm STATIC
+	backend.cpp
+	device.cpp
+	connection.cpp
+	accesspoint.cpp
+	wireless.cpp
+	utils.cpp
+	enums.hpp
+	${NM_DBUS_INTERFACES}
+)
+
+target_include_directories(quickshell-network-nm PUBLIC 
+	${CMAKE_CURRENT_BINARY_DIR}
+)
+
+target_link_libraries(quickshell-network-nm PRIVATE Qt::Qml Qt::DBus)
+qs_add_link_dependencies(quickshell-network-nm quickshell-dbus)

src/network/nm/accesspoint.cpp

@@ -10,8 +10,8 @@
 
 #include "../../core/logcat.hpp"
 #include "../../dbus/properties.hpp"
+#include "dbus_nm_accesspoint.h"
 #include "enums.hpp"
-#include "nm/dbus_nm_accesspoint.h"
 
 namespace qs::network {
 using namespace qs::dbus;

src/network/nm/accesspoint.hpp

@@ -8,8 +8,8 @@
 
 #include "../../dbus/properties.hpp"
 #include "../wifi.hpp"
+#include "dbus_nm_accesspoint.h"
 #include "enums.hpp"
-#include "nm/dbus_nm_accesspoint.h"
 
 namespace qs::dbus {
 
@@ -59,7 +59,6 @@ class NMAccessPoint: public QObject {
 
 signals:
 	void loaded();
-	void disappeared();
 	void ssidChanged(const QByteArray& ssid);
 	void signalStrengthChanged(quint8 signal);
 	void flagsChanged(NM80211ApFlags::Enum flags);