Merge pull request #1596 from turannul/pr/refactor-battery-pt2

Battery refactor pt2

Modules/Bar/Widgets/Battery.qml

@@ -53,28 +53,21 @@ Item {
   readonly property bool testPluggedIn: false
   readonly property string deviceNativePath: widgetSettings.deviceNativePath || ""
 
-  readonly property var battery: BatteryService.findUPowerDevice(deviceNativePath)
-  readonly property var bluetoothDevice: deviceNativePath ? BatteryService.findBluetoothDevice(deviceNativePath) : null
-  readonly property var device: bluetoothDevice || battery
+  readonly property var device: BatteryService.resolveDevice(deviceNativePath)
+  readonly property var battery: device && !BatteryService.isBluetoothDevice(device) ? device : null
+  readonly property var bluetoothDevice: device && BatteryService.isBluetoothDevice(device) ? device : null
   readonly property bool hasBluetoothBattery: BatteryService.isBluetoothDevice(device)
 
-  readonly property bool isReady: testMode ? true : (initializationComplete && BatteryService.isDeviceReady(device))
+  readonly property bool isReady: testMode ? true : (BatteryService.ready && BatteryService.isDeviceReady(device))
   readonly property real percent: testMode ? testPercent : (isReady ? BatteryService.getPercentage(device) : 0)
   readonly property bool isCharging: testMode ? testCharging : (isReady ? BatteryService.isCharging(device) : false)
   readonly property bool isPluggedIn: testMode ? testPluggedIn : (isReady ? BatteryService.isPluggedIn(device) : false)
 
-  property bool initializationComplete: false
   property bool hasNotifiedLowBattery: false
 
   visible: shouldShow
   opacity: shouldShow ? 1.0 : 0.0
 
-  Timer {
-    interval: 500
-    running: true
-    onTriggered: root.initializationComplete = true
-  }
-
   readonly property bool isDevicePresent: {
     if (testMode)
       return true;
@@ -156,9 +149,9 @@ Item {
     suffix: "%"
     autoHide: false
     forceOpen: isReady && displayMode === "alwaysShow"
-    forceClose: displayMode === "alwaysHide" || (initializationComplete && !isReady)
-    customBackgroundColor: !initializationComplete ? "transparent" : (isCharging ? Color.mPrimary : (isLowBattery ? Color.mError : "transparent"))
-    customTextIconColor: !initializationComplete ? "transparent" : (isCharging ? Color.mOnPrimary : (isLowBattery ? Color.mOnError : "transparent"))
+    forceClose: displayMode === "alwaysHide" || (BatteryService.ready && !isReady)
+    customBackgroundColor: !BatteryService.ready ? "transparent" : (isCharging ? Color.mPrimary : (isLowBattery ? Color.mError : "transparent"))
+    customTextIconColor: !BatteryService.ready ? "transparent" : (isCharging ? Color.mOnPrimary : (isLowBattery ? Color.mOnError : "transparent"))
 
     tooltipText: {
       let lines = [];

Modules/LockScreen/LockScreen.qml

@@ -77,14 +77,8 @@ Loader {
 
           Item {
             id: batteryIndicator
-            property bool initializationComplete: false
-            Timer {
-              interval: 500
-              running: true
-              onTriggered: batteryIndicator.initializationComplete = true
-            }
 
-            property bool isReady: initializationComplete && BatteryService.batteryReady
+            property bool isReady: BatteryService.ready && BatteryService.batteryReady
             property real percent: BatteryService.batteryPercentage
             property bool charging: BatteryService.batteryCharging
             property bool pluggedIn: BatteryService.batteryPluggedIn

Modules/Panels/Battery/BatteryPanel.qml

@@ -35,15 +35,9 @@ SmartPanel {
     }
 
     readonly property string deviceNativePath: getBatteryDevicePath()
-    readonly property var selectedBattery: BatteryService.findUPowerDevice(deviceNativePath)
-    readonly property var selectedBluetoothDevice: deviceNativePath ? BatteryService.findBluetoothDevice(deviceNativePath) : null
-    readonly property var selectedDevice: {
-      var dev = selectedBluetoothDevice || selectedBattery;
-      if (BatteryService.isDevicePresent(dev))
-        return dev;
 
-      return allDevices.length > 0 ? allDevices[0] : null;
-    }
+    // Use the centralized helper to find the specific device or fallback to primary
+    readonly property var selectedDevice: BatteryService.resolveDevice(deviceNativePath)
 
     // Check if selected device is actually present/connected
     readonly property bool isDevicePresent: BatteryService.isDevicePresent(selectedDevice)
@@ -52,76 +46,22 @@ SmartPanel {
     readonly property int percent: isReady ? Math.round(BatteryService.getPercentage(selectedDevice)) : -1
     readonly property bool isCharging: BatteryService.isCharging(selectedDevice)
     readonly property bool isPluggedIn: BatteryService.isPluggedIn(selectedDevice)
-    readonly property bool healthAvailable: (isReady && selectedBattery && selectedBattery.healthSupported) || (selectedBattery && BatteryService.healthAvailable)
-    readonly property int healthPercent: (isReady && selectedBattery && selectedBattery.healthSupported) ? Math.round(selectedBattery.healthPercentage) : BatteryService.healthPercent
+
+    readonly property bool isLaptopBattery: selectedDevice && !BatteryService.isBluetoothDevice(selectedDevice)
+
+    readonly property bool healthAvailable: (isReady && isLaptopBattery && selectedDevice.healthSupported) || (isLaptopBattery && BatteryService.healthAvailable)
+    readonly property int healthPercent: (isReady && isLaptopBattery && selectedDevice.healthSupported) ? Math.round(selectedDevice.healthPercentage) : BatteryService.healthPercent
 
     readonly property string deviceName: BatteryService.getDeviceName(selectedDevice)
     readonly property string panelTitle: deviceName ? `${I18n.tr("common.battery")} - ${deviceName}` : I18n.tr("common.battery")
 
-    readonly property var allDevices: {
-      var list = [];
-      var seenPaths = new Set();
+    // Use the centralized list of all devices
+    readonly property var allDevices: BatteryService.devices
 
-      // Add UPower batteries
-      if (UPower.devices) {
-        var upowerArray = UPower.devices.values || [];
-        for (var i = 0; i < upowerArray.length; i++) {
-          var d = upowerArray[i];
-          if (BatteryService.isDevicePresent(d) && d.type === UPowerDeviceType.Battery) {
-            if (d.nativePath && !seenPaths.has(d.nativePath)) {
-              list.push(d);
-              seenPaths.add(d.nativePath);
-            }
-          }
-        }
-      }
-      // Add Bluetooth batteries
-      if (BluetoothService.devices) {
-        var btArray = BluetoothService.devices.values || [];
-        for (var j = 0; j < btArray.length; j++) {
-          var btd = btArray[j];
-          if (BatteryService.isDevicePresent(btd) && btd.batteryAvailable) {
-            // Bluetooth devices use address as unique ID
-            if (btd.address && !seenPaths.has(btd.address)) {
-              list.push(btd);
-              seenPaths.add(btd.address);
-            }
-          }
-        }
-      }
+    readonly property var laptopBatteries: BatteryService.laptopBatteries
+    readonly property var otherDevices: BatteryService.externalBatteries
 
-      // Fallback: if no specific batteries found but display device is a battery, use it
-      if (list.length === 0 && UPower.displayDevice && UPower.displayDevice.type === UPowerDeviceType.Battery && BatteryService.isDevicePresent(UPower.displayDevice)) {
-        list.push(UPower.displayDevice);
-      }
-
-      return list;
-    }
-
-    readonly property var laptopBatteries: allDevices.filter(d => !BatteryService.isBluetoothDevice(d))
-    readonly property var otherDevices: allDevices.filter(d => BatteryService.isBluetoothDevice(d))
-
-    readonly property string timeText: {
-      if (!isReady || !isDevicePresent) {
-        return I18n.tr("battery.no-battery-detected");
-      }
-      if (isPluggedIn) {
-        return I18n.tr("battery.plugged-in");
-      }
-      if (selectedDevice) {
-        if (selectedDevice.timeToFull > 0) {
-          return I18n.tr("battery.time-until-full", {
-                           "time": Time.formatVagueHumanReadableDuration(selectedDevice.timeToFull)
-                         });
-        }
-        if (selectedDevice.timeToEmpty > 0) {
-          return I18n.tr("battery.time-left", {
-                           "time": Time.formatVagueHumanReadableDuration(selectedDevice.timeToEmpty)
-                         });
-        }
-      }
-      return I18n.tr("common.idle");
-    }
+    readonly property string timeText: BatteryService.getTimeRemainingText(selectedDevice)
     readonly property string iconName: BatteryService.getIcon(percent, isCharging, isPluggedIn, isReady)
 
     property var batteryWidgetInstance: BarService.lookupWidget("Battery", screen ? screen.name : null)
@@ -302,7 +242,7 @@ SmartPanel {
               ColumnLayout {
                 Layout.fillWidth: true
                 spacing: Style.marginS
-                visible: modelData.healthSupported || (modelData === selectedBattery && BatteryService.healthAvailable)
+                visible: modelData.healthSupported || (modelData === BatteryService.primaryDevice && BatteryService.healthAvailable)
                 RowLayout {
                   Layout.fillWidth: true
                   spacing: Style.marginS
@@ -333,14 +273,14 @@ SmartPanel {
                       height: parent.height
                       radius: parent.radius
                       width: {
-                        var h = modelData.healthSupported ? modelData.healthPercentage : (modelData === selectedBattery ? BatteryService.healthPercent : 0);
+                        var h = modelData.healthSupported ? modelData.healthPercentage : (modelData === BatteryService.primaryDevice ? BatteryService.healthPercent : 0);
                         if (h <= 0)
                           return 0;
                         var ratio = Math.max(0, Math.min(1, h / 100));
                         return parent.width * ratio;
                       }
                       color: {
-                        var h = modelData.healthSupported ? modelData.healthPercentage : (modelData === selectedBattery ? BatteryService.healthPercent : 0);
+                        var h = modelData.healthSupported ? modelData.healthPercentage : (modelData === BatteryService.primaryDevice ? BatteryService.healthPercent : 0);
                         return h >= 80 ? Color.mPrimary : (h >= 50 ? Color.mTertiary : Color.mError);
                       }
                     }
@@ -349,7 +289,7 @@ SmartPanel {
                     Layout.preferredWidth: 40 * Style.uiScaleRatio
                     horizontalAlignment: Text.AlignRight
 
-                    readonly property int h: modelData.healthSupported ? Math.round(modelData.healthPercentage) : (modelData === selectedBattery ? BatteryService.healthPercent : -1)
+                    readonly property int h: modelData.healthSupported ? Math.round(modelData.healthPercentage) : (modelData === BatteryService.primaryDevice ? BatteryService.healthPercent : -1)
                     text: h >= 0 ? `${h}%` : "--"
                     color: Color.mOnSurface
                     pointSize: Style.fontSizeS

Modules/Panels/Settings/Bar/WidgetSettings/BatterySettings.qml

@@ -1,8 +1,8 @@
 import QtQuick
 import QtQuick.Controls
 import QtQuick.Layouts
-import Quickshell.Services.UPower
 import qs.Commons
+import qs.Services.Hardware
 import qs.Widgets
 
 ColumnLayout {
@@ -24,58 +24,12 @@ ColumnLayout {
   property bool valueHideIfNotDetected: widgetData.hideIfNotDetected !== undefined ? widgetData.hideIfNotDetected : widgetMetadata.hideIfNotDetected
   property bool valueHideIfIdle: widgetData.hideIfIdle !== undefined ? widgetData.hideIfIdle : widgetMetadata.hideIfIdle
 
-  // Build model of available battery devices
-  function buildDeviceModel() {
-    var model = [
-          {
-            "key": "",
-            "name": I18n.tr("bar.battery.device-default")
-          }
-        ];
-
-    if (!UPower.devices) {
-      return model;
-    }
-
-    var deviceArray = UPower.devices.values || [];
-    for (var i = 0; i < deviceArray.length; i++) {
-      var device = deviceArray[i];
-      if (!device || device.type === UPowerDeviceType.LinePower) {
-        continue;
-      }
-      var displayName = device.model || device.nativePath || "Unknown";
-      model.push({
-                   "key": device.nativePath || "",
-                   "name": displayName
-                 });
-    }
-    return model;
-  }
-
-  readonly property int _deviceCount: (UPower.devices && UPower.devices.values) ? UPower.devices.values.length : 0
-  property var deviceModel: buildDeviceModel()
-
-  on_DeviceCountChanged: {
-    deviceModel = buildDeviceModel();
-  }
+  property var deviceModel: BatteryService.getDeviceOptionsModel()
 
   Connections {
-    target: UPower.devices
-    function onValuesChanged() {
-      deviceModel = buildDeviceModel();
-    }
-  }
-
-  Timer {
-    id: refreshTimer
-    interval: 2000
-    running: true
-    repeat: true
-    onTriggered: {
-      var currentCount = (UPower.devices && UPower.devices.values) ? UPower.devices.values.length : 0;
-      if (currentCount !== root._deviceCount) {
-        deviceModel = buildDeviceModel();
-      }
+    target: BatteryService
+    function onDevicesChanged() {
+      deviceModel = BatteryService.getDeviceOptionsModel();
     }
   }
 
@@ -128,7 +82,7 @@ ColumnLayout {
     NIconButton {
       icon: "refresh"
       tooltipText: "Refresh device list"
-      onClicked: deviceModel = buildDeviceModel()
+      onClicked: deviceModel = BatteryService.getDeviceOptionsModel()
     }
   }
 

Services/Hardware/BatteryService.qml

@@ -11,14 +11,60 @@ import qs.Services.UI
 Singleton {
   id: root
 
-  // Cached device lookups (computed once, used by all properties)
-  readonly property var _laptopBattery: {
-    if (!UPower.devices)
-    return UPower.displayDevice;
+  // 1. Centralized list of all batteries
+  readonly property var devices: {
+    var list = [];
+    var seenPaths = new Set();
 
-    var devices = UPower.devices.values || [];
+    // Add UPower batteries
+    var upowerBatteryCount = 0;
+    if (UPower.devices) {
+      var upowerArray = UPower.devices.values || [];
+      for (var i = 0; i < upowerArray.length; i++) {
+        var d = upowerArray[i];
+        if (isDevicePresent(d) && d.type === UPowerDeviceType.Battery) {
+          if (d.nativePath && !seenPaths.has(d.nativePath)) {
+            list.push(d);
+            seenPaths.add(d.nativePath);
+            upowerBatteryCount++;
+          }
+        }
+      }
+    }
+    // Add DisplayDevice (Aggregate)
+    if (UPower.displayDevice && UPower.displayDevice.type === UPowerDeviceType.Battery && isDevicePresent(UPower.displayDevice)) {
+      if (upowerBatteryCount !== 1) {
+        list.push(UPower.displayDevice);
+      }
+    }
+    // Add Bluetooth batteries
+    if (BluetoothService.devices) {
+      var btArray = BluetoothService.devices.values || [];
+      for (var j = 0; j < btArray.length; j++) {
+        var btd = btArray[j];
+        if (isDevicePresent(btd) && btd.batteryAvailable) {
+          // Bluetooth devices use address as unique ID
+          if (btd.address && !seenPaths.has(btd.address)) {
+            list.push(btd);
+            seenPaths.add(btd.address);
+          }
+        }
+      }
+    }
+    return list;
+  }
 
-    // 1. Explicitly look for BAT0 first
+  // 2. Determine the primary device (System Battery)
+  readonly property var primaryDevice: {
+    if (devices.length === 0)
+    return null;
+
+    // Prioritize DisplayDevice (Aggregate)
+    if (UPower.displayDevice && UPower.displayDevice.type === UPowerDeviceType.Battery && isDevicePresent(UPower.displayDevice)) {
+      return UPower.displayDevice;
+    }
+
+    // Prioritize BAT0
     for (var i = 0; i < devices.length; i++) {
       var d = devices[i];
       if (d && (d.nativePath === "BAT0" || d.objectPath === "/org/freedesktop/UPower/devices/battery_BAT0")) {
@@ -26,94 +72,69 @@ Singleton {
       }
     }
 
-    // 2. Fallback to displayDevice if it's a laptop battery
-    if (UPower.displayDevice && UPower.displayDevice.isLaptopBattery) {
-      return UPower.displayDevice;
-    }
-
-    // 3. Any other device marked as a laptop battery
+    // Prioritize (any) Laptop Battery
     for (var j = 0; j < devices.length; j++) {
-      var device = devices[j];
-      if (device && device.type === UPowerDeviceType.Battery && device.isLaptopBattery) {
-        return device;
+      var dev = devices[j];
+      if (dev && !isBluetoothDevice(dev) && dev.isLaptopBattery) {
+        return dev;
       }
     }
 
-    if (UPower.displayDevice.isPresent) {
-      return UPower.displayDevice;
-    }
-    return null;
+    // Fallback to the first available device
+    return devices[0];
   }
 
-  readonly property var _bluetoothBattery: {
-    var devices = BluetoothService.devices ? (BluetoothService.devices.values || []) : [];
-    for (var i = 0; i < devices.length; i++) {
-      var device = devices[i];
-      if (device && device.connected && device.batteryAvailable) {
-        return device;
-      }
-    }
-    return null;
-  }
-
-  // Primary battery device (prioritizes laptop over Bluetooth)
-  readonly property var primaryDevice: _laptopBattery || _bluetoothBattery || null
-
   // Whether the primary device is a laptop battery
-  readonly property bool isLaptopBattery: _laptopBattery !== null && primaryDevice === _laptopBattery
+  readonly property bool isLaptopBattery: primaryDevice !== null && !isBluetoothDevice(primaryDevice) && primaryDevice.isLaptopBattery
 
+  // Global properties for the Primary Device (used by LockScreen etc)
   readonly property real batteryPercentage: getPercentage(primaryDevice)
-
   readonly property bool batteryCharging: isCharging(primaryDevice)
-
   readonly property bool batteryPluggedIn: isPluggedIn(primaryDevice)
-
   readonly property bool batteryReady: isDeviceReady(primaryDevice)
-
   readonly property bool batteryPresent: isDevicePresent(primaryDevice)
 
+  // Exposed subsets of devices
+  readonly property var laptopBatteries: devices.filter(d => !isBluetoothDevice(d))
+  readonly property var externalBatteries: devices.filter(d => isBluetoothDevice(d))
+
   property bool healthAvailable: false
   property int healthPercent: -1
 
-  function findUPowerDevice(nativePath) {
-    if (!nativePath || nativePath === "") {
-      return _laptopBattery;
-    }
+  // Initialization state
+  property bool initializationComplete: false
+  readonly property bool ready: initializationComplete
 
-    if (!UPower.devices) {
-      return null;
-    }
-
-    var deviceArray = UPower.devices.values || [];
-    for (var i = 0; i < deviceArray.length; i++) {
-      var device = deviceArray[i];
-      if (device && device.nativePath === nativePath) {
-        if (device.type === UPowerDeviceType.LinePower) {
-          continue;
-        }
-        return device;
-      }
-    }
-    return null;
+  Timer {
+    interval: 500
+    running: true
+    repeat: false
+    onTriggered: root.initializationComplete = true
   }
 
-  function findBluetoothDevice(nativePath) {
-    if (!nativePath || !BluetoothService.devices) {
-      return null;
+  // 3. Helper to resolve a device by path, or return primary if path is empty/invalid
+  function resolveDevice(nativePath) {
+    if (!nativePath || nativePath === "") {
+      return primaryDevice;
     }
 
-    var macMatch = nativePath.match(/([0-9a-fA-F]{2}:[0-9a-fA-F]{2}:[0-9a-fA-F]{2}:[0-9a-fA-F]{2}:[0-9a-fA-F]{2}:[0-9a-fA-F]{2})/);
-    if (!macMatch) {
-      return null;
+    // Check for DisplayDevice explicitly if requested via "DisplayDevice" or empty string
+    if (nativePath === "DisplayDevice" && UPower.displayDevice) {
+      return UPower.displayDevice;
     }
 
-    var macAddress = macMatch[1].toUpperCase();
-    var deviceArray = BluetoothService.devices.values || [];
-
-    for (var i = 0; i < deviceArray.length; i++) {
-      var device = deviceArray[i];
-      if (device && device.address && device.address.toUpperCase() === macAddress) {
-        return device;
+    // Search in our cached list
+    for (var i = 0; i < devices.length; i++) {
+      var d = devices[i];
+      if (isBluetoothDevice(d)) {
+        if (d.address && d.address.toUpperCase() === nativePath.toUpperCase())
+          return d;
+        // Try matching MAC in path string if passed format differs
+        if (nativePath.includes(d.address.toUpperCase()))
+          return d;
+      } else {
+        if (d.nativePath === nativePath)
+          return d;
       }
     }
     return null;
@@ -123,7 +144,7 @@ Singleton {
     if (!device)
       return false;
 
-    // Handle Bluetooth devices (identified by having batteryAvailable property)
+    // Handle Bluetooth devices
     if (device.batteryAvailable !== undefined) {
       return device.connected === true;
     }
@@ -133,11 +154,8 @@ Singleton {
       if (device.type === UPowerDeviceType.Battery && device.isPresent !== undefined) {
         return device.isPresent === true;
       }
-
-      // Fallback for non-battery UPower devices or if isPresent is missing
       return device.ready && device.percentage !== undefined;
     }
-
     return false;
   }
 
@@ -148,7 +166,6 @@ Singleton {
     if (device.batteryAvailable !== undefined) {
       return device.battery !== undefined;
     }
-
     return device.ready && device.percentage !== undefined;
   }
 
@@ -197,6 +214,68 @@ Singleton {
     return "";
   }
 
+  function getTimeRemainingText(device) {
+    if (!ready || !isDevicePresent(device)) {
+      return I18n.tr("battery.no-battery-detected");
+    }
+    if (isPluggedIn(device)) {
+      return I18n.tr("battery.plugged-in");
+    }
+    if (device) {
+      if (device.timeToFull > 0) {
+        return I18n.tr("battery.time-until-full", {
+                         "time": Time.formatVagueHumanReadableDuration(device.timeToFull)
+                       });
+      }
+      if (device.timeToEmpty > 0) {
+        return I18n.tr("battery.time-left", {
+                         "time": Time.formatVagueHumanReadableDuration(device.timeToEmpty)
+                       });
+      }
+    }
+    return I18n.tr("common.idle");
+  }
+
+  function getDeviceOptionsModel() {
+    var model = [
+          {
+            "key": "",
+            "name": I18n.tr("bar.battery.device-default")
+          }
+        ];
+
+    for (var i = 0; i < devices.length; i++) {
+      var d = devices[i];
+      var name = "";
+
+      // Determine friendly name
+      if (isBluetoothDevice(d)) {
+        name = d.name || "Bluetooth Device";
+      } else if (d === UPower.displayDevice) {
+        name = I18n.tr("common.battery-aggregate") || "Display Device";
+      } else {
+        name = d.model || I18n.tr("common.battery");
+      }
+
+      // Determine ID/Path
+      var key = isBluetoothDevice(d) ? d.address : d.nativePath;
+      if (!key && d === UPower.displayDevice)
+        key = "DisplayDevice";
+
+      // Format: "Model (ID)"
+      var displayName = name;
+      if (key && key !== "DisplayDevice") {
+        displayName = `${name} (${key})`;
+      }
+
+      model.push({
+                   "key": key || "",
+                   "name": displayName
+                 });
+    }
+    return model;
+  }
+
   function refreshHealth() {
     if (!isLaptopBattery || !primaryDevice) {
       healthAvailable = false;
@@ -208,7 +287,8 @@ Singleton {
 
   Process {
     id: healthProcess
-    command: ["sh", "-c", "upower -i $(upower -e | grep battery | head -n 1) 2>/dev/null | grep -iE 'capacity'"]
+    // Dynamically target the primary device if possible, otherwise fall back to first battery
+    command: ["sh", "-c", `upower -i ${primaryDevice.nativePath ? "/org/freedesktop/UPower/devices/battery_" + primaryDevice.nativePath : "$(upower -e | grep battery | head -n 1)"} 2>/dev/null | grep -iE 'capacity'`]
     environment: ({
                     "LC_ALL": "C"
                   })