[lm-sensors] PATCH: add support for lm_sensors 3.0.0 to gkrellm

[lm-sensors] PATCH: add support for lm_sensors 3.0.0 to gkrellm

[Hans de Goede]

[-- Attachment #1: Type: text/plain, Size: 578 bytes --]

Hi all,

For those interested. I've made the patch so that gkrellm can be compiled 
against the new version (or the old) and so that the identification strings of 
the sensors do not change and thus old gkrellm configs will continue to work as is.

Regards,

Hans

p.s.

I've also send this upstream, and its available here:
http://people.atrpms.net/~hdegoede/gkrellm-2.3.0-libsensors4.patch

The later is to be able to put up a link on the download page for the 3.0.0 
RC's from the wiki, I haven't done this yet as this patch will not work with 
rc2, it requires current svn.

[-- Attachment #2: gkrellm-2.3.0-libsensors4.patch --]
[-- Type: text/x-patch, Size: 7446 bytes --]

diff -up gkrellm-2.3.0/src/sysdeps/linux.c.libsensors4 gkrellm-2.3.0/src/sysdeps/linux.c
--- gkrellm-2.3.0/src/sysdeps/linux.c.libsensors4	2007-07-07 01:57:20.000000000 +0200
+++ gkrellm-2.3.0/src/sysdeps/linux.c	2007-10-28 11:08:47.000000000 +0100
@@ -2055,14 +2055,14 @@ check_voltage_name(const gchar *name, gi
 static gboolean
 libsensors_init(void)
 	{
-	gint			nr, nr1, nr2, len;
-	const sensors_chip_name		*name;
-	const sensors_feature_data	*feature;
-	gchar			*label, *s;
-	FILE			*f;
+	gint			nr, len, iodev;
+	const sensors_chip_name	*name;
+	gchar			*label, *busname, *s;
 	gchar			id_name[512], sensor_path[512];
 	gint			type, n_sensors_added;
 	ConfigMap		*config_map;
+#if SENSORS_API_VERSION < 0x400 /* libsensor 3 code */
+	FILE			*f;
 
 	f = fopen("/etc/sensors.conf", "r");
 	if (!f)
@@ -2087,7 +2087,8 @@ libsensors_init(void)
 	nr = 0;
 	while ((name = sensors_get_detected_chips(&nr)))
 		{
-		nr1 = 0, nr2 = 0;
+		const sensors_feature_data	*feature;
+		gint nr1 = 0, nr2 = 0;
 		while ((feature = sensors_get_all_features(*name, &nr1, &nr2)))
 			{
 			if (   sensors_get_ignored(*name, feature->number)
@@ -2137,6 +2138,11 @@ libsensors_init(void)
 					continue;
 					}
 
+				/* notice that we store the bus and addr both in iodev as
+				|  2 _signed 16 bit ints. */
+				iodev = (name->bus & 0xFFFF) | (name->addr << 16);
+				busname = name->busname;
+				
 				if (sensors_get_label(*name, feature->number, &label) != 0)
 					{
 					if (_GK.debug_level & DEBUG_SENSORS)
@@ -2144,7 +2150,119 @@ libsensors_init(void)
 								id_name);
 					label = NULL;
 					}
+#else /* libsensors4 code */
+	if (sensors_init(NULL) != 0)
+		{
+		if (_GK.debug_level & DEBUG_SENSORS)
+			printf("libsensors: init failed!\n");
+		return FALSE;
+		}
+
+	if (_GK.debug_level & DEBUG_SENSORS)
+		printf("libsensors: init OK\n");
+
+	n_sensors_added = 0;
+	nr = 0;
+	while ((name = sensors_get_detected_chips(NULL, &nr)))
+		{
+		const sensors_subfeature *feature;
+		const sensors_feature *main_feature;
+		gint nr1 = 0;
+		while ((main_feature = sensors_get_features(name, &nr1)))
+			{
+			switch (name->bus.type)
+			  {
+			  case SENSORS_BUS_TYPE_I2C:
+			  case SENSORS_BUS_TYPE_SPI:
+				snprintf(id_name, sizeof(id_name), "%s@%d:%x/%s",
+					name->prefix, name->bus.nr, name->addr, main_feature->name);
+				break;
+			  default:
+				snprintf(id_name, sizeof(id_name), "%s@%x/%s",
+					name->prefix, name->addr, main_feature->name);
+			          
+			  }
+			/* We need to store both the prefix and the path, but we
+			|  only have one string, so concat them together separated by :
+			*/
+			snprintf(sensor_path, sizeof (sensor_path), "%s:%s",
+				name->prefix, name->path ? name->path : "NULL");
+
+			switch (main_feature->type)
+			  {
+			  case SENSORS_FEATURE_IN:
+				type = SENSOR_VOLTAGE;
+				feature = sensors_get_subfeature(name, 
+						main_feature, SENSORS_SUBFEATURE_IN_INPUT);
+			  	break;
+			  case SENSORS_FEATURE_FAN:
+				type = SENSOR_FAN;
+				feature = sensors_get_subfeature(name, 
+						main_feature, SENSORS_SUBFEATURE_FAN_INPUT);
+				break;
+			  case SENSORS_FEATURE_TEMP:
+				type = SENSOR_TEMPERATURE;
+				feature = sensors_get_subfeature(name, 
+						main_feature, SENSORS_SUBFEATURE_TEMP_INPUT);
+				break;
+			  default:
+				if (_GK.debug_level & DEBUG_SENSORS)
+					printf("libsensors: error determining type for: %s\n",
+							id_name);
+				continue;
+			  }
+
+			if (!feature)
+				{
+				if (_GK.debug_level & DEBUG_SENSORS)
+					printf("libsensors: error could not get input subfeature for: %s\n",
+							id_name);
+				continue;
+				}
+
+			/* failsafe tests, will bus type and nr fit in 8 bits
+			   signed and addr fit in 16 bits signed ?
+			*/
+			if (name->bus.type != ((name->bus.type << 24) >> 24))
+				{
+				if (_GK.debug_level & DEBUG_SENSORS)
+					printf("libsensors: bus-type bigger than 8 bits: %s\n",
+							id_name);
+				continue;
+				}
+			if (name->bus.nr != ((name->bus.nr << 24) >> 24))
+				{
+				if (_GK.debug_level & DEBUG_SENSORS)
+					printf("libsensors: bus-nr bigger than 8 bits: %s\n",
+							id_name);
+				continue;
+				}
+			if (name->addr != ((name->addr << 16) >> 16))
+				{
+				if (_GK.debug_level & DEBUG_SENSORS)
+					printf("libsensors: addr bigger than 16 bits: %s\n",
+							id_name);
+				continue;
+				}
 
+			/* notice that we store the bus id, type and addr in
+			   iodev as 2 signed 8 bit ints and one 16 bit int */
+			iodev = (name->bus.type & 0xFF) |
+				((name->bus.nr & 0xFF) << 8) |
+				(name->addr << 16);
+			busname = name->path;
+
+			label = sensors_get_label(name, main_feature);
+			if (!label && (_GK.debug_level & DEBUG_SENSORS))
+				printf("libsensors: error getting label for: %s\n",
+						id_name);
+
+			/* additional { to match "if (get_ignored(..) {"
+			   from libsensors3 code */
+				{
+#endif
+				if (label)
+				{
 				/* Strip some common post/prefixes for smaller default labels
 				*/
 				len = strlen(label);
@@ -2171,14 +2289,13 @@ libsensors_init(void)
 						memmove(label, label + 4, strlen (label + 4) + 1);
 					break;
 				  }
-				/* notice that we store the bus and addr both in iodev as
-				|  2 _signed 16 bit ints.  Default factor of zero tells
-				|  sensor.c that sensor formula is handled, ie via
+				}
+				/* Default factor of zero tells sensor.c
+				|  that sensor formula is handled, ie via
 				|  /etc/sensors.conf.
 				*/
 				gkrellm_sensors_add_sensor(type, sensor_path, id_name,
-							feature->number,
-							(name->bus & 0xFFFF) | (name->addr << 16),
+							feature->number, iodev,
 							LIBSENSORS_INTERFACE, 0.0, 0.0,
 							NULL, label);
 				++n_sensors_added;
@@ -2188,10 +2305,10 @@ libsensors_init(void)
 				if (_GK.debug_level & DEBUG_SENSORS)
 					printf("%s %s %x\n",
 							sensor_path, id_name,
-							(name->bus & 0xFFFF) | (name->addr << 16));
+							iodev);
 
-				if (   name->busname
-				    && (s = strrchr(name->busname, '/')) != NULL
+				if (   busname
+				    && (s = strrchr(busname, '/')) != NULL
 				    && *(s + 1)
 				   )
 					{
@@ -2229,8 +2346,9 @@ libsensors_get_value(char *sensor_path, 
 		}
 	*p = 0;						/* We must undo this !! (or make a copy) */
 	name.prefix = sensor_path;
-	name.bus = (iodev << 16) >> 16;	/* sign extend the low 16 bits */
 	name.addr = iodev >> 16;
+#if SENSORS_API_VERSION < 0x400 /* libsensor 3 code */
+	name.bus = (iodev << 16) >> 16;	/* sign extend the low 16 bits */
 	name.busname = p + 1;
 	if (!strcmp(name.busname, "NULL"))
 		name.busname = NULL;
@@ -2248,6 +2366,32 @@ libsensors_get_value(char *sensor_path, 
 					name.prefix, name.addr, id);
 		}
 
+#else /* libsensors4 code */
+	name.bus.type = (iodev << 24) >> 24; /* sign extend the low 8 bits */
+	name.bus.nr   = (iodev << 16) >> 24; /* sign extend the 2nd byte */
+	name.path = p + 1;
+	if (!strcmp(name.path, "NULL"))
+		name.path = NULL;
+
+	result = sensors_get_value(&name, id, &val) == 0;
+	
+	if (!result && (_GK.debug_level & DEBUG_SENSORS))
+		{
+			switch (name.bus.type)
+			  {
+			  case SENSORS_BUS_TYPE_I2C:
+			  case SENSORS_BUS_TYPE_SPI:
+				printf(
+					"libsensors: error getting value for: %s@%d:%x feature: %d\n",
+					name.prefix, (int)name.bus.nr, name.addr, id);
+				break;
+			  default:
+				printf("libsensors: error getting value for: %s@%x feature: %d\n",
+					name.prefix, name.addr, id);
+			  }
+		}
+#endif
+
 	if (value)
 		*value = val;
 

[-- Attachment #3: Type: text/plain, Size: 153 bytes --]

_______________________________________________
lm-sensors mailing list
lm-sensors@lm-sensors.org
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors

Re: [lm-sensors] PATCH: add support for lm_sensors 3.0.0 to gkrellm

[Jean Delvare]

Hi Hans,

On Sun, 28 Oct 2007 11:26:48 +0100, Hans de Goede wrote:
> For those interested. I've made the patch so that gkrellm can be compiled 
> against the new version (or the old) and so that the identification strings of 
> the sensors do not change and thus old gkrellm configs will continue to work as is.

Random comments:

> +			snprintf(sensor_path, sizeof (sensor_path), "%s:%s",
> +				name->prefix, name->path ? name->path : "NULL");

name->path can't actually be NULL. BTW, I wonder why you need
name->path at all. This is meant for libsensors internal use,
applications shouldn't care about it. Also note that the path is not
guaranteed to be consistent across reboots.

> +			/* failsafe tests, will bus type and nr fit in 8 bits
> +			   signed and addr fit in 16 bits signed ?
> +			*/

Unfortunately not: for SPI, the bus number can require 16 bits. It's
due to the weird way spi-core uses to number buses dynamically, it
should really be addressed someday, but at the moment it isn't.

> I've also send this upstream, and its available here:
> http://people.atrpms.net/~hdegoede/gkrellm-2.3.0-libsensors4.patch

Note that I've stopped referring to "libsensors4" after I realized it
could cause confusion. While the binary file will be named
"libsensors.so.4", the library version will be advertised as
"libsensors 3.0.0". So for example I've renamed the xsensors patch to:

xsensors-0.60-libsensors-3.patch

And I suggest that you do the same. The soname is an internal detail
end users don't even need to know.

> The later is to be able to put up a link on the download page for the 3.0.0 
> RC's from the wiki, I haven't done this yet as this patch will not work with 
> rc2, it requires current svn.

rc3 is released now, so you could proceed. However you'll need special
permissions to edit the Download page. So I can either grant you these
permissions if you want, or I can add the link for you if you prefer.

-- 
Jean Delvare

_______________________________________________
lm-sensors mailing list
lm-sensors@lm-sensors.org
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors

Re: [lm-sensors] PATCH: add support for lm_sensors 3.0.0 to gkrellm

[Hans de Goede]

Jean Delvare wrote:
> Hi Hans,
> 
> On Sun, 28 Oct 2007 11:26:48 +0100, Hans de Goede wrote:
>> For those interested. I've made the patch so that gkrellm can be compiled 
>> against the new version (or the old) and so that the identification strings of 
>> the sensors do not change and thus old gkrellm configs will continue to work as is.
> 
> Random comments:
> 
>> +			snprintf(sensor_path, sizeof (sensor_path), "%s:%s",
>> +				name->prefix, name->path ? name->path : "NULL");
> 
> name->path can't actually be NULL. BTW, I wonder why you need
> name->path at all. This is meant for libsensors internal use,
> applications shouldn't care about it. Also note that the path is not
> guaranteed to be consistent across reboots.
> 

gkrellm used todo all reading of sysfs files (and other type of info like ibm 
acpi) itself, it has an interface for this where the read_method for a sensors 
"driver" (for example direct /sysfs access, libsensors, hddtemp deamon, ...) 
gets passed 1 string and 2 ints.  The above and below code is a way of 
shoehorning libsensors chip_name struct into one string and one int (the second 
int is used for the feature number).

>> +			/* failsafe tests, will bus type and nr fit in 8 bits
>> +			   signed and addr fit in 16 bits signed ?
>> +			*/
> 
> Unfortunately not: for SPI, the bus number can require 16 bits. It's
> due to the weird way spi-core uses to number buses dynamically, it
> should really be addressed someday, but at the moment it isn't.
> 

UGH, how often does this happen? Note that currently gkrellm will simply skip 
sensors for which the bus number > 8 bits. I've noticed that the pointers 
returned by: sensors_get_detected_chips are constant between calls to 
sensors_init() and sensors_cleanup(), so I could change the shoehorning to put 
a pointer to the chipname in hex format into the string (on 64 bit it won't fit 
into an int) but that feels really really dirty!

>> I've also send this upstream, and its available here:
>> http://people.atrpms.net/~hdegoede/gkrellm-2.3.0-libsensors4.patch
> 
> Note that I've stopped referring to "libsensors4" after I realized it
> could cause confusion. While the binary file will be named
> "libsensors.so.4", the library version will be advertised as
> "libsensors 3.0.0". So for example I've renamed the xsensors patch to:
> 
> xsensors-0.60-libsensors-3.patch
> 
> And I suggest that you do the same. The soname is an internal detail
> end users don't even need to know.
> 

Okay, I got the nomenclature from you, I'll stop using it now.

>> The later is to be able to put up a link on the download page for the 3.0.0 
>> RC's from the wiki, I haven't done this yet as this patch will not work with 
>> rc2, it requires current svn.
> 
> rc3 is released now, so you could proceed. However you'll need special
> permissions to edit the Download page. So I can either grant you these
> permissions if you want, or I can add the link for you if you prefer.
> 

Either way is fine with me, but I'll probably be doing more patches as time 
permits and I would like to put those on the wiki too, so I guess having 
editing rights is the easiest in the end.

Regards,

Hans

_______________________________________________
lm-sensors mailing list
lm-sensors@lm-sensors.org
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors

Re: [lm-sensors] PATCH: add support for lm_sensors 3.0.0 to gkrellm

[Jean Delvare]

On Sun, 28 Oct 2007 15:23:23 +0100, Hans de Goede wrote:
> Jean Delvare wrote:
> > name->path can't actually be NULL. BTW, I wonder why you need
> > name->path at all. This is meant for libsensors internal use,
> > applications shouldn't care about it. Also note that the path is not
> > guaranteed to be consistent across reboots.
> 
> gkrellm used todo all reading of sysfs files (and other type of info like ibm 
> acpi) itself, it has an interface for this where the read_method for a sensors 
> "driver" (for example direct /sysfs access, libsensors, hddtemp deamon, ...) 
> gets passed 1 string and 2 ints.  The above and below code is a way of 
> shoehorning libsensors chip_name struct into one string and one int (the second 
> int is used for the feature number).

OK, but now with the new libsensors, there's really no reason why
gkrellm would want to read from sysfs directly. So why not simply drop
this possibility to make the code more simple?

> (...)
> > Unfortunately not: for SPI, the bus number can require 16 bits. It's
> > due to the weird way spi-core uses to number buses dynamically, it
> > should really be addressed someday, but at the moment it isn't.
> > 
> 
> UGH, how often does this happen? Note that currently gkrellm will simply skip 
> sensors for which the bus number > 8 bits. I've noticed that the pointers 
> returned by: sensors_get_detected_chips are constant between calls to 
> sensors_init() and sensors_cleanup(), so I could change the shoehorning to put 
> a pointer to the chipname in hex format into the string (on 64 bit it won't fit 
> into an int) but that feels really really dirty!

It happens each time an SPI bus is numbered dynamically (i.e. isn't
considered one of "main" SPI buses of the system.) Typically this
happens when doing SPI over parallel port. But at the moment, the only
SPI hardware monitoring device we support is the LM70, and I'm not sure
how used it is... probably not much.

So it's not a big problem IMHO, especially as whoever is using the SPI
bus for hardware monitoring purposes most probably isn't running
gkrellm. If I were you I wouldn't bother, your time can probably be
used better for other tasks. Whoever really cares would better fix the
SPI bus numbering algorithm in the kernel anyway.

> (...)
> > rc3 is released now, so you could proceed. However you'll need special
> > permissions to edit the Download page. So I can either grant you these
> > permissions if you want, or I can add the link for you if you prefer.
> 
> Either way is fine with me, but I'll probably be doing more patches as time 
> permits and I would like to put those on the wiki too, so I guess having 
> editing rights is the easiest in the end.

You are now a wiki admin, so you can edit read-only pages. Handle with
care ;)

-- 
Jean Delvare

_______________________________________________
lm-sensors mailing list
lm-sensors@lm-sensors.org
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors