Re: [lm-sensors] lm-sensors 3.0.0-rc1 has been released!

[Hans de Goede <j.w.r.degoede@hhs.nl>]

Jean Delvare wrote:
> Hi Hans,
> 
> On Sun, 21 Oct 2007 00:41:59 +0200, Hans de Goede wrote:
>> Jean Delvare wrote:
>>> On Fri, 19 Oct 2007 22:18:02 +0200, Hans de Goede wrote:
>>>> Ideally my first naive code should just work for simple applications which only 
>>>> want to read the main / _input feature, I haven't checked yet, but I think 
>>>> making my initial code work, doesn't match well with the current libsensors 
>>>> structure, 
>>> Indeed. Features and subfeatures are now separate entities. When I
>>> proposed to do that change a few months ago, nobody objected.
>> I didn't understand back then that foo#_input would become a subfeature too, I 
>> thought that would be part of the main feature.
> 
> That would have been a mess, I'm glad I didn't do that ;)
> 
>> (...)
>>> First of all, please note that the main value in question may not
>>> exist. Your code should be ready for it to happen.
>>>
>> Do features that do have tresholds / other settings but no reading of the 
>> actual input exist?
> 
> As crazy as it sounds, yes, it exists. The early revisions of the
> GL518SM report no voltage measurements, but have min and max voltage
> settings and report alarms when those are crossed. Currently the
> gl518sm driver reports 0 for the measurements in this case, but that's
> not correct, it should not report them at all. I'll fix it.
> 
>>> My guess was that applications would need to switch on the feature type
>>> anyway, as different types are usually displayed differently, so I
>>> never considered this to be a problem. All of sensors, sensord and
>>> xsensors do, I'm surprised that gkrellm doesn't. How does it display
>>> the proper unit? How does it adjust the number of decimal places?
>> This is correct, there indeed is such a switch present, the code I postred was 
>> simplified for the discussion. Still needing to make an additional function 
>> call to get to the _input subfeature feels like a detour.
>>
>>> I am also surprised that you call sensors_get_value() directly in the
>>> loop that discovers the features. Applications which display values
>>> continuously (as opposed to sensors' one-shot) tend to store the
>>> feature numbers at initialization time, and then call
>>> sensors_get_value() on them directly without looping over
>>> sensors_get_features() again.
>> Actually you are right, the feature number gets remembered. (again I simplified 
>> the code).
>>
>>> Adding input_subfeature to struct sensors_subfeature is possible if it
>>> helps you. It should be fairly easy. I'd just like to understand why
>>> gkrellm's needs differ from the 3 applications I've ported myself.
>> Well it doesn't need it, but having it would make the code cleaner.
> 
> Well, if you already have a switch/case and store the subfeature
> numbers, then a shortcut to get the input subfeature number doesn't save
> that much, neither in terms of code nor in terms of speed. I don't
> think it's worth adding.
> 

Okay, thats fine with me.

>>>> Besides that adding a define to sensors.h which can be tested to find out 
>>>> against which libsensors version code is compiling would be a good idea too.
>>> As the underlying build system needs to know what library will be used
>>> (for proper linkage),
>> No it doesn't -lsensors will work for both linsensors3 and libsensors4 actually 
>> the ./configure check for libsensors in gkrellm works with both without 
>> modification (it checks for sensors/sensors.h && sensors_init()).
> 
> How do you deal with the case where both libsensors.so.3 and
> libsensors.so.4 are available at link time?
> 

Well -lsensors will search for libsensors.so, which is a symlink to one of the 
two, I would expect this symlink to point to the same version as 
/usr/include/sensors/sensors.h is.

When talking distros, the symlinks usually reside in the -devel package. One 
can then make either the 2 -devel pakcages conflict, or put the symlink in a 
subdir of /usr/lib[64] and have a pkg-config file (which are usually versioned) 
and use pkg-config to add the correct -L/..... path to the LD_FLAGS.

This (symlinks in subdirs of /usr/lib, pkg-config) is how many libraries which 
are designed for parallel installs do it. In the case of lm_sensors the plan 
for Feodra is to switch to libsensors4 and just patch all users to move to the 
new API, I much rather spend some time writing patches for libsensors using 
apps, then spend time to make the 2 parallel installable. Also notice that as 
the 2 are inherently not paralell installable as the both want to install 
/usr/include/sensors/sensors.h


>>> I expected applications to deal with the
>>> different versions on their own. But if you think that a #define in
>>> sensors.h would help, just go ahead, that's fine with me.
>> Well as explained above ./configure and the Makefiles don't need to know about 
>> the different versions, So only some code changes are needed, for which it 
>> would be nice if a #define is present to test for.
> 
> As I said, just go ahead and add the #define you want. Feel free to add
> the "same" #define to trunk if it helps.
> 

Will do.

Regards,

Hans

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