[upower PATCH 1/2] simplify 'on-battery' detection to avoid depending on unreliable battery state
Michal Schmidt
mschmidt at redhat.com
Tue Oct 12 05:42:30 PDT 2010
When calculating the global 'on-battery' state upowerd queries the state of AC
supplies as well as the state of all batteries.
On my Fujitsu Esprimo Mobile M9400 laptop the AC information is always correct,
but the battery is not giving useful status (from looking at ACPI DSDT,
I concluded that its _BST method fills the State field incorrectly).
In /sys the battery reports "Charging" correctly when AC is online, but it
always says "Unknown" otherwise.
upowerd has a few heuristics to guess the real battery state when it sees
"Unknown". One way it does that is by looking at the current global
'on-battery' state. But then the global 'on-battery' state is computed using
the state of the battery. This is a circular definition.
The circle results in bigger trouble than necessary on my laptop:
1. The laptop is plugged into AC power. 'on-battery' is 'no'. Good.
2. I unplug AC power. The battery starts to report 'Unknown' in /sys.
3. upowerd guesses the battery state based on the global 'on-battery' state
which is still 'no' at this point. So its wrong guess is FULLY_CHARGED.
4. Because the battery state is FULLY_CHARGED (i.e. something else than
DISCHARGING), the next 'on-battery' state will be 'no' again.
5. after a while, goto 3
After a couple of hours the battery runs out with g-p-m all the time believing
it is fully charged.
A fix could be to remove all the guesswork in step 3. But the heuristics may be
useful in other cases.
So another way to fix it is by breaking the circle at step 4. The 'on-battery'
state should reflect the (inverse of) AC online state, nothing else. Then the
guessing can work correctly (AC offline => on-battery => the battery is likely
discharging).
---
src/freebsd/up-device-supply.c | 1 -
src/linux/up-device-supply.c | 1 -
src/up-daemon.c | 35 ++---------------------------------
src/up-device.c | 19 -------------------
src/up-device.h | 4 ----
5 files changed, 2 insertions(+), 58 deletions(-)
diff --git a/src/freebsd/up-device-supply.c b/src/freebsd/up-device-supply.c
index b499d57..fa3fb24 100644
--- a/src/freebsd/up-device-supply.c
+++ b/src/freebsd/up-device-supply.c
@@ -524,7 +524,6 @@ up_device_supply_class_init (UpDeviceSupplyClass *klass)
UpDeviceClass *device_class = UP_DEVICE_CLASS (klass);
object_class->finalize = up_device_supply_finalize;
- device_class->get_on_battery = up_device_supply_get_on_battery;
device_class->get_low_battery = up_device_supply_get_low_battery;
device_class->get_online = up_device_supply_get_online;
device_class->coldplug = up_device_supply_coldplug;
diff --git a/src/linux/up-device-supply.c b/src/linux/up-device-supply.c
index b1e03c3..3287efb 100644
--- a/src/linux/up-device-supply.c
+++ b/src/linux/up-device-supply.c
@@ -909,7 +909,6 @@ up_device_supply_class_init (UpDeviceSupplyClass *klass)
UpDeviceClass *device_class = UP_DEVICE_CLASS (klass);
object_class->finalize = up_device_supply_finalize;
- device_class->get_on_battery = up_device_supply_get_on_battery;
device_class->get_low_battery = up_device_supply_get_low_battery;
device_class->get_online = up_device_supply_get_online;
device_class->coldplug = up_device_supply_coldplug;
diff --git a/src/up-daemon.c b/src/up-daemon.c
index 532ebe8..5c8eb02 100644
--- a/src/up-daemon.c
+++ b/src/up-daemon.c
@@ -94,7 +94,6 @@ struct UpDaemonPrivate
};
static void up_daemon_finalize (GObject *object);
-static gboolean up_daemon_get_on_battery_local (UpDaemon *daemon);
static gboolean up_daemon_get_on_low_battery_local (UpDaemon *daemon);
static gboolean up_daemon_get_on_ac_local (UpDaemon *daemon);
@@ -113,35 +112,6 @@ G_DEFINE_TYPE (UpDaemon, up_daemon, G_TYPE_OBJECT)
#define UP_DAEMON_POLL_BATTERY_NUMBER_TIMES 5
/**
- * up_daemon_get_on_battery_local:
- *
- * As soon as _any_ battery goes discharging, this is true
- **/
-static gboolean
-up_daemon_get_on_battery_local (UpDaemon *daemon)
-{
- guint i;
- gboolean ret;
- gboolean result = FALSE;
- gboolean on_battery;
- UpDevice *device;
- GPtrArray *array;
-
- /* ask each device */
- array = up_device_list_get_array (daemon->priv->power_devices);
- for (i=0; i<array->len; i++) {
- device = (UpDevice *) g_ptr_array_index (array, i);
- ret = up_device_get_on_battery (device, &on_battery);
- if (ret && on_battery) {
- result = TRUE;
- break;
- }
- }
- g_ptr_array_unref (array);
- return result;
-}
-
-/**
* up_daemon_get_number_devices_of_type:
**/
guint
@@ -724,8 +694,7 @@ up_daemon_startup (UpDaemon *daemon)
}
/* get battery state */
- on_battery = (up_daemon_get_on_battery_local (daemon) &&
- !up_daemon_get_on_ac_local (daemon));
+ on_battery = !up_daemon_get_on_ac_local (daemon);
on_low_battery = up_daemon_get_on_low_battery_local (daemon);
up_daemon_set_on_battery (daemon, on_battery);
up_daemon_set_on_low_battery (daemon, on_low_battery);
@@ -865,7 +834,7 @@ up_daemon_device_changed_cb (UpDevice *device, UpDaemon *daemon)
}
/* second, check if the on_battery and on_low_battery state has changed */
- ret = (up_daemon_get_on_battery_local (daemon) && !up_daemon_get_on_ac_local (daemon));
+ ret = !up_daemon_get_on_ac_local (daemon);
if (ret != priv->on_battery) {
up_daemon_set_on_battery (daemon, ret);
diff --git a/src/up-device.c b/src/up-device.c
index a6de925..5612963 100644
--- a/src/up-device.c
+++ b/src/up-device.c
@@ -367,25 +367,6 @@ up_device_set_property (GObject *object, guint prop_id, const GValue *value, GPa
}
/**
- * up_device_get_on_battery:
- *
- * Note: Only implement for system devices, i.e. ones supplying the system
- **/
-gboolean
-up_device_get_on_battery (UpDevice *device, gboolean *on_battery)
-{
- UpDeviceClass *klass = UP_DEVICE_GET_CLASS (device);
-
- g_return_val_if_fail (UP_IS_DEVICE (device), FALSE);
-
- /* no support */
- if (klass->get_on_battery == NULL)
- return FALSE;
-
- return klass->get_on_battery (device, on_battery);
-}
-
-/**
* up_device_get_low_battery:
*
* Note: Only implement for system devices, i.e. ones supplying the system
diff --git a/src/up-device.h b/src/up-device.h
index 020e529..f98b809 100644
--- a/src/up-device.h
+++ b/src/up-device.h
@@ -53,8 +53,6 @@ typedef struct
gboolean (*coldplug) (UpDevice *device);
gboolean (*refresh) (UpDevice *device);
const gchar *(*get_id) (UpDevice *device);
- gboolean (*get_on_battery) (UpDevice *device,
- gboolean *on_battery);
gboolean (*get_low_battery) (UpDevice *device,
gboolean *low_battery);
gboolean (*get_online) (UpDevice *device,
@@ -82,8 +80,6 @@ gboolean up_device_coldplug (UpDevice *device,
UpDaemon *up_device_get_daemon (UpDevice *device);
GObject *up_device_get_native (UpDevice *device);
const gchar *up_device_get_object_path (UpDevice *device);
-gboolean up_device_get_on_battery (UpDevice *device,
- gboolean *on_battery);
gboolean up_device_get_low_battery (UpDevice *device,
gboolean *low_battery);
gboolean up_device_get_online (UpDevice *device,
More information about the devkit-devel
mailing list