Hello Jörn, thanks for looking into this! I'm also forwarding your results on gkrellm mailing list, so upstream authors can have a look at your patch and comment/commit it.
Dear gkrellm authors, attached is a patch to fix a problem with battery level upon suspension; you can also have a look at the full bug history to get a better idea of how Jörn reached to this patch: http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=630117 Cheers, Sandro On Sun, Apr 1, 2012 at 23:48, Jörn Engel <jo...@logfs.org> wrote: > On Thinkpads T410s I have observed sysfs to switch between energy_full > and friends and charge_full and friends across suspend/resume. Result > is a battery level of 0 after resume, which is annoying at best and can > result in unwanted reboots at worst (when abusing gkrellm to suspend the > system if battery runs low). > > While one can argue that the kernel should make up its mind and stick to > one variant, the bad kernels are an ugly fact of life and we have to > deal with them. > > signed-off-by: Joern Engel <jo...@logfs.org> > --- > src/sysdeps/linux.c | 157 > ++++++++++++++++++--------------------------------- > 1 files changed, 56 insertions(+), 101 deletions(-) > > diff --git a/src/sysdeps/linux.c b/src/sysdeps/linux.c > index e4ff625..a167d70 100644 > --- a/src/sysdeps/linux.c > +++ b/src/sysdeps/linux.c > @@ -1801,14 +1801,15 @@ acpi_battery_data(BatteryFile *bf) > #define SYSFS_TYPE_AC_ADAPTER "mains" > > > +#define VARIANTS 5 > typedef struct syspower > { > gint type; > gint id; > - gint charge_units; > + gint charge_units[VARIANTS]; > gchar const *sysdir; > - gchar const *sys_charge_full; > - gchar const *sys_charge_now; > + gchar const *sys_charge_full[VARIANTS]; > + gchar const *sys_charge_now[VARIANTS]; > gboolean present; > gboolean ac_present; > gboolean charging; > @@ -1872,7 +1873,6 @@ sysfs_power_data (struct syspower *sp) > gchar buf[128]; > gchar *syszap; > gboolean charging; > - gboolean stat_full; > > time_left = -1; > charge_full = charge_now = 0; > @@ -1909,23 +1909,36 @@ sysfs_power_data (struct syspower *sp) > > if (present) > { > - if (read_sysfs_entry (buf, sizeof (buf), sp->sys_charge_full)) > - { > - charge_full = strtoll (buf, NULL, 0); > - } > - if (read_sysfs_entry (buf, sizeof (buf), sp->sys_charge_now)) > - { > - charge_now = strtoll (buf, NULL, 0); > - } > - if (sp->charge_units == CHGUNITS_PERCENT) > - { > - percent = charge_now; > - } > - else > - { > - if (charge_full > 0) > - percent = charge_now * 100 / charge_full; > - } > + int i; > + > + for (i = 0; i < VARIANTS; i++) { > + if (read_sysfs_entry (buf, sizeof (buf), > sp->sys_charge_full[i])) > + { > + charge_full = strtoll (buf, NULL, 0); > + } > + else > + { > + continue; > + } > + if (read_sysfs_entry (buf, sizeof (buf), > sp->sys_charge_now[i])) > + { > + charge_now = strtoll (buf, NULL, 0); > + } > + else > + { > + continue; > + } > + if (sp->charge_units[i] == CHGUNITS_PERCENT) > + { > + percent = charge_now; > + } > + else > + { > + if (charge_full > 0) > + percent = charge_now * 100 / > charge_full; > + } > + break; > + } > > /* Get charging status. */ > *syszap = '\0'; > @@ -1933,7 +1946,6 @@ sysfs_power_data (struct syspower *sp) > if (read_sysfs_entry (buf, sizeof (buf), sysentry)) > { > charging = !strcasecmp (buf, "charging"); > - stat_full = !strcasecmp (buf, "full"); > } > } > > @@ -1951,12 +1963,10 @@ setup_sysfs_ac_power (gchar const *sysdir) > if (_GK.debug_level & DEBUG_BATTERY) > g_debug ("setup_sysfs_ac_power: %s\n", sysdir); > sp = g_new0 (syspower, 1); > + memset(sp, 0, sizeof(*sp)); > sp->type = PWRTYPE_MAINS; > sp->id = g_pwr_id++; > - sp->charge_units = CHGUNITS_INVALID; > sp->sysdir = g_strdup (sysdir); > - sp->sys_charge_full = > - sp->sys_charge_now = NULL; > > /* Add mains power sources to head of list. */ > g_sysfs_power_list = g_list_prepend (g_sysfs_power_list, sp); > @@ -1968,94 +1978,39 @@ static gboolean > setup_sysfs_battery (gchar const *sysdir) > { > syspower *sp; > - gchar *sys_charge_full = NULL, > - *sys_charge_now = NULL; > - gint units; > - gboolean retval = FALSE; > > + sp = g_new0 (syspower, 1); > + sp->type = PWRTYPE_BATTERY; > + sp->id = g_pwr_id++; > + sp->sysdir = g_strdup (sysdir); > /* > * There are three flavors of reporting: 'energy', 'charge', and > - * 'capacity'. Check for them in that order. (Apologies for the > - * ugliness; you try coding an unrolled 'if ((A || B) && C)' and make > it > - * pretty.) > + * 'capacity'. Check for them in that order. > */ > - if (_GK.debug_level & DEBUG_BATTERY) > - g_debug ("setup_sysfs_battery: %s\n", sysdir); > - units = CHGUNITS_uWH; > - sys_charge_full = g_strconcat (sysdir, "/energy_full", NULL); > - if (access (sys_charge_full, F_OK | R_OK)) > - { > - g_free (sys_charge_full); > - sys_charge_full = g_strconcat (sysdir, "/energy_full_design", > NULL); > - if (access (sys_charge_full, F_OK | R_OK)) > - { > - goto try_charge; /* Look down */ > - } > - } > - sys_charge_now = g_strconcat (sysdir, "/energy_now", NULL); > - if (!access (sys_charge_now, F_OK | R_OK)) > - goto done; /* Look down */ > - > -try_charge: > - if (sys_charge_full) g_free (sys_charge_full), sys_charge_full = > NULL; > - if (sys_charge_now) g_free (sys_charge_now), > sys_charge_now = NULL; > - > - units = CHGUNITS_uAH; > - sys_charge_full = g_strconcat (sysdir, "/charge_full", NULL); > - if (access (sys_charge_full, F_OK | R_OK)) > - { > - g_free (sys_charge_full); > - sys_charge_full = g_strconcat (sysdir, "/charge_full_design", > NULL); > - if (access (sys_charge_full, F_OK | R_OK)) > - { > - goto try_capacity; /* Look down */ > - } > - } > - sys_charge_now = g_strconcat (sysdir, "/charge_now", NULL); > - if (!access (sys_charge_now, F_OK | R_OK)) > - goto done; /* Look down */ > + sp->charge_units[0] = CHGUNITS_uWH; > + sp->sys_charge_full[0] = g_strconcat (sysdir, "/energy_full", NULL); > + sp->sys_charge_now[0] = g_strconcat (sysdir, "/energy_now", NULL); > > -try_capacity: > - if (sys_charge_full) g_free (sys_charge_full), sys_charge_full = > NULL; > - if (sys_charge_now) g_free (sys_charge_now), > sys_charge_now = NULL; > + sp->charge_units[1] = CHGUNITS_uWH; > + sp->sys_charge_full[1] = g_strconcat (sysdir, "/energy_full_design", > NULL); > + sp->sys_charge_now[1] = g_strconcat (sysdir, "/energy_now", NULL); > > - /* This one's a little simpler... */ > - units = CHGUNITS_PERCENT; > - /* > - * FIXME: I have no idea if 'capacity_full' actually shows up, since > - * 'capacity' always defines "full" as always 100% > - */ > - sys_charge_full = g_strconcat (sysdir, "/capacity_full", NULL); > - if (access (sys_charge_full, F_OK | R_OK)) > - goto ackphft; /* Look down */ > + sp->charge_units[2] = CHGUNITS_uAH; > + sp->sys_charge_full[2] = g_strconcat (sysdir, "/charge_full", NULL); > + sp->sys_charge_now[2] = g_strconcat (sysdir, "/charge_now", NULL); > > - sys_charge_now = g_strconcat (sysdir, "/capacity_now", NULL); > - if (access (sys_charge_now, F_OK | R_OK)) > - goto ackphft; /* Look down */ > + sp->charge_units[3] = CHGUNITS_uAH; > + sp->sys_charge_full[3] = g_strconcat (sysdir, "/charge_full_design", > NULL); > + sp->sys_charge_now[3] = g_strconcat (sysdir, "/charge_now", NULL); > > -done: > - sp = g_new0 (syspower, 1); > - sp->type = PWRTYPE_BATTERY; > - sp->id = g_pwr_id++; > - sp->charge_units = units; > - sp->sysdir = g_strdup (sysdir); > - sp->sys_charge_full = sys_charge_full; > - sp->sys_charge_now = sys_charge_now; > + sp->charge_units[4] = CHGUNITS_PERCENT; > + sp->sys_charge_full[4] = g_strconcat (sysdir, "/capacity_full", NULL); > + sp->sys_charge_now[4] = g_strconcat (sysdir, "/capacity_now", NULL); > > /* Battery power sources are appended to the end of the list. */ > g_sysfs_power_list = g_list_append (g_sysfs_power_list, sp); > - if (_GK.debug_level & DEBUG_BATTERY) > - g_debug ("setup_sysfs_battery: %s, %s\n", > - sys_charge_full, sys_charge_now); > - retval = TRUE; > > - if (0) > - { > -ackphft: > - if (sys_charge_full) g_free (sys_charge_full); > - if (sys_charge_now) g_free (sys_charge_now); > - } > - return retval; > + return TRUE; > } > > static gboolean > -- > 1.7.9.1 > > > -- Sandro Tosi (aka morph, morpheus, matrixhasu) My website: http://matrixhasu.altervista.org/ Me at Debian: http://wiki.debian.org/SandroTosi -- To UNSUBSCRIBE, email to debian-bugs-dist-requ...@lists.debian.org with a subject of "unsubscribe". Trouble? Contact listmas...@lists.debian.org
