[lm-sensors] Hacking on libsensors

[lm-sensors] Hacking on libsensors

[Jean Delvare]

Hi all,

My employer, Novell, has organized a "hack week". This is one week where
every employee is free to hack on whatever he/she wants [1].
Unsurprisingly I decided I would spend that time on libsensors. I
posted the idea here:
http://idea.opensuse.org/content/ideas/generic-sensors-library

I already made preliminary cleanups yesterday and this morning.
Tomorrow, serious things will begin. I thought I'd mention my plans
here, so that people get a chance to object before I commit my changes.

First of all, I want to remove all chip-specific code from both
libsensors and sensors. I think we all agree that it will ultimately go
away, and removing it from the way right now will make it easier to
focus on the new interface we want to expose. This will break sensord
(and all 3rd party libsensors users), but I don't really care. sensord
is not built by default, and we can fix it later. 3rd party apps will
need to be fixed later too.

Secondly, the sensors command-line interface. I would like to get rid
of -u (Treat chips as unknown ones) and -g (Use generic printing
routine for all chips), because they will be the default (and only way)
with the new libsensors interface.

I am also thinking of getting rid of -U (Do not show unknown chips),
because with the hwmon class, such chips can't be reached to start
with. However this raises the question of which kernel versions we want
the new libsensors (and the lm-sensors package in general) to support.
I don't think we want to support anything before 2.6.5 because the
sysfs interface changed too much before this version, and i2c had some
problems too (e.g. i2c-dev numbers not in sync with i2c-adapter
numbers.) But if we only support hwmon class access, this means we only
support kernels 2.6.14 and later. This is perfectly fine with me, but
I'm curious if others will object.

Then I'll be inspecting the libsensors interface as it is now. This is
the single most important thing to look at in order to be able to
release a new libsensors quickly. The rest can be improved later, but
the interface will be pretty much carved in stone for the next 8 years
(assuming the new library lasts as long as the previous one did, and I
hope so) once we release the first public version. Everyone, please
realize that the new interface will not be compatible with the old one,
and the new libsensors will get a new .so number. This means that this
is the perfect time to make all the cleanups and changes we can't do in
a stable library. It's basically now or never.

In parallel, I want to write a perl script to convert libsensors
configuration files from the old symbol names to the new ones. And move
the i2c tools to a separate package, as mentioned yesterday.

[1] http://idea.opensuse.org/content/hackweek

-- 
Jean Delvare

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

Re: [lm-sensors] Hacking on libsensors

[Hans de Goede]

Jean Delvare wrote:
> Hi all,
> 
> My employer, Novell, has organized a "hack week". This is one week where
> every employee is free to hack on whatever he/she wants [1].
> Unsurprisingly I decided I would spend that time on libsensors. I
> posted the idea here:
> http://idea.opensuse.org/content/ideas/generic-sensors-library
> 


It sounds like an excellent plan to start working on the 3.0 branch! Let me 
know if you need any help, and/or when you want me to test this on the machines 
I have access too.

Regards,

Hans

p.s.

Some time ago we had a discussion about amongst other things some bad 
assumptions in the generic voltage printing code. I promised to fix this, but 
its still on my todo, I will happily fix this once things stabilize (I would 
like to avoid svn commit conflicts), if you feel inclined to fix it yourself 
thats fine too, here is the important part of our previous conversation on this:

 > Also print_generic_chip_in(), has some dangerous and unneeded assumptions, for
 > example:
 > -it assumes that if there is a inX_max that there will also always be a inX_min
 > -it assumes that if there is no inX_max there will also be no alarms
 >
Somewhat less "unneeded" assumption
 > -it doesn't check for inX_max_alarm without an inX_min_alarm and visa versa, I
 >   admit this would be a driver bug, as then the driver should have just a
 >   inX_alarm, but still we might want to concider this.



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

Re: [lm-sensors] Hacking on libsensors

[Jean Delvare]

Hi Hans,

On Wed, 27 Jun 2007 17:19:56 +0200, Hans de Goede wrote:
> Jean Delvare wrote:
> > Hi all,
> > 
> > My employer, Novell, has organized a "hack week". This is one week where
> > every employee is free to hack on whatever he/she wants [1].
> > Unsurprisingly I decided I would spend that time on libsensors. I
> > posted the idea here:
> > http://idea.opensuse.org/content/ideas/generic-sensors-library
> 
> It sounds like an excellent plan to start working on the 3.0 branch! Let me 
> know if you need any help, and/or when you want me to test this on the machines 
> I have access too.

I'll let you know. However, I fear that I've been spending too much
time on the cleanup steps. 3 days have passed already, only 2 left, and
I still didn't have a look at the libsensors interface. Not to say that
the work I've done wasn't needed, it certainly was, but I don't think
there will be much to test at the end of this week. I'll have to find
more time to keep working on it in the next few weeks.

> Some time ago we had a discussion about amongst other things some bad 
> assumptions in the generic voltage printing code. I promised to fix this, but 
> its still on my todo, I will happily fix this once things stabilize (I would 
> like to avoid svn commit conflicts), if you feel inclined to fix it yourself 
> thats fine too, here is the important part of our previous conversation on this:
> 
>  > Also print_generic_chip_in(), has some dangerous and unneeded assumptions, for
>  > example:
>  > -it assumes that if there is a inX_max that there will also always be a inX_min
>  > -it assumes that if there is no inX_max there will also be no alarms
>  >
> Somewhat less "unneeded" assumption
>  > -it doesn't check for inX_max_alarm without an inX_min_alarm and visa versa, I
>  >   admit this would be a driver bug, as then the driver should have just a
>  >   inX_alarm, but still we might want to concider this.

Please commit your changes as soon as possible. Don't worry about
conflicts, I won't touch this part of the code before you've committed
your own changes.

Thanks,
-- 
Jean Delvare

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

Re: [lm-sensors] Hacking on libsensors

[Jean Delvare]

On Wed, 27 Jun 2007 21:02:44 +0200, Jean Delvare wrote:
> On Wed, 27 Jun 2007 17:19:56 +0200, Hans de Goede wrote:
> > It sounds like an excellent plan to start working on the 3.0 branch! Let me 
> > know if you need any help, and/or when you want me to test this on the machines 
> > I have access too.
> 
> I'll let you know. However, I fear that I've been spending too much
> time on the cleanup steps. 3 days have passed already, only 2 left, and
> I still didn't have a look at the libsensors interface. Not to say that
> the work I've done wasn't needed, it certainly was, but I don't think
> there will be much to test at the end of this week. I'll have to find
> more time to keep working on it in the next few weeks.

OK, I've managed to make some significant progress today. All the
chip-specific code is gone:
http://www.lm-sensors.org/changeset/4503
http://www.lm-sensors.org/changeset/4506
http://www.lm-sensors.org/changeset/4507

Then I've unified the sensors_proc_chips and sensors_chip_features_list
arrays so the code handling the chip information gathered from sysfs is
more simple and more efficient, memory-wise:
http://www.lm-sensors.org/changeset/4508
http://www.lm-sensors.org/changeset/4510

And lastly, the parallel feature names and magnitudes are gone:
http://www.lm-sensors.org/changeset/4513

Needless to say, the size of the libsensors, in terms of both number of
lines of code, and binary size, went down drastically:
-rwxr-xr-x 1 root root 342310 jun 25 13:34 libsensors.so.3.1.3
-rwxr-xr-x 1 root root  73206 jun 28 22:59 libsensors.so.4.0.0

And same goes for sensors itself:
-rwxr-xr-x 1 root root 153255 jun 25 13:34 sensors
-rwxr-xr-x 1 root root  26535 jun 28 22:59 sensors3

But I've also hit a minor problem, which I'm an not sure how to solve.
The *_input sysfs files are handled differently in libsensors, because
they act as the main feature for each channel, so they are referenced
by the channel name instead of the sysfs file name (e.g. temp1 instead
of temp1_input). For all the other values, the symbol name is the same
as the sysfs file name. For now I've handled it with a hack in
sensors_read_proc() and sensors_write_proc() so that the right file is
accessed, but this isn't clean, and not very efficient either.

I am wondering if we shouldn't introduce the concept of channel in the
new libsensors. These would be kind of group leaders as far as feature
mappings are concerned, and they would get the label and compute
statements too. Group leaders would be special in that they wouldn't
correspond to a sysfs file. This would make things cleaner, and there
may be some room for performance and memory consumption improvements
too if we go that way. But it would also add some complexity. I need to
think about it some more, for now it's only an idea - and not one I can
implement this week, anyway. Comments are welcome.

-- 
Jean Delvare

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

Re: [lm-sensors] Hacking on libsensors

[Hans de Goede]

Jean Delvare wrote:
> Hi Hans,
> 
> Please commit your changes as soon as possible. Don't worry about
> conflicts, I won't touch this part of the code before you've committed
> your own changes.
> 

Done.

Regards,

Hans

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

Re: [lm-sensors] Hacking on libsensors

[Jean Delvare]

On Thu, 28 Jun 2007 23:17:17 +0200, Hans de Goede wrote:
> Jean Delvare wrote:
> > Hi Hans,
> > 
> > Please commit your changes as soon as possible. Don't worry about
> > conflicts, I won't touch this part of the code before you've committed
> > your own changes.
> 
> Done.

Looks good, thanks.

-- 
Jean Delvare

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