linux: stop assuming power supply of unknown type as battery

Some vendor kernel (most notably Android devices) expose various types
of BMS (battery management system) as power supplies. This is something
UPower has never designed to deal with, and thus UPower should not
represent or consider it to be a battery.

Fortunately, most of the time the actual "battery" power supply has the
correct type, so we can safely ignore those devices which have unknown
type. Also, the code that assumes power supply of unknown type seems
pretty dated and probably doesn't make sense anymore. So, let's remove
this assumption altogether.

3 files changed, 70 insertions, 11 deletions

diff --git a/src/linux/integration-test.py b/src/linux/integration-test.py
index 4eda932..e686125 100755
--- a/src/linux/integration-test.py
+++ b/src/linux/integration-test.py
@@ -2646,6 +2646,60 @@ class Tests(dbusmock.DBusTestCase):
 
         self.stop_daemon()
 
+    def test_ignore_unknown_power_supply_type(self):
+        '''
+        Ignore devices with unknown power supply type and doesn't look like a charger.
+        '''
+
+        # Sample data taken from Fairphone 4, running Linux kernel 4.19 w/
+        # vendor patches (abbreviated).
+
+        # Actual battery.
+        battery = self.testbed.add_device('power_supply', 'battery', None,
+                ['capacity', '77',
+                 'charging_enabled', '1',
+                 'health', 'Good',
+                 'present', '1',
+                 'status', 'Charging',
+                 'technology', 'Li-ion',
+                 'type', 'Battery',
+                 'voltage_max', '4400000',
+                 'voltage_now', '4268584'], [])
+
+        # BMS (should be ignored)
+        bms = self.testbed.add_device('power_supply', 'bms', None,
+                ['type', 'BMS',
+                 'capacity', '77',
+                 'capacity_raw', '7685',
+                 'current_avg', '-1677247',
+                 'current_now', '-1543885',
+                 'power_avg', '29886557',
+                 'power_now', '52842898',
+                 'real_capacity', '77',
+                 'temp', '300',
+                 'voltage_avg', '4322887',
+                 'voltage_max', '4400000',
+                 'voltage_min', '3400000',
+                 'voltage_now', '4298363',
+                 'voltage_ocv', '4102200'], [])
+
+        # "Charge pump master" (not sure what it is either, should be ignored)
+        charge_pump_master = self.testbed.add_device(
+            'power_supply', 'charge_pump_master', None,
+                ['chip_version', '3',
+                 'min_icl', '1000000',
+                 'model_name', 'SMB1398_V2',
+                 'parallel_mode', '1',
+                 'parallel_output_mode', '2',
+                 'type', 'Nothing attached'], [])
+
+        self.start_daemon(warns=True)
+        devs = self.proxy.EnumerateDevices()
+        # Only the battery should be listed, not the BMS or charge pump master.
+        self.assertEqual(len(devs), 1)
+
+        self.stop_daemon()
+
     #
     # libupower-glib tests (through introspection)
     #
diff --git a/src/linux/up-device-supply-battery.c b/src/linux/up-device-supply-battery.c
index f37c8a4..456f692 100644
--- a/src/linux/up-device-supply-battery.c
+++ b/src/linux/up-device-supply-battery.c
@@ -265,15 +265,10 @@ up_device_supply_coldplug (UpDevice *device)
 	if (scope != NULL && g_ascii_strcasecmp (scope, "system") != 0)
 		g_warning ("Assuming system scope even though scope is %s", scope);
 
-	/* type should be a battery, but also accept unknown if "online" does not exist */
+	/* type must be a battery. */
 	type = g_udev_device_get_sysfs_attr (native, "type");
-	if (!type || g_ascii_strcasecmp (type, "battery") != 0) {
-		if (g_udev_device_has_sysfs_attr (native, "online"))
-			return FALSE;
-
-		/* this is a good guess as UPS and CSR are not in the kernel */
-		g_warning ("Assuming battery as sysfs attribute 'type' is %s", type);
-	}
+	if (!type || g_ascii_strcasecmp (type, "battery") != 0)
+		return FALSE;
 
 	return TRUE;
 }
diff --git a/src/linux/up-device-supply.c b/src/linux/up-device-supply.c
index 4081322..27f73af 100644
--- a/src/linux/up-device-supply.c
+++ b/src/linux/up-device-supply.c
@@ -594,13 +594,23 @@ up_device_supply_coldplug (UpDevice *device)
 	/* try to detect using the device type */
 	type = up_device_supply_guess_type (native, native_path);
 
-	/* if reading the device type did not work, use the previous method */
+	/* if reading the device type did not work, use heuristic */
 	if (type == UP_DEVICE_KIND_UNKNOWN) {
 		if (g_udev_device_has_sysfs_attr_uncached (native, "online")) {
+			g_debug ("'online' attribute was found. "
+				 "Assume it is a line power supply.");
 			type = UP_DEVICE_KIND_LINE_POWER;
 		} else {
-			/* this is a good guess as UPS and CSR are not in the kernel */
-			type = UP_DEVICE_KIND_BATTERY;
+			/*
+			 * This might be a battery or a UPS, but it might also
+			 * be something else that we really don't know how to
+			 * handle (e.g. BMS, exposed by Android-centric vendor
+			 * kernels in parallel to actual battery).
+			 *
+			 * As such, we have no choice but to assume that we
+			 * can't handle this device, and ignore it.
+			 */
+			return FALSE;
 		}
 	}