Patchwork lib: fwts_battery: added interface for cycle count. acpi: battery: added cycle count check for acpi batteries.

login
register
mail settings
Submitter Alex Hung
Date May 15, 2012, 9:40 a.m.
Message ID <1337074834-5573-1-git-send-email-alex.hung@canonical.com>
Download mbox | patch
Permalink /patch/159280/
State Rejected
Headers show

Comments

Alex Hung - May 15, 2012, 9:40 a.m.
Signed-off-by: Alex Hung <alex.hung@canonical.com>
---
 src/acpi/battery/battery.c     |   18 +++++++
 src/lib/include/fwts_battery.h |    1 +
 src/lib/src/fwts_battery.c     |  111 ++++++++++++++++++++++++++++++++++++++++
 3 files changed, 130 insertions(+)
Colin King - May 15, 2012, 12:06 p.m.
On 15/05/12 10:40, Alex Hung wrote:
> Signed-off-by: Alex Hung<alex.hung@canonical.com>
> ---
>   src/acpi/battery/battery.c     |   18 +++++++
>   src/lib/include/fwts_battery.h |    1 +
>   src/lib/src/fwts_battery.c     |  111 ++++++++++++++++++++++++++++++++++++++++
>   3 files changed, 130 insertions(+)
>
> diff --git a/src/acpi/battery/battery.c b/src/acpi/battery/battery.c
> index 660d89b..d9e4d5e 100644
> --- a/src/acpi/battery/battery.c
> +++ b/src/acpi/battery/battery.c
> @@ -170,6 +170,23 @@ static void check_discharging(fwts_framework *fw, int index, char *name)
>   		name);
>   }
>
> +static void check_battery_cycle_count(fwts_framework *fw, int index, char *name)
> +{
> +	int cycle_count;
> +
> +	fwts_printf(fw, "==== Checking cycle count of battery '%s' ====\n", name);
> +	if (fwts_battery_get_cycle_count(fw, index,&cycle_count) == FWTS_OK) {
> +		if (cycle_count == 0) {
> +			fwts_log_info(fw,
> +				"Please ignore this error with a new battery");
> +			fwts_failed(fw, LOG_LEVEL_LOW, "BatteryZeroCycleCount",
> +			"System firmware may not support cycle count interface "
> +			"or it reports it incorrectly for battery %s.",
> +			name);
> +		}
> +	}
> +
> +}
>
>   static void do_battery_test(fwts_framework *fw, int index)
>   {
> @@ -193,6 +210,7 @@ static void do_battery_test(fwts_framework *fw, int index)
>   	fwts_printf(fw, "==== Please now PLUG IN the AC power of the machine ====\n");
>   	wait_for_acpi_event(fw, name);
>   	check_charging(fw, index, name);
> +	check_battery_cycle_count(fw, index, name);
>   }
>
>   static int battery_test1(fwts_framework *fw)
> diff --git a/src/lib/include/fwts_battery.h b/src/lib/include/fwts_battery.h
> index 591c4bb..83d381b 100644
> --- a/src/lib/include/fwts_battery.h
> +++ b/src/lib/include/fwts_battery.h
> @@ -26,6 +26,7 @@
>   #define FWTS_BATTERY_ALL		(-1)
>
>   int fwts_battery_get_count(fwts_framework *fw, int *count);
> +int fwts_battery_get_cycle_count(fwts_framework *fw, int index, int *cycle_count);
>   int fwts_battery_get_capacity(fwts_framework *fw, int type, int index, uint32_t *capacity_mAh, uint32_t *capacity_mWh);
>   int fwts_battery_get_name(fwts_framework *fw, int index, char *name);
>
> diff --git a/src/lib/src/fwts_battery.c b/src/lib/src/fwts_battery.c
> index 008951d..5cc5ca7 100644
> --- a/src/lib/src/fwts_battery.c
> +++ b/src/lib/src/fwts_battery.c
> @@ -261,6 +261,117 @@ static int fwts_battery_get_name_proc_fs(fwts_framework *fw, DIR *dir, int index
>   	return FWTS_ERROR;
>   }
>
> +static int fwts_battery_get_cycle_count_sys_fs(fwts_framework *fw, DIR *dir, int index, int *cycle_count)
> +{
> +	struct dirent *entry;
> +	char *field_cycle_count;
> +	size_t field_cycle_count_len;
> +	int  i = 0;
> +
> +	field_cycle_count = "POWER_SUPPLY_CYCLE_COUNT=";
> +	field_cycle_count_len = strlen(field_cycle_count);
> +
> +	do {
> +		entry = readdir(dir);
> +		if (entry&&  strlen(entry->d_name)>  2) {
> +			char path[PATH_MAX];
> +			char *data;
> +			int  val;
> +			FILE *fp;
> +			bool match;
> +
> +			/* Check that type field matches the expected type */
> +			snprintf(path, sizeof(path), "%s/%s/type", FWTS_SYS_CLASS_POWER_SUPPLY, entry->d_name);
> +			if ((data = fwts_get(path)) != NULL) {
> +				bool mismatch = (strstr(data, "Battery") == NULL);
> +				free(data);
> +				if (mismatch)
> +					continue;	/* type don't match, skip this entry */
> +			} else
> +				continue;		/* can't check type, skip this entry */
> +			match = ((index == FWTS_BATTERY_ALL) || (index == i));
> +			i++;
> +			if (!match)
> +				continue;
> +	
> +			snprintf(path, sizeof(path), "%s/%s/uevent", FWTS_SYS_CLASS_POWER_SUPPLY, entry->d_name);
> +			if ((fp = fopen(path, "r")) == NULL) {
> +				fwts_log_info(fw, "Battery %s present but undersupported - no state present.", entry->d_name);
> +			} else {
> +				char buffer[4096];
> +				while (fgets(buffer, sizeof(buffer)-1, fp) != NULL) {
> +					if (strstr(buffer, field_cycle_count)&&
> +					    strlen(buffer)>  field_cycle_count_len) {
> +						sscanf(buffer+field_cycle_count_len, "%d",&val);
> +						*cycle_count = val;
> +					}
> +				}
> +			}
> +		}
> +	} while (entry);
> +
> +	return FWTS_OK;
> +}
> +
> +static int fwts_battery_get_cycle_count_proc_fs(fwts_framework *fw, DIR *dir, int index, int *cycle_count)
> +{
> +	struct dirent *entry;
> +	char *file;
> +	char *field;
> +	int  i = 0;
> +
> +	file = "info";
> +	field = "cycle count";
> +
> +	do {
> +		entry = readdir(dir);
> +		if (entry&&  strlen(entry->d_name)>  2) {
> +			char path[PATH_MAX];
> +			int  val;
> +			FILE *fp;
> +			bool match = ((index == FWTS_BATTERY_ALL) || (index == i));
> +
> +			i++;
> +			if (!match)
> +				continue;
> +
> +			snprintf(path, sizeof(path), "%s/%s/%s", FWTS_PROC_ACPI_BATTERY, entry->d_name, file);
> +			if ((fp = fopen(path, "r")) == NULL) {
> +				fwts_log_info(fw, "Battery %s present but undersupported - no state present.", entry->d_name);
> +			} else {
> +				char buffer[4096];
> +				while (fgets(buffer, sizeof(buffer)-1, fp) != NULL) {
> +					if (strstr(buffer, field)&&
> +					    strlen(buffer)>  25) {
> +						sscanf(buffer+25, "%d",&val);
> +						*cycle_count = val;
> +						break;
> +					}
> +				}
> +			}
> +		}
> +	} while (entry);
> +	return FWTS_OK;
> +}
> +
> +int fwts_battery_get_cycle_count(fwts_framework *fw, int index, int *cycle_count)
> +{
> +	int ret;
> +	DIR *dir;

Since fwts_battery_get_cycle_count_sys_fs() and 
fwts_battery_get_cycle_count_proc_fs() can return without updating 
cycle_count we should probably initialise cycle_count before calling 
just in-case it is not updated, e.g.

	*cycle_count = 0;

> +
> +	if ((dir = opendir(FWTS_SYS_CLASS_POWER_SUPPLY)) != NULL) {
> +		ret = fwts_battery_get_cycle_count_sys_fs(fw, dir, index, cycle_count);
> +		closedir(dir);
> +	} else if ((dir = opendir(FWTS_PROC_ACPI_BATTERY)) != NULL) {
> +		ret = fwts_battery_get_cycle_count_proc_fs(fw, dir, index, cycle_count);
> +		closedir(dir);
> +	} else {
> +		return FWTS_ERROR;
> +	}
> +
> +	return ret;
> +}
> +
>   int fwts_battery_get_name(fwts_framework *fw, int index, char *name)
>   {
>   	int ret;

Colin
Alex Hung - May 15, 2012, 2:04 p.m.
Hi Colin,

Thanks for spotting out the error. I will fix it.

Cheers,
Alex Hung

On 05/15/2012 08:06 PM, Colin Ian King wrote:
> On 15/05/12 10:40, Alex Hung wrote:
>> Signed-off-by: Alex Hung<alex.hung@canonical.com>
>> ---
>> src/acpi/battery/battery.c | 18 +++++++
>> src/lib/include/fwts_battery.h | 1 +
>> src/lib/src/fwts_battery.c | 111 ++++++++++++++++++++++++++++++++++++++++
>> 3 files changed, 130 insertions(+)
>>
>
> Since fwts_battery_get_cycle_count_sys_fs() and
> fwts_battery_get_cycle_count_proc_fs() can return without updating
> cycle_count we should probably initialise cycle_count before calling
> just in-case it is not updated, e.g.
>
> *cycle_count = 0;
>
>> +
>> + if ((dir = opendir(FWTS_SYS_CLASS_POWER_SUPPLY)) != NULL) {
>> + ret = fwts_battery_get_cycle_count_sys_fs(fw, dir, index, cycle_count);
>> + closedir(dir);
>> + } else if ((dir = opendir(FWTS_PROC_ACPI_BATTERY)) != NULL) {
>> + ret = fwts_battery_get_cycle_count_proc_fs(fw, dir, index,
>> cycle_count);
>> + closedir(dir);
>> + } else {
>> + return FWTS_ERROR;
>> + }
>> +
>> + return ret;
>> +}
>> +
>> int fwts_battery_get_name(fwts_framework *fw, int index, char *name)
>> {
>> int ret;
>
> Colin
>

Patch

diff --git a/src/acpi/battery/battery.c b/src/acpi/battery/battery.c
index 660d89b..d9e4d5e 100644
--- a/src/acpi/battery/battery.c
+++ b/src/acpi/battery/battery.c
@@ -170,6 +170,23 @@  static void check_discharging(fwts_framework *fw, int index, char *name)
 		name);
 }
 
+static void check_battery_cycle_count(fwts_framework *fw, int index, char *name)
+{
+	int cycle_count;
+
+	fwts_printf(fw, "==== Checking cycle count of battery '%s' ====\n", name);
+	if (fwts_battery_get_cycle_count(fw, index, &cycle_count) == FWTS_OK) {
+		if (cycle_count == 0) {
+			fwts_log_info(fw,
+				"Please ignore this error with a new battery");
+			fwts_failed(fw, LOG_LEVEL_LOW, "BatteryZeroCycleCount",
+			"System firmware may not support cycle count interface "
+			"or it reports it incorrectly for battery %s.",
+			name);
+		}
+	}
+
+}
 
 static void do_battery_test(fwts_framework *fw, int index)
 {
@@ -193,6 +210,7 @@  static void do_battery_test(fwts_framework *fw, int index)
 	fwts_printf(fw, "==== Please now PLUG IN the AC power of the machine ====\n");
 	wait_for_acpi_event(fw, name);
 	check_charging(fw, index, name);
+	check_battery_cycle_count(fw, index, name);
 }
 
 static int battery_test1(fwts_framework *fw)
diff --git a/src/lib/include/fwts_battery.h b/src/lib/include/fwts_battery.h
index 591c4bb..83d381b 100644
--- a/src/lib/include/fwts_battery.h
+++ b/src/lib/include/fwts_battery.h
@@ -26,6 +26,7 @@ 
 #define FWTS_BATTERY_ALL		(-1)
 
 int fwts_battery_get_count(fwts_framework *fw, int *count);
+int fwts_battery_get_cycle_count(fwts_framework *fw, int index, int *cycle_count);
 int fwts_battery_get_capacity(fwts_framework *fw, int type, int index, uint32_t *capacity_mAh, uint32_t *capacity_mWh);
 int fwts_battery_get_name(fwts_framework *fw, int index, char *name);
 
diff --git a/src/lib/src/fwts_battery.c b/src/lib/src/fwts_battery.c
index 008951d..5cc5ca7 100644
--- a/src/lib/src/fwts_battery.c
+++ b/src/lib/src/fwts_battery.c
@@ -261,6 +261,117 @@  static int fwts_battery_get_name_proc_fs(fwts_framework *fw, DIR *dir, int index
 	return FWTS_ERROR;
 }
 
+static int fwts_battery_get_cycle_count_sys_fs(fwts_framework *fw, DIR *dir, int index, int *cycle_count)
+{
+	struct dirent *entry;
+	char *field_cycle_count;
+	size_t field_cycle_count_len;
+	int  i = 0;
+
+	field_cycle_count = "POWER_SUPPLY_CYCLE_COUNT=";
+	field_cycle_count_len = strlen(field_cycle_count);
+
+	do {
+		entry = readdir(dir);
+		if (entry && strlen(entry->d_name) > 2) {
+			char path[PATH_MAX];
+			char *data;
+			int  val;
+			FILE *fp;
+			bool match;
+
+			/* Check that type field matches the expected type */
+			snprintf(path, sizeof(path), "%s/%s/type", FWTS_SYS_CLASS_POWER_SUPPLY, entry->d_name);
+			if ((data = fwts_get(path)) != NULL) {
+				bool mismatch = (strstr(data, "Battery") == NULL);
+				free(data);
+				if (mismatch)
+					continue;	/* type don't match, skip this entry */
+			} else
+				continue;		/* can't check type, skip this entry */
+			match = ((index == FWTS_BATTERY_ALL) || (index == i));
+			i++;
+			if (!match)
+				continue;
+	
+			snprintf(path, sizeof(path), "%s/%s/uevent", FWTS_SYS_CLASS_POWER_SUPPLY, entry->d_name);
+			if ((fp = fopen(path, "r")) == NULL) {
+				fwts_log_info(fw, "Battery %s present but undersupported - no state present.", entry->d_name);
+			} else {
+				char buffer[4096];
+				while (fgets(buffer, sizeof(buffer)-1, fp) != NULL) {
+					if (strstr(buffer, field_cycle_count) &&
+					    strlen(buffer) > field_cycle_count_len) {
+						sscanf(buffer+field_cycle_count_len, "%d", &val);
+						*cycle_count = val;
+					}
+				}
+			}
+		}
+	} while (entry);
+
+	return FWTS_OK;
+}
+
+static int fwts_battery_get_cycle_count_proc_fs(fwts_framework *fw, DIR *dir, int index, int *cycle_count)
+{
+	struct dirent *entry;
+	char *file;
+	char *field;
+	int  i = 0;
+
+	file = "info";
+	field = "cycle count";
+
+	do {
+		entry = readdir(dir);
+		if (entry && strlen(entry->d_name) > 2) {
+			char path[PATH_MAX];
+			int  val;
+			FILE *fp;
+			bool match = ((index == FWTS_BATTERY_ALL) || (index == i));
+
+			i++;
+			if (!match)
+				continue;
+
+			snprintf(path, sizeof(path), "%s/%s/%s", FWTS_PROC_ACPI_BATTERY, entry->d_name, file);
+			if ((fp = fopen(path, "r")) == NULL) {
+				fwts_log_info(fw, "Battery %s present but undersupported - no state present.", entry->d_name);
+			} else {
+				char buffer[4096];
+				while (fgets(buffer, sizeof(buffer)-1, fp) != NULL) {
+					if (strstr(buffer, field) &&
+					    strlen(buffer) > 25) {
+						sscanf(buffer+25, "%d", &val);
+						*cycle_count = val;
+						break;
+					}
+				}
+			}
+		}
+	} while (entry);
+	return FWTS_OK;
+}
+
+int fwts_battery_get_cycle_count(fwts_framework *fw, int index, int *cycle_count)
+{
+	int ret;
+	DIR *dir;
+
+	if ((dir = opendir(FWTS_SYS_CLASS_POWER_SUPPLY)) != NULL) {
+		ret = fwts_battery_get_cycle_count_sys_fs(fw, dir, index, cycle_count);
+		closedir(dir);
+	} else if ((dir = opendir(FWTS_PROC_ACPI_BATTERY)) != NULL) {
+		ret = fwts_battery_get_cycle_count_proc_fs(fw, dir, index, cycle_count);
+		closedir(dir);
+	} else {
+		return FWTS_ERROR;
+	}
+
+	return ret;
+}
+
 int fwts_battery_get_name(fwts_framework *fw, int index, char *name)
 {
 	int ret;