diff mbox series

[v5,matchbox-panel-2] panel: added sysfs support

Message ID 20250716031705.110773-1-dixitparmar19@gmail.com
State New
Headers show
Series [v5,matchbox-panel-2] panel: added sysfs support | expand

Commit Message

Dixit Parmar July 16, 2025, 3:17 a.m. UTC
battery applet supports reading battery and supply
information from sysfs entries, /sys/class/power_supply/.

[YOCTO #12904]
https://bugzilla.yoctoproject.org/show_bug.cgi?id=12904

Signed-off-by: Dixit Parmar <dixitparmar19@gmail.com>
---
V4: https://lists.yoctoproject.org/g/yocto-patches/message/789
---
 applets/battery/Makefile.am     |   4 +
 applets/battery/battery-acpi.c  |   2 +
 applets/battery/battery-apm.c   |   2 +
 applets/battery/battery-sysfs.c | 148 ++++++++++++++++++++++++++++++++
 applets/battery/battery.c       |   2 +
 applets/battery/battery.h       |   3 +-
 configure.ac                    |   4 +
 7 files changed, 163 insertions(+), 2 deletions(-)
 create mode 100644 applets/battery/battery-sysfs.c

Comments

Dixit Parmar Aug. 11, 2025, 5:34 a.m. UTC | #1
Hi Ross and all,

As it has been almost a month since this new patch version submission, I would like to send a gentle reminder to bring this patch to the top of your review list. It's of course not urgent, but an ACK to confirm if it looks good or not would be highly appreciated.

Thanks,
Dixit
Gyorgy Sarvari Aug. 11, 2025, 8 a.m. UTC | #2
Though I'm not Ross, added some notes inline. I gotta admit that haven't
read the reviews for v1-v3, only for v4: if there is something that was
discussed beforehand, and I'm just repeating it, just ignore that part.

On 7/16/25 05:17, Dixit Parmar via lists.yoctoproject.org wrote:
> battery applet supports reading battery and supply
> information from sysfs entries, /sys/class/power_supply/.
>
> [YOCTO #12904]
> https://bugzilla.yoctoproject.org/show_bug.cgi?id=12904
>
> Signed-off-by: Dixit Parmar <dixitparmar19@gmail.com>
> ---
> V4: https://lists.yoctoproject.org/g/yocto-patches/message/789
> ---
>  applets/battery/Makefile.am     |   4 +
>  applets/battery/battery-acpi.c  |   2 +
>  applets/battery/battery-apm.c   |   2 +
>  applets/battery/battery-sysfs.c | 148 ++++++++++++++++++++++++++++++++
>  applets/battery/battery.c       |   2 +
>  applets/battery/battery.h       |   3 +-
>  configure.ac                    |   4 +
>  7 files changed, 163 insertions(+), 2 deletions(-)
>  create mode 100644 applets/battery/battery-sysfs.c
>
> diff --git a/applets/battery/Makefile.am b/applets/battery/Makefile.am
> index 9b02240..1b93a0b 100644
> --- a/applets/battery/Makefile.am
> +++ b/applets/battery/Makefile.am
> @@ -5,6 +5,10 @@ applet_LTLIBRARIES = libbattery.la
>  libbattery_la_SOURCES = battery.c battery.h
>  libbattery_la_CPPFLAGS = $(AM_CPPFLAGS) -DDATADIR=\"$(pkgdatadir)/battery/\"
>  
> +if HAVE_SYSFS
> +libbattery_la_SOURCES += battery-sysfs.c
> +endif
> +
>  if HAVE_LIBAPM
>  libbattery_la_LIBADD = -lapm
>  libbattery_la_SOURCES += battery-apm.c
> diff --git a/applets/battery/battery-acpi.c b/applets/battery/battery-acpi.c
> index d1d3de9..42ef9c8 100644
> --- a/applets/battery/battery-acpi.c
> +++ b/applets/battery/battery-acpi.c
> @@ -14,6 +14,8 @@ global_t global;
>  adapter_t *ac;
>  int batt_state, ac_state;
>  
> +void pm_cleanup(void) { }
> +
>  int pm_support(void)
>  {
>  	if(check_acpi_support() == NOT_SUPPORTED){
> diff --git a/applets/battery/battery-apm.c b/applets/battery/battery-apm.c
> index edaba43..f1501a0 100644
> --- a/applets/battery/battery-apm.c
> +++ b/applets/battery/battery-apm.c
> @@ -9,6 +9,8 @@
>  #include "battery.h"
>  #include <apm.h>
>  
> +void pm_cleanup(void) { }
> +
>  int pm_support(void)
>  {
>  	if (1 == apm_exists ()) {
> diff --git a/applets/battery/battery-sysfs.c b/applets/battery/battery-sysfs.c
> new file mode 100644
> index 0000000..acb01b3
> --- /dev/null
> +++ b/applets/battery/battery-sysfs.c
> @@ -0,0 +1,148 @@
> +/*
> + * (C) Dixit Parmar
> + *
> + * SPDX-License-Identifier: GPL-2.0-or-later
> + *
> + * Licensed under the GPL v2 or greater.
> + */
> +
> +#include "battery.h"
> +#include <glib.h>
> +
> +#define PROCFS_PS_DIR "/sys/class/power_supply"
> +
> +static gchar *batt_dev;
> +static gchar *ac_dev;
> +static guchar batt_exist = 0;
> +static guchar ac_exist = 0;

I'm not very well versed in gtk, so if anyone tells me that it is
idiomatic, I will believe it, but: why are these guchar? The code
doesn't seem to do anything char-like with them, they are used as bools.
Why not guint, or gboolean?

> +
> +void pm_cleanup(void)
> +{
> +        g_free(batt_dev);
> +        g_free(ac_dev);
> +}
> +
> +int pm_support(void)
> +{
> +	GDir *dir = g_dir_open(PROCFS_PS_DIR, 0, NULL);
> +	if (dir != NULL) {
> +		const gchar *subdir_name;
> +		while ((subdir_name = g_dir_read_name(dir)) != NULL) {
> +			gchar *subdir_path = g_build_filename(PROCFS_PS_DIR, subdir_name, NULL);
> +			// Check if it's a directory
> +			if (g_file_test(subdir_path, G_FILE_TEST_IS_DIR)) {
> +				gchar *type_file_path = g_build_filename(subdir_path, "type", NULL);
> +				// Check if the "type" file exists
> +				if (g_file_test(type_file_path, G_FILE_TEST_EXISTS)) {
> +					gchar *file_content = NULL;
> +					gsize length;
> +
> +					if (g_file_get_contents(type_file_path, &file_content, &length, NULL)) {
> +						if(!batt_exist && strncmp(file_content, "Battery", strlen("Battery")) == 0) {
> +							batt_dev = g_malloc0(strlen(subdir_name) + 1);
> +                                                        strncpy(batt_dev, subdir_name, strlen(subdir_name));
> +							batt_exist = 1;
> +						}
> +						if(!ac_exist && strncmp(file_content, "Mains", strlen("Mains")) == 0) {
> +							ac_dev = g_malloc0(strlen(subdir_name) + 1);
> +							strncpy(ac_dev, subdir_name, strlen(subdir_name));
> +							ac_exist = 1;
> +						}
> +						g_free(file_content);
> +					}
> +				}
> +				g_free(type_file_path);
> +			}
> +			g_free(subdir_path);
> +			if(batt_exist && ac_exist)
> +				break;
> +		}
> +		g_dir_close(dir);
> +	}
> +	return (int)batt_exist;
> +}
> +
> +static gboolean read_val_from_file(const gchar *dev, const gchar *f, gint64 *val)
> +{
> +        gboolean ret = TRUE;

Should this be not FALSE by default? If the next "if" branch is false,
it stays TRUE, even though it fails to read anything.

> +	gchar *endptr;
> +
> +	if(dev != NULL && f != NULL && val != NULL) {
> +		gchar *file_path = g_build_filename(PROCFS_PS_DIR, dev, f, NULL);
> +                gchar *file_content;
> +                gsize length;
> +
> +                if(g_file_get_contents(file_path, &file_content, &length, NULL)) {
> +                       *val = g_ascii_strtoll(file_content, &endptr, 10);
> +                } else {
> +                    ret = FALSE;
> +                }
> +
> +                g_free(file_content);
> +		g_free(file_path);
> +		return ret;

This return above seems to be superfluous in view of the next statement.

> +	}
> +	return ret;
> +}
> +
> +const char* pm_battery_icon(void)
> +{
> +	const char *icon;
> +
> +        gint64 temp_val = 0;

Space and tabs are mixed here. There are some other instances of this in
this patch.

> +	gint64 ac_sts = 0;

Nitpick: batt_present and batt_percentage are nicely spelled out. Could
this be called also ac_status?

> +	gint64 batt_present = 0;
> +	gint64 batt_percentage = 0;
> +
> +	if(batt_exist == 0)
> +		return NULL;
> +
> +	/* Check battery presence */
> +	if(read_val_from_file(batt_dev, "present", &temp_val))
> +		batt_present = temp_val;

Nitpick: I wonder if checking for "present" is the best way to determine
if a battery is present. This file might not exist in all drivers, in
which case the battery is considered to be present (according to sysfs
docs). On the other hand, as a first version I consider it good enough,
it can be iterated, when/if needed.

> +
> +	if(batt_present == 0) {
> +		/* Battery is removed */
> +		icon = "ac-adapter.png";
> +		return icon;
> +	}
> +
> +	/* Check Adaptor status*/
> +	if(read_val_from_file(ac_dev, "online", &temp_val))
> +		ac_sts = temp_val;
> +

ac_exist isn't verified before trying to read. While I believe it
shouldn't lead to anything bad, but it is checked later when the icon is
displayed.

> +	/* Check battery percentage */
> +	if(read_val_from_file(batt_dev, "capacity", &temp_val))
> +		batt_percentage = temp_val;
> +
> +	if(ac_exist && ac_sts == 1) {
> +		/* We're charging */
> +		if (batt_percentage < 10)
> +			icon = "battery-charging-000.png";
> +		else if (batt_percentage < 30)
> +			icon = "battery-charging-020.png";
> +		else if (batt_percentage < 50)
> +			icon = "battery-charging-040.png";
> +		else if (batt_percentage < 70)
> +			icon = "battery-charging-060.png";
> +		else if (batt_percentage < 90)
> +			icon = "battery-charging-080.png";
> +		else
> +			icon = "battery-charging-100.png";
> +	} else {
> +		if (batt_percentage < 10)
> +			icon = "battery-discharging-000.png";
> +		else if (batt_percentage < 30)
> +			icon = "battery-discharging-020.png";
> +		else if (batt_percentage < 50)
> +			icon = "battery-discharging-040.png";
> +		else if (batt_percentage < 70)
> +			icon = "battery-discharging-060.png";
> +		else if (batt_percentage < 90)
> +			icon = "battery-discharging-080.png";
> +		else
> +			icon = "battery-discharging-100.png";
> +	}
> +
> +	return icon;
> +}
> diff --git a/applets/battery/battery.c b/applets/battery/battery.c
> index b11517d..755042b 100644
> --- a/applets/battery/battery.c
> +++ b/applets/battery/battery.c
> @@ -24,6 +24,8 @@ battery_applet_free (BatteryApplet *applet)
>          g_source_remove (applet->timeout_id);
>  
>          g_slice_free (BatteryApplet, applet);
> +
> +        pm_cleanup ();
>  }
>  
>  static gboolean
> diff --git a/applets/battery/battery.h b/applets/battery/battery.h
> index 534988f..7c54a3a 100644
> --- a/applets/battery/battery.h
> +++ b/applets/battery/battery.h
> @@ -9,8 +9,7 @@
>  #ifndef MB_APPLET_BATTERY_H
>  #define MB_APPLET_BATTERY_H
>  
> -#include <string.h>
> -
> +void pm_cleanup(void);
>  int pm_support(void);
>  const char* pm_battery_icon(void);
>  
> diff --git a/configure.ac b/configure.ac
> index be4d591..0f607f8 100644
> --- a/configure.ac
> +++ b/configure.ac
> @@ -61,6 +61,9 @@ AC_ARG_WITH(
>  )
>  
>  case "$with_battery" in
> +     "sysfs") enable_linux_sysfs=yes
> +     enable_battery=yes
> +     ;;
>       "acpi") AC_CHECK_HEADERS(libacpi.h, enable_linux_acpi=yes, AC_MSG_FAILURE([You need to install libacpi]))
>       enable_battery=yes
>       ;;
> @@ -75,6 +78,7 @@ esac
>  AM_CONDITIONAL(HAVE_BATTERY, test x$enable_battery = xyes)
>  AM_CONDITIONAL(HAVE_LIBAPM, test x$enable_linux_apm = xyes)
>  AM_CONDITIONAL(HAVE_LIBACPI, test x$enable_linux_acpi = xyes)
> +AM_CONDITIONAL(HAVE_SYSFS, test x$enable_linux_sysfs = xyes)
>  
>  # glib-genmarshal
>  GLIB_GENMARSHAL=`$PKG_CONFIG --variable=glib_genmarshal glib-2.0`
Alexander Kanavin Aug. 11, 2025, 8:08 a.m. UTC | #3
Ross is on holiday now. This work is massively appreciated, I'll make
sure to poke him if needed :-)

Alex

On Mon, 11 Aug 2025 at 07:34, Dixit Parmar via lists.yoctoproject.org
<dixitparmar19=gmail.com@lists.yoctoproject.org> wrote:
>
> Hi Ross and all,
>
> As it has been almost a month since this new patch version submission, I would like to send a gentle reminder to bring this patch to the top of your review list. It's of course not urgent, but an ACK to confirm if it looks good or not would be highly appreciated.
>
> Thanks,
> Dixit
>
> _._,_._,_
> ________________________________
> Links:
>
> You receive all messages sent to this group.
>
> View/Reply Online (#1924) | Reply to Group | Reply to Sender | Mute This Topic | New Topic
> Your Subscription | Contact Group Owner | Unsubscribe [alex.kanavin@gmail.com]
>
> _._,_._,_
Gyorgy Sarvari Aug. 11, 2025, 8:46 a.m. UTC | #4
One more note, I'm sorry sorry for forgetting this from the previous mail.

On 7/16/25 05:17, Dixit Parmar via lists.yoctoproject.org wrote:
> +
> +static gboolean read_val_from_file(const gchar *dev, const gchar *f, gint64 *val)
> +{
> +        gboolean ret = TRUE;
> +	gchar *endptr;
> +

The endptr variable is not used at all, I think it can be removed.
According to the docs g_ascii_strtoll accepts NULL also instead of a
pointer as the second argument.

> +	if(dev != NULL && f != NULL && val != NULL) {
> +		gchar *file_path = g_build_filename(PROCFS_PS_DIR, dev, f, NULL);
> +                gchar *file_content;
> +                gsize length;
> +
> +                if(g_file_get_contents(file_path, &file_content, &length, NULL)) {
> +                       *val = g_ascii_strtoll(file_content, &endptr, 10);
> +                } else {
> +                    ret = FALSE;
> +                }
> +
> +                g_free(file_content);
> +		g_free(file_path);
> +		return ret;
> +	}
> +	return ret;
> +}
>
Dixit Parmar Aug. 13, 2025, 3:29 a.m. UTC | #5
> 
> I'm not very well versed in gtk, so if anyone tells me that it is
> idiomatic, I will believe it, but: why are these guchar? The code
> doesn't seem to do anything char-like with them, they are used as bools.
> Why not guint, or gboolean?

Ideally it should be gboolean, I dont recall why I kept it guchar. I will shift to gboolean and see.

> 
> 
>> +static gboolean read_val_from_file(const gchar *dev, const gchar *f,
>> gint64 *val)
>> +{
>> + gboolean ret = TRUE;
> 
> Should this be not FALSE by default? If the next "if" branch is false,
> it stays TRUE, even though it fails to read anything.

Its rare that we'll land to that condition but yeah theoretically it should be false. Will revise this function.

> 
> 
>> + gchar *endptr;
>> +
>> + if(dev != NULL && f != NULL && val != NULL) {
>> + gchar *file_path = g_build_filename(PROCFS_PS_DIR, dev, f, NULL);
>> + gchar *file_content;
>> + gsize length;
>> +
>> + if(g_file_get_contents(file_path, &file_content, &length, NULL)) {
>> + *val = g_ascii_strtoll(file_content, &endptr, 10);
>> + } else {
>> + ret = FALSE;
>> + }
>> +
>> + g_free(file_content);
>> + g_free(file_path);
>> + return ret;
> 
> This return above seems to be superfluous in view of the next statement.
> 
> 
>> + }
>> + return ret;
>> +}
>> +
>> +const char* pm_battery_icon(void)
>> +{
>> + const char *icon;
>> +
>> + gint64 temp_val = 0;
> 
> Space and tabs are mixed here. There are some other instances of this in
> this patch.

Yeah, I noticed that once I submitted, I wonder why I am not seeing this garbage locally in vim. Need to check.

> 
> 
>> + gint64 ac_sts = 0;
> 
> Nitpick: batt_present and batt_percentage are nicely spelled out. Could
> this be called also ac_status?

True, will make it uniform.

> 
> 
>> + gint64 batt_present = 0;
>> + gint64 batt_percentage = 0;
>> +
>> + if(batt_exist == 0)
>> + return NULL;
>> +
>> + /* Check battery presence */
>> + if(read_val_from_file(batt_dev, "present", &temp_val))
>> + batt_present = temp_val;
> 
> Nitpick: I wonder if checking for "present" is the best way to determine
> if a battery is present. This file might not exist in all drivers, in
> which case the battery is considered to be present (according to sysfs
> docs). On the other hand, as a first version I consider it good enough,
> it can be iterated, when/if needed.

In the whatever drivers I have seen so far while implementing this, present is the base entry for any battery device
in power subsystem.

> 
> 
>> +
>> + if(batt_present == 0) {
>> + /* Battery is removed */
>> + icon = "ac-adapter.png";
>> + return icon;
>> + }
>> +
>> + /* Check Adaptor status*/
>> + if(read_val_from_file(ac_dev, "online", &temp_val))
>> + ac_sts = temp_val;
>> +
> 
> ac_exist isn't verified before trying to read. While I believe it
> shouldn't lead to anything bad, but it is checked later when the icon is
> displayed.

Will add the check.

Thanks for the review,
Dixit
Dixit Parmar Aug. 13, 2025, 3:31 a.m. UTC | #6
On Mon, Aug 11, 2025 at 01:39 PM, Alexander Kanavin wrote:

> 
> Ross is on holiday now. This work is massively appreciated, I'll make
> sure to poke him if needed :-)
> 
> Alex

Hi Alex,
No please, let him enjoy. Anyway I got some suggestions to go for v6. Thanks

> 
> On Mon, 11 Aug 2025 at 07:34, Dixit Parmar via lists.yoctoproject.org
> <dixitparmar19=gmail.com@lists.yoctoproject.org> wrote:
> 
>> Hi Ross and all,
>> 
>> As it has been almost a month since this new patch version submission, I
>> would like to send a gentle reminder to bring this patch to the top of
>> your review list. It's of course not urgent, but an ACK to confirm if it
>> looks good or not would be highly appreciated.
>> 
>> Thanks,
>> Dixit
> 
>
Dixit Parmar Aug. 13, 2025, 3:32 a.m. UTC | #7
On Mon, Aug 11, 2025 at 02:16 PM, Gyorgy Sarvari wrote:

> 
> One more note, I'm sorry sorry for forgetting this from the previous mail.
> 
> 
> On 7/16/25 05:17, Dixit Parmar via lists.yoctoproject.org wrote:
> 
>> +
>> +static gboolean read_val_from_file(const gchar *dev, const gchar *f,
>> gint64 *val)
>> +{
>> + gboolean ret = TRUE;
>> + gchar *endptr;
>> +
> 
> The endptr variable is not used at all, I think it can be removed.
> According to the docs g_ascii_strtoll accepts NULL also instead of a
> pointer as the second argument.

Noted.
diff mbox series

Patch

diff --git a/applets/battery/Makefile.am b/applets/battery/Makefile.am
index 9b02240..1b93a0b 100644
--- a/applets/battery/Makefile.am
+++ b/applets/battery/Makefile.am
@@ -5,6 +5,10 @@  applet_LTLIBRARIES = libbattery.la
 libbattery_la_SOURCES = battery.c battery.h
 libbattery_la_CPPFLAGS = $(AM_CPPFLAGS) -DDATADIR=\"$(pkgdatadir)/battery/\"
 
+if HAVE_SYSFS
+libbattery_la_SOURCES += battery-sysfs.c
+endif
+
 if HAVE_LIBAPM
 libbattery_la_LIBADD = -lapm
 libbattery_la_SOURCES += battery-apm.c
diff --git a/applets/battery/battery-acpi.c b/applets/battery/battery-acpi.c
index d1d3de9..42ef9c8 100644
--- a/applets/battery/battery-acpi.c
+++ b/applets/battery/battery-acpi.c
@@ -14,6 +14,8 @@  global_t global;
 adapter_t *ac;
 int batt_state, ac_state;
 
+void pm_cleanup(void) { }
+
 int pm_support(void)
 {
 	if(check_acpi_support() == NOT_SUPPORTED){
diff --git a/applets/battery/battery-apm.c b/applets/battery/battery-apm.c
index edaba43..f1501a0 100644
--- a/applets/battery/battery-apm.c
+++ b/applets/battery/battery-apm.c
@@ -9,6 +9,8 @@ 
 #include "battery.h"
 #include <apm.h>
 
+void pm_cleanup(void) { }
+
 int pm_support(void)
 {
 	if (1 == apm_exists ()) {
diff --git a/applets/battery/battery-sysfs.c b/applets/battery/battery-sysfs.c
new file mode 100644
index 0000000..acb01b3
--- /dev/null
+++ b/applets/battery/battery-sysfs.c
@@ -0,0 +1,148 @@ 
+/*
+ * (C) Dixit Parmar
+ *
+ * SPDX-License-Identifier: GPL-2.0-or-later
+ *
+ * Licensed under the GPL v2 or greater.
+ */
+
+#include "battery.h"
+#include <glib.h>
+
+#define PROCFS_PS_DIR "/sys/class/power_supply"
+
+static gchar *batt_dev;
+static gchar *ac_dev;
+static guchar batt_exist = 0;
+static guchar ac_exist = 0;
+
+void pm_cleanup(void)
+{
+        g_free(batt_dev);
+        g_free(ac_dev);
+}
+
+int pm_support(void)
+{
+	GDir *dir = g_dir_open(PROCFS_PS_DIR, 0, NULL);
+	if (dir != NULL) {
+		const gchar *subdir_name;
+		while ((subdir_name = g_dir_read_name(dir)) != NULL) {
+			gchar *subdir_path = g_build_filename(PROCFS_PS_DIR, subdir_name, NULL);
+			// Check if it's a directory
+			if (g_file_test(subdir_path, G_FILE_TEST_IS_DIR)) {
+				gchar *type_file_path = g_build_filename(subdir_path, "type", NULL);
+				// Check if the "type" file exists
+				if (g_file_test(type_file_path, G_FILE_TEST_EXISTS)) {
+					gchar *file_content = NULL;
+					gsize length;
+
+					if (g_file_get_contents(type_file_path, &file_content, &length, NULL)) {
+						if(!batt_exist && strncmp(file_content, "Battery", strlen("Battery")) == 0) {
+							batt_dev = g_malloc0(strlen(subdir_name) + 1);
+                                                        strncpy(batt_dev, subdir_name, strlen(subdir_name));
+							batt_exist = 1;
+						}
+						if(!ac_exist && strncmp(file_content, "Mains", strlen("Mains")) == 0) {
+							ac_dev = g_malloc0(strlen(subdir_name) + 1);
+							strncpy(ac_dev, subdir_name, strlen(subdir_name));
+							ac_exist = 1;
+						}
+						g_free(file_content);
+					}
+				}
+				g_free(type_file_path);
+			}
+			g_free(subdir_path);
+			if(batt_exist && ac_exist)
+				break;
+		}
+		g_dir_close(dir);
+	}
+	return (int)batt_exist;
+}
+
+static gboolean read_val_from_file(const gchar *dev, const gchar *f, gint64 *val)
+{
+        gboolean ret = TRUE;
+	gchar *endptr;
+
+	if(dev != NULL && f != NULL && val != NULL) {
+		gchar *file_path = g_build_filename(PROCFS_PS_DIR, dev, f, NULL);
+                gchar *file_content;
+                gsize length;
+
+                if(g_file_get_contents(file_path, &file_content, &length, NULL)) {
+                       *val = g_ascii_strtoll(file_content, &endptr, 10);
+                } else {
+                    ret = FALSE;
+                }
+
+                g_free(file_content);
+		g_free(file_path);
+		return ret;
+	}
+	return ret;
+}
+
+const char* pm_battery_icon(void)
+{
+	const char *icon;
+
+        gint64 temp_val = 0;
+	gint64 ac_sts = 0;
+	gint64 batt_present = 0;
+	gint64 batt_percentage = 0;
+
+	if(batt_exist == 0)
+		return NULL;
+
+	/* Check battery presence */
+	if(read_val_from_file(batt_dev, "present", &temp_val))
+		batt_present = temp_val;
+
+	if(batt_present == 0) {
+		/* Battery is removed */
+		icon = "ac-adapter.png";
+		return icon;
+	}
+
+	/* Check Adaptor status*/
+	if(read_val_from_file(ac_dev, "online", &temp_val))
+		ac_sts = temp_val;
+
+	/* Check battery percentage */
+	if(read_val_from_file(batt_dev, "capacity", &temp_val))
+		batt_percentage = temp_val;
+
+	if(ac_exist && ac_sts == 1) {
+		/* We're charging */
+		if (batt_percentage < 10)
+			icon = "battery-charging-000.png";
+		else if (batt_percentage < 30)
+			icon = "battery-charging-020.png";
+		else if (batt_percentage < 50)
+			icon = "battery-charging-040.png";
+		else if (batt_percentage < 70)
+			icon = "battery-charging-060.png";
+		else if (batt_percentage < 90)
+			icon = "battery-charging-080.png";
+		else
+			icon = "battery-charging-100.png";
+	} else {
+		if (batt_percentage < 10)
+			icon = "battery-discharging-000.png";
+		else if (batt_percentage < 30)
+			icon = "battery-discharging-020.png";
+		else if (batt_percentage < 50)
+			icon = "battery-discharging-040.png";
+		else if (batt_percentage < 70)
+			icon = "battery-discharging-060.png";
+		else if (batt_percentage < 90)
+			icon = "battery-discharging-080.png";
+		else
+			icon = "battery-discharging-100.png";
+	}
+
+	return icon;
+}
diff --git a/applets/battery/battery.c b/applets/battery/battery.c
index b11517d..755042b 100644
--- a/applets/battery/battery.c
+++ b/applets/battery/battery.c
@@ -24,6 +24,8 @@  battery_applet_free (BatteryApplet *applet)
         g_source_remove (applet->timeout_id);
 
         g_slice_free (BatteryApplet, applet);
+
+        pm_cleanup ();
 }
 
 static gboolean
diff --git a/applets/battery/battery.h b/applets/battery/battery.h
index 534988f..7c54a3a 100644
--- a/applets/battery/battery.h
+++ b/applets/battery/battery.h
@@ -9,8 +9,7 @@ 
 #ifndef MB_APPLET_BATTERY_H
 #define MB_APPLET_BATTERY_H
 
-#include <string.h>
-
+void pm_cleanup(void);
 int pm_support(void);
 const char* pm_battery_icon(void);
 
diff --git a/configure.ac b/configure.ac
index be4d591..0f607f8 100644
--- a/configure.ac
+++ b/configure.ac
@@ -61,6 +61,9 @@  AC_ARG_WITH(
 )
 
 case "$with_battery" in
+     "sysfs") enable_linux_sysfs=yes
+     enable_battery=yes
+     ;;
      "acpi") AC_CHECK_HEADERS(libacpi.h, enable_linux_acpi=yes, AC_MSG_FAILURE([You need to install libacpi]))
      enable_battery=yes
      ;;
@@ -75,6 +78,7 @@  esac
 AM_CONDITIONAL(HAVE_BATTERY, test x$enable_battery = xyes)
 AM_CONDITIONAL(HAVE_LIBAPM, test x$enable_linux_apm = xyes)
 AM_CONDITIONAL(HAVE_LIBACPI, test x$enable_linux_acpi = xyes)
+AM_CONDITIONAL(HAVE_SYSFS, test x$enable_linux_sysfs = xyes)
 
 # glib-genmarshal
 GLIB_GENMARSHAL=`$PKG_CONFIG --variable=glib_genmarshal glib-2.0`