Bug 153232 - ksysguardd doesnt compile against lm-sensors 3.0.0
Summary: ksysguardd doesnt compile against lm-sensors 3.0.0
Status: RESOLVED FIXED
Alias: None
Product: ksysguard
Classification: Applications
Component: ksysguardd (show other bugs)
Version: unspecified
Platform: Compiled Sources Linux
: NOR normal
Target Milestone: ---
Assignee: KSysGuard Developers
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2007-12-01 19:47 UTC by Jan Mette
Modified: 2008-03-05 09:41 UTC (History)
4 users (show)

See Also:
Latest Commit:
Version Fixed In:


Attachments
patch to make ksysguard svn compile with lm-sensors-3 (4.65 KB, patch)
2007-12-25 20:01 UTC, Chetan Reddy
Details
patch to ksysguardd for lmsensors3 (4.57 KB, patch)
2007-12-28 01:26 UTC, Chetan Reddy
Details
PATCH: fix compile (and run) of ksysguard with lm-sensors-3.x.x (3.74 KB, patch)
2008-01-01 22:09 UTC, Hans de Goede
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Jan Mette 2007-12-01 19:47:13 UTC
Version:            (using KDE KDE 3.5.8)
Installed from:    Compiled From Sources
Compiler:          gcc 4.2.2 
OS:                Linux

Hi there,

since the upgrade of lm-sensors to 3.0.0, ksysguardd doesnt compile anymore... Here is the output of the compilation process:

gcc -DHAVE_CONFIG_H -I. -I../../.. -I./../../CContLib -I./.. -I/opt/kde/include -I/opt/qt/include -I. -D_GNU_SOURCE -DQT_THREAD_SUPPORT -D_REENTRANT -D_FILE_OFFSET_BITS=64 -D_LARGE_FILES=1 -Wall -std=iso9899:1990 -W -Wall -Wchar-subscripts -Wshadow -Wpointer-arith -Wmissing-prototypes -Wwrite-strings -D_XOPEN_SOURCE=500 -D_BSD_SOURCE -DNDEBUG -O2 -march=i686 -mtune=generic -O2 -pipe -Wformat-security -Wmissing-format-attribute -D_GNU_SOURCE -c lmsensors.c
lmsensors.c:39: error: expected ':', ',', ';', '}' or '__attribute__' before '*' token
lmsensors.c: In function 'initLmSensors':
lmsensors.c:88: warning: passing argument 1 of 'sensors_get_detected_chips' from incompatible pointer type
lmsensors.c:88: error: too few arguments to function 'sensors_get_detected_chips'
lmsensors.c:90: error: expected '=', ',', ';', 'asm' or '__attribute__' before '*' token
lmsensors.c:90: error: 'sfd' undeclared (first use in this function)
lmsensors.c:90: error: (Each undeclared identifier is reported only once
lmsensors.c:90: error: for each function it appears in.)
lmsensors.c:92: warning: implicit declaration of function 'sensors_get_all_features'
lmsensors.c:93: error: 'SENSORS_NO_MAPPING' undeclared (first use in this function)
lmsensors.c:98: error: incompatible type for argument 1 of 'sensors_get_label'
lmsensors.c:98: error: too many arguments to function 'sensors_get_label'
lmsensors.c:112: error: 'LMSENSOR' has no member named 'sfd'
lmsensors.c: In function 'printLmSensor':
lmsensors.c:141: warning: implicit declaration of function 'sensors_get_feature'
lmsensors.c:141: error: 'LMSENSOR' has no member named 'sfd'
make[4]: *** [lmsensors.o] Error 1
make[4]: Leaving directory `/home/jan/Arch/work/kdebase/src/kdebase-3.5.8/ksysguard/ksysguardd/Linux'
make[3]: *** [all-recursive] Error 1
make[3]: Leaving directory `/home/jan/Arch/work/kdebase/src/kdebase-3.5.8/ksysguard/ksysguardd'
make[2]: *** [all-recursive] Error 1
make[2]: Leaving directory `/home/jan/Arch/work/kdebase/src/kdebase-3.5.8/ksysguard'
make[1]: *** [all-recursive] Error 1
make[1]: Leaving directory `/home/jan/Arch/work/kdebase/src/kdebase-3.5.8'
make: *** [all] Error 2

Seems the lm-sensors API has been changed. Tell me if you need more info.

Thanks

Jan
Comment 1 Hai Zaar 2007-12-09 12:00:01 UTC
Same here
Comment 2 Yigal Korman 2007-12-09 12:01:05 UTC
yep got the same error with kde 3.5.6
Comment 3 Hans de Goede 2007-12-09 22:34:16 UTC
Hi All,

I'm a lm_sensors developer and the breakage is expected, as lm_sensors-3.0.0 comes with a new API (with much needed improvements).

Fixing this should be easy, although different the API's are alike quite a bit. I've already written patches for several apps, see for example:
http://people.atrpms.net/~hdegoede/sensors-applet-1.8.1-lmsensors-3x.patch
http://people.atrpms.net/~hdegoede/xfce4-sensors-plugin-0.10.99.2-lm_sensors3x.patch

I'm currently rather busy with other stuff I'm afraid though. But if someone else writes a patch I'm more then willing to answer any questions you might have about the new API and to review the resulting patch.

You can contact me through this BZ ticket, or directly by mail.

Regards,

Hans
Comment 4 Chetan Reddy 2007-12-25 20:01:38 UTC
Created attachment 22676 [details]
patch to make ksysguard svn compile with lm-sensors-3

Here is a  patch i made that works for me. I'm very new to lmsensors so i don't
know if the patch is correct. The patch is made against trunk. It also fixes a
typo in CMakeLists.txt so that libsensors(any version) is actually used. That
part should not be relevant for kde3 users.
Comment 5 Chetan Reddy 2007-12-25 20:03:55 UTC
Also, i have kde commit privileges from a couple of years ago, so i can commit it to trunk if it works for other people.
Comment 6 Hans de Goede 2007-12-25 21:19:52 UTC
I'm familiar with lm_sensors, so give me a few days and I'll review it to make sure its ok from lm_sensors usage pov.
Comment 7 Rex Dieter 2007-12-27 02:55:35 UTC
Thanks Hans.
Comment 8 John Tapsell 2007-12-27 10:20:50 UTC
Chetan,

  From a ksysguard pov, the patch looks fine.  Feel free to commit
when you are ready.
Comment 9 Hans de Goede 2007-12-27 13:37:15 UTC
Okay, 

Some initial comments from reviewing the patch:

1) The new code in chipName() is wrong, spi and i2c both have bus-numbers and
   thus both should use the default case.

2) However with lm_sensors-3.0.0 the entire code can simply be replaced by:
   sensors_snprintf_chip_name(buffer, sizeof(buffer), chip);

3) There is no need to try to fopen /etc/sensors3.conf and then 
   /etc/sensors.conf, with lm_sensors-3.0.0 you can just pass NULL to 
   sensors_init(), if you want to use the default config file, which will
   first check for /etc/sensors3.conf and then try /etc/sensors.conf

4) Enumerating over all subfeatures is probably not what you want, notice that
   subfeatures have distinct queryable types now like in_input / in_min /
   in_max / in_alarm to name a few examples for voltage sensor. On my 
   motherboard with the abituguru driver for example you will get 174 sensors
   if you see each subfeature as a distinct sensor, however in reality I only
   have 19 sensors, with a lot of attributes like min/max values, beep when
   passing threshold yes/no, etc. The fact then one can now tell wether 
   something is a real feature or merely a subfeature / attribute, and that one
   can now query the exact type from a fixed list is one of the big improvements
   in the new API.

I'll take a closer look, actually building and running the code to see how to best fix 4.
Comment 10 Chetan Reddy 2007-12-28 01:26:38 UTC
Created attachment 22724 [details]
patch to ksysguardd for lmsensors3

I've incorporated changes suggested in 1,2 and 3 into the new patch. Regarding
4, I think in ksysguard, the users can have access to all the
subfeatures/attributes and can choose which of them they want on their
worksheet. I've made a change where the type of BEEP subfeatures is shown as
"bool" rather that "float". Also a small memory leak fix. If a few other people
report that the patch works(with ksysguard running for a while), i'll commit.
Comment 11 Hans de Goede 2007-12-28 09:49:07 UTC
Looks better,

The free(label) must be done for the old api too, so loose the #ifdef around it.

About 4) Yesterday I've checked ksysguard from kde3, and that one only shows sensor inputs, not all their attributes, the atrributes are filtered out when using the old api by the search_ctnr( LmSensors, sensorCmp, p ) call, as the main input and all its attrributes get the same fullName, and sensors_get_all_features() returns the features sorted, starting with the main input and then all its subfeatures, so the subfeatures never get added as there already is an entry in the container with the same fullName. I was working on a patch against 3.5.8 yesterday which would show the same behavior with the new lmsensors API, without depending on fullName being the same (without needing to use search_ctnr() at all), but then I got side tracked debugging mesa issues. Forward porting my patch (once written) to svn should be trivial, so please let me know which behavior you want, if you want to list all subfeatures too, then your current patch is fine, if not let me know and I'll finish my patch.

As for detecting bools when deciding to display them all now, you are missing all the _ALARM types, which are also bools.
Comment 12 Chetan Reddy 2007-12-28 16:14:17 UTC
I just checked ksysguard on my work machine(kde3), and it seems more intuitive to use that way. You can go ahead and make the patch. Let me know if you don't find time or some such and i'll do it.
Comment 13 Hans de Goede 2008-01-01 22:09:00 UTC
Created attachment 22792 [details]
PATCH: fix compile (and run) of ksysguard with lm-sensors-3.x.x

As promised.

Notice for porting this to kde4, the only thing I changed compared to the
currently attached kde4 patch is the loop in initLmSensors().
Comment 14 John Tapsell 2008-01-02 11:32:52 UTC
Hans,

  Excellent.  Thanks for doing this.  I'll leave this to you to commit?

John
Comment 15 Hans de Goede 2008-01-02 11:35:38 UTC
I don't have kde commit rights, I'm an lm_sensors developer not a kde developer.
Comment 16 Chetan Reddy 2008-01-02 15:31:02 UTC
John,

You can go ahead and commit the patch. I am not able to find my (old) kde svn  credentials right now.

Chetan
Comment 17 John Tapsell 2008-01-02 15:41:30 UTC
Is "patch to ksysguardd for lmsensors3"  ready to be committed then to KDE4?  Does "PATCH: fix compile (and run) of ksysguard with lm-sensors-3.x.x"  miss anything that the KDE4 patch has, or are they functionally the same?
Comment 18 Hans de Goede 2008-01-02 15:44:58 UTC
"PATCH: fix compile (and run) of ksysguard with lm-sensors-3.x.x" for kde3 has some fixes to the scanning loop compared to "patch to ksysguardd for lmsensors3" for KDE4, the kde3 patch can be applied as is, the kde4 patch needs to first have my scanning loop improvements ported to it, I can do that but I do not have a kde4 testing rig so I would rather have someone who can actually test it does it, should be trivial.
Comment 19 Chetan Reddy 2008-01-02 15:50:03 UTC
In addition to applying "PATCH: fix compile (and run) of ksysguard with lm-sensors-3.x.x", you should also fix a small typo in ksysguardd/CMakeLists.txt

-macro_bool_to_01(SENSORS_FOUND, HAVE_LMSENSORS)
+macro_bool_to_01(SENSORS_FOUND HAVE_LMSENSORS)
Comment 20 Chetan Reddy 2008-01-03 05:18:22 UTC
committed Hans' patch and fixed the CMakeLists typo in commit #756296
Comment 21 John Tapsell 2008-01-03 08:59:30 UTC
Excellent.  Thanks all!
Comment 22 John Tapsell 2008-01-03 14:18:53 UTC
It doesn't compile now for me.  I have the old version of lm_sensors

/home/kdedev/src/kde/kdebase/workspace/ksysguard/ksysguardd/Linux/lmsensors.c:41:5:
warning: "SENSORS_API_VERSION" is not defined

I'll have a play

On 3 Jan 2008 04:18:23 -0000, Chetan Reddy <chetanreddy@gmail.com> wrote:
[bugs.kde.org quoted mail]
Comment 23 Hans de Goede 2008-01-03 14:39:13 UTC
Thats fixed if you use the latest lm_sensors-2.10.x series, otherwise don't make warnings errors.

There are no other options, the SENSORS_API_VERSION define was specifically added for this, and also added to the 2.10.x series to fix the warning. But its just a warning, SENSORS_API_VERSION when used as an integer for preprocessor logic will evaluate to 0, so the check will fail and you will get the old code.

Alternatively you could rewrite the #if to:
#if defined SENSORS_API_VERSION && (SENSORS_API_VERSION & 0x400)

Comment 24 John Tapsell 2008-01-03 14:53:36 UTC
Okay I added
#ifndef SENSORS_API_VERSION
#define SENSORS_API_VERSION 0x000
#endif

I also had to add
#ifndef SENSORS_CHIP_NAME_BUS_PCI
#define SENSORS_CHIP_NAME_BUS_PCI -5
#endif

It seems it's not defined for some people.  And they also have version
2.10.  Any ideas about this?

John
Comment 25 Hans de Goede 2008-01-03 15:02:08 UTC
I also had to add
#ifndef SENSORS_CHIP_NAME_BUS_PCI
#define SENSORS_CHIP_NAME_BUS_PCI -5
#endif

It seems it's not defined for some people.  And they also have version
2.10.  Any ideas about this?

John

---

I'm clueless, better ask at: lm-sensors@lm-sensors.org

I'm sure someone there will know.
Comment 26 John Tapsell 2008-03-04 23:43:57 UTC
Quite a few people are complaining that in ksysguard 3.5.9  lmsensors
no longer works for them, when it work in 3.5.8.  Is this related to
these fixes?

John
Comment 27 Hans de Goede 2008-03-05 09:41:53 UTC
In reply to comment #26:
> Quite a few people are complaining that in ksysguard 3.5.9  lmsensors
> no longer works for them, when it work in 3.5.8.  Is this related to
> these fixes?
>
> John

Could be, can someone attach the patch as it ultimately got committed to the 3.5.x branch? Then I'll take a look.