: sensors: Automatically scale energy/power

PATCH In review Darrick J. Wong 2 participants 3 messages Mar 31, 2008 – Apr 14, 2008

lore

Share & tools

lore

Darrick J. Wong

author

2008-03-31 23:54 UTC #0 lore raw

PATCH 4/4 : sensors: Automatically scale energy/power

Automatically scale energy and power values when printing them in cooked mode.

Signed-off-by Darrick J. Wong <[email protected]>

Index: lm-sensors-3.0.0/lib/sysfs.c
=================================--- lm-sensors-3.0.0.orig/lib/sysfs.c 2008-03-31 16:35:30.000000000 -0700
+++ lm-sensors-3.0.0/lib/sysfs.c 2008-03-31 16:35:46.000000000 -0700
@@ -155,10 +155,6 @@
return 1000;
case SENSORS_SUBFEATURE_FAN_INPUT:
return 1;
- case SENSORS_SUBFEATURE_POWER_AVERAGE:
- return 1000000;
- case SENSORS_SUBFEATURE_ENERGY_INPUT:
- return 1000000;
case SENSORS_SUBFEATURE_POWER_AVERAGE_INTERVAL:
return 1000;
}
Index: lm-sensors-3.0.0/prog/sensors/chips.c
=================================--- lm-sensors-3.0.0.orig/prog/sensors/chips.c 2008-03-31 16:35:38.000000000 -0700
+++ lm-sensors-3.0.0/prog/sensors/chips.c 2008-03-31 16:35:46.000000000 -0700
@@ -401,10 +401,40 @@
printf("\n");
}

+struct scale_table {
+ float upper_bound;
+ const char *units;
+};
+
+static void scale_value(struct scale_table *scales, float *value,
+ const char **unitstr)
+{
+ float divisor = 1;
+
+ while (scales->upper_bound != 0 && *value > scales->upper_bound) {
+ divisor = scales->upper_bound;
+ scales++;
+ }
+
+ *value /= divisor;
+
+ *unitstr = scales->units;
+}
+
+struct scale_table power_scales[] = {
+ {1e3, "uW"},
+ {1e6, "mW"},
+ {1e9, "W"},
+ {1e12, "KW"},
+ {1e15, "MW"},
+ {0, "GW"},
+};
+
static void print_chip_power(const sensors_chip_name *name,
const sensors_feature *feature, int label_size)
{
+ float val;
int need_space = 0;
const sensors_subfeature *sf, *sfmin, *sfmax, *sfint; char *label;
@@ -419,9 +449,11 @@

sf = sensors_get_subfeature(name, feature,
SENSORS_SUBFEATURE_POWER_AVERAGE);
- if (sf)
- printf("%6.2f W", get_value(name, sf));
- else
+ if (sf) {
+ val = get_value(name, sf);
+ scale_value(power_scales, &val, &label);
+ printf("%6.2f %s", val, label);
+ } else
printf(" N/A");

sfmin = sensors_get_subfeature(name, feature,
@@ -434,13 +466,17 @@
printf(" (");

if (sfmin) {
- printf("min = %6.2f W", get_value(name, sfmin));
+ val = get_value(name, sfmin);
+ scale_value(power_scales, &val, &label);
+ printf("min = %6.2f %s", val, label);
need_space = 1;
}

if (sfmax) {
- printf("%smax = %6.2f W", (need_space ? ", " : ""),
- get_value(name, sfmax));
+ val = get_value(name, sfmax);
+ scale_value(power_scales, &val, &label);
+ printf("%smax = %6.2f %s", (need_space ? ", " : ""),
+ val, label);
need_space = 1;
}

@@ -455,10 +491,20 @@
printf("\n");
}

+struct scale_table energy_scales[] = {
+ {1e3, "uJ"},
+ {1e6, "mJ"},
+ {1e9, "J"},
+ {1e12, "KJ"},
+ {1e15, "MJ"},
+ {0, "GJ"},
+};
+
static void print_chip_energy(const sensors_chip_name *name,
const sensors_feature *feature, int label_size)
{
+ float val;
const sensors_subfeature *sf;
char *label;

@@ -472,9 +518,11 @@

sf = sensors_get_subfeature(name, feature,
SENSORS_SUBFEATURE_ENERGY_INPUT);
- if (sf)
- printf("%6.2f J", get_value(name, sf));
- else
+ if (sf) {
+ val = get_value(name, sf);
+ scale_value(energy_scales, &val, &label);
+ printf("%6.2f %s", val, label);
+ } else
printf(" N/A");

printf("\n");

_______________________________________________
lm-sensors mailing list
[email protected]
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors

Jean Delvare

2008-04-02 14:20 UTC #1 lore raw

Re: [lm-sensors] [PATCH 4/4]: sensors: Automatically scale

Hi Darrick,

Quoted 4 lines · Darrick J. Wong

Automatically scale energy and power values when printing them in cooked mode.
Signed-off-by Darrick J. Wong <[email protected]>

First of all, this patch adds the following warnings:

prog/sensors/chips.c: In function "print_chip_power":
prog/sensors/chips.c:454: warning: passing argument 3 of "scale_value" from incompatible pointer type prog/sensors/chips.c:470: warning: passing argument 3 of "scale_value" from incompatible pointer type prog/sensors/chips.c:477: warning: passing argument 3 of "scale_value" from incompatible pointer type prog/sensors/chips.c: In function "print_chip_energy":
prog/sensors/chips.c:523: warning: passing argument 3 of "scale_value" from incompatible pointer type

Please fix.

Quoted 5 lines

Index: lm-sensors-3.0.0/lib/sysfs.c
=================================> --- lm-sensors-3.0.0.orig/lib/sysfs.c 2008-03-31 16:35:30.000000000 -0700
+++ lm-sensors-3.0.0/lib/sysfs.c 2008-03-31 16:35:46.000000000 -0700

Review line 160

Quoted 10 lines

(quoted diff) +0 -4

@@ -155,10 +155,6 @@
 		return 1000;
 	case SENSORS_SUBFEATURE_FAN_INPUT:
 		return 1;
-	case SENSORS_SUBFEATURE_POWER_AVERAGE:
-		return 1000000;
-	case SENSORS_SUBFEATURE_ENERGY_INPUT:
-		return 1000000;
 	case SENSORS_SUBFEATURE_POWER_AVERAGE_INTERVAL:
 		return 1000;
 	}

This revert was certainly not meant to be.

Review line 405

Quoted 9 lines

Index: lm-sensors-3.0.0/prog/sensors/chips.c
=================================> --- lm-sensors-3.0.0.orig/prog/sensors/chips.c 2008-03-31 16:35:38.000000000 -0700
+++ lm-sensors-3.0.0/prog/sensors/chips.c 2008-03-31 16:35:46.000000000 -0700
(quoted diff) +2 -0
@@ -401,10 +401,40 @@
 	printf("\n");
 }
 
+struct scale_table {
+	float upper_bound;

We use double everywhere, please stick to that to avoid converting back and forth.

Review line 406

Quoted 1 line

(quoted diff) +1 -0
+	const char *units;

I suggest "unit" instead - there's just one at a time.

Quoted 8 lines

(quoted diff) +8 -0

+};
+
+static void scale_value(struct scale_table *scales, float *value,
+			const char **unitstr)
+{
+	float divisor = 1;
+
+	while (scales->upper_bound != 0 && *value > scales->upper_bound) {

Review line 424

Quoted 10 lines

(quoted diff) +10 -0

+		divisor = scales->upper_bound;
+		scales++;
+	}
+
+	*value /= divisor;
+
+	*unitstr = scales->units;
+}
+
+struct scale_table power_scales[] = {

Should be static const.

Review line 428

Quoted 4 lines

(quoted diff) +4 -0

+	{1e3,  "uW"},
+	{1e6,  "mW"},
+	{1e9,   "W"},
+	{1e12, "KW"},

Kilo is lowercase k.

Review line 431

Quoted 3 lines

(quoted diff) +3 -0

+	{1e15, "MW"},
+	{0,    "GW"},
+};

I'm a bit surprised, I expected 1e-6 for uW, 1e-3 for mW, 0 for W, etc? Does your code actually output correct values? Or course you would then have to adjust your algorithm as you would no longer be able to use 0 as a terminator - but that's easy to work around.

Quoted 10 lines

(quoted diff) +2 -0

+
 static void print_chip_power(const sensors_chip_name *name,
 			     const sensors_feature *feature,
 			     int label_size)
 {
+	float val;
 	int need_space = 0;
 	const sensors_subfeature *sf, *sfmin, *sfmax, *sfint;
 	char *label;

Review line 455

Quoted 10 lines

(quoted diff) +4 -3

@@ -419,9 +449,11 @@
 
 	sf = sensors_get_subfeature(name, feature,
 				    SENSORS_SUBFEATURE_POWER_AVERAGE);
-	if (sf)
-		printf("%6.2f W", get_value(name, sf));
-	else
+	if (sf) {
+		val = get_value(name, sf);
+		scale_value(power_scales, &val, &label);
+		printf("%6.2f %s", val, label);

This reuse of label is a bit confusing. I'd prefer that you define a new variable for that.

Quoted 27 lines

(quoted diff) +8 -3

+	} else
 		printf("     N/A");
 
 	sfmin = sensors_get_subfeature(name, feature,

@@ -434,13 +466,17 @@
 		printf("  (");
 
 		if (sfmin) {
-			printf("min = %6.2f W", get_value(name, sfmin));
+			val = get_value(name, sfmin);
+			scale_value(power_scales, &val, &label);
+			printf("min = %6.2f %s", val, label);
 			need_space = 1;
 		}
 
 		if (sfmax) {
-			printf("%smax = %6.2f W", (need_space ? ", " : ""),
-			       get_value(name, sfmax));
+			val = get_value(name, sfmax);
+			scale_value(power_scales, &val, &label);
+			printf("%smax = %6.2f %s", (need_space ? ", " : ""),
+			       val, label);
 			need_space = 1;
 		}

@@ -455,10 +491,20 @@
 	printf("\n");

Review line 501

Quoted 10 lines

(quoted diff) +8 -0

 }
 
+struct scale_table energy_scales[] = {
+	{1e3,  "uJ"},
+	{1e6,  "mJ"},
+	{1e9,   "J"},
+	{1e12, "KJ"},
+	{1e15, "MJ"},
+	{0,    "GJ"},
+};

This table is really the same as power_scales with just W becoming J. I think it would be better to have a single table for all units, and to add the unit symbol on the fly. That way we won't need to add another table if we ever want to automatically scale values expressed in another unit.

Quoted 24 lines

(quoted diff) +7 -3

+
 static void print_chip_energy(const sensors_chip_name *name,
 			      const sensors_feature *feature,
 			      int label_size)
 {
+	float val;
 	const sensors_subfeature *sf;
 	char *label;

@@ -472,9 +518,11 @@
 
 	sf = sensors_get_subfeature(name, feature,
 				    SENSORS_SUBFEATURE_ENERGY_INPUT);
-	if (sf)
-		printf("%6.2f J", get_value(name, sf));
-	else
+	if (sf) {
+		val = get_value(name, sf);
+		scale_value(energy_scales, &val, &label);
+		printf("%6.2f %s", val, label);
+	} else
 		printf("     N/A");
 
 	printf("\n");

Signature · 6 lines

Jean Delvare

_______________________________________________
lm-sensors mailing list
[email protected]
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors

Darrick J. Wong

author

2008-04-14 18:37 UTC #2 lore raw

Re: [lm-sensors] [PATCH 4/4]: sensors: Automatically scale

In reply to Darrick J. Wong

Quoted 8 lines · Jean Delvare

Quoted 8 lines
Index: lm-sensors-3.0.0/lib/sysfs.c
=================================> > --- lm-sensors-3.0.0.orig/lib/sysfs.c 2008-03-31 16:35:30.000000000 -0700
+++ lm-sensors-3.0.0/lib/sysfs.c 2008-03-31 16:35:46.000000000 -0700
(quoted diff) +0 -1
@@ -155,10 +155,6 @@
 		return 1000;
 	case SENSORS_SUBFEATURE_FAN_INPUT:
 		return 1;
-	case SENSORS_SUBFEATURE_POWER_AVERAGE:

Quoted 8 lines

Review line 160
Quoted 6 lines
(quoted diff) +0 -3
-		return 1000000;
-	case SENSORS_SUBFEATURE_ENERGY_INPUT:
-		return 1000000;
 	case SENSORS_SUBFEATURE_POWER_AVERAGE_INTERVAL:
 		return 1000;
 	}
This revert was certainly not meant to be.

Actually, it was--my intent for the autoscale functionality was that this static translation goes away and programs are left to scale the raw value as they see fit.

That said, I can see that it would be better for libsensors to export values in a prefix-less unit and _then_ programs can autoscale as they desire. I'll make it do that.

Quoted 4 lines

I'm a bit surprised, I expected 1e-6 for uW, 1e-3 for mW, 0 for W, etc? Does your code actually output correct values? Or course you would then have to adjust your algorithm as you would no longer be able to use 0 as a terminator - but that's easy to work around.

The numeric figure is the upper bound for using that unit--if the number you're trying to autoscale is less than the upper bound, you can use the unit; otherwise, keep going. Hence 1e-3 for uW, 1 for mW, and 1e3 for W once libsensors starts exporting raw values again.

Also, zero works as a terminator as long as all entries in the table represent numeric powers.

Quoted 5 lines

This table is really the same as power_scales with just W becoming J. I think it would be better to have a single table for all units, and to add the unit symbol on the fly. That way we won't need to add another table if we ever want to automatically scale values expressed in another unit.

I'll move the table into the scale function, then. That will enable me to ensure that zero works as a terminator.

--D

_______________________________________________
lm-sensors mailing list
[email protected]
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors