Bug 153232

Same here

yep got the same error with kde 3.5.6

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

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.

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.

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.

Thanks Hans.

Chetan,

  From a ksysguard pov, the patch looks fine.  Feel free to commit
when you are ready.

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.

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.

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.

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.

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().

Hans,

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

John

I don't have kde commit rights, I'm an lm_sensors developer not a kde developer.

John,

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

Chetan

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?

"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.

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)

committed Hans' patch and fixed the CMakeLists typo in commit #756296

Excellent.  Thanks all!

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]

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)

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

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.

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

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.