[lm-sensors] libsensors patch to gkrellm - a report

[lm-sensors] libsensors patch to gkrellm - a report

[Hans de Goede]

Hi all,

Rudolf Marek wrote:
> Hello,
> 
> I just compiled latest gkrellm with libsensors and noticed following:
> 

Sorry for the very late reply.

> 1) for k8temp driver it displays temp1-temp4 even when I have only 
> temp1_input
>    (prehaps there must be similar test as it is in "sensors" program)
> 

Actually, the problem is in libsensors / sensors.conf . gkrellm-2.2.10 
(which has my patch integrated) checks
sensors_get_ignored() and only shows non ignored sensors. So there are 2 
errors at the other side here:
1) You should put an ignore tempX for those 3 temps in /etc/sensors.conf
2) since only temp1_input shows up under sysfs, libsensors should return
    0 from sensors_get_ignored() for the other sensors regardless of
    of /etc/sensors.conf, as it simply isn't there

I believe that 2) and then 1) are the right solution here, I've written 
a patch which does a testread of the sensor (attached) and only adds it 
to the list of sensors if the testread succeeds, which fixes this with 
k8temp. But that is not a good idea IMHO, then you're using dynamic data 
for something static as the list of sensors in gkrellm is created upon 
startup and from then on its static. The trial read is dynamic / not 
reliable as sometimes one read can fail for one reason or the other and 
the next read will succeed.

Bill please do NOT apply this patch its for testing only. Rudolf, can 
you give the patch a run, it should fix your k8temp problem and read on ...

> 2) For some reason I have fan1 fan2 fan3 fan5 fan4 in the rows. Maybe 
> because I dont have fan4 (not enabled by driver w83627ehf)
> 

A bit strange this, yes. Probably again you haven't told libsensors to 
ignore fan4 for this chip in /etc/sensors.conf, neither has libsensors 
come up with the idea todo this itself. Can you try that and / or the 
attached patch to see if that helps?

Rudolf + Jean, would it be a good idea to write a patch for libsensors 
to make it automaticly set features not found in sysfs to ignore?

Regards,

Hans
-------------- next part --------------
A non-text attachment was scrubbed...
Name: gkrellm-2.2.10-test-read.patch
Type: text/x-patch
Size: 892 bytes
Desc: not available
Url : http://lists.lm-sensors.org/pipermail/lm-sensors/attachments/20070309/c46cc062/attachment.bin 

[lm-sensors] libsensors patch to gkrellm - a report

[Jean Delvare]

Hi Hans,

On Fri, 09 Mar 2007 20:44:20 +0100, Hans de Goede wrote:
> Rudolf + Jean, would it be a good idea to write a patch for libsensors 
> to make it automaticly set features not found in sysfs to ignore?

This should no longer be needed once we implement dynamic feature
discovery in libsensors. If I had some time to work on libsensors, I'd
rather spend it on reviewing these patches we already have than on
additional workarounds to make the current design look less broken.

-- 
Jean Delvare

[lm-sensors] libsensors patch to gkrellm - a report

[Hans de Goede]

Jean Delvare wrote:
> Hi Hans,
> 
> On Fri, 09 Mar 2007 20:44:20 +0100, Hans de Goede wrote:
>> Rudolf + Jean, would it be a good idea to write a patch for libsensors 
>> to make it automaticly set features not found in sysfs to ignore?
> 
> This should no longer be needed once we implement dynamic feature
> discovery in libsensors. If I had some time to work on libsensors, I'd
> rather spend it on reviewing these patches we already have than on
> additional workarounds to make the current design look less broken.
> 

True, I've already poked one of the students who has written the dynamic 
features support patches to get moving with regards to merging them. However 
this was a semester project and the semester is over. So I don't know how this 
will fare. If he doesn't get moving soon I'll jump in and start posting them 
for review and make the necessary fixes myself.

That still leaves the question where to put these patches. I myself don't 
really like the whole grand 3.0 scheme, because we are all really busy with 
other stuff and IMHO don't seem to have the manpower todo a whole new release, 
and also I don't see any improvements planned to warrant this version jump and 
more important to break ABI compatibility. libsensors API is not ideal, if we 
are going to break the API + ABI, then I would like to first see a redesign of 
said API. So I would rather see small incremental steps. For example putting 
dyn features detect in 2.10.x (or 2.12.0), but at first only for new chips, 
then check with 2.6 only chips, and remove those one at a time from the static 
list. An other small increment could be adding include directive support to 
sensors.conf. An other incremenuld be to put in #ifdef's around 2.4 compat / 
only code so that people who want to can build a version without 2.4 support.

Anyways I think I've made my preference clear. If you and Mark prefer doing a 
3.0 and putting new features there, then my student or I will start looking at 
the 3.0 svn branch and adjusting the patches as needed. So let me know what it 
will be.

Regards,

Hans