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
