All of lore.kernel.org
 help / color / mirror / Atom feed
* [lm-sensors] libsensors API changes
@ 2007-09-19 21:44 Jean Delvare
  2007-09-21  7:24 ` Hans de Goede
  2007-09-23 21:18 ` Jean Delvare
  0 siblings, 2 replies; 3+ messages in thread
From: Jean Delvare @ 2007-09-19 21:44 UTC (permalink / raw)
  To: lm-sensors

Hi all,

In the past few days, I have been thinking about the libsensors4 API,
in particular the way the chip features are accessed. Right now we have
a single function, sensors_get_all_features(), returning all features
in sequence, mixing main features and subfeatures. Actually, main
features and subfeatures are only differentiated by the mapping field
value, but otherwise they are represented by the same structure.

Looking at the code of several user-space applications using
libsensors, this interface is not very convenient. Except for "sensors
-u", which makes some sense as I think it is the reason why this
interface exists in the first place. Here is a list of things I found
the applications would take benefit of if libsensors would propose a
suitable interface:

* Separate lists for main features and subfeatures of each main feature.

* Subfeature existence information (does feature F of chip C have
subfeature X?)

* Direct subfeature value lookup (what's the value of subfeature X of
feature F of chip C?)

So I am in the process of extending the libsensors4 API to implement
these interfaces. The first point can be either done easily (simply
splitting sensors_get_all_features() to two separate functions) or
taken further: I think it would make sense to have a separate structure
for main features or "channels" (e.g. in0, temp1...) and for
subfeatures (e.g. in0_input, temp1_min...) This would make things much
more structured. For example, labels essentially apply to main
features, not subfeatures. Same goes for compute statements. OTOH, set
statements apply to subfeatures, not main features.

Going that route would speed up the library code at some places. It
would also let us clear the fooN / fooN_input confusion which forces us
to have some ugly hard-coded exceptions in the library. The drawback is
that we may lose some flexibility by going that route, so I'm not sure
how far we want to go.

The last two points would avoid some duplication in applications, and
would make their code look much nicer. There is a concern with
algorithmic complexity though: direct value lookups are likely to be in
O(N) while for example "sensors" manages to do it in O(1) at the
moment. However, the number of subfeatures being relatively small for
each main feature, it might not be an issue at all.

I am working on some patches implementing these API changes, I hope to
be able to post them for comments and review at the end of the week.
Until then, I would also welcome comments on the general ideas I just
exposed.

Thanks,
-- 
Jean Delvare

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

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [lm-sensors] libsensors API changes
  2007-09-19 21:44 [lm-sensors] libsensors API changes Jean Delvare
@ 2007-09-21  7:24 ` Hans de Goede
  2007-09-23 21:18 ` Jean Delvare
  1 sibling, 0 replies; 3+ messages in thread
From: Hans de Goede @ 2007-09-21  7:24 UTC (permalink / raw)
  To: lm-sensors

Jean Delvare wrote:
> Hi all,
> 
> In the past few days, I have been thinking about the libsensors4 API,
> in particular the way the chip features are accessed. Right now we have
> a single function, sensors_get_all_features(), returning all features
> in sequence, mixing main features and subfeatures. Actually, main
> features and subfeatures are only differentiated by the mapping field
> value, but otherwise they are represented by the same structure.
> 
> Looking at the code of several user-space applications using
> libsensors, this interface is not very convenient. Except for "sensors
> -u", which makes some sense as I think it is the reason why this
> interface exists in the first place. Here is a list of things I found
> the applications would take benefit of if libsensors would propose a
> suitable interface:
> 
> * Separate lists for main features and subfeatures of each main feature.
> 

+1

> * Subfeature existence information (does feature F of chip C have
> subfeature X?)
> 

+1

> * Direct subfeature value lookup (what's the value of subfeature X of
> feature F of chip C?)
> 

+1

> So I am in the process of extending the libsensors4 API to implement
> these interfaces. The first point can be either done easily (simply
> splitting sensors_get_all_features() to two separate functions) or
> taken further: I think it would make sense to have a separate structure
> for main features or "channels" (e.g. in0, temp1...) and for
> subfeatures (e.g. in0_input, temp1_min...) This would make things much
> more structured. For example, labels essentially apply to main
> features, not subfeatures. Same goes for compute statements. OTOH, set
> statements apply to subfeatures, not main features.
> 
> Going that route would speed up the library code at some places. It
> would also let us clear the fooN / fooN_input confusion which forces us
> to have some ugly hard-coded exceptions in the library. The drawback is
> that we may lose some flexibility by going that route, so I'm not sure
> how far we want to go.
> 
> The last two points would avoid some duplication in applications, and
> would make their code look much nicer. There is a concern with
> algorithmic complexity though: direct value lookups are likely to be in
> O(N) while for example "sensors" manages to do it in O(1) at the
> moment. However, the number of subfeatures being relatively small for
> each main feature, it might not be an issue at all.
> 
> I am working on some patches implementing these API changes, I hope to
> be able to post them for comments and review at the end of the week.
> Until then, I would also welcome comments on the general ideas I just
> exposed.
> 


The basic ideas I'm vary much in favor off, as for the under the hood 
implementation, I haven't given much thought to that, I trust you will come up 
with something good.

Regards,

Hans


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

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [lm-sensors] libsensors API changes
  2007-09-19 21:44 [lm-sensors] libsensors API changes Jean Delvare
  2007-09-21  7:24 ` Hans de Goede
@ 2007-09-23 21:18 ` Jean Delvare
  1 sibling, 0 replies; 3+ messages in thread
From: Jean Delvare @ 2007-09-23 21:18 UTC (permalink / raw)
  To: lm-sensors

Hi Hans,

On Fri, 21 Sep 2007 09:24:00 +0200, Hans de Goede wrote:
> Jean Delvare wrote:
> > Looking at the code of several user-space applications using
> > libsensors, this interface is not very convenient. Except for "sensors
> > -u", which makes some sense as I think it is the reason why this
> > interface exists in the first place. Here is a list of things I found
> > the applications would take benefit of if libsensors would propose a
> > suitable interface:
> > 
> > * Separate lists for main features and subfeatures of each main feature.
> > 
> 
> +1

Implemented in r4831. We now have:

/* This returns all main features of a specific chip. nr is an internally
   used variable. Set it to zero to start at the begin of the list. If no
   more features are found NULL is returned.
   Do not try to change the returned structure; you will corrupt internal
   data structures. */
const sensors_feature *
sensors_get_features(const sensors_chip_name *name, int *nr);

/* This returns all subfeatures of a given main feature. nr is an internally
   used variable. Set it to zero to start at the begin of the list. If no
   more features are found NULL is returned.
   Do not try to change the returned structure; you will corrupt internal
   data structures. */
const sensors_subfeature *
sensors_get_all_subfeatures(const sensors_chip_name *name,
			    const sensors_feature *feature, int *nr);


> > * Subfeature existence information (does feature F of chip C have
> > subfeature X?)
> > 
> 
> +1
> 
> > * Direct subfeature value lookup (what's the value of subfeature X of
> > feature F of chip C?)
> > 
> 
> +1

Both implemented in r4846:

/* This returns the subfeature of the given type for a given main feature,
   if it exists, NULL otherwise.
   Do not try to change the returned structure; you will corrupt internal
   data structures. */
const sensors_subfeature *
sensors_get_subfeature(const sensors_chip_name *name,
		       const sensors_feature *feature,
		       sensors_subfeature_type type);

> (...)
> > I am working on some patches implementing these API changes, I hope to
> > be able to post them for comments and review at the end of the week.
> > Until then, I would also welcome comments on the general ideas I just
> > exposed.
> 
> The basic ideas I'm vary much in favor off, as for the under the hood 
> implementation, I haven't given much thought to that, I trust you will come up 
> with something good.

I've committed all my patches now, so that we don't lose too much time.
Still, if you or anyone else has objections to what I did, please speak
up and we'll address them.

I'm done with the libsensors API review. I've done my best to make it
more convenient to use. I've also made quite a lot of performance
improvements. I sincerely hope that this new library will work for as
long as its predecessor did.

This basically means that we are next to the point where we can release
lm-sensors 3.0.0! My plan is to release a RC1 next week, and give
application authors some time to port their application to the new API
and comment on it. Or we can try porting them on our own and see how it
goes. I've ported xsensors already. It would be good to have all
popular sensor apps ported quickly so as to make sure that the new API
fits the needs. If you need help with a port, just ask me.

-- 
Jean Delvare

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

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2007-09-23 21:18 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2007-09-19 21:44 [lm-sensors] libsensors API changes Jean Delvare
2007-09-21  7:24 ` Hans de Goede
2007-09-23 21:18 ` Jean Delvare

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.